From bf269e634caee2022e99189de8337bd7fcd8f0e7 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 11 Jul 2022 16:43:48 +0200 Subject: podman machine: do not commit proxies into config file qemu fails when the same `fw_cfg` options is used more than once. Since the current logic always adds a new option on each machine load this will fail on the second start. We can fix this by checking if the option is already set and replace but I think it is easier to just not commit the option in the config and add it dynamically on start. User that hit this bug have to recreate the machine. [NO NEW TESTS NEEDED] Fixes #14636 Fixes #14837 Signed-off-by: Paul Holzinger --- pkg/machine/qemu/machine.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'pkg') diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 6134e69e1..3b57455c4 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -240,20 +240,6 @@ func (p *Provider) LoadVMByName(name string) (machine.VM, error) { return nil, err } - // It is here for providing the ability to propagate - // proxy settings (e.g. HTTP_PROXY and others) on a start - // and avoid a need of re-creating/re-initiating a VM - if proxyOpts := machine.GetProxyVariables(); len(proxyOpts) > 0 { - proxyStr := "name=opt/com.coreos/environment,string=" - var proxies string - for k, v := range proxyOpts { - proxies = fmt.Sprintf("%s%s=\"%s\"|", proxies, k, v) - } - proxyStr = fmt.Sprintf("%s%s", proxyStr, base64.StdEncoding.EncodeToString([]byte(proxies))) - vm.CmdLine = append(vm.CmdLine, "-fw_cfg", proxyStr) - } - - logrus.Debug(vm.CmdLine) return vm, nil } @@ -573,15 +559,29 @@ func (v *MachineVM) Start(name string, _ machine.StartOptions) error { attr := new(os.ProcAttr) files := []*os.File{dnr, dnw, dnw, fd} attr.Files = files - logrus.Debug(v.CmdLine) cmdLine := v.CmdLine + // It is here for providing the ability to propagate + // proxy settings (e.g. HTTP_PROXY and others) on a start + // and avoid a need of re-creating/re-initiating a VM + if proxyOpts := machine.GetProxyVariables(); len(proxyOpts) > 0 { + proxyStr := "name=opt/com.coreos/environment,string=" + var proxies string + for k, v := range proxyOpts { + proxies = fmt.Sprintf("%s%s=\"%s\"|", proxies, k, v) + } + proxyStr = fmt.Sprintf("%s%s", proxyStr, base64.StdEncoding.EncodeToString([]byte(proxies))) + cmdLine = append(cmdLine, "-fw_cfg", proxyStr) + } + // Disable graphic window when not in debug mode // Done in start, so we're not suck with the debug level we used on init if !logrus.IsLevelEnabled(logrus.DebugLevel) { cmdLine = append(cmdLine, "-display", "none") } + logrus.Debugf("qemu cmd: %v", cmdLine) + stderrBuf := &bytes.Buffer{} cmd := &exec.Cmd{ -- cgit v1.2.3-54-g00ecf From 7b4ebfa657d605d0767be7ccb2607c923ce4311f Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 12 Jul 2022 11:47:30 +0200 Subject: podman: always call into SetupRootless Signed-off-by: Giuseppe Scrivano --- cmd/podman/common/completion.go | 3 +-- cmd/podman/root.go | 3 +-- pkg/domain/infra/abi/system.go | 4 ++++ 3 files changed, 6 insertions(+), 4 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go index 6e6c33f9b..02369c74a 100644 --- a/cmd/podman/common/completion.go +++ b/cmd/podman/common/completion.go @@ -18,7 +18,6 @@ import ( "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/libpod/events" "github.com/containers/podman/v4/pkg/domain/entities" - "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/signal" systemdDefine "github.com/containers/podman/v4/pkg/systemd/define" "github.com/containers/podman/v4/pkg/util" @@ -54,7 +53,7 @@ func setupContainerEngine(cmd *cobra.Command) (entities.ContainerEngine, error) cobra.CompErrorln(err.Error()) return nil, err } - if !registry.IsRemote() && rootless.IsRootless() { + if !registry.IsRemote() { _, noMoveProcess := cmd.Annotations[registry.NoMoveProcess] err := containerEngine.SetupRootless(registry.Context(), noMoveProcess) diff --git a/cmd/podman/root.go b/cmd/podman/root.go index f28d92e2f..0520a0784 100644 --- a/cmd/podman/root.go +++ b/cmd/podman/root.go @@ -19,7 +19,6 @@ import ( "github.com/containers/podman/v4/pkg/checkpoint/crutils" "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/parallel" - "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/version" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -265,7 +264,7 @@ func persistentPreRunE(cmd *cobra.Command, args []string) error { // 2) running as non-root // 3) command doesn't require Parent Namespace _, found := cmd.Annotations[registry.ParentNSRequired] - if !registry.IsRemote() && rootless.IsRootless() && !found { + if !registry.IsRemote() && !found { _, noMoveProcess := cmd.Annotations[registry.NoMoveProcess] err := registry.ContainerEngine().SetupRootless(registry.Context(), noMoveProcess) if err != nil { diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index 0faae01c8..eed80dd79 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -67,6 +67,10 @@ func (ic *ContainerEngine) Info(ctx context.Context) (*define.Info, error) { } func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) error { + if !rootless.IsRootless() { + return nil + } + // do it only after podman has already re-execed and running with uid==0. hasCapSysAdmin, err := unshare.HasCapSysAdmin() if err != nil { -- cgit v1.2.3-54-g00ecf From 35dc17bfe619444471317d067325d6f92752dc8f Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 12 Jul 2022 11:12:27 +0200 Subject: abi: create new cgroup when running in a container if podman is running in the root cgroup, it will create a new subcgroup and move itself there. [NO NEW TESTS NEEDED] it needs nested podman Closes: https://github.com/containers/podman/issues/14884 Signed-off-by: Giuseppe Scrivano --- pkg/domain/infra/abi/system.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index eed80dd79..3389abd88 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -67,6 +67,18 @@ func (ic *ContainerEngine) Info(ctx context.Context) (*define.Info, error) { } func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) error { + runsUnderSystemd := utils.RunsOnSystemd() + if !runsUnderSystemd { + isPid1 := os.Getpid() == 1 + if _, found := os.LookupEnv("container"); isPid1 || found { + if err := utils.MaybeMoveToSubCgroup(); err != nil { + // it is a best effort operation, so just print the + // error for debugging purposes. + logrus.Debugf("Could not move to subcgroup: %v", err) + } + } + } + if !rootless.IsRootless() { return nil } @@ -86,7 +98,6 @@ func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) if err != nil { return err } - runsUnderSystemd := utils.RunsOnSystemd() unitName := fmt.Sprintf("podman-%d.scope", os.Getpid()) if runsUnderSystemd || conf.Engine.CgroupManager == config.SystemdCgroupsManager { if err := utils.RunUnderSystemdScope(os.Getpid(), "user.slice", unitName); err != nil { -- cgit v1.2.3-54-g00ecf From f7707c81a0ff91364541f3be600a970fd1b59476 Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Mon, 11 Jul 2022 12:31:02 -0500 Subject: Fix last machine test The list --format json test case had a typo like error. [NO NEW TESTS NEEDED] Signed-off-by: Brent Baude --- pkg/machine/e2e/list_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/machine/e2e/list_test.go b/pkg/machine/e2e/list_test.go index fb855c61e..8b7443d47 100644 --- a/pkg/machine/e2e/list_test.go +++ b/pkg/machine/e2e/list_test.go @@ -135,7 +135,7 @@ var _ = Describe("podman machine list", func() { Expect(listSession2).To(Exit(0)) var listResponse []*entities.ListReporter - err = jsoniter.Unmarshal(listSession.Bytes(), &listResponse) + err = jsoniter.Unmarshal(listSession2.Bytes(), &listResponse) Expect(err).To(BeNil()) // table format includes the header -- cgit v1.2.3-54-g00ecf From a5827e13cd6168741badd29e95a81b3c992bf301 Mon Sep 17 00:00:00 2001 From: Gerard Braad Date: Wed, 13 Jul 2022 05:36:06 +0000 Subject: Fixes #14698 Use prepared image for WSL2 machine init This commit introduces a prepared image for setting up the WSL2 environment. This means that the deployment will take considerable less time to finish (as it does not need to run an update and package install), but also allows to rely on a cached image to re-init the environment without the need for an internet connection. [NO NEW TESTS NEEDED] Signed-off-by: Gerard Braad --- pkg/machine/fedora.go | 65 ++++++++++------------------------------------ pkg/machine/wsl/machine.go | 28 +++----------------- 2 files changed, 17 insertions(+), 76 deletions(-) (limited to 'pkg') diff --git a/pkg/machine/fedora.go b/pkg/machine/fedora.go index 46e450418..2c6d3d810 100644 --- a/pkg/machine/fedora.go +++ b/pkg/machine/fedora.go @@ -6,29 +6,23 @@ package machine import ( "errors" "fmt" - "io" - "io/ioutil" + "os" + "net/http" "net/url" - "os" "path/filepath" - "regexp" - - "github.com/sirupsen/logrus" ) const ( - githubURL = "http://github.com/fedora-cloud/docker-brew-fedora/" + githubLatestReleaseURL = "https://github.com/containers/podman-wsl-fedora/releases/latest/download/rootfs.tar.xz" ) -var fedoraxzRegex = regexp.MustCompile(`fedora[^\"]+xz`) - type FedoraDownload struct { Download } func NewFedoraDownloader(vmType, vmName, releaseStream string) (DistributionDownload, error) { - imageName, downloadURL, size, err := getFedoraDownload(releaseStream) + downloadURL, size, err := getFedoraDownload(githubLatestReleaseURL) if err != nil { return nil, err } @@ -38,6 +32,8 @@ func NewFedoraDownloader(vmType, vmName, releaseStream string) (DistributionDown return nil, err } + imageName := "rootfs.tar.xz" + f := FedoraDownload{ Download: Download{ Arch: getFcosArch(), @@ -69,56 +65,21 @@ func (f FedoraDownload) HasUsableCache() (bool, error) { return info.Size() == f.Size, nil } -func truncRead(url string) ([]byte, error) { - resp, err := http.Get(url) - if err != nil { - return nil, err - } - - defer func() { - if err := resp.Body.Close(); err != nil { - logrus.Error(err) - } - }() - - body, err := ioutil.ReadAll(io.LimitReader(resp.Body, 10*1024*1024)) - if err != nil { - return nil, err - } - - _, _ = io.Copy(io.Discard, resp.Body) - - return body, nil -} - -func getFedoraDownload(releaseStream string) (string, *url.URL, int64, error) { - dirURL := githubURL + "tree/" + releaseStream + "/" + getFcosArch() + "/" - body, err := truncRead(dirURL) - if err != nil { - return "", nil, -1, err - } - - file := fedoraxzRegex.FindString(string(body)) - if len(file) == 0 { - return "", nil, -1, fmt.Errorf("could not locate Fedora download at %s", dirURL) - } - - rawURL := githubURL + "raw/" + releaseStream + "/" + getFcosArch() + "/" - newLocation := rawURL + file - downloadURL, err := url.Parse(newLocation) +func getFedoraDownload(releaseURL string) (*url.URL, int64, error) { + downloadURL, err := url.Parse(releaseURL) if err != nil { - return "", nil, -1, fmt.Errorf("invalid URL generated from discovered Fedora file: %s: %w", newLocation, err) + return nil, -1, fmt.Errorf("invalid URL generated from discovered Fedora file: %s: %w", releaseURL, err) } - resp, err := http.Head(newLocation) + resp, err := http.Head(releaseURL) if err != nil { - return "", nil, -1, fmt.Errorf("head request failed: %s: %w", newLocation, err) + return nil, -1, fmt.Errorf("head request failed: %s: %w", releaseURL, err) } _ = resp.Body.Close() if resp.StatusCode != http.StatusOK { - return "", nil, -1, fmt.Errorf("head request failed [%d] on download: %s", resp.StatusCode, newLocation) + return nil, -1, fmt.Errorf("head request failed: %s: %w", releaseURL, err) } - return file, downloadURL, resp.ContentLength, nil + return downloadURL, resp.ContentLength, nil } diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index 450d8b877..189723ac7 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -386,7 +386,7 @@ func downloadDistro(v *MachineVM, opts machine.InitOptions) error { if _, e := strconv.Atoi(opts.ImagePath); e == nil { v.ImageStream = opts.ImagePath - dd, err = machine.NewFedoraDownloader(vmtype, v.Name, v.ImageStream) + dd, err = machine.NewFedoraDownloader(vmtype, v.Name, opts.ImagePath) } else { v.ImageStream = "custom" dd, err = machine.NewGenericDownloader(vmtype, v.Name, opts.ImagePath) @@ -449,34 +449,14 @@ func provisionWSLDist(v *MachineVM) (string, error) { } dist := toDist(v.Name) - fmt.Println("Importing operating system into WSL (this may take 5+ minutes on a new WSL install)...") + fmt.Println("Importing operating system into WSL (this may take a few minutes on a new WSL install)...") if err = runCmdPassThrough("wsl", "--import", dist, distTarget, v.ImagePath); err != nil { return "", fmt.Errorf("the WSL import of guest OS failed: %w", err) } - fmt.Println("Installing packages (this will take awhile)...") - if err = runCmdPassThrough("wsl", "-d", dist, "dnf", "upgrade", "-y"); err != nil { - return "", fmt.Errorf("package upgrade on guest OS failed: %w", err) - } - - fmt.Println("Enabling Copr") - if err = runCmdPassThrough("wsl", "-d", dist, "dnf", "install", "-y", "'dnf-command(copr)'"); err != nil { - return "", fmt.Errorf("enabling copr failed: %w", err) - } - - fmt.Println("Enabling podman4 repo") - if err = runCmdPassThrough("wsl", "-d", dist, "dnf", "-y", "copr", "enable", "rhcontainerbot/podman4"); err != nil { - return "", fmt.Errorf("enabling copr failed: %w", err) - } - - if err = runCmdPassThrough("wsl", "-d", dist, "dnf", "install", - "podman", "podman-docker", "openssh-server", "procps-ng", "-y"); err != nil { - return "", fmt.Errorf("package installation on guest OS failed: %w", err) - } - // Fixes newuidmap - if err = runCmdPassThrough("wsl", "-d", dist, "dnf", "reinstall", "shadow-utils", "-y"); err != nil { - return "", fmt.Errorf("package reinstallation of shadow-utils on guest OS failed: %w", err) + if err = runCmdPassThrough("wsl", "-d", dist, "rpm", "-q", "--restore", "shadow-utils", "2>/dev/null"); err != nil { + return "", fmt.Errorf("package permissions restore of shadow-utils on guest OS failed: %w", err) } // Windows 11 (NT Version = 10, Build 22000) generates harmless but scary messages on every -- cgit v1.2.3-54-g00ecf From a78be890ee0098c6a6809b562c4806da8fe344b5 Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Thu, 14 Jul 2022 13:33:23 +0200 Subject: Switch to `github.com/blang/semver/v4` Switch to the latest version of the now go module compatible release. [NO NEW TESTS NEEDED] Signed-off-by: Sascha Grunert --- go.mod | 2 +- go.sum | 2 + pkg/api/handlers/utils/handler.go | 2 +- pkg/bindings/connection.go | 2 +- vendor/github.com/blang/semver/v4/LICENSE | 22 ++ vendor/github.com/blang/semver/v4/go.mod | 3 + vendor/github.com/blang/semver/v4/json.go | 23 ++ vendor/github.com/blang/semver/v4/range.go | 416 ++++++++++++++++++++++++ vendor/github.com/blang/semver/v4/semver.go | 476 ++++++++++++++++++++++++++++ vendor/github.com/blang/semver/v4/sort.go | 28 ++ vendor/github.com/blang/semver/v4/sql.go | 30 ++ vendor/modules.txt | 4 +- version/version.go | 2 +- 13 files changed, 1007 insertions(+), 5 deletions(-) create mode 100644 vendor/github.com/blang/semver/v4/LICENSE create mode 100644 vendor/github.com/blang/semver/v4/go.mod create mode 100644 vendor/github.com/blang/semver/v4/json.go create mode 100644 vendor/github.com/blang/semver/v4/range.go create mode 100644 vendor/github.com/blang/semver/v4/semver.go create mode 100644 vendor/github.com/blang/semver/v4/sort.go create mode 100644 vendor/github.com/blang/semver/v4/sql.go (limited to 'pkg') diff --git a/go.mod b/go.mod index bc39c9cf3..e9f2ce343 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.16 require ( github.com/BurntSushi/toml v1.1.0 - github.com/blang/semver v3.5.1+incompatible + github.com/blang/semver/v4 v4.0.0 github.com/buger/goterm v1.0.4 github.com/checkpoint-restore/checkpointctl v0.0.0-20220321135231-33f4a66335f0 github.com/checkpoint-restore/go-criu/v5 v5.3.0 diff --git a/go.sum b/go.sum index 13056f853..f31cebeb8 100644 --- a/go.sum +++ b/go.sum @@ -171,6 +171,8 @@ github.com/blang/semver v3.1.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnweb github.com/blang/semver v3.5.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdnnjpJbkM4JQ= github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= +github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= +github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/blizzy78/varnamelen v0.3.0/go.mod h1:hbwRdBvoBqxk34XyQ6HA0UH3G0/1TKuv5AC4eaBT0Ec= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4= github.com/bombsimon/wsl/v3 v3.3.0/go.mod h1:st10JtZYLE4D5sC7b8xV4zTKZwAQjCH/Hy2Pm1FNZIc= diff --git a/pkg/api/handlers/utils/handler.go b/pkg/api/handlers/utils/handler.go index 9562ebbbc..f2f8ab1dc 100644 --- a/pkg/api/handlers/utils/handler.go +++ b/pkg/api/handlers/utils/handler.go @@ -10,7 +10,7 @@ import ( "strings" "unsafe" - "github.com/blang/semver" + "github.com/blang/semver/v4" "github.com/containers/podman/v4/version" "github.com/gorilla/mux" jsoniter "github.com/json-iterator/go" diff --git a/pkg/bindings/connection.go b/pkg/bindings/connection.go index 7dda955a2..b994a5857 100644 --- a/pkg/bindings/connection.go +++ b/pkg/bindings/connection.go @@ -13,7 +13,7 @@ import ( "strings" "time" - "github.com/blang/semver" + "github.com/blang/semver/v4" "github.com/containers/podman/v4/pkg/terminal" "github.com/containers/podman/v4/version" "github.com/sirupsen/logrus" diff --git a/vendor/github.com/blang/semver/v4/LICENSE b/vendor/github.com/blang/semver/v4/LICENSE new file mode 100644 index 000000000..5ba5c86fc --- /dev/null +++ b/vendor/github.com/blang/semver/v4/LICENSE @@ -0,0 +1,22 @@ +The MIT License + +Copyright (c) 2014 Benedikt Lang + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. + diff --git a/vendor/github.com/blang/semver/v4/go.mod b/vendor/github.com/blang/semver/v4/go.mod new file mode 100644 index 000000000..06d262218 --- /dev/null +++ b/vendor/github.com/blang/semver/v4/go.mod @@ -0,0 +1,3 @@ +module github.com/blang/semver/v4 + +go 1.14 diff --git a/vendor/github.com/blang/semver/v4/json.go b/vendor/github.com/blang/semver/v4/json.go new file mode 100644 index 000000000..a74bf7c44 --- /dev/null +++ b/vendor/github.com/blang/semver/v4/json.go @@ -0,0 +1,23 @@ +package semver + +import ( + "encoding/json" +) + +// MarshalJSON implements the encoding/json.Marshaler interface. +func (v Version) MarshalJSON() ([]byte, error) { + return json.Marshal(v.String()) +} + +// UnmarshalJSON implements the encoding/json.Unmarshaler interface. +func (v *Version) UnmarshalJSON(data []byte) (err error) { + var versionString string + + if err = json.Unmarshal(data, &versionString); err != nil { + return + } + + *v, err = Parse(versionString) + + return +} diff --git a/vendor/github.com/blang/semver/v4/range.go b/vendor/github.com/blang/semver/v4/range.go new file mode 100644 index 000000000..95f7139b9 --- /dev/null +++ b/vendor/github.com/blang/semver/v4/range.go @@ -0,0 +1,416 @@ +package semver + +import ( + "fmt" + "strconv" + "strings" + "unicode" +) + +type wildcardType int + +const ( + noneWildcard wildcardType = iota + majorWildcard wildcardType = 1 + minorWildcard wildcardType = 2 + patchWildcard wildcardType = 3 +) + +func wildcardTypefromInt(i int) wildcardType { + switch i { + case 1: + return majorWildcard + case 2: + return minorWildcard + case 3: + return patchWildcard + default: + return noneWildcard + } +} + +type comparator func(Version, Version) bool + +var ( + compEQ comparator = func(v1 Version, v2 Version) bool { + return v1.Compare(v2) == 0 + } + compNE = func(v1 Version, v2 Version) bool { + return v1.Compare(v2) != 0 + } + compGT = func(v1 Version, v2 Version) bool { + return v1.Compare(v2) == 1 + } + compGE = func(v1 Version, v2 Version) bool { + return v1.Compare(v2) >= 0 + } + compLT = func(v1 Version, v2 Version) bool { + return v1.Compare(v2) == -1 + } + compLE = func(v1 Version, v2 Version) bool { + return v1.Compare(v2) <= 0 + } +) + +type versionRange struct { + v Version + c comparator +} + +// rangeFunc creates a Range from the given versionRange. +func (vr *versionRange) rangeFunc() Range { + return Range(func(v Version) bool { + return vr.c(v, vr.v) + }) +} + +// Range represents a range of versions. +// A Range can be used to check if a Version satisfies it: +// +// range, err := semver.ParseRange(">1.0.0 <2.0.0") +// range(semver.MustParse("1.1.1") // returns true +type Range func(Version) bool + +// OR combines the existing Range with another Range using logical OR. +func (rf Range) OR(f Range) Range { + return Range(func(v Version) bool { + return rf(v) || f(v) + }) +} + +// AND combines the existing Range with another Range using logical AND. +func (rf Range) AND(f Range) Range { + return Range(func(v Version) bool { + return rf(v) && f(v) + }) +} + +// ParseRange parses a range and returns a Range. +// If the range could not be parsed an error is returned. +// +// Valid ranges are: +// - "<1.0.0" +// - "<=1.0.0" +// - ">1.0.0" +// - ">=1.0.0" +// - "1.0.0", "=1.0.0", "==1.0.0" +// - "!1.0.0", "!=1.0.0" +// +// A Range can consist of multiple ranges separated by space: +// Ranges can be linked by logical AND: +// - ">1.0.0 <2.0.0" would match between both ranges, so "1.1.1" and "1.8.7" but not "1.0.0" or "2.0.0" +// - ">1.0.0 <3.0.0 !2.0.3-beta.2" would match every version between 1.0.0 and 3.0.0 except 2.0.3-beta.2 +// +// Ranges can also be linked by logical OR: +// - "<2.0.0 || >=3.0.0" would match "1.x.x" and "3.x.x" but not "2.x.x" +// +// AND has a higher precedence than OR. It's not possible to use brackets. +// +// Ranges can be combined by both AND and OR +// +// - `>1.0.0 <2.0.0 || >3.0.0 !4.2.1` would match `1.2.3`, `1.9.9`, `3.1.1`, but not `4.2.1`, `2.1.1` +func ParseRange(s string) (Range, error) { + parts := splitAndTrim(s) + orParts, err := splitORParts(parts) + if err != nil { + return nil, err + } + expandedParts, err := expandWildcardVersion(orParts) + if err != nil { + return nil, err + } + var orFn Range + for _, p := range expandedParts { + var andFn Range + for _, ap := range p { + opStr, vStr, err := splitComparatorVersion(ap) + if err != nil { + return nil, err + } + vr, err := buildVersionRange(opStr, vStr) + if err != nil { + return nil, fmt.Errorf("Could not parse Range %q: %s", ap, err) + } + rf := vr.rangeFunc() + + // Set function + if andFn == nil { + andFn = rf + } else { // Combine with existing function + andFn = andFn.AND(rf) + } + } + if orFn == nil { + orFn = andFn + } else { + orFn = orFn.OR(andFn) + } + + } + return orFn, nil +} + +// splitORParts splits the already cleaned parts by '||'. +// Checks for invalid positions of the operator and returns an +// error if found. +func splitORParts(parts []string) ([][]string, error) { + var ORparts [][]string + last := 0 + for i, p := range parts { + if p == "||" { + if i == 0 { + return nil, fmt.Errorf("First element in range is '||'") + } + ORparts = append(ORparts, parts[last:i]) + last = i + 1 + } + } + if last == len(parts) { + return nil, fmt.Errorf("Last element in range is '||'") + } + ORparts = append(ORparts, parts[last:]) + return ORparts, nil +} + +// buildVersionRange takes a slice of 2: operator and version +// and builds a versionRange, otherwise an error. +func buildVersionRange(opStr, vStr string) (*versionRange, error) { + c := parseComparator(opStr) + if c == nil { + return nil, fmt.Errorf("Could not parse comparator %q in %q", opStr, strings.Join([]string{opStr, vStr}, "")) + } + v, err := Parse(vStr) + if err != nil { + return nil, fmt.Errorf("Could not parse version %q in %q: %s", vStr, strings.Join([]string{opStr, vStr}, ""), err) + } + + return &versionRange{ + v: v, + c: c, + }, nil + +} + +// inArray checks if a byte is contained in an array of bytes +func inArray(s byte, list []byte) bool { + for _, el := range list { + if el == s { + return true + } + } + return false +} + +// splitAndTrim splits a range string by spaces and cleans whitespaces +func splitAndTrim(s string) (result []string) { + last := 0 + var lastChar byte + excludeFromSplit := []byte{'>', '<', '='} + for i := 0; i < len(s); i++ { + if s[i] == ' ' && !inArray(lastChar, excludeFromSplit) { + if last < i-1 { + result = append(result, s[last:i]) + } + last = i + 1 + } else if s[i] != ' ' { + lastChar = s[i] + } + } + if last < len(s)-1 { + result = append(result, s[last:]) + } + + for i, v := range result { + result[i] = strings.Replace(v, " ", "", -1) + } + + // parts := strings.Split(s, " ") + // for _, x := range parts { + // if s := strings.TrimSpace(x); len(s) != 0 { + // result = append(result, s) + // } + // } + return +} + +// splitComparatorVersion splits the comparator from the version. +// Input must be free of leading or trailing spaces. +func splitComparatorVersion(s string) (string, string, error) { + i := strings.IndexFunc(s, unicode.IsDigit) + if i == -1 { + return "", "", fmt.Errorf("Could not get version from string: %q", s) + } + return strings.TrimSpace(s[0:i]), s[i:], nil +} + +// getWildcardType will return the type of wildcard that the +// passed version contains +func getWildcardType(vStr string) wildcardType { + parts := strings.Split(vStr, ".") + nparts := len(parts) + wildcard := parts[nparts-1] + + possibleWildcardType := wildcardTypefromInt(nparts) + if wildcard == "x" { + return possibleWildcardType + } + + return noneWildcard +} + +// createVersionFromWildcard will convert a wildcard version +// into a regular version, replacing 'x's with '0's, handling +// special cases like '1.x.x' and '1.x' +func createVersionFromWildcard(vStr string) string { + // handle 1.x.x + vStr2 := strings.Replace(vStr, ".x.x", ".x", 1) + vStr2 = strings.Replace(vStr2, ".x", ".0", 1) + parts := strings.Split(vStr2, ".") + + // handle 1.x + if len(parts) == 2 { + return vStr2 + ".0" + } + + return vStr2 +} + +// incrementMajorVersion will increment the major version +// of the passed version +func incrementMajorVersion(vStr string) (string, error) { + parts := strings.Split(vStr, ".") + i, err := strconv.Atoi(parts[0]) + if err != nil { + return "", err + } + parts[0] = strconv.Itoa(i + 1) + + return strings.Join(parts, "."), nil +} + +// incrementMajorVersion will increment the minor version +// of the passed version +func incrementMinorVersion(vStr string) (string, error) { + parts := strings.Split(vStr, ".") + i, err := strconv.Atoi(parts[1]) + if err != nil { + return "", err + } + parts[1] = strconv.Itoa(i + 1) + + return strings.Join(parts, "."), nil +} + +// expandWildcardVersion will expand wildcards inside versions +// following these rules: +// +// * when dealing with patch wildcards: +// >= 1.2.x will become >= 1.2.0 +// <= 1.2.x will become < 1.3.0 +// > 1.2.x will become >= 1.3.0 +// < 1.2.x will become < 1.2.0 +// != 1.2.x will become < 1.2.0 >= 1.3.0 +// +// * when dealing with minor wildcards: +// >= 1.x will become >= 1.0.0 +// <= 1.x will become < 2.0.0 +// > 1.x will become >= 2.0.0 +// < 1.0 will become < 1.0.0 +// != 1.x will become < 1.0.0 >= 2.0.0 +// +// * when dealing with wildcards without +// version operator: +// 1.2.x will become >= 1.2.0 < 1.3.0 +// 1.x will become >= 1.0.0 < 2.0.0 +func expandWildcardVersion(parts [][]string) ([][]string, error) { + var expandedParts [][]string + for _, p := range parts { + var newParts []string + for _, ap := range p { + if strings.Contains(ap, "x") { + opStr, vStr, err := splitComparatorVersion(ap) + if err != nil { + return nil, err + } + + versionWildcardType := getWildcardType(vStr) + flatVersion := createVersionFromWildcard(vStr) + + var resultOperator string + var shouldIncrementVersion bool + switch opStr { + case ">": + resultOperator = ">=" + shouldIncrementVersion = true + case ">=": + resultOperator = ">=" + case "<": + resultOperator = "<" + case "<=": + resultOperator = "<" + shouldIncrementVersion = true + case "", "=", "==": + newParts = append(newParts, ">="+flatVersion) + resultOperator = "<" + shouldIncrementVersion = true + case "!=", "!": + newParts = append(newParts, "<"+flatVersion) + resultOperator = ">=" + shouldIncrementVersion = true + } + + var resultVersion string + if shouldIncrementVersion { + switch versionWildcardType { + case patchWildcard: + resultVersion, _ = incrementMinorVersion(flatVersion) + case minorWildcard: + resultVersion, _ = incrementMajorVersion(flatVersion) + } + } else { + resultVersion = flatVersion + } + + ap = resultOperator + resultVersion + } + newParts = append(newParts, ap) + } + expandedParts = append(expandedParts, newParts) + } + + return expandedParts, nil +} + +func parseComparator(s string) comparator { + switch s { + case "==": + fallthrough + case "": + fallthrough + case "=": + return compEQ + case ">": + return compGT + case ">=": + return compGE + case "<": + return compLT + case "<=": + return compLE + case "!": + fallthrough + case "!=": + return compNE + } + + return nil +} + +// MustParseRange is like ParseRange but panics if the range cannot be parsed. +func MustParseRange(s string) Range { + r, err := ParseRange(s) + if err != nil { + panic(`semver: ParseRange(` + s + `): ` + err.Error()) + } + return r +} diff --git a/vendor/github.com/blang/semver/v4/semver.go b/vendor/github.com/blang/semver/v4/semver.go new file mode 100644 index 000000000..307de610f --- /dev/null +++ b/vendor/github.com/blang/semver/v4/semver.go @@ -0,0 +1,476 @@ +package semver + +import ( + "errors" + "fmt" + "strconv" + "strings" +) + +const ( + numbers string = "0123456789" + alphas = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-" + alphanum = alphas + numbers +) + +// SpecVersion is the latest fully supported spec version of semver +var SpecVersion = Version{ + Major: 2, + Minor: 0, + Patch: 0, +} + +// Version represents a semver compatible version +type Version struct { + Major uint64 + Minor uint64 + Patch uint64 + Pre []PRVersion + Build []string //No Precedence +} + +// Version to string +func (v Version) String() string { + b := make([]byte, 0, 5) + b = strconv.AppendUint(b, v.Major, 10) + b = append(b, '.') + b = strconv.AppendUint(b, v.Minor, 10) + b = append(b, '.') + b = strconv.AppendUint(b, v.Patch, 10) + + if len(v.Pre) > 0 { + b = append(b, '-') + b = append(b, v.Pre[0].String()...) + + for _, pre := range v.Pre[1:] { + b = append(b, '.') + b = append(b, pre.String()...) + } + } + + if len(v.Build) > 0 { + b = append(b, '+') + b = append(b, v.Build[0]...) + + for _, build := range v.Build[1:] { + b = append(b, '.') + b = append(b, build...) + } + } + + return string(b) +} + +// FinalizeVersion discards prerelease and build number and only returns +// major, minor and patch number. +func (v Version) FinalizeVersion() string { + b := make([]byte, 0, 5) + b = strconv.AppendUint(b, v.Major, 10) + b = append(b, '.') + b = strconv.AppendUint(b, v.Minor, 10) + b = append(b, '.') + b = strconv.AppendUint(b, v.Patch, 10) + return string(b) +} + +// Equals checks if v is equal to o. +func (v Version) Equals(o Version) bool { + return (v.Compare(o) == 0) +} + +// EQ checks if v is equal to o. +func (v Version) EQ(o Version) bool { + return (v.Compare(o) == 0) +} + +// NE checks if v is not equal to o. +func (v Version) NE(o Version) bool { + return (v.Compare(o) != 0) +} + +// GT checks if v is greater than o. +func (v Version) GT(o Version) bool { + return (v.Compare(o) == 1) +} + +// GTE checks if v is greater than or equal to o. +func (v Version) GTE(o Version) bool { + return (v.Compare(o) >= 0) +} + +// GE checks if v is greater than or equal to o. +func (v Version) GE(o Version) bool { + return (v.Compare(o) >= 0) +} + +// LT checks if v is less than o. +func (v Version) LT(o Version) bool { + return (v.Compare(o) == -1) +} + +// LTE checks if v is less than or equal to o. +func (v Version) LTE(o Version) bool { + return (v.Compare(o) <= 0) +} + +// LE checks if v is less than or equal to o. +func (v Version) LE(o Version) bool { + return (v.Compare(o) <= 0) +} + +// Compare compares Versions v to o: +// -1 == v is less than o +// 0 == v is equal to o +// 1 == v is greater than o +func (v Version) Compare(o Version) int { + if v.Major != o.Major { + if v.Major > o.Major { + return 1 + } + return -1 + } + if v.Minor != o.Minor { + if v.Minor > o.Minor { + return 1 + } + return -1 + } + if v.Patch != o.Patch { + if v.Patch > o.Patch { + return 1 + } + return -1 + } + + // Quick comparison if a version has no prerelease versions + if len(v.Pre) == 0 && len(o.Pre) == 0 { + return 0 + } else if len(v.Pre) == 0 && len(o.Pre) > 0 { + return 1 + } else if len(v.Pre) > 0 && len(o.Pre) == 0 { + return -1 + } + + i := 0 + for ; i < len(v.Pre) && i < len(o.Pre); i++ { + if comp := v.Pre[i].Compare(o.Pre[i]); comp == 0 { + continue + } else if comp == 1 { + return 1 + } else { + return -1 + } + } + + // If all pr versions are the equal but one has further prversion, this one greater + if i == len(v.Pre) && i == len(o.Pre) { + return 0 + } else if i == len(v.Pre) && i < len(o.Pre) { + return -1 + } else { + return 1 + } + +} + +// IncrementPatch increments the patch version +func (v *Version) IncrementPatch() error { + v.Patch++ + return nil +} + +// IncrementMinor increments the minor version +func (v *Version) IncrementMinor() error { + v.Minor++ + v.Patch = 0 + return nil +} + +// IncrementMajor increments the major version +func (v *Version) IncrementMajor() error { + v.Major++ + v.Minor = 0 + v.Patch = 0 + return nil +} + +// Validate validates v and returns error in case +func (v Version) Validate() error { + // Major, Minor, Patch already validated using uint64 + + for _, pre := range v.Pre { + if !pre.IsNum { //Numeric prerelease versions already uint64 + if len(pre.VersionStr) == 0 { + return fmt.Errorf("Prerelease can not be empty %q", pre.VersionStr) + } + if !containsOnly(pre.VersionStr, alphanum) { + return fmt.Errorf("Invalid character(s) found in prerelease %q", pre.VersionStr) + } + } + } + + for _, build := range v.Build { + if len(build) == 0 { + return fmt.Errorf("Build meta data can not be empty %q", build) + } + if !containsOnly(build, alphanum) { + return fmt.Errorf("Invalid character(s) found in build meta data %q", build) + } + } + + return nil +} + +// New is an alias for Parse and returns a pointer, parses version string and returns a validated Version or error +func New(s string) (*Version, error) { + v, err := Parse(s) + vp := &v + return vp, err +} + +// Make is an alias for Parse, parses version string and returns a validated Version or error +func Make(s string) (Version, error) { + return Parse(s) +} + +// ParseTolerant allows for certain version specifications that do not strictly adhere to semver +// specs to be parsed by this library. It does so by normalizing versions before passing them to +// Parse(). It currently trims spaces, removes a "v" prefix, adds a 0 patch number to versions +// with only major and minor components specified, and removes leading 0s. +func ParseTolerant(s string) (Version, error) { + s = strings.TrimSpace(s) + s = strings.TrimPrefix(s, "v") + + // Split into major.minor.(patch+pr+meta) + parts := strings.SplitN(s, ".", 3) + // Remove leading zeros. + for i, p := range parts { + if len(p) > 1 { + p = strings.TrimLeft(p, "0") + if len(p) == 0 || !strings.ContainsAny(p[0:1], "0123456789") { + p = "0" + p + } + parts[i] = p + } + } + // Fill up shortened versions. + if len(parts) < 3 { + if strings.ContainsAny(parts[len(parts)-1], "+-") { + return Version{}, errors.New("Short version cannot contain PreRelease/Build meta data") + } + for len(parts) < 3 { + parts = append(parts, "0") + } + } + s = strings.Join(parts, ".") + + return Parse(s) +} + +// Parse parses version string and returns a validated Version or error +func Parse(s string) (Version, error) { + if len(s) == 0 { + return Version{}, errors.New("Version string empty") + } + + // Split into major.minor.(patch+pr+meta) + parts := strings.SplitN(s, ".", 3) + if len(parts) != 3 { + return Version{}, errors.New("No Major.Minor.Patch elements found") + } + + // Major + if !containsOnly(parts[0], numbers) { + return Version{}, fmt.Errorf("Invalid character(s) found in major number %q", parts[0]) + } + if hasLeadingZeroes(parts[0]) { + return Version{}, fmt.Errorf("Major number must not contain leading zeroes %q", parts[0]) + } + major, err := strconv.ParseUint(parts[0], 10, 64) + if err != nil { + return Version{}, err + } + + // Minor + if !containsOnly(parts[1], numbers) { + return Version{}, fmt.Errorf("Invalid character(s) found in minor number %q", parts[1]) + } + if hasLeadingZeroes(parts[1]) { + return Version{}, fmt.Errorf("Minor number must not contain leading zeroes %q", parts[1]) + } + minor, err := strconv.ParseUint(parts[1], 10, 64) + if err != nil { + return Version{}, err + } + + v := Version{} + v.Major = major + v.Minor = minor + + var build, prerelease []string + patchStr := parts[2] + + if buildIndex := strings.IndexRune(patchStr, '+'); buildIndex != -1 { + build = strings.Split(patchStr[buildIndex+1:], ".") + patchStr = patchStr[:buildIndex] + } + + if preIndex := strings.IndexRune(patchStr, '-'); preIndex != -1 { + prerelease = strings.Split(patchStr[preIndex+1:], ".") + patchStr = patchStr[:preIndex] + } + + if !containsOnly(patchStr, numbers) { + return Version{}, fmt.Errorf("Invalid character(s) found in patch number %q", patchStr) + } + if hasLeadingZeroes(patchStr) { + return Version{}, fmt.Errorf("Patch number must not contain leading zeroes %q", patchStr) + } + patch, err := strconv.ParseUint(patchStr, 10, 64) + if err != nil { + return Version{}, err + } + + v.Patch = patch + + // Prerelease + for _, prstr := range prerelease { + parsedPR, err := NewPRVersion(prstr) + if err != nil { + return Version{}, err + } + v.Pre = append(v.Pre, parsedPR) + } + + // Build meta data + for _, str := range build { + if len(str) == 0 { + return Version{}, errors.New("Build meta data is empty") + } + if !containsOnly(str, alphanum) { + return Version{}, fmt.Errorf("Invalid character(s) found in build meta data %q", str) + } + v.Build = append(v.Build, str) + } + + return v, nil +} + +// MustParse is like Parse but panics if the version cannot be parsed. +func MustParse(s string) Version { + v, err := Parse(s) + if err != nil { + panic(`semver: Parse(` + s + `): ` + err.Error()) + } + return v +} + +// PRVersion represents a PreRelease Version +type PRVersion struct { + VersionStr string + VersionNum uint64 + IsNum bool +} + +// NewPRVersion creates a new valid prerelease version +func NewPRVersion(s string) (PRVersion, error) { + if len(s) == 0 { + return PRVersion{}, errors.New("Prerelease is empty") + } + v := PRVersion{} + if containsOnly(s, numbers) { + if hasLeadingZeroes(s) { + return PRVersion{}, fmt.Errorf("Numeric PreRelease version must not contain leading zeroes %q", s) + } + num, err := strconv.ParseUint(s, 10, 64) + + // Might never be hit, but just in case + if err != nil { + return PRVersion{}, err + } + v.VersionNum = num + v.IsNum = true + } else if containsOnly(s, alphanum) { + v.VersionStr = s + v.IsNum = false + } else { + return PRVersion{}, fmt.Errorf("Invalid character(s) found in prerelease %q", s) + } + return v, nil +} + +// IsNumeric checks if prerelease-version is numeric +func (v PRVersion) IsNumeric() bool { + return v.IsNum +} + +// Compare compares two PreRelease Versions v and o: +// -1 == v is less than o +// 0 == v is equal to o +// 1 == v is greater than o +func (v PRVersion) Compare(o PRVersion) int { + if v.IsNum && !o.IsNum { + return -1 + } else if !v.IsNum && o.IsNum { + return 1 + } else if v.IsNum && o.IsNum { + if v.VersionNum == o.VersionNum { + return 0 + } else if v.VersionNum > o.VersionNum { + return 1 + } else { + return -1 + } + } else { // both are Alphas + if v.VersionStr == o.VersionStr { + return 0 + } else if v.VersionStr > o.VersionStr { + return 1 + } else { + return -1 + } + } +} + +// PreRelease version to string +func (v PRVersion) String() string { + if v.IsNum { + return strconv.FormatUint(v.VersionNum, 10) + } + return v.VersionStr +} + +func containsOnly(s string, set string) bool { + return strings.IndexFunc(s, func(r rune) bool { + return !strings.ContainsRune(set, r) + }) == -1 +} + +func hasLeadingZeroes(s string) bool { + return len(s) > 1 && s[0] == '0' +} + +// NewBuildVersion creates a new valid build version +func NewBuildVersion(s string) (string, error) { + if len(s) == 0 { + return "", errors.New("Buildversion is empty") + } + if !containsOnly(s, alphanum) { + return "", fmt.Errorf("Invalid character(s) found in build meta data %q", s) + } + return s, nil +} + +// FinalizeVersion returns the major, minor and patch number only and discards +// prerelease and build number. +func FinalizeVersion(s string) (string, error) { + v, err := Parse(s) + if err != nil { + return "", err + } + v.Pre = nil + v.Build = nil + + finalVer := v.String() + return finalVer, nil +} diff --git a/vendor/github.com/blang/semver/v4/sort.go b/vendor/github.com/blang/semver/v4/sort.go new file mode 100644 index 000000000..e18f88082 --- /dev/null +++ b/vendor/github.com/blang/semver/v4/sort.go @@ -0,0 +1,28 @@ +package semver + +import ( + "sort" +) + +// Versions represents multiple versions. +type Versions []Version + +// Len returns length of version collection +func (s Versions) Len() int { + return len(s) +} + +// Swap swaps two versions inside the collection by its indices +func (s Versions) Swap(i, j int) { + s[i], s[j] = s[j], s[i] +} + +// Less checks if version at index i is less than version at index j +func (s Versions) Less(i, j int) bool { + return s[i].LT(s[j]) +} + +// Sort sorts a slice of versions +func Sort(versions []Version) { + sort.Sort(Versions(versions)) +} diff --git a/vendor/github.com/blang/semver/v4/sql.go b/vendor/github.com/blang/semver/v4/sql.go new file mode 100644 index 000000000..db958134f --- /dev/null +++ b/vendor/github.com/blang/semver/v4/sql.go @@ -0,0 +1,30 @@ +package semver + +import ( + "database/sql/driver" + "fmt" +) + +// Scan implements the database/sql.Scanner interface. +func (v *Version) Scan(src interface{}) (err error) { + var str string + switch src := src.(type) { + case string: + str = src + case []byte: + str = string(src) + default: + return fmt.Errorf("version.Scan: cannot convert %T to string", src) + } + + if t, err := Parse(str); err == nil { + *v = t + } + + return +} + +// Value implements the database/sql/driver.Valuer interface. +func (v Version) Value() (driver.Value, error) { + return v.String(), nil +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 195c40405..bd6f81903 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -41,8 +41,10 @@ github.com/acarl005/stripansi # github.com/beorn7/perks v1.0.1 github.com/beorn7/perks/quantile # github.com/blang/semver v3.5.1+incompatible -## explicit github.com/blang/semver +# github.com/blang/semver/v4 v4.0.0 +## explicit +github.com/blang/semver/v4 # github.com/buger/goterm v1.0.4 ## explicit github.com/buger/goterm diff --git a/version/version.go b/version/version.go index 57403b42c..0a84bb235 100644 --- a/version/version.go +++ b/version/version.go @@ -1,7 +1,7 @@ package version import ( - "github.com/blang/semver" + "github.com/blang/semver/v4" ) type ( -- cgit v1.2.3-54-g00ecf From 6d84a9952f1e5be1a187bcc6d9bbc2532331cfc8 Mon Sep 17 00:00:00 2001 From: Karthik Elango Date: Tue, 28 Jun 2022 15:31:20 -0400 Subject: Podman stop --filter flag Filter flag is added for podman stop and podman --remote stop. Filtering logic is implemented in getContainersAndInputByContext(). Start filtering can be manipulated to use this logic as well to limit redundancy. Signed-off-by: Karthik Elango --- cmd/podman/containers/stop.go | 17 ++++++++++-- cmd/podman/validate/args.go | 7 +++++ docs/source/markdown/podman-stop.1.md | 24 +++++++++++++++++ pkg/api/handlers/compat/containers_stop.go | 2 -- pkg/domain/entities/containers.go | 1 + pkg/domain/infra/abi/containers.go | 31 +++++++++++++++++----- pkg/domain/infra/tunnel/containers.go | 6 ++--- pkg/domain/infra/tunnel/helpers.go | 14 ++++++---- test/e2e/stop_test.go | 42 ++++++++++++++++++++++++++++++ 9 files changed, 125 insertions(+), 19 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/containers/stop.go b/cmd/podman/containers/stop.go index 2ddd169a1..261f441c3 100644 --- a/cmd/podman/containers/stop.go +++ b/cmd/podman/containers/stop.go @@ -49,7 +49,9 @@ var ( ) var ( - stopOptions = entities.StopOptions{} + stopOptions = entities.StopOptions{ + Filters: make(map[string][]string), + } stopTimeout uint ) @@ -67,6 +69,10 @@ func stopFlags(cmd *cobra.Command) { flags.UintVarP(&stopTimeout, timeFlagName, "t", containerConfig.Engine.StopTimeout, "Seconds to wait for stop before killing the container") _ = cmd.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone) + filterFlagName := "filter" + flags.StringSliceVarP(&filters, filterFlagName, "f", []string{}, "Filter output based on conditions given") + _ = cmd.RegisterFlagCompletionFunc(filterFlagName, common.AutocompletePsFilters) + if registry.IsRemote() { _ = flags.MarkHidden("cidfile") _ = flags.MarkHidden("ignore") @@ -97,7 +103,6 @@ func stop(cmd *cobra.Command, args []string) error { if cmd.Flag("time").Changed { stopOptions.Timeout = &stopTimeout } - for _, cidFile := range cidFiles { content, err := ioutil.ReadFile(cidFile) if err != nil { @@ -107,6 +112,14 @@ func stop(cmd *cobra.Command, args []string) error { args = append(args, id) } + for _, f := range filters { + split := strings.SplitN(f, "=", 2) + if len(split) < 2 { + return fmt.Errorf("invalid filter %q", f) + } + stopOptions.Filters[split[0]] = append(stopOptions.Filters[split[0]], split[1]) + } + responses, err := registry.ContainerEngine().ContainerStop(context.Background(), args, stopOptions) if err != nil { return err diff --git a/cmd/podman/validate/args.go b/cmd/podman/validate/args.go index 39eedca64..6d212665d 100644 --- a/cmd/podman/validate/args.go +++ b/cmd/podman/validate/args.go @@ -86,6 +86,13 @@ func CheckAllLatestAndIDFile(c *cobra.Command, args []string, ignoreArgLen bool, specifiedIDFile = true } + if c.Flags().Changed("filter") { + if argLen > 0 { + return errors.New("--filter takes no arguments") + } + return nil + } + if specifiedIDFile && (specifiedAll || specifiedLatest) { return fmt.Errorf("--all, --latest, and --%s cannot be used together", idFileFlag) } else if specifiedAll && specifiedLatest { diff --git a/docs/source/markdown/podman-stop.1.md b/docs/source/markdown/podman-stop.1.md index e35ab9182..cfc49afa1 100644 --- a/docs/source/markdown/podman-stop.1.md +++ b/docs/source/markdown/podman-stop.1.md @@ -25,6 +25,30 @@ Stop all running containers. This does not include paused containers. Read container ID from the specified file and remove the container. Can be specified multiple times. +#### **--filter**, **-f**=*filter* + +Filter what containers are going to be stopped. +Multiple filters can be given with multiple uses of the --filter flag. +Filters with the same key work inclusive with the only exception being +`label` which is exclusive. Filters with different keys always work exclusive. + +Valid filters are listed below: + +| **Filter** | **Description** | +| --------------- | -------------------------------------------------------------------------------- | +| id | [ID] Container's ID (accepts regex) | +| name | [Name] Container's name (accepts regex) | +| label | [Key] or [Key=Value] Label assigned to a container | +| exited | [Int] Container's exit code | +| status | [Status] Container's status: 'created', 'exited', 'paused', 'running', 'unknown' | +| ancestor | [ImageName] Image or descendant used to create container | +| before | [ID] or [Name] Containers created before this container | +| since | [ID] or [Name] Containers created since this container | +| volume | [VolumeName] or [MountpointDestination] Volume mounted in container | +| health | [Status] healthy or unhealthy | +| pod | [Pod] name or full or partial ID of pod | +| network | [Network] name or full ID of network | + #### **--ignore**, **-i** Ignore errors when specified containers are not in the container store. A user diff --git a/pkg/api/handlers/compat/containers_stop.go b/pkg/api/handlers/compat/containers_stop.go index 33bb3a679..c9a27dd83 100644 --- a/pkg/api/handlers/compat/containers_stop.go +++ b/pkg/api/handlers/compat/containers_stop.go @@ -33,9 +33,7 @@ func StopContainer(w http.ResponseWriter, r *http.Request) { utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse parameters for %s: %w", r.URL.String(), err)) return } - name := utils.GetName(r) - options := entities.StopOptions{ Ignore: query.Ignore, } diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index 17408f12f..934a7cbdc 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -80,6 +80,7 @@ type PauseUnpauseReport struct { } type StopOptions struct { + Filters map[string][]string All bool Ignore bool Latest bool diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 23a591604..04eb85504 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -37,12 +37,29 @@ import ( ) // getContainersAndInputByContext gets containers whether all, latest, or a slice of names/ids -// is specified. It also returns a list of the corresponding input name used to look up each container. -func getContainersAndInputByContext(all, latest bool, names []string, runtime *libpod.Runtime) (ctrs []*libpod.Container, rawInput []string, err error) { +// is specified. It also returns a list of the corresponding input name used to lookup each container. +func getContainersAndInputByContext(all, latest bool, names []string, filters map[string][]string, runtime *libpod.Runtime) (ctrs []*libpod.Container, rawInput []string, err error) { var ctr *libpod.Container ctrs = []*libpod.Container{} + filterFuncs := make([]libpod.ContainerFilter, 0, len(filters)) switch { + case len(filters) > 0: + for k, v := range filters { + generatedFunc, err := dfilters.GenerateContainerFilterFuncs(k, v, runtime) + if err != nil { + return nil, nil, err + } + filterFuncs = append(filterFuncs, generatedFunc) + } + ctrs, err = runtime.GetContainers(filterFuncs...) + if err != nil { + return nil, nil, err + } + rawInput = []string{} + for _, candidate := range ctrs { + rawInput = append(rawInput, candidate.ID()) + } case all: ctrs, err = runtime.GetAllContainers() case latest: @@ -66,13 +83,13 @@ func getContainersAndInputByContext(all, latest bool, names []string, runtime *l } } } - return + return ctrs, rawInput, err } // getContainersByContext gets containers whether all, latest, or a slice of names/ids // is specified. func getContainersByContext(all, latest bool, names []string, runtime *libpod.Runtime) (ctrs []*libpod.Container, err error) { - ctrs, _, err = getContainersAndInputByContext(all, latest, names, runtime) + ctrs, _, err = getContainersAndInputByContext(all, latest, names, nil, runtime) return } @@ -150,7 +167,7 @@ func (ic *ContainerEngine) ContainerUnpause(ctx context.Context, namesOrIds []st } func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []string, options entities.StopOptions) ([]*entities.StopReport, error) { names := namesOrIds - ctrs, rawInputs, err := getContainersAndInputByContext(options.All, options.Latest, names, ic.Libpod) + ctrs, rawInputs, err := getContainersAndInputByContext(options.All, options.Latest, names, options.Filters, ic.Libpod) if err != nil && !(options.Ignore && errors.Is(err, define.ErrNoSuchCtr)) { return nil, err } @@ -228,7 +245,7 @@ func (ic *ContainerEngine) ContainerKill(ctx context.Context, namesOrIds []strin if err != nil { return nil, err } - ctrs, rawInputs, err := getContainersAndInputByContext(options.All, options.Latest, namesOrIds, ic.Libpod) + ctrs, rawInputs, err := getContainersAndInputByContext(options.All, options.Latest, namesOrIds, nil, ic.Libpod) if err != nil { return nil, err } @@ -874,7 +891,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri } } } - ctrs, rawInputs, err := getContainersAndInputByContext(all, options.Latest, containersNamesOrIds, ic.Libpod) + ctrs, rawInputs, err := getContainersAndInputByContext(all, options.Latest, containersNamesOrIds, options.Filters, ic.Libpod) if err != nil { return nil, err } diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 5568ccde8..fcabff7c4 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -91,8 +91,7 @@ func (ic *ContainerEngine) ContainerUnpause(ctx context.Context, namesOrIds []st } func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []string, opts entities.StopOptions) ([]*entities.StopReport, error) { - reports := []*entities.StopReport{} - ctrs, rawInputs, err := getContainersAndInputByContext(ic.ClientCtx, opts.All, opts.Ignore, namesOrIds) + ctrs, rawInputs, err := getContainersAndInputByContext(ic.ClientCtx, opts.All, opts.Ignore, namesOrIds, opts.Filters) if err != nil { return nil, err } @@ -104,6 +103,7 @@ func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []strin if to := opts.Timeout; to != nil { options.WithTimeout(*to) } + reports := []*entities.StopReport{} for _, c := range ctrs { report := entities.StopReport{ Id: c.ID, @@ -134,7 +134,7 @@ func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []strin } func (ic *ContainerEngine) ContainerKill(ctx context.Context, namesOrIds []string, opts entities.KillOptions) ([]*entities.KillReport, error) { - ctrs, rawInputs, err := getContainersAndInputByContext(ic.ClientCtx, opts.All, false, namesOrIds) + ctrs, rawInputs, err := getContainersAndInputByContext(ic.ClientCtx, opts.All, false, namesOrIds, nil) if err != nil { return nil, err } diff --git a/pkg/domain/infra/tunnel/helpers.go b/pkg/domain/infra/tunnel/helpers.go index 24b2b619d..9ff1641f0 100644 --- a/pkg/domain/infra/tunnel/helpers.go +++ b/pkg/domain/infra/tunnel/helpers.go @@ -15,25 +15,29 @@ import ( // FIXME: the `ignore` parameter is very likely wrong here as it should rather // be used on *errors* from operations such as remove. func getContainersByContext(contextWithConnection context.Context, all, ignore bool, namesOrIDs []string) ([]entities.ListContainer, error) { - ctrs, _, err := getContainersAndInputByContext(contextWithConnection, all, ignore, namesOrIDs) + ctrs, _, err := getContainersAndInputByContext(contextWithConnection, all, ignore, namesOrIDs, nil) return ctrs, err } -func getContainersAndInputByContext(contextWithConnection context.Context, all, ignore bool, namesOrIDs []string) ([]entities.ListContainer, []string, error) { +func getContainersAndInputByContext(contextWithConnection context.Context, all, ignore bool, namesOrIDs []string, filters map[string][]string) ([]entities.ListContainer, []string, error) { if all && len(namesOrIDs) > 0 { return nil, nil, errors.New("cannot look up containers and all") } - options := new(containers.ListOptions).WithAll(true).WithSync(true) + options := new(containers.ListOptions).WithAll(true).WithSync(true).WithFilters(filters) allContainers, err := containers.List(contextWithConnection, options) if err != nil { return nil, nil, err } rawInputs := []string{} - if all { + switch { + case len(filters) > 0: + for i := range allContainers { + namesOrIDs = append(namesOrIDs, allContainers[i].ID) + } + case all: for i := range allContainers { rawInputs = append(rawInputs, allContainers[i].ID) } - return allContainers, rawInputs, err } diff --git a/test/e2e/stop_test.go b/test/e2e/stop_test.go index 97d8ba701..7a258466a 100644 --- a/test/e2e/stop_test.go +++ b/test/e2e/stop_test.go @@ -1,6 +1,7 @@ package integration import ( + "fmt" "io/ioutil" "os" "strings" @@ -363,4 +364,45 @@ var _ = Describe("Podman stop", func() { Expect(session).Should(Exit(0)) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) }) + + It("podman stop --filter", func() { + session1 := podmanTest.Podman([]string{"container", "create", ALPINE}) + session1.WaitWithDefaultTimeout() + Expect(session1).Should(Exit(0)) + cid1 := session1.OutputToString() + + session1 = podmanTest.Podman([]string{"container", "create", ALPINE}) + session1.WaitWithDefaultTimeout() + Expect(session1).Should(Exit(0)) + cid2 := session1.OutputToString() + + session1 = podmanTest.Podman([]string{"container", "create", ALPINE}) + session1.WaitWithDefaultTimeout() + Expect(session1).Should(Exit(0)) + cid3 := session1.OutputToString() + shortCid3 := cid3[0:5] + + session1 = podmanTest.Podman([]string{"start", "--all"}) + session1.WaitWithDefaultTimeout() + Expect(session1).Should(Exit(0)) + + session1 = podmanTest.Podman([]string{"stop", cid1, "-f", "status=running"}) + session1.WaitWithDefaultTimeout() + Expect(session1).Should(Exit(125)) + + session1 = podmanTest.Podman([]string{"stop", "-a", "--filter", fmt.Sprintf("id=%swrongid", shortCid3)}) + session1.WaitWithDefaultTimeout() + Expect(session1).Should(Exit(0)) + Expect(session1.OutputToString()).To(HaveLen(0)) + + session1 = podmanTest.Podman([]string{"stop", "-a", "--filter", fmt.Sprintf("id=%s", shortCid3)}) + session1.WaitWithDefaultTimeout() + Expect(session1).Should(Exit(0)) + Expect(session1.OutputToString()).To(BeEquivalentTo(cid3)) + + session1 = podmanTest.Podman([]string{"stop", "-f", fmt.Sprintf("id=%s", cid2)}) + session1.WaitWithDefaultTimeout() + Expect(session1).Should(Exit(0)) + Expect(session1.OutputToString()).To(BeEquivalentTo(cid2)) + }) }) -- cgit v1.2.3-54-g00ecf From e473c5e4b741cef2c1174cb4ec51000f443e6877 Mon Sep 17 00:00:00 2001 From: Ashley Cui Date: Fri, 15 Jul 2022 15:20:16 -0400 Subject: Machine init: create .ssh dir if not exist When initing a machine, we generate ssh keys in `$HOME/.ssh`. If there is not .ssh dir, we should create it, so the init does not fail. Signed-off-by: Ashley Cui --- pkg/machine/keys.go | 3 +++ 1 file changed, 3 insertions(+) (limited to 'pkg') diff --git a/pkg/machine/keys.go b/pkg/machine/keys.go index 0c7d7114e..94cbdac04 100644 --- a/pkg/machine/keys.go +++ b/pkg/machine/keys.go @@ -21,6 +21,9 @@ var sshCommand = []string{"ssh-keygen", "-N", "", "-t", "ed25519", "-f"} // CreateSSHKeys makes a priv and pub ssh key for interacting // the a VM. func CreateSSHKeys(writeLocation string) (string, error) { + if err := os.MkdirAll(filepath.Dir(writeLocation), 0700); err != nil { + return "", err + } if err := generatekeys(writeLocation); err != nil { return "", err } -- cgit v1.2.3-54-g00ecf From 17dbce2fb060b4803b2dae4eb6b78fdebea5b61f Mon Sep 17 00:00:00 2001 From: Ashley Cui Date: Fri, 8 Jul 2022 20:10:25 -0400 Subject: Clean up cached machine images When initing machines, we download a machine image, and uncompress and copy the image for the actual vm image. When a user constantly pulls new machines, there may be a buildup of old, unused machine images. This commit cleans ups the unused cached images. Changes: - If the machine is pulled from a URL or from the FCOS releases, we pull them into XDG_DATA_HOME/containers/podman/machine/vmType/cache - Cache cleanups only happen if there is a cache miss, and we need to pull a new image - For Fedora and FCOS, we actually use the cache, so we go through the cache dir and remove any images older than 2 weeks (FCOS's release cycle), on a cache miss. - For generic files pulled from a URL, we don't actually cache, so we delete the pulled file immediately after creating a machine image - For generic files from a local path, the original file will never be cleaned up Note that because we cache in a different dir, this will not clean up old images pulled before this commit. [NO NEW TESTS NEEDED] Signed-off-by: Ashley Cui --- pkg/machine/config.go | 19 ++++++++++++++++-- pkg/machine/fcos.go | 19 +++++++++++++++--- pkg/machine/fedora.go | 18 ++++++++++++++--- pkg/machine/pull.go | 54 +++++++++++++++++++++++++++++++++++++++++++-------- 4 files changed, 94 insertions(+), 16 deletions(-) (limited to 'pkg') diff --git a/pkg/machine/config.go b/pkg/machine/config.go index 29cd7bc00..253601dad 100644 --- a/pkg/machine/config.go +++ b/pkg/machine/config.go @@ -73,6 +73,7 @@ type Download struct { Arch string Artifact string CompressionType string + CacheDir string Format string ImageName string LocalPath string @@ -139,6 +140,7 @@ type VM interface { type DistributionDownload interface { HasUsableCache() (bool, error) Get() *Download + CleanCache() error } type InspectInfo struct { ConfigPath VMFile @@ -172,6 +174,19 @@ func (rc RemoteConnectionType) MakeSSHURL(host, path, port, userName string) url return uri } +// GetCacheDir returns the dir where VM images are downladed into when pulled +func GetCacheDir(vmType string) (string, error) { + dataDir, err := GetDataDir(vmType) + if err != nil { + return "", err + } + cacheDir := filepath.Join(dataDir, "cache") + if _, err := os.Stat(cacheDir); !errors.Is(err, os.ErrNotExist) { + return cacheDir, nil + } + return cacheDir, os.MkdirAll(cacheDir, 0755) +} + // GetDataDir returns the filepath where vm images should // live for podman-machine. func GetDataDir(vmType string) (string, error) { @@ -180,7 +195,7 @@ func GetDataDir(vmType string) (string, error) { return "", err } dataDir := filepath.Join(dataDirPrefix, vmType) - if _, err := os.Stat(dataDir); !os.IsNotExist(err) { + if _, err := os.Stat(dataDir); !errors.Is(err, os.ErrNotExist) { return dataDir, nil } mkdirErr := os.MkdirAll(dataDir, 0755) @@ -205,7 +220,7 @@ func GetConfDir(vmType string) (string, error) { return "", err } confDir := filepath.Join(confDirPrefix, vmType) - if _, err := os.Stat(confDir); !os.IsNotExist(err) { + if _, err := os.Stat(confDir); !errors.Is(err, os.ErrNotExist) { return confDir, nil } mkdirErr := os.MkdirAll(confDir, 0755) diff --git a/pkg/machine/fcos.go b/pkg/machine/fcos.go index 4ccb99e96..246f92a19 100644 --- a/pkg/machine/fcos.go +++ b/pkg/machine/fcos.go @@ -13,6 +13,7 @@ import ( "path/filepath" "runtime" "strings" + "time" "github.com/coreos/stream-metadata-go/fedoracoreos" "github.com/coreos/stream-metadata-go/release" @@ -53,7 +54,7 @@ func NewFcosDownloader(vmType, vmName, imageStream string) (DistributionDownload return nil, err } - dataDir, err := GetDataDir(vmType) + cacheDir, err := GetCacheDir(vmType) if err != nil { return nil, err } @@ -62,15 +63,20 @@ func NewFcosDownloader(vmType, vmName, imageStream string) (DistributionDownload Download: Download{ Arch: getFcosArch(), Artifact: artifact, + CacheDir: cacheDir, Format: Format, ImageName: imageName, - LocalPath: filepath.Join(dataDir, imageName), + LocalPath: filepath.Join(cacheDir, imageName), Sha256sum: info.Sha256Sum, URL: url, VMName: vmName, }, } - fcd.Download.LocalUncompressedFile = fcd.getLocalUncompressedName() + dataDir, err := GetDataDir(vmType) + if err != nil { + return nil, err + } + fcd.Download.LocalUncompressedFile = fcd.getLocalUncompressedFile(dataDir) return fcd, nil } @@ -108,6 +114,13 @@ func (f FcosDownload) HasUsableCache() (bool, error) { return sum.Encoded() == f.Sha256sum, nil } +func (f FcosDownload) CleanCache() error { + // Set cached image to expire after 2 weeks + // FCOS refreshes around every 2 weeks, assume old images aren't needed + expire := 14 * 24 * time.Hour + return removeImageAfterExpire(f.CacheDir, expire) +} + func getFcosArch() string { var arch string // TODO fill in more architectures diff --git a/pkg/machine/fedora.go b/pkg/machine/fedora.go index 2c6d3d810..7c80fc5d3 100644 --- a/pkg/machine/fedora.go +++ b/pkg/machine/fedora.go @@ -11,6 +11,7 @@ import ( "net/http" "net/url" "path/filepath" + "time" ) const ( @@ -27,7 +28,7 @@ func NewFedoraDownloader(vmType, vmName, releaseStream string) (DistributionDown return nil, err } - dataDir, err := GetDataDir(vmType) + cacheDir, err := GetCacheDir(vmType) if err != nil { return nil, err } @@ -38,15 +39,20 @@ func NewFedoraDownloader(vmType, vmName, releaseStream string) (DistributionDown Download: Download{ Arch: getFcosArch(), Artifact: artifact, + CacheDir: cacheDir, Format: Format, ImageName: imageName, - LocalPath: filepath.Join(dataDir, imageName), + LocalPath: filepath.Join(cacheDir, imageName), URL: downloadURL, VMName: vmName, Size: size, }, } - f.Download.LocalUncompressedFile = f.getLocalUncompressedName() + dataDir, err := GetDataDir(vmType) + if err != nil { + return nil, err + } + f.Download.LocalUncompressedFile = f.getLocalUncompressedFile(dataDir) return f, nil } @@ -65,6 +71,12 @@ func (f FedoraDownload) HasUsableCache() (bool, error) { return info.Size() == f.Size, nil } +func (f FedoraDownload) CleanCache() error { + // Set cached image to expire after 2 weeks + expire := 14 * 24 * time.Hour + return removeImageAfterExpire(f.CacheDir, expire) +} + func getFedoraDownload(releaseURL string) (*url.URL, int64, error) { downloadURL, err := url.Parse(releaseURL) if err != nil { diff --git a/pkg/machine/pull.go b/pkg/machine/pull.go index 7e6f01bad..08baa7df8 100644 --- a/pkg/machine/pull.go +++ b/pkg/machine/pull.go @@ -5,6 +5,7 @@ package machine import ( "bufio" + "errors" "fmt" "io" "io/ioutil" @@ -39,6 +40,10 @@ func NewGenericDownloader(vmType, vmName, pullPath string) (DistributionDownload if err != nil { return nil, err } + cacheDir, err := GetCacheDir(vmType) + if err != nil { + return nil, err + } dl := Download{} // Is pullpath a file or url? getURL, err := url2.Parse(pullPath) @@ -48,25 +53,23 @@ func NewGenericDownloader(vmType, vmName, pullPath string) (DistributionDownload if len(getURL.Scheme) > 0 { urlSplit := strings.Split(getURL.Path, "/") imageName = urlSplit[len(urlSplit)-1] - dl.LocalUncompressedFile = filepath.Join(dataDir, imageName) dl.URL = getURL - dl.LocalPath = filepath.Join(dataDir, imageName) + dl.LocalPath = filepath.Join(cacheDir, imageName) } else { // Dealing with FilePath imageName = filepath.Base(pullPath) - dl.LocalUncompressedFile = filepath.Join(dataDir, imageName) dl.LocalPath = pullPath } dl.VMName = vmName dl.ImageName = imageName + dl.LocalUncompressedFile = filepath.Join(dataDir, imageName) // The download needs to be pulled into the datadir gd := GenericDownload{Download: dl} - gd.LocalUncompressedFile = gd.getLocalUncompressedName() return gd, nil } -func (d Download) getLocalUncompressedName() string { +func (d Download) getLocalUncompressedFile(dataDir string) string { var ( extension string ) @@ -78,8 +81,8 @@ func (d Download) getLocalUncompressedName() string { case strings.HasSuffix(d.LocalPath, ".xz"): extension = ".xz" } - uncompressedFilename := filepath.Join(filepath.Dir(d.LocalPath), d.VMName+"_"+d.ImageName) - return strings.TrimSuffix(uncompressedFilename, extension) + uncompressedFilename := d.VMName + "_" + d.ImageName + return filepath.Join(dataDir, strings.TrimSuffix(uncompressedFilename, extension)) } func (g GenericDownload) Get() *Download { @@ -91,6 +94,18 @@ func (g GenericDownload) HasUsableCache() (bool, error) { return g.URL == nil, nil } +// CleanCache cleans out downloaded uncompressed image files +func (g GenericDownload) CleanCache() error { + // Remove any image that has been downloaded via URL + // We never read from cache for generic downloads + if g.URL != nil { + if err := os.Remove(g.LocalPath); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + } + return nil +} + func DownloadImage(d DistributionDownload) error { // check if the latest image is already present ok, err := d.HasUsableCache() @@ -101,8 +116,14 @@ func DownloadImage(d DistributionDownload) error { if err := DownloadVMImage(d.Get().URL, d.Get().LocalPath); err != nil { return err } + // Clean out old cached images, since we didn't find needed image in cache + defer func() { + if err = d.CleanCache(); err != nil { + logrus.Warnf("error cleaning machine image cache: %s", err) + } + }() } - return Decompress(d.Get().LocalPath, d.Get().getLocalUncompressedName()) + return Decompress(d.Get().LocalPath, d.Get().LocalUncompressedFile) } // DownloadVMImage downloads a VM image from url to given path @@ -253,3 +274,20 @@ func decompressEverythingElse(src string, output io.WriteCloser) error { _, err = io.Copy(output, uncompressStream) return err } + +func removeImageAfterExpire(dir string, expire time.Duration) error { + now := time.Now() + err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + // Delete any cache files that are older than expiry date + if !info.IsDir() && (now.Sub(info.ModTime()) > expire) { + err := os.Remove(path) + if err != nil && !errors.Is(err, os.ErrNotExist) { + logrus.Warnf("unable to clean up cached image: %s", path) + } else { + logrus.Debugf("cleaning up cached image: %s", path) + } + } + return nil + }) + return err +} -- cgit v1.2.3-54-g00ecf From d2e2756a4cf036c30dc1b9d804c4a3ed331efd5b Mon Sep 17 00:00:00 2001 From: "Jason T. Greene" Date: Mon, 18 Jul 2022 10:34:42 -0500 Subject: Print rootfs download as a specific version on Win - Also save the file using this convention. - Change the general pull mechanism to print the local file as opposed to the remote to enable this - no change in observed behavior on mac Signed-off-by: Jason T. Greene --- pkg/machine/fedora.go | 32 +++++++++++++++++++++++++------- pkg/machine/pull.go | 7 +++---- 2 files changed, 28 insertions(+), 11 deletions(-) (limited to 'pkg') diff --git a/pkg/machine/fedora.go b/pkg/machine/fedora.go index 7c80fc5d3..7ac4692e3 100644 --- a/pkg/machine/fedora.go +++ b/pkg/machine/fedora.go @@ -6,7 +6,10 @@ package machine import ( "errors" "fmt" + "io" "os" + "path" + "strings" "net/http" "net/url" @@ -23,7 +26,7 @@ type FedoraDownload struct { } func NewFedoraDownloader(vmType, vmName, releaseStream string) (DistributionDownload, error) { - downloadURL, size, err := getFedoraDownload(githubLatestReleaseURL) + downloadURL, version, size, err := getFedoraDownload(githubLatestReleaseURL) if err != nil { return nil, err } @@ -33,7 +36,7 @@ func NewFedoraDownloader(vmType, vmName, releaseStream string) (DistributionDown return nil, err } - imageName := "rootfs.tar.xz" + imageName := fmt.Sprintf("fedora-podman-%s.tar.xz", version) f := FedoraDownload{ Download: Download{ @@ -77,21 +80,36 @@ func (f FedoraDownload) CleanCache() error { return removeImageAfterExpire(f.CacheDir, expire) } -func getFedoraDownload(releaseURL string) (*url.URL, int64, error) { +func getFedoraDownload(releaseURL string) (*url.URL, string, int64, error) { downloadURL, err := url.Parse(releaseURL) if err != nil { - return nil, -1, fmt.Errorf("invalid URL generated from discovered Fedora file: %s: %w", releaseURL, err) + return nil, "", -1, fmt.Errorf("invalid URL generated from discovered Fedora file: %s: %w", releaseURL, err) } resp, err := http.Head(releaseURL) if err != nil { - return nil, -1, fmt.Errorf("head request failed: %s: %w", releaseURL, err) + return nil, "", -1, fmt.Errorf("head request failed: %s: %w", releaseURL, err) } _ = resp.Body.Close() + contentLen := resp.ContentLength if resp.StatusCode != http.StatusOK { - return nil, -1, fmt.Errorf("head request failed: %s: %w", releaseURL, err) + return nil, "", -1, fmt.Errorf("head request failed: %s: %w", releaseURL, err) } - return downloadURL, resp.ContentLength, nil + verURL := *downloadURL + verURL.Path = path.Join(path.Dir(downloadURL.Path), "version") + + resp, err = http.Get(verURL.String()) + if err != nil { + return nil, "", -1, fmt.Errorf("get request failed: %s: %w", verURL.String(), err) + } + + bytes, err := io.ReadAll(&io.LimitedReader{R: resp.Body, N: 1024}) + if err != nil { + return nil, "", -1, fmt.Errorf("failed reading: %s: %w", verURL.String(), err) + } + _ = resp.Body.Close() + + return downloadURL, strings.TrimSpace(string(bytes)), contentLen, nil } diff --git a/pkg/machine/pull.go b/pkg/machine/pull.go index 08baa7df8..26b6adc67 100644 --- a/pkg/machine/pull.go +++ b/pkg/machine/pull.go @@ -113,7 +113,7 @@ func DownloadImage(d DistributionDownload) error { return err } if !ok { - if err := DownloadVMImage(d.Get().URL, d.Get().LocalPath); err != nil { + if err := DownloadVMImage(d.Get().URL, d.Get().ImageName, d.Get().LocalPath); err != nil { return err } // Clean out old cached images, since we didn't find needed image in cache @@ -128,7 +128,7 @@ func DownloadImage(d DistributionDownload) error { // DownloadVMImage downloads a VM image from url to given path // with download status -func DownloadVMImage(downloadURL *url2.URL, localImagePath string) error { +func DownloadVMImage(downloadURL *url2.URL, imageName string, localImagePath string) error { out, err := os.Create(localImagePath) if err != nil { return err @@ -153,8 +153,7 @@ func DownloadVMImage(downloadURL *url2.URL, localImagePath string) error { return fmt.Errorf("downloading VM image %s: %s", downloadURL, resp.Status) } size := resp.ContentLength - urlSplit := strings.Split(downloadURL.Path, "/") - prefix := "Downloading VM image: " + urlSplit[len(urlSplit)-1] + prefix := "Downloading VM image: " + imageName onComplete := prefix + ": done" p := mpb.New( -- cgit v1.2.3-54-g00ecf From de8d2cc9444c82196cc234582359ca98aa2708ec Mon Sep 17 00:00:00 2001 From: Toshiki Sonoda Date: Tue, 19 Jul 2022 10:00:33 +0900 Subject: "pod pause/unpause/stop" append "report.Errs" to "reports" There is a possibility that podman does not output expected error message. (e.g. When pause rootless cgroups v1 container on host) This problem is solved by appending `report.Errs` to `reports` before `continue`. [NO NEW TESTS NEEDED] Signed-off-by: Toshiki Sonoda --- pkg/domain/infra/abi/pods.go | 3 +++ 1 file changed, 3 insertions(+) (limited to 'pkg') diff --git a/pkg/domain/infra/abi/pods.go b/pkg/domain/infra/abi/pods.go index 9a16f7fcd..03c8082c4 100644 --- a/pkg/domain/infra/abi/pods.go +++ b/pkg/domain/infra/abi/pods.go @@ -138,6 +138,7 @@ func (ic *ContainerEngine) PodPause(ctx context.Context, namesOrIds []string, op errs, err := p.Pause(ctx) if err != nil && !errors.Is(err, define.ErrPodPartialFail) { report.Errs = []error{err} + reports = append(reports, &report) continue } if len(errs) > 0 { @@ -171,6 +172,7 @@ func (ic *ContainerEngine) PodUnpause(ctx context.Context, namesOrIds []string, errs, err := p.Unpause(ctx) if err != nil && !errors.Is(err, define.ErrPodPartialFail) { report.Errs = []error{err} + reports = append(reports, &report) continue } if len(errs) > 0 { @@ -196,6 +198,7 @@ func (ic *ContainerEngine) PodStop(ctx context.Context, namesOrIds []string, opt errs, err := p.StopWithTimeout(ctx, false, options.Timeout) if err != nil && !errors.Is(err, define.ErrPodPartialFail) { report.Errs = []error{err} + reports = append(reports, &report) continue } if len(errs) > 0 { -- cgit v1.2.3-54-g00ecf From c3c07ed09eb9e5998f2c67f1054850a5b8266aae Mon Sep 17 00:00:00 2001 From: Urvashi Mohnani Date: Tue, 19 Jul 2022 11:58:27 -0400 Subject: Update init ctr default for play kube Update the init container type default to once instead of always to match k8s behavior. Add a new annotation that can be used to change the init ctr type in the kube yaml. Signed-off-by: Urvashi Mohnani --- docs/source/markdown/podman-play-kube.1.md | 2 +- libpod/define/annotations.go | 5 +++++ pkg/domain/infra/abi/play.go | 6 +++++- test/e2e/play_kube_test.go | 29 +++++++++++++++++++++++++++-- 4 files changed, 38 insertions(+), 4 deletions(-) (limited to 'pkg') diff --git a/docs/source/markdown/podman-play-kube.1.md b/docs/source/markdown/podman-play-kube.1.md index 47e438bd6..66341d875 100644 --- a/docs/source/markdown/podman-play-kube.1.md +++ b/docs/source/markdown/podman-play-kube.1.md @@ -22,7 +22,7 @@ Currently, the supported Kubernetes kinds are: Only two volume types are supported by play kube, the *hostPath* and *persistentVolumeClaim* volume types. For the *hostPath* volume type, only the *default (empty)*, *DirectoryOrCreate*, *Directory*, *FileOrCreate*, *File*, *Socket*, *CharDevice* and *BlockDevice* subtypes are supported. Podman interprets the value of *hostPath* *path* as a file path when it contains at least one forward slash, otherwise Podman treats the value as the name of a named volume. When using a *persistentVolumeClaim*, the value for *claimName* is the name for the Podman named volume. -Note: When playing a kube YAML with init containers, the init container will be created with init type value `always`. +Note: When playing a kube YAML with init containers, the init container will be created with init type value `once`. To change the default type, use the `io.podman.annotations.init.container.type` annotation to set the type to `always`. Note: *hostPath* volume types created by play kube will be given an SELinux shared label (z), bind mounts are not relabeled (use `chcon -t container_file_t -R `). diff --git a/libpod/define/annotations.go b/libpod/define/annotations.go index 8f5279981..580286d6a 100644 --- a/libpod/define/annotations.go +++ b/libpod/define/annotations.go @@ -135,6 +135,11 @@ const ( // creating a checkpoint image to specify the name of host distribution on // which the checkpoint was created. CheckpointAnnotationDistributionName = "io.podman.annotations.checkpoint.distribution.name" + + // InitContainerType is used by play kube when playing a kube yaml to specify the type + // of the init container. + InitContainerType = "io.podman.annotations.init.container.type" + // MaxKubeAnnotation is the max length of annotations allowed by Kubernetes. MaxKubeAnnotation = 63 ) diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index c913fdb69..8b47eff53 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -507,13 +507,17 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY for k, v := range podSpec.PodSpecGen.Labels { // add podYAML labels labels[k] = v } + initCtrType := annotations[define.InitContainerType] + if initCtrType == "" { + initCtrType = define.OneShotInitContainer + } specgenOpts := kube.CtrSpecGenOptions{ Annotations: annotations, ConfigMaps: configMaps, Container: initCtr, Image: pulledImage, - InitContainerType: define.AlwaysInitContainer, + InitContainerType: initCtrType, Labels: labels, LogDriver: options.LogDriver, LogOptions: options.LogOptions, diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 457aaebb2..9e2d4de19 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -1559,8 +1559,10 @@ var _ = Describe("Podman play kube", func() { }) // If you have an init container in the pod yaml, podman should create and run the init container with play kube - It("podman play kube test with init containers", func() { - pod := getPod(withPodInitCtr(getCtr(withImage(ALPINE), withCmd([]string{"echo", "hello"}), withInitCtr(), withName("init-test"))), withCtr(getCtr(withImage(ALPINE), withCmd([]string{"top"})))) + // With annotation set to always + It("podman play kube test with init containers and annotation set", func() { + // With the init container type annotation set to always + pod := getPod(withAnnotation("io.podman.annotations.init.container.type", "always"), withPodInitCtr(getCtr(withImage(ALPINE), withCmd([]string{"echo", "hello"}), withInitCtr(), withName("init-test"))), withCtr(getCtr(withImage(ALPINE), withCmd([]string{"top"})))) err := generateKubeYaml("pod", pod, kubeYaml) Expect(err).To(BeNil()) @@ -1585,6 +1587,29 @@ var _ = Describe("Podman play kube", func() { Expect(inspect.OutputToString()).To(ContainSubstring("running")) }) + // If you have an init container in the pod yaml, podman should create and run the init container with play kube + // Using default init container type (once) + It("podman play kube test with init container type set to default value", func() { + // Using the default init container type (once) + pod := getPod(withPodInitCtr(getCtr(withImage(ALPINE), withCmd([]string{"echo", "hello"}), withInitCtr(), withName("init-test"))), withCtr(getCtr(withImage(ALPINE), withCmd([]string{"top"})))) + err := generateKubeYaml("pod", pod, kubeYaml) + Expect(err).To(BeNil()) + + kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + // Expect the number of containers created to be 2, infra and regular container + numOfCtrs := podmanTest.NumberOfContainers() + Expect(numOfCtrs).To(Equal(2)) + + // Regular container should be in running state + inspect := podmanTest.Podman([]string{"inspect", "--format", "{{.State.Status}}", "testPod-" + defaultCtrName}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).Should(Exit(0)) + Expect(inspect.OutputToString()).To(ContainSubstring("running")) + }) + // If you supply only args for a Container, the default Entrypoint defined in the Docker image is run with the args that you supplied. It("podman play kube test correct command with only set args in yaml file", func() { pod := getPod(withCtr(getCtr(withImage(REGISTRY_IMAGE), withCmd(nil), withArg([]string{"echo", "hello"})))) -- cgit v1.2.3-54-g00ecf From d151edeeaf4ce35586952ceef48a0c7c054c583b Mon Sep 17 00:00:00 2001 From: Ashley Cui Date: Wed, 20 Jul 2022 13:29:46 -0400 Subject: Fix machine test DownloadVMImage takes an extra argument. Signed-off-by: Ashley Cui --- pkg/machine/e2e/machine_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/machine/e2e/machine_test.go b/pkg/machine/e2e/machine_test.go index 7b063937d..93eabdad3 100644 --- a/pkg/machine/e2e/machine_test.go +++ b/pkg/machine/e2e/machine_test.go @@ -57,7 +57,7 @@ var _ = BeforeSuite(func() { Fail(fmt.Sprintf("unable to create url for download: %q", err)) } now := time.Now() - if err := machine.DownloadVMImage(getMe, fqImageName+".xz"); err != nil { + if err := machine.DownloadVMImage(getMe, suiteImageName, fqImageName+".xz"); err != nil { Fail(fmt.Sprintf("unable to download machine image: %q", err)) } fmt.Println("Download took: ", time.Since(now).String()) -- cgit v1.2.3-54-g00ecf From 73ecc5a4bdd620124650dbdb621df667beb8438f Mon Sep 17 00:00:00 2001 From: Aditya R Date: Tue, 12 Jul 2022 18:34:44 +0530 Subject: pkg,libpod: remove pkg/hooks and use hooks from c/common PR https://github.com/containers/common/pull/1071 moved `pkg/hooks` to `c/common` hence remove that from podman and use `pkg/hooks` from `c/common` [NO NEW TESTS NEEDED] [NO TESTS NEEDED] Signed-off-by: Aditya R --- libpod/container_internal.go | 4 +- pkg/hooks/0.1.0/hook.go | 88 -------- pkg/hooks/0.1.0/hook_test.go | 182 ---------------- pkg/hooks/1.0.0/hook.go | 89 -------- pkg/hooks/1.0.0/hook_test.go | 214 ------------------- pkg/hooks/1.0.0/when.go | 96 --------- pkg/hooks/1.0.0/when_test.go | 329 ----------------------------- pkg/hooks/README.md | 22 -- pkg/hooks/docs/.gitignore | 1 - pkg/hooks/docs/oci-hooks.5.md | 182 ---------------- pkg/hooks/exec/exec.go | 69 ------ pkg/hooks/exec/exec_test.go | 222 ------------------- pkg/hooks/exec/runtimeconfigfilter.go | 72 ------- pkg/hooks/exec/runtimeconfigfilter_test.go | 265 ----------------------- pkg/hooks/hooks.go | 145 ------------- pkg/hooks/hooks_test.go | 218 ------------------- pkg/hooks/monitor.go | 66 ------ pkg/hooks/monitor_test.go | 324 ---------------------------- pkg/hooks/read.go | 101 --------- pkg/hooks/read_test.go | 194 ----------------- pkg/hooks/version.go | 6 - 21 files changed, 2 insertions(+), 2887 deletions(-) delete mode 100644 pkg/hooks/0.1.0/hook.go delete mode 100644 pkg/hooks/0.1.0/hook_test.go delete mode 100644 pkg/hooks/1.0.0/hook.go delete mode 100644 pkg/hooks/1.0.0/hook_test.go delete mode 100644 pkg/hooks/1.0.0/when.go delete mode 100644 pkg/hooks/1.0.0/when_test.go delete mode 100644 pkg/hooks/README.md delete mode 100644 pkg/hooks/docs/.gitignore delete mode 100644 pkg/hooks/docs/oci-hooks.5.md delete mode 100644 pkg/hooks/exec/exec.go delete mode 100644 pkg/hooks/exec/exec_test.go delete mode 100644 pkg/hooks/exec/runtimeconfigfilter.go delete mode 100644 pkg/hooks/exec/runtimeconfigfilter_test.go delete mode 100644 pkg/hooks/hooks.go delete mode 100644 pkg/hooks/hooks_test.go delete mode 100644 pkg/hooks/monitor.go delete mode 100644 pkg/hooks/monitor_test.go delete mode 100644 pkg/hooks/read.go delete mode 100644 pkg/hooks/read_test.go delete mode 100644 pkg/hooks/version.go (limited to 'pkg') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index caf7f3849..7e330430c 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -22,12 +22,12 @@ import ( "github.com/containers/common/pkg/cgroups" "github.com/containers/common/pkg/chown" "github.com/containers/common/pkg/config" + "github.com/containers/common/pkg/hooks" + "github.com/containers/common/pkg/hooks/exec" cutil "github.com/containers/common/pkg/util" "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/libpod/events" "github.com/containers/podman/v4/pkg/ctime" - "github.com/containers/podman/v4/pkg/hooks" - "github.com/containers/podman/v4/pkg/hooks/exec" "github.com/containers/podman/v4/pkg/lookup" "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/selinux" diff --git a/pkg/hooks/0.1.0/hook.go b/pkg/hooks/0.1.0/hook.go deleted file mode 100644 index c3df5fa1d..000000000 --- a/pkg/hooks/0.1.0/hook.go +++ /dev/null @@ -1,88 +0,0 @@ -// Package hook is the 0.1.0 hook configuration structure. -package hook - -import ( - "encoding/json" - "errors" - "strings" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - rspec "github.com/opencontainers/runtime-spec/specs-go" -) - -// Version is the hook configuration version defined in this package. -const Version = "0.1.0" - -// Hook is the hook configuration structure. -type Hook struct { - Hook *string `json:"hook"` - Arguments []string `json:"arguments,omitempty"` - - // https://github.com/cri-o/cri-o/pull/1235 - Stages []string `json:"stages"` - Stage []string `json:"stage"` - - Cmds []string `json:"cmds,omitempty"` - Cmd []string `json:"cmd,omitempty"` - - Annotations []string `json:"annotations,omitempty"` - Annotation []string `json:"annotation,omitempty"` - - HasBindMounts *bool `json:"hasbindmounts,omitempty"` -} - -func Read(content []byte) (hook *current.Hook, err error) { - var raw Hook - - if err = json.Unmarshal(content, &raw); err != nil { - return nil, err - } - - if raw.Hook == nil { - return nil, errors.New("missing required property: hook") - } - - if raw.Stages == nil { - raw.Stages = raw.Stage - } else if raw.Stage != nil { - return nil, errors.New("cannot set both 'stage' and 'stages'") - } - if raw.Stages == nil { - return nil, errors.New("missing required property: stages") - } - - if raw.Cmds == nil { - raw.Cmds = raw.Cmd - } else if raw.Cmd != nil { - return nil, errors.New("cannot set both 'cmd' and 'cmds'") - } - - if raw.Annotations == nil { - raw.Annotations = raw.Annotation - } else if raw.Annotation != nil { - return nil, errors.New("cannot set both 'annotation' and 'annotations'") - } - - hook = ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: *raw.Hook, - }, - When: current.When{ - Commands: raw.Cmds, - HasBindMounts: raw.HasBindMounts, - Or: true, - }, - Stages: raw.Stages, - } - if raw.Arguments != nil { - hook.Hook.Args = append([]string{*raw.Hook}, raw.Arguments...) - } - if raw.Annotations != nil { - hook.When.Annotations = map[string]string{ - ".*": strings.Join(raw.Annotations, "|"), - } - } - - return hook, nil -} diff --git a/pkg/hooks/0.1.0/hook_test.go b/pkg/hooks/0.1.0/hook_test.go deleted file mode 100644 index 5beadbaaf..000000000 --- a/pkg/hooks/0.1.0/hook_test.go +++ /dev/null @@ -1,182 +0,0 @@ -package hook - -import ( - "testing" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -func TestGood(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"cmds\": [\"sh\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Commands: []string{"sh"}, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestInvalidJSON(t *testing.T) { - _, err := Read([]byte("{")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^unexpected end of JSON input$", err.Error()) -} - -func TestArguments(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"arguments\": [\"d\", \"e\"], \"stages\": [\"prestart\"], \"cmds\": [\"sh\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - Args: []string{"/a/b/c", "d", "e"}, - }, - When: current.When{ - Commands: []string{"sh"}, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestEmptyObject(t *testing.T) { - _, err := Read([]byte("{}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^missing required property: hook$", err.Error()) -} - -func TestNoStages(t *testing.T) { - _, err := Read([]byte("{\"hook\": \"/a/b/c\"}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^missing required property: stages$", err.Error()) -} - -func TestStage(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{Or: true}, - Stages: []string{"prestart"}, - }, hook) -} - -func TestStagesAndStage(t *testing.T) { - _, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"stage\": [\"prestart\"]}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^cannot set both 'stage' and 'stages'$", err.Error()) -} - -func TestCmd(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"cmd\": [\"sh\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Commands: []string{"sh"}, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestCmdsAndCmd(t *testing.T) { - _, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"cmds\": [\"sh\"], \"cmd\": [\"true\"]}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^cannot set both 'cmd' and 'cmds'$", err.Error()) -} - -func TestAnnotations(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"annotations\": [\"a\", \"b\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Annotations: map[string]string{".*": "a|b"}, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestAnnotation(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"annotation\": [\"a\", \"b\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Annotations: map[string]string{".*": "a|b"}, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestAnnotationsAndAnnotation(t *testing.T) { - _, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"annotations\": [\"a\"], \"annotation\": [\"b\"]}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^cannot set both 'annotation' and 'annotations'$", err.Error()) -} - -func TestHasBindMounts(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"hasbindmounts\": true}")) - if err != nil { - t.Fatal(err) - } - hasBindMounts := true - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - HasBindMounts: &hasBindMounts, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} diff --git a/pkg/hooks/1.0.0/hook.go b/pkg/hooks/1.0.0/hook.go deleted file mode 100644 index 71f940a64..000000000 --- a/pkg/hooks/1.0.0/hook.go +++ /dev/null @@ -1,89 +0,0 @@ -// Package hook is the 1.0.0 hook configuration structure. -package hook - -import ( - "encoding/json" - "errors" - "fmt" - "os" - "regexp" - - rspec "github.com/opencontainers/runtime-spec/specs-go" -) - -// Version is the hook configuration version defined in this package. -const Version = "1.0.0" - -// Hook is the hook configuration structure. -type Hook struct { - Version string `json:"version"` - Hook rspec.Hook `json:"hook"` - When When `json:"when"` - Stages []string `json:"stages"` -} - -// Read reads hook JSON bytes, verifies them, and returns the hook configuration. -func Read(content []byte) (hook *Hook, err error) { - if err = json.Unmarshal(content, &hook); err != nil { - return nil, err - } - return hook, nil -} - -// Validate performs load-time hook validation. -func (hook *Hook) Validate(extensionStages []string) (err error) { - if hook == nil { - return errors.New("nil hook") - } - - if hook.Version != Version { - return fmt.Errorf("unexpected hook version %q (expecting %v)", hook.Version, Version) - } - - if hook.Hook.Path == "" { - return errors.New("missing required property: hook.path") - } - - if _, err := os.Stat(hook.Hook.Path); err != nil { - return err - } - - for key, value := range hook.When.Annotations { - if _, err = regexp.Compile(key); err != nil { - return fmt.Errorf("invalid annotation key %q: %w", key, err) - } - if _, err = regexp.Compile(value); err != nil { - return fmt.Errorf("invalid annotation value %q: %w", value, err) - } - } - - for _, command := range hook.When.Commands { - if _, err = regexp.Compile(command); err != nil { - return fmt.Errorf("invalid command %q: %w", command, err) - } - } - - if hook.Stages == nil { - return errors.New("missing required property: stages") - } - - validStages := map[string]bool{ - "createContainer": true, - "createRuntime": true, - "prestart": true, - "poststart": true, - "poststop": true, - "startContainer": true, - } - for _, stage := range extensionStages { - validStages[stage] = true - } - - for _, stage := range hook.Stages { - if !validStages[stage] { - return fmt.Errorf("unknown stage %q", stage) - } - } - - return nil -} diff --git a/pkg/hooks/1.0.0/hook_test.go b/pkg/hooks/1.0.0/hook_test.go deleted file mode 100644 index bd6d6b654..000000000 --- a/pkg/hooks/1.0.0/hook_test.go +++ /dev/null @@ -1,214 +0,0 @@ -package hook - -import ( - "os" - "path/filepath" - "runtime" - "testing" - - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -// path is the path to an example hook executable. -var path string - -func TestGoodRead(t *testing.T) { - hook, err := Read([]byte("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"/a/b/c\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}")) - if err != nil { - t.Fatal(err) - } - always := true - assert.Equal(t, &Hook{ - Version: Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: When{ - Always: &always, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestInvalidJSON(t *testing.T) { - _, err := Read([]byte("{")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^unexpected end of JSON input$", err.Error()) -} - -func TestGoodValidate(t *testing.T) { - always := true - hook := &Hook{ - Version: Version, - Hook: rspec.Hook{ - Path: path, - }, - When: When{ - Always: &always, - }, - Stages: []string{"prestart"}, - } - err := hook.Validate([]string{}) - if err != nil { - t.Fatal(err) - } -} - -func TestNilValidation(t *testing.T) { - var hook *Hook - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^nil hook$", err.Error()) -} - -func TestWrongVersion(t *testing.T) { - hook := Hook{Version: "0.1.0"} - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^unexpected hook version \"0.1.0\" \\(expecting 1.0.0\\)$", err.Error()) -} - -func TestNoHookPath(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{}, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^missing required property: hook.path$", err.Error()) -} - -func TestUnknownHookPath(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: filepath.Join("does", "not", "exist"), - }, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^stat does/not/exist: no such file or directory$", err.Error()) - if !os.IsNotExist(err) { - t.Fatal("opaque wrapping for not-exist errors") - } -} - -func TestNoStages(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^missing required property: stages$", err.Error()) -} - -func TestInvalidStage(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - Stages: []string{"does-not-exist"}, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^unknown stage \"does-not-exist\"$", err.Error()) -} - -func TestExtensionStage(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - Stages: []string{"prestart", "b"}, - } - err := hook.Validate([]string{"a", "b", "c"}) - if err != nil { - t.Fatal(err) - } -} - -func TestInvalidAnnotationKey(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - When: When{ - Annotations: map[string]string{ - "[": "a", - }, - }, - Stages: []string{"prestart"}, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^invalid annotation key \"\\[\": error parsing regexp: .*", err.Error()) -} - -func TestInvalidAnnotationValue(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - When: When{ - Annotations: map[string]string{ - "a": "[", - }, - }, - Stages: []string{"prestart"}, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^invalid annotation value \"\\[\": error parsing regexp: .*", err.Error()) -} - -func TestInvalidCommand(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - When: When{ - Commands: []string{"["}, - }, - Stages: []string{"prestart"}, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^invalid command \"\\[\": error parsing regexp: .*", err.Error()) -} - -func init() { - if runtime.GOOS != "windows" { - path = "/bin/sh" - } else { - panic("we need a reliable executable path on Windows") - } -} diff --git a/pkg/hooks/1.0.0/when.go b/pkg/hooks/1.0.0/when.go deleted file mode 100644 index a1351890f..000000000 --- a/pkg/hooks/1.0.0/when.go +++ /dev/null @@ -1,96 +0,0 @@ -package hook - -import ( - "errors" - "fmt" - "regexp" - - rspec "github.com/opencontainers/runtime-spec/specs-go" -) - -// When holds hook-injection conditions. -type When struct { - Always *bool `json:"always,omitempty"` - Annotations map[string]string `json:"annotations,omitempty"` - Commands []string `json:"commands,omitempty"` - HasBindMounts *bool `json:"hasBindMounts,omitempty"` - - // Or enables any-of matching. - // - // Deprecated: this property is for is backwards-compatibility with - // 0.1.0 hooks. It will be removed when we drop support for them. - Or bool `json:"-"` -} - -// Match returns true if the given conditions match the configuration. -func (when *When) Match(config *rspec.Spec, annotations map[string]string, hasBindMounts bool) (match bool, err error) { - matches := 0 - - if when.Always != nil { - if *when.Always { - if when.Or { - return true, nil - } - matches++ - } else if !when.Or { - return false, nil - } - } - - if when.HasBindMounts != nil { - if *when.HasBindMounts && hasBindMounts { - if when.Or { - return true, nil - } - matches++ - } else if !when.Or { - return false, nil - } - } - - for keyPattern, valuePattern := range when.Annotations { - match := false - for key, value := range annotations { - match, err = regexp.MatchString(keyPattern, key) - if err != nil { - return false, fmt.Errorf("annotation key: %w", err) - } - if match { - match, err = regexp.MatchString(valuePattern, value) - if err != nil { - return false, fmt.Errorf("annotation value: %w", err) - } - if match { - break - } - } - } - if match { - if when.Or { - return true, nil - } - matches++ - } else if !when.Or { - return false, nil - } - } - - if config.Process != nil && len(when.Commands) > 0 { - if len(config.Process.Args) == 0 { - return false, errors.New("process.args must have at least one entry") - } - command := config.Process.Args[0] - for _, cmdPattern := range when.Commands { - match, err := regexp.MatchString(cmdPattern, command) - if err != nil { - return false, fmt.Errorf("command: %w", err) - } - if match { - return true, nil - } - } - return false, nil - } - - return matches > 0, nil -} diff --git a/pkg/hooks/1.0.0/when_test.go b/pkg/hooks/1.0.0/when_test.go deleted file mode 100644 index 94b0c3830..000000000 --- a/pkg/hooks/1.0.0/when_test.go +++ /dev/null @@ -1,329 +0,0 @@ -package hook - -import ( - "fmt" - "testing" - - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -func TestNoMatch(t *testing.T) { - config := &rspec.Spec{} - for _, o := range []bool{true, false} { - or := o - t.Run(fmt.Sprintf("or %t", or), func(t *testing.T) { - when := When{Or: or} - match, err := when.Match(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, false, match) - }) - } -} - -func TestAlways(t *testing.T) { - config := &rspec.Spec{} - processStruct := &rspec.Process{ - Args: []string{"/bin/sh", "a", "b"}, - } - for _, a := range []bool{true, false} { - always := a - for _, o := range []bool{true, false} { - or := o - for _, p := range []*rspec.Process{processStruct, nil} { - process := p - t.Run(fmt.Sprintf("always %t, or %t, has process %t", always, or, process != nil), func(t *testing.T) { - config.Process = process - when := When{Always: &always, Or: or} - match, err := when.Match(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, always, match) - }) - } - } - } -} - -func TestHasBindMountsAnd(t *testing.T) { - hasBindMounts := true - when := When{HasBindMounts: &hasBindMounts} - config := &rspec.Spec{} - for _, b := range []bool{false, true} { - containerHasBindMounts := b - t.Run(fmt.Sprintf("%t", containerHasBindMounts), func(t *testing.T) { - match, err := when.Match(config, map[string]string{}, containerHasBindMounts) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, containerHasBindMounts, match) - }) - } -} - -func TestHasBindMountsOr(t *testing.T) { - hasBindMounts := true - when := When{HasBindMounts: &hasBindMounts, Or: true} - config := &rspec.Spec{} - for _, b := range []bool{false, true} { - containerHasBindMounts := b - t.Run(fmt.Sprintf("%t", containerHasBindMounts), func(t *testing.T) { - match, err := when.Match(config, map[string]string{}, containerHasBindMounts) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, containerHasBindMounts, match) - }) - } -} - -func TestAnnotations(t *testing.T) { - when := When{ - Annotations: map[string]string{ - "^a$": "^b$", - "^c$": "^d$", - }, - } - config := &rspec.Spec{} - for _, tt := range []struct { - name string - annotations map[string]string - or bool - match bool - }{ - { - name: "matching both, and", - annotations: map[string]string{ - "a": "b", - "c": "d", - "e": "f", - }, - or: false, - match: true, - }, - { - name: "matching one, and", - annotations: map[string]string{ - "a": "b", - }, - or: false, - match: false, - }, - { - name: "matching one, or", - annotations: map[string]string{ - "a": "b", - }, - or: true, - match: true, - }, - { - name: "key-only, or", - annotations: map[string]string{ - "a": "bc", - }, - or: true, - match: false, - }, - { - name: "value-only, or", - annotations: map[string]string{ - "ac": "b", - }, - or: true, - match: false, - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - when.Or = test.or - match, err := when.Match(config, test.annotations, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.match, match) - }) - } -} - -func TestCommands(t *testing.T) { - when := When{ - Commands: []string{ - "^/bin/sh$", - }, - } - config := &rspec.Spec{} - for _, tt := range []struct { - name string - process *rspec.Process - match bool - }{ - { - name: "good", - process: &rspec.Process{ - Args: []string{"/bin/sh", "a", "b"}, - }, - match: true, - }, - { - name: "extra characters", - process: &rspec.Process{ - Args: []string{"/bin/shell", "a", "b"}, - }, - match: false, - }, - { - name: "process unset", - match: false, - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - config.Process = test.process - match, err := when.Match(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.match, match) - }) - } -} - -func TestCommandsEmptyProcessArgs(t *testing.T) { - when := When{ - Commands: []string{ - "^/bin/sh$", - }, - } - config := &rspec.Spec{ - Process: &rspec.Process{}, - } - _, err := when.Match(config, map[string]string{}, false) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^process\\.args must have at least one entry$", err.Error()) -} - -func TestHasBindMountsAndCommands(t *testing.T) { - hasBindMounts := true - when := When{ - HasBindMounts: &hasBindMounts, - Commands: []string{ - "^/bin/sh$", - }, - } - config := &rspec.Spec{Process: &rspec.Process{}} - for _, tt := range []struct { - name string - command string - hasBindMounts bool - or bool - match bool - }{ - { - name: "both, and", - command: "/bin/sh", - hasBindMounts: true, - or: false, - match: true, - }, - { - name: "both, or", - command: "/bin/sh", - hasBindMounts: true, - or: true, - match: true, - }, - { - name: "bind, and", - command: "/bin/shell", - hasBindMounts: true, - or: false, - match: false, - }, - { - name: "bind, or", - command: "/bin/shell", - hasBindMounts: true, - or: true, - match: true, - }, - { - name: "command, and", - command: "/bin/sh", - hasBindMounts: false, - or: false, - match: false, - }, - { - name: "command, or", - command: "/bin/sh", - hasBindMounts: false, - or: true, - match: true, - }, - { - name: "neither, and", - command: "/bin/shell", - hasBindMounts: false, - or: false, - match: false, - }, - { - name: "neither, or", - command: "/bin/shell", - hasBindMounts: false, - or: true, - match: false, - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - config.Process.Args = []string{test.command} - when.Or = test.or - match, err := when.Match(config, map[string]string{}, test.hasBindMounts) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.match, match) - }) - } -} - -func TestInvalidRegexp(t *testing.T) { - config := &rspec.Spec{Process: &rspec.Process{Args: []string{"/bin/sh"}}} - for _, tt := range []struct { - name string - when When - expected string - }{ - { - name: "invalid-annotation-key", - when: When{Annotations: map[string]string{"[": "a"}}, - expected: "^annotation key: error parsing regexp: .*", - }, - { - name: "invalid-annotation-value", - when: When{Annotations: map[string]string{"a": "["}}, - expected: "^annotation value: error parsing regexp: .*", - }, - { - name: "invalid-command", - when: When{Commands: []string{"["}}, - expected: "^command: error parsing regexp: .*", - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - _, err := test.when.Match(config, map[string]string{"a": "b"}, false) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, test.expected, err.Error()) - }) - } -} diff --git a/pkg/hooks/README.md b/pkg/hooks/README.md deleted file mode 100644 index f6a03a775..000000000 --- a/pkg/hooks/README.md +++ /dev/null @@ -1,22 +0,0 @@ -# OCI Hooks Configuration - -For POSIX platforms, the [OCI runtime configuration][runtime-spec] supports [hooks][spec-hooks] for configuring custom actions related to the life cycle of the container. -The way you enable the hooks above is by editing the OCI runtime configuration before running the OCI runtime (e.g. [`runc`][runc]). -CRI-O and `podman create` create the OCI configuration for you, and this documentation allows developers to configure them to set their intended hooks. - -One problem with hooks is that the runtime actually stalls execution of the container before running the hooks and stalls completion of the container, until all hooks complete. -This can cause some performance issues. -Also a lot of hooks just check if certain configuration is set and then exit early, without doing anything. -For example the [oci-systemd-hook][] only executes if the command is `init` or `systemd`, otherwise it just exits. -This means if we automatically enabled all hooks, every container would have to execute `oci-systemd-hook`, even if they don't run systemd inside of the container. -Performance would also suffer if we executed each hook at each stage ([pre-start][], [post-start][], and [post-stop][]). - -The hooks configuration is documented in [`oci-hooks.5`](docs/oci-hooks.5.md). - -[oci-systemd-hook]: https://github.com/projectatomic/oci-systemd-hook -[post-start]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#poststart -[post-stop]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#poststop -[pre-start]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#prestart -[runc]: https://github.com/opencontainers/runc -[runtime-spec]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/spec.md -[spec-hooks]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#posix-platform-hooks diff --git a/pkg/hooks/docs/.gitignore b/pkg/hooks/docs/.gitignore deleted file mode 100644 index 5d06cd466..000000000 --- a/pkg/hooks/docs/.gitignore +++ /dev/null @@ -1 +0,0 @@ -*.5 diff --git a/pkg/hooks/docs/oci-hooks.5.md b/pkg/hooks/docs/oci-hooks.5.md deleted file mode 100644 index 9a1a35682..000000000 --- a/pkg/hooks/docs/oci-hooks.5.md +++ /dev/null @@ -1,182 +0,0 @@ -% oci-hooks 5 OCI Hooks Configuration -% W. Trevor King -% MAY 2018 - -# NAME - -oci-hooks - OCI hooks configuration directories - -# SYNOPSIS - -`/usr/share/containers/oci/hooks.d/*.json` - -# DESCRIPTION - -Provides a way for users to configure the intended hooks for Open Container Initiative containers so they will only be executed for containers that need their functionality, and then only for the stages where they're needed. - -## Directories - -Hooks are configured with JSON files (ending with a `.json` extension) in a series of hook directories. -The default directory is `/usr/share/containers/oci/hooks.d`, but tools consuming this format may change that default, include additional directories, or provide their callers with ways to adjust the configuration directories. - -If multiple directories are configured, a JSON filename in a preferred directory masks entries with the same filename in directories with lower precedence. For example, if a consuming tool watches for hooks in `/etc/containers/oci/hooks.d` and `/usr/share/containers/oci/hooks.d` (in order of decreasing precedence), then a hook definition in `/etc/containers/oci/hooks.d/01-my-hook.json` will mask any definition in `/usr/share/containers/oci/hooks.d/01-my-hook.json`. - -Tools consuming this format may also opt to monitor the hook directories for changes, in which case they will notice additions, changes, and removals to JSON files without needing to be restarted or otherwise signaled. When the tool monitors multiple hooks directories, the precedence discussed in the previous paragraph still applies. For example, if a consuming tool watches for hooks in `/etc/containers/oci/hooks.d` and `/usr/share/containers/oci/hooks.d` (in order of decreasing precedence), then writing a new hook definition to `/etc/containers/oci/hooks.d/01-my-hook.json` will mask the hook previously loaded from `/usr/share/containers/oci/hooks.d/01-my-hook.json`. Subsequent changes to `/usr/share/containers/oci/hooks.d/01-my-hook.json` will have no effect on the consuming tool as long as `/etc/containers/oci/hooks.d/01-my-hook.json` exists. Removing `/etc/containers/oci/hooks.d/01-my-hook.json` will reload the hook from `/usr/share/containers/oci/hooks.d/01-my-hook.json`. - -Hooks are injected in the order obtained by sorting the JSON file names, after converting them to lower case, based on their Unicode code points. -For example, a matching hook defined in `01-my-hook.json` would be injected before matching hooks defined in `02-another-hook.json` and `01-UPPERCASE.json`. -It is strongly recommended to make the sort order unambiguous depending on an ASCII-only prefix (like the `01`/`02` above). - -Each JSON file should contain an object with one of the following schemas. - -## 1.0.0 Hook Schema - -`version` (required string) - Sets the hook-definition version. For this schema version, the value be `1.0.0`. - -`hook` (required object) - The hook to inject, with the hook-entry schema defined by the 1.0.1 OCI Runtime Specification. - -`when` (required object) - Conditions under which the hook is injected. The following properties can be specified, and at least one must be specified: - - * `always` (optional boolean) - If set `true`, this condition matches. - * `annotations` (optional object) - If all `annotations` key/value pairs match a key/value pair from the configured annotations, this condition matches. - Both keys and values must be POSIX extended regular expressions. - * `commands` (optional array of strings) - If the configured `process.args[0]` matches an entry, this condition matches. - Entries must be POSIX extended regular expressions. - * `hasBindMounts` (optional boolean) - If `hasBindMounts` is true and the caller requested host-to-container bind mounts, this condition matches. - -`stages` (required array of strings) - Stages when the hook must be injected. Entries must be chosen from the 1.0.1 OCI Runtime Specification hook stages or from extension stages supported by the package consumer. - -If *all* of the conditions set in `when` match, then the `hook` must be injected for the stages set in `stages`. - -## 0.1.0 Hook Schema - -`hook` (required string) - Sets `path` in the injected hook. - -`arguments` (optional array of strings) - Additional arguments to pass to the hook. The injected hook's `args` is `hook` with `arguments` appended. - -`stages` (required array of strings) - Stages when the hook must be injected. `stage` is an allowed synonym for this property, but you must not set both `stages` and `stage`. Entries must be chosen from the 1.0.1 OCI Runtime Specification hook stages or from extension stages supported by the package consumer. - -`cmds` (optional array of strings) - The hook must be injected if the configured `process.args[0]` matches an entry. `cmd` is an allowed synonym for this property, but you must not set both `cmds` and `cmd`. Entries must be POSIX extended regular expressions. - -`annotations` (optional array of strings) - The hook must be injected if an `annotations` entry matches a value from the configured annotations. `annotation` is an allowed synonym for this property, but you must not set both `annotations` and `annotation`. Entries must be POSIX extended regular expressions. - -`hasbindmounts` (optional boolean) - The hook must be injected if `hasBindMounts` is true and the caller requested host-to-container bind mounts. - -# EXAMPLE - -## 1.0.0 Hook Schema - -The following configuration injects `oci-systemd-hook` in the pre-start and post-stop stages if `process.args[0]` ends with `/init` or `/systemd`: - -```console -$ cat /etc/containers/oci/hooks.d/oci-systemd-hook.json -{ - "version": "1.0.0", - "hook": { - "path": "/usr/libexec/oci/hooks.d/oci-systemd-hook" - }, - "when": { - "commands": [".*/init$" , ".*/systemd$"] - }, - "stages": ["prestart", "poststop"] -} -``` - -The following example injects `oci-umount --debug` in the pre-start stage if the container is configured to bind-mount host directories into the container. - -```console -$ cat /etc/containers/oci/hooks.d/oci-umount.json -{ - "version": "1.0.0", - "hook": { - "path": "/usr/libexec/oci/hooks.d/oci-umount", - "args": ["oci-umount", "--debug"], - }, - "when": { - "hasBindMounts": true - }, - "stages": ["prestart"] -} -``` - -The following example injects `nvidia-container-runtime-hook prestart` with particular environment variables in the pre-start stage if the container is configured with an `annotations` entry whose key matches `^com\.example\.department$` and whose value matches `.*fluid-dynamics.*`. - -```console -$ cat /etc/containers/oci/hooks.d/nvidia.json -{ - "version": "1.0.0", - "hook": { - "path": "/usr/sbin/nvidia-container-runtime-hook", - "args": ["nvidia-container-runtime-hook", "prestart"], - "env": [ - "NVIDIA_REQUIRE_CUDA=cuda>=9.1", - "NVIDIA_VISIBLE_DEVICES=GPU-fef8089b" - ] - }, - "when": { - "annotations": { - "^com\\.example\\.department$": ".*fluid-dynamics$" - } - }, - "stages": ["prestart"] -} -``` - -## 0.1.0 Hook Schema - -The following configuration injects `oci-systemd-hook` in the pre-start and post-stop stages if `process.args[0]` ends with `/init` or `/systemd`: - -```console -$ cat /etc/containers/oci/hooks.d/oci-systemd-hook.json -{ - "cmds": [".*/init$" , ".*/systemd$"], - "hook": "/usr/libexec/oci/hooks.d/oci-systemd-hook", - "stages": ["prestart", "poststop"] -} -``` - -The following example injects `oci-umount --debug` in the pre-start stage if the container is configured to bind-mount host directories into the container. - -```console -$ cat /etc/containers/oci/hooks.d/oci-umount.json -{ - "hook": "/usr/libexec/oci/hooks.d/oci-umount", - "arguments": ["--debug"], - "hasbindmounts": true, - "stages": ["prestart"] -} -``` - -The following example injects `nvidia-container-runtime-hook prestart` in the pre-start stage if the container is configured with an `annotations` entry whose value matches `.*fluid-dynamics.*`. - -```console -$ cat /etc/containers/oci/hooks.d/osystemd-hook.json -{ - "hook": "/usr/sbin/nvidia-container-runtime-hook", - "arguments": ["prestart"], - "annotations: [".*fluid-dynamics.*"], - "stages": ["prestart"] -} -``` - -# SEE ALSO - -`oci-systemd-hook(1)`, `oci-umount(1)`, `locale(7)` - -* [OCI Runtime Specification, 1.0.1, POSIX-platform hooks](https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#posix-platform-hooks) -* [OCI Runtime Specification, 1.0.1, process](https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#process) -* [POSIX extended regular expressions (EREs)](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_04) diff --git a/pkg/hooks/exec/exec.go b/pkg/hooks/exec/exec.go deleted file mode 100644 index bc639245f..000000000 --- a/pkg/hooks/exec/exec.go +++ /dev/null @@ -1,69 +0,0 @@ -// Package exec provides utilities for executing Open Container Initiative runtime hooks. -package exec - -import ( - "bytes" - "context" - "fmt" - "io" - osexec "os/exec" - "time" - - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/sirupsen/logrus" -) - -// DefaultPostKillTimeout is the recommended default post-kill timeout. -const DefaultPostKillTimeout = time.Duration(10) * time.Second - -// Run executes the hook and waits for it to complete or for the -// context or hook-specified timeout to expire. -func Run(ctx context.Context, hook *rspec.Hook, state []byte, stdout io.Writer, stderr io.Writer, postKillTimeout time.Duration) (hookErr, err error) { - cmd := osexec.Cmd{ - Path: hook.Path, - Args: hook.Args, - Env: hook.Env, - Stdin: bytes.NewReader(state), - Stdout: stdout, - Stderr: stderr, - } - if cmd.Env == nil { - cmd.Env = []string{} - } - - if hook.Timeout != nil { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, time.Duration(*hook.Timeout)*time.Second) - defer cancel() - } - - err = cmd.Start() - if err != nil { - return err, err - } - exit := make(chan error, 1) - go func() { - err := cmd.Wait() - if err != nil { - err = fmt.Errorf("executing %v: %w", cmd.Args, err) - } - exit <- err - }() - - select { - case err = <-exit: - return err, err - case <-ctx.Done(): - if err := cmd.Process.Kill(); err != nil { - logrus.Errorf("Failed to kill pid %v", cmd.Process) - } - timer := time.NewTimer(postKillTimeout) - defer timer.Stop() - select { - case <-timer.C: - err = fmt.Errorf("failed to reap process within %s of the kill signal", postKillTimeout) - case err = <-exit: - } - return err, ctx.Err() - } -} diff --git a/pkg/hooks/exec/exec_test.go b/pkg/hooks/exec/exec_test.go deleted file mode 100644 index 1e105373d..000000000 --- a/pkg/hooks/exec/exec_test.go +++ /dev/null @@ -1,222 +0,0 @@ -package exec - -import ( - "bytes" - "context" - "fmt" - "os" - "runtime" - "strings" - "testing" - "time" - - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -// path is the path to an example hook executable. -var path string - -// unavoidableEnvironmentKeys may be injected even if the hook -// executable is executed with a requested empty environment. -var unavoidableEnvironmentKeys []string - -func TestRun(t *testing.T) { - ctx := context.Background() - hook := &rspec.Hook{ - Path: path, - Args: []string{"sh", "-c", "cat"}, - } - var stderr, stdout bytes.Buffer - hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout) - if err != nil { - t.Fatal(err) - } - if hookErr != nil { - t.Fatal(hookErr) - } - assert.Equal(t, "{}", stdout.String()) - assert.Equal(t, "", stderr.String()) -} - -func TestRunIgnoreOutput(t *testing.T) { - ctx := context.Background() - hook := &rspec.Hook{ - Path: path, - Args: []string{"sh", "-c", "cat"}, - } - hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, DefaultPostKillTimeout) - if err != nil { - t.Fatal(err) - } - if hookErr != nil { - t.Fatal(hookErr) - } -} - -func TestRunFailedStart(t *testing.T) { - ctx := context.Background() - hook := &rspec.Hook{ - Path: "/does/not/exist", - } - hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, DefaultPostKillTimeout) - if err == nil { - t.Fatal("unexpected success") - } - if !os.IsNotExist(err) { - t.Fatal(err) - } - assert.Equal(t, err, hookErr) -} - -func parseEnvironment(input string) (env map[string]string, err error) { - env = map[string]string{} - lines := strings.Split(input, "\n") - for i, line := range lines { - if line == "" && i == len(lines)-1 { - continue // no content after the terminal newline - } - keyValue := strings.SplitN(line, "=", 2) - if len(keyValue) < 2 { - return env, fmt.Errorf("no = in environment line: %q", line) - } - env[keyValue[0]] = keyValue[1] - } - for _, key := range unavoidableEnvironmentKeys { - delete(env, key) - } - return env, nil -} - -func TestRunEnvironment(t *testing.T) { - ctx := context.Background() - hook := &rspec.Hook{ - Path: path, - Args: []string{"sh", "-c", "env"}, - } - for _, tt := range []struct { - name string - env []string - expected map[string]string - }{ - { - name: "unset", - expected: map[string]string{}, - }, - { - name: "set empty", - env: []string{}, - expected: map[string]string{}, - }, - { - name: "set", - env: []string{ - "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", - "TERM=xterm", - }, - expected: map[string]string{ - "PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", - "TERM": "xterm", - }, - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - var stderr, stdout bytes.Buffer - hook.Env = test.env - hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout) - if err != nil { - t.Fatal(err) - } - if hookErr != nil { - t.Fatal(hookErr) - } - assert.Equal(t, "", stderr.String()) - - env, err := parseEnvironment(stdout.String()) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.expected, env) - }) - } -} - -func TestRunCancel(t *testing.T) { - hook := &rspec.Hook{ - Path: path, - Args: []string{"sh", "-c", "echo waiting; sleep 2; echo done"}, - } - one := 1 - for _, tt := range []struct { - name string - contextTimeout time.Duration - hookTimeout *int - expectedHookError string - expectedRunError error - expectedStdout string - }{ - { - name: "no timeouts", - expectedStdout: "waiting\ndone\n", - }, - { - name: "context timeout", - contextTimeout: time.Duration(1) * time.Second, - expectedStdout: "waiting\n", - expectedHookError: "^executing \\[sh -c echo waiting; sleep 2; echo done]: signal: killed$", - expectedRunError: context.DeadlineExceeded, - }, - { - name: "hook timeout", - hookTimeout: &one, - expectedStdout: "waiting\n", - expectedHookError: "^executing \\[sh -c echo waiting; sleep 2; echo done]: signal: killed$", - expectedRunError: context.DeadlineExceeded, - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - ctx := context.Background() - var stderr, stdout bytes.Buffer - if test.contextTimeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, test.contextTimeout) - defer cancel() - } - hook.Timeout = test.hookTimeout - hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout) - assert.Equal(t, test.expectedRunError, err) - if test.expectedHookError == "" { - if hookErr != nil { - t.Fatal(hookErr) - } - } else { - assert.Regexp(t, test.expectedHookError, hookErr.Error()) - } - assert.Equal(t, "", stderr.String()) - assert.Equal(t, test.expectedStdout, stdout.String()) - }) - } -} - -func TestRunKillTimeout(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), time.Duration(500)*time.Millisecond) - defer cancel() - hook := &rspec.Hook{ - Path: path, - Args: []string{"sh", "-c", "sleep 1"}, - } - hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, time.Duration(0)) - assert.Equal(t, context.DeadlineExceeded, err) - assert.Regexp(t, "^(failed to reap process within 0s of the kill signal|executing \\[sh -c sleep 1]: signal: killed)$", hookErr) -} - -func init() { - if runtime.GOOS != "windows" { - path = "/bin/sh" - unavoidableEnvironmentKeys = []string{"PWD", "SHLVL", "_"} - } else { - panic("we need a reliable executable path on Windows") - } -} diff --git a/pkg/hooks/exec/runtimeconfigfilter.go b/pkg/hooks/exec/runtimeconfigfilter.go deleted file mode 100644 index 72d4b8979..000000000 --- a/pkg/hooks/exec/runtimeconfigfilter.go +++ /dev/null @@ -1,72 +0,0 @@ -package exec - -import ( - "bytes" - "context" - "encoding/json" - "fmt" - "reflect" - "time" - - "github.com/davecgh/go-spew/spew" - spec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/pmezard/go-difflib/difflib" - "github.com/sirupsen/logrus" -) - -var spewConfig = spew.ConfigState{ - Indent: " ", - DisablePointerAddresses: true, - DisableCapacities: true, - SortKeys: true, -} - -// RuntimeConfigFilter calls a series of hooks. But instead of -// passing container state on their standard input, -// RuntimeConfigFilter passes the proposed runtime configuration (and -// reads back a possibly-altered form from their standard output). -func RuntimeConfigFilter(ctx context.Context, hooks []spec.Hook, config *spec.Spec, postKillTimeout time.Duration) (hookErr, err error) { - data, err := json.Marshal(config) - if err != nil { - return nil, err - } - for i, hook := range hooks { - hook := hook - var stdout bytes.Buffer - hookErr, err = Run(ctx, &hook, data, &stdout, nil, postKillTimeout) - if err != nil { - return hookErr, err - } - - data = stdout.Bytes() - var newConfig spec.Spec - err = json.Unmarshal(data, &newConfig) - if err != nil { - logrus.Debugf("invalid JSON from config-filter hook %d:\n%s", i, string(data)) - return nil, fmt.Errorf("unmarshal output from config-filter hook %d: %w", i, err) - } - - if !reflect.DeepEqual(config, &newConfig) { - oldConfig := spewConfig.Sdump(config) - newConfig := spewConfig.Sdump(&newConfig) - diff, err := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ - A: difflib.SplitLines(oldConfig), - B: difflib.SplitLines(newConfig), - FromFile: "Old", - FromDate: "", - ToFile: "New", - ToDate: "", - Context: 1, - }) - if err == nil { - logrus.Debugf("precreate hook %d made configuration changes:\n%s", i, diff) - } else { - logrus.Warnf("Precreate hook %d made configuration changes, but we could not compute a diff: %v", i, err) - } - } - - *config = newConfig - } - - return nil, nil -} diff --git a/pkg/hooks/exec/runtimeconfigfilter_test.go b/pkg/hooks/exec/runtimeconfigfilter_test.go deleted file mode 100644 index a4e9b1fdb..000000000 --- a/pkg/hooks/exec/runtimeconfigfilter_test.go +++ /dev/null @@ -1,265 +0,0 @@ -package exec - -import ( - "context" - "encoding/json" - "errors" - "os" - "testing" - "time" - - spec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -func TestRuntimeConfigFilter(t *testing.T) { - unexpectedEndOfJSONInput := json.Unmarshal([]byte("{\n"), nil) //nolint:govet // this should force the error - fileMode := os.FileMode(0600) - rootUint32 := uint32(0) - binUser := int(1) - for _, tt := range []struct { - name string - contextTimeout time.Duration - hooks []spec.Hook - input *spec.Spec - expected *spec.Spec - expectedHookError string - expectedRunError error - expectedRunErrorString string - }{ - { - name: "no-op", - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", "cat"}, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - }, - { - name: "device injection", - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", `sed 's|\("gid":0}\)|\1,{"path": "/dev/sda","type":"b","major":8,"minor":0,"fileMode":384,"uid":0,"gid":0}|'`}, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - Linux: &spec.Linux{ - Devices: []spec.LinuxDevice{ - { - Path: "/dev/fuse", - Type: "c", - Major: 10, - Minor: 229, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - }, - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - Linux: &spec.Linux{ - Devices: []spec.LinuxDevice{ - { - Path: "/dev/fuse", - Type: "c", - Major: 10, - Minor: 229, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - { - Path: "/dev/sda", - Type: "b", - Major: 8, - Minor: 0, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - }, - }, - }, - }, - { - name: "chaining", - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", `sed 's|\("gid":0}\)|\1,{"path": "/dev/sda","type":"b","major":8,"minor":0,"fileMode":384,"uid":0,"gid":0}|'`}, - }, - { - Path: path, - Args: []string{"sh", "-c", `sed 's|/dev/sda|/dev/sdb|'`}, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - Linux: &spec.Linux{ - Devices: []spec.LinuxDevice{ - { - Path: "/dev/fuse", - Type: "c", - Major: 10, - Minor: 229, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - }, - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - Linux: &spec.Linux{ - Devices: []spec.LinuxDevice{ - { - Path: "/dev/fuse", - Type: "c", - Major: 10, - Minor: 229, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - { - Path: "/dev/sdb", - Type: "b", - Major: 8, - Minor: 0, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - }, - }, - }, - }, - { - name: "context timeout", - contextTimeout: time.Duration(1) * time.Second, - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", "sleep 2"}, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expectedHookError: "^executing \\[sh -c sleep 2]: signal: killed$", - expectedRunError: context.DeadlineExceeded, - }, - { - name: "hook timeout", - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", "sleep 2"}, - Timeout: &binUser, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expectedHookError: "^executing \\[sh -c sleep 2]: signal: killed$", - expectedRunError: context.DeadlineExceeded, - }, - { - name: "invalid JSON", - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", "echo '{'"}, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expectedRunError: unexpectedEndOfJSONInput, - expectedRunErrorString: unexpectedEndOfJSONInput.Error(), - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - ctx := context.Background() - if test.contextTimeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, test.contextTimeout) - defer cancel() - } - hookErr, err := RuntimeConfigFilter(ctx, test.hooks, test.input, DefaultPostKillTimeout) - if test.expectedRunError != nil { - if test.expectedRunErrorString != "" { - assert.Contains(t, err.Error(), test.expectedRunErrorString) - } else { - assert.True(t, errors.Is(err, test.expectedRunError)) - } - } - if test.expectedHookError == "" { - if hookErr != nil { - t.Fatal(hookErr) - } - } else { - assert.Regexp(t, test.expectedHookError, hookErr.Error()) - } - assert.Equal(t, test.expected, test.input) - }) - } -} diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go deleted file mode 100644 index 14f98b1de..000000000 --- a/pkg/hooks/hooks.go +++ /dev/null @@ -1,145 +0,0 @@ -// Package hooks implements hook configuration and handling for CRI-O and libpod. -package hooks - -import ( - "context" - "fmt" - "os" - "sort" - "strings" - "sync" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/sirupsen/logrus" -) - -// Version is the current hook configuration version. -const Version = current.Version - -const ( - // DefaultDir is the default directory containing system hook configuration files. - DefaultDir = "/usr/share/containers/oci/hooks.d" - - // OverrideDir is the directory for hook configuration files overriding the default entries. - OverrideDir = "/etc/containers/oci/hooks.d" -) - -// Manager provides an opaque interface for managing CRI-O hooks. -type Manager struct { - hooks map[string]*current.Hook - directories []string - extensionStages []string - lock sync.Mutex -} - -type namedHook struct { - name string - hook *current.Hook -} - -// New creates a new hook manager. Directories are ordered by -// increasing preference (hook configurations in later directories -// override configurations with the same filename from earlier -// directories). -// -// extensionStages allows callers to add additional stages beyond -// those specified in the OCI Runtime Specification and to control -// OCI-defined stages instead of delegating to the OCI runtime. See -// Hooks() for more information. -func New(ctx context.Context, directories []string, extensionStages []string) (manager *Manager, err error) { - manager = &Manager{ - hooks: map[string]*current.Hook{}, - directories: directories, - extensionStages: extensionStages, - } - - for _, dir := range directories { - err = ReadDir(dir, manager.extensionStages, manager.hooks) - if err != nil && !os.IsNotExist(err) { - return nil, err - } - } - - return manager, nil -} - -// filenames returns sorted hook entries. -func (m *Manager) namedHooks() (hooks []*namedHook) { - m.lock.Lock() - defer m.lock.Unlock() - - hooks = make([]*namedHook, len(m.hooks)) - i := 0 - for name, hook := range m.hooks { - hooks[i] = &namedHook{ - name: name, - hook: hook, - } - i++ - } - - return hooks -} - -// Hooks injects OCI runtime hooks for a given container configuration. -// -// If extensionStages was set when initializing the Manager, -// matching hooks requesting those stages will be returned in -// extensionStageHooks. This takes precedence over their inclusion in -// the OCI configuration. For example: -// -// manager, err := New(ctx, []string{DefaultDir}, []string{"poststop"}) -// extensionStageHooks, err := manager.Hooks(config, annotations, hasBindMounts) -// -// will have any matching post-stop hooks in extensionStageHooks and -// will not insert them into config.Hooks.Poststop. -func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBindMounts bool) (extensionStageHooks map[string][]rspec.Hook, err error) { - hooks := m.namedHooks() - sort.Slice(hooks, func(i, j int) bool { return strings.ToLower(hooks[i].name) < strings.ToLower(hooks[j].name) }) - localStages := map[string]bool{} // stages destined for extensionStageHooks - for _, stage := range m.extensionStages { - localStages[stage] = true - } - for _, namedHook := range hooks { - match, err := namedHook.hook.When.Match(config, annotations, hasBindMounts) - if err != nil { - return extensionStageHooks, fmt.Errorf("matching hook %q: %w", namedHook.name, err) - } - if match { - logrus.Debugf("hook %s matched; adding to stages %v", namedHook.name, namedHook.hook.Stages) - if config.Hooks == nil { - config.Hooks = &rspec.Hooks{} - } - for _, stage := range namedHook.hook.Stages { - if _, ok := localStages[stage]; ok { - if extensionStageHooks == nil { - extensionStageHooks = map[string][]rspec.Hook{} - } - extensionStageHooks[stage] = append(extensionStageHooks[stage], namedHook.hook.Hook) - } else { - switch stage { - case "createContainer": - config.Hooks.CreateContainer = append(config.Hooks.CreateContainer, namedHook.hook.Hook) - case "createRuntime": - config.Hooks.CreateRuntime = append(config.Hooks.CreateRuntime, namedHook.hook.Hook) - case "prestart": - config.Hooks.Prestart = append(config.Hooks.Prestart, namedHook.hook.Hook) - case "poststart": - config.Hooks.Poststart = append(config.Hooks.Poststart, namedHook.hook.Hook) - case "poststop": - config.Hooks.Poststop = append(config.Hooks.Poststop, namedHook.hook.Hook) - case "startContainer": - config.Hooks.StartContainer = append(config.Hooks.StartContainer, namedHook.hook.Hook) - default: - return extensionStageHooks, fmt.Errorf("hook %q: unknown stage %q", namedHook.name, stage) - } - } - } - } else { - logrus.Debugf("hook %s did not match", namedHook.name) - } - } - - return extensionStageHooks, nil -} diff --git a/pkg/hooks/hooks_test.go b/pkg/hooks/hooks_test.go deleted file mode 100644 index d5d0c2a32..000000000 --- a/pkg/hooks/hooks_test.go +++ /dev/null @@ -1,218 +0,0 @@ -package hooks - -import ( - "context" - "fmt" - "io/ioutil" - "path/filepath" - "runtime" - "testing" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -// path is the path to an example hook executable. -var path string - -func TestGoodNew(t *testing.T) { - ctx := context.Background() - - dir := t.TempDir() - - for i, name := range []string{ - "01-my-hook.json", - "01-UPPERCASE.json", - "02-another-hook.json", - } { - jsonPath := filepath.Join(dir, name) - var extraStages string - if i == 0 { - extraStages = ", \"poststart\", \"poststop\"" - } - err := ioutil.WriteFile(jsonPath, []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\", \"timeout\": %d}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"%s]}", path, i+1, extraStages)), 0644) - if err != nil { - t.Fatal(err) - } - } - - manager, err := New(ctx, []string{dir}, []string{}) - if err != nil { - t.Fatal(err) - } - - config := &rspec.Spec{} - extensionStageHooks, err := manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - one := 1 - two := 2 - three := 3 - assert.Equal(t, &rspec.Hooks{ - Prestart: []rspec.Hook{ - { - Path: path, - Timeout: &one, - }, - { - Path: path, - Timeout: &two, - }, - { - Path: path, - Timeout: &three, - }, - }, - Poststart: []rspec.Hook{ - { - Path: path, - Timeout: &one, - }, - }, - Poststop: []rspec.Hook{ - { - Path: path, - Timeout: &one, - }, - }, - }, config.Hooks) - - var nilExtensionStageHooks map[string][]rspec.Hook - assert.Equal(t, nilExtensionStageHooks, extensionStageHooks) -} - -func TestBadNew(t *testing.T) { - ctx := context.Background() - - dir := t.TempDir() - - jsonPath := filepath.Join(dir, "a.json") - err := ioutil.WriteFile(jsonPath, []byte("{\"version\": \"-1\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - _, err = New(ctx, []string{dir}, []string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^parsing hook \"[^\"]*a.json\": unrecognized hook version: \"-1\"$", err.Error()) -} - -func TestBrokenMatch(t *testing.T) { - manager := Manager{ - hooks: map[string]*current.Hook{ - "a.json": { - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Commands: []string{"["}, - }, - Stages: []string{"prestart"}, - }, - }, - } - config := &rspec.Spec{ - Process: &rspec.Process{ - Args: []string{"/bin/sh"}, - }, - } - extensionStageHooks, err := manager.Hooks(config, map[string]string{}, false) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^matching hook \"a\\.json\": command: error parsing regexp: .*", err.Error()) - - var nilExtensionStageHooks map[string][]rspec.Hook - assert.Equal(t, nilExtensionStageHooks, extensionStageHooks) -} - -func TestInvalidStage(t *testing.T) { - always := true - manager := Manager{ - hooks: map[string]*current.Hook{ - "a.json": { - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Always: &always, - }, - Stages: []string{"does-not-exist"}, - }, - }, - } - extensionStageHooks, err := manager.Hooks(&rspec.Spec{}, map[string]string{}, false) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^hook \"a\\.json\": unknown stage \"does-not-exist\"$", err.Error()) - - var nilExtensionStageHooks map[string][]rspec.Hook - assert.Equal(t, nilExtensionStageHooks, extensionStageHooks) -} - -func TestExtensionStage(t *testing.T) { - always := true - manager := Manager{ - hooks: map[string]*current.Hook{ - "a.json": { - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Always: &always, - }, - Stages: []string{"prestart", "poststop", "a", "b"}, - }, - }, - extensionStages: []string{"poststop", "a", "b", "c"}, - } - - config := &rspec.Spec{} - extensionStageHooks, err := manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, &rspec.Hooks{ - Prestart: []rspec.Hook{ - { - Path: "/a/b/c", - }, - }, - }, config.Hooks) - - assert.Equal(t, map[string][]rspec.Hook{ - "poststop": { - { - Path: "/a/b/c", - }, - }, - "a": { - { - Path: "/a/b/c", - }, - }, - "b": { - { - Path: "/a/b/c", - }, - }, - }, extensionStageHooks) -} - -func init() { - if runtime.GOOS != "windows" { - path = "/bin/sh" - } else { - panic("we need a reliable executable path on Windows") - } -} diff --git a/pkg/hooks/monitor.go b/pkg/hooks/monitor.go deleted file mode 100644 index d2d7140a5..000000000 --- a/pkg/hooks/monitor.go +++ /dev/null @@ -1,66 +0,0 @@ -package hooks - -import ( - "context" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - "github.com/fsnotify/fsnotify" - "github.com/sirupsen/logrus" -) - -// Monitor dynamically monitors hook directories for additions, -// updates, and removals. -// -// This function writes two empty structs to the sync channel: the -// first is written after the watchers are established and the second -// when this function exits. The expected usage is: -// -// ctx, cancel := context.WithCancel(context.Background()) -// sync := make(chan error, 2) -// go m.Monitor(ctx, sync) -// err := <-sync // block until writers are established -// if err != nil { -// return err // failed to establish watchers -// } -// // do stuff -// cancel() -// err = <-sync // block until monitor finishes -func (m *Manager) Monitor(ctx context.Context, sync chan<- error) { - watcher, err := fsnotify.NewWatcher() - if err != nil { - sync <- err - return - } - defer watcher.Close() - - for _, dir := range m.directories { - err = watcher.Add(dir) - if err != nil { - logrus.Errorf("Failed to watch %q for hooks", dir) - sync <- err - return - } - logrus.Debugf("monitoring %q for hooks", dir) - } - - sync <- nil - - for { - select { - case event := <-watcher.Events: - m.hooks = make(map[string]*current.Hook) - for _, dir := range m.directories { - err = ReadDir(dir, m.extensionStages, m.hooks) - if err != nil { - logrus.Errorf("Failed loading hooks for %s: %v", event.Name, err) - } - } - case <-ctx.Done(): - err = ctx.Err() - logrus.Debugf("hook monitoring canceled: %v", err) - sync <- err - close(sync) - return - } - } -} diff --git a/pkg/hooks/monitor_test.go b/pkg/hooks/monitor_test.go deleted file mode 100644 index 1067d2920..000000000 --- a/pkg/hooks/monitor_test.go +++ /dev/null @@ -1,324 +0,0 @@ -package hooks - -import ( - "context" - "fmt" - "io/ioutil" - "os" - "path/filepath" - "testing" - "time" - - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -func TestMonitorOneDirGood(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - dir := t.TempDir() - - manager, err := New(ctx, []string{dir}, []string{}) - if err != nil { - t.Fatal(err) - } - - sync := make(chan error, 2) - go manager.Monitor(ctx, sync) - err = <-sync - if err != nil { - t.Fatal(err) - } - - jsonPath := filepath.Join(dir, "a.json") - - t.Run("good-addition", func(t *testing.T) { - err = ioutil.WriteFile(jsonPath, []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\", \"poststart\", \"poststop\"]}", path)), 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, &rspec.Hooks{ - Prestart: []rspec.Hook{ - { - Path: path, - }, - }, - Poststart: []rspec.Hook{ - { - Path: path, - }, - }, - Poststop: []rspec.Hook{ - { - Path: path, - }, - }, - }, config.Hooks) - }) - - t.Run("good-removal", func(t *testing.T) { - err = os.Remove(jsonPath) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - expected := config.Hooks - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, expected, config.Hooks) - }) - - t.Run("bad-addition", func(t *testing.T) { - err = ioutil.WriteFile(jsonPath, []byte("{\"version\": \"-1\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - expected := config.Hooks - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, expected, config.Hooks) - - err = os.Remove(jsonPath) - if err != nil { - t.Fatal(err) - } - }) - - cancel() - err = <-sync - assert.Equal(t, context.Canceled, err) -} - -func TestMonitorTwoDirGood(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - primaryDir := t.TempDir() - fallbackDir := t.TempDir() - - manager, err := New(ctx, []string{fallbackDir, primaryDir}, []string{}) - if err != nil { - t.Fatal(err) - } - - sync := make(chan error, 2) - go manager.Monitor(ctx, sync) - err = <-sync - if err != nil { - t.Fatal(err) - } - - fallbackPath := filepath.Join(fallbackDir, "a.json") - fallbackJSON := []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}", path)) - fallbackInjected := &rspec.Hooks{ - Prestart: []rspec.Hook{ - { - Path: path, - }, - }, - } - - t.Run("good-fallback-addition", func(t *testing.T) { - err = ioutil.WriteFile(fallbackPath, fallbackJSON, 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, fallbackInjected, config.Hooks) - }) - - primaryPath := filepath.Join(primaryDir, "a.json") - primaryJSON := []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\", \"timeout\": 1}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}", path)) - one := 1 - primaryInjected := &rspec.Hooks{ - Prestart: []rspec.Hook{ - { - Path: path, - Timeout: &one, - }, - }, - } - - t.Run("good-primary-override", func(t *testing.T) { - err = ioutil.WriteFile(primaryPath, primaryJSON, 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, primaryInjected, config.Hooks) - }) - - t.Run("good-fallback-removal", func(t *testing.T) { - err = os.Remove(fallbackPath) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, primaryInjected, config.Hooks) // masked by primary - }) - - t.Run("good-fallback-restore", func(t *testing.T) { - err = ioutil.WriteFile(fallbackPath, fallbackJSON, 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, primaryInjected, config.Hooks) // masked by primary - }) - - primaryPath2 := filepath.Join(primaryDir, "0a.json") // 0a because it will be before a.json alphabetically - - t.Run("bad-primary-new-addition", func(t *testing.T) { - err = ioutil.WriteFile(primaryPath2, []byte("{\"version\": \"-1\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - fmt.Println("expected: ", config.Hooks) - expected := primaryInjected // 0a.json is bad, a.json is still good - _, err = manager.Hooks(config, map[string]string{}, false) - fmt.Println("actual: ", config.Hooks) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, expected, config.Hooks) - }) - - t.Run("bad-primary-same-addition", func(t *testing.T) { - err = ioutil.WriteFile(primaryPath, []byte("{\"version\": \"-1\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - expected := fallbackInjected - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, expected, config.Hooks) - }) - - t.Run("good-primary-removal", func(t *testing.T) { - err = os.Remove(primaryPath) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, fallbackInjected, config.Hooks) - }) - - t.Run("good-non-json-addition", func(t *testing.T) { - err = ioutil.WriteFile(filepath.Join(fallbackDir, "README"), []byte("Hello"), 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, fallbackInjected, config.Hooks) - }) - - t.Run("good-fallback-removal", func(t *testing.T) { - err = os.Remove(fallbackPath) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - expected := config.Hooks - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, expected, config.Hooks) - }) - - cancel() - err = <-sync - assert.Equal(t, context.Canceled, err) -} - -func TestMonitorBadWatcher(t *testing.T) { - ctx := context.Background() - - manager, err := New(ctx, []string{}, []string{}) - if err != nil { - t.Fatal(err) - } - manager.directories = []string{"/does/not/exist"} - - sync := make(chan error, 2) - go manager.Monitor(ctx, sync) - err = <-sync - if !os.IsNotExist(err) { - t.Fatal("opaque wrapping for not-exist errors") - } -} diff --git a/pkg/hooks/read.go b/pkg/hooks/read.go deleted file mode 100644 index 379ed67ef..000000000 --- a/pkg/hooks/read.go +++ /dev/null @@ -1,101 +0,0 @@ -// Package hooks implements CRI-O's hook handling. -package hooks - -import ( - "encoding/json" - "errors" - "fmt" - "io/ioutil" - "os" - "path/filepath" - "strings" - - old "github.com/containers/podman/v4/pkg/hooks/0.1.0" - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - "github.com/sirupsen/logrus" -) - -type reader func(content []byte) (*current.Hook, error) - -var ( - // ErrNoJSONSuffix represents hook-add attempts where the filename - // does not end in '.json'. - ErrNoJSONSuffix = errors.New("hook filename does not end in '.json'") - - // Readers registers per-version hook readers. - Readers = map[string]reader{} -) - -// Read reads a hook JSON file, verifies it, and returns the hook configuration. -func Read(path string, extensionStages []string) (*current.Hook, error) { - if !strings.HasSuffix(path, ".json") { - return nil, ErrNoJSONSuffix - } - content, err := ioutil.ReadFile(path) - if err != nil { - return nil, err - } - hook, err := read(content) - if err != nil { - return nil, fmt.Errorf("parsing hook %q: %w", path, err) - } - err = hook.Validate(extensionStages) - return hook, err -} - -func read(content []byte) (hook *current.Hook, err error) { - var ver version - if err := json.Unmarshal(content, &ver); err != nil { - return nil, fmt.Errorf("version check: %w", err) - } - reader, ok := Readers[ver.Version] - if !ok { - return nil, fmt.Errorf("unrecognized hook version: %q", ver.Version) - } - - hook, err = reader(content) - if err != nil { - return hook, fmt.Errorf("%v: %w", ver.Version, err) - } - return hook, err -} - -// ReadDir reads hook JSON files from a directory into the given map, -// clobbering any previous entries with the same filenames. -func ReadDir(path string, extensionStages []string, hooks map[string]*current.Hook) error { - logrus.Debugf("reading hooks from %s", path) - files, err := ioutil.ReadDir(path) - if err != nil { - return err - } - res := err - for _, file := range files { - filePath := filepath.Join(path, file.Name()) - hook, err := Read(filePath, extensionStages) - if err != nil { - if err == ErrNoJSONSuffix { - continue - } - if os.IsNotExist(err) { - if err2, ok := err.(*os.PathError); ok && err2.Path == filePath { - continue - } - } - if res == nil { - res = err - } else { - res = fmt.Errorf("%v: %w", err, res) - } - continue - } - hooks[file.Name()] = hook - logrus.Debugf("added hook %s", filePath) - } - return res -} - -func init() { - Readers[current.Version] = current.Read - Readers[old.Version] = old.Read - Readers[""] = old.Read -} diff --git a/pkg/hooks/read_test.go b/pkg/hooks/read_test.go deleted file mode 100644 index 381d66bbe..000000000 --- a/pkg/hooks/read_test.go +++ /dev/null @@ -1,194 +0,0 @@ -package hooks - -import ( - "fmt" - "io/ioutil" - "os" - "path/filepath" - "testing" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -func TestNoJSONSuffix(t *testing.T) { - _, err := Read("abc", []string{}) - assert.Equal(t, err, ErrNoJSONSuffix) -} - -func TestUnknownPath(t *testing.T) { - _, err := Read(filepath.Join("does", "not", "exist.json"), []string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^open does/not/exist.json: no such file or directory$", err.Error()) - if !os.IsNotExist(err) { - t.Fatal("opaque wrapping for not-exist errors") - } -} - -func TestGoodFile(t *testing.T) { - dir := t.TempDir() - - jsonPath := filepath.Join(dir, "hook.json") - err := ioutil.WriteFile(jsonPath, []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}", path)), 0644) - if err != nil { - t.Fatal(err) - } - - hook, err := Read(jsonPath, []string{}) - if err != nil { - t.Fatal(err) - } - always := true - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: path, - }, - When: current.When{ - Always: &always, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestBadFile(t *testing.T) { - dir := t.TempDir() - - path := filepath.Join(dir, "hook.json") - err := ioutil.WriteFile(path, []byte("{\"version\": \"1.0.0\", \"hook\": \"not-a-string\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - _, err = Read(path, []string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^parsing hook \"[^\"]*hook.json\": 1.0.0: json: cannot unmarshal string into Go struct field Hook.hook of type specs.Hook$", err.Error()) -} - -func TestGoodBytes(t *testing.T) { - hook, err := read([]byte("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"/a/b/c\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}")) - if err != nil { - t.Fatal(err) - } - always := true - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Always: &always, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestInvalidJSON(t *testing.T) { - _, err := read([]byte("{")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^version check: unexpected end of JSON input$", err.Error()) -} - -func TestInvalidVersion(t *testing.T) { - _, err := read([]byte("{\"version\": \"-1\"}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^unrecognized hook version: \"-1\"$", err.Error()) -} - -func TestInvalidCurrentJSON(t *testing.T) { - _, err := read([]byte("{\"version\": \"1.0.0\", \"hook\": \"not-a-string\"}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^1.0.0: json: cannot unmarshal string into Go struct field Hook.hook of type specs.Hook$", err.Error()) -} - -func TestGoodDir(t *testing.T) { - dir := t.TempDir() - - err := ioutil.WriteFile(filepath.Join(dir, "README"), []byte("not a hook"), 0644) - if err != nil { - t.Fatal(err) - } - - jsonPath := filepath.Join(dir, "a.json") - err = ioutil.WriteFile(jsonPath, []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}", path)), 0644) - if err != nil { - t.Fatal(err) - } - - hooks := map[string]*current.Hook{} - err = ReadDir(dir, []string{}, hooks) - if err != nil { - t.Fatal(err) - } - - always := true - assert.Equal(t, map[string]*current.Hook{ - "a.json": { - Version: current.Version, - Hook: rspec.Hook{ - Path: path, - }, - When: current.When{ - Always: &always, - }, - Stages: []string{"prestart"}, - }, - }, hooks) -} - -func TestUnknownDir(t *testing.T) { - hooks := map[string]*current.Hook{} - err := ReadDir(filepath.Join("does", "not", "exist"), []string{}, hooks) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^open does/not/exist: no such file or directory$", err.Error()) - if !os.IsNotExist(err) { - t.Fatal("opaque wrapping for not-exist errors") - } -} - -func TestBadDir(t *testing.T) { - dir := t.TempDir() - - jsonPath := filepath.Join(dir, "a.json") - err := ioutil.WriteFile(jsonPath, []byte("{\"version\": \"-1\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - hooks := map[string]*current.Hook{} - err = ReadDir(dir, []string{}, hooks) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^parsing hook \"[^\"]*a.json\": unrecognized hook version: \"-1\"$", err.Error()) -} - -func TestHookExecutableDoesNotExit(t *testing.T) { - dir := t.TempDir() - - jsonPath := filepath.Join(dir, "hook.json") - err := ioutil.WriteFile(jsonPath, []byte("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"/does/not/exist\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}"), 0644) - if err != nil { - t.Fatal(err) - } - - hooks := map[string]*current.Hook{} - err = ReadDir(dir, []string{}, hooks) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^stat /does/not/exist: no such file or directory$", err.Error()) -} diff --git a/pkg/hooks/version.go b/pkg/hooks/version.go deleted file mode 100644 index 637d8e2f4..000000000 --- a/pkg/hooks/version.go +++ /dev/null @@ -1,6 +0,0 @@ -package hooks - -// version a structure for checking the version of a hook configuration. -type version struct { - Version string `json:"version"` -} -- cgit v1.2.3-54-g00ecf From 2350ef9dd4c0ffc7c92583b33f63da0bc9502bba Mon Sep 17 00:00:00 2001 From: "Jason T. Greene" Date: Tue, 19 Jul 2022 14:06:47 -0500 Subject: Fix potential leak on mid-stream read error Signed-off-by: Jason T. Greene --- pkg/machine/fedora.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/machine/fedora.go b/pkg/machine/fedora.go index 7ac4692e3..497265269 100644 --- a/pkg/machine/fedora.go +++ b/pkg/machine/fedora.go @@ -105,11 +105,11 @@ func getFedoraDownload(releaseURL string) (*url.URL, string, int64, error) { return nil, "", -1, fmt.Errorf("get request failed: %s: %w", verURL.String(), err) } + defer resp.Body.Close() bytes, err := io.ReadAll(&io.LimitedReader{R: resp.Body, N: 1024}) if err != nil { return nil, "", -1, fmt.Errorf("failed reading: %s: %w", verURL.String(), err) } - _ = resp.Body.Close() return downloadURL, strings.TrimSpace(string(bytes)), contentLen, nil } -- cgit v1.2.3-54-g00ecf From ce790e61b1a0a983a1900b76b1ad9864d15b1496 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 21 Jul 2022 11:36:32 +0200 Subject: enable linter for pkg/machine/e2e Rename all files to _test.go and rename the package to e2e_test. This makes the linter less strict about things like dot imports. Add some unused nolint directives to silence some warnings, these can be used to find untested options so someone could add tests for them. Fixes #14996 Signed-off-by: Paul Holzinger --- hack/golangci-lint.sh | 4 +- pkg/machine/e2e/basic_test.go | 4 +- pkg/machine/e2e/config.go | 183 --------------------------------- pkg/machine/e2e/config_basic.go | 19 ---- pkg/machine/e2e/config_basic_test.go | 18 ++++ pkg/machine/e2e/config_info.go | 20 ---- pkg/machine/e2e/config_info_test.go | 20 ++++ pkg/machine/e2e/config_init.go | 102 ------------------ pkg/machine/e2e/config_init_test.go | 102 ++++++++++++++++++ pkg/machine/e2e/config_inspect.go | 24 ----- pkg/machine/e2e/config_inspect_test.go | 24 +++++ pkg/machine/e2e/config_list.go | 45 -------- pkg/machine/e2e/config_list_test.go | 45 ++++++++ pkg/machine/e2e/config_rm.go | 56 ---------- pkg/machine/e2e/config_rm_test.go | 56 ++++++++++ pkg/machine/e2e/config_set.go | 43 -------- pkg/machine/e2e/config_set_test.go | 43 ++++++++ pkg/machine/e2e/config_ssh.go | 33 ------ pkg/machine/e2e/config_ssh_test.go | 31 ++++++ pkg/machine/e2e/config_start.go | 16 --- pkg/machine/e2e/config_start_test.go | 15 +++ pkg/machine/e2e/config_stop.go | 16 --- pkg/machine/e2e/config_stop_test.go | 15 +++ pkg/machine/e2e/config_test.go | 175 +++++++++++++++++++++++++++++++ pkg/machine/e2e/info_test.go | 2 +- pkg/machine/e2e/init_test.go | 6 +- pkg/machine/e2e/inspect_test.go | 8 +- pkg/machine/e2e/list_test.go | 8 +- pkg/machine/e2e/machine_test.go | 2 +- pkg/machine/e2e/rm_test.go | 2 +- pkg/machine/e2e/set_test.go | 6 +- pkg/machine/e2e/ssh_test.go | 12 +-- pkg/machine/e2e/start_test.go | 2 +- pkg/machine/e2e/stop_test.go | 2 +- 34 files changed, 573 insertions(+), 586 deletions(-) delete mode 100644 pkg/machine/e2e/config.go delete mode 100644 pkg/machine/e2e/config_basic.go create mode 100644 pkg/machine/e2e/config_basic_test.go delete mode 100644 pkg/machine/e2e/config_info.go create mode 100644 pkg/machine/e2e/config_info_test.go delete mode 100644 pkg/machine/e2e/config_init.go create mode 100644 pkg/machine/e2e/config_init_test.go delete mode 100644 pkg/machine/e2e/config_inspect.go create mode 100644 pkg/machine/e2e/config_inspect_test.go delete mode 100644 pkg/machine/e2e/config_list.go create mode 100644 pkg/machine/e2e/config_list_test.go delete mode 100644 pkg/machine/e2e/config_rm.go create mode 100644 pkg/machine/e2e/config_rm_test.go delete mode 100644 pkg/machine/e2e/config_set.go create mode 100644 pkg/machine/e2e/config_set_test.go delete mode 100644 pkg/machine/e2e/config_ssh.go create mode 100644 pkg/machine/e2e/config_ssh_test.go delete mode 100644 pkg/machine/e2e/config_start.go create mode 100644 pkg/machine/e2e/config_start_test.go delete mode 100644 pkg/machine/e2e/config_stop.go create mode 100644 pkg/machine/e2e/config_stop_test.go create mode 100644 pkg/machine/e2e/config_test.go (limited to 'pkg') diff --git a/hack/golangci-lint.sh b/hack/golangci-lint.sh index 2eaf206d7..dd06004ac 100755 --- a/hack/golangci-lint.sh +++ b/hack/golangci-lint.sh @@ -9,9 +9,9 @@ BUILD_TAGS[abi]="${BUILD_TAGS[default]},systemd" BUILD_TAGS[tunnel]="${BUILD_TAGS[default]},remote" declare -A SKIP_DIRS -SKIP_DIRS[abi]="pkg/machine/e2e" +SKIP_DIRS[abi]="" # TODO: add "remote" build tag to pkg/api -SKIP_DIRS[tunnel]="pkg/api,pkg/machine/e2e" +SKIP_DIRS[tunnel]="pkg/api" [[ $1 == run ]] && shift diff --git a/pkg/machine/e2e/basic_test.go b/pkg/machine/e2e/basic_test.go index f67fb4c67..da0310485 100644 --- a/pkg/machine/e2e/basic_test.go +++ b/pkg/machine/e2e/basic_test.go @@ -1,4 +1,4 @@ -package e2e +package e2e_test import ( . "github.com/onsi/ginkgo" @@ -20,7 +20,7 @@ var _ = Describe("run basic podman commands", func() { }) It("Basic ops", func() { - name := randomString(12) + name := randomString() i := new(initMachine) session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath).withNow()).run() Expect(err).To(BeNil()) diff --git a/pkg/machine/e2e/config.go b/pkg/machine/e2e/config.go deleted file mode 100644 index b3fe74b0c..000000000 --- a/pkg/machine/e2e/config.go +++ /dev/null @@ -1,183 +0,0 @@ -package e2e - -import ( - "encoding/json" - "fmt" - "os" - "os/exec" - "path/filepath" - "strings" - "time" - - "github.com/containers/podman/v4/pkg/machine" - "github.com/containers/podman/v4/pkg/machine/qemu" - "github.com/containers/podman/v4/pkg/util" - "github.com/containers/storage/pkg/stringid" - . "github.com/onsi/ginkgo" //nolint:golint,stylecheck - . "github.com/onsi/gomega" - "github.com/onsi/gomega/gexec" - . "github.com/onsi/gomega/gexec" //nolint:golint,stylecheck -) - -var originalHomeDir = os.Getenv("HOME") - -const ( - defaultTimeout time.Duration = 90 * time.Second -) - -type machineCommand interface { - buildCmd(m *machineTestBuilder) []string -} - -type MachineTestBuilder interface { - setName(string) *MachineTestBuilder - setCmd(mc machineCommand) *MachineTestBuilder - setTimeout(duration time.Duration) *MachineTestBuilder - run() (*machineSession, error) -} -type machineSession struct { - *gexec.Session -} - -type machineTestBuilder struct { - cmd []string - imagePath string - name string - names []string - podmanBinary string - timeout time.Duration -} -type qemuMachineInspectInfo struct { - State machine.Status - VM qemu.MachineVM -} - -// waitWithTimeout waits for a command to complete for a given -// number of seconds -func (ms *machineSession) waitWithTimeout(timeout time.Duration) { - Eventually(ms, timeout).Should(Exit()) - os.Stdout.Sync() - os.Stderr.Sync() -} - -func (ms *machineSession) Bytes() []byte { - return []byte(ms.outputToString()) -} - -func (ms *machineSession) outputToStringSlice() []string { - var results []string - output := string(ms.Out.Contents()) - for _, line := range strings.Split(output, "\n") { - if line != "" { - results = append(results, line) - } - } - return results -} - -// outputToString returns the output from a session in string form -func (ms *machineSession) outputToString() string { - if ms == nil || ms.Out == nil || ms.Out.Contents() == nil { - return "" - } - - fields := strings.Fields(string(ms.Out.Contents())) - return strings.Join(fields, " ") -} - -// errorToString returns the error output from a session in string form -func (ms *machineSession) errorToString() string { - if ms == nil || ms.Err == nil || ms.Err.Contents() == nil { - return "" - } - return string(ms.Err.Contents()) -} - -// newMB constructor for machine test builders -func newMB() (*machineTestBuilder, error) { - mb := machineTestBuilder{ - timeout: defaultTimeout, - } - cwd, err := os.Getwd() - if err != nil { - return nil, err - } - mb.podmanBinary = filepath.Join(cwd, "../../../bin/podman-remote") - if os.Getenv("PODMAN_BINARY") != "" { - mb.podmanBinary = os.Getenv("PODMAN_BINARY") - } - return &mb, nil -} - -// setName sets the name of the virtuaql machine for the command -func (m *machineTestBuilder) setName(name string) *machineTestBuilder { - m.name = name - return m -} - -// setCmd takes a machineCommand struct and assembles a cmd line -// representation of the podman machine command -func (m *machineTestBuilder) setCmd(mc machineCommand) *machineTestBuilder { - // If no name for the machine exists, we set a random name. - if !util.StringInSlice(m.name, m.names) { - if len(m.name) < 1 { - m.name = randomString(12) - } - m.names = append(m.names, m.name) - } - m.cmd = mc.buildCmd(m) - return m -} - -func (m *machineTestBuilder) setTimeout(timeout time.Duration) *machineTestBuilder { - m.timeout = timeout - return m -} - -// toQemuInspectInfo is only for inspecting qemu machines. Other providers will need -// to make their own. -func (mb *machineTestBuilder) toQemuInspectInfo() ([]machine.InspectInfo, int, error) { - args := []string{"machine", "inspect"} - args = append(args, mb.names...) - session, err := runWrapper(mb.podmanBinary, args, defaultTimeout, true) - if err != nil { - return nil, -1, err - } - mii := []machine.InspectInfo{} - err = json.Unmarshal(session.Bytes(), &mii) - return mii, session.ExitCode(), err -} - -func (m *machineTestBuilder) runWithoutWait() (*machineSession, error) { - return runWrapper(m.podmanBinary, m.cmd, m.timeout, false) -} - -func (m *machineTestBuilder) run() (*machineSession, error) { - return runWrapper(m.podmanBinary, m.cmd, m.timeout, true) -} - -func runWrapper(podmanBinary string, cmdArgs []string, timeout time.Duration, wait bool) (*machineSession, error) { - if len(os.Getenv("DEBUG")) > 0 { - cmdArgs = append([]string{"--log-level=debug"}, cmdArgs...) - } - fmt.Println(podmanBinary + " " + strings.Join(cmdArgs, " ")) - c := exec.Command(podmanBinary, cmdArgs...) - session, err := Start(c, GinkgoWriter, GinkgoWriter) - if err != nil { - Fail(fmt.Sprintf("Unable to start session: %q", err)) - return nil, err - } - ms := machineSession{session} - if wait { - ms.waitWithTimeout(timeout) - fmt.Println("output:", ms.outputToString()) - } - return &ms, nil -} - -func (m *machineTestBuilder) init() {} - -// randomString returns a string of given length composed of random characters -func randomString(n int) string { - return stringid.GenerateRandomID()[0:12] -} diff --git a/pkg/machine/e2e/config_basic.go b/pkg/machine/e2e/config_basic.go deleted file mode 100644 index be0896156..000000000 --- a/pkg/machine/e2e/config_basic.go +++ /dev/null @@ -1,19 +0,0 @@ -package e2e - -type basicMachine struct { - args []string - cmd []string -} - -func (s basicMachine) buildCmd(m *machineTestBuilder) []string { - cmd := []string{"-r"} - if len(s.args) > 0 { - cmd = append(cmd, s.args...) - } - return cmd -} - -func (s *basicMachine) withPodmanCommand(args []string) *basicMachine { - s.args = args - return s -} diff --git a/pkg/machine/e2e/config_basic_test.go b/pkg/machine/e2e/config_basic_test.go new file mode 100644 index 000000000..d1cb24174 --- /dev/null +++ b/pkg/machine/e2e/config_basic_test.go @@ -0,0 +1,18 @@ +package e2e_test + +type basicMachine struct { + args []string +} + +func (s basicMachine) buildCmd(m *machineTestBuilder) []string { + cmd := []string{"-r"} + if len(s.args) > 0 { + cmd = append(cmd, s.args...) + } + return cmd +} + +func (s *basicMachine) withPodmanCommand(args []string) *basicMachine { + s.args = args + return s +} diff --git a/pkg/machine/e2e/config_info.go b/pkg/machine/e2e/config_info.go deleted file mode 100644 index 410c7e518..000000000 --- a/pkg/machine/e2e/config_info.go +++ /dev/null @@ -1,20 +0,0 @@ -package e2e - -type infoMachine struct { - format string - cmd []string -} - -func (i *infoMachine) buildCmd(m *machineTestBuilder) []string { - cmd := []string{"machine", "info"} - if len(i.format) > 0 { - cmd = append(cmd, "--format", i.format) - } - i.cmd = cmd - return cmd -} - -func (i *infoMachine) withFormat(format string) *infoMachine { - i.format = format - return i -} diff --git a/pkg/machine/e2e/config_info_test.go b/pkg/machine/e2e/config_info_test.go new file mode 100644 index 000000000..4da40ab99 --- /dev/null +++ b/pkg/machine/e2e/config_info_test.go @@ -0,0 +1,20 @@ +package e2e_test + +type infoMachine struct { + format string + cmd []string +} + +func (i *infoMachine) buildCmd(m *machineTestBuilder) []string { + cmd := []string{"machine", "info"} + if len(i.format) > 0 { + cmd = append(cmd, "--format", i.format) + } + i.cmd = cmd + return cmd +} + +func (i *infoMachine) withFormat(format string) *infoMachine { + i.format = format + return i +} diff --git a/pkg/machine/e2e/config_init.go b/pkg/machine/e2e/config_init.go deleted file mode 100644 index 7f18cce7d..000000000 --- a/pkg/machine/e2e/config_init.go +++ /dev/null @@ -1,102 +0,0 @@ -package e2e - -import ( - "strconv" -) - -type initMachine struct { - /* - --cpus uint Number of CPUs (default 1) - --disk-size uint Disk size in GB (default 100) - --ignition-path string Path to ignition file - --image-path string Path to qcow image (default "testing") - -m, --memory uint Memory in MB (default 2048) - --now Start machine now - --rootful Whether this machine should prefer rootful container execution - --timezone string Set timezone (default "local") - -v, --volume stringArray Volumes to mount, source:target - --volume-driver string Optional volume driver - - */ - cpus *uint - diskSize *uint - ignitionPath string - imagePath string - memory *uint - now bool - timezone string - rootful bool - volumes []string - - cmd []string -} - -func (i *initMachine) buildCmd(m *machineTestBuilder) []string { - cmd := []string{"machine", "init"} - if i.cpus != nil { - cmd = append(cmd, "--cpus", strconv.Itoa(int(*i.cpus))) - } - if i.diskSize != nil { - cmd = append(cmd, "--disk-size", strconv.Itoa(int(*i.diskSize))) - } - if l := len(i.ignitionPath); l > 0 { - cmd = append(cmd, "--ignition-path", i.ignitionPath) - } - if l := len(i.imagePath); l > 0 { - cmd = append(cmd, "--image-path", i.imagePath) - } - if i.memory != nil { - cmd = append(cmd, "--memory", strconv.Itoa(int(*i.memory))) - } - if l := len(i.timezone); l > 0 { - cmd = append(cmd, "--timezone", i.timezone) - } - for _, v := range i.volumes { - cmd = append(cmd, "--volume", v) - } - if i.now { - cmd = append(cmd, "--now") - } - cmd = append(cmd, m.name) - i.cmd = cmd - return cmd -} - -func (i *initMachine) withCPUs(num uint) *initMachine { - i.cpus = &num - return i -} -func (i *initMachine) withDiskSize(size uint) *initMachine { - i.diskSize = &size - return i -} - -func (i *initMachine) withIgnitionPath(path string) *initMachine { - i.ignitionPath = path - return i -} - -func (i *initMachine) withImagePath(path string) *initMachine { - i.imagePath = path - return i -} - -func (i *initMachine) withMemory(num uint) *initMachine { - i.memory = &num - return i -} - -func (i *initMachine) withNow() *initMachine { - i.now = true - return i -} - -func (i *initMachine) withTimezone(tz string) *initMachine { - i.timezone = tz - return i -} - -func (i *initMachine) withVolume(v string) *initMachine { - i.volumes = append(i.volumes, v) - return i -} diff --git a/pkg/machine/e2e/config_init_test.go b/pkg/machine/e2e/config_init_test.go new file mode 100644 index 000000000..d6c7990b0 --- /dev/null +++ b/pkg/machine/e2e/config_init_test.go @@ -0,0 +1,102 @@ +package e2e_test + +import ( + "strconv" +) + +type initMachine struct { + /* + --cpus uint Number of CPUs (default 1) + --disk-size uint Disk size in GB (default 100) + --ignition-path string Path to ignition file + --image-path string Path to qcow image (default "testing") + -m, --memory uint Memory in MB (default 2048) + --now Start machine now + --rootful Whether this machine should prefer rootful container execution + --timezone string Set timezone (default "local") + -v, --volume stringArray Volumes to mount, source:target + --volume-driver string Optional volume driver + + */ + cpus *uint + diskSize *uint + ignitionPath string + imagePath string + memory *uint + now bool + timezone string + rootful bool //nolint:unused,structcheck + volumes []string + + cmd []string +} + +func (i *initMachine) buildCmd(m *machineTestBuilder) []string { + cmd := []string{"machine", "init"} + if i.cpus != nil { + cmd = append(cmd, "--cpus", strconv.Itoa(int(*i.cpus))) + } + if i.diskSize != nil { + cmd = append(cmd, "--disk-size", strconv.Itoa(int(*i.diskSize))) + } + if l := len(i.ignitionPath); l > 0 { + cmd = append(cmd, "--ignition-path", i.ignitionPath) + } + if l := len(i.imagePath); l > 0 { + cmd = append(cmd, "--image-path", i.imagePath) + } + if i.memory != nil { + cmd = append(cmd, "--memory", strconv.Itoa(int(*i.memory))) + } + if l := len(i.timezone); l > 0 { + cmd = append(cmd, "--timezone", i.timezone) + } + for _, v := range i.volumes { + cmd = append(cmd, "--volume", v) + } + if i.now { + cmd = append(cmd, "--now") + } + cmd = append(cmd, m.name) + i.cmd = cmd + return cmd +} + +func (i *initMachine) withCPUs(num uint) *initMachine { + i.cpus = &num + return i +} +func (i *initMachine) withDiskSize(size uint) *initMachine { + i.diskSize = &size + return i +} + +func (i *initMachine) withIgnitionPath(path string) *initMachine { //nolint:unused + i.ignitionPath = path + return i +} + +func (i *initMachine) withImagePath(path string) *initMachine { + i.imagePath = path + return i +} + +func (i *initMachine) withMemory(num uint) *initMachine { + i.memory = &num + return i +} + +func (i *initMachine) withNow() *initMachine { + i.now = true + return i +} + +func (i *initMachine) withTimezone(tz string) *initMachine { + i.timezone = tz + return i +} + +func (i *initMachine) withVolume(v string) *initMachine { + i.volumes = append(i.volumes, v) + return i +} diff --git a/pkg/machine/e2e/config_inspect.go b/pkg/machine/e2e/config_inspect.go deleted file mode 100644 index 74c9a5d9c..000000000 --- a/pkg/machine/e2e/config_inspect.go +++ /dev/null @@ -1,24 +0,0 @@ -package e2e - -type inspectMachine struct { - /* - --format string Format volume output using JSON or a Go template - */ - cmd []string - format string -} - -func (i *inspectMachine) buildCmd(m *machineTestBuilder) []string { - cmd := []string{"machine", "inspect"} - if len(i.format) > 0 { - cmd = append(cmd, "--format", i.format) - } - cmd = append(cmd, m.names...) - i.cmd = cmd - return cmd -} - -func (i *inspectMachine) withFormat(format string) *inspectMachine { - i.format = format - return i -} diff --git a/pkg/machine/e2e/config_inspect_test.go b/pkg/machine/e2e/config_inspect_test.go new file mode 100644 index 000000000..ffd74220f --- /dev/null +++ b/pkg/machine/e2e/config_inspect_test.go @@ -0,0 +1,24 @@ +package e2e_test + +type inspectMachine struct { + /* + --format string Format volume output using JSON or a Go template + */ + cmd []string + format string +} + +func (i *inspectMachine) buildCmd(m *machineTestBuilder) []string { + cmd := []string{"machine", "inspect"} + if len(i.format) > 0 { + cmd = append(cmd, "--format", i.format) + } + cmd = append(cmd, m.names...) + i.cmd = cmd + return cmd +} + +func (i *inspectMachine) withFormat(format string) *inspectMachine { + i.format = format + return i +} diff --git a/pkg/machine/e2e/config_list.go b/pkg/machine/e2e/config_list.go deleted file mode 100644 index 150f984bc..000000000 --- a/pkg/machine/e2e/config_list.go +++ /dev/null @@ -1,45 +0,0 @@ -package e2e - -type listMachine struct { - /* - --format string Format volume output using JSON or a Go template (default "{{.Name}}\t{{.VMType}}\t{{.Created}}\t{{.LastUp}}\t{{.CPUs}}\t{{.Memory}}\t{{.DiskSize}}\n") - --noheading Do not print headers - -q, --quiet Show only machine names - */ - - format string - noHeading bool - quiet bool - - cmd []string -} - -func (i *listMachine) buildCmd(m *machineTestBuilder) []string { - cmd := []string{"machine", "list"} - if len(i.format) > 0 { - cmd = append(cmd, "--format", i.format) - } - if i.noHeading { - cmd = append(cmd, "--noheading") - } - if i.quiet { - cmd = append(cmd, "--quiet") - } - i.cmd = cmd - return cmd -} - -func (i *listMachine) withNoHeading() *listMachine { - i.noHeading = true - return i -} - -func (i *listMachine) withQuiet() *listMachine { - i.quiet = true - return i -} - -func (i *listMachine) withFormat(format string) *listMachine { - i.format = format - return i -} diff --git a/pkg/machine/e2e/config_list_test.go b/pkg/machine/e2e/config_list_test.go new file mode 100644 index 000000000..78f9edc62 --- /dev/null +++ b/pkg/machine/e2e/config_list_test.go @@ -0,0 +1,45 @@ +package e2e_test + +type listMachine struct { + /* + --format string Format volume output using JSON or a Go template (default "{{.Name}}\t{{.VMType}}\t{{.Created}}\t{{.LastUp}}\t{{.CPUs}}\t{{.Memory}}\t{{.DiskSize}}\n") + --noheading Do not print headers + -q, --quiet Show only machine names + */ + + format string + noHeading bool + quiet bool + + cmd []string +} + +func (i *listMachine) buildCmd(m *machineTestBuilder) []string { + cmd := []string{"machine", "list"} + if len(i.format) > 0 { + cmd = append(cmd, "--format", i.format) + } + if i.noHeading { + cmd = append(cmd, "--noheading") + } + if i.quiet { + cmd = append(cmd, "--quiet") + } + i.cmd = cmd + return cmd +} + +func (i *listMachine) withNoHeading() *listMachine { + i.noHeading = true + return i +} + +func (i *listMachine) withQuiet() *listMachine { + i.quiet = true + return i +} + +func (i *listMachine) withFormat(format string) *listMachine { + i.format = format + return i +} diff --git a/pkg/machine/e2e/config_rm.go b/pkg/machine/e2e/config_rm.go deleted file mode 100644 index 6cf262a22..000000000 --- a/pkg/machine/e2e/config_rm.go +++ /dev/null @@ -1,56 +0,0 @@ -package e2e - -type rmMachine struct { - /* - -f, --force Stop and do not prompt before rming - --save-ignition Do not delete ignition file - --save-image Do not delete the image file - --save-keys Do not delete SSH keys - - */ - force bool - saveIgnition bool - saveImage bool - saveKeys bool - - cmd []string -} - -func (i *rmMachine) buildCmd(m *machineTestBuilder) []string { - cmd := []string{"machine", "rm"} - if i.force { - cmd = append(cmd, "--force") - } - if i.saveIgnition { - cmd = append(cmd, "--save-ignition") - } - if i.saveImage { - cmd = append(cmd, "--save-image") - } - if i.saveKeys { - cmd = append(cmd, "--save-keys") - } - cmd = append(cmd, m.name) - i.cmd = cmd - return cmd -} - -func (i *rmMachine) withForce() *rmMachine { - i.force = true - return i -} - -func (i *rmMachine) withSaveIgnition() *rmMachine { - i.saveIgnition = true - return i -} - -func (i *rmMachine) withSaveImage() *rmMachine { - i.saveImage = true - return i -} - -func (i *rmMachine) withSaveKeys() *rmMachine { - i.saveKeys = true - return i -} diff --git a/pkg/machine/e2e/config_rm_test.go b/pkg/machine/e2e/config_rm_test.go new file mode 100644 index 000000000..1f9c9b4ec --- /dev/null +++ b/pkg/machine/e2e/config_rm_test.go @@ -0,0 +1,56 @@ +package e2e_test + +type rmMachine struct { + /* + -f, --force Stop and do not prompt before rming + --save-ignition Do not delete ignition file + --save-image Do not delete the image file + --save-keys Do not delete SSH keys + + */ + force bool + saveIgnition bool + saveImage bool + saveKeys bool + + cmd []string +} + +func (i *rmMachine) buildCmd(m *machineTestBuilder) []string { + cmd := []string{"machine", "rm"} + if i.force { + cmd = append(cmd, "--force") + } + if i.saveIgnition { + cmd = append(cmd, "--save-ignition") + } + if i.saveImage { + cmd = append(cmd, "--save-image") + } + if i.saveKeys { + cmd = append(cmd, "--save-keys") + } + cmd = append(cmd, m.name) + i.cmd = cmd + return cmd +} + +func (i *rmMachine) withForce() *rmMachine { + i.force = true + return i +} + +func (i *rmMachine) withSaveIgnition() *rmMachine { //nolint:unused + i.saveIgnition = true + return i +} + +func (i *rmMachine) withSaveImage() *rmMachine { //nolint:unused + i.saveImage = true + return i +} + +func (i *rmMachine) withSaveKeys() *rmMachine { //nolint:unused + i.saveKeys = true + return i +} diff --git a/pkg/machine/e2e/config_set.go b/pkg/machine/e2e/config_set.go deleted file mode 100644 index b310ab1b9..000000000 --- a/pkg/machine/e2e/config_set.go +++ /dev/null @@ -1,43 +0,0 @@ -package e2e - -import ( - "strconv" -) - -type setMachine struct { - cpus *uint - diskSize *uint - memory *uint - - cmd []string -} - -func (i *setMachine) buildCmd(m *machineTestBuilder) []string { - cmd := []string{"machine", "set"} - if i.cpus != nil { - cmd = append(cmd, "--cpus", strconv.Itoa(int(*i.cpus))) - } - if i.diskSize != nil { - cmd = append(cmd, "--disk-size", strconv.Itoa(int(*i.diskSize))) - } - if i.memory != nil { - cmd = append(cmd, "--memory", strconv.Itoa(int(*i.memory))) - } - cmd = append(cmd, m.name) - i.cmd = cmd - return cmd -} - -func (i *setMachine) withCPUs(num uint) *setMachine { - i.cpus = &num - return i -} -func (i *setMachine) withDiskSize(size uint) *setMachine { - i.diskSize = &size - return i -} - -func (i *setMachine) withMemory(num uint) *setMachine { - i.memory = &num - return i -} diff --git a/pkg/machine/e2e/config_set_test.go b/pkg/machine/e2e/config_set_test.go new file mode 100644 index 000000000..3c773b970 --- /dev/null +++ b/pkg/machine/e2e/config_set_test.go @@ -0,0 +1,43 @@ +package e2e_test + +import ( + "strconv" +) + +type setMachine struct { + cpus *uint + diskSize *uint + memory *uint + + cmd []string +} + +func (i *setMachine) buildCmd(m *machineTestBuilder) []string { + cmd := []string{"machine", "set"} + if i.cpus != nil { + cmd = append(cmd, "--cpus", strconv.Itoa(int(*i.cpus))) + } + if i.diskSize != nil { + cmd = append(cmd, "--disk-size", strconv.Itoa(int(*i.diskSize))) + } + if i.memory != nil { + cmd = append(cmd, "--memory", strconv.Itoa(int(*i.memory))) + } + cmd = append(cmd, m.name) + i.cmd = cmd + return cmd +} + +func (i *setMachine) withCPUs(num uint) *setMachine { + i.cpus = &num + return i +} +func (i *setMachine) withDiskSize(size uint) *setMachine { + i.diskSize = &size + return i +} + +func (i *setMachine) withMemory(num uint) *setMachine { + i.memory = &num + return i +} diff --git a/pkg/machine/e2e/config_ssh.go b/pkg/machine/e2e/config_ssh.go deleted file mode 100644 index b09eed47d..000000000 --- a/pkg/machine/e2e/config_ssh.go +++ /dev/null @@ -1,33 +0,0 @@ -package e2e - -type sshMachine struct { - /* - --username string Username to use when ssh-ing into the VM. - */ - - username string - sshCommand []string - - cmd []string -} - -func (s sshMachine) buildCmd(m *machineTestBuilder) []string { - cmd := []string{"machine", "ssh"} - if len(m.name) > 0 { - cmd = append(cmd, m.name) - } - if len(s.sshCommand) > 0 { - cmd = append(cmd, s.sshCommand...) - } - return cmd -} - -func (s *sshMachine) withUsername(name string) *sshMachine { - s.username = name - return s -} - -func (s *sshMachine) withSSHComand(sshCommand []string) *sshMachine { - s.sshCommand = sshCommand - return s -} diff --git a/pkg/machine/e2e/config_ssh_test.go b/pkg/machine/e2e/config_ssh_test.go new file mode 100644 index 000000000..f062625fa --- /dev/null +++ b/pkg/machine/e2e/config_ssh_test.go @@ -0,0 +1,31 @@ +package e2e_test + +type sshMachine struct { + /* + --username string Username to use when ssh-ing into the VM. + */ + + username string //nolint:unused + sshCommand []string +} + +func (s sshMachine) buildCmd(m *machineTestBuilder) []string { + cmd := []string{"machine", "ssh"} + if len(m.name) > 0 { + cmd = append(cmd, m.name) + } + if len(s.sshCommand) > 0 { + cmd = append(cmd, s.sshCommand...) + } + return cmd +} + +func (s *sshMachine) withUsername(name string) *sshMachine { //nolint:unused + s.username = name + return s +} + +func (s *sshMachine) withSSHComand(sshCommand []string) *sshMachine { + s.sshCommand = sshCommand + return s +} diff --git a/pkg/machine/e2e/config_start.go b/pkg/machine/e2e/config_start.go deleted file mode 100644 index 86b1721f8..000000000 --- a/pkg/machine/e2e/config_start.go +++ /dev/null @@ -1,16 +0,0 @@ -package e2e - -type startMachine struct { - /* - No command line args other than a machine vm name (also not required) - */ - cmd []string -} - -func (s startMachine) buildCmd(m *machineTestBuilder) []string { - cmd := []string{"machine", "start"} - if len(m.name) > 0 { - cmd = append(cmd, m.name) - } - return cmd -} diff --git a/pkg/machine/e2e/config_start_test.go b/pkg/machine/e2e/config_start_test.go new file mode 100644 index 000000000..d9efbf489 --- /dev/null +++ b/pkg/machine/e2e/config_start_test.go @@ -0,0 +1,15 @@ +package e2e_test + +type startMachine struct { + /* + No command line args other than a machine vm name (also not required) + */ +} + +func (s startMachine) buildCmd(m *machineTestBuilder) []string { + cmd := []string{"machine", "start"} + if len(m.name) > 0 { + cmd = append(cmd, m.name) + } + return cmd +} diff --git a/pkg/machine/e2e/config_stop.go b/pkg/machine/e2e/config_stop.go deleted file mode 100644 index 04dcfb524..000000000 --- a/pkg/machine/e2e/config_stop.go +++ /dev/null @@ -1,16 +0,0 @@ -package e2e - -type stopMachine struct { - /* - No command line args other than a machine vm name (also not required) - */ - cmd []string -} - -func (s stopMachine) buildCmd(m *machineTestBuilder) []string { - cmd := []string{"machine", "stop"} - if len(m.name) > 0 { - cmd = append(cmd, m.name) - } - return cmd -} diff --git a/pkg/machine/e2e/config_stop_test.go b/pkg/machine/e2e/config_stop_test.go new file mode 100644 index 000000000..41142ec7e --- /dev/null +++ b/pkg/machine/e2e/config_stop_test.go @@ -0,0 +1,15 @@ +package e2e_test + +type stopMachine struct { + /* + No command line args other than a machine vm name (also not required) + */ +} + +func (s stopMachine) buildCmd(m *machineTestBuilder) []string { + cmd := []string{"machine", "stop"} + if len(m.name) > 0 { + cmd = append(cmd, m.name) + } + return cmd +} diff --git a/pkg/machine/e2e/config_test.go b/pkg/machine/e2e/config_test.go new file mode 100644 index 000000000..9940e711b --- /dev/null +++ b/pkg/machine/e2e/config_test.go @@ -0,0 +1,175 @@ +package e2e_test + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/containers/podman/v4/pkg/machine" + "github.com/containers/podman/v4/pkg/util" + "github.com/containers/storage/pkg/stringid" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gexec" +) + +var originalHomeDir = os.Getenv("HOME") + +const ( + defaultTimeout time.Duration = 90 * time.Second +) + +type machineCommand interface { + buildCmd(m *machineTestBuilder) []string +} + +type MachineTestBuilder interface { + setName(string) *MachineTestBuilder + setCmd(mc machineCommand) *MachineTestBuilder + setTimeout(duration time.Duration) *MachineTestBuilder + run() (*machineSession, error) +} +type machineSession struct { + *Session +} + +type machineTestBuilder struct { + cmd []string + imagePath string + name string + names []string + podmanBinary string + timeout time.Duration +} + +// waitWithTimeout waits for a command to complete for a given +// number of seconds +func (ms *machineSession) waitWithTimeout(timeout time.Duration) { + Eventually(ms, timeout).Should(Exit()) + os.Stdout.Sync() + os.Stderr.Sync() +} + +func (ms *machineSession) Bytes() []byte { + return []byte(ms.outputToString()) +} + +func (ms *machineSession) outputToStringSlice() []string { + var results []string + output := string(ms.Out.Contents()) + for _, line := range strings.Split(output, "\n") { + if line != "" { + results = append(results, line) + } + } + return results +} + +// outputToString returns the output from a session in string form +func (ms *machineSession) outputToString() string { + if ms == nil || ms.Out == nil || ms.Out.Contents() == nil { + return "" + } + + fields := strings.Fields(string(ms.Out.Contents())) + return strings.Join(fields, " ") +} + +// errorToString returns the error output from a session in string form +func (ms *machineSession) errorToString() string { + if ms == nil || ms.Err == nil || ms.Err.Contents() == nil { + return "" + } + return string(ms.Err.Contents()) +} + +// newMB constructor for machine test builders +func newMB() (*machineTestBuilder, error) { + mb := machineTestBuilder{ + timeout: defaultTimeout, + } + cwd, err := os.Getwd() + if err != nil { + return nil, err + } + mb.podmanBinary = filepath.Join(cwd, "../../../bin/podman-remote") + if os.Getenv("PODMAN_BINARY") != "" { + mb.podmanBinary = os.Getenv("PODMAN_BINARY") + } + return &mb, nil +} + +// setName sets the name of the virtuaql machine for the command +func (m *machineTestBuilder) setName(name string) *machineTestBuilder { + m.name = name + return m +} + +// setCmd takes a machineCommand struct and assembles a cmd line +// representation of the podman machine command +func (m *machineTestBuilder) setCmd(mc machineCommand) *machineTestBuilder { + // If no name for the machine exists, we set a random name. + if !util.StringInSlice(m.name, m.names) { + if len(m.name) < 1 { + m.name = randomString() + } + m.names = append(m.names, m.name) + } + m.cmd = mc.buildCmd(m) + return m +} + +func (m *machineTestBuilder) setTimeout(timeout time.Duration) *machineTestBuilder { + m.timeout = timeout + return m +} + +// toQemuInspectInfo is only for inspecting qemu machines. Other providers will need +// to make their own. +func (m *machineTestBuilder) toQemuInspectInfo() ([]machine.InspectInfo, int, error) { + args := []string{"machine", "inspect"} + args = append(args, m.names...) + session, err := runWrapper(m.podmanBinary, args, defaultTimeout, true) + if err != nil { + return nil, -1, err + } + mii := []machine.InspectInfo{} + err = json.Unmarshal(session.Bytes(), &mii) + return mii, session.ExitCode(), err +} + +func (m *machineTestBuilder) runWithoutWait() (*machineSession, error) { + return runWrapper(m.podmanBinary, m.cmd, m.timeout, false) +} + +func (m *machineTestBuilder) run() (*machineSession, error) { + return runWrapper(m.podmanBinary, m.cmd, m.timeout, true) +} + +func runWrapper(podmanBinary string, cmdArgs []string, timeout time.Duration, wait bool) (*machineSession, error) { + if len(os.Getenv("DEBUG")) > 0 { + cmdArgs = append([]string{"--log-level=debug"}, cmdArgs...) + } + fmt.Println(podmanBinary + " " + strings.Join(cmdArgs, " ")) + c := exec.Command(podmanBinary, cmdArgs...) + session, err := Start(c, GinkgoWriter, GinkgoWriter) + if err != nil { + Fail(fmt.Sprintf("Unable to start session: %q", err)) + return nil, err + } + ms := machineSession{session} + if wait { + ms.waitWithTimeout(timeout) + fmt.Println("output:", ms.outputToString()) + } + return &ms, nil +} + +// randomString returns a string of given length composed of random characters +func randomString() string { + return stringid.GenerateRandomID()[0:12] +} diff --git a/pkg/machine/e2e/info_test.go b/pkg/machine/e2e/info_test.go index eeabb78af..759beecb5 100644 --- a/pkg/machine/e2e/info_test.go +++ b/pkg/machine/e2e/info_test.go @@ -1,4 +1,4 @@ -package e2e +package e2e_test import ( "github.com/containers/podman/v4/cmd/podman/machine" diff --git a/pkg/machine/e2e/init_test.go b/pkg/machine/e2e/init_test.go index 40f140cae..52a62cee3 100644 --- a/pkg/machine/e2e/init_test.go +++ b/pkg/machine/e2e/init_test.go @@ -1,4 +1,4 @@ -package e2e +package e2e_test import ( "io/ioutil" @@ -78,7 +78,7 @@ var _ = Describe("podman machine init", func() { }) It("machine init with cpus, disk size, memory, timezone", func() { - name := randomString(12) + name := randomString() i := new(initMachine) session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath).withCPUs(2).withDiskSize(102).withMemory(4000).withTimezone("Pacific/Honolulu")).run() Expect(err).To(BeNil()) @@ -130,7 +130,7 @@ var _ = Describe("podman machine init", func() { mount := tmpDir + ":/testmountdir" defer os.RemoveAll(tmpDir) - name := randomString(12) + name := randomString() i := new(initMachine) session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath).withVolume(mount)).run() Expect(err).To(BeNil()) diff --git a/pkg/machine/e2e/inspect_test.go b/pkg/machine/e2e/inspect_test.go index 93fb8cc2b..0ab928205 100644 --- a/pkg/machine/e2e/inspect_test.go +++ b/pkg/machine/e2e/inspect_test.go @@ -1,4 +1,4 @@ -package e2e +package e2e_test import ( "strings" @@ -52,15 +52,15 @@ var _ = Describe("podman machine stop", func() { }) It("inspect with go format", func() { - name := randomString(12) + name := randomString() i := new(initMachine) session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run() Expect(err).To(BeNil()) Expect(session).To(Exit(0)) // regular inspect should - inspectJson := new(inspectMachine) - inspectSession, err := mb.setName(name).setCmd(inspectJson).run() + inspectJSON := new(inspectMachine) + inspectSession, err := mb.setName(name).setCmd(inspectJSON).run() Expect(err).To(BeNil()) Expect(inspectSession).To(Exit(0)) diff --git a/pkg/machine/e2e/list_test.go b/pkg/machine/e2e/list_test.go index 8b7443d47..5c7ae6c5e 100644 --- a/pkg/machine/e2e/list_test.go +++ b/pkg/machine/e2e/list_test.go @@ -1,4 +1,4 @@ -package e2e +package e2e_test import ( "strings" @@ -45,8 +45,8 @@ var _ = Describe("podman machine list", func() { It("list machines with quiet or noheading", func() { // Random names for machines to test list - name1 := randomString(12) - name2 := randomString(12) + name1 := randomString() + name2 := randomString() list := new(listMachine) firstList, err := mb.setCmd(list.withQuiet()).run() @@ -109,7 +109,7 @@ var _ = Describe("podman machine list", func() { It("list with --format", func() { // Random names for machines to test list - name1 := randomString(12) + name1 := randomString() i := new(initMachine) session, err := mb.setName(name1).setCmd(i.withImagePath(mb.imagePath)).run() diff --git a/pkg/machine/e2e/machine_test.go b/pkg/machine/e2e/machine_test.go index 93eabdad3..5de04b9f7 100644 --- a/pkg/machine/e2e/machine_test.go +++ b/pkg/machine/e2e/machine_test.go @@ -1,4 +1,4 @@ -package e2e +package e2e_test import ( "fmt" diff --git a/pkg/machine/e2e/rm_test.go b/pkg/machine/e2e/rm_test.go index 43b8c594c..e33eaf702 100644 --- a/pkg/machine/e2e/rm_test.go +++ b/pkg/machine/e2e/rm_test.go @@ -1,4 +1,4 @@ -package e2e +package e2e_test import ( . "github.com/onsi/ginkgo" diff --git a/pkg/machine/e2e/set_test.go b/pkg/machine/e2e/set_test.go index 80cb89488..df4136a82 100644 --- a/pkg/machine/e2e/set_test.go +++ b/pkg/machine/e2e/set_test.go @@ -1,4 +1,4 @@ -package e2e +package e2e_test import ( "runtime" @@ -22,7 +22,7 @@ var _ = Describe("podman machine set", func() { }) It("set machine cpus, disk, memory", func() { - name := randomString(12) + name := randomString() i := new(initMachine) session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run() Expect(err).To(BeNil()) @@ -75,7 +75,7 @@ var _ = Describe("podman machine set", func() { }) It("no settings should change if no flags", func() { - name := randomString(12) + name := randomString() i := new(initMachine) session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run() Expect(err).To(BeNil()) diff --git a/pkg/machine/e2e/ssh_test.go b/pkg/machine/e2e/ssh_test.go index 9ee31ac26..6d23a024e 100644 --- a/pkg/machine/e2e/ssh_test.go +++ b/pkg/machine/e2e/ssh_test.go @@ -1,4 +1,4 @@ -package e2e +package e2e_test import ( . "github.com/onsi/ginkgo" @@ -20,17 +20,17 @@ var _ = Describe("podman machine ssh", func() { }) It("bad machine name", func() { - name := randomString(12) + name := randomString() ssh := sshMachine{} session, err := mb.setName(name).setCmd(ssh).run() Expect(err).To(BeNil()) Expect(session).To(Exit(125)) // TODO seems like stderr is not being returned; re-enabled when fixed - //Expect(session.outputToString()).To(ContainSubstring("not exist")) + // Expect(session.outputToString()).To(ContainSubstring("not exist")) }) It("ssh to non-running machine", func() { - name := randomString(12) + name := randomString() i := new(initMachine) session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run() Expect(err).To(BeNil()) @@ -40,12 +40,12 @@ var _ = Describe("podman machine ssh", func() { sshSession, err := mb.setName(name).setCmd(ssh).run() Expect(err).To(BeNil()) // TODO seems like stderr is not being returned; re-enabled when fixed - //Expect(sshSession.outputToString()).To(ContainSubstring("is not running")) + // Expect(sshSession.outputToString()).To(ContainSubstring("is not running")) Expect(sshSession).To(Exit(125)) }) It("ssh to running machine and check os-type", func() { - name := randomString(12) + name := randomString() i := new(initMachine) session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath).withNow()).run() Expect(err).To(BeNil()) diff --git a/pkg/machine/e2e/start_test.go b/pkg/machine/e2e/start_test.go index 1de66eb9a..1f9405569 100644 --- a/pkg/machine/e2e/start_test.go +++ b/pkg/machine/e2e/start_test.go @@ -1,4 +1,4 @@ -package e2e +package e2e_test import ( "github.com/containers/podman/v4/pkg/machine" diff --git a/pkg/machine/e2e/stop_test.go b/pkg/machine/e2e/stop_test.go index 0c27045a6..621bbdb16 100644 --- a/pkg/machine/e2e/stop_test.go +++ b/pkg/machine/e2e/stop_test.go @@ -1,4 +1,4 @@ -package e2e +package e2e_test import ( . "github.com/onsi/ginkgo" -- cgit v1.2.3-54-g00ecf From 69fcf04c69068a0610a496d479dba86d46f0901f Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 21 Jul 2022 11:41:13 +0200 Subject: fix some pkg/machine/e2e test to read stderr Also fix the machine ssh code order to provide a better error message. Signed-off-by: Paul Holzinger --- cmd/podman/machine/ssh.go | 9 +++++---- pkg/machine/e2e/ssh_test.go | 6 ++---- 2 files changed, 7 insertions(+), 8 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/machine/ssh.go b/cmd/podman/machine/ssh.go index cb2f62f51..8534b8efa 100644 --- a/cmd/podman/machine/ssh.go +++ b/cmd/podman/machine/ssh.go @@ -80,6 +80,11 @@ func ssh(cmd *cobra.Command, args []string) error { } } + vm, err = provider.LoadVMByName(vmName) + if err != nil { + return fmt.Errorf("vm %s not found: %w", vmName, err) + } + if !validVM && sshOpts.Username == "" { sshOpts.Username, err = remoteConnectionUsername() if err != nil { @@ -87,10 +92,6 @@ func ssh(cmd *cobra.Command, args []string) error { } } - vm, err = provider.LoadVMByName(vmName) - if err != nil { - return fmt.Errorf("vm %s not found: %w", vmName, err) - } err = vm.SSH(vmName, sshOpts) return utils.HandleOSExecError(err) } diff --git a/pkg/machine/e2e/ssh_test.go b/pkg/machine/e2e/ssh_test.go index 6d23a024e..52d714c91 100644 --- a/pkg/machine/e2e/ssh_test.go +++ b/pkg/machine/e2e/ssh_test.go @@ -25,8 +25,7 @@ var _ = Describe("podman machine ssh", func() { session, err := mb.setName(name).setCmd(ssh).run() Expect(err).To(BeNil()) Expect(session).To(Exit(125)) - // TODO seems like stderr is not being returned; re-enabled when fixed - // Expect(session.outputToString()).To(ContainSubstring("not exist")) + Expect(session.errorToString()).To(ContainSubstring("not exist")) }) It("ssh to non-running machine", func() { @@ -39,8 +38,7 @@ var _ = Describe("podman machine ssh", func() { ssh := sshMachine{} sshSession, err := mb.setName(name).setCmd(ssh).run() Expect(err).To(BeNil()) - // TODO seems like stderr is not being returned; re-enabled when fixed - // Expect(sshSession.outputToString()).To(ContainSubstring("is not running")) + Expect(sshSession.errorToString()).To(ContainSubstring("is not running")) Expect(sshSession).To(Exit(125)) }) -- cgit v1.2.3-54-g00ecf From cbcbde587de9f14e696be9a58d6e36c7c45e926d Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 21 Jul 2022 11:57:23 +0200 Subject: pkg/machine/e2e: do not import from cmd/podman The same problem again as 4374038cc67405e3f5555b1870d5bb7f6570fa5d. Also fix the incorrect --format autocompletion struct. It should be avoided to import cmd/podman/... packages from outside of cmd/podman. This can lead in weird hard to debug import paths but also can have negative consequences when imported in unit tests. In this case it will set XDG_CONFIG_HOME and thus the machine tests this dir over the tmp HOME env variable which is set at a later point. This caused machine files to be leaked into the actual users home dir. Signed-off-by: Paul Holzinger --- cmd/podman/machine/info.go | 29 +++++------------------------ pkg/domain/entities/machine.go | 22 ++++++++++++++++++++++ pkg/machine/e2e/info_test.go | 4 ++-- 3 files changed, 29 insertions(+), 26 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/machine/info.go b/cmd/podman/machine/info.go index 9932027d8..418060675 100644 --- a/cmd/podman/machine/info.go +++ b/cmd/podman/machine/info.go @@ -16,6 +16,7 @@ import ( "github.com/containers/podman/v4/cmd/podman/registry" "github.com/containers/podman/v4/cmd/podman/validate" "github.com/containers/podman/v4/libpod/define" + "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/machine" "github.com/ghodss/yaml" "github.com/spf13/cobra" @@ -40,26 +41,6 @@ var ( inFormat string ) -// Info contains info on the machine host and version info -type Info struct { - Host *HostInfo `json:"Host"` - Version define.Version `json:"Version"` -} - -// HostInfo contains info on the machine host -type HostInfo struct { - Arch string `json:"Arch"` - CurrentMachine string `json:"CurrentMachine"` - DefaultMachine string `json:"DefaultMachine"` - EventsDir string `json:"EventsDir"` - MachineConfigDir string `json:"MachineConfigDir"` - MachineImageDir string `json:"MachineImageDir"` - MachineState string `json:"MachineState"` - NumberOfMachines int `json:"NumberOfMachines"` - OS string `json:"OS"` - VMType string `json:"VMType"` -} - func init() { registry.Commands = append(registry.Commands, registry.CliCommand{ Command: infoCmd, @@ -69,11 +50,11 @@ func init() { flags := infoCmd.Flags() formatFlagName := "format" flags.StringVarP(&inFormat, formatFlagName, "f", "", "Change the output format to JSON or a Go template") - _ = infoCmd.RegisterFlagCompletionFunc(formatFlagName, common.AutocompleteFormat(&define.Info{})) + _ = infoCmd.RegisterFlagCompletionFunc(formatFlagName, common.AutocompleteFormat(&entities.MachineInfo{})) } func info(cmd *cobra.Command, args []string) error { - info := Info{} + info := entities.MachineInfo{} version, err := define.GetVersion() if err != nil { return fmt.Errorf("error getting version info %w", err) @@ -112,8 +93,8 @@ func info(cmd *cobra.Command, args []string) error { return nil } -func hostInfo() (*HostInfo, error) { - host := HostInfo{} +func hostInfo() (*entities.MachineHostInfo, error) { + host := entities.MachineHostInfo{} host.Arch = runtime.GOARCH host.OS = runtime.GOOS diff --git a/pkg/domain/entities/machine.go b/pkg/domain/entities/machine.go index 6ba53dbd1..4fd0413c9 100644 --- a/pkg/domain/entities/machine.go +++ b/pkg/domain/entities/machine.go @@ -1,5 +1,7 @@ package entities +import "github.com/containers/podman/v4/libpod/define" + type ListReporter struct { Name string Default bool @@ -16,3 +18,23 @@ type ListReporter struct { RemoteUsername string IdentityPath string } + +// MachineInfo contains info on the machine host and version info +type MachineInfo struct { + Host *MachineHostInfo `json:"Host"` + Version define.Version `json:"Version"` +} + +// MachineHostInfo contains info on the machine host +type MachineHostInfo struct { + Arch string `json:"Arch"` + CurrentMachine string `json:"CurrentMachine"` + DefaultMachine string `json:"DefaultMachine"` + EventsDir string `json:"EventsDir"` + MachineConfigDir string `json:"MachineConfigDir"` + MachineImageDir string `json:"MachineImageDir"` + MachineState string `json:"MachineState"` + NumberOfMachines int `json:"NumberOfMachines"` + OS string `json:"OS"` + VMType string `json:"VMType"` +} diff --git a/pkg/machine/e2e/info_test.go b/pkg/machine/e2e/info_test.go index 759beecb5..fe0cfba32 100644 --- a/pkg/machine/e2e/info_test.go +++ b/pkg/machine/e2e/info_test.go @@ -1,7 +1,7 @@ package e2e_test import ( - "github.com/containers/podman/v4/cmd/podman/machine" + "github.com/containers/podman/v4/pkg/domain/entities" jsoniter "github.com/json-iterator/go" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -51,7 +51,7 @@ var _ = Describe("podman machine info", func() { Expect(err).NotTo(HaveOccurred()) Expect(infoSession).Should(Exit(0)) - infoReport := &machine.Info{} + infoReport := &entities.MachineInfo{} err = jsoniter.Unmarshal(infoSession.Bytes(), infoReport) Expect(err).To(BeNil()) }) -- cgit v1.2.3-54-g00ecf From 1f48980af9e44f4096b8e67edd5f4596f69e204f Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 21 Jul 2022 18:11:53 +0200 Subject: fix broken machine test The memory both local and in the CI test is converted to 3822. I don't know why this changed but I want to have this working again. For the future we should look at a more robust solution. Fixes #15012 Signed-off-by: Paul Holzinger --- pkg/machine/e2e/init_test.go | 2 +- pkg/machine/e2e/set_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'pkg') diff --git a/pkg/machine/e2e/init_test.go b/pkg/machine/e2e/init_test.go index 52a62cee3..b246dc4da 100644 --- a/pkg/machine/e2e/init_test.go +++ b/pkg/machine/e2e/init_test.go @@ -108,7 +108,7 @@ var _ = Describe("podman machine init", func() { switch runtime.GOOS { // os's handle memory differently case "linux": - Expect(memorySession.outputToString()).To(ContainSubstring("3821")) + Expect(memorySession.outputToString()).To(ContainSubstring("3822")) case "darwin": Expect(memorySession.outputToString()).To(ContainSubstring("3824")) default: diff --git a/pkg/machine/e2e/set_test.go b/pkg/machine/e2e/set_test.go index df4136a82..4839e33da 100644 --- a/pkg/machine/e2e/set_test.go +++ b/pkg/machine/e2e/set_test.go @@ -62,7 +62,7 @@ var _ = Describe("podman machine set", func() { switch runtime.GOOS { // it seems macos and linux handle memory differently case "linux": - Expect(memorySession.outputToString()).To(ContainSubstring("3821")) + Expect(memorySession.outputToString()).To(ContainSubstring("3822")) case "darwin": Expect(memorySession.outputToString()).To(ContainSubstring("3824")) default: -- cgit v1.2.3-54-g00ecf From fde39edb9cc173871925a66df3941917e33bf45e Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 21 Jul 2022 14:09:44 +0200 Subject: remote push: show copy progress `podman-remote push` has shown absolutely no progress at all. Fix that by doing essentially the same as the remote-pull code does. The get-free-out-of-jail-card for backwards compatibility is to let the `quiet` parameter default to true. Since the --quioet flag wasn't working before either, older Podman clients do not set it. Also add regression tests to make sure we won't regress again. Fixes: #11554 Fixes: #14971 Signed-off-by: Valentin Rothberg --- pkg/api/handlers/libpod/images.go | 71 --------------- pkg/api/handlers/libpod/images_push.go | 144 ++++++++++++++++++++++++++++++ pkg/api/server/register_images.go | 5 ++ pkg/bindings/images/images.go | 40 --------- pkg/bindings/images/push.go | 96 ++++++++++++++++++++ pkg/bindings/images/types.go | 2 + pkg/bindings/images/types_push_options.go | 15 ++++ pkg/domain/entities/images.go | 15 +++- pkg/domain/infra/abi/images.go | 3 +- pkg/domain/infra/tunnel/images.go | 2 +- test/e2e/push_test.go | 27 ++++-- 11 files changed, 297 insertions(+), 123 deletions(-) create mode 100644 pkg/api/handlers/libpod/images_push.go create mode 100644 pkg/bindings/images/push.go (limited to 'pkg') diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index ed1c65f8e..67943ecf1 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/pkg/api/handlers/libpod/images.go @@ -1,7 +1,6 @@ package libpod import ( - "context" "errors" "fmt" "io" @@ -14,13 +13,11 @@ import ( "github.com/containers/buildah" "github.com/containers/common/libimage" "github.com/containers/image/v5/manifest" - "github.com/containers/image/v5/types" "github.com/containers/podman/v4/libpod" "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/pkg/api/handlers" "github.com/containers/podman/v4/pkg/api/handlers/utils" api "github.com/containers/podman/v4/pkg/api/types" - "github.com/containers/podman/v4/pkg/auth" "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/domain/entities/reports" "github.com/containers/podman/v4/pkg/domain/infra/abi" @@ -416,74 +413,6 @@ func ImagesImport(w http.ResponseWriter, r *http.Request) { utils.WriteResponse(w, http.StatusOK, report) } -// PushImage is the handler for the compat http endpoint for pushing images. -func PushImage(w http.ResponseWriter, r *http.Request) { - decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) - runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) - - query := struct { - All bool `schema:"all"` - Destination string `schema:"destination"` - Format string `schema:"format"` - RemoveSignatures bool `schema:"removeSignatures"` - TLSVerify bool `schema:"tlsVerify"` - }{ - // This is where you can override the golang default value for one of fields - } - if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse parameters for %s: %w", r.URL.String(), err)) - return - } - - source := strings.TrimSuffix(utils.GetName(r), "/push") // GetName returns the entire path - if _, err := utils.ParseStorageReference(source); err != nil { - utils.Error(w, http.StatusBadRequest, err) - return - } - - destination := query.Destination - if destination == "" { - destination = source - } - - if err := utils.IsRegistryReference(destination); err != nil { - utils.Error(w, http.StatusBadRequest, err) - return - } - - authconf, authfile, err := auth.GetCredentials(r) - if err != nil { - utils.Error(w, http.StatusBadRequest, err) - return - } - defer auth.RemoveAuthfile(authfile) - var username, password string - if authconf != nil { - username = authconf.Username - password = authconf.Password - } - options := entities.ImagePushOptions{ - All: query.All, - Authfile: authfile, - Format: query.Format, - Password: password, - Quiet: true, - RemoveSignatures: query.RemoveSignatures, - Username: username, - } - if _, found := r.URL.Query()["tlsVerify"]; found { - options.SkipTLSVerify = types.NewOptionalBool(!query.TLSVerify) - } - - imageEngine := abi.ImageEngine{Libpod: runtime} - if err := imageEngine.Push(context.Background(), source, destination, options); err != nil { - utils.Error(w, http.StatusBadRequest, fmt.Errorf("error pushing image %q: %w", destination, err)) - return - } - - utils.WriteResponse(w, http.StatusOK, "") -} - func CommitContainer(w http.ResponseWriter, r *http.Request) { var ( destImage string diff --git a/pkg/api/handlers/libpod/images_push.go b/pkg/api/handlers/libpod/images_push.go new file mode 100644 index 000000000..f427dc01b --- /dev/null +++ b/pkg/api/handlers/libpod/images_push.go @@ -0,0 +1,144 @@ +package libpod + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "strings" + + "github.com/containers/image/v5/types" + "github.com/containers/podman/v4/libpod" + "github.com/containers/podman/v4/pkg/api/handlers/utils" + api "github.com/containers/podman/v4/pkg/api/types" + "github.com/containers/podman/v4/pkg/auth" + "github.com/containers/podman/v4/pkg/channel" + "github.com/containers/podman/v4/pkg/domain/entities" + "github.com/containers/podman/v4/pkg/domain/infra/abi" + "github.com/gorilla/schema" + "github.com/sirupsen/logrus" +) + +// PushImage is the handler for the compat http endpoint for pushing images. +func PushImage(w http.ResponseWriter, r *http.Request) { + decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) + runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) + + query := struct { + All bool `schema:"all"` + Destination string `schema:"destination"` + Format string `schema:"format"` + RemoveSignatures bool `schema:"removeSignatures"` + TLSVerify bool `schema:"tlsVerify"` + Quiet bool `schema:"quiet"` + }{ + // #14971: older versions did not sent *any* data, so we need + // to be quiet by default to remain backwards compatible + Quiet: true, + } + if err := decoder.Decode(&query, r.URL.Query()); err != nil { + utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse parameters for %s: %w", r.URL.String(), err)) + return + } + + source := strings.TrimSuffix(utils.GetName(r), "/push") // GetName returns the entire path + if _, err := utils.ParseStorageReference(source); err != nil { + utils.Error(w, http.StatusBadRequest, err) + return + } + + destination := query.Destination + if destination == "" { + destination = source + } + + if err := utils.IsRegistryReference(destination); err != nil { + utils.Error(w, http.StatusBadRequest, err) + return + } + + authconf, authfile, err := auth.GetCredentials(r) + if err != nil { + utils.Error(w, http.StatusBadRequest, err) + return + } + defer auth.RemoveAuthfile(authfile) + + var username, password string + if authconf != nil { + username = authconf.Username + password = authconf.Password + } + options := entities.ImagePushOptions{ + All: query.All, + Authfile: authfile, + Format: query.Format, + Password: password, + Quiet: true, + RemoveSignatures: query.RemoveSignatures, + Username: username, + } + + if _, found := r.URL.Query()["tlsVerify"]; found { + options.SkipTLSVerify = types.NewOptionalBool(!query.TLSVerify) + } + + imageEngine := abi.ImageEngine{Libpod: runtime} + + // Let's keep thing simple when running in quiet mode and push directly. + if query.Quiet { + if err := imageEngine.Push(context.Background(), source, destination, options); err != nil { + utils.Error(w, http.StatusBadRequest, fmt.Errorf("error pushing image %q: %w", destination, err)) + return + } + utils.WriteResponse(w, http.StatusOK, "") + return + } + + writer := channel.NewWriter(make(chan []byte)) + defer writer.Close() + options.Writer = writer + + pushCtx, pushCancel := context.WithCancel(r.Context()) + var pushError error + go func() { + defer pushCancel() + pushError = imageEngine.Push(pushCtx, source, destination, options) + }() + + flush := func() { + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() + } + } + + w.WriteHeader(http.StatusOK) + w.Header().Set("Content-Type", "application/json") + flush() + + enc := json.NewEncoder(w) + enc.SetEscapeHTML(true) + for { + var report entities.ImagePushReport + select { + case s := <-writer.Chan(): + report.Stream = string(s) + if err := enc.Encode(report); err != nil { + logrus.Warnf("Failed to encode json: %v", err) + } + flush() + case <-pushCtx.Done(): + if pushError != nil { + report.Error = pushError.Error() + if err := enc.Encode(report); err != nil { + logrus.Warnf("Failed to encode json: %v", err) + } + } + flush() + return + case <-r.Context().Done(): + // Client has closed connection + return + } + } +} diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index a2f46cb35..11ab8cae0 100644 --- a/pkg/api/server/register_images.go +++ b/pkg/api/server/register_images.go @@ -730,6 +730,11 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // description: Require TLS verification. // type: boolean // default: true + // - in: query + // name: quiet + // description: "silences extra stream data on push" + // type: boolean + // default: true // - in: header // name: X-Registry-Auth // type: string diff --git a/pkg/bindings/images/images.go b/pkg/bindings/images/images.go index cd5147629..bb7867c4e 100644 --- a/pkg/bindings/images/images.go +++ b/pkg/bindings/images/images.go @@ -267,46 +267,6 @@ func Import(ctx context.Context, r io.Reader, options *ImportOptions) (*entities return &report, response.Process(&report) } -// Push is the binding for libpod's v2 endpoints for push images. Note that -// `source` must be a referring to an image in the remote's container storage. -// The destination must be a reference to a registry (i.e., of docker transport -// or be normalized to one). Other transports are rejected as they do not make -// sense in a remote context. -func Push(ctx context.Context, source string, destination string, options *PushOptions) error { - if options == nil { - options = new(PushOptions) - } - conn, err := bindings.GetClient(ctx) - if err != nil { - return err - } - header, err := auth.MakeXRegistryAuthHeader(&imageTypes.SystemContext{AuthFilePath: options.GetAuthfile()}, options.GetUsername(), options.GetPassword()) - if err != nil { - return err - } - - params, err := options.ToParams() - if err != nil { - return err - } - // SkipTLSVerify is special. We need to delete the param added by - // toparams and change the key and flip the bool - if options.SkipTLSVerify != nil { - params.Del("SkipTLSVerify") - params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify())) - } - params.Set("destination", destination) - - path := fmt.Sprintf("/images/%s/push", source) - response, err := conn.DoRequest(ctx, nil, http.MethodPost, path, params, header) - if err != nil { - return err - } - defer response.Body.Close() - - return response.Process(err) -} - // Search is the binding for libpod's v2 endpoints for Search images. func Search(ctx context.Context, term string, options *SearchOptions) ([]entities.ImageSearchReport, error) { if options == nil { diff --git a/pkg/bindings/images/push.go b/pkg/bindings/images/push.go new file mode 100644 index 000000000..8db3726e6 --- /dev/null +++ b/pkg/bindings/images/push.go @@ -0,0 +1,96 @@ +package images + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "io/ioutil" + "net/http" + "os" + "strconv" + + imageTypes "github.com/containers/image/v5/types" + "github.com/containers/podman/v4/pkg/auth" + "github.com/containers/podman/v4/pkg/bindings" + "github.com/containers/podman/v4/pkg/domain/entities" +) + +// Push is the binding for libpod's endpoints for push images. Note that +// `source` must be a referring to an image in the remote's container storage. +// The destination must be a reference to a registry (i.e., of docker transport +// or be normalized to one). Other transports are rejected as they do not make +// sense in a remote context. +func Push(ctx context.Context, source string, destination string, options *PushOptions) error { + if options == nil { + options = new(PushOptions) + } + conn, err := bindings.GetClient(ctx) + if err != nil { + return err + } + header, err := auth.MakeXRegistryAuthHeader(&imageTypes.SystemContext{AuthFilePath: options.GetAuthfile()}, options.GetUsername(), options.GetPassword()) + if err != nil { + return err + } + + params, err := options.ToParams() + if err != nil { + return err + } + // SkipTLSVerify is special. We need to delete the param added by + // toparams and change the key and flip the bool + if options.SkipTLSVerify != nil { + params.Del("SkipTLSVerify") + params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify())) + } + params.Set("destination", destination) + + path := fmt.Sprintf("/images/%s/push", source) + response, err := conn.DoRequest(ctx, nil, http.MethodPost, path, params, header) + if err != nil { + return err + } + defer response.Body.Close() + + if !response.IsSuccess() { + return response.Process(err) + } + + // Historically push writes status to stderr + writer := io.Writer(os.Stderr) + if options.GetQuiet() { + writer = ioutil.Discard + } + + dec := json.NewDecoder(response.Body) + for { + var report entities.ImagePushReport + if err := dec.Decode(&report); err != nil { + if errors.Is(err, io.EOF) { + break + } + return err + } + + select { + case <-response.Request.Context().Done(): + break + default: + // non-blocking select + } + + switch { + case report.Stream != "": + fmt.Fprint(writer, report.Stream) + case report.Error != "": + // There can only be one error. + return errors.New(report.Error) + default: + return fmt.Errorf("failed to parse push results stream, unexpected input: %v", report) + } + } + + return nil +} diff --git a/pkg/bindings/images/types.go b/pkg/bindings/images/types.go index 3728ae5c0..0e672cdea 100644 --- a/pkg/bindings/images/types.go +++ b/pkg/bindings/images/types.go @@ -133,6 +133,8 @@ type PushOptions struct { RemoveSignatures *bool // Username for authenticating against the registry. Username *string + // Quiet can be specified to suppress progress when pushing. + Quiet *bool } //go:generate go run ../generator/generator.go SearchOptions diff --git a/pkg/bindings/images/types_push_options.go b/pkg/bindings/images/types_push_options.go index 25f6c5546..63a19fb81 100644 --- a/pkg/bindings/images/types_push_options.go +++ b/pkg/bindings/images/types_push_options.go @@ -136,3 +136,18 @@ func (o *PushOptions) GetUsername() string { } return *o.Username } + +// WithQuiet set field Quiet to given value +func (o *PushOptions) WithQuiet(value bool) *PushOptions { + o.Quiet = &value + return o +} + +// GetQuiet returns value of field Quiet +func (o *PushOptions) GetQuiet() bool { + if o.Quiet == nil { + var z bool + return z + } + return *o.Quiet +} diff --git a/pkg/domain/entities/images.go b/pkg/domain/entities/images.go index da317cfad..b8b346005 100644 --- a/pkg/domain/entities/images.go +++ b/pkg/domain/entities/images.go @@ -1,6 +1,7 @@ package entities import ( + "io" "net/url" "time" @@ -192,8 +193,7 @@ type ImagePushOptions struct { // image. Default is manifest type of source, with fallbacks. // Ignored for remote calls. Format string - // Quiet can be specified to suppress pull progress when pulling. Ignored - // for remote calls. + // Quiet can be specified to suppress push progress when pushing. Quiet bool // Rm indicates whether to remove the manifest list if push succeeds Rm bool @@ -211,6 +211,17 @@ type ImagePushOptions struct { Progress chan types.ProgressProperties // CompressionFormat is the format to use for the compression of the blobs CompressionFormat string + // Writer is used to display copy information including progress bars. + Writer io.Writer +} + +// ImagePushReport is the response from pushing an image. +// Currently only used in the remote API. +type ImagePushReport struct { + // Stream used to provide push progress + Stream string `json:"stream,omitempty"` + // Error contains text of errors from pushing + Error string `json:"error,omitempty"` } // ImageSearchOptions are the arguments for searching images. diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index 38008c7b9..ff42b0367 100644 --- a/pkg/domain/infra/abi/images.go +++ b/pkg/domain/infra/abi/images.go @@ -305,6 +305,7 @@ func (ir *ImageEngine) Push(ctx context.Context, source string, destination stri pushOptions.RemoveSignatures = options.RemoveSignatures pushOptions.SignBy = options.SignBy pushOptions.InsecureSkipTLSVerify = options.SkipTLSVerify + pushOptions.Writer = options.Writer compressionFormat := options.CompressionFormat if compressionFormat == "" { @@ -322,7 +323,7 @@ func (ir *ImageEngine) Push(ctx context.Context, source string, destination stri pushOptions.CompressionFormat = &algo } - if !options.Quiet { + if !options.Quiet && pushOptions.Writer == nil { pushOptions.Writer = os.Stderr } diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index 18f750dcc..9ad408850 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/pkg/domain/infra/tunnel/images.go @@ -240,7 +240,7 @@ func (ir *ImageEngine) Import(ctx context.Context, opts entities.ImageImportOpti func (ir *ImageEngine) Push(ctx context.Context, source string, destination string, opts entities.ImagePushOptions) error { options := new(images.PushOptions) - options.WithAll(opts.All).WithCompress(opts.Compress).WithUsername(opts.Username).WithPassword(opts.Password).WithAuthfile(opts.Authfile).WithFormat(opts.Format).WithRemoveSignatures(opts.RemoveSignatures) + options.WithAll(opts.All).WithCompress(opts.Compress).WithUsername(opts.Username).WithPassword(opts.Password).WithAuthfile(opts.Authfile).WithFormat(opts.Format).WithRemoveSignatures(opts.RemoveSignatures).WithQuiet(opts.Quiet) if s := opts.SkipTLSVerify; s != types.OptionalBoolUndefined { if s == types.OptionalBoolTrue { diff --git a/test/e2e/push_test.go b/test/e2e/push_test.go index 97567e40d..f2a103f6b 100644 --- a/test/e2e/push_test.go +++ b/test/e2e/push_test.go @@ -116,15 +116,26 @@ var _ = Describe("Podman push", func() { push := podmanTest.Podman([]string{"push", "-q", "--tls-verify=false", "--remove-signatures", ALPINE, "localhost:5000/my-alpine"}) push.WaitWithDefaultTimeout() Expect(push).Should(Exit(0)) + Expect(len(push.ErrorToString())).To(Equal(0)) - SkipIfRemote("Remote does not support --digestfile") - // Test --digestfile option - push2 := podmanTest.Podman([]string{"push", "--tls-verify=false", "--digestfile=/tmp/digestfile.txt", "--remove-signatures", ALPINE, "localhost:5000/my-alpine"}) - push2.WaitWithDefaultTimeout() - fi, err := os.Lstat("/tmp/digestfile.txt") - Expect(err).To(BeNil()) - Expect(fi.Name()).To(Equal("digestfile.txt")) - Expect(push2).Should(Exit(0)) + push = podmanTest.Podman([]string{"push", "--tls-verify=false", "--remove-signatures", ALPINE, "localhost:5000/my-alpine"}) + push.WaitWithDefaultTimeout() + Expect(push).Should(Exit(0)) + output := push.ErrorToString() + Expect(output).To(ContainSubstring("Copying blob ")) + Expect(output).To(ContainSubstring("Copying config ")) + Expect(output).To(ContainSubstring("Writing manifest to image destination")) + Expect(output).To(ContainSubstring("Storing signatures")) + + if !IsRemote() { // Remote does not support --digestfile + // Test --digestfile option + push2 := podmanTest.Podman([]string{"push", "--tls-verify=false", "--digestfile=/tmp/digestfile.txt", "--remove-signatures", ALPINE, "localhost:5000/my-alpine"}) + push2.WaitWithDefaultTimeout() + fi, err := os.Lstat("/tmp/digestfile.txt") + Expect(err).To(BeNil()) + Expect(fi.Name()).To(Equal("digestfile.txt")) + Expect(push2).Should(Exit(0)) + } }) It("podman push to local registry with authorization", func() { -- cgit v1.2.3-54-g00ecf From fa7e9f0f81f5478d456d2c4d6f891c636ddb9a49 Mon Sep 17 00:00:00 2001 From: Jakub Guzik Date: Tue, 19 Jul 2022 08:46:22 +0200 Subject: Compat API: unify pull/push and add missing progress info Progress bar in JSONMessage is missing compared to docker output both in pull and push. Additionaly, pull was not using JSONMessage while push was using the type. [NO NEW TESTS NEEDED] Signed-off-by: Jakub Guzik --- pkg/api/handlers/compat/images.go | 32 +++++++++++++++++--------------- pkg/api/handlers/compat/images_push.go | 1 + 2 files changed, 18 insertions(+), 15 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/images.go b/pkg/api/handlers/compat/images.go index 2f8d151d8..39bd165d6 100644 --- a/pkg/api/handlers/compat/images.go +++ b/pkg/api/handlers/compat/images.go @@ -23,6 +23,7 @@ import ( "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/domain/infra/abi" "github.com/containers/storage" + "github.com/docker/docker/pkg/jsonmessage" "github.com/gorilla/schema" "github.com/opencontainers/go-digest" "github.com/sirupsen/logrus" @@ -325,16 +326,8 @@ func CreateImageFromImage(w http.ResponseWriter, r *http.Request) { loop: // break out of for/select infinite loop for { - var report struct { - Stream string `json:"stream,omitempty"` - Status string `json:"status,omitempty"` - Progress struct { - Current uint64 `json:"current,omitempty"` - Total int64 `json:"total,omitempty"` - } `json:"progressDetail,omitempty"` - Error string `json:"error,omitempty"` - Id string `json:"id,omitempty"` //nolint:revive,stylecheck - } + report := jsonmessage.JSONMessage{} + report.Progress = &jsonmessage.JSONProgress{} select { case e := <-progress: switch e.Event { @@ -342,14 +335,15 @@ loop: // break out of for/select infinite loop report.Status = "Pulling fs layer" case types.ProgressEventRead: report.Status = "Downloading" - report.Progress.Current = e.Offset + report.Progress.Current = int64(e.Offset) report.Progress.Total = e.Artifact.Size + report.ProgressMessage = report.Progress.String() case types.ProgressEventSkipped: report.Status = "Already exists" case types.ProgressEventDone: report.Status = "Download complete" } - report.Id = e.Artifact.Digest.Encoded()[0:12] + report.ID = e.Artifact.Digest.Encoded()[0:12] if err := enc.Encode(report); err != nil { logrus.Warnf("Failed to json encode error %q", err.Error()) } @@ -358,7 +352,11 @@ loop: // break out of for/select infinite loop err := pullRes.err pulledImages := pullRes.images if err != nil { - report.Error = err.Error() + msg := err.Error() + report.Error = &jsonmessage.JSONError{ + Message: msg, + } + report.ErrorMessage = msg } else { if len(pulledImages) > 0 { img := pulledImages[0].ID() @@ -367,9 +365,13 @@ loop: // break out of for/select infinite loop } else { report.Status = "Download complete" } - report.Id = img[0:12] + report.ID = img[0:12] } else { - report.Error = "internal error: no images pulled" + msg := "internal error: no images pulled" + report.Error = &jsonmessage.JSONError{ + Message: msg, + } + report.ErrorMessage = msg } } if err := enc.Encode(report); err != nil { diff --git a/pkg/api/handlers/compat/images_push.go b/pkg/api/handlers/compat/images_push.go index bb82ef10d..f29808124 100644 --- a/pkg/api/handlers/compat/images_push.go +++ b/pkg/api/handlers/compat/images_push.go @@ -156,6 +156,7 @@ loop: // break out of for/select infinite loop Current: int64(e.Offset), Total: e.Artifact.Size, } + report.ProgressMessage = report.Progress.String() case types.ProgressEventSkipped: report.Status = "Layer already exists" case types.ProgressEventDone: -- cgit v1.2.3-54-g00ecf From aef8039d3755e1aa44b2b28c9ca8c4cc52d65251 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 21 Jul 2022 13:42:21 +0200 Subject: compat api: allow default bridge name for networks Docker uses "bridge" as default network name so some tools expect this to work with network list or inspect. To fix this we change "bridge" to the podman default ("podman") name. Fixes #14983 Signed-off-by: Paul Holzinger --- pkg/api/handlers/compat/networks.go | 33 ++++++++++++++++++++++----------- test/apiv2/35-networks.at | 12 ++++++++++++ 2 files changed, 34 insertions(+), 11 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/networks.go b/pkg/api/handlers/compat/networks.go index 65177218a..bb1fa9ac3 100644 --- a/pkg/api/handlers/compat/networks.go +++ b/pkg/api/handlers/compat/networks.go @@ -23,6 +23,13 @@ import ( "github.com/sirupsen/logrus" ) +func normalizeNetworkName(rt *libpod.Runtime, name string) (string, bool) { + if name == nettypes.BridgeNetworkDriver { + return rt.Network().DefaultNetworkName(), true + } + return name, false +} + func InspectNetwork(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) @@ -44,13 +51,13 @@ func InspectNetwork(w http.ResponseWriter, r *http.Request) { utils.Error(w, http.StatusBadRequest, define.ErrInvalidArg) return } - name := utils.GetName(r) + name, changed := normalizeNetworkName(runtime, utils.GetName(r)) net, err := runtime.Network().NetworkInspect(name) if err != nil { utils.NetworkNotFound(w, name, err) return } - report, err := convertLibpodNetworktoDockerNetwork(runtime, net) + report, err := convertLibpodNetworktoDockerNetwork(runtime, &net, changed) if err != nil { utils.InternalServerError(w, err) return @@ -58,7 +65,7 @@ func InspectNetwork(w http.ResponseWriter, r *http.Request) { utils.WriteResponse(w, http.StatusOK, report) } -func convertLibpodNetworktoDockerNetwork(runtime *libpod.Runtime, network nettypes.Network) (*types.NetworkResource, error) { +func convertLibpodNetworktoDockerNetwork(runtime *libpod.Runtime, network *nettypes.Network, changeDefaultName bool) (*types.NetworkResource, error) { cons, err := runtime.GetAllContainers() if err != nil { return nil, err @@ -107,11 +114,15 @@ func convertLibpodNetworktoDockerNetwork(runtime *libpod.Runtime, network nettyp Config: ipamConfigs, } + name := network.Name + if changeDefaultName && name == runtime.Network().DefaultNetworkName() { + name = nettypes.BridgeNetworkDriver + } report := types.NetworkResource{ - Name: network.Name, - ID: network.ID, - Driver: network.Driver, - // TODO add Created: , + Name: name, + ID: network.ID, + Driver: network.Driver, + Created: network.Created, Internal: network.Internal, EnableIPv6: network.IPv6Enabled, Labels: network.Labels, @@ -149,7 +160,7 @@ func ListNetworks(w http.ResponseWriter, r *http.Request) { } reports := make([]*types.NetworkResource, 0, len(nets)) for _, net := range nets { - report, err := convertLibpodNetworktoDockerNetwork(runtime, net) + report, err := convertLibpodNetworktoDockerNetwork(runtime, &net, true) if err != nil { utils.InternalServerError(w, err) return @@ -305,7 +316,7 @@ func RemoveNetwork(w http.ResponseWriter, r *http.Request) { Timeout: query.Timeout, } - name := utils.GetName(r) + name, _ := normalizeNetworkName(runtime, utils.GetName(r)) reports, err := ic.NetworkRm(r.Context(), []string{name}, options) if err != nil { utils.Error(w, http.StatusInternalServerError, err) @@ -340,7 +351,7 @@ func Connect(w http.ResponseWriter, r *http.Request) { netOpts := nettypes.PerNetworkOptions{} - name := utils.GetName(r) + name, _ := normalizeNetworkName(runtime, utils.GetName(r)) if netConnect.EndpointConfig != nil { if netConnect.EndpointConfig.Aliases != nil { netOpts.Aliases = netConnect.EndpointConfig.Aliases @@ -416,7 +427,7 @@ func Disconnect(w http.ResponseWriter, r *http.Request) { return } - name := utils.GetName(r) + name, _ := normalizeNetworkName(runtime, utils.GetName(r)) err := runtime.DisconnectContainerFromNetwork(netDisconnect.Container, name, netDisconnect.Force) if err != nil { if errors.Is(err, define.ErrNoSuchCtr) { diff --git a/test/apiv2/35-networks.at b/test/apiv2/35-networks.at index fcff26521..07ba45efb 100644 --- a/test/apiv2/35-networks.at +++ b/test/apiv2/35-networks.at @@ -84,12 +84,24 @@ t GET networks?filters='{"dangling":["true","0"]}' 500 \ t GET networks?filters='{"name":["doesnotexists"]}' 200 \ "[]" +# check default name in list endpoint +t GET networks 200 \ + .[].Name~.*bridge.* + # network inspect docker t GET networks/$network1_id 200 \ .Name=network1 \ .Id=$network1_id \ .Scope=local +# inspect default bridge network +t GET networks/bridge 200 \ + .Name=bridge + +# inspect default bridge network with real podman name should return real name +t GET networks/podman 200 \ + .Name=podman + # network create docker t POST networks/create Name=net3\ IPAM='{"Config":[]}' 201 # network delete docker -- cgit v1.2.3-54-g00ecf From 553a700966b6f5ae6dc092a7ad4eece9e2c4b32b Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 21 Jul 2022 13:57:11 +0200 Subject: compat api: always turn on network isolation for networks Fix some network option parsing logic to use constants. Always use the isolate option since this is what docker does. Remove the icc option, this is different from isolate and it is not implemented. Signed-off-by: Paul Holzinger --- pkg/api/handlers/compat/networks.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/networks.go b/pkg/api/handlers/compat/networks.go index bb1fa9ac3..29d1398cf 100644 --- a/pkg/api/handlers/compat/networks.go +++ b/pkg/api/handlers/compat/networks.go @@ -193,27 +193,22 @@ func CreateNetwork(w http.ResponseWriter, r *http.Request) { network.Options = make(map[string]string) - // TODO: we should consider making this constants in c/common/libnetwork/types + // dockers bridge networks are always isolated from each other + if network.Driver == nettypes.BridgeNetworkDriver { + network.Options[nettypes.IsolateOption] = "true" + } + for opt, optVal := range networkCreate.Options { switch opt { - case "mtu": + case nettypes.MTUOption: fallthrough case "com.docker.network.driver.mtu": - if network.Driver == nettypes.BridgeNetworkDriver { - network.Options["mtu"] = optVal - } - case "icc": - fallthrough - case "com.docker.network.bridge.enable_icc": - // TODO: needs to be implemented - if network.Driver == nettypes.BridgeNetworkDriver { - responseWarning = "com.docker.network.bridge.enable_icc is not currently implemented" - } + network.Options[nettypes.MTUOption] = optVal case "com.docker.network.bridge.name": if network.Driver == nettypes.BridgeNetworkDriver { network.NetworkInterface = optVal } - case "mode": + case nettypes.ModeOption: if network.Driver == nettypes.MacVLANNetworkDriver || network.Driver == nettypes.IPVLANNetworkDriver { network.Options[opt] = optVal } -- cgit v1.2.3-54-g00ecf From c4616510a2c7509be20120353e539804b161922d Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 22 Jul 2022 14:16:25 +0200 Subject: API: libpod/create use correct default umask Make sure containers created via API have the correct umask from containers.conf set. Fixes #15036 Signed-off-by: Paul Holzinger --- pkg/api/handlers/libpod/containers_create.go | 3 +++ test/apiv2/20-containers.at | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/api/handlers/libpod/containers_create.go b/pkg/api/handlers/libpod/containers_create.go index e4964d602..1307c267a 100644 --- a/pkg/api/handlers/libpod/containers_create.go +++ b/pkg/api/handlers/libpod/containers_create.go @@ -31,6 +31,9 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) { ContainerNetworkConfig: specgen.ContainerNetworkConfig{ UseImageHosts: conf.Containers.NoHosts, }, + ContainerSecurityConfig: specgen.ContainerSecurityConfig{ + Umask: conf.Containers.Umask, + }, } if err := json.NewDecoder(r.Body).Decode(&sg); err != nil { diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index 6ef4ef917..a8d9baef3 100644 --- a/test/apiv2/20-containers.at +++ b/test/apiv2/20-containers.at @@ -123,7 +123,8 @@ t GET libpod/containers/${cid}/json 200 \ .Id=$cid \ .State.Status~\\\(exited\\\|stopped\\\) \ .State.Running=false \ - .State.ExitCode=0 + .State.ExitCode=0 \ + .Config.Umask=0022 # regression check for #15036 t DELETE libpod/containers/$cid 200 .[0].Id=$cid CNAME=myfoo -- cgit v1.2.3-54-g00ecf From 64bc2ee97f2dbcada22cff4364416abb81a3272a Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Mon, 25 Jul 2022 08:07:43 -0600 Subject: Semiperiodoc cleanup of obsolete FIXMEs Some refer to issues that are closed. Remove them. Some are runc bugs that will never be fixed. Say so, and remove the FIXME. One (bps/iops) should probably be fixed. File an issue for it, and update comment to include the issue# so my find-obsolete-skips script can track it. And one (rootless mount with a "kernel bug?" comment) is still not fixed. Leave the skip, but add a comment documenting the symptom. Signed-off-by: Ed Santiago --- pkg/bindings/test/images_test.go | 2 -- test/buildah-bud/apply-podman-deltas | 4 ++-- test/e2e/checkpoint_test.go | 5 ----- test/e2e/containers_conf_test.go | 3 ++- test/e2e/run_test.go | 13 +++++-------- test/system/520-checkpoint.bats | 2 +- 6 files changed, 10 insertions(+), 19 deletions(-) (limited to 'pkg') diff --git a/pkg/bindings/test/images_test.go b/pkg/bindings/test/images_test.go index a005be6ac..8f76ce456 100644 --- a/pkg/bindings/test/images_test.go +++ b/pkg/bindings/test/images_test.go @@ -120,8 +120,6 @@ var _ = Describe("Podman images", func() { // deleting hence image cannot be deleted until the container is deleted. _, errs = images.Remove(bt.conn, []string{alpine.shortName}, nil) code, _ = bindings.CheckResponseCode(errs[0]) - // FIXME FIXME FIXME: #12441: another invalid error - // FIXME FIXME FIXME: this time msg="Image used by SHA: ..." Expect(code).To(BeNumerically("==", -1)) // Removing the image "alpine" where force = true diff --git a/test/buildah-bud/apply-podman-deltas b/test/buildah-bud/apply-podman-deltas index d6d539fa4..6578afc93 100755 --- a/test/buildah-bud/apply-podman-deltas +++ b/test/buildah-bud/apply-podman-deltas @@ -163,7 +163,7 @@ skip "does not work under podman" \ # which we could and perhaps should fix in the buildah repo via: # - ... ${TESTSDIR}/bud/layers-squash/Dockerfile.hardlinks # + ... -f Dockerfile.hardlinks ${TESTSDIR}/bud/layers-squash -skip "FIXME FIXME FIXME: argument-order incompatible with podman" \ +skip "argument-order incompatible with podman" \ "bud-squash-hardlinks" # Fails with "Error: context must be a directory: /path/to/Dockerfile" @@ -231,7 +231,7 @@ skip_if_remote "FIXME FIXME FIXME: does this test make sense in remote?" \ ############################################################################### # BEGIN tests which are skipped due to actual podman or podman-remote bugs. -skip_if_remote "Podman #12838: different error messages" \ +skip_if_remote "different error messages between podman & podman-remote" \ "bud with .dockerignore #2" # These two tests, new in 2022-01, invoke podman (create, export) in ways diff --git a/test/e2e/checkpoint_test.go b/test/e2e/checkpoint_test.go index 5ccafeb37..c09b0bf08 100644 --- a/test/e2e/checkpoint_test.go +++ b/test/e2e/checkpoint_test.go @@ -1452,11 +1452,6 @@ var _ = Describe("Podman checkpoint", func() { }) It("podman checkpoint and restore container with --file-locks", func() { - if !strings.Contains(podmanTest.OCIRuntime, "runc") { - // TODO: Enable test for crun when this feature has been released - // https://github.com/containers/crun/pull/783 - Skip("FIXME: requires crun >= 1.4") - } localRunString := getRunString([]string{"--name", "test_name", ALPINE, "flock", "test.lock", "sleep", "100"}) session := podmanTest.Podman(localRunString) session.WaitWithDefaultTimeout() diff --git a/test/e2e/containers_conf_test.go b/test/e2e/containers_conf_test.go index 819efa628..41e78ce0e 100644 --- a/test/e2e/containers_conf_test.go +++ b/test/e2e/containers_conf_test.go @@ -535,7 +535,8 @@ var _ = Describe("Verify podman containers.conf usage", func() { It("podman containers.conf cgroups=disabled", func() { if !strings.Contains(podmanTest.OCIRuntime, "crun") { - Skip("FIXME: requires crun") + // Assume this will never be fixed in runc + Skip("NoCgroups requires crun") } conffile := filepath.Join(podmanTest.TempDir, "container.conf") diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 6edb705a1..7e00326f4 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -714,8 +714,7 @@ USER bin`, BB) }) It("podman run device-read-bps test", func() { - SkipIfRootless("FIXME: requested cgroup controller `io` is not available") - SkipIfRootlessCgroupsV1("Setting device-read-bps not supported on cgroupv1 for rootless users") + SkipIfRootless("Setting device-read-bps not supported for rootless users") var session *PodmanSessionIntegration @@ -733,8 +732,7 @@ USER bin`, BB) }) It("podman run device-write-bps test", func() { - SkipIfRootless("FIXME: requested cgroup controller `io` is not available") - SkipIfRootlessCgroupsV1("Setting device-write-bps not supported on cgroupv1 for rootless users") + SkipIfRootless("Setting device-write-bps not supported for rootless users") var session *PodmanSessionIntegration @@ -751,8 +749,7 @@ USER bin`, BB) }) It("podman run device-read-iops test", func() { - SkipIfRootless("FIXME: requested cgroup controller `io` is not available") - SkipIfRootlessCgroupsV1("Setting device-read-iops not supported on cgroupv1 for rootless users") + SkipIfRootless("Setting device-read-iops not supported for rootless users") var session *PodmanSessionIntegration if CGROUPSV2 { @@ -769,8 +766,7 @@ USER bin`, BB) }) It("podman run device-write-iops test", func() { - SkipIfRootless("FIXME: requested cgroup controller `io` is not available") - SkipIfRootlessCgroupsV1("Setting device-write-iops not supported on cgroupv1 for rootless users") + SkipIfRootless("Setting device-write-iops not supported for rootless users") var session *PodmanSessionIntegration if CGROUPSV2 { @@ -1238,6 +1234,7 @@ USER mail`, BB) }) It("podman run --mount type=bind,bind-nonrecursive", func() { + // crun: mount `/` to `/host`: Invalid argument SkipIfRootless("FIXME: rootless users are not allowed to mount bind-nonrecursive (Could this be a Kernel bug?") session := podmanTest.Podman([]string{"run", "--mount", "type=bind,bind-nonrecursive,slave,src=/,target=/host", fedoraMinimal, "findmnt", "-nR", "/host"}) session.WaitWithDefaultTimeout() diff --git a/test/system/520-checkpoint.bats b/test/system/520-checkpoint.bats index 7f60f01b3..7c8fc143a 100644 --- a/test/system/520-checkpoint.bats +++ b/test/system/520-checkpoint.bats @@ -12,7 +12,7 @@ function setup() { # could run to see if it's fixed, but it's way too complicated. Since # integration tests also skip checkpoint tests on Ubuntu, do the same here. if is_ubuntu; then - skip "FIXME: checkpointing broken in Ubuntu 2004, 2104, 2110, ..." + skip "FIXME: checkpointing broken in Ubuntu 2004, 2104, 2110, 2204, ..." fi # None of these tests work rootless.... -- cgit v1.2.3-54-g00ecf From e6ebfbd1e0106d8ddcf19a1ec3f97052592f49ad Mon Sep 17 00:00:00 2001 From: Vladimir Kochnev Date: Mon, 25 Jul 2022 16:00:23 +0300 Subject: Set TLSVerify=true by default for API endpoints Option defaults in API must be the same as in CLI. ``` % podman image push --help % podman image pull --help % podman manifest push --help % podman image search --help ``` All of these CLI commands them have --tls-verify=true by default: ``` --tls-verify require HTTPS and verify certificates when accessing the registry (default true) ``` As for `podman image build`, it doesn't have any means to control `tlsVerify` parameter but it must be true by default. Signed-off-by: Vladimir Kochnev --- pkg/api/handlers/compat/images_build.go | 1 + pkg/api/handlers/compat/images_search.go | 1 + pkg/api/handlers/libpod/images_push.go | 1 + pkg/api/handlers/libpod/manifests.go | 1 + pkg/api/server/register_images.go | 8 ++++---- pkg/api/server/register_manifest.go | 10 +++++----- test/apiv2/12-imagesMore.at | 5 ++++- test/apiv2/15-manifest.at | 2 ++ 8 files changed, 19 insertions(+), 10 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index a9185c3d3..15cfc824e 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -140,6 +140,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { Registry: "docker.io", Rm: true, ShmSize: 64 * 1024 * 1024, + TLSVerify: true, } decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) diff --git a/pkg/api/handlers/compat/images_search.go b/pkg/api/handlers/compat/images_search.go index a6fd3a3a1..2fc95e84e 100644 --- a/pkg/api/handlers/compat/images_search.go +++ b/pkg/api/handlers/compat/images_search.go @@ -26,6 +26,7 @@ func SearchImages(w http.ResponseWriter, r *http.Request) { ListTags bool `json:"listTags"` }{ // This is where you can override the golang default value for one of fields + TLSVerify: true, } if err := decoder.Decode(&query, r.URL.Query()); err != nil { diff --git a/pkg/api/handlers/libpod/images_push.go b/pkg/api/handlers/libpod/images_push.go index f427dc01b..9ee651f5b 100644 --- a/pkg/api/handlers/libpod/images_push.go +++ b/pkg/api/handlers/libpod/images_push.go @@ -32,6 +32,7 @@ func PushImage(w http.ResponseWriter, r *http.Request) { TLSVerify bool `schema:"tlsVerify"` Quiet bool `schema:"quiet"` }{ + TLSVerify: true, // #14971: older versions did not sent *any* data, so we need // to be quiet by default to remain backwards compatible Quiet: true, diff --git a/pkg/api/handlers/libpod/manifests.go b/pkg/api/handlers/libpod/manifests.go index 3235a2972..43c7139d3 100644 --- a/pkg/api/handlers/libpod/manifests.go +++ b/pkg/api/handlers/libpod/manifests.go @@ -310,6 +310,7 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) { TLSVerify bool `schema:"tlsVerify"` }{ // Add defaults here once needed. + TLSVerify: true, } if err := decoder.Decode(&query, r.URL.Query()); err != nil { utils.Error(w, http.StatusBadRequest, diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index 11ab8cae0..1bfedd77e 100644 --- a/pkg/api/server/register_images.go +++ b/pkg/api/server/register_images.go @@ -192,8 +192,8 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // - in: query // name: tlsVerify // type: boolean - // default: false - // description: skip TLS verification for registries + // default: true + // description: Require HTTPS and verify signatures when contacting registries. // - in: query // name: listTags // type: boolean @@ -1120,8 +1120,8 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // - in: query // name: tlsVerify // type: boolean - // default: false - // description: skip TLS verification for registries + // default: true + // description: Require HTTPS and verify signatures when contacting registries. // - in: query // name: listTags // type: boolean diff --git a/pkg/api/server/register_manifest.go b/pkg/api/server/register_manifest.go index 4fadb92fd..19b507047 100644 --- a/pkg/api/server/register_manifest.go +++ b/pkg/api/server/register_manifest.go @@ -69,12 +69,12 @@ func (s *APIServer) registerManifestHandlers(r *mux.Router) error { // name: all // description: push all images // type: boolean - // default: false + // default: true // - in: query // name: tlsVerify // type: boolean - // default: false - // description: skip TLS verification for registries + // default: true + // description: Require HTTPS and verify signatures when contacting registries. // responses: // 200: // schema: @@ -195,8 +195,8 @@ func (s *APIServer) registerManifestHandlers(r *mux.Router) error { // - in: query // name: tlsVerify // type: boolean - // default: false - // description: skip TLS verification for registries + // default: true + // description: Require HTTPS and verify signatures when contacting registries. // - in: body // name: options // description: options for mutating a manifest diff --git a/test/apiv2/12-imagesMore.at b/test/apiv2/12-imagesMore.at index d4b09174f..498d67569 100644 --- a/test/apiv2/12-imagesMore.at +++ b/test/apiv2/12-imagesMore.at @@ -28,7 +28,10 @@ t GET libpod/images/$IMAGE/json 200 \ .RepoTags[1]=localhost:$REGISTRY_PORT/myrepo:mytag # Push to local registry... -t POST "images/localhost:$REGISTRY_PORT/myrepo/push?tlsVerify=false&tag=mytag" 200 +t POST "images/localhost:$REGISTRY_PORT/myrepo/push?tag=mytag" 200 \ + .error~".*x509: certificate signed by unknown authority" +t POST "images/localhost:$REGISTRY_PORT/myrepo/push?tlsVerify=false&tag=mytag" 200 \ + .error~null # ...and check output. We can't use our built-in checks because this output # is a sequence of JSON objects, i.e., individual ones, not in a JSON array. diff --git a/test/apiv2/15-manifest.at b/test/apiv2/15-manifest.at index 970bed5a8..6584ea8e4 100644 --- a/test/apiv2/15-manifest.at +++ b/test/apiv2/15-manifest.at @@ -31,6 +31,8 @@ t POST /v3.4.0/libpod/manifests/$id_abc/add images="[\"containers-storage:$id_ab t PUT /v4.0.0/libpod/manifests/$id_xyz operation='update' images="[\"containers-storage:$id_xyz_image\"]" 200 t POST "/v3.4.0/libpod/manifests/abc:latest/push?destination=localhost:$REGISTRY_PORT%2Fabc:latest&tlsVerify=false&all=true" 200 +t POST "/v4.0.0/libpod/manifests/xyz:latest/registry/localhost:$REGISTRY_PORT%2Fxyz:latest?all=true" 400 \ + .cause='x509: certificate signed by unknown authority' t POST "/v4.0.0/libpod/manifests/xyz:latest/registry/localhost:$REGISTRY_PORT%2Fxyz:latest?tlsVerify=false&all=true" 200 # /v3.x cannot delete a manifest list -- cgit v1.2.3-54-g00ecf From c3e0f8ebef13e107b179fb6cf867ca8902f3761a Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Mon, 25 Jul 2022 17:10:27 +0200 Subject: machine: Fix check which is always true Before making / mutable/immutable, podman-machine checks if the mount is being done in /home or /mnt. However the current check is always going to be true: ``` !strings.HasPrefix(mount.Target, "/home") || !strings.HasPrefix(mount.Target, "/mnt") ``` is false when mount.Target starts with "/home" and mount.Target starts with "/mnt", which cannot happen at the same time. The correct check is: ``` !strings.HasPrefix(mount.Target, "/home") && !strings.HasPrefix(mount.Target, "/mnt") ``` which can also be written as: ``` !(strings.HasPrefix(mount.Target, "/home") || strings.HasPrefix(mount.Target, "/mnt")) ``` The impact is not too bad, it results in extra 'chattr -i' calls which should be unneeded. [NO NEW TESTS NEEDED] Signed-off-by: Christophe Fergeau --- pkg/machine/qemu/machine.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pkg') diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 3b57455c4..7974c261e 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -670,11 +670,11 @@ func (v *MachineVM) Start(name string, _ machine.StartOptions) error { // because / is immutable, we have to monkey around with permissions // if we dont mount in /home or /mnt args := []string{"-q", "--"} - if !strings.HasPrefix(mount.Target, "/home") || !strings.HasPrefix(mount.Target, "/mnt") { + if !strings.HasPrefix(mount.Target, "/home") && !strings.HasPrefix(mount.Target, "/mnt") { args = append(args, "sudo", "chattr", "-i", "/", ";") } args = append(args, "sudo", "mkdir", "-p", mount.Target) - if !strings.HasPrefix(mount.Target, "/home") || !strings.HasPrefix(mount.Target, "/mnt") { + if !strings.HasPrefix(mount.Target, "/home") && !strings.HasPrefix(mount.Target, "/mnt") { args = append(args, ";", "sudo", "chattr", "+i", "/", ";") } err = v.SSH(name, machine.SSHOptions{Args: args}) -- cgit v1.2.3-54-g00ecf