From 0dbbb1cb3f6ed2983105620bc49191e3b0436f37 Mon Sep 17 00:00:00 2001 From: Toshiki Sonoda Date: Fri, 12 Aug 2022 09:22:53 +0900 Subject: Add restart --cidfile, --filter --cidfile : Read container ID from the specified file and restart the container. --filter : restart the filtered container. Signed-off-by: Toshiki Sonoda --- pkg/domain/entities/containers.go | 6 ++++-- pkg/domain/infra/abi/containers.go | 27 +++++++++++++++++++-------- pkg/domain/infra/tunnel/containers.go | 11 ++++++++--- 3 files changed, 31 insertions(+), 13 deletions(-) (limited to 'pkg') diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index 3ba507750..91ccdc2b2 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -119,6 +119,7 @@ type KillReport struct { } type RestartOptions struct { + Filters map[string][]string All bool Latest bool Running bool @@ -126,8 +127,9 @@ type RestartOptions struct { } type RestartReport struct { - Err error - Id string //nolint:revive,stylecheck + Err error + Id string //nolint:revive,stylecheck + RawInput string } type RmOptions struct { diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 5b5bc665e..08d845d70 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -309,31 +309,42 @@ func (ic *ContainerEngine) ContainerKill(ctx context.Context, namesOrIds []strin func (ic *ContainerEngine) ContainerRestart(ctx context.Context, namesOrIds []string, options entities.RestartOptions) ([]*entities.RestartReport, error) { var ( - ctrs []*libpod.Container - err error + ctrs []*libpod.Container + err error + rawInputs = []string{} ) if options.Running { ctrs, err = ic.Libpod.GetRunningContainers() + for _, candidate := range ctrs { + rawInputs = append(rawInputs, candidate.ID()) + } + if err != nil { return nil, err } } else { - ctrs, err = getContainersByContext(options.All, options.Latest, namesOrIds, ic.Libpod) + ctrs, rawInputs, err = getContainersAndInputByContext(options.All, options.Latest, namesOrIds, options.Filters, ic.Libpod) if err != nil { return nil, err } } - + idToRawInput := map[string]string{} + if len(rawInputs) == len(ctrs) { + for i := range ctrs { + idToRawInput[ctrs[i].ID()] = rawInputs[i] + } + } reports := make([]*entities.RestartReport, 0, len(ctrs)) - for _, con := range ctrs { - timeout := con.StopTimeout() + for _, c := range ctrs { + timeout := c.StopTimeout() if options.Timeout != nil { timeout = *options.Timeout } reports = append(reports, &entities.RestartReport{ - Id: con.ID(), - Err: con.RestartWithTimeout(ctx, timeout), + Id: c.ID(), + Err: c.RestartWithTimeout(ctx, timeout), + RawInput: idToRawInput[c.ID()], }) } return reports, nil diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index d49f029d5..046509140 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -183,17 +183,22 @@ func (ic *ContainerEngine) ContainerRestart(ctx context.Context, namesOrIds []st if to := opts.Timeout; to != nil { options.WithTimeout(int(*to)) } - ctrs, err := getContainersByContext(ic.ClientCtx, opts.All, false, namesOrIds) + ctrs, rawInputs, err := getContainersAndInputByContext(ic.ClientCtx, opts.All, false, namesOrIds, opts.Filters) if err != nil { return nil, err } + idToRawInput := map[string]string{} + for i := range ctrs { + idToRawInput[ctrs[i].ID] = rawInputs[i] + } for _, c := range ctrs { if opts.Running && c.State != define.ContainerStateRunning.String() { continue } reports = append(reports, &entities.RestartReport{ - Id: c.ID, - Err: containers.Restart(ic.ClientCtx, c.ID, options), + Id: c.ID, + Err: containers.Restart(ic.ClientCtx, c.ID, options), + RawInput: idToRawInput[c.ID], }) } return reports, nil -- cgit v1.2.3-54-g00ecf From 7e7a79b075f7d65657d95169f02c2c1c03198b93 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Tue, 16 Aug 2022 18:30:19 -0400 Subject: podman manifest create: accept --amend and --insecure flags Accept a --amend flag in `podman manifest create`, and treat `--insecure` as we would `--tls-verify=false` in `podman manifest`'s "add", "create", and "push" subcommands. Signed-off-by: Nalin Dahyabhai --- cmd/podman/manifest/add.go | 12 ++++++++- cmd/podman/manifest/create.go | 32 ++++++++++++++++++++++-- cmd/podman/manifest/push.go | 15 ++++++++--- docs/source/markdown/podman-manifest-create.1.md | 12 +++++++++ pkg/api/handlers/libpod/manifests.go | 3 ++- pkg/api/server/register_manifest.go | 4 +++ pkg/bindings/manifests/types.go | 3 ++- pkg/bindings/manifests/types_create_options.go | 15 +++++++++++ pkg/domain/entities/manifest.go | 5 ++++ pkg/domain/infra/abi/manifest.go | 10 +++++++- pkg/domain/infra/tunnel/manifest.go | 2 +- test/e2e/manifest_test.go | 8 ++++++ 12 files changed, 111 insertions(+), 10 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/manifest/add.go b/cmd/podman/manifest/add.go index 35583ffcb..09a1a9a36 100644 --- a/cmd/podman/manifest/add.go +++ b/cmd/podman/manifest/add.go @@ -2,6 +2,7 @@ package manifest import ( "context" + "errors" "fmt" "github.com/containers/common/pkg/auth" @@ -20,6 +21,7 @@ type manifestAddOptsWrapper struct { entities.ManifestAddOptions TLSVerifyCLI bool // CLI only + Insecure bool // CLI only CredentialsCLI string } @@ -77,6 +79,8 @@ func init() { flags.StringVar(&manifestAddOpts.OSVersion, osVersionFlagName, "", "override the OS `version` of the specified image") _ = addCmd.RegisterFlagCompletionFunc(osVersionFlagName, completion.AutocompleteNone) + flags.BoolVar(&manifestAddOpts.Insecure, "insecure", false, "neither require HTTPS nor verify certificates when accessing the registry") + _ = flags.MarkHidden("insecure") flags.BoolVar(&manifestAddOpts.TLSVerifyCLI, "tls-verify", true, "require HTTPS and verify certificates when accessing the registry") variantFlagName := "variant" @@ -89,7 +93,7 @@ func init() { } func add(cmd *cobra.Command, args []string) error { - if err := auth.CheckAuthFile(manifestPushOpts.Authfile); err != nil { + if err := auth.CheckAuthFile(manifestAddOpts.Authfile); err != nil { return err } @@ -109,6 +113,12 @@ func add(cmd *cobra.Command, args []string) error { if cmd.Flags().Changed("tls-verify") { manifestAddOpts.SkipTLSVerify = types.NewOptionalBool(!manifestAddOpts.TLSVerifyCLI) } + if cmd.Flags().Changed("insecure") { + if manifestAddOpts.SkipTLSVerify != types.OptionalBoolUndefined { + return errors.New("--insecure may not be used with --tls-verify") + } + manifestAddOpts.SkipTLSVerify = types.NewOptionalBool(manifestAddOpts.Insecure) + } listID, err := registry.ImageEngine().ManifestAdd(context.Background(), args[0], args[1:], manifestAddOpts.ManifestAddOptions) if err != nil { diff --git a/cmd/podman/manifest/create.go b/cmd/podman/manifest/create.go index 435b4a57c..0a0ea1d88 100644 --- a/cmd/podman/manifest/create.go +++ b/cmd/podman/manifest/create.go @@ -1,16 +1,26 @@ package manifest import ( + "errors" "fmt" + "github.com/containers/image/v5/types" "github.com/containers/podman/v4/cmd/podman/common" "github.com/containers/podman/v4/cmd/podman/registry" "github.com/containers/podman/v4/pkg/domain/entities" "github.com/spf13/cobra" ) +// manifestCreateOptsWrapper wraps entities.ManifestCreateOptions and prevents leaking +// CLI-only fields into the API types. +type manifestCreateOptsWrapper struct { + entities.ManifestCreateOptions + + TLSVerifyCLI, Insecure bool // CLI only +} + var ( - manifestCreateOpts = entities.ManifestCreateOptions{} + manifestCreateOpts = manifestCreateOptsWrapper{} createCmd = &cobra.Command{ Use: "create [options] LIST [IMAGE...]", Short: "Create manifest list or image index", @@ -32,10 +42,28 @@ func init() { }) flags := createCmd.Flags() flags.BoolVar(&manifestCreateOpts.All, "all", false, "add all of the lists' images if the images to add are lists") + flags.BoolVar(&manifestCreateOpts.Amend, "amend", false, "modify an existing list if one with the desired name already exists") + flags.BoolVar(&manifestCreateOpts.Insecure, "insecure", false, "neither require HTTPS nor verify certificates when accessing the registry") + _ = flags.MarkHidden("insecure") + flags.BoolVar(&manifestCreateOpts.TLSVerifyCLI, "tls-verify", true, "require HTTPS and verify certificates when accessing the registry") } func create(cmd *cobra.Command, args []string) error { - imageID, err := registry.ImageEngine().ManifestCreate(registry.Context(), args[0], args[1:], manifestCreateOpts) + // TLS verification in c/image is controlled via a `types.OptionalBool` + // which allows for distinguishing among set-true, set-false, unspecified + // which is important to implement a sane way of dealing with defaults of + // boolean CLI flags. + if cmd.Flags().Changed("tls-verify") { + manifestCreateOpts.SkipTLSVerify = types.NewOptionalBool(!manifestCreateOpts.TLSVerifyCLI) + } + if cmd.Flags().Changed("insecure") { + if manifestCreateOpts.SkipTLSVerify != types.OptionalBoolUndefined { + return errors.New("--insecure may not be used with --tls-verify") + } + manifestCreateOpts.SkipTLSVerify = types.NewOptionalBool(manifestCreateOpts.Insecure) + } + + imageID, err := registry.ImageEngine().ManifestCreate(registry.Context(), args[0], args[1:], manifestCreateOpts.ManifestCreateOptions) if err != nil { return err } diff --git a/cmd/podman/manifest/push.go b/cmd/podman/manifest/push.go index 756ed2a74..fd67769b8 100644 --- a/cmd/podman/manifest/push.go +++ b/cmd/podman/manifest/push.go @@ -1,6 +1,7 @@ package manifest import ( + "errors" "fmt" "io/ioutil" @@ -20,9 +21,9 @@ import ( type manifestPushOptsWrapper struct { entities.ImagePushOptions - TLSVerifyCLI bool // CLI only - CredentialsCLI string - SignPassphraseFileCLI string + TLSVerifyCLI, Insecure bool // CLI only + CredentialsCLI string + SignPassphraseFileCLI string } var ( @@ -82,6 +83,8 @@ func init() { _ = pushCmd.RegisterFlagCompletionFunc(signPassphraseFileFlagName, completion.AutocompleteDefault) flags.BoolVar(&manifestPushOpts.TLSVerifyCLI, "tls-verify", true, "require HTTPS and verify certificates when accessing the registry") + flags.BoolVar(&manifestPushOpts.Insecure, "insecure", false, "neither require HTTPS nor verify certificates when accessing the registry") + _ = flags.MarkHidden("insecure") flags.BoolVarP(&manifestPushOpts.Quiet, "quiet", "q", false, "don't output progress information when pushing lists") flags.SetNormalizeFunc(utils.AliasFlags) @@ -130,6 +133,12 @@ func push(cmd *cobra.Command, args []string) error { if cmd.Flags().Changed("tls-verify") { manifestPushOpts.SkipTLSVerify = types.NewOptionalBool(!manifestPushOpts.TLSVerifyCLI) } + if cmd.Flags().Changed("insecure") { + if manifestPushOpts.SkipTLSVerify != types.OptionalBoolUndefined { + return errors.New("--insecure may not be used with --tls-verify") + } + manifestPushOpts.SkipTLSVerify = types.NewOptionalBool(manifestPushOpts.Insecure) + } digest, err := registry.ImageEngine().ManifestPush(registry.Context(), args[0], args[1], manifestPushOpts.ImagePushOptions) if err != nil { return err diff --git a/docs/source/markdown/podman-manifest-create.1.md b/docs/source/markdown/podman-manifest-create.1.md index 77a4b9db6..f2aac6069 100644 --- a/docs/source/markdown/podman-manifest-create.1.md +++ b/docs/source/markdown/podman-manifest-create.1.md @@ -22,11 +22,23 @@ If any of the images which should be added to the new list or index are themselves lists or indexes, add all of their contents. By default, only one image from such a list will be added to the newly-created list or index. +#### **--amend** + +If a manifest list named *listnameorindexname* already exists, modify the +preexisting list instead of exiting with an error. The contents of +*listnameorindexname* are not modified if no *imagename*s are given. + +#### **--tls-verify** + +Require HTTPS and verify certificates when talking to container registries. (defaults to true) + ## EXAMPLES ``` podman manifest create mylist:v1.11 9cfd24048d5fc80903f088f1531a21bff01172abe66effa8941a4c2308dc745f +podman manifest create --amend mylist:v1.11 +9cfd24048d5fc80903f088f1531a21bff01172abe66effa8941a4c2308dc745f ``` ``` diff --git a/pkg/api/handlers/libpod/manifests.go b/pkg/api/handlers/libpod/manifests.go index b0c93f3b9..fa83bbfe1 100644 --- a/pkg/api/handlers/libpod/manifests.go +++ b/pkg/api/handlers/libpod/manifests.go @@ -36,6 +36,7 @@ func ManifestCreate(w http.ResponseWriter, r *http.Request) { Name string `schema:"name"` Images []string `schema:"images"` All bool `schema:"all"` + Amend bool `schema:"amend"` }{ // Add defaults here once needed. } @@ -70,7 +71,7 @@ func ManifestCreate(w http.ResponseWriter, r *http.Request) { imageEngine := abi.ImageEngine{Libpod: runtime} - createOptions := entities.ManifestCreateOptions{All: query.All} + createOptions := entities.ManifestCreateOptions{All: query.All, Amend: query.Amend} manID, err := imageEngine.ManifestCreate(r.Context(), query.Name, query.Images, createOptions) if err != nil { utils.InternalServerError(w, err) diff --git a/pkg/api/server/register_manifest.go b/pkg/api/server/register_manifest.go index c22479cf9..7a55eaefe 100644 --- a/pkg/api/server/register_manifest.go +++ b/pkg/api/server/register_manifest.go @@ -117,6 +117,10 @@ func (s *APIServer) registerManifestHandlers(r *mux.Router) error { // name: all // type: boolean // description: add all contents if given list + // - in: query + // name: amend + // type: boolean + // description: modify an existing list if one with the desired name already exists // - in: body // name: options // description: options for new manifest diff --git a/pkg/bindings/manifests/types.go b/pkg/bindings/manifests/types.go index e23ef798d..5f2557fe1 100644 --- a/pkg/bindings/manifests/types.go +++ b/pkg/bindings/manifests/types.go @@ -8,7 +8,8 @@ type InspectOptions struct { //go:generate go run ../generator/generator.go CreateOptions // CreateOptions are optional options for creating manifests type CreateOptions struct { - All *bool + All *bool + Amend *bool } //go:generate go run ../generator/generator.go ExistsOptions diff --git a/pkg/bindings/manifests/types_create_options.go b/pkg/bindings/manifests/types_create_options.go index 960332a82..09942c00a 100644 --- a/pkg/bindings/manifests/types_create_options.go +++ b/pkg/bindings/manifests/types_create_options.go @@ -31,3 +31,18 @@ func (o *CreateOptions) GetAll() bool { } return *o.All } + +// WithAmend set field Amend to given value +func (o *CreateOptions) WithAmend(value bool) *CreateOptions { + o.Amend = &value + return o +} + +// GetAmend returns value of field Amend +func (o *CreateOptions) GetAmend() bool { + if o.Amend == nil { + var z bool + return z + } + return *o.Amend +} diff --git a/pkg/domain/entities/manifest.go b/pkg/domain/entities/manifest.go index 126b76c62..f17079271 100644 --- a/pkg/domain/entities/manifest.go +++ b/pkg/domain/entities/manifest.go @@ -4,7 +4,12 @@ import "github.com/containers/image/v5/types" // ManifestCreateOptions provides model for creating manifest type ManifestCreateOptions struct { + // True when adding lists to include all images All bool `schema:"all"` + // Amend an extant list if there's already one with the desired name + Amend bool `schema:"amend"` + // Should TLS registry certificate be verified? + SkipTLSVerify types.OptionalBool `json:"-" schema:"-"` } // ManifestAddOptions provides model for adding digests to manifest list diff --git a/pkg/domain/infra/abi/manifest.go b/pkg/domain/infra/abi/manifest.go index e0c11267e..7e8c86526 100644 --- a/pkg/domain/infra/abi/manifest.go +++ b/pkg/domain/infra/abi/manifest.go @@ -32,7 +32,15 @@ func (ir *ImageEngine) ManifestCreate(ctx context.Context, name string, images [ manifestList, err := ir.Libpod.LibimageRuntime().CreateManifestList(name) if err != nil { - return "", err + if errors.Is(err, storage.ErrDuplicateName) && opts.Amend { + amendList, amendErr := ir.Libpod.LibimageRuntime().LookupManifestList(name) + if amendErr != nil { + return "", err + } + manifestList = amendList + } else { + return "", err + } } addOptions := &libimage.ManifestListAddOptions{All: opts.All} diff --git a/pkg/domain/infra/tunnel/manifest.go b/pkg/domain/infra/tunnel/manifest.go index 2a514861d..2e6134051 100644 --- a/pkg/domain/infra/tunnel/manifest.go +++ b/pkg/domain/infra/tunnel/manifest.go @@ -15,7 +15,7 @@ import ( // ManifestCreate implements manifest create via ImageEngine func (ir *ImageEngine) ManifestCreate(ctx context.Context, name string, images []string, opts entities.ManifestCreateOptions) (string, error) { - options := new(manifests.CreateOptions).WithAll(opts.All) + options := new(manifests.CreateOptions).WithAll(opts.All).WithAmend(opts.Amend) imageID, err := manifests.Create(ir.ClientCtx, name, images, options) if err != nil { return imageID, fmt.Errorf("error creating manifest: %w", err) diff --git a/test/e2e/manifest_test.go b/test/e2e/manifest_test.go index ee954a1a4..145a016ea 100644 --- a/test/e2e/manifest_test.go +++ b/test/e2e/manifest_test.go @@ -49,6 +49,14 @@ var _ = Describe("Podman manifest", func() { session := podmanTest.Podman([]string{"manifest", "create", "foo"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) + + session = podmanTest.Podman([]string{"manifest", "create", "foo"}) + session.WaitWithDefaultTimeout() + Expect(session).To(ExitWithError()) + + session = podmanTest.Podman([]string{"manifest", "create", "--amend", "foo"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) }) It("create w/ image", func() { -- cgit v1.2.3-54-g00ecf From 2a6daa1e313c18814192548627058a85dc97f158 Mon Sep 17 00:00:00 2001 From: Lokesh Mandvekar Date: Wed, 17 Aug 2022 09:11:06 -0400 Subject: Cirrus: add podman_machine_aarch64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run machine tests on every PR as label-driven machine test triggering is currently hard to predict and debug. Co-authored-by: Ed Santiago Co-authored-by: Miloslav Trmač Signed-off-by: Lokesh Mandvekar --- .cirrus.yml | 35 +++++++++++++++++++++++++++-------- contrib/cirrus/cirrus_yaml_test.py | 2 +- pkg/machine/e2e/basic_test.go | 8 ++++++++ test/system/200-pod.bats | 2 +- 4 files changed, 37 insertions(+), 10 deletions(-) (limited to 'pkg') diff --git a/.cirrus.yml b/.cirrus.yml index f94ee2f3b..e3ddc4933 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -681,11 +681,6 @@ podman_machine_task: # Required_pr_labels does not apply to non-PRs. # Do not run on tags, branches, [CI:BUILD], or [CI:DOCS]. only_if: *not_tag_branch_build_docs - # This task costs about $4 per attempt to execute. - # Only run it if a magic PR label is present. - # DO NOT ADD THIS TASK AS DEPENDENCY FOR `success_task` - # it will cause an infinate-block / never completing build. - required_pr_labels: test_podman_machine depends_on: - build - local_integration_test @@ -708,6 +703,31 @@ podman_machine_task: always: *int_logs_artifacts +podman_machine_aarch64_task: + name: *std_name_fmt + alias: podman_machine_aarch64 + only_if: *not_tag_branch_build_docs + depends_on: + - build_aarch64 + - validate_aarch64 + - local_integration_test + - remote_integration_test + - container_integration_test + - rootless_integration_test + ec2_instance: + <<: *standard_build_ec2_aarch64 + env: + TEST_FLAVOR: "machine" + EC2_INST_TYPE: c6g.metal + PRIV_NAME: "rootless" # intended use-case + DISTRO_NV: "${FEDORA_AARCH64_NAME}" + VM_IMAGE_NAME: "${FEDORA_AARCH64_AMI}" + clone_script: *get_gosrc_aarch64 + setup_script: *setup + main_script: *main + always: *int_logs_artifacts + + # Always run subsequent to integration tests. While parallelism is lost # with runtime, debugging system-test failures can be more challenging # for some golang developers. Otherwise the following tasks run across @@ -1003,9 +1023,8 @@ success_task: - remote_integration_test - container_integration_test - rootless_integration_test - # Label triggered task. If made automatic, remove line below - # AND bypass in contrib/cirrus/cirrus_yaml_test.py for this name. - # - podman_machine + - podman_machine + - podman_machine_aarch64 - local_system_test - local_system_test_aarch64 - remote_system_test diff --git a/contrib/cirrus/cirrus_yaml_test.py b/contrib/cirrus/cirrus_yaml_test.py index 3968b8b1b..a7fff8d3f 100755 --- a/contrib/cirrus/cirrus_yaml_test.py +++ b/contrib/cirrus/cirrus_yaml_test.py @@ -26,7 +26,7 @@ class TestCaseBase(unittest.TestCase): class TestDependsOn(TestCaseBase): ALL_TASK_NAMES = None - SUCCESS_DEPS_EXCLUDE = set(['success', 'artifacts', 'podman_machine', + SUCCESS_DEPS_EXCLUDE = set(['success', 'artifacts', 'test_image_build', 'release', 'release_test']) def setUp(self): diff --git a/pkg/machine/e2e/basic_test.go b/pkg/machine/e2e/basic_test.go index da0310485..fa1728770 100644 --- a/pkg/machine/e2e/basic_test.go +++ b/pkg/machine/e2e/basic_test.go @@ -1,6 +1,8 @@ package e2e_test import ( + "os" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gexec" @@ -20,6 +22,12 @@ var _ = Describe("run basic podman commands", func() { }) It("Basic ops", func() { + // golangci-lint has trouble with actually skipping tests marked Skip + // so skip it on cirrus envs and where CIRRUS_CI isn't set. + if os.Getenv("CIRRUS_CI") != "false" { + Skip("FIXME: #15347 - ssh know hosts broken - fails on PR runs and on x86_64") + } + name := randomString() i := new(initMachine) session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath).withNow()).run() diff --git a/test/system/200-pod.bats b/test/system/200-pod.bats index b1b9ee5e1..b9063ad1b 100644 --- a/test/system/200-pod.bats +++ b/test/system/200-pod.bats @@ -478,7 +478,7 @@ spec: } @test "pod resource limits" { - # FIXME: #15074 - possible flake on aarch64 + skip_if_aarch64 "FIXME: #15074 - flakes on aarch64 non-remote" skip_if_remote "resource limits only implemented on non-remote" skip_if_rootless "resource limits only work with root" skip_if_cgroupsv1 "resource limits only meaningful on cgroups V2" -- cgit v1.2.3-54-g00ecf From aa197a65ff245f4d244968fd6010537a76a42e10 Mon Sep 17 00:00:00 2001 From: Josh Patterson Date: Wed, 17 Aug 2022 14:39:32 -0400 Subject: sort hc.Binds returned from compat api Signed-off-by: Josh Patterson --- pkg/api/handlers/compat/containers.go | 1 + 1 file changed, 1 insertion(+) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/containers.go b/pkg/api/handlers/compat/containers.go index ae063dc9f..0b82c48f6 100644 --- a/pkg/api/handlers/compat/containers.go +++ b/pkg/api/handlers/compat/containers.go @@ -467,6 +467,7 @@ func LibpodToContainerJSON(l *libpod.Container, sz bool) (*types.ContainerJSON, if err := json.Unmarshal(h, &hc); err != nil { return nil, err } + sort.Strings(hc.Binds) // k8s-file == json-file if hc.LogConfig.Type == define.KubernetesLogging { -- cgit v1.2.3-54-g00ecf From b9fb60c68acb57e56edae0b35dd408f454d32be5 Mon Sep 17 00:00:00 2001 From: Vladimir Kochnev Date: Wed, 17 Aug 2022 03:04:55 +0300 Subject: Simplify ImagesPull for when Quiet flag is on Refactor ImagesPull the same way the ImagesPush and ManifestPush are done. [NO NEW TESTS NEEDED] Signed-off-by: Vladimir Kochnev --- pkg/api/handlers/libpod/images_pull.go | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/libpod/images_pull.go b/pkg/api/handlers/libpod/images_pull.go index 7e24ae5ac..57b2e3a78 100644 --- a/pkg/api/handlers/libpod/images_pull.go +++ b/pkg/api/handlers/libpod/images_pull.go @@ -82,17 +82,32 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) { pullOptions.IdentityToken = authConf.IdentityToken } - writer := channel.NewWriter(make(chan []byte)) - defer writer.Close() - - pullOptions.Writer = writer - pullPolicy, err := config.ParsePullPolicy(query.PullPolicy) if err != nil { utils.Error(w, http.StatusBadRequest, err) return } + // Let's keep thing simple when running in quiet mode and pull directly. + if query.Quiet { + images, err := runtime.LibimageRuntime().Pull(r.Context(), query.Reference, pullPolicy, pullOptions) + var report entities.ImagePullReport + if err != nil { + report.Error = err.Error() + } + for _, image := range images { + report.Images = append(report.Images, image.ID()) + // Pull last ID from list and publish in 'id' stanza. This maintains previous API contract + report.ID = image.ID() + } + utils.WriteResponse(w, http.StatusOK, report) + return + } + + writer := channel.NewWriter(make(chan []byte)) + defer writer.Close() + pullOptions.Writer = writer + var pulledImages []*libimage.Image var pullError error runCtx, cancel := context.WithCancel(r.Context()) @@ -118,10 +133,8 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) { select { case s := <-writer.Chan(): report.Stream = string(s) - if !query.Quiet { - if err := enc.Encode(report); err != nil { - logrus.Warnf("Failed to encode json: %v", err) - } + if err := enc.Encode(report); err != nil { + logrus.Warnf("Failed to encode json: %v", err) } flush() case <-runCtx.Done(): -- cgit v1.2.3-54-g00ecf From e48681e600e55718a9699006ac940738fa08f896 Mon Sep 17 00:00:00 2001 From: Vladimir Kochnev Date: Wed, 17 Aug 2022 03:07:01 +0300 Subject: Use request Context() in API handlers Request object has its own context which must be used during a request lifetime instead of just context.Background() [NO NEW TESTS NEEDED] Signed-off-by: Vladimir Kochnev --- pkg/api/handlers/compat/auth.go | 3 +-- pkg/api/handlers/compat/images_build.go | 2 +- pkg/api/handlers/libpod/containers_create.go | 5 ++--- pkg/api/handlers/libpod/images_push.go | 2 +- pkg/api/handlers/libpod/manifests.go | 4 ++-- pkg/api/handlers/types.go | 2 +- 6 files changed, 8 insertions(+), 10 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/auth.go b/pkg/api/handlers/compat/auth.go index 37d2b784d..ee478b9e3 100644 --- a/pkg/api/handlers/compat/auth.go +++ b/pkg/api/handlers/compat/auth.go @@ -1,7 +1,6 @@ package compat import ( - "context" "encoding/json" "errors" "fmt" @@ -44,7 +43,7 @@ func Auth(w http.ResponseWriter, r *http.Request) { fmt.Println("Authenticating with existing credentials...") registry := stripAddressOfScheme(authConfig.ServerAddress) - if err := DockerClient.CheckAuth(context.Background(), sysCtx, authConfig.Username, authConfig.Password, registry); err == nil { + if err := DockerClient.CheckAuth(r.Context(), sysCtx, authConfig.Username, authConfig.Password, registry); err == nil { utils.WriteResponse(w, http.StatusOK, entities.AuthReport{ IdentityToken: "", Status: "Login Succeeded", diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index a00f0b089..020991cc7 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -694,7 +694,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { success bool ) - runCtx, cancel := context.WithCancel(context.Background()) + runCtx, cancel := context.WithCancel(r.Context()) go func() { defer cancel() imageID, _, err = runtime.Build(r.Context(), buildOptions, containerFiles...) diff --git a/pkg/api/handlers/libpod/containers_create.go b/pkg/api/handlers/libpod/containers_create.go index 1307c267a..429f45f91 100644 --- a/pkg/api/handlers/libpod/containers_create.go +++ b/pkg/api/handlers/libpod/containers_create.go @@ -1,7 +1,6 @@ package libpod import ( - "context" "encoding/json" "fmt" "net/http" @@ -63,12 +62,12 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) { utils.InternalServerError(w, err) return } - rtSpec, spec, opts, err := generate.MakeContainer(context.Background(), runtime, &sg, false, nil) + rtSpec, spec, opts, err := generate.MakeContainer(r.Context(), runtime, &sg, false, nil) if err != nil { utils.InternalServerError(w, err) return } - ctr, err := generate.ExecuteCreate(context.Background(), runtime, rtSpec, spec, false, opts...) + ctr, err := generate.ExecuteCreate(r.Context(), runtime, rtSpec, spec, false, opts...) if err != nil { utils.InternalServerError(w, err) return diff --git a/pkg/api/handlers/libpod/images_push.go b/pkg/api/handlers/libpod/images_push.go index e931fd2f9..be6f5b131 100644 --- a/pkg/api/handlers/libpod/images_push.go +++ b/pkg/api/handlers/libpod/images_push.go @@ -90,7 +90,7 @@ func PushImage(w http.ResponseWriter, r *http.Request) { // 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 { + if err := imageEngine.Push(r.Context(), source, destination, options); err != nil { utils.Error(w, http.StatusBadRequest, fmt.Errorf("error pushing image %q: %w", destination, err)) return } diff --git a/pkg/api/handlers/libpod/manifests.go b/pkg/api/handlers/libpod/manifests.go index fa83bbfe1..8391def5c 100644 --- a/pkg/api/handlers/libpod/manifests.go +++ b/pkg/api/handlers/libpod/manifests.go @@ -293,7 +293,7 @@ func ManifestPushV3(w http.ResponseWriter, r *http.Request) { options.SkipTLSVerify = types.NewOptionalBool(!query.TLSVerify) } imageEngine := abi.ImageEngine{Libpod: runtime} - digest, err := imageEngine.ManifestPush(context.Background(), source, query.Destination, options) + digest, err := imageEngine.ManifestPush(r.Context(), source, query.Destination, options) if err != nil { utils.Error(w, http.StatusBadRequest, fmt.Errorf("error pushing image %q: %w", query.Destination, err)) return @@ -367,7 +367,7 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) { // Let's keep thing simple when running in quiet mode and push directly. if query.Quiet { - digest, err := imageEngine.ManifestPush(context.Background(), source, destination, options) + digest, err := imageEngine.ManifestPush(r.Context(), source, destination, options) if err != nil { utils.Error(w, http.StatusBadRequest, fmt.Errorf("error pushing image %q: %w", destination, err)) return diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go index b533e131c..6d245d950 100644 --- a/pkg/api/handlers/types.go +++ b/pkg/api/handlers/types.go @@ -162,7 +162,7 @@ type ExecStartConfig struct { func ImageDataToImageInspect(ctx context.Context, l *libimage.Image) (*ImageInspect, error) { options := &libimage.InspectOptions{WithParent: true, WithSize: true} - info, err := l.Inspect(context.Background(), options) + info, err := l.Inspect(ctx, options) if err != nil { return nil, err } -- cgit v1.2.3-54-g00ecf From 3bf52aa338b33de719e087e15402081568453284 Mon Sep 17 00:00:00 2001 From: Vladimir Kochnev Date: Fri, 19 Aug 2022 00:41:22 +0300 Subject: Add ProgressWriter to PullOptions Signed-off-by: Vladimir Kochnev --- cmd/podman/images/pull.go | 5 +++++ pkg/bindings/images/pull.go | 13 ++++++++----- pkg/bindings/images/push.go | 9 +++++---- pkg/bindings/images/types.go | 2 ++ pkg/bindings/images/types_pull_options.go | 16 ++++++++++++++++ pkg/bindings/manifests/manifests.go | 6 ++++-- pkg/bindings/test/images_test.go | 24 ++++++++++++++++++++++-- pkg/bindings/test/manifests_test.go | 23 ++++++++++++++++++++--- pkg/domain/entities/images.go | 2 ++ pkg/domain/infra/abi/images.go | 3 ++- pkg/domain/infra/tunnel/images.go | 1 + test/e2e/manifest_test.go | 27 +++++++++++++++++++++++++++ test/e2e/pull_test.go | 14 ++++++++++++++ 13 files changed, 128 insertions(+), 17 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/images/pull.go b/cmd/podman/images/pull.go index 8211ceba5..fe9d1e9b6 100644 --- a/cmd/podman/images/pull.go +++ b/cmd/podman/images/pull.go @@ -155,6 +155,11 @@ func imagePull(cmd *cobra.Command, args []string) error { pullOptions.Username = creds.Username pullOptions.Password = creds.Password } + + if !pullOptions.Quiet { + pullOptions.Writer = os.Stderr + } + // Let's do all the remaining Yoga in the API to prevent us from // scattering logic across (too) many parts of the code. var errs utils.OutputErrors diff --git a/pkg/bindings/images/pull.go b/pkg/bindings/images/pull.go index 1a4aa3038..109981c63 100644 --- a/pkg/bindings/images/pull.go +++ b/pkg/bindings/images/pull.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "net/http" "os" "strconv" @@ -57,10 +56,14 @@ func Pull(ctx context.Context, rawImage string, options *PullOptions) ([]string, return nil, response.Process(err) } - // Historically pull writes status to stderr - stderr := io.Writer(os.Stderr) + var writer io.Writer if options.GetQuiet() { - stderr = ioutil.Discard + writer = io.Discard + } else if progressWriter := options.GetProgressWriter(); progressWriter != nil { + writer = progressWriter + } else { + // Historically push writes status to stderr + writer = os.Stderr } dec := json.NewDecoder(response.Body) @@ -84,7 +87,7 @@ func Pull(ctx context.Context, rawImage string, options *PullOptions) ([]string, switch { case report.Stream != "": - fmt.Fprint(stderr, report.Stream) + fmt.Fprint(writer, report.Stream) case report.Error != "": pullErrors = append(pullErrors, errors.New(report.Error)) case len(report.Images) > 0: diff --git a/pkg/bindings/images/push.go b/pkg/bindings/images/push.go index 5069dd780..f1e059f8c 100644 --- a/pkg/bindings/images/push.go +++ b/pkg/bindings/images/push.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "net/http" "os" "strconv" @@ -58,12 +57,14 @@ func Push(ctx context.Context, source string, destination string, options *PushO return response.Process(err) } - // Historically push writes status to stderr - writer := io.Writer(os.Stderr) + var writer io.Writer if options.GetQuiet() { - writer = ioutil.Discard + writer = io.Discard } else if progressWriter := options.GetProgressWriter(); progressWriter != nil { writer = progressWriter + } else { + // Historically push writes status to stderr + writer = os.Stderr } dec := json.NewDecoder(response.Body) diff --git a/pkg/bindings/images/types.go b/pkg/bindings/images/types.go index 7b28c499e..3ecfb9e09 100644 --- a/pkg/bindings/images/types.go +++ b/pkg/bindings/images/types.go @@ -182,6 +182,8 @@ type PullOptions struct { Policy *string // Password for authenticating against the registry. Password *string + // ProgressWriter is a writer where pull progress are sent. + ProgressWriter *io.Writer // Quiet can be specified to suppress pull progress when pulling. Ignored // for remote calls. Quiet *bool diff --git a/pkg/bindings/images/types_pull_options.go b/pkg/bindings/images/types_pull_options.go index 4cd525185..c1a88fd9e 100644 --- a/pkg/bindings/images/types_pull_options.go +++ b/pkg/bindings/images/types_pull_options.go @@ -2,6 +2,7 @@ package images import ( + "io" "net/url" "github.com/containers/podman/v4/pkg/bindings/internal/util" @@ -107,6 +108,21 @@ func (o *PullOptions) GetPassword() string { return *o.Password } +// WithProgressWriter set field ProgressWriter to given value +func (o *PullOptions) WithProgressWriter(value io.Writer) *PullOptions { + o.ProgressWriter = &value + return o +} + +// GetProgressWriter returns value of field ProgressWriter +func (o *PullOptions) GetProgressWriter() io.Writer { + if o.ProgressWriter == nil { + var z io.Writer + return z + } + return *o.ProgressWriter +} + // WithQuiet set field Quiet to given value func (o *PullOptions) WithQuiet(value bool) *PullOptions { o.Quiet = &value diff --git a/pkg/bindings/manifests/manifests.go b/pkg/bindings/manifests/manifests.go index 49e4089f5..0163d21a0 100644 --- a/pkg/bindings/manifests/manifests.go +++ b/pkg/bindings/manifests/manifests.go @@ -182,12 +182,14 @@ func Push(ctx context.Context, name, destination string, options *images.PushOpt return "", response.Process(err) } - // Historically push writes status to stderr - writer := io.Writer(os.Stderr) + var writer io.Writer if options.GetQuiet() { writer = io.Discard } else if progressWriter := options.GetProgressWriter(); progressWriter != nil { writer = progressWriter + } else { + // Historically push writes status to stderr + writer = os.Stderr } dec := json.NewDecoder(response.Body) diff --git a/pkg/bindings/test/images_test.go b/pkg/bindings/test/images_test.go index 9c9796661..53c5a1e83 100644 --- a/pkg/bindings/test/images_test.go +++ b/pkg/bindings/test/images_test.go @@ -1,11 +1,14 @@ package bindings_test import ( + "bytes" + "fmt" "net/http" "os" "path/filepath" "time" + podmanRegistry "github.com/containers/podman/v4/hack/podman-registry-go" "github.com/containers/podman/v4/pkg/bindings" "github.com/containers/podman/v4/pkg/bindings/containers" "github.com/containers/podman/v4/pkg/bindings/images" @@ -362,9 +365,14 @@ var _ = Describe("Podman images", func() { It("Image Pull", func() { rawImage := "docker.io/library/busybox:latest" - pulledImages, err := images.Pull(bt.conn, rawImage, nil) + var writer bytes.Buffer + pullOpts := new(images.PullOptions).WithProgressWriter(&writer) + pulledImages, err := images.Pull(bt.conn, rawImage, pullOpts) Expect(err).NotTo(HaveOccurred()) Expect(len(pulledImages)).To(Equal(1)) + output := writer.String() + Expect(output).To(ContainSubstring("Trying to pull ")) + Expect(output).To(ContainSubstring("Getting image source signatures")) exists, err := images.Exists(bt.conn, rawImage, nil) Expect(err).NotTo(HaveOccurred()) @@ -380,7 +388,19 @@ var _ = Describe("Podman images", func() { }) It("Image Push", func() { - Skip("TODO: implement test for image push to registry") + registry, err := podmanRegistry.Start() + Expect(err).To(BeNil()) + + var writer bytes.Buffer + pushOpts := new(images.PushOptions).WithUsername(registry.User).WithPassword(registry.Password).WithSkipTLSVerify(true).WithProgressWriter(&writer).WithQuiet(false) + err = images.Push(bt.conn, alpine.name, fmt.Sprintf("localhost:%s/test:latest", registry.Port), pushOpts) + Expect(err).ToNot(HaveOccurred()) + + output := writer.String() + 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")) }) It("Build no options", func() { diff --git a/pkg/bindings/test/manifests_test.go b/pkg/bindings/test/manifests_test.go index 6a34ef5a6..d6749f920 100644 --- a/pkg/bindings/test/manifests_test.go +++ b/pkg/bindings/test/manifests_test.go @@ -1,9 +1,12 @@ package bindings_test import ( + "bytes" + "fmt" "net/http" "time" + podmanRegistry "github.com/containers/podman/v4/hack/podman-registry-go" "github.com/containers/podman/v4/pkg/bindings" "github.com/containers/podman/v4/pkg/bindings/images" "github.com/containers/podman/v4/pkg/bindings/manifests" @@ -12,7 +15,7 @@ import ( "github.com/onsi/gomega/gexec" ) -var _ = Describe("podman manifest", func() { +var _ = Describe("Podman manifests", func() { var ( bt *bindingTest s *gexec.Session @@ -172,7 +175,21 @@ var _ = Describe("podman manifest", func() { Expect(list.Manifests[0].Platform.OS).To(Equal("foo")) }) - It("push manifest", func() { - Skip("TODO: implement test for manifest push to registry") + It("Manifest Push", func() { + registry, err := podmanRegistry.Start() + Expect(err).To(BeNil()) + + name := "quay.io/libpod/foobar:latest" + _, err = manifests.Create(bt.conn, name, []string{alpine.name}, nil) + Expect(err).ToNot(HaveOccurred()) + + var writer bytes.Buffer + pushOpts := new(images.PushOptions).WithUsername(registry.User).WithPassword(registry.Password).WithAll(true).WithSkipTLSVerify(true).WithProgressWriter(&writer).WithQuiet(false) + _, err = manifests.Push(bt.conn, name, fmt.Sprintf("localhost:%s/test:latest", registry.Port), pushOpts) + Expect(err).ToNot(HaveOccurred()) + + output := writer.String() + Expect(output).To(ContainSubstring("Writing manifest list to image destination")) + Expect(output).To(ContainSubstring("Storing list signatures")) }) }) diff --git a/pkg/domain/entities/images.go b/pkg/domain/entities/images.go index 21c1372b9..cad11b0ab 100644 --- a/pkg/domain/entities/images.go +++ b/pkg/domain/entities/images.go @@ -156,6 +156,8 @@ type ImagePullOptions struct { SkipTLSVerify types.OptionalBool // PullPolicy whether to pull new image PullPolicy config.PullPolicy + // Writer is used to display copy information including progress bars. + Writer io.Writer } // ImagePullReport is the response from pulling one or more images. diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index 77d1bf0db..f9839f62f 100644 --- a/pkg/domain/infra/abi/images.go +++ b/pkg/domain/infra/abi/images.go @@ -237,8 +237,9 @@ func (ir *ImageEngine) Pull(ctx context.Context, rawImage string, options entiti pullOptions.Variant = options.Variant pullOptions.SignaturePolicyPath = options.SignaturePolicy pullOptions.InsecureSkipTLSVerify = options.SkipTLSVerify + pullOptions.Writer = options.Writer - if !options.Quiet { + if !options.Quiet && pullOptions.Writer == nil { pullOptions.Writer = os.Stderr } diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index bb3014099..2716aaf2a 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/pkg/domain/infra/tunnel/images.go @@ -110,6 +110,7 @@ func (ir *ImageEngine) Pull(ctx context.Context, rawImage string, opts entities. options.WithAllTags(opts.AllTags).WithAuthfile(opts.Authfile).WithArch(opts.Arch).WithOS(opts.OS) options.WithVariant(opts.Variant).WithPassword(opts.Password) options.WithQuiet(opts.Quiet).WithUsername(opts.Username).WithPolicy(opts.PullPolicy.String()) + options.WithProgressWriter(opts.Writer) if s := opts.SkipTLSVerify; s != types.OptionalBoolUndefined { if s == types.OptionalBoolTrue { options.WithSkipTLSVerify(true) diff --git a/test/e2e/manifest_test.go b/test/e2e/manifest_test.go index 1c4aad710..60b72dcaa 100644 --- a/test/e2e/manifest_test.go +++ b/test/e2e/manifest_test.go @@ -350,6 +350,33 @@ var _ = Describe("Podman manifest", func() { Expect(foundZstdFile).To(BeTrue()) }) + It("push progress", func() { + SkipIfRemote("manifest push to dir not supported in remote mode") + + session := podmanTest.Podman([]string{"manifest", "create", "foo", imageList}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + dest := filepath.Join(podmanTest.TempDir, "pushed") + err := os.MkdirAll(dest, os.ModePerm) + Expect(err).To(BeNil()) + defer func() { + os.RemoveAll(dest) + }() + + session = podmanTest.Podman([]string{"push", "foo", "-q", "dir:" + dest}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.ErrorToString()).To(BeEmpty()) + + session = podmanTest.Podman([]string{"push", "foo", "dir:" + dest}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + output := session.ErrorToString() + Expect(output).To(ContainSubstring("Writing manifest list to image destination")) + Expect(output).To(ContainSubstring("Storing list signatures")) + }) + It("authenticated push", func() { registryOptions := &podmanRegistry.Options{ Image: "docker-archive:" + imageTarPath(REGISTRY_IMAGE), diff --git a/test/e2e/pull_test.go b/test/e2e/pull_test.go index 12f14fdc8..ba717f393 100644 --- a/test/e2e/pull_test.go +++ b/test/e2e/pull_test.go @@ -545,4 +545,18 @@ var _ = Describe("Podman pull", func() { Expect(data[0]).To(HaveField("Os", runtime.GOOS)) Expect(data[0]).To(HaveField("Architecture", "arm64")) }) + + It("podman pull progress", func() { + session := podmanTest.Podman([]string{"pull", ALPINE}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + output := session.ErrorToString() + Expect(output).To(ContainSubstring("Getting image source signatures")) + Expect(output).To(ContainSubstring("Copying blob ")) + + session = podmanTest.Podman([]string{"pull", "-q", ALPINE}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.ErrorToString()).To(BeEmpty()) + }) }) -- cgit v1.2.3-54-g00ecf From 53369aaa1575caa652461d3671713577fa0ac291 Mon Sep 17 00:00:00 2001 From: Charlie Doern Date: Mon, 22 Aug 2022 14:25:10 -0400 Subject: pass environment variables to container clone the env vars are held in the spec rather than the config, so they need to be mapped manually. They are also of a different format so special handling needed to be added. All env from the parent container will now be passed to the clone. resolves #15242 Signed-off-by: Charlie Doern --- pkg/specgen/generate/container.go | 18 +++++++++++++++--- test/e2e/container_clone_test.go | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) (limited to 'pkg') diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go index ec85f0f79..85cd8f5ca 100644 --- a/pkg/specgen/generate/container.go +++ b/pkg/specgen/generate/container.go @@ -347,9 +347,21 @@ func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, contaierID s conf.Systemd = tmpSystemd conf.Mounts = tmpMounts - if conf.Spec != nil && conf.Spec.Linux != nil && conf.Spec.Linux.Resources != nil { - if specg.ResourceLimits == nil { - specg.ResourceLimits = conf.Spec.Linux.Resources + if conf.Spec != nil { + if conf.Spec.Linux != nil && conf.Spec.Linux.Resources != nil { + if specg.ResourceLimits == nil { + specg.ResourceLimits = conf.Spec.Linux.Resources + } + } + if conf.Spec.Process != nil && conf.Spec.Process.Env != nil { + env := make(map[string]string) + for _, entry := range conf.Spec.Process.Env { + split := strings.Split(entry, "=") + if len(split) == 2 { + env[split[0]] = split[1] + } + } + specg.Env = env } } diff --git a/test/e2e/container_clone_test.go b/test/e2e/container_clone_test.go index 94ccd6ffe..b3c22470b 100644 --- a/test/e2e/container_clone_test.go +++ b/test/e2e/container_clone_test.go @@ -292,4 +292,20 @@ var _ = Describe("Podman container clone", func() { Expect(ok).To(BeTrue()) }) + + It("podman container clone env test", func() { + session := podmanTest.Podman([]string{"run", "--name", "env_ctr", "-e", "ENV_TEST=123", ALPINE, "printenv", "ENV_TEST"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + session = podmanTest.Podman([]string{"container", "clone", "env_ctr"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + session = podmanTest.Podman([]string{"start", "-a", "env_ctr-clone"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).Should(ContainSubstring("123")) + + }) }) -- cgit v1.2.3-54-g00ecf From 716ac1c866787d7d6b1ee8b800da527fd5b980d0 Mon Sep 17 00:00:00 2001 From: Toshiki Sonoda Date: Tue, 23 Aug 2022 09:58:34 +0900 Subject: Refactor: About the RawInput process Refactor the RawInput process of the `rm` and `start` subcommands, like the other subcommands such as `restart, stop, etc`. [NO NEW TESTS NEEDED] Signed-off-by: Toshiki Sonoda --- cmd/podman/containers/rm.go | 7 +++++-- pkg/domain/infra/abi/containers.go | 24 +++++++++++++----------- pkg/domain/infra/tunnel/containers.go | 16 +++++++++------- 3 files changed, 27 insertions(+), 20 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/containers/rm.go b/cmd/podman/containers/rm.go index 1e3976389..9c760e752 100644 --- a/cmd/podman/containers/rm.go +++ b/cmd/podman/containers/rm.go @@ -149,7 +149,8 @@ func removeContainers(namesOrIDs []string, rmOptions entities.RmOptions, setExit return err } for _, r := range responses { - if r.Err != nil { + switch { + case r.Err != nil: if errors.Is(r.Err, define.ErrWillDeadlock) { logrus.Errorf("Potential deadlock detected - please run 'podman system renumber' to resolve") } @@ -160,8 +161,10 @@ func removeContainers(namesOrIDs []string, rmOptions entities.RmOptions, setExit setExitCode(r.Err) } errs = append(errs, r.Err) - } else { + case r.RawInput != "": fmt.Println(r.RawInput) + default: + fmt.Println(r.Id) } } return errs.PrintErrors() diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 08d845d70..0a8e5bc2f 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -381,7 +381,7 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, // this will fail and code will fall through to removing the container from libpod.` tmpNames := []string{} for _, ctr := range names { - report := reports.RmReport{Id: ctr, RawInput: ctr} + report := reports.RmReport{Id: ctr} report.Err = ic.Libpod.RemoveStorageContainer(ctr, options.Force) //nolint:gocritic if report.Err == nil { @@ -938,13 +938,15 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri if err != nil { return nil, err } + idToRawInput := map[string]string{} + if len(rawInputs) == len(ctrs) { + for i := range ctrs { + idToRawInput[ctrs[i].ID()] = rawInputs[i] + } + } // There can only be one container if attach was used for i := range ctrs { ctr := ctrs[i] - rawInput := ctr.ID() - if !options.All { - rawInput = rawInputs[i] - } ctrState, err := ctr.State() if err != nil { return nil, err @@ -958,7 +960,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri // Exit cleanly immediately reports = append(reports, &entities.ContainerStartReport{ Id: ctr.ID(), - RawInput: rawInput, + RawInput: idToRawInput[ctr.ID()], Err: nil, ExitCode: 0, }) @@ -969,7 +971,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri logrus.Debugf("Deadlock error: %v", err) reports = append(reports, &entities.ContainerStartReport{ Id: ctr.ID(), - RawInput: rawInput, + RawInput: idToRawInput[ctr.ID()], Err: err, ExitCode: define.ExitCode(err), }) @@ -979,7 +981,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri if ctrRunning { reports = append(reports, &entities.ContainerStartReport{ Id: ctr.ID(), - RawInput: rawInput, + RawInput: idToRawInput[ctr.ID()], Err: nil, ExitCode: 0, }) @@ -989,7 +991,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri if err != nil { reports = append(reports, &entities.ContainerStartReport{ Id: ctr.ID(), - RawInput: rawInput, + RawInput: idToRawInput[ctr.ID()], Err: err, ExitCode: exitCode, }) @@ -1004,7 +1006,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri exitCode = ic.GetContainerExitCode(ctx, ctr) reports = append(reports, &entities.ContainerStartReport{ Id: ctr.ID(), - RawInput: rawInput, + RawInput: idToRawInput[ctr.ID()], Err: err, ExitCode: exitCode, }) @@ -1017,7 +1019,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri // If the container is in a pod, also set to recursively start dependencies report := &entities.ContainerStartReport{ Id: ctr.ID(), - RawInput: rawInput, + RawInput: idToRawInput[ctr.ID()], ExitCode: 125, } if err := ctr.Start(ctx, true); err != nil { diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 046509140..023bee430 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -672,10 +672,16 @@ func logIfRmError(id string, err error, reports []*reports.RmReport) { func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []string, options entities.ContainerStartOptions) ([]*entities.ContainerStartReport, error) { reports := []*entities.ContainerStartReport{} var exitCode = define.ExecErrorCodeGeneric - ctrs, namesOrIds, err := getContainersAndInputByContext(ic.ClientCtx, options.All, false, namesOrIds, options.Filters) + ctrs, rawInputs, err := getContainersAndInputByContext(ic.ClientCtx, options.All, false, namesOrIds, options.Filters) if err != nil { return nil, err } + idToRawInput := map[string]string{} + if len(rawInputs) == len(ctrs) { + for i := range ctrs { + idToRawInput[ctrs[i].ID] = rawInputs[i] + } + } removeOptions := new(containers.RemoveOptions).WithVolumes(true).WithForce(false) removeContainer := func(id string) { reports, err := containers.Remove(ic.ClientCtx, id, removeOptions) @@ -683,15 +689,11 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri } // There can only be one container if attach was used - for i, ctr := range ctrs { + for _, ctr := range ctrs { name := ctr.ID - rawInput := ctr.ID - if !options.All { - rawInput = namesOrIds[i] - } report := entities.ContainerStartReport{ Id: name, - RawInput: rawInput, + RawInput: idToRawInput[name], ExitCode: exitCode, } ctrRunning := ctr.State == define.ContainerStateRunning.String() -- cgit v1.2.3-54-g00ecf From 64339d47c105373557248d45fddf7ab2db435180 Mon Sep 17 00:00:00 2001 From: Toshiki Sonoda Date: Tue, 23 Aug 2022 11:54:31 +0900 Subject: Warning messages are printed and ignored if we use an unsupported option When an unsupported limit on cgroups V1 rootless systems is requested, podman prints an warning message and ignores the option/flag. ``` Target options/flags: --cpu-period, --cpu-quota, --cpu-rt-period, --cpu-rt-runtime, --cpus, --cpu-shares, --cpuset-cpus, --cpuset-mems, --memory, --memory-reservation, --memory-swap, --memory-swappiness, --blkio-weight, --device-read-bps, --device-write-bps, --device-read-iops, --device-write-iops, --blkio-weight-device ``` Related to https://github.com/containers/podman/discussions/10152 Signed-off-by: Toshiki Sonoda --- docs/source/markdown/options/blkio-weight.md | 2 ++ docs/source/markdown/options/cpu-period.md | 2 ++ docs/source/markdown/options/cpu-quota.md | 2 ++ docs/source/markdown/options/cpu-rt-period.md | 2 +- docs/source/markdown/options/cpu-rt-runtime.md | 2 +- docs/source/markdown/options/cpu-shares.md | 2 ++ docs/source/markdown/options/cpuset-cpus.md | 2 ++ docs/source/markdown/options/cpuset-mems.md | 2 ++ docs/source/markdown/options/memory-swappiness.md | 2 +- docs/source/markdown/podman-container-clone.1.md.in | 12 ++++++++++++ docs/source/markdown/podman-create.1.md.in | 16 ++++++++++++++++ docs/source/markdown/podman-run.1.md.in | 16 ++++++++++++++++ pkg/specgen/generate/validate.go | 6 ++++++ test/e2e/container_clone_test.go | 1 + test/e2e/create_test.go | 2 ++ test/e2e/generate_kube_test.go | 2 ++ test/e2e/generate_spec_test.go | 2 ++ test/system/030-run.bats | 7 ++++++- 18 files changed, 78 insertions(+), 4 deletions(-) (limited to 'pkg') diff --git a/docs/source/markdown/options/blkio-weight.md b/docs/source/markdown/options/blkio-weight.md index eb8e94144..04a1071c0 100644 --- a/docs/source/markdown/options/blkio-weight.md +++ b/docs/source/markdown/options/blkio-weight.md @@ -1,3 +1,5 @@ #### **--blkio-weight**=*weight* Block IO relative weight. The _weight_ is a value between **10** and **1000**. + +This option is not supported on cgroups V1 rootless systems. diff --git a/docs/source/markdown/options/cpu-period.md b/docs/source/markdown/options/cpu-period.md index efbe6c2ab..5c5eb56e7 100644 --- a/docs/source/markdown/options/cpu-period.md +++ b/docs/source/markdown/options/cpu-period.md @@ -8,3 +8,5 @@ microseconds. On some systems, changing the resource limits may not be allowed for non-root users. For more details, see https://github.com/containers/podman/blob/main/troubleshooting.md#26-running-containers-with-resource-limits-fails-with-a-permissions-error + +This option is not supported on cgroups V1 rootless systems. diff --git a/docs/source/markdown/options/cpu-quota.md b/docs/source/markdown/options/cpu-quota.md index 753797bad..81d5db3d2 100644 --- a/docs/source/markdown/options/cpu-quota.md +++ b/docs/source/markdown/options/cpu-quota.md @@ -10,3 +10,5 @@ ends (controllable via **--cpu-period**). On some systems, changing the resource limits may not be allowed for non-root users. For more details, see https://github.com/containers/podman/blob/main/troubleshooting.md#26-running-containers-with-resource-limits-fails-with-a-permissions-error + +This option is not supported on cgroups V1 rootless systems. diff --git a/docs/source/markdown/options/cpu-rt-period.md b/docs/source/markdown/options/cpu-rt-period.md index 9014beb33..36e88632e 100644 --- a/docs/source/markdown/options/cpu-rt-period.md +++ b/docs/source/markdown/options/cpu-rt-period.md @@ -4,4 +4,4 @@ Limit the CPU real-time period in microseconds. Limit the container's Real Time CPU usage. This option tells the kernel to restrict the container's Real Time CPU usage to the period specified. -This option is not supported on cgroups V2 systems. +This option is only supported on cgroups V1 rootful systems. diff --git a/docs/source/markdown/options/cpu-rt-runtime.md b/docs/source/markdown/options/cpu-rt-runtime.md index 05b1d3b96..64f0ec38b 100644 --- a/docs/source/markdown/options/cpu-rt-runtime.md +++ b/docs/source/markdown/options/cpu-rt-runtime.md @@ -7,4 +7,4 @@ Period of 1,000,000us and Runtime of 950,000us means that this container could c The sum of all runtimes across containers cannot exceed the amount allotted to the parent cgroup. -This option is not supported on cgroups V2 systems. +This option is only supported on cgroups V1 rootful systems. diff --git a/docs/source/markdown/options/cpu-shares.md b/docs/source/markdown/options/cpu-shares.md index c2115c1bf..c0e2c3035 100644 --- a/docs/source/markdown/options/cpu-shares.md +++ b/docs/source/markdown/options/cpu-shares.md @@ -37,3 +37,5 @@ this can result in the following division of CPU shares: On some systems, changing the resource limits may not be allowed for non-root users. For more details, see https://github.com/containers/podman/blob/main/troubleshooting.md#26-running-containers-with-resource-limits-fails-with-a-permissions-error + +This option is not supported on cgroups V1 rootless systems. diff --git a/docs/source/markdown/options/cpuset-cpus.md b/docs/source/markdown/options/cpuset-cpus.md index a67766897..8a2a82e9f 100644 --- a/docs/source/markdown/options/cpuset-cpus.md +++ b/docs/source/markdown/options/cpuset-cpus.md @@ -7,3 +7,5 @@ CPUs in which to allow execution. Can be specified as a comma-separated list On some systems, changing the resource limits may not be allowed for non-root users. For more details, see https://github.com/containers/podman/blob/main/troubleshooting.md#26-running-containers-with-resource-limits-fails-with-a-permissions-error + +This option is not supported on cgroups V1 rootless systems. diff --git a/docs/source/markdown/options/cpuset-mems.md b/docs/source/markdown/options/cpuset-mems.md index 1eeab7b13..b86d0ef6b 100644 --- a/docs/source/markdown/options/cpuset-mems.md +++ b/docs/source/markdown/options/cpuset-mems.md @@ -10,3 +10,5 @@ two memory nodes. On some systems, changing the resource limits may not be allowed for non-root users. For more details, see https://github.com/containers/podman/blob/main/troubleshooting.md#26-running-containers-with-resource-limits-fails-with-a-permissions-error + +This option is not supported on cgroups V1 rootless systems. diff --git a/docs/source/markdown/options/memory-swappiness.md b/docs/source/markdown/options/memory-swappiness.md index 65f0ef310..1e6a51188 100644 --- a/docs/source/markdown/options/memory-swappiness.md +++ b/docs/source/markdown/options/memory-swappiness.md @@ -2,4 +2,4 @@ Tune a container's memory swappiness behavior. Accepts an integer between *0* and *100*. -This flag is not supported on cgroups V2 systems. +This flag is only supported on cgroups V1 rootful systems. diff --git a/docs/source/markdown/podman-container-clone.1.md.in b/docs/source/markdown/podman-container-clone.1.md.in index cf760d7a2..26f414b62 100644 --- a/docs/source/markdown/podman-container-clone.1.md.in +++ b/docs/source/markdown/podman-container-clone.1.md.in @@ -40,6 +40,8 @@ Set a number of CPUs for the container that overrides the original containers CP This is shorthand for **--cpu-period** and **--cpu-quota**, so only **--cpus** or either both the **--cpu-period** and **--cpu-quota** options can be set. +This option is not supported on cgroups V1 rootless systems. + @@option cpuset-cpus If none are specified, the original container's CPUset is used. @@ -54,10 +56,14 @@ If none are specified, the original container's CPU memory nodes are used. Limit read rate (bytes per second) from a device (e.g. --device-read-bps=/dev/sda:1mb). +This option is not supported on cgroups V1 rootless systems. + #### **--device-write-bps**=*path* Limit write rate (bytes per second) to a device (e.g. --device-write-bps=/dev/sda:1mb) +This option is not supported on cgroups V1 rootless systems. + #### **--force**, **-f** Force removal of the original container that we are cloning. Can only be used in conjunction with **--destroy**. @@ -74,6 +80,8 @@ system's page size (the value would be very large, that's millions of trillions) If no memory limits are specified, the original container's will be used. +This option is not supported on cgroups V1 rootless systems. + #### **--memory-reservation**=*limit* Memory soft limit (format: `[]`, where unit = b (bytes), k (kibibytes), m (mebibytes), or g (gibibytes)) @@ -84,6 +92,8 @@ reservation. So you should always set the value below **--memory**, otherwise th hard limit will take precedence. By default, memory reservation will be the same as memory limit from the container being cloned. +This option is not supported on cgroups V1 rootless systems. + #### **--memory-swap**=*limit* A limit value equal to memory plus swap. Must be used with the **-m** @@ -95,6 +105,8 @@ The format of `LIMIT` is `[]`. Unit can be `b` (bytes), `k` (kibibytes), `m` (mebibytes), or `g` (gibibytes). If you don't specify a unit, `b` is used. Set LIMIT to `-1` to enable unlimited swap. +This option is not supported on cgroups V1 rootless systems. + @@option memory-swappiness #### **--name** diff --git a/docs/source/markdown/podman-create.1.md.in b/docs/source/markdown/podman-create.1.md.in index 7ec4fc66f..74348ac7d 100644 --- a/docs/source/markdown/podman-create.1.md.in +++ b/docs/source/markdown/podman-create.1.md.in @@ -131,6 +131,8 @@ On some systems, changing the CPU limits may not be allowed for non-root users. For more details, see https://github.com/containers/podman/blob/main/troubleshooting.md#26-running-containers-with-cpu-limits-fails-with-a-permissions-error +This option is not supported on cgroups V1 rootless systems. + @@option cpuset-cpus @@option cpuset-mems @@ -165,18 +167,26 @@ Add a rule to the cgroup allowed devices list. The rule is expected to be in the Limit read rate (bytes per second) from a device (e.g. --device-read-bps=/dev/sda:1mb) +This option is not supported on cgroups V1 rootless systems. + #### **--device-read-iops**=*path* Limit read rate (IO per second) from a device (e.g. --device-read-iops=/dev/sda:1000) +This option is not supported on cgroups V1 rootless systems. + #### **--device-write-bps**=*path* Limit write rate (bytes per second) to a device (e.g. --device-write-bps=/dev/sda:1mb) +This option is not supported on cgroups V1 rootless systems. + #### **--device-write-iops**=*path* Limit write rate (IO per second) to a device (e.g. --device-write-iops=/dev/sda:1000) +This option is not supported on cgroups V1 rootless systems. + #### **--disable-content-trust** This is a Docker specific option to disable image verification to a Docker @@ -366,6 +376,8 @@ RAM. If a limit of 0 is specified (not using **-m**), the container's memory is not limited. The actual limit may be rounded up to a multiple of the operating system's page size (the value would be very large, that's millions of trillions). +This option is not supported on cgroups V1 rootless systems. + #### **--memory-reservation**=*limit* Memory soft limit (format: `[]`, where unit = b (bytes), k (kibibytes), m (mebibytes), or g (gibibytes)) @@ -376,6 +388,8 @@ reservation. So you should always set the value below **--memory**, otherwise th hard limit will take precedence. By default, memory reservation will be the same as memory limit. +This option is not supported on cgroups V1 rootless systems. + #### **--memory-swap**=*limit* A limit value equal to memory plus swap. Must be used with the **-m** @@ -387,6 +401,8 @@ The format of `LIMIT` is `[]`. Unit can be `b` (bytes), `k` (kibibytes), `m` (mebibytes), or `g` (gibibytes). If you don't specify a unit, `b` is used. Set LIMIT to `-1` to enable unlimited swap. +This option is not supported on cgroups V1 rootless systems. + @@option memory-swappiness @@option mount diff --git a/docs/source/markdown/podman-run.1.md.in b/docs/source/markdown/podman-run.1.md.in index d10520e35..e943ec005 100644 --- a/docs/source/markdown/podman-run.1.md.in +++ b/docs/source/markdown/podman-run.1.md.in @@ -146,6 +146,8 @@ On some systems, changing the CPU limits may not be allowed for non-root users. For more details, see https://github.com/containers/podman/blob/main/troubleshooting.md#26-running-containers-with-cpu-limits-fails-with-a-permissions-error +This option is not supported on cgroups V1 rootless systems. + @@option cpuset-cpus @@option cpuset-mems @@ -196,18 +198,26 @@ Add a rule to the cgroup allowed devices list Limit read rate (in bytes per second) from a device (e.g. **--device-read-bps=/dev/sda:1mb**). +This option is not supported on cgroups V1 rootless systems. + #### **--device-read-iops**=*path:rate* Limit read rate (in IO operations per second) from a device (e.g. **--device-read-iops=/dev/sda:1000**). +This option is not supported on cgroups V1 rootless systems. + #### **--device-write-bps**=*path:rate* Limit write rate (in bytes per second) to a device (e.g. **--device-write-bps=/dev/sda:1mb**). +This option is not supported on cgroups V1 rootless systems. + #### **--device-write-iops**=*path:rate* Limit write rate (in IO operations per second) to a device (e.g. **--device-write-iops=/dev/sda:1000**). +This option is not supported on cgroups V1 rootless systems. + #### **--disable-content-trust** This is a Docker specific option to disable image verification to a Docker @@ -377,6 +387,8 @@ RAM. If a limit of 0 is specified (not using **-m**), the container's memory is not limited. The actual limit may be rounded up to a multiple of the operating system's page size (the value would be very large, that's millions of trillions). +This option is not supported on cgroups V1 rootless systems. + #### **--memory-reservation**=*number[unit]* Memory soft limit. A _unit_ can be **b** (bytes), **k** (kibibytes), **m** (mebibytes), or **g** (gibibytes). @@ -387,6 +399,8 @@ reservation. So you should always set the value below **--memory**, otherwise th hard limit will take precedence. By default, memory reservation will be the same as memory limit. +This option is not supported on cgroups V1 rootless systems. + #### **--memory-swap**=*number[unit]* A limit value equal to memory plus swap. @@ -399,6 +413,8 @@ the value of **--memory**. Set _number_ to **-1** to enable unlimited swap. +This option is not supported on cgroups V1 rootless systems. + @@option memory-swappiness @@option mount diff --git a/pkg/specgen/generate/validate.go b/pkg/specgen/generate/validate.go index 9c933d747..3c5d5fb96 100644 --- a/pkg/specgen/generate/validate.go +++ b/pkg/specgen/generate/validate.go @@ -9,6 +9,7 @@ import ( "github.com/containers/common/pkg/cgroups" "github.com/containers/common/pkg/sysinfo" + "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/specgen" "github.com/containers/podman/v4/utils" ) @@ -19,6 +20,11 @@ func verifyContainerResourcesCgroupV1(s *specgen.SpecGenerator) ([]string, error sysInfo := sysinfo.New(true) + if s.ResourceLimits != nil && rootless.IsRootless() { + s.ResourceLimits = nil + warnings = append(warnings, "Resource limits are not supported and ignored on cgroups V1 rootless systems") + } + if s.ResourceLimits == nil { return warnings, nil } diff --git a/test/e2e/container_clone_test.go b/test/e2e/container_clone_test.go index 94ccd6ffe..d7641e42a 100644 --- a/test/e2e/container_clone_test.go +++ b/test/e2e/container_clone_test.go @@ -87,6 +87,7 @@ var _ = Describe("Podman container clone", func() { }) It("podman container clone resource limits override", func() { + SkipIfRootlessCgroupsV1("Not supported for rootless + CgroupsV1") create := podmanTest.Podman([]string{"create", "--cpus=5", ALPINE}) create.WaitWithDefaultTimeout() Expect(create).To(Exit(0)) diff --git a/test/e2e/create_test.go b/test/e2e/create_test.go index 9679aad24..b35d0f3c5 100644 --- a/test/e2e/create_test.go +++ b/test/e2e/create_test.go @@ -438,6 +438,7 @@ var _ = Describe("Podman create", func() { }) It("podman create with -m 1000000 sets swap to 2000000", func() { + SkipIfRootlessCgroupsV1("Not supported for rootless + CgroupsV1") numMem := 1000000 ctrName := "testCtr" session := podmanTest.Podman([]string{"create", "-t", "-m", fmt.Sprintf("%db", numMem), "--name", ctrName, ALPINE, "/bin/sh"}) @@ -452,6 +453,7 @@ var _ = Describe("Podman create", func() { }) It("podman create --cpus 5 sets nanocpus", func() { + SkipIfRootlessCgroupsV1("Not supported for rootless + CgroupsV1") numCpus := 5 nanoCPUs := numCpus * 1000000000 ctrName := "testCtr" diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go index 142f32d19..e7ceaf2d2 100644 --- a/test/e2e/generate_kube_test.go +++ b/test/e2e/generate_kube_test.go @@ -490,6 +490,7 @@ var _ = Describe("Podman generate kube", func() { }) It("podman generate kube on pod with memory limit", func() { + SkipIfRootlessCgroupsV1("Not supported for rootless + CgroupsV1") podName := "testMemoryLimit" podSession := podmanTest.Podman([]string{"pod", "create", "--name", podName}) podSession.WaitWithDefaultTimeout() @@ -515,6 +516,7 @@ var _ = Describe("Podman generate kube", func() { }) It("podman generate kube on pod with cpu limit", func() { + SkipIfRootlessCgroupsV1("Not supported for rootless + CgroupsV1") podName := "testCpuLimit" podSession := podmanTest.Podman([]string{"pod", "create", "--name", podName}) podSession.WaitWithDefaultTimeout() diff --git a/test/e2e/generate_spec_test.go b/test/e2e/generate_spec_test.go index 57cd9546b..9188b5222 100644 --- a/test/e2e/generate_spec_test.go +++ b/test/e2e/generate_spec_test.go @@ -41,6 +41,7 @@ var _ = Describe("Podman generate spec", func() { }) It("podman generate spec basic usage", func() { + SkipIfRootlessCgroupsV1("Not supported for rootless + CgroupsV1") session := podmanTest.Podman([]string{"create", "--cpus", "5", "--name", "specgen", ALPINE}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) @@ -51,6 +52,7 @@ var _ = Describe("Podman generate spec", func() { }) It("podman generate spec file", func() { + SkipIfRootlessCgroupsV1("Not supported for rootless + CgroupsV1") session := podmanTest.Podman([]string{"create", "--cpus", "5", "--name", "specgen", ALPINE}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 908c169ee..a3bfe5780 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -56,7 +56,12 @@ echo $rand | 0 | $rand @test "podman run --memory=0 runtime option" { run_podman run --memory=0 --rm $IMAGE echo hello - is "$output" "hello" "failed to run when --memory is set to 0" + if is_rootless && ! is_cgroupsv2; then + is "${lines[0]}" "Resource limits are not supported and ignored on cgroups V1 rootless systems" "--memory is not supported" + is "${lines[1]}" "hello" "--memory is ignored" + else + is "$output" "hello" "failed to run when --memory is set to 0" + fi } # 'run --preserve-fds' passes a number of additional file descriptors into the container -- cgit v1.2.3-54-g00ecf From f87f6d2fc193c9b53ac8ba634954a811a32fd057 Mon Sep 17 00:00:00 2001 From: Arthur Sengileyev Date: Tue, 16 Aug 2022 13:39:02 +0300 Subject: Improved Windows compatibility Signed-off-by: Arthur Sengileyev --- cmd/rootlessport/main.go | 2 +- pkg/machine/pull.go | 4 ++-- pkg/machine/qemu/machine.go | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'pkg') diff --git a/cmd/rootlessport/main.go b/cmd/rootlessport/main.go index 5410cd14a..d8d6ffcee 100644 --- a/cmd/rootlessport/main.go +++ b/cmd/rootlessport/main.go @@ -225,7 +225,7 @@ outer: // https://github.com/containers/podman/issues/11248 // Copy /dev/null to stdout and stderr to prevent SIGPIPE errors - if f, err := os.OpenFile("/dev/null", os.O_WRONLY, 0755); err == nil { + if f, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0755); err == nil { unix.Dup2(int(f.Fd()), 1) //nolint:errcheck unix.Dup2(int(f.Fd()), 2) //nolint:errcheck f.Close() diff --git a/pkg/machine/pull.go b/pkg/machine/pull.go index 26b6adc67..22a1b4c0a 100644 --- a/pkg/machine/pull.go +++ b/pkg/machine/pull.go @@ -213,8 +213,8 @@ func decompressXZ(src string, output io.WriteCloser) error { var read io.Reader var cmd *exec.Cmd // Prefer xz utils for fastest performance, fallback to go xi2 impl - if _, err := exec.LookPath("xzcat"); err == nil { - cmd = exec.Command("xzcat", "-k", src) + if _, err := exec.LookPath("xz"); err == nil { + cmd = exec.Command("xz", "-d", "-c", "-k", src) read, err = cmd.StdoutPipe() if err != nil { return err diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 213f7ce5d..71752a101 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -545,12 +545,12 @@ func (v *MachineVM) Start(name string, _ machine.StartOptions) error { return err } defer fd.Close() - dnr, err := os.OpenFile("/dev/null", os.O_RDONLY, 0755) + dnr, err := os.OpenFile(os.DevNull, os.O_RDONLY, 0755) if err != nil { return err } defer dnr.Close() - dnw, err := os.OpenFile("/dev/null", os.O_WRONLY, 0755) + dnw, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0755) if err != nil { return err } @@ -1216,11 +1216,11 @@ func (v *MachineVM) startHostNetworking() (string, apiForwardingState, error) { } attr := new(os.ProcAttr) - dnr, err := os.OpenFile("/dev/null", os.O_RDONLY, 0755) + dnr, err := os.OpenFile(os.DevNull, os.O_RDONLY, 0755) if err != nil { return "", noForwarding, err } - dnw, err := os.OpenFile("/dev/null", os.O_WRONLY, 0755) + dnw, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0755) if err != nil { return "", noForwarding, err } -- cgit v1.2.3-54-g00ecf From 65efcdf709a83e6d013b4e65247750c97236d89a Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 23 Aug 2022 10:51:21 -0400 Subject: Allow podman to run in an environment with keys containing spaces Fixes: https://github.com/containers/podman/issues/15251 Signed-off-by: Daniel J Walsh --- pkg/env/env.go | 20 ++++++++++++++++---- pkg/specgen/generate/container.go | 6 ++---- pkg/specgenutil/specgen.go | 5 +---- test/e2e/run_env_test.go | 7 +++++++ 4 files changed, 26 insertions(+), 12 deletions(-) (limited to 'pkg') diff --git a/pkg/env/env.go b/pkg/env/env.go index 8af9fd77c..fb7949ad8 100644 --- a/pkg/env/env.go +++ b/pkg/env/env.go @@ -37,6 +37,22 @@ func Slice(m map[string]string) []string { return env } +// Map transforms the specified slice of environment variables into a +// map. +func Map(slice []string) map[string]string { + envmap := make(map[string]string, len(slice)) + for _, val := range slice { + data := strings.SplitN(val, "=", 2) + + if len(data) > 1 { + envmap[data[0]] = data[1] + } else { + envmap[data[0]] = "" + } + } + return envmap +} + // Join joins the two environment maps with override overriding base. func Join(base map[string]string, override map[string]string) map[string]string { if len(base) == 0 { @@ -87,10 +103,6 @@ func parseEnv(env map[string]string, line string) error { } // trim the front of a variable, but nothing else name := strings.TrimLeft(data[0], whiteSpaces) - if strings.ContainsAny(name, whiteSpaces) { - return fmt.Errorf("name %q has white spaces, poorly formatted name", name) - } - if len(data) > 1 { env[name] = data[1] } else { diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go index 85cd8f5ca..0d5027d76 100644 --- a/pkg/specgen/generate/container.go +++ b/pkg/specgen/generate/container.go @@ -140,10 +140,8 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat } // First transform the os env into a map. We need it for the labels later in // any case. - osEnv, err := envLib.ParseSlice(os.Environ()) - if err != nil { - return nil, fmt.Errorf("error parsing host environment variables: %w", err) - } + osEnv := envLib.Map(os.Environ()) + // Caller Specified defaults if s.EnvHost { defaultEnvs = envLib.Join(defaultEnvs, osEnv) diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go index 7392e7b44..c706b396f 100644 --- a/pkg/specgenutil/specgen.go +++ b/pkg/specgenutil/specgen.go @@ -362,10 +362,7 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions // First transform the os env into a map. We need it for the labels later in // any case. - osEnv, err := envLib.ParseSlice(os.Environ()) - if err != nil { - return fmt.Errorf("error parsing host environment variables: %w", err) - } + osEnv := envLib.Map(os.Environ()) if !s.EnvHost { s.EnvHost = c.EnvHost diff --git a/test/e2e/run_env_test.go b/test/e2e/run_env_test.go index bab52efc5..4580d68ce 100644 --- a/test/e2e/run_env_test.go +++ b/test/e2e/run_env_test.go @@ -58,6 +58,13 @@ var _ = Describe("Podman run", func() { Expect(session).Should(Exit(0)) Expect(session.OutputToString()).To(ContainSubstring("/bin")) + // Verify environ keys with spaces do not blow up podman command + os.Setenv("FOO BAR", "BAZ") + session = podmanTest.Podman([]string{"run", "--rm", ALPINE, "true"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + os.Unsetenv("FOO BAR") + os.Setenv("FOO", "BAR") session = podmanTest.Podman([]string{"run", "--rm", "--env", "FOO", ALPINE, "printenv", "FOO"}) session.WaitWithDefaultTimeout() -- cgit v1.2.3-54-g00ecf From 0f739355635d5bc4d538cf88009d7af533e7c289 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 23 Aug 2022 14:46:43 -0400 Subject: Add support for containers.conf volume timeouts Also, do a general cleanup of all the timeout code. Changes include: - Convert from int to *uint where possible. Timeouts cannot be negative, hence the uint change; and a timeout of 0 is valid, so we need a new way to detect that the user set a timeout (hence, pointer). - Change name in the database to avoid conflicts between new data type and old one. This will cause timeouts set with 4.2.0 to be lost, but considering nobody is using the feature at present (and the lack of validation means we could have invalid, negative timeouts in the DB) this feels safe. - Ensure volume plugin timeouts can only be used with volumes created using a plugin. Timeouts on the local driver are nonsensical. - Remove the existing test, as it did not use a volume plugin. Write a new test that does. The actual plumbing of the containers.conf timeout in is one line in volume_api.go; the remainder are the above-described cleanups. Signed-off-by: Matthew Heon --- go.mod | 2 +- go.sum | 10 ++-- libpod/define/volume_inspect.go | 2 +- libpod/options.go | 14 ++++- libpod/plugin/volume_api.go | 19 +++--- libpod/runtime.go | 2 +- libpod/runtime_volume_linux.go | 2 +- libpod/volume.go | 2 +- libpod/volume_inspect.go | 7 ++- pkg/domain/infra/abi/parse/parse.go | 5 +- test/e2e/config/containers.conf | 2 + test/e2e/volume_create_test.go | 15 ----- test/e2e/volume_plugin_test.go | 34 +++++++++++ test/testvol/util.go | 2 +- .../Microsoft/hcsshim/internal/jobobject/iocp.go | 2 +- .../hcsshim/internal/jobobject/jobobject.go | 55 +++++++++++++++--- .../Microsoft/hcsshim/internal/jobobject/limits.go | 4 +- .../Microsoft/hcsshim/internal/queue/mq.go | 61 +++++++------------- .../Microsoft/hcsshim/internal/winapi/jobobject.go | 2 +- .../Microsoft/hcsshim/internal/winapi/process.go | 2 +- .../Microsoft/hcsshim/internal/winapi/system.go | 3 +- .../hcsshim/internal/winapi/zsyscall_windows.go | 6 +- .../containers/common/libimage/inspect.go | 2 +- .../github.com/containers/common/libimage/load.go | 2 +- .../containers/common/libnetwork/cni/network.go | 14 ++++- .../common/libnetwork/network/interface.go | 1 + .../containers/common/pkg/config/config.go | 67 ++++++++++++++++++++++ .../containers/common/pkg/config/config_darwin.go | 2 + .../containers/common/pkg/config/containers.conf | 17 +++++- .../containers/common/pkg/config/default.go | 6 +- .../containers/common/pkg/config/default_darwin.go | 5 ++ .../common/pkg/config/default_freebsd.go | 5 ++ .../containers/common/pkg/config/default_linux.go | 5 ++ .../common/pkg/config/default_windows.go | 5 ++ .../common/pkg/subscriptions/subscriptions.go | 2 +- vendor/modules.txt | 6 +- 36 files changed, 284 insertions(+), 108 deletions(-) (limited to 'pkg') diff --git a/go.mod b/go.mod index 635c0a17d..c0a23d962 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/containernetworking/cni v1.1.2 github.com/containernetworking/plugins v1.1.1 github.com/containers/buildah v1.27.0 - github.com/containers/common v0.49.2-0.20220817132854-f6679f170eca + github.com/containers/common v0.49.2-0.20220823130605-72a7da3358ac github.com/containers/conmon v2.0.20+incompatible github.com/containers/image/v5 v5.22.0 github.com/containers/ocicrypt v1.1.5 diff --git a/go.sum b/go.sum index 5053589c5..a476729ee 100644 --- a/go.sum +++ b/go.sum @@ -140,8 +140,9 @@ github.com/Microsoft/hcsshim v0.8.21/go.mod h1:+w2gRZ5ReXQhFOrvSQeNfhrYB/dg3oDwT github.com/Microsoft/hcsshim v0.8.22/go.mod h1:91uVCVzvX2QD16sMCenoxxXo6L1wJnLMX2PSufFMtF0= github.com/Microsoft/hcsshim v0.8.23/go.mod h1:4zegtUJth7lAvFyc6cH2gGQ5B3OFQim01nnU2M8jKDg= github.com/Microsoft/hcsshim v0.9.2/go.mod h1:7pLA8lDk46WKDWlVsENo92gC0XFa8rbKfyFRBqxEbCc= -github.com/Microsoft/hcsshim v0.9.3 h1:k371PzBuRrz2b+ebGuI2nVgVhgsVX60jMfSw80NECxo= github.com/Microsoft/hcsshim v0.9.3/go.mod h1:7pLA8lDk46WKDWlVsENo92gC0XFa8rbKfyFRBqxEbCc= +github.com/Microsoft/hcsshim v0.9.4 h1:mnUj0ivWy6UzbB1uLFqKR6F+ZyiDc7j4iGgHTpO+5+I= +github.com/Microsoft/hcsshim v0.9.4/go.mod h1:7pLA8lDk46WKDWlVsENo92gC0XFa8rbKfyFRBqxEbCc= github.com/Microsoft/hcsshim/test v0.0.0-20201218223536-d3e5debf77da/go.mod h1:5hlzMzRKMLyo42nCZ9oml8AdTlq/0cvIaBv6tK1RehU= github.com/Microsoft/hcsshim/test v0.0.0-20210227013316-43a75bb4edd3/go.mod h1:mw7qgWloBUl75W/gVH3cQszUg1+gUITj7D6NY7ywVnY= github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ= @@ -323,8 +324,9 @@ github.com/containerd/containerd v1.5.7/go.mod h1:gyvv6+ugqY25TiXxcZC3L5yOeYgEw0 github.com/containerd/containerd v1.5.8/go.mod h1:YdFSv5bTFLpG2HIYmfqDpSYYTDX+mc5qtSuYx1YUb/s= github.com/containerd/containerd v1.5.9/go.mod h1:fvQqCfadDGga5HZyn3j4+dx56qj2I9YwBrlSdalvJYQ= github.com/containerd/containerd v1.6.1/go.mod h1:1nJz5xCZPusx6jJU8Frfct988y0NpumIq9ODB0kLtoE= -github.com/containerd/containerd v1.6.6 h1:xJNPhbrmz8xAMDNoVjHy9YHtWwEQNS+CDkcIRh7t8Y0= github.com/containerd/containerd v1.6.6/go.mod h1:ZoP1geJldzCVY3Tonoz7b1IXk8rIX0Nltt5QE4OMNk0= +github.com/containerd/containerd v1.6.8 h1:h4dOFDwzHmqFEP754PgfgTeVXFnLiRc6kiqC7tplDJs= +github.com/containerd/containerd v1.6.8/go.mod h1:By6p5KqPK0/7/CgO/A6t/Gz+CUYUu2zf1hUaaymVXB0= github.com/containerd/continuity v0.0.0-20190426062206-aaeac12a7ffc/go.mod h1:GL3xCUCBDV3CZiTSEKksMWbLE66hEyuu9qyDOOqM47Y= github.com/containerd/continuity v0.0.0-20190815185530-f2a389ac0a02/go.mod h1:GL3xCUCBDV3CZiTSEKksMWbLE66hEyuu9qyDOOqM47Y= github.com/containerd/continuity v0.0.0-20191127005431-f65d91d395eb/go.mod h1:GL3xCUCBDV3CZiTSEKksMWbLE66hEyuu9qyDOOqM47Y= @@ -395,8 +397,8 @@ github.com/containernetworking/plugins v1.1.1/go.mod h1:Sr5TH/eBsGLXK/h71HeLfX19 github.com/containers/buildah v1.27.0 h1:LJ1ks7vKxwPzJGr5BWVvigbtVL9w7XeHtNEmiIOPJqI= github.com/containers/buildah v1.27.0/go.mod h1:anH3ExvDXRNP9zLQCrOc1vWb5CrhqLF/aYFim4tslvA= github.com/containers/common v0.49.1/go.mod h1:ueM5hT0itKqCQvVJDs+EtjornAQtrHYxQJzP2gxeGIg= -github.com/containers/common v0.49.2-0.20220817132854-f6679f170eca h1:OjhEBVpFskIJ6Vq9nikYW7M6YXfkTxOBu+EQBoCyhuM= -github.com/containers/common v0.49.2-0.20220817132854-f6679f170eca/go.mod h1:eT2iSsNzjOlF5VFLkyj9OU2SXznURvEYndsioQImuoE= +github.com/containers/common v0.49.2-0.20220823130605-72a7da3358ac h1:rLbTzosxPKrQd+EgMRxfC1WYm3azPiQfig+Lr7mCQ4k= +github.com/containers/common v0.49.2-0.20220823130605-72a7da3358ac/go.mod h1:xC4qkLfW9R+YSDknlT9xU+NDNxIw017U8AyohGtr9Ec= github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg= github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I= github.com/containers/image/v5 v5.22.0 h1:KemxPmD4D2YYOFZN2SgoTk7nBFcnwPiPW0MqjYtknSE= diff --git a/libpod/define/volume_inspect.go b/libpod/define/volume_inspect.go index 9279812da..76120647c 100644 --- a/libpod/define/volume_inspect.go +++ b/libpod/define/volume_inspect.go @@ -57,7 +57,7 @@ type InspectVolumeData struct { // UID/GID. NeedsChown bool `json:"NeedsChown,omitempty"` // Timeout is the specified driver timeout if given - Timeout int `json:"Timeout,omitempty"` + Timeout uint `json:"Timeout,omitempty"` } type VolumeReload struct { diff --git a/libpod/options.go b/libpod/options.go index 43ed1ff78..d31741094 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1695,14 +1695,22 @@ func withSetAnon() VolumeCreateOption { } } -// WithVolumeDriverTimeout sets the volume creation timeout period -func WithVolumeDriverTimeout(timeout int) VolumeCreateOption { +// WithVolumeDriverTimeout sets the volume creation timeout period. +// Only usable if a non-local volume driver is in use. +func WithVolumeDriverTimeout(timeout uint) VolumeCreateOption { return func(volume *Volume) error { if volume.valid { return define.ErrVolumeFinalized } - volume.config.Timeout = timeout + if volume.config.Driver == "" || volume.config.Driver == define.VolumeDriverLocal { + return fmt.Errorf("Volume driver timeout can only be used with non-local volume drivers: %w", define.ErrInvalidArg) + } + + tm := timeout + + volume.config.Timeout = &tm + return nil } } diff --git a/libpod/plugin/volume_api.go b/libpod/plugin/volume_api.go index 0a5eaae53..b13578388 100644 --- a/libpod/plugin/volume_api.go +++ b/libpod/plugin/volume_api.go @@ -3,6 +3,7 @@ package plugin import ( "bytes" "context" + "errors" "fmt" "io/ioutil" "net" @@ -13,8 +14,7 @@ import ( "sync" "time" - "errors" - + "github.com/containers/common/pkg/config" "github.com/containers/podman/v4/libpod/define" "github.com/docker/go-plugins-helpers/sdk" "github.com/docker/go-plugins-helpers/volume" @@ -40,7 +40,6 @@ var ( ) const ( - defaultTimeout = 5 * time.Second volumePluginType = "VolumeDriver" ) @@ -129,7 +128,7 @@ func validatePlugin(newPlugin *VolumePlugin) error { // GetVolumePlugin gets a single volume plugin, with the given name, at the // given path. -func GetVolumePlugin(name string, path string, timeout int) (*VolumePlugin, error) { +func GetVolumePlugin(name string, path string, timeout *uint, cfg *config.Config) (*VolumePlugin, error) { pluginsLock.Lock() defer pluginsLock.Unlock() @@ -152,13 +151,11 @@ func GetVolumePlugin(name string, path string, timeout int) (*VolumePlugin, erro // Need an HTTP client to force a Unix connection. // And since we can reuse it, might as well cache it. client := new(http.Client) - client.Timeout = defaultTimeout - // if the user specified a non-zero timeout, use their value. Else, keep the default. - if timeout != 0 { - if time.Duration(timeout)*time.Second < defaultTimeout { - logrus.Warnf("the default timeout for volume creation is %d seconds, setting a time less than that may break this feature.", defaultTimeout) - } - client.Timeout = time.Duration(timeout) * time.Second + client.Timeout = 5 * time.Second + if timeout != nil { + client.Timeout = time.Duration(*timeout) * time.Second + } else if cfg != nil { + client.Timeout = time.Duration(cfg.Engine.VolumePluginTimeout) * time.Second } // This bit borrowed from pkg/bindings/connection.go client.Transport = &http.Transport{ diff --git a/libpod/runtime.go b/libpod/runtime.go index 684f4abd7..9b97fd724 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -1097,7 +1097,7 @@ func (r *Runtime) getVolumePlugin(volConfig *VolumeConfig) (*plugin.VolumePlugin return nil, fmt.Errorf("no volume plugin with name %s available: %w", name, define.ErrMissingPlugin) } - return plugin.GetVolumePlugin(name, pluginPath, timeout) + return plugin.GetVolumePlugin(name, pluginPath, timeout, r.config) } // GetSecretsStorageDir returns the directory that the secrets manager should take diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index 1f354e41b..65f2a1005 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -184,7 +184,7 @@ func (r *Runtime) UpdateVolumePlugins(ctx context.Context) *define.VolumeReload ) for driverName, socket := range r.config.Engine.VolumePlugins { - driver, err := volplugin.GetVolumePlugin(driverName, socket, 0) + driver, err := volplugin.GetVolumePlugin(driverName, socket, nil, r.config) if err != nil { errs = append(errs, err) continue diff --git a/libpod/volume.go b/libpod/volume.go index 2e8cd77a5..a054e4032 100644 --- a/libpod/volume.go +++ b/libpod/volume.go @@ -56,7 +56,7 @@ type VolumeConfig struct { // quota tracking. DisableQuota bool `json:"disableQuota,omitempty"` // Timeout allows users to override the default driver timeout of 5 seconds - Timeout int + Timeout *uint `json:"timeout,omitempty"` } // VolumeState holds the volume's mutable state. diff --git a/libpod/volume_inspect.go b/libpod/volume_inspect.go index dd2f3fd01..c3872bca7 100644 --- a/libpod/volume_inspect.go +++ b/libpod/volume_inspect.go @@ -64,7 +64,12 @@ func (v *Volume) Inspect() (*define.InspectVolumeData, error) { data.MountCount = v.state.MountCount data.NeedsCopyUp = v.state.NeedsCopyUp data.NeedsChown = v.state.NeedsChown - data.Timeout = v.config.Timeout + + if v.config.Timeout != nil { + data.Timeout = *v.config.Timeout + } else if v.UsesVolumeDriver() { + data.Timeout = v.runtime.config.Engine.VolumePluginTimeout + } return data, nil } diff --git a/pkg/domain/infra/abi/parse/parse.go b/pkg/domain/infra/abi/parse/parse.go index 19699589b..fb2876bb2 100644 --- a/pkg/domain/infra/abi/parse/parse.go +++ b/pkg/domain/infra/abi/parse/parse.go @@ -86,8 +86,11 @@ func VolumeOptions(opts map[string]string) ([]libpod.VolumeCreateOption, error) if err != nil { return nil, fmt.Errorf("cannot convert Timeout %s to an integer: %w", splitO[1], err) } + if intTimeout < 0 { + return nil, fmt.Errorf("volume timeout cannot be negative (got %d)", intTimeout) + } logrus.Debugf("Removing timeout from options and adding WithTimeout for Timeout %d", intTimeout) - libpodOptions = append(libpodOptions, libpod.WithVolumeDriverTimeout(intTimeout)) + libpodOptions = append(libpodOptions, libpod.WithVolumeDriverTimeout(uint(intTimeout))) default: finalVal = append(finalVal, o) } diff --git a/test/e2e/config/containers.conf b/test/e2e/config/containers.conf index c33f32ab4..94bb316b1 100644 --- a/test/e2e/config/containers.conf +++ b/test/e2e/config/containers.conf @@ -61,6 +61,8 @@ no_hosts=true network_cmd_options=["allow_host_loopback=true"] service_timeout=1234 +volume_plugin_timeout = 15 + # We need to ensure each test runs on a separate plugin instance... # For now, let's just make a bunch of plugin paths and have each test use one. [engine.volume_plugins] diff --git a/test/e2e/volume_create_test.go b/test/e2e/volume_create_test.go index 7a975f6a5..499283cab 100644 --- a/test/e2e/volume_create_test.go +++ b/test/e2e/volume_create_test.go @@ -162,19 +162,4 @@ var _ = Describe("Podman volume create", func() { Expect(inspectOpts).Should(Exit(0)) Expect(inspectOpts.OutputToString()).To(Equal(optionStrFormatExpect)) }) - - It("podman create volume with o=timeout", func() { - volName := "testVol" - timeout := 10 - timeoutStr := "10" - session := podmanTest.Podman([]string{"volume", "create", "--opt", fmt.Sprintf("o=timeout=%d", timeout), volName}) - session.WaitWithDefaultTimeout() - Expect(session).Should(Exit(0)) - - inspectTimeout := podmanTest.Podman([]string{"volume", "inspect", "--format", "{{ .Timeout }}", volName}) - inspectTimeout.WaitWithDefaultTimeout() - Expect(inspectTimeout).Should(Exit(0)) - Expect(inspectTimeout.OutputToString()).To(Equal(timeoutStr)) - - }) }) diff --git a/test/e2e/volume_plugin_test.go b/test/e2e/volume_plugin_test.go index b585f8dd8..a44e75a54 100644 --- a/test/e2e/volume_plugin_test.go +++ b/test/e2e/volume_plugin_test.go @@ -256,4 +256,38 @@ Removed: Expect(session.OutputToStringArray()).To(ContainElements(localvol, vol2)) Expect(session.ErrorToString()).To(Equal("")) // make no errors are shown }) + + It("volume driver timeouts test", func() { + podmanTest.AddImageToRWStore(volumeTest) + + pluginStatePath := filepath.Join(podmanTest.TempDir, "volumes") + err := os.Mkdir(pluginStatePath, 0755) + Expect(err).ToNot(HaveOccurred()) + + // Keep this distinct within tests to avoid multiple tests using the same plugin. + pluginName := "testvol6" + plugin := podmanTest.Podman([]string{"run", "--security-opt", "label=disable", "-v", "/run/docker/plugins:/run/docker/plugins", "-v", fmt.Sprintf("%v:%v", pluginStatePath, pluginStatePath), "-d", volumeTest, "--sock-name", pluginName, "--path", pluginStatePath}) + plugin.WaitWithDefaultTimeout() + Expect(plugin).Should(Exit(0)) + + volName := "testVolume1" + create := podmanTest.Podman([]string{"volume", "create", "--driver", pluginName, volName}) + create.WaitWithDefaultTimeout() + Expect(create).Should(Exit(0)) + + volInspect := podmanTest.Podman([]string{"volume", "inspect", "--format", "{{ .Timeout }}", volName}) + volInspect.WaitWithDefaultTimeout() + Expect(volInspect).Should(Exit(0)) + Expect(volInspect.OutputToString()).To(ContainSubstring("15")) + + volName2 := "testVolume2" + create2 := podmanTest.Podman([]string{"volume", "create", "--driver", pluginName, "--opt", "o=timeout=3", volName2}) + create2.WaitWithDefaultTimeout() + Expect(create2).Should(Exit(0)) + + volInspect2 := podmanTest.Podman([]string{"volume", "inspect", "--format", "{{ .Timeout }}", volName2}) + volInspect2.WaitWithDefaultTimeout() + Expect(volInspect2).Should(Exit(0)) + Expect(volInspect2.OutputToString()).To(ContainSubstring("3")) + }) }) diff --git a/test/testvol/util.go b/test/testvol/util.go index b50bb3afb..b4961e097 100644 --- a/test/testvol/util.go +++ b/test/testvol/util.go @@ -25,5 +25,5 @@ func getPluginName(pathOrName string) string { func getPlugin(sockNameOrPath string) (*plugin.VolumePlugin, error) { path := getSocketPath(sockNameOrPath) name := getPluginName(sockNameOrPath) - return plugin.GetVolumePlugin(name, path, 0) + return plugin.GetVolumePlugin(name, path, nil, nil) } diff --git a/vendor/github.com/Microsoft/hcsshim/internal/jobobject/iocp.go b/vendor/github.com/Microsoft/hcsshim/internal/jobobject/iocp.go index 3d640ac7b..5d6acd69e 100644 --- a/vendor/github.com/Microsoft/hcsshim/internal/jobobject/iocp.go +++ b/vendor/github.com/Microsoft/hcsshim/internal/jobobject/iocp.go @@ -57,7 +57,7 @@ func pollIOCP(ctx context.Context, iocpHandle windows.Handle) { }).Warn("failed to parse job object message") continue } - if err := msq.Write(notification); err == queue.ErrQueueClosed { + if err := msq.Enqueue(notification); err == queue.ErrQueueClosed { // Write will only return an error when the queue is closed. // The only time a queue would ever be closed is when we call `Close` on // the job it belongs to which also removes it from the jobMap, so something diff --git a/vendor/github.com/Microsoft/hcsshim/internal/jobobject/jobobject.go b/vendor/github.com/Microsoft/hcsshim/internal/jobobject/jobobject.go index 9c2726416..c9fdd921a 100644 --- a/vendor/github.com/Microsoft/hcsshim/internal/jobobject/jobobject.go +++ b/vendor/github.com/Microsoft/hcsshim/internal/jobobject/jobobject.go @@ -68,6 +68,9 @@ type Options struct { // `UseNTVariant` specifies if we should use the `Nt` variant of Open/CreateJobObject. // Defaults to false. UseNTVariant bool + // `IOTracking` enables tracking I/O statistics on the job object. More specifically this + // calls SetInformationJobObject with the JobObjectIoAttribution class. + EnableIOTracking bool } // Create creates a job object. @@ -134,6 +137,12 @@ func Create(ctx context.Context, options *Options) (_ *JobObject, err error) { job.mq = mq } + if options.EnableIOTracking { + if err := enableIOTracking(jobHandle); err != nil { + return nil, err + } + } + return job, nil } @@ -235,7 +244,7 @@ func (job *JobObject) PollNotification() (interface{}, error) { if job.mq == nil { return nil, ErrNotRegistered } - return job.mq.ReadOrWait() + return job.mq.Dequeue() } // UpdateProcThreadAttribute updates the passed in ProcThreadAttributeList to contain what is necessary to @@ -330,7 +339,7 @@ func (job *JobObject) Pids() ([]uint32, error) { err := winapi.QueryInformationJobObject( job.handle, winapi.JobObjectBasicProcessIdList, - uintptr(unsafe.Pointer(&info)), + unsafe.Pointer(&info), uint32(unsafe.Sizeof(info)), nil, ) @@ -356,7 +365,7 @@ func (job *JobObject) Pids() ([]uint32, error) { if err = winapi.QueryInformationJobObject( job.handle, winapi.JobObjectBasicProcessIdList, - uintptr(unsafe.Pointer(&buf[0])), + unsafe.Pointer(&buf[0]), uint32(len(buf)), nil, ); err != nil { @@ -384,7 +393,7 @@ func (job *JobObject) QueryMemoryStats() (*winapi.JOBOBJECT_MEMORY_USAGE_INFORMA if err := winapi.QueryInformationJobObject( job.handle, winapi.JobObjectMemoryUsageInformation, - uintptr(unsafe.Pointer(&info)), + unsafe.Pointer(&info), uint32(unsafe.Sizeof(info)), nil, ); err != nil { @@ -406,7 +415,7 @@ func (job *JobObject) QueryProcessorStats() (*winapi.JOBOBJECT_BASIC_ACCOUNTING_ if err := winapi.QueryInformationJobObject( job.handle, winapi.JobObjectBasicAccountingInformation, - uintptr(unsafe.Pointer(&info)), + unsafe.Pointer(&info), uint32(unsafe.Sizeof(info)), nil, ); err != nil { @@ -415,7 +424,9 @@ func (job *JobObject) QueryProcessorStats() (*winapi.JOBOBJECT_BASIC_ACCOUNTING_ return &info, nil } -// QueryStorageStats gets the storage (I/O) stats for the job object. +// QueryStorageStats gets the storage (I/O) stats for the job object. This call will error +// if either `EnableIOTracking` wasn't set to true on creation of the job, or SetIOTracking() +// hasn't been called since creation of the job. func (job *JobObject) QueryStorageStats() (*winapi.JOBOBJECT_IO_ATTRIBUTION_INFORMATION, error) { job.handleLock.RLock() defer job.handleLock.RUnlock() @@ -430,7 +441,7 @@ func (job *JobObject) QueryStorageStats() (*winapi.JOBOBJECT_IO_ATTRIBUTION_INFO if err := winapi.QueryInformationJobObject( job.handle, winapi.JobObjectIoAttribution, - uintptr(unsafe.Pointer(&info)), + unsafe.Pointer(&info), uint32(unsafe.Sizeof(info)), nil, ); err != nil { @@ -476,7 +487,7 @@ func (job *JobObject) QueryPrivateWorkingSet() (uint64, error) { status := winapi.NtQueryInformationProcess( h, winapi.ProcessVmCounters, - uintptr(unsafe.Pointer(&vmCounters)), + unsafe.Pointer(&vmCounters), uint32(unsafe.Sizeof(vmCounters)), nil, ) @@ -497,3 +508,31 @@ func (job *JobObject) QueryPrivateWorkingSet() (uint64, error) { return jobWorkingSetSize, nil } + +// SetIOTracking enables IO tracking for processes in the job object. +// This enables use of the QueryStorageStats method. +func (job *JobObject) SetIOTracking() error { + job.handleLock.RLock() + defer job.handleLock.RUnlock() + + if job.handle == 0 { + return ErrAlreadyClosed + } + + return enableIOTracking(job.handle) +} + +func enableIOTracking(job windows.Handle) error { + info := winapi.JOBOBJECT_IO_ATTRIBUTION_INFORMATION{ + ControlFlags: winapi.JOBOBJECT_IO_ATTRIBUTION_CONTROL_ENABLE, + } + if _, err := windows.SetInformationJobObject( + job, + winapi.JobObjectIoAttribution, + uintptr(unsafe.Pointer(&info)), + uint32(unsafe.Sizeof(info)), + ); err != nil { + return fmt.Errorf("failed to enable IO tracking on job object: %w", err) + } + return nil +} diff --git a/vendor/github.com/Microsoft/hcsshim/internal/jobobject/limits.go b/vendor/github.com/Microsoft/hcsshim/internal/jobobject/limits.go index 4be297788..4efde292c 100644 --- a/vendor/github.com/Microsoft/hcsshim/internal/jobobject/limits.go +++ b/vendor/github.com/Microsoft/hcsshim/internal/jobobject/limits.go @@ -202,7 +202,7 @@ func (job *JobObject) getExtendedInformation() (*windows.JOBOBJECT_EXTENDED_LIMI if err := winapi.QueryInformationJobObject( job.handle, windows.JobObjectExtendedLimitInformation, - uintptr(unsafe.Pointer(&info)), + unsafe.Pointer(&info), uint32(unsafe.Sizeof(info)), nil, ); err != nil { @@ -224,7 +224,7 @@ func (job *JobObject) getCPURateControlInformation() (*winapi.JOBOBJECT_CPU_RATE if err := winapi.QueryInformationJobObject( job.handle, windows.JobObjectCpuRateControlInformation, - uintptr(unsafe.Pointer(&info)), + unsafe.Pointer(&info), uint32(unsafe.Sizeof(info)), nil, ); err != nil { diff --git a/vendor/github.com/Microsoft/hcsshim/internal/queue/mq.go b/vendor/github.com/Microsoft/hcsshim/internal/queue/mq.go index e177c9a62..4eb9bb9f1 100644 --- a/vendor/github.com/Microsoft/hcsshim/internal/queue/mq.go +++ b/vendor/github.com/Microsoft/hcsshim/internal/queue/mq.go @@ -5,10 +5,7 @@ import ( "sync" ) -var ( - ErrQueueClosed = errors.New("the queue is closed for reading and writing") - ErrQueueEmpty = errors.New("the queue is empty") -) +var ErrQueueClosed = errors.New("the queue is closed for reading and writing") // MessageQueue represents a threadsafe message queue to be used to retrieve or // write messages to. @@ -29,8 +26,8 @@ func NewMessageQueue() *MessageQueue { } } -// Write writes `msg` to the queue. -func (mq *MessageQueue) Write(msg interface{}) error { +// Enqueue writes `msg` to the queue. +func (mq *MessageQueue) Enqueue(msg interface{}) error { mq.m.Lock() defer mq.m.Unlock() @@ -43,55 +40,37 @@ func (mq *MessageQueue) Write(msg interface{}) error { return nil } -// Read will read a value from the queue if available, otherwise return an error. -func (mq *MessageQueue) Read() (interface{}, error) { +// Dequeue will read a value from the queue and remove it. If the queue +// is empty, this will block until the queue is closed or a value gets enqueued. +func (mq *MessageQueue) Dequeue() (interface{}, error) { mq.m.Lock() defer mq.m.Unlock() - if mq.closed { - return nil, ErrQueueClosed - } - if mq.isEmpty() { - return nil, ErrQueueEmpty + + for !mq.closed && mq.size() == 0 { + mq.c.Wait() } - val := mq.messages[0] - mq.messages[0] = nil - mq.messages = mq.messages[1:] - return val, nil -} -// ReadOrWait will read a value from the queue if available, else it will wait for a -// value to become available. This will block forever if nothing gets written or until -// the queue gets closed. -func (mq *MessageQueue) ReadOrWait() (interface{}, error) { - mq.m.Lock() + // We got woken up, check if it's because the queue got closed. if mq.closed { - mq.m.Unlock() return nil, ErrQueueClosed } - if mq.isEmpty() { - for !mq.closed && mq.isEmpty() { - mq.c.Wait() - } - mq.m.Unlock() - return mq.Read() - } + val := mq.messages[0] mq.messages[0] = nil mq.messages = mq.messages[1:] - mq.m.Unlock() return val, nil } -// IsEmpty returns if the queue is empty -func (mq *MessageQueue) IsEmpty() bool { +// Size returns the size of the queue. +func (mq *MessageQueue) Size() int { mq.m.RLock() defer mq.m.RUnlock() - return len(mq.messages) == 0 + return mq.size() } -// Nonexported empty check that doesn't lock so we can call this in Read and Write. -func (mq *MessageQueue) isEmpty() bool { - return len(mq.messages) == 0 +// Nonexported size check to check if the queue is empty inside already locked functions. +func (mq *MessageQueue) size() int { + return len(mq.messages) } // Close closes the queue for future writes or reads. Any attempts to read or write from the @@ -99,13 +78,15 @@ func (mq *MessageQueue) isEmpty() bool { func (mq *MessageQueue) Close() { mq.m.Lock() defer mq.m.Unlock() - // Already closed + + // Already closed, noop if mq.closed { return } + mq.messages = nil mq.closed = true - // If there's anybody currently waiting on a value from ReadOrWait, we need to + // If there's anybody currently waiting on a value from Dequeue, we need to // broadcast so the read(s) can return ErrQueueClosed. mq.c.Broadcast() } diff --git a/vendor/github.com/Microsoft/hcsshim/internal/winapi/jobobject.go b/vendor/github.com/Microsoft/hcsshim/internal/winapi/jobobject.go index 479649db3..7eb13f8f0 100644 --- a/vendor/github.com/Microsoft/hcsshim/internal/winapi/jobobject.go +++ b/vendor/github.com/Microsoft/hcsshim/internal/winapi/jobobject.go @@ -175,7 +175,7 @@ type JOBOBJECT_ASSOCIATE_COMPLETION_PORT struct { // LPDWORD lpReturnLength // ); // -//sys QueryInformationJobObject(jobHandle windows.Handle, infoClass uint32, jobObjectInfo uintptr, jobObjectInformationLength uint32, lpReturnLength *uint32) (err error) = kernel32.QueryInformationJobObject +//sys QueryInformationJobObject(jobHandle windows.Handle, infoClass uint32, jobObjectInfo unsafe.Pointer, jobObjectInformationLength uint32, lpReturnLength *uint32) (err error) = kernel32.QueryInformationJobObject // HANDLE OpenJobObjectW( // DWORD dwDesiredAccess, diff --git a/vendor/github.com/Microsoft/hcsshim/internal/winapi/process.go b/vendor/github.com/Microsoft/hcsshim/internal/winapi/process.go index 5f9e03fd2..222529f43 100644 --- a/vendor/github.com/Microsoft/hcsshim/internal/winapi/process.go +++ b/vendor/github.com/Microsoft/hcsshim/internal/winapi/process.go @@ -18,7 +18,7 @@ const ProcessVmCounters = 3 // [out, optional] PULONG ReturnLength // ); // -//sys NtQueryInformationProcess(processHandle windows.Handle, processInfoClass uint32, processInfo uintptr, processInfoLength uint32, returnLength *uint32) (status uint32) = ntdll.NtQueryInformationProcess +//sys NtQueryInformationProcess(processHandle windows.Handle, processInfoClass uint32, processInfo unsafe.Pointer, processInfoLength uint32, returnLength *uint32) (status uint32) = ntdll.NtQueryInformationProcess // typedef struct _VM_COUNTERS_EX // { diff --git a/vendor/github.com/Microsoft/hcsshim/internal/winapi/system.go b/vendor/github.com/Microsoft/hcsshim/internal/winapi/system.go index 327f57d7c..78fe01a4b 100644 --- a/vendor/github.com/Microsoft/hcsshim/internal/winapi/system.go +++ b/vendor/github.com/Microsoft/hcsshim/internal/winapi/system.go @@ -12,7 +12,8 @@ const STATUS_INFO_LENGTH_MISMATCH = 0xC0000004 // ULONG SystemInformationLength, // PULONG ReturnLength // ); -//sys NtQuerySystemInformation(systemInfoClass int, systemInformation uintptr, systemInfoLength uint32, returnLength *uint32) (status uint32) = ntdll.NtQuerySystemInformation +// +//sys NtQuerySystemInformation(systemInfoClass int, systemInformation unsafe.Pointer, systemInfoLength uint32, returnLength *uint32) (status uint32) = ntdll.NtQuerySystemInformation type SYSTEM_PROCESS_INFORMATION struct { NextEntryOffset uint32 // ULONG diff --git a/vendor/github.com/Microsoft/hcsshim/internal/winapi/zsyscall_windows.go b/vendor/github.com/Microsoft/hcsshim/internal/winapi/zsyscall_windows.go index 39fb3e1ad..1f16cf0b8 100644 --- a/vendor/github.com/Microsoft/hcsshim/internal/winapi/zsyscall_windows.go +++ b/vendor/github.com/Microsoft/hcsshim/internal/winapi/zsyscall_windows.go @@ -100,7 +100,7 @@ func resizePseudoConsole(hPc windows.Handle, size uint32) (hr error) { return } -func NtQuerySystemInformation(systemInfoClass int, systemInformation uintptr, systemInfoLength uint32, returnLength *uint32) (status uint32) { +func NtQuerySystemInformation(systemInfoClass int, systemInformation unsafe.Pointer, systemInfoLength uint32, returnLength *uint32) (status uint32) { r0, _, _ := syscall.Syscall6(procNtQuerySystemInformation.Addr(), 4, uintptr(systemInfoClass), uintptr(systemInformation), uintptr(systemInfoLength), uintptr(unsafe.Pointer(returnLength)), 0, 0) status = uint32(r0) return @@ -152,7 +152,7 @@ func IsProcessInJob(procHandle windows.Handle, jobHandle windows.Handle, result return } -func QueryInformationJobObject(jobHandle windows.Handle, infoClass uint32, jobObjectInfo uintptr, jobObjectInformationLength uint32, lpReturnLength *uint32) (err error) { +func QueryInformationJobObject(jobHandle windows.Handle, infoClass uint32, jobObjectInfo unsafe.Pointer, jobObjectInformationLength uint32, lpReturnLength *uint32) (err error) { r1, _, e1 := syscall.Syscall6(procQueryInformationJobObject.Addr(), 5, uintptr(jobHandle), uintptr(infoClass), uintptr(jobObjectInfo), uintptr(jobObjectInformationLength), uintptr(unsafe.Pointer(lpReturnLength)), 0) if r1 == 0 { if e1 != 0 { @@ -244,7 +244,7 @@ func LocalFree(ptr uintptr) { return } -func NtQueryInformationProcess(processHandle windows.Handle, processInfoClass uint32, processInfo uintptr, processInfoLength uint32, returnLength *uint32) (status uint32) { +func NtQueryInformationProcess(processHandle windows.Handle, processInfoClass uint32, processInfo unsafe.Pointer, processInfoLength uint32, returnLength *uint32) (status uint32) { r0, _, _ := syscall.Syscall6(procNtQueryInformationProcess.Addr(), 5, uintptr(processHandle), uintptr(processInfoClass), uintptr(processInfo), uintptr(processInfoLength), uintptr(unsafe.Pointer(returnLength)), 0) status = uint32(r0) return diff --git a/vendor/github.com/containers/common/libimage/inspect.go b/vendor/github.com/containers/common/libimage/inspect.go index 5da8df1bf..c6632d9a2 100644 --- a/vendor/github.com/containers/common/libimage/inspect.go +++ b/vendor/github.com/containers/common/libimage/inspect.go @@ -190,7 +190,7 @@ func (i *Image) Inspect(ctx context.Context, options *InspectOptions) (*ImageDat // NOTE: Health checks may be listed in the container config or // the config. data.HealthCheck = dockerManifest.ContainerConfig.Healthcheck - if data.HealthCheck == nil { + if data.HealthCheck == nil && dockerManifest.Config != nil { data.HealthCheck = dockerManifest.Config.Healthcheck } } diff --git a/vendor/github.com/containers/common/libimage/load.go b/vendor/github.com/containers/common/libimage/load.go index 89faa4635..593eef04b 100644 --- a/vendor/github.com/containers/common/libimage/load.go +++ b/vendor/github.com/containers/common/libimage/load.go @@ -99,7 +99,7 @@ func (r *Runtime) Load(ctx context.Context, path string, options *LoadOptions) ( } // loadMultiImageDockerArchive loads the docker archive specified by ref. In -// case the path@reference notation was used, only the specifiec image will be +// case the path@reference notation was used, only the specified image will be // loaded. Otherwise, all images will be loaded. func (r *Runtime) loadMultiImageDockerArchive(ctx context.Context, ref types.ImageReference, options *CopyOptions) ([]string, error) { // If we cannot stat the path, it either does not exist OR the correct diff --git a/vendor/github.com/containers/common/libnetwork/cni/network.go b/vendor/github.com/containers/common/libnetwork/cni/network.go index fce8f0066..11f1bbe14 100644 --- a/vendor/github.com/containers/common/libnetwork/cni/network.go +++ b/vendor/github.com/containers/common/libnetwork/cni/network.go @@ -19,6 +19,7 @@ import ( "github.com/containers/common/pkg/config" "github.com/containers/storage/pkg/lockfile" "github.com/sirupsen/logrus" + "golang.org/x/sys/unix" ) type cniNetwork struct { @@ -62,6 +63,8 @@ type InitConfig struct { CNIConfigDir string // CNIPluginDirs is a list of directories where cni should look for the plugins. CNIPluginDirs []string + // RunDir is a directory where temporary files can be stored. + RunDir string // DefaultNetwork is the name for the default network. DefaultNetwork string @@ -81,7 +84,16 @@ func NewCNINetworkInterface(conf *InitConfig) (types.ContainerNetwork, error) { // TODO: consider using a shared memory lock lock, err := lockfile.GetLockfile(filepath.Join(conf.CNIConfigDir, "cni.lock")) if err != nil { - return nil, err + // If we're on a read-only filesystem, there is no risk of + // contention. Fall back to a local lockfile. + if errors.Is(err, unix.EROFS) { + lock, err = lockfile.GetLockfile(filepath.Join(conf.RunDir, "cni.lock")) + if err != nil { + return nil, err + } + } else { + return nil, err + } } defaultNetworkName := conf.DefaultNetwork diff --git a/vendor/github.com/containers/common/libnetwork/network/interface.go b/vendor/github.com/containers/common/libnetwork/network/interface.go index 639ff4e45..545655fd3 100644 --- a/vendor/github.com/containers/common/libnetwork/network/interface.go +++ b/vendor/github.com/containers/common/libnetwork/network/interface.go @@ -169,6 +169,7 @@ func getCniInterface(conf *config.Config) (types.ContainerNetwork, error) { return cni.NewCNINetworkInterface(&cni.InitConfig{ CNIConfigDir: confDir, CNIPluginDirs: conf.Network.CNIPluginDirs, + RunDir: conf.Engine.TmpDir, DefaultNetwork: conf.Network.DefaultNetwork, DefaultSubnet: conf.Network.DefaultSubnet, DefaultsubnetPools: conf.Network.DefaultSubnetPools, diff --git a/vendor/github.com/containers/common/pkg/config/config.go b/vendor/github.com/containers/common/pkg/config/config.go index de1d91ae3..858f961b6 100644 --- a/vendor/github.com/containers/common/pkg/config/config.go +++ b/vendor/github.com/containers/common/pkg/config/config.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "sort" "strings" "sync" @@ -27,6 +28,8 @@ const ( _configPath = "containers/containers.conf" // UserOverrideContainersConfig holds the containers config path overridden by the rootless user UserOverrideContainersConfig = ".config/" + _configPath + // Token prefix for looking for helper binary under $BINDIR + bindirPrefix = "$BINDIR" ) // RuntimeStateStore is a constant indicating which state store implementation @@ -454,6 +457,13 @@ type EngineConfig struct { // may not be by other drivers. VolumePath string `toml:"volume_path,omitempty"` + // VolumePluginTimeout sets the default timeout, in seconds, for + // operations that must contact a volume plugin. Plugins are external + // programs accessed via REST API; this sets a timeout for requests to + // that API. + // A value of 0 is treated as no timeout. + VolumePluginTimeout uint `toml:"volume_plugin_timeout,omitempty,omitzero"` + // VolumePlugins is a set of plugins that can be used as the backend for // Podman named volumes. Each volume is specified as a name (what Podman // will refer to the plugin as) mapped to a path, which must point to a @@ -815,6 +825,18 @@ func (c *Config) Validate() error { return nil } +// URI returns the URI Path to the machine image +func (m *MachineConfig) URI() string { + uri := m.Image + for _, val := range []string{"$ARCH", "$arch"} { + uri = strings.Replace(uri, val, runtime.GOARCH, 1) + } + for _, val := range []string{"$OS", "$os"} { + uri = strings.Replace(uri, val, runtime.GOOS, 1) + } + return uri +} + func (c *EngineConfig) findRuntime() string { // Search for crun first followed by runc, kata, runsc for _, name := range []string{"crun", "runc", "runj", "kata", "runsc"} { @@ -1241,10 +1263,37 @@ func (c *Config) ActiveDestination() (uri, identity string, err error) { return "", "", errors.New("no service destination configured") } +var ( + bindirFailed = false + bindirCached = "" +) + +func findBindir() string { + if bindirCached != "" || bindirFailed { + return bindirCached + } + execPath, err := os.Executable() + if err == nil { + // Resolve symbolic links to find the actual binary file path. + execPath, err = filepath.EvalSymlinks(execPath) + } + if err != nil { + // If failed to find executable (unlikely to happen), warn about it. + // The bindirFailed flag will track this, so we only warn once. + logrus.Warnf("Failed to find $BINDIR: %v", err) + bindirFailed = true + return "" + } + bindirCached = filepath.Dir(execPath) + return bindirCached +} + // FindHelperBinary will search the given binary name in the configured directories. // If searchPATH is set to true it will also search in $PATH. func (c *Config) FindHelperBinary(name string, searchPATH bool) (string, error) { dirList := c.Engine.HelperBinariesDir + bindirPath := "" + bindirSearched := false // If set, search this directory first. This is used in testing. if dir, found := os.LookupEnv("CONTAINERS_HELPER_BINARY_DIR"); found { @@ -1252,6 +1301,24 @@ func (c *Config) FindHelperBinary(name string, searchPATH bool) (string, error) } for _, path := range dirList { + if path == bindirPrefix || strings.HasPrefix(path, bindirPrefix+string(filepath.Separator)) { + // Calculate the path to the executable first time we encounter a $BINDIR prefix. + if !bindirSearched { + bindirSearched = true + bindirPath = findBindir() + } + // If there's an error, don't stop the search for the helper binary. + // findBindir() will have warned once during the first failure. + if bindirPath == "" { + continue + } + // Replace the $BINDIR prefix with the path to the directory of the current binary. + if path == bindirPrefix { + path = bindirPath + } else { + path = filepath.Join(bindirPath, strings.TrimPrefix(path, bindirPrefix+string(filepath.Separator))) + } + } fullpath := filepath.Join(path, name) if fi, err := os.Stat(fullpath); err == nil && fi.Mode().IsRegular() { return fullpath, nil diff --git a/vendor/github.com/containers/common/pkg/config/config_darwin.go b/vendor/github.com/containers/common/pkg/config/config_darwin.go index 0ab9e0294..5283665e1 100644 --- a/vendor/github.com/containers/common/pkg/config/config_darwin.go +++ b/vendor/github.com/containers/common/pkg/config/config_darwin.go @@ -35,4 +35,6 @@ var defaultHelperBinariesDir = []string{ "/usr/local/lib/podman", "/usr/libexec/podman", "/usr/lib/podman", + // Relative to the binary directory + "$BINDIR/../libexec/podman", } diff --git a/vendor/github.com/containers/common/pkg/config/containers.conf b/vendor/github.com/containers/common/pkg/config/containers.conf index d1ac7c0e8..5b5aaa00a 100644 --- a/vendor/github.com/containers/common/pkg/config/containers.conf +++ b/vendor/github.com/containers/common/pkg/config/containers.conf @@ -605,6 +605,12 @@ default_sysctls = [ # #volume_path = "/var/lib/containers/storage/volumes" +# Default timeout (in seconds) for volume plugin operations. +# Plugins are external programs accessed via a REST API; this sets a timeout +# for requests to that API. +# A value of 0 is treated as no timeout. +#volume_plugin_timeout = 5 + # Paths to look for a valid OCI runtime (crun, runc, kata, runsc, krun, etc) [engine.runtimes] #crun = [ @@ -665,9 +671,16 @@ default_sysctls = [ # #disk_size=10 -# The image used when creating a podman-machine VM. +# Default image URI when creating a new VM using `podman machine init`. +# Options: On Linux/Mac, `testing`, `stable`, `next`. On Windows, the major +# version of the OS (e.g `36`) for Fedora 36. For all platforms you can +# alternatively specify a custom download URL to an image. Container engines +# translate URIs $OS and $ARCH to the native OS and ARCH. URI +# "https://example.com/$OS/$ARCH/foobar.ami" becomes +# "https://example.com/linux/amd64/foobar.ami" on a Linux AMD machine. +# The default value is `testing`. # -#image = "testing" +# image = "testing" # Memory in MB a machine is created with. # diff --git a/vendor/github.com/containers/common/pkg/config/default.go b/vendor/github.com/containers/common/pkg/config/default.go index 6bca7312a..b0d62779b 100644 --- a/vendor/github.com/containers/common/pkg/config/default.go +++ b/vendor/github.com/containers/common/pkg/config/default.go @@ -168,6 +168,8 @@ const ( SeccompOverridePath = _etcDir + "/containers/seccomp.json" // SeccompDefaultPath defines the default seccomp path. SeccompDefaultPath = _installPrefix + "/share/containers/seccomp.json" + // DefaultVolumePluginTimeout is the default volume plugin timeout, in seconds + DefaultVolumePluginTimeout = 5 ) // DefaultConfig defines the default values from containers.conf. @@ -264,7 +266,7 @@ func defaultMachineConfig() MachineConfig { Image: getDefaultMachineImage(), Memory: 2048, User: getDefaultMachineUser(), - Volumes: []string{"$HOME:$HOME"}, + Volumes: getDefaultMachineVolumes(), } } @@ -304,6 +306,8 @@ func defaultConfigFromMemory() (*EngineConfig, error) { c.StaticDir = filepath.Join(storeOpts.GraphRoot, "libpod") c.VolumePath = filepath.Join(storeOpts.GraphRoot, "volumes") + c.VolumePluginTimeout = DefaultVolumePluginTimeout + c.HelperBinariesDir = defaultHelperBinariesDir if additionalHelperBinariesDir != "" { c.HelperBinariesDir = append(c.HelperBinariesDir, additionalHelperBinariesDir) diff --git a/vendor/github.com/containers/common/pkg/config/default_darwin.go b/vendor/github.com/containers/common/pkg/config/default_darwin.go index c502ea55e..5d857df4f 100644 --- a/vendor/github.com/containers/common/pkg/config/default_darwin.go +++ b/vendor/github.com/containers/common/pkg/config/default_darwin.go @@ -11,3 +11,8 @@ func getDefaultLockType() string { func getLibpodTmpDir() string { return "/run/libpod" } + +// getDefaultMachineVolumes returns default mounted volumes (possibly with env vars, which will be expanded) +func getDefaultMachineVolumes() []string { + return []string{"$HOME:$HOME"} +} diff --git a/vendor/github.com/containers/common/pkg/config/default_freebsd.go b/vendor/github.com/containers/common/pkg/config/default_freebsd.go index 8b10ac1f7..9c827dbfe 100644 --- a/vendor/github.com/containers/common/pkg/config/default_freebsd.go +++ b/vendor/github.com/containers/common/pkg/config/default_freebsd.go @@ -18,3 +18,8 @@ func getDefaultLockType() string { func getLibpodTmpDir() string { return "/var/run/libpod" } + +// getDefaultMachineVolumes returns default mounted volumes (possibly with env vars, which will be expanded) +func getDefaultMachineVolumes() []string { + return []string{"$HOME:$HOME"} +} diff --git a/vendor/github.com/containers/common/pkg/config/default_linux.go b/vendor/github.com/containers/common/pkg/config/default_linux.go index 86873beb1..15052c10e 100644 --- a/vendor/github.com/containers/common/pkg/config/default_linux.go +++ b/vendor/github.com/containers/common/pkg/config/default_linux.go @@ -70,3 +70,8 @@ func getDefaultLockType() string { func getLibpodTmpDir() string { return "/run/libpod" } + +// getDefaultMachineVolumes returns default mounted volumes (possibly with env vars, which will be expanded) +func getDefaultMachineVolumes() []string { + return []string{"$HOME:$HOME"} +} diff --git a/vendor/github.com/containers/common/pkg/config/default_windows.go b/vendor/github.com/containers/common/pkg/config/default_windows.go index 1ff88fc42..08a0bf223 100644 --- a/vendor/github.com/containers/common/pkg/config/default_windows.go +++ b/vendor/github.com/containers/common/pkg/config/default_windows.go @@ -44,3 +44,8 @@ func getDefaultLockType() string { func getLibpodTmpDir() string { return "/run/libpod" } + +// getDefaultMachineVolumes returns default mounted volumes (possibly with env vars, which will be expanded) +func getDefaultMachineVolumes() []string { + return []string{} +} diff --git a/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go b/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go index ff82b5a39..02b6dfb09 100644 --- a/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go +++ b/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go @@ -372,7 +372,7 @@ func mountExists(mounts []rspec.Mount, dest string) bool { return false } -// resolveSymbolicLink resolves a possbile symlink path. If the path is a symlink, returns resolved +// resolveSymbolicLink resolves symlink paths. If the path is a symlink, returns resolved // path; if not, returns the original path. func resolveSymbolicLink(path string) (string, error) { info, err := os.Lstat(path) diff --git a/vendor/modules.txt b/vendor/modules.txt index eb9c7a34d..170f8fb98 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -11,7 +11,7 @@ github.com/Microsoft/go-winio/backuptar github.com/Microsoft/go-winio/pkg/guid github.com/Microsoft/go-winio/pkg/security github.com/Microsoft/go-winio/vhd -# github.com/Microsoft/hcsshim v0.9.3 +# github.com/Microsoft/hcsshim v0.9.4 github.com/Microsoft/hcsshim github.com/Microsoft/hcsshim/computestorage github.com/Microsoft/hcsshim/internal/cow @@ -67,7 +67,7 @@ github.com/container-orchestrated-devices/container-device-interface/pkg/cdi github.com/container-orchestrated-devices/container-device-interface/specs-go # github.com/containerd/cgroups v1.0.3 github.com/containerd/cgroups/stats/v1 -# github.com/containerd/containerd v1.6.6 +# github.com/containerd/containerd v1.6.8 github.com/containerd/containerd/errdefs github.com/containerd/containerd/log github.com/containerd/containerd/pkg/userns @@ -114,7 +114,7 @@ github.com/containers/buildah/pkg/rusage github.com/containers/buildah/pkg/sshagent github.com/containers/buildah/pkg/util github.com/containers/buildah/util -# github.com/containers/common v0.49.2-0.20220817132854-f6679f170eca +# github.com/containers/common v0.49.2-0.20220823130605-72a7da3358ac ## explicit github.com/containers/common/libimage github.com/containers/common/libimage/define -- cgit v1.2.3-54-g00ecf From b4584ea8546be227c74174130ec3a04a93996d28 Mon Sep 17 00:00:00 2001 From: Aditya R Date: Wed, 24 Aug 2022 11:22:33 +0530 Subject: run,create: add support for --env-merge for preprocessing vars Allow end users to preprocess default environment variables before injecting them into container using `--env-merge` Usage ``` podman run -it --rm --env-merge some=${some}-edit --env-merge some2=${some2}-edit2 myimage sh ``` Closes: https://github.com/containers/podman/issues/15288 Signed-off-by: Aditya R --- cmd/podman/common/create.go | 8 ++++++++ docs/source/markdown/options/env-merge.md | 5 +++++ docs/source/markdown/podman-create.1.md.in | 2 ++ docs/source/markdown/podman-run.1.md.in | 2 ++ go.mod | 1 + pkg/api/handlers/compat/containers_create.go | 1 + pkg/api/handlers/types.go | 1 + pkg/domain/entities/pods.go | 1 + pkg/specgen/generate/container.go | 12 ++++++++++++ pkg/specgen/specgen.go | 3 +++ pkg/specgenutil/specgen.go | 3 +++ test/e2e/run_env_test.go | 11 +++++++++++ vendor/modules.txt | 1 + 13 files changed, 51 insertions(+) create mode 100644 docs/source/markdown/options/env-merge.md (limited to 'pkg') diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index 00873b95b..1e573cc2d 100644 --- a/cmd/podman/common/create.go +++ b/cmd/podman/common/create.go @@ -124,6 +124,14 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions, "This is a Docker specific option and is a NOOP", ) + envMergeFlagName := "env-merge" + createFlags.StringArrayVar( + &cf.EnvMerge, + envMergeFlagName, []string{}, + "Preprocess environment variables from image before injecting them into the container", + ) + _ = cmd.RegisterFlagCompletionFunc(envMergeFlagName, completion.AutocompleteNone) + envFlagName := "env" createFlags.StringArrayP( envFlagName, "e", Env(), diff --git a/docs/source/markdown/options/env-merge.md b/docs/source/markdown/options/env-merge.md new file mode 100644 index 000000000..aa1aa003d --- /dev/null +++ b/docs/source/markdown/options/env-merge.md @@ -0,0 +1,5 @@ +#### **--env-merge**=*env* + +Preprocess default environment variables for the containers. For example +if image contains environment variable `hello=world` user can preprocess +it using `--env-merge hello=${hello}-some` so new value will be `hello=world-some`. diff --git a/docs/source/markdown/podman-create.1.md.in b/docs/source/markdown/podman-create.1.md.in index 9518568bb..0e65e7e3a 100644 --- a/docs/source/markdown/podman-create.1.md.in +++ b/docs/source/markdown/podman-create.1.md.in @@ -208,6 +208,8 @@ Read in a line delimited file of environment variables. See **Environment** note @@option env-host +@@option env-merge + @@option expose #### **--gidmap**=*container_gid:host_gid:amount* diff --git a/docs/source/markdown/podman-run.1.md.in b/docs/source/markdown/podman-run.1.md.in index 3e31ccc45..98d57a9c1 100644 --- a/docs/source/markdown/podman-run.1.md.in +++ b/docs/source/markdown/podman-run.1.md.in @@ -243,6 +243,8 @@ Read in a line delimited file of environment variables. See **Environment** note @@option env-host +@@option env-merge + @@option expose #### **--gidmap**=*container_gid:host_gid:amount* diff --git a/go.mod b/go.mod index 635c0a17d..a694e6add 100644 --- a/go.mod +++ b/go.mod @@ -49,6 +49,7 @@ require ( github.com/opencontainers/runtime-spec v1.0.3-0.20211214071223-8958f93039ab github.com/opencontainers/runtime-tools v0.9.1-0.20220714195903-17b3287fafb7 github.com/opencontainers/selinux v1.10.1 + github.com/openshift/imagebuilder v1.2.4-0.20220711175835-4151e43600df github.com/rootless-containers/rootlesskit v1.0.1 github.com/sirupsen/logrus v1.9.0 github.com/spf13/cobra v1.5.0 diff --git a/pkg/api/handlers/compat/containers_create.go b/pkg/api/handlers/compat/containers_create.go index 9fff8b4c8..d4f5d5f36 100644 --- a/pkg/api/handlers/compat/containers_create.go +++ b/pkg/api/handlers/compat/containers_create.go @@ -408,6 +408,7 @@ func cliOpts(cc handlers.CreateContainerConfig, rtc *config.Config) (*entities.C Systemd: "true", // podman default TmpFS: parsedTmp, TTY: cc.Config.Tty, + EnvMerge: cc.EnvMerge, UnsetEnv: cc.UnsetEnv, UnsetEnvAll: cc.UnsetEnvAll, User: cc.Config.User, diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go index b533e131c..ebbc5f63a 100644 --- a/pkg/api/handlers/types.go +++ b/pkg/api/handlers/types.go @@ -127,6 +127,7 @@ type CreateContainerConfig struct { dockerContainer.Config // desired container configuration HostConfig dockerContainer.HostConfig // host dependent configuration for container NetworkingConfig dockerNetwork.NetworkingConfig // network configuration for container + EnvMerge []string // preprocess env variables from image before injecting into containers UnsetEnv []string // unset specified default environment variables UnsetEnvAll bool // unset all default environment variables } diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go index 14ce370c1..33ca2c807 100644 --- a/pkg/domain/entities/pods.go +++ b/pkg/domain/entities/pods.go @@ -263,6 +263,7 @@ type ContainerCreateOptions struct { TTY bool Timezone string Umask string + EnvMerge []string UnsetEnv []string UnsetEnvAll bool UIDMap []string diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go index 85cd8f5ca..e293ce010 100644 --- a/pkg/specgen/generate/container.go +++ b/pkg/specgen/generate/container.go @@ -19,6 +19,7 @@ import ( "github.com/containers/podman/v4/pkg/signal" "github.com/containers/podman/v4/pkg/specgen" spec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/openshift/imagebuilder" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -131,6 +132,17 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat defaultEnvs = envLib.Join(envLib.DefaultEnvVariables(), envLib.Join(defaultEnvs, envs)) } + for _, e := range s.EnvMerge { + processedWord, err := imagebuilder.ProcessWord(e, envLib.Slice(defaultEnvs)) + if err != nil { + return nil, fmt.Errorf("unable to process variables for --env-merge %s: %w", e, err) + } + splitWord := strings.Split(processedWord, "=") + if _, ok := defaultEnvs[splitWord[0]]; ok { + defaultEnvs[splitWord[0]] = splitWord[1] + } + } + for _, e := range s.UnsetEnv { delete(defaultEnvs, e) } diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go index b90f07ef8..51b6736a9 100644 --- a/pkg/specgen/specgen.go +++ b/pkg/specgen/specgen.go @@ -204,6 +204,9 @@ type ContainerBasicConfig struct { // The execution domain system allows Linux to provide limited support // for binaries compiled under other UNIX-like operating systems. Personality *spec.LinuxPersonality `json:"personality,omitempty"` + // EnvMerge takes the specified environment variables from image and preprocess them before injecting them into the + // container. + EnvMerge []string `json:"envmerge,omitempty"` // UnsetEnv unsets the specified default environment variables from the image or from buildin or containers.conf // Optional. UnsetEnv []string `json:"unsetenv,omitempty"` diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go index 7392e7b44..aab2eebd5 100644 --- a/pkg/specgenutil/specgen.go +++ b/pkg/specgenutil/specgen.go @@ -839,6 +839,9 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions if !s.Volatile { s.Volatile = c.Rm } + if len(s.EnvMerge) == 0 || len(c.EnvMerge) != 0 { + s.EnvMerge = c.EnvMerge + } if len(s.UnsetEnv) == 0 || len(c.UnsetEnv) != 0 { s.UnsetEnv = c.UnsetEnv } diff --git a/test/e2e/run_env_test.go b/test/e2e/run_env_test.go index bab52efc5..9e78e150a 100644 --- a/test/e2e/run_env_test.go +++ b/test/e2e/run_env_test.go @@ -82,6 +82,17 @@ var _ = Describe("Podman run", func() { Expect(session.OutputToString()).To(ContainSubstring("HOSTNAME")) }) + It("podman run with --env-merge", func() { + dockerfile := `FROM quay.io/libpod/alpine:latest +ENV hello=world +` + podmanTest.BuildImage(dockerfile, "test", "false") + session := podmanTest.Podman([]string{"run", "--rm", "--env-merge", "hello=${hello}-earth", "test", "env"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(ContainSubstring("world-earth")) + }) + It("podman run --env-host environment test", func() { env := append(os.Environ(), "FOO=BAR") session := podmanTest.PodmanAsUser([]string{"run", "--rm", "--env-host", ALPINE, "/bin/printenv", "FOO"}, 0, 0, "", env) diff --git a/vendor/modules.txt b/vendor/modules.txt index eb9c7a34d..feb9f00d5 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -620,6 +620,7 @@ github.com/opencontainers/selinux/go-selinux/label github.com/opencontainers/selinux/pkg/pwalk github.com/opencontainers/selinux/pkg/pwalkdir # github.com/openshift/imagebuilder v1.2.4-0.20220711175835-4151e43600df +## explicit github.com/openshift/imagebuilder github.com/openshift/imagebuilder/dockerfile/command github.com/openshift/imagebuilder/dockerfile/parser -- cgit v1.2.3-54-g00ecf From 1788b26c431f461698d33f230ccfc8a3aa10033b Mon Sep 17 00:00:00 2001 From: Arthur Sengileyev Date: Tue, 23 Aug 2022 17:09:14 +0300 Subject: Fixes isRootfull check using qemu machine on Windows Signed-off-by: Arthur Sengileyev --- pkg/machine/qemu/claim_unsupported.go | 4 ++-- pkg/machine/qemu/config.go | 3 --- pkg/machine/qemu/machine.go | 14 +++++++++++--- 3 files changed, 13 insertions(+), 8 deletions(-) (limited to 'pkg') diff --git a/pkg/machine/qemu/claim_unsupported.go b/pkg/machine/qemu/claim_unsupported.go index e0b3dd3d3..187ef9d69 100644 --- a/pkg/machine/qemu/claim_unsupported.go +++ b/pkg/machine/qemu/claim_unsupported.go @@ -1,5 +1,5 @@ -//go:build !darwin && !windows -// +build !darwin,!windows +//go:build !darwin +// +build !darwin package qemu diff --git a/pkg/machine/qemu/config.go b/pkg/machine/qemu/config.go index bada1af9b..8081727f6 100644 --- a/pkg/machine/qemu/config.go +++ b/pkg/machine/qemu/config.go @@ -1,6 +1,3 @@ -//go:build (amd64 && !windows) || (arm64 && !windows) -// +build amd64,!windows arm64,!windows - package qemu import ( diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 213f7ce5d..20e3a3bb1 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -870,7 +870,7 @@ func NewQMPMonitor(network, name string, timeout time.Duration) (Monitor, error) if err != nil { return Monitor{}, err } - if !rootless.IsRootless() { + if isRootful() { rtDir = "/run" } rtDir = filepath.Join(rtDir, "podman") @@ -1371,7 +1371,7 @@ func (v *MachineVM) setPIDSocket() error { if err != nil { return err } - if !rootless.IsRootless() { + if isRootful() { rtPath = "/run" } socketDir := filepath.Join(rtPath, "podman") @@ -1397,7 +1397,7 @@ func (v *MachineVM) getSocketandPid() (string, string, error) { if err != nil { return "", "", err } - if !rootless.IsRootless() { + if isRootful() { rtPath = "/run" } socketDir := filepath.Join(rtPath, "podman") @@ -1735,3 +1735,11 @@ func isProcessAlive(pid int) bool { func (p *Provider) VMType() string { return vmtype } + +func isRootful() bool { + // Rootless is not relevant on Windows. In the future rootless.IsRootless + // could be switched to return true on Windows, and other codepaths migrated + // for now will check additionally for valid os.Getuid + + return !rootless.IsRootless() && os.Getuid() != -1 +} -- cgit v1.2.3-54-g00ecf From df1d8d0e9353528708a87609b932ae6b833c7ec0 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 19:57:43 +0200 Subject: Remove commented out code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can always recover it from git, but it seems to serve no purpose anyway. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/domain/infra/abi/trust.go | 4 ---- 1 file changed, 4 deletions(-) (limited to 'pkg') diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index 0e3d8fad9..cefe76da7 100644 --- a/pkg/domain/infra/abi/trust.go +++ b/pkg/domain/infra/abi/trust.go @@ -142,16 +142,12 @@ func getPolicyShowOutput(policyContentStruct trust.PolicyContent, systemRegistri Transport: transport, Type: trustTypeDescription(repoval[0].Type), } - // TODO - keyarr is not used and I don't know its intent; commenting out for now for someone to fix later - // keyarr := []string{} uids := []string{} for _, repoele := range repoval { if len(repoele.KeyPath) > 0 { - // keyarr = append(keyarr, repoele.KeyPath) uids = append(uids, trust.GetGPGIdFromKeyPath(repoele.KeyPath)...) } if len(repoele.KeyData) > 0 { - // keyarr = append(keyarr, string(repoele.KeyData)) uids = append(uids, trust.GetGPGIdFromKeyData(repoele.KeyData)...) } } -- cgit v1.2.3-54-g00ecf From 1d2def8d062588c46d066d54bba4284f72fec334 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 22:34:23 +0200 Subject: Remove an unused trust.ShowOutput type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 8 -------- 1 file changed, 8 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 663a1b5e2..28dac1ab6 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -53,14 +53,6 @@ type RegistryNamespace struct { SigStoreStaging string `json:"sigstore-staging"` // For writing only. } -// ShowOutput keep the fields for image trust show command -type ShowOutput struct { - Repo string - Trusttype string - GPGid string - Sigstore string -} - // systemRegistriesDirPath is the path to registries.d. const systemRegistriesDirPath = "/etc/containers/registries.d" -- cgit v1.2.3-54-g00ecf From 5be00f2270d4da30b0bffb15fca47f0325a963e2 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 21:06:29 +0200 Subject: Reorganize pkg/trust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the existing code into policy.go and registries.go, depending on which files it concerns. Only moves unchanged code, should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/config.go | 12 --- pkg/trust/policy.go | 125 +++++++++++++++++++++++++ pkg/trust/registries.go | 121 ++++++++++++++++++++++++ pkg/trust/trust.go | 241 ++---------------------------------------------- 4 files changed, 255 insertions(+), 244 deletions(-) delete mode 100644 pkg/trust/config.go create mode 100644 pkg/trust/policy.go create mode 100644 pkg/trust/registries.go (limited to 'pkg') diff --git a/pkg/trust/config.go b/pkg/trust/config.go deleted file mode 100644 index 6186d4cbd..000000000 --- a/pkg/trust/config.go +++ /dev/null @@ -1,12 +0,0 @@ -package trust - -// Policy describes a basic trust policy configuration -type Policy struct { - Transport string `json:"transport"` - Name string `json:"name,omitempty"` - RepoName string `json:"repo_name,omitempty"` - Keys []string `json:"keys,omitempty"` - SignatureStore string `json:"sigstore,omitempty"` - Type string `json:"type"` - GPGId string `json:"gpg_id,omitempty"` -} diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go new file mode 100644 index 000000000..9c44f7da2 --- /dev/null +++ b/pkg/trust/policy.go @@ -0,0 +1,125 @@ +package trust + +import ( + "bufio" + "bytes" + "encoding/base64" + "encoding/json" + "fmt" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/containers/image/v5/types" + "github.com/sirupsen/logrus" +) + +// PolicyContent struct for policy.json file +type PolicyContent struct { + Default []RepoContent `json:"default"` + Transports TransportsContent `json:"transports,omitempty"` +} + +// RepoContent struct used under each repo +type RepoContent struct { + Type string `json:"type"` + KeyType string `json:"keyType,omitempty"` + KeyPath string `json:"keyPath,omitempty"` + KeyData string `json:"keyData,omitempty"` + SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` +} + +// RepoMap map repo name to policycontent for each repo +type RepoMap map[string][]RepoContent + +// TransportsContent struct for content under "transports" +type TransportsContent map[string]RepoMap + +// DefaultPolicyPath returns a path to the default policy of the system. +func DefaultPolicyPath(sys *types.SystemContext) string { + systemDefaultPolicyPath := "/etc/containers/policy.json" + if sys != nil { + if sys.SignaturePolicyPath != "" { + return sys.SignaturePolicyPath + } + if sys.RootForImplicitAbsolutePaths != "" { + return filepath.Join(sys.RootForImplicitAbsolutePaths, systemDefaultPolicyPath) + } + } + return systemDefaultPolicyPath +} + +// CreateTmpFile creates a temp file under dir and writes the content into it +func CreateTmpFile(dir, pattern string, content []byte) (string, error) { + tmpfile, err := ioutil.TempFile(dir, pattern) + if err != nil { + return "", err + } + defer tmpfile.Close() + + if _, err := tmpfile.Write(content); err != nil { + return "", err + } + return tmpfile.Name(), nil +} + +// GetGPGIdFromKeyPath return user keyring from key path +func GetGPGIdFromKeyPath(path string) []string { + cmd := exec.Command("gpg2", "--with-colons", path) + results, err := cmd.Output() + if err != nil { + logrus.Errorf("Getting key identity: %s", err) + return nil + } + return parseUids(results) +} + +// GetGPGIdFromKeyData return user keyring from keydata +func GetGPGIdFromKeyData(key string) []string { + decodeKey, err := base64.StdEncoding.DecodeString(key) + if err != nil { + logrus.Errorf("%s, error decoding key data", err) + return nil + } + tmpfileName, err := CreateTmpFile("", "", decodeKey) + if err != nil { + logrus.Errorf("Creating key date temp file %s", err) + } + defer os.Remove(tmpfileName) + return GetGPGIdFromKeyPath(tmpfileName) +} + +func parseUids(colonDelimitKeys []byte) []string { + var parseduids []string + scanner := bufio.NewScanner(bytes.NewReader(colonDelimitKeys)) + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, "uid:") || strings.HasPrefix(line, "pub:") { + uid := strings.Split(line, ":")[9] + if uid == "" { + continue + } + parseduid := uid + if strings.Contains(uid, "<") && strings.Contains(uid, ">") { + parseduid = strings.SplitN(strings.SplitAfterN(uid, "<", 2)[1], ">", 2)[0] + } + parseduids = append(parseduids, parseduid) + } + } + return parseduids +} + +// GetPolicy parse policy.json into PolicyContent struct +func GetPolicy(policyPath string) (PolicyContent, error) { + var policyContentStruct PolicyContent + policyContent, err := ioutil.ReadFile(policyPath) + if err != nil { + return policyContentStruct, fmt.Errorf("unable to read policy file: %w", err) + } + if err := json.Unmarshal(policyContent, &policyContentStruct); err != nil { + return policyContentStruct, fmt.Errorf("could not parse trust policies from %s: %w", policyPath, err) + } + return policyContentStruct, nil +} diff --git a/pkg/trust/registries.go b/pkg/trust/registries.go new file mode 100644 index 000000000..ba6ffe281 --- /dev/null +++ b/pkg/trust/registries.go @@ -0,0 +1,121 @@ +package trust + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "strings" + + "github.com/containers/image/v5/types" + "github.com/docker/docker/pkg/homedir" + "github.com/ghodss/yaml" +) + +// RegistryConfiguration is one of the files in registriesDirPath configuring lookaside locations, or the result of merging them all. +// NOTE: Keep this in sync with docs/registries.d.md! +type RegistryConfiguration struct { + DefaultDocker *RegistryNamespace `json:"default-docker"` + // The key is a namespace, using fully-expanded Docker reference format or parent namespaces (per dockerReference.PolicyConfiguration*), + Docker map[string]RegistryNamespace `json:"docker"` +} + +// RegistryNamespace defines lookaside locations for a single namespace. +type RegistryNamespace struct { + SigStore string `json:"sigstore"` // For reading, and if SigStoreStaging is not present, for writing. + SigStoreStaging string `json:"sigstore-staging"` // For writing only. +} + +// systemRegistriesDirPath is the path to registries.d. +const systemRegistriesDirPath = "/etc/containers/registries.d" + +// userRegistriesDir is the path to the per user registries.d. +var userRegistriesDir = filepath.FromSlash(".config/containers/registries.d") + +// RegistriesDirPath returns a path to registries.d +func RegistriesDirPath(sys *types.SystemContext) string { + if sys != nil && sys.RegistriesDirPath != "" { + return sys.RegistriesDirPath + } + userRegistriesDirPath := filepath.Join(homedir.Get(), userRegistriesDir) + if _, err := os.Stat(userRegistriesDirPath); err == nil { + return userRegistriesDirPath + } + if sys != nil && sys.RootForImplicitAbsolutePaths != "" { + return filepath.Join(sys.RootForImplicitAbsolutePaths, systemRegistriesDirPath) + } + + return systemRegistriesDirPath +} + +// LoadAndMergeConfig loads configuration files in dirPath +func LoadAndMergeConfig(dirPath string) (*RegistryConfiguration, error) { + mergedConfig := RegistryConfiguration{Docker: map[string]RegistryNamespace{}} + dockerDefaultMergedFrom := "" + nsMergedFrom := map[string]string{} + + dir, err := os.Open(dirPath) + if err != nil { + if os.IsNotExist(err) { + return &mergedConfig, nil + } + return nil, err + } + configNames, err := dir.Readdirnames(0) + if err != nil { + return nil, err + } + for _, configName := range configNames { + if !strings.HasSuffix(configName, ".yaml") { + continue + } + configPath := filepath.Join(dirPath, configName) + configBytes, err := ioutil.ReadFile(configPath) + if err != nil { + return nil, err + } + var config RegistryConfiguration + err = yaml.Unmarshal(configBytes, &config) + if err != nil { + return nil, fmt.Errorf("error parsing %s: %w", configPath, err) + } + if config.DefaultDocker != nil { + if mergedConfig.DefaultDocker != nil { + return nil, fmt.Errorf(`error parsing signature storage configuration: "default-docker" defined both in "%s" and "%s"`, + dockerDefaultMergedFrom, configPath) + } + mergedConfig.DefaultDocker = config.DefaultDocker + dockerDefaultMergedFrom = configPath + } + for nsName, nsConfig := range config.Docker { // includes config.Docker == nil + if _, ok := mergedConfig.Docker[nsName]; ok { + return nil, fmt.Errorf(`error parsing signature storage configuration: "docker" namespace "%s" defined both in "%s" and "%s"`, + nsName, nsMergedFrom[nsName], configPath) + } + mergedConfig.Docker[nsName] = nsConfig + nsMergedFrom[nsName] = configPath + } + } + return &mergedConfig, nil +} + +// HaveMatchRegistry checks if trust settings for the registry have been configured in yaml file +func HaveMatchRegistry(key string, registryConfigs *RegistryConfiguration) *RegistryNamespace { + searchKey := key + if !strings.Contains(searchKey, "/") { + val, exists := registryConfigs.Docker[searchKey] + if exists { + return &val + } + } + for range strings.Split(key, "/") { + val, exists := registryConfigs.Docker[searchKey] + if exists { + return &val + } + if strings.Contains(searchKey, "/") { + searchKey = searchKey[:strings.LastIndex(searchKey, "/")] + } + } + return registryConfigs.DefaultDocker +} diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 28dac1ab6..6186d4cbd 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -1,235 +1,12 @@ package trust -import ( - "bufio" - "bytes" - "encoding/base64" - "encoding/json" - "fmt" - "io/ioutil" - "os" - "os/exec" - "path/filepath" - "strings" - - "github.com/containers/image/v5/types" - "github.com/docker/docker/pkg/homedir" - "github.com/ghodss/yaml" - "github.com/sirupsen/logrus" -) - -// PolicyContent struct for policy.json file -type PolicyContent struct { - Default []RepoContent `json:"default"` - Transports TransportsContent `json:"transports,omitempty"` -} - -// RepoContent struct used under each repo -type RepoContent struct { - Type string `json:"type"` - KeyType string `json:"keyType,omitempty"` - KeyPath string `json:"keyPath,omitempty"` - KeyData string `json:"keyData,omitempty"` - SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` -} - -// RepoMap map repo name to policycontent for each repo -type RepoMap map[string][]RepoContent - -// TransportsContent struct for content under "transports" -type TransportsContent map[string]RepoMap - -// RegistryConfiguration is one of the files in registriesDirPath configuring lookaside locations, or the result of merging them all. -// NOTE: Keep this in sync with docs/registries.d.md! -type RegistryConfiguration struct { - DefaultDocker *RegistryNamespace `json:"default-docker"` - // The key is a namespace, using fully-expanded Docker reference format or parent namespaces (per dockerReference.PolicyConfiguration*), - Docker map[string]RegistryNamespace `json:"docker"` -} - -// RegistryNamespace defines lookaside locations for a single namespace. -type RegistryNamespace struct { - SigStore string `json:"sigstore"` // For reading, and if SigStoreStaging is not present, for writing. - SigStoreStaging string `json:"sigstore-staging"` // For writing only. -} - -// systemRegistriesDirPath is the path to registries.d. -const systemRegistriesDirPath = "/etc/containers/registries.d" - -// userRegistriesDir is the path to the per user registries.d. -var userRegistriesDir = filepath.FromSlash(".config/containers/registries.d") - -// DefaultPolicyPath returns a path to the default policy of the system. -func DefaultPolicyPath(sys *types.SystemContext) string { - systemDefaultPolicyPath := "/etc/containers/policy.json" - if sys != nil { - if sys.SignaturePolicyPath != "" { - return sys.SignaturePolicyPath - } - if sys.RootForImplicitAbsolutePaths != "" { - return filepath.Join(sys.RootForImplicitAbsolutePaths, systemDefaultPolicyPath) - } - } - return systemDefaultPolicyPath -} - -// RegistriesDirPath returns a path to registries.d -func RegistriesDirPath(sys *types.SystemContext) string { - if sys != nil && sys.RegistriesDirPath != "" { - return sys.RegistriesDirPath - } - userRegistriesDirPath := filepath.Join(homedir.Get(), userRegistriesDir) - if _, err := os.Stat(userRegistriesDirPath); err == nil { - return userRegistriesDirPath - } - if sys != nil && sys.RootForImplicitAbsolutePaths != "" { - return filepath.Join(sys.RootForImplicitAbsolutePaths, systemRegistriesDirPath) - } - - return systemRegistriesDirPath -} - -// LoadAndMergeConfig loads configuration files in dirPath -func LoadAndMergeConfig(dirPath string) (*RegistryConfiguration, error) { - mergedConfig := RegistryConfiguration{Docker: map[string]RegistryNamespace{}} - dockerDefaultMergedFrom := "" - nsMergedFrom := map[string]string{} - - dir, err := os.Open(dirPath) - if err != nil { - if os.IsNotExist(err) { - return &mergedConfig, nil - } - return nil, err - } - configNames, err := dir.Readdirnames(0) - if err != nil { - return nil, err - } - for _, configName := range configNames { - if !strings.HasSuffix(configName, ".yaml") { - continue - } - configPath := filepath.Join(dirPath, configName) - configBytes, err := ioutil.ReadFile(configPath) - if err != nil { - return nil, err - } - var config RegistryConfiguration - err = yaml.Unmarshal(configBytes, &config) - if err != nil { - return nil, fmt.Errorf("error parsing %s: %w", configPath, err) - } - if config.DefaultDocker != nil { - if mergedConfig.DefaultDocker != nil { - return nil, fmt.Errorf(`error parsing signature storage configuration: "default-docker" defined both in "%s" and "%s"`, - dockerDefaultMergedFrom, configPath) - } - mergedConfig.DefaultDocker = config.DefaultDocker - dockerDefaultMergedFrom = configPath - } - for nsName, nsConfig := range config.Docker { // includes config.Docker == nil - if _, ok := mergedConfig.Docker[nsName]; ok { - return nil, fmt.Errorf(`error parsing signature storage configuration: "docker" namespace "%s" defined both in "%s" and "%s"`, - nsName, nsMergedFrom[nsName], configPath) - } - mergedConfig.Docker[nsName] = nsConfig - nsMergedFrom[nsName] = configPath - } - } - return &mergedConfig, nil -} - -// HaveMatchRegistry checks if trust settings for the registry have been configured in yaml file -func HaveMatchRegistry(key string, registryConfigs *RegistryConfiguration) *RegistryNamespace { - searchKey := key - if !strings.Contains(searchKey, "/") { - val, exists := registryConfigs.Docker[searchKey] - if exists { - return &val - } - } - for range strings.Split(key, "/") { - val, exists := registryConfigs.Docker[searchKey] - if exists { - return &val - } - if strings.Contains(searchKey, "/") { - searchKey = searchKey[:strings.LastIndex(searchKey, "/")] - } - } - return registryConfigs.DefaultDocker -} - -// CreateTmpFile creates a temp file under dir and writes the content into it -func CreateTmpFile(dir, pattern string, content []byte) (string, error) { - tmpfile, err := ioutil.TempFile(dir, pattern) - if err != nil { - return "", err - } - defer tmpfile.Close() - - if _, err := tmpfile.Write(content); err != nil { - return "", err - } - return tmpfile.Name(), nil -} - -// GetGPGIdFromKeyPath return user keyring from key path -func GetGPGIdFromKeyPath(path string) []string { - cmd := exec.Command("gpg2", "--with-colons", path) - results, err := cmd.Output() - if err != nil { - logrus.Errorf("Getting key identity: %s", err) - return nil - } - return parseUids(results) -} - -// GetGPGIdFromKeyData return user keyring from keydata -func GetGPGIdFromKeyData(key string) []string { - decodeKey, err := base64.StdEncoding.DecodeString(key) - if err != nil { - logrus.Errorf("%s, error decoding key data", err) - return nil - } - tmpfileName, err := CreateTmpFile("", "", decodeKey) - if err != nil { - logrus.Errorf("Creating key date temp file %s", err) - } - defer os.Remove(tmpfileName) - return GetGPGIdFromKeyPath(tmpfileName) -} - -func parseUids(colonDelimitKeys []byte) []string { - var parseduids []string - scanner := bufio.NewScanner(bytes.NewReader(colonDelimitKeys)) - for scanner.Scan() { - line := scanner.Text() - if strings.HasPrefix(line, "uid:") || strings.HasPrefix(line, "pub:") { - uid := strings.Split(line, ":")[9] - if uid == "" { - continue - } - parseduid := uid - if strings.Contains(uid, "<") && strings.Contains(uid, ">") { - parseduid = strings.SplitN(strings.SplitAfterN(uid, "<", 2)[1], ">", 2)[0] - } - parseduids = append(parseduids, parseduid) - } - } - return parseduids -} - -// GetPolicy parse policy.json into PolicyContent struct -func GetPolicy(policyPath string) (PolicyContent, error) { - var policyContentStruct PolicyContent - policyContent, err := ioutil.ReadFile(policyPath) - if err != nil { - return policyContentStruct, fmt.Errorf("unable to read policy file: %w", err) - } - if err := json.Unmarshal(policyContent, &policyContentStruct); err != nil { - return policyContentStruct, fmt.Errorf("could not parse trust policies from %s: %w", policyPath, err) - } - return policyContentStruct, nil +// Policy describes a basic trust policy configuration +type Policy struct { + Transport string `json:"transport"` + Name string `json:"name,omitempty"` + RepoName string `json:"repo_name,omitempty"` + Keys []string `json:"keys,omitempty"` + SignatureStore string `json:"sigstore,omitempty"` + Type string `json:"type"` + GPGId string `json:"gpg_id,omitempty"` } -- cgit v1.2.3-54-g00ecf From 4c5366ee0306777d164dfa6ce02c97e29849d9f2 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 21:08:41 +0200 Subject: Make trust.CreateTempFile private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing uses it outside the package. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 9c44f7da2..62950131d 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -51,8 +51,8 @@ func DefaultPolicyPath(sys *types.SystemContext) string { return systemDefaultPolicyPath } -// CreateTmpFile creates a temp file under dir and writes the content into it -func CreateTmpFile(dir, pattern string, content []byte) (string, error) { +// createTmpFile creates a temp file under dir and writes the content into it +func createTmpFile(dir, pattern string, content []byte) (string, error) { tmpfile, err := ioutil.TempFile(dir, pattern) if err != nil { return "", err @@ -83,7 +83,7 @@ func GetGPGIdFromKeyData(key string) []string { logrus.Errorf("%s, error decoding key data", err) return nil } - tmpfileName, err := CreateTmpFile("", "", decodeKey) + tmpfileName, err := createTmpFile("", "", decodeKey) if err != nil { logrus.Errorf("Creating key date temp file %s", err) } -- cgit v1.2.3-54-g00ecf From 4f68075306efb9381de3c5ea5762a3f843137b56 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 19:33:00 +0200 Subject: Add a variable for scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only process the incoming args[] (which is a single-element array for some reason) once, and use a semantic variable name for the value we care about. Should not change behavior, the only caller already supposedly ensures that len(args) == 1. Signed-off-by: Miloslav Trmač --- pkg/domain/infra/abi/trust.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'pkg') diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index cefe76da7..61bf03727 100644 --- a/pkg/domain/infra/abi/trust.go +++ b/pkg/domain/infra/abi/trust.go @@ -46,6 +46,11 @@ func (ir *ImageEngine) ShowTrust(ctx context.Context, args []string, options ent } func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options entities.SetTrustOptions) error { + if len(args) != 1 { + return fmt.Errorf("SetTrust called with unexpected %d args", len(args)) + } + scope := args[0] + var ( policyContentStruct trust.PolicyContent newReposContent []trust.RepoContent @@ -81,7 +86,7 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti } else { newReposContent = append(newReposContent, trust.RepoContent{Type: trustType}) } - if args[0] == "default" { + if scope == "default" { policyContentStruct.Default = newReposContent } else { if len(policyContentStruct.Default) == 0 { @@ -89,9 +94,9 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti } registryExists := false for transport, transportval := range policyContentStruct.Transports { - _, registryExists = transportval[args[0]] + _, registryExists = transportval[scope] if registryExists { - policyContentStruct.Transports[transport][args[0]] = newReposContent + policyContentStruct.Transports[transport][scope] = newReposContent break } } @@ -102,7 +107,7 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti if policyContentStruct.Transports["docker"] == nil { policyContentStruct.Transports["docker"] = make(map[string][]trust.RepoContent) } - policyContentStruct.Transports["docker"][args[0]] = append(policyContentStruct.Transports["docker"][args[0]], newReposContent...) + policyContentStruct.Transports["docker"][scope] = append(policyContentStruct.Transports["docker"][scope], newReposContent...) } } -- cgit v1.2.3-54-g00ecf From cbdbb025a3f6e6e5417cdade032075d679842056 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 21:39:14 +0200 Subject: Move most of imageEngine.SetTrust to pkg/trust.AddPolicyEntries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will allow us to write unit tests without setting up the complete Podman runtime (and without the Linux dependency). Also, actually add a basic smoke test of the core functionality. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/domain/infra/abi/trust.go | 68 +++------------------------------------ pkg/trust/policy.go | 74 +++++++++++++++++++++++++++++++++++++++++++ pkg/trust/policy_test.go | 71 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 63 deletions(-) create mode 100644 pkg/trust/policy_test.go (limited to 'pkg') diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index 61bf03727..381ea5deb 100644 --- a/pkg/domain/infra/abi/trust.go +++ b/pkg/domain/infra/abi/trust.go @@ -2,11 +2,8 @@ package abi import ( "context" - "encoding/json" - "errors" "fmt" "io/ioutil" - "os" "strings" "github.com/containers/podman/v4/pkg/domain/entities" @@ -51,71 +48,16 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti } scope := args[0] - var ( - policyContentStruct trust.PolicyContent - newReposContent []trust.RepoContent - ) - trustType := options.Type - if trustType == "accept" { - trustType = "insecureAcceptAnything" - } - - pubkeysfile := options.PubKeysFile - if len(pubkeysfile) == 0 && trustType == "signedBy" { - return errors.New("at least one public key must be defined for type 'signedBy'") - } - policyPath := trust.DefaultPolicyPath(ir.Libpod.SystemContext()) if len(options.PolicyPath) > 0 { policyPath = options.PolicyPath } - _, err := os.Stat(policyPath) - if !os.IsNotExist(err) { - policyContent, err := ioutil.ReadFile(policyPath) - if err != nil { - return err - } - if err := json.Unmarshal(policyContent, &policyContentStruct); err != nil { - return errors.New("could not read trust policies") - } - } - if len(pubkeysfile) != 0 { - for _, filepath := range pubkeysfile { - newReposContent = append(newReposContent, trust.RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) - } - } else { - newReposContent = append(newReposContent, trust.RepoContent{Type: trustType}) - } - if scope == "default" { - policyContentStruct.Default = newReposContent - } else { - if len(policyContentStruct.Default) == 0 { - return errors.New("default trust policy must be set") - } - registryExists := false - for transport, transportval := range policyContentStruct.Transports { - _, registryExists = transportval[scope] - if registryExists { - policyContentStruct.Transports[transport][scope] = newReposContent - break - } - } - if !registryExists { - if policyContentStruct.Transports == nil { - policyContentStruct.Transports = make(map[string]trust.RepoMap) - } - if policyContentStruct.Transports["docker"] == nil { - policyContentStruct.Transports["docker"] = make(map[string][]trust.RepoContent) - } - policyContentStruct.Transports["docker"][scope] = append(policyContentStruct.Transports["docker"][scope], newReposContent...) - } - } - data, err := json.MarshalIndent(policyContentStruct, "", " ") - if err != nil { - return fmt.Errorf("error setting trust policy: %w", err) - } - return ioutil.WriteFile(policyPath, data, 0644) + return trust.AddPolicyEntries(policyPath, trust.AddPolicyEntriesInput{ + Scope: scope, + Type: options.Type, + PubKeyFiles: options.PubKeysFile, + }) } func getPolicyShowOutput(policyContentStruct trust.PolicyContent, systemRegistriesDirPath string) ([]*trust.Policy, error) { diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 62950131d..352be781c 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -5,6 +5,7 @@ import ( "bytes" "encoding/base64" "encoding/json" + "errors" "fmt" "io/ioutil" "os" @@ -123,3 +124,76 @@ func GetPolicy(policyPath string) (PolicyContent, error) { } return policyContentStruct, nil } + +// AddPolicyEntriesInput collects some parameters to AddPolicyEntries, +// primarily so that the callers use named values instead of just strings in a sequence. +type AddPolicyEntriesInput struct { + Scope string // "default" or a docker/atomic scope name + Type string + PubKeyFiles []string // For signature enforcement types, paths to public keys files (where the image needs to be signed by at least one key from _each_ of the files). File format depends on Type. +} + +// AddPolicyEntries adds one or more policy entries necessary to implement AddPolicyEntriesInput. +func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { + var ( + policyContentStruct PolicyContent + newReposContent []RepoContent + ) + trustType := input.Type + if trustType == "accept" { + trustType = "insecureAcceptAnything" + } + + pubkeysfile := input.PubKeyFiles + if len(pubkeysfile) == 0 && trustType == "signedBy" { + return errors.New("at least one public key must be defined for type 'signedBy'") + } + + _, err := os.Stat(policyPath) + if !os.IsNotExist(err) { + policyContent, err := ioutil.ReadFile(policyPath) + if err != nil { + return err + } + if err := json.Unmarshal(policyContent, &policyContentStruct); err != nil { + return errors.New("could not read trust policies") + } + } + if len(pubkeysfile) != 0 { + for _, filepath := range pubkeysfile { + newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) + } + } else { + newReposContent = append(newReposContent, RepoContent{Type: trustType}) + } + if input.Scope == "default" { + policyContentStruct.Default = newReposContent + } else { + if len(policyContentStruct.Default) == 0 { + return errors.New("default trust policy must be set") + } + registryExists := false + for transport, transportval := range policyContentStruct.Transports { + _, registryExists = transportval[input.Scope] + if registryExists { + policyContentStruct.Transports[transport][input.Scope] = newReposContent + break + } + } + if !registryExists { + if policyContentStruct.Transports == nil { + policyContentStruct.Transports = make(map[string]RepoMap) + } + if policyContentStruct.Transports["docker"] == nil { + policyContentStruct.Transports["docker"] = make(map[string][]RepoContent) + } + policyContentStruct.Transports["docker"][input.Scope] = append(policyContentStruct.Transports["docker"][input.Scope], newReposContent...) + } + } + + data, err := json.MarshalIndent(policyContentStruct, "", " ") + if err != nil { + return fmt.Errorf("error setting trust policy: %w", err) + } + return ioutil.WriteFile(policyPath, data, 0644) +} diff --git a/pkg/trust/policy_test.go b/pkg/trust/policy_test.go new file mode 100644 index 000000000..1f2f585c8 --- /dev/null +++ b/pkg/trust/policy_test.go @@ -0,0 +1,71 @@ +package trust + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/containers/image/v5/signature" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAddPolicyEntries(t *testing.T) { + tempDir := t.TempDir() + policyPath := filepath.Join(tempDir, "policy.json") + + minimalPolicy := &signature.Policy{ + Default: []signature.PolicyRequirement{ + signature.NewPRInsecureAcceptAnything(), + }, + } + minimalPolicyJSON, err := json.Marshal(minimalPolicy) + require.NoError(t, err) + err = os.WriteFile(policyPath, minimalPolicyJSON, 0600) + require.NoError(t, err) + + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ + Scope: "default", + Type: "reject", + }) + assert.NoError(t, err) + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ + Scope: "quay.io/accepted", + Type: "accept", + }) + assert.NoError(t, err) + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ + Scope: "quay.io/multi-signed", + Type: "signedBy", + PubKeyFiles: []string{"/1.pub", "/2.pub"}, + }) + assert.NoError(t, err) + + // Test that the outcome is consumable, and compare it with the expected values. + parsedPolicy, err := signature.NewPolicyFromFile(policyPath) + require.NoError(t, err) + assert.Equal(t, &signature.Policy{ + Default: signature.PolicyRequirements{ + signature.NewPRReject(), + }, + Transports: map[string]signature.PolicyTransportScopes{ + "docker": { + "quay.io/accepted": { + signature.NewPRInsecureAcceptAnything(), + }, + "quay.io/multi-signed": { + xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + }, + }, + }, parsedPolicy) +} + +// xNewPRSignedByKeyPath is a wrapper for NewPRSignedByKeyPath which must not fail. +func xNewPRSignedByKeyPath(t *testing.T, keyPath string, signedIdentity signature.PolicyReferenceMatch) signature.PolicyRequirement { + pr, err := signature.NewPRSignedByKeyPath(signature.SBKeyTypeGPGKeys, keyPath, signedIdentity) + require.NoError(t, err) + return pr +} -- cgit v1.2.3-54-g00ecf From e2d1bdd1d8c10617818e5805330c54523580b647 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 19:39:11 +0200 Subject: Improve validation of data in ImageEngine.SetTrust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Also reject public keys with types that don't use them - Reject unknown trust types - And add unit tests Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 18 +++++++++++++++--- pkg/trust/policy_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 352be781c..df4f49ff1 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -143,10 +143,22 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { if trustType == "accept" { trustType = "insecureAcceptAnything" } - pubkeysfile := input.PubKeyFiles - if len(pubkeysfile) == 0 && trustType == "signedBy" { - return errors.New("at least one public key must be defined for type 'signedBy'") + + // The error messages in validation failures use input.Type instead of trustType to match the user’s input. + switch trustType { + case "insecureAcceptAnything", "reject": + if len(pubkeysfile) != 0 { + return fmt.Errorf("%d public keys unexpectedly provided for trust type %v", len(pubkeysfile), input.Type) + } + + case "signedBy": + if len(pubkeysfile) == 0 { + return errors.New("at least one public key must be defined for type 'signedBy'") + } + + default: + return fmt.Errorf("unknown trust type %q", input.Type) } _, err := os.Stat(policyPath) diff --git a/pkg/trust/policy_test.go b/pkg/trust/policy_test.go index 1f2f585c8..c4781335f 100644 --- a/pkg/trust/policy_test.go +++ b/pkg/trust/policy_test.go @@ -25,6 +25,38 @@ func TestAddPolicyEntries(t *testing.T) { err = os.WriteFile(policyPath, minimalPolicyJSON, 0600) require.NoError(t, err) + // Invalid input: + for _, invalid := range []AddPolicyEntriesInput{ + { + Scope: "default", + Type: "accept", + PubKeyFiles: []string{"/does-not-make-sense"}, + }, + { + Scope: "default", + Type: "insecureAcceptAnything", + PubKeyFiles: []string{"/does-not-make-sense"}, + }, + { + Scope: "default", + Type: "reject", + PubKeyFiles: []string{"/does-not-make-sense"}, + }, + { + Scope: "default", + Type: "signedBy", + PubKeyFiles: []string{}, // A key is missing + }, + { + Scope: "default", + Type: "this-is-unknown", + PubKeyFiles: []string{}, + }, + } { + err := AddPolicyEntries(policyPath, invalid) + assert.Error(t, err, "%#v", invalid) + } + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ Scope: "default", Type: "reject", -- cgit v1.2.3-54-g00ecf From 9828bc44534d6527d44351470d5f943281b7dfba Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 19:42:08 +0200 Subject: Create new policy entries together with validating input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That way, we don't have to switch over trustType twice. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index df4f49ff1..77e02a05c 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -151,11 +151,15 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { if len(pubkeysfile) != 0 { return fmt.Errorf("%d public keys unexpectedly provided for trust type %v", len(pubkeysfile), input.Type) } + newReposContent = append(newReposContent, RepoContent{Type: trustType}) case "signedBy": if len(pubkeysfile) == 0 { return errors.New("at least one public key must be defined for type 'signedBy'") } + for _, filepath := range pubkeysfile { + newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) + } default: return fmt.Errorf("unknown trust type %q", input.Type) @@ -171,13 +175,6 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { return errors.New("could not read trust policies") } } - if len(pubkeysfile) != 0 { - for _, filepath := range pubkeysfile { - newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) - } - } else { - newReposContent = append(newReposContent, RepoContent{Type: trustType}) - } if input.Scope == "default" { policyContentStruct.Default = newReposContent } else { -- cgit v1.2.3-54-g00ecf From ff3f574fc0db5e442adfac54b86af7c462595ffc Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 19:48:26 +0200 Subject: Add support for sigstoreSigned in (podman image trust set) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NOTE: This does not edit the use-sigstore-attachments value in registries.d, similarly to how (podman image trust set) didn't set the lookaside paths for simple signing. Signed-off-by: Miloslav Trmač --- cmd/podman/images/trust_set.go | 4 ++-- docs/source/markdown/podman-image-trust.1.md | 9 ++++++--- pkg/trust/policy.go | 8 ++++++++ pkg/trust/policy_test.go | 22 ++++++++++++++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/images/trust_set.go b/cmd/podman/images/trust_set.go index 832e9f724..e7339f0b1 100644 --- a/cmd/podman/images/trust_set.go +++ b/cmd/podman/images/trust_set.go @@ -53,7 +53,7 @@ File(s) must exist before using this command`) } func setTrust(cmd *cobra.Command, args []string) error { - validTrustTypes := []string{"accept", "insecureAcceptAnything", "reject", "signedBy"} + validTrustTypes := []string{"accept", "insecureAcceptAnything", "reject", "signedBy", "sigstoreSigned"} valid, err := isValidImageURI(args[0]) if err != nil || !valid { @@ -61,7 +61,7 @@ func setTrust(cmd *cobra.Command, args []string) error { } if !util.StringInSlice(setOptions.Type, validTrustTypes) { - return fmt.Errorf("invalid choice: %s (choose from 'accept', 'reject', 'signedBy')", setOptions.Type) + return fmt.Errorf("invalid choice: %s (choose from 'accept', 'reject', 'signedBy', 'sigstoreSigned')", setOptions.Type) } return registry.ImageEngine().SetTrust(registry.Context(), args, setOptions) } diff --git a/docs/source/markdown/podman-image-trust.1.md b/docs/source/markdown/podman-image-trust.1.md index 4e80bdcf5..2a7da82cc 100644 --- a/docs/source/markdown/podman-image-trust.1.md +++ b/docs/source/markdown/podman-image-trust.1.md @@ -32,7 +32,8 @@ Trust **type** provides a way to: Allowlist ("accept") or Denylist ("reject") registries or -Require signature (“signedBy”). +Require a simple signing signature (“signedBy”), +Require a sigstore signature ("sigstoreSigned"). Trust may be updated using the command **podman image trust set** for an existing trust scope. @@ -45,12 +46,14 @@ Trust may be updated using the command **podman image trust set** for an existin #### **--pubkeysfile**, **-f**=*KEY1* A path to an exported public key on the local system. Key paths will be referenced in policy.json. Any path to a file may be used but locating the file in **/etc/pki/containers** is recommended. Options may be used multiple times to - require an image be signed by multiple keys. The **--pubkeysfile** option is required for the **signedBy** type. + require an image be signed by multiple keys. The **--pubkeysfile** option is required for the **signedBy** and **sigstoreSigned** types. #### **--type**, **-t**=*value* The trust type for this policy entry. Accepted values: - **signedBy** (default): Require signatures with corresponding list of + **signedBy** (default): Require simple signing signatures with corresponding list of + public keys + **sigstoreSigned**: Require sigstore signatures with corresponding list of public keys **accept**: do not require any signatures for this registry scope diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 77e02a05c..3a31b9338 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -161,6 +161,14 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) } + case "sigstoreSigned": + if len(pubkeysfile) == 0 { + return errors.New("at least one public key must be defined for type 'sigstoreSigned'") + } + for _, filepath := range pubkeysfile { + newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyPath: filepath}) + } + default: return fmt.Errorf("unknown trust type %q", input.Type) } diff --git a/pkg/trust/policy_test.go b/pkg/trust/policy_test.go index c4781335f..c2c2d93be 100644 --- a/pkg/trust/policy_test.go +++ b/pkg/trust/policy_test.go @@ -47,6 +47,11 @@ func TestAddPolicyEntries(t *testing.T) { Type: "signedBy", PubKeyFiles: []string{}, // A key is missing }, + { + Scope: "default", + Type: "sigstoreSigned", + PubKeyFiles: []string{}, // A key is missing + }, { Scope: "default", Type: "this-is-unknown", @@ -73,6 +78,12 @@ func TestAddPolicyEntries(t *testing.T) { PubKeyFiles: []string{"/1.pub", "/2.pub"}, }) assert.NoError(t, err) + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ + Scope: "quay.io/sigstore-signed", + Type: "sigstoreSigned", + PubKeyFiles: []string{"/1.pub", "/2.pub"}, + }) + assert.NoError(t, err) // Test that the outcome is consumable, and compare it with the expected values. parsedPolicy, err := signature.NewPolicyFromFile(policyPath) @@ -90,6 +101,10 @@ func TestAddPolicyEntries(t *testing.T) { xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), }, + "quay.io/sigstore-signed": { + xNewPRSigstoreSignedKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSigstoreSignedKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, }, }, }, parsedPolicy) @@ -101,3 +116,10 @@ func xNewPRSignedByKeyPath(t *testing.T, keyPath string, signedIdentity signatur require.NoError(t, err) return pr } + +// xNewPRSigstoreSignedKeyPath is a wrapper for NewPRSigstoreSignedKeyPath which must not fail. +func xNewPRSigstoreSignedKeyPath(t *testing.T, keyPath string, signedIdentity signature.PolicyReferenceMatch) signature.PolicyRequirement { + pr, err := signature.NewPRSigstoreSignedKeyPath(keyPath, signedIdentity) + require.NoError(t, err) + return pr +} -- cgit v1.2.3-54-g00ecf From 7723a1ea654624b5cfcedc6d94e947169967c183 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 22:09:58 +0200 Subject: Move most of ImageEngine.ShowTrust into pkg/trust.PolicyDescription MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will allow us to write unit tests without setting up the complete Podman runtime (and without the Linux dependency). Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/domain/infra/abi/trust.go | 68 +------------------------------------------ pkg/trust/policy.go | 10 +++++++ pkg/trust/trust.go | 68 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 67 deletions(-) (limited to 'pkg') diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index 381ea5deb..c58ddff06 100644 --- a/pkg/domain/infra/abi/trust.go +++ b/pkg/domain/infra/abi/trust.go @@ -4,11 +4,9 @@ import ( "context" "fmt" "io/ioutil" - "strings" "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/trust" - "github.com/sirupsen/logrus" ) func (ir *ImageEngine) ShowTrust(ctx context.Context, args []string, options entities.ShowTrustOptions) (*entities.ShowTrustReport, error) { @@ -31,11 +29,7 @@ func (ir *ImageEngine) ShowTrust(ctx context.Context, args []string, options ent if len(options.RegistryPath) > 0 { report.SystemRegistriesDirPath = options.RegistryPath } - policyContentStruct, err := trust.GetPolicy(policyPath) - if err != nil { - return nil, fmt.Errorf("could not read trust policies: %w", err) - } - report.Policies, err = getPolicyShowOutput(policyContentStruct, report.SystemRegistriesDirPath) + report.Policies, err = trust.PolicyDescription(policyPath, report.SystemRegistriesDirPath) if err != nil { return nil, fmt.Errorf("could not show trust policies: %w", err) } @@ -59,63 +53,3 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti PubKeyFiles: options.PubKeysFile, }) } - -func getPolicyShowOutput(policyContentStruct trust.PolicyContent, systemRegistriesDirPath string) ([]*trust.Policy, error) { - var output []*trust.Policy - - registryConfigs, err := trust.LoadAndMergeConfig(systemRegistriesDirPath) - if err != nil { - return nil, err - } - - if len(policyContentStruct.Default) > 0 { - defaultPolicyStruct := trust.Policy{ - Transport: "all", - Name: "* (default)", - RepoName: "default", - Type: trustTypeDescription(policyContentStruct.Default[0].Type), - } - output = append(output, &defaultPolicyStruct) - } - for transport, transval := range policyContentStruct.Transports { - if transport == "docker" { - transport = "repository" - } - - for repo, repoval := range transval { - tempTrustShowOutput := trust.Policy{ - Name: repo, - RepoName: repo, - Transport: transport, - Type: trustTypeDescription(repoval[0].Type), - } - uids := []string{} - for _, repoele := range repoval { - if len(repoele.KeyPath) > 0 { - uids = append(uids, trust.GetGPGIdFromKeyPath(repoele.KeyPath)...) - } - if len(repoele.KeyData) > 0 { - uids = append(uids, trust.GetGPGIdFromKeyData(repoele.KeyData)...) - } - } - tempTrustShowOutput.GPGId = strings.Join(uids, ", ") - - registryNamespace := trust.HaveMatchRegistry(repo, registryConfigs) - if registryNamespace != nil { - tempTrustShowOutput.SignatureStore = registryNamespace.SigStore - } - output = append(output, &tempTrustShowOutput) - } - } - return output, nil -} - -var typeDescription = map[string]string{"insecureAcceptAnything": "accept", "signedBy": "signed", "reject": "reject"} - -func trustTypeDescription(trustType string) string { - trustDescription, exist := typeDescription[trustType] - if !exist { - logrus.Warnf("Invalid trust type %s", trustType) - } - return trustDescription -} diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 3a31b9338..0dc46eac3 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -125,6 +125,16 @@ func GetPolicy(policyPath string) (PolicyContent, error) { return policyContentStruct, nil } +var typeDescription = map[string]string{"insecureAcceptAnything": "accept", "signedBy": "signed", "reject": "reject"} + +func trustTypeDescription(trustType string) string { + trustDescription, exist := typeDescription[trustType] + if !exist { + logrus.Warnf("Invalid trust type %s", trustType) + } + return trustDescription +} + // AddPolicyEntriesInput collects some parameters to AddPolicyEntries, // primarily so that the callers use named values instead of just strings in a sequence. type AddPolicyEntriesInput struct { diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 6186d4cbd..2813b126d 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -1,5 +1,10 @@ package trust +import ( + "fmt" + "strings" +) + // Policy describes a basic trust policy configuration type Policy struct { Transport string `json:"transport"` @@ -10,3 +15,66 @@ type Policy struct { Type string `json:"type"` GPGId string `json:"gpg_id,omitempty"` } + +// PolicyDescription returns an user-focused description of the policy in policyPath and registries.d data from registriesDirPath. +func PolicyDescription(policyPath, registriesDirPath string) ([]*Policy, error) { + policyContentStruct, err := GetPolicy(policyPath) + if err != nil { + return nil, fmt.Errorf("could not read trust policies: %w", err) + } + res, err := getPolicyShowOutput(policyContentStruct, registriesDirPath) + if err != nil { + return nil, fmt.Errorf("could not show trust policies: %w", err) + } + return res, nil +} + +func getPolicyShowOutput(policyContentStruct PolicyContent, systemRegistriesDirPath string) ([]*Policy, error) { + var output []*Policy + + registryConfigs, err := LoadAndMergeConfig(systemRegistriesDirPath) + if err != nil { + return nil, err + } + + if len(policyContentStruct.Default) > 0 { + defaultPolicyStruct := Policy{ + Transport: "all", + Name: "* (default)", + RepoName: "default", + Type: trustTypeDescription(policyContentStruct.Default[0].Type), + } + output = append(output, &defaultPolicyStruct) + } + for transport, transval := range policyContentStruct.Transports { + if transport == "docker" { + transport = "repository" + } + + for repo, repoval := range transval { + tempTrustShowOutput := Policy{ + Name: repo, + RepoName: repo, + Transport: transport, + Type: trustTypeDescription(repoval[0].Type), + } + uids := []string{} + for _, repoele := range repoval { + if len(repoele.KeyPath) > 0 { + uids = append(uids, GetGPGIdFromKeyPath(repoele.KeyPath)...) + } + if len(repoele.KeyData) > 0 { + uids = append(uids, GetGPGIdFromKeyData(repoele.KeyData)...) + } + } + tempTrustShowOutput.GPGId = strings.Join(uids, ", ") + + registryNamespace := HaveMatchRegistry(repo, registryConfigs) + if registryNamespace != nil { + tempTrustShowOutput.SignatureStore = registryNamespace.SigStore + } + output = append(output, &tempTrustShowOutput) + } + } + return output, nil +} -- cgit v1.2.3-54-g00ecf From 35fa8c16a2e921ac7c45d6df3fa09a0ec6fdbbdd Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 22:36:38 +0200 Subject: Make most of pkg/trust package-private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now have only a few entrypoints that are called externally, so make the rest private. This will make it more obvious that we are not breaking any external users. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 51 +++++++++++++++++++++++++------------------------ pkg/trust/registries.go | 24 +++++++++++------------ pkg/trust/trust.go | 12 ++++++------ 3 files changed, 44 insertions(+), 43 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 0dc46eac3..d2b904b07 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -17,14 +17,15 @@ import ( "github.com/sirupsen/logrus" ) -// PolicyContent struct for policy.json file -type PolicyContent struct { - Default []RepoContent `json:"default"` - Transports TransportsContent `json:"transports,omitempty"` +// policyContent is the overall structure of a policy.json file (= c/image/v5/signature.Policy) +type policyContent struct { + Default []repoContent `json:"default"` + Transports transportsContent `json:"transports,omitempty"` } -// RepoContent struct used under each repo -type RepoContent struct { +// repoContent is a single policy requirement (one of possibly several for a scope), representing all of the individual alternatives in a single merged struct +// (= c/image/v5/signature.{PolicyRequirement,pr*}) +type repoContent struct { Type string `json:"type"` KeyType string `json:"keyType,omitempty"` KeyPath string `json:"keyPath,omitempty"` @@ -32,11 +33,11 @@ type RepoContent struct { SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` } -// RepoMap map repo name to policycontent for each repo -type RepoMap map[string][]RepoContent +// repoMap maps a scope name to requirements that apply to that scope (= c/image/v5/signature.PolicyTransportScopes) +type repoMap map[string][]repoContent -// TransportsContent struct for content under "transports" -type TransportsContent map[string]RepoMap +// transportsContent contains policies for individual transports (= c/image/v5/signature.Policy.Transports) +type transportsContent map[string]repoMap // DefaultPolicyPath returns a path to the default policy of the system. func DefaultPolicyPath(sys *types.SystemContext) string { @@ -66,8 +67,8 @@ func createTmpFile(dir, pattern string, content []byte) (string, error) { return tmpfile.Name(), nil } -// GetGPGIdFromKeyPath return user keyring from key path -func GetGPGIdFromKeyPath(path string) []string { +// getGPGIdFromKeyPath returns GPG key IDs of keys stored at the provided path. +func getGPGIdFromKeyPath(path string) []string { cmd := exec.Command("gpg2", "--with-colons", path) results, err := cmd.Output() if err != nil { @@ -77,8 +78,8 @@ func GetGPGIdFromKeyPath(path string) []string { return parseUids(results) } -// GetGPGIdFromKeyData return user keyring from keydata -func GetGPGIdFromKeyData(key string) []string { +// getGPGIdFromKeyData returns GPG key IDs of keys in the provided keyring. +func getGPGIdFromKeyData(key string) []string { decodeKey, err := base64.StdEncoding.DecodeString(key) if err != nil { logrus.Errorf("%s, error decoding key data", err) @@ -89,7 +90,7 @@ func GetGPGIdFromKeyData(key string) []string { logrus.Errorf("Creating key date temp file %s", err) } defer os.Remove(tmpfileName) - return GetGPGIdFromKeyPath(tmpfileName) + return getGPGIdFromKeyPath(tmpfileName) } func parseUids(colonDelimitKeys []byte) []string { @@ -112,9 +113,9 @@ func parseUids(colonDelimitKeys []byte) []string { return parseduids } -// GetPolicy parse policy.json into PolicyContent struct -func GetPolicy(policyPath string) (PolicyContent, error) { - var policyContentStruct PolicyContent +// getPolicy parses policy.json into policyContent. +func getPolicy(policyPath string) (policyContent, error) { + var policyContentStruct policyContent policyContent, err := ioutil.ReadFile(policyPath) if err != nil { return policyContentStruct, fmt.Errorf("unable to read policy file: %w", err) @@ -146,8 +147,8 @@ type AddPolicyEntriesInput struct { // AddPolicyEntries adds one or more policy entries necessary to implement AddPolicyEntriesInput. func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { var ( - policyContentStruct PolicyContent - newReposContent []RepoContent + policyContentStruct policyContent + newReposContent []repoContent ) trustType := input.Type if trustType == "accept" { @@ -161,14 +162,14 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { if len(pubkeysfile) != 0 { return fmt.Errorf("%d public keys unexpectedly provided for trust type %v", len(pubkeysfile), input.Type) } - newReposContent = append(newReposContent, RepoContent{Type: trustType}) + newReposContent = append(newReposContent, repoContent{Type: trustType}) case "signedBy": if len(pubkeysfile) == 0 { return errors.New("at least one public key must be defined for type 'signedBy'") } for _, filepath := range pubkeysfile { - newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) + newReposContent = append(newReposContent, repoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) } case "sigstoreSigned": @@ -176,7 +177,7 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { return errors.New("at least one public key must be defined for type 'sigstoreSigned'") } for _, filepath := range pubkeysfile { - newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyPath: filepath}) + newReposContent = append(newReposContent, repoContent{Type: trustType, KeyPath: filepath}) } default: @@ -209,10 +210,10 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { } if !registryExists { if policyContentStruct.Transports == nil { - policyContentStruct.Transports = make(map[string]RepoMap) + policyContentStruct.Transports = make(map[string]repoMap) } if policyContentStruct.Transports["docker"] == nil { - policyContentStruct.Transports["docker"] = make(map[string][]RepoContent) + policyContentStruct.Transports["docker"] = make(map[string][]repoContent) } policyContentStruct.Transports["docker"][input.Scope] = append(policyContentStruct.Transports["docker"][input.Scope], newReposContent...) } diff --git a/pkg/trust/registries.go b/pkg/trust/registries.go index ba6ffe281..da2e7eb42 100644 --- a/pkg/trust/registries.go +++ b/pkg/trust/registries.go @@ -12,16 +12,16 @@ import ( "github.com/ghodss/yaml" ) -// RegistryConfiguration is one of the files in registriesDirPath configuring lookaside locations, or the result of merging them all. +// registryConfiguration is one of the files in registriesDirPath configuring lookaside locations, or the result of merging them all. // NOTE: Keep this in sync with docs/registries.d.md! -type RegistryConfiguration struct { - DefaultDocker *RegistryNamespace `json:"default-docker"` +type registryConfiguration struct { + DefaultDocker *registryNamespace `json:"default-docker"` // The key is a namespace, using fully-expanded Docker reference format or parent namespaces (per dockerReference.PolicyConfiguration*), - Docker map[string]RegistryNamespace `json:"docker"` + Docker map[string]registryNamespace `json:"docker"` } -// RegistryNamespace defines lookaside locations for a single namespace. -type RegistryNamespace struct { +// registryNamespace defines lookaside locations for a single namespace. +type registryNamespace struct { SigStore string `json:"sigstore"` // For reading, and if SigStoreStaging is not present, for writing. SigStoreStaging string `json:"sigstore-staging"` // For writing only. } @@ -48,9 +48,9 @@ func RegistriesDirPath(sys *types.SystemContext) string { return systemRegistriesDirPath } -// LoadAndMergeConfig loads configuration files in dirPath -func LoadAndMergeConfig(dirPath string) (*RegistryConfiguration, error) { - mergedConfig := RegistryConfiguration{Docker: map[string]RegistryNamespace{}} +// loadAndMergeConfig loads registries.d configuration files in dirPath +func loadAndMergeConfig(dirPath string) (*registryConfiguration, error) { + mergedConfig := registryConfiguration{Docker: map[string]registryNamespace{}} dockerDefaultMergedFrom := "" nsMergedFrom := map[string]string{} @@ -74,7 +74,7 @@ func LoadAndMergeConfig(dirPath string) (*RegistryConfiguration, error) { if err != nil { return nil, err } - var config RegistryConfiguration + var config registryConfiguration err = yaml.Unmarshal(configBytes, &config) if err != nil { return nil, fmt.Errorf("error parsing %s: %w", configPath, err) @@ -99,8 +99,8 @@ func LoadAndMergeConfig(dirPath string) (*RegistryConfiguration, error) { return &mergedConfig, nil } -// HaveMatchRegistry checks if trust settings for the registry have been configured in yaml file -func HaveMatchRegistry(key string, registryConfigs *RegistryConfiguration) *RegistryNamespace { +// haveMatchRegistry returns configuration from registryConfigs that is configured for key. +func haveMatchRegistry(key string, registryConfigs *registryConfiguration) *registryNamespace { searchKey := key if !strings.Contains(searchKey, "/") { val, exists := registryConfigs.Docker[searchKey] diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 2813b126d..606e4ed93 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -18,7 +18,7 @@ type Policy struct { // PolicyDescription returns an user-focused description of the policy in policyPath and registries.d data from registriesDirPath. func PolicyDescription(policyPath, registriesDirPath string) ([]*Policy, error) { - policyContentStruct, err := GetPolicy(policyPath) + policyContentStruct, err := getPolicy(policyPath) if err != nil { return nil, fmt.Errorf("could not read trust policies: %w", err) } @@ -29,10 +29,10 @@ func PolicyDescription(policyPath, registriesDirPath string) ([]*Policy, error) return res, nil } -func getPolicyShowOutput(policyContentStruct PolicyContent, systemRegistriesDirPath string) ([]*Policy, error) { +func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirPath string) ([]*Policy, error) { var output []*Policy - registryConfigs, err := LoadAndMergeConfig(systemRegistriesDirPath) + registryConfigs, err := loadAndMergeConfig(systemRegistriesDirPath) if err != nil { return nil, err } @@ -61,15 +61,15 @@ func getPolicyShowOutput(policyContentStruct PolicyContent, systemRegistriesDirP uids := []string{} for _, repoele := range repoval { if len(repoele.KeyPath) > 0 { - uids = append(uids, GetGPGIdFromKeyPath(repoele.KeyPath)...) + uids = append(uids, getGPGIdFromKeyPath(repoele.KeyPath)...) } if len(repoele.KeyData) > 0 { - uids = append(uids, GetGPGIdFromKeyData(repoele.KeyData)...) + uids = append(uids, getGPGIdFromKeyData(repoele.KeyData)...) } } tempTrustShowOutput.GPGId = strings.Join(uids, ", ") - registryNamespace := HaveMatchRegistry(repo, registryConfigs) + registryNamespace := haveMatchRegistry(repo, registryConfigs) if registryNamespace != nil { tempTrustShowOutput.SignatureStore = registryNamespace.SigStore } -- cgit v1.2.3-54-g00ecf From 4b2bd1036b4952a35a526202c8965cd3b32162ad Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 22:56:14 +0200 Subject: Make the output of (podman image trust show) deterministic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sort map keys instead of iterating in the Go-imposed random order. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 606e4ed93..e93b4cd9d 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -2,6 +2,7 @@ package trust import ( "fmt" + "sort" "strings" ) @@ -46,12 +47,26 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP } output = append(output, &defaultPolicyStruct) } - for transport, transval := range policyContentStruct.Transports { + // FIXME: This should use x/exp/maps.Keys after we update to Go 1.18. + transports := []string{} + for t := range policyContentStruct.Transports { + transports = append(transports, t) + } + sort.Strings(transports) + for _, transport := range transports { + transval := policyContentStruct.Transports[transport] if transport == "docker" { transport = "repository" } - for repo, repoval := range transval { + // FIXME: This should use x/exp/maps.Keys after we update to Go 1.18. + scopes := []string{} + for s := range transval { + scopes = append(scopes, s) + } + sort.Strings(scopes) + for _, repo := range scopes { + repoval := transval[repo] tempTrustShowOutput := Policy{ Name: repo, RepoName: repo, -- cgit v1.2.3-54-g00ecf From 4df1e2524b9a8b3ff2d3768ac7fe54e98a966886 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 22:56:54 +0200 Subject: Add a unit test for trust.PolicyDescription MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add at least a basic unit test for the various entry types. So that we don't have to actually deal with GPG keys and /usr/bin/gpg*, parametrize the code with a gpgIDReader , and pass a fake one in the unit test. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 8 +++- pkg/trust/testdata/default.yaml | 25 +++++++++++ pkg/trust/testdata/redhat.yaml | 3 ++ pkg/trust/trust.go | 13 ++++-- pkg/trust/trust_test.go | 92 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 pkg/trust/testdata/default.yaml create mode 100644 pkg/trust/testdata/redhat.yaml create mode 100644 pkg/trust/trust_test.go (limited to 'pkg') diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index d2b904b07..7f32e2afc 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -53,6 +53,10 @@ func DefaultPolicyPath(sys *types.SystemContext) string { return systemDefaultPolicyPath } +// gpgIDReader returns GPG key IDs of keys stored at the provided path. +// It exists only for tests, production code should always use getGPGIdFromKeyPath. +type gpgIDReader func(string) []string + // createTmpFile creates a temp file under dir and writes the content into it func createTmpFile(dir, pattern string, content []byte) (string, error) { tmpfile, err := ioutil.TempFile(dir, pattern) @@ -79,7 +83,7 @@ func getGPGIdFromKeyPath(path string) []string { } // getGPGIdFromKeyData returns GPG key IDs of keys in the provided keyring. -func getGPGIdFromKeyData(key string) []string { +func getGPGIdFromKeyData(idReader gpgIDReader, key string) []string { decodeKey, err := base64.StdEncoding.DecodeString(key) if err != nil { logrus.Errorf("%s, error decoding key data", err) @@ -90,7 +94,7 @@ func getGPGIdFromKeyData(key string) []string { logrus.Errorf("Creating key date temp file %s", err) } defer os.Remove(tmpfileName) - return getGPGIdFromKeyPath(tmpfileName) + return idReader(tmpfileName) } func parseUids(colonDelimitKeys []byte) []string { diff --git a/pkg/trust/testdata/default.yaml b/pkg/trust/testdata/default.yaml new file mode 100644 index 000000000..31bcd35ef --- /dev/null +++ b/pkg/trust/testdata/default.yaml @@ -0,0 +1,25 @@ +# This is a default registries.d configuration file. You may +# add to this file or create additional files in registries.d/. +# +# lookaside: indicates a location that is read and write +# lookaside-staging: indicates a location that is only for write +# +# lookaside and lookaside-staging take a value of the following: +# lookaside: {schema}://location +# +# For reading signatures, schema may be http, https, or file. +# For writing signatures, schema may only be file. + +# This is the default signature write location for docker registries. +default-docker: +# lookaside: file:///var/lib/containers/sigstore + lookaside-staging: file:///var/lib/containers/sigstore + +# The 'docker' indicator here is the start of the configuration +# for docker registries. +# +# docker: +# +# privateregistry.com: +# lookaside: http://privateregistry.com/sigstore/ +# lookaside-staging: /mnt/nfs/privateregistry/sigstore diff --git a/pkg/trust/testdata/redhat.yaml b/pkg/trust/testdata/redhat.yaml new file mode 100644 index 000000000..35f2c611c --- /dev/null +++ b/pkg/trust/testdata/redhat.yaml @@ -0,0 +1,3 @@ +docker: + registry.redhat.io: + sigstore: https://registry.redhat.io/containers/sigstore diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index e93b4cd9d..9dd6878f9 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -19,18 +19,23 @@ type Policy struct { // PolicyDescription returns an user-focused description of the policy in policyPath and registries.d data from registriesDirPath. func PolicyDescription(policyPath, registriesDirPath string) ([]*Policy, error) { + return policyDescriptionWithGPGIDReader(policyPath, registriesDirPath, getGPGIdFromKeyPath) +} + +// policyDescriptionWithGPGIDReader is PolicyDescription with a gpgIDReader parameter. It exists only to make testing easier. +func policyDescriptionWithGPGIDReader(policyPath, registriesDirPath string, idReader gpgIDReader) ([]*Policy, error) { policyContentStruct, err := getPolicy(policyPath) if err != nil { return nil, fmt.Errorf("could not read trust policies: %w", err) } - res, err := getPolicyShowOutput(policyContentStruct, registriesDirPath) + res, err := getPolicyShowOutput(policyContentStruct, registriesDirPath, idReader) if err != nil { return nil, fmt.Errorf("could not show trust policies: %w", err) } return res, nil } -func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirPath string) ([]*Policy, error) { +func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirPath string, idReader gpgIDReader) ([]*Policy, error) { var output []*Policy registryConfigs, err := loadAndMergeConfig(systemRegistriesDirPath) @@ -76,10 +81,10 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP uids := []string{} for _, repoele := range repoval { if len(repoele.KeyPath) > 0 { - uids = append(uids, getGPGIdFromKeyPath(repoele.KeyPath)...) + uids = append(uids, idReader(repoele.KeyPath)...) } if len(repoele.KeyData) > 0 { - uids = append(uids, getGPGIdFromKeyData(repoele.KeyData)...) + uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) } } tempTrustShowOutput.GPGId = strings.Join(uids, ", ") diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go new file mode 100644 index 000000000..fc906572d --- /dev/null +++ b/pkg/trust/trust_test.go @@ -0,0 +1,92 @@ +package trust + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/containers/image/v5/signature" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPolicyDescription(t *testing.T) { + tempDir := t.TempDir() + policyPath := filepath.Join(tempDir, "policy.json") + + // Override getGPGIdFromKeyPath because we don't want to bother with (and spend the unit-test time on) generating valid GPG keys, and running the real GPG binary. + // Instead of reading the files at all, just expect file names like /id1,id2,...,idN.pub + idReader := func(keyPath string) []string { + require.True(t, strings.HasPrefix(keyPath, "/")) + require.True(t, strings.HasSuffix(keyPath, ".pub")) + return strings.Split(keyPath[1:len(keyPath)-4], ",") + } + + for _, c := range []struct { + policy *signature.Policy + expected []*Policy + }{ + { + &signature.Policy{ + Default: signature.PolicyRequirements{ + signature.NewPRReject(), + }, + Transports: map[string]signature.PolicyTransportScopes{ + "docker": { + "quay.io/accepted": { + signature.NewPRInsecureAcceptAnything(), + }, + "registry.redhat.io": { + xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + "quay.io/multi-signed": { + xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + }, + }, + }, + []*Policy{ + { + Transport: "all", + Name: "* (default)", + RepoName: "default", + Type: "reject", + }, + { + Transport: "repository", + Name: "quay.io/accepted", + RepoName: "quay.io/accepted", + Type: "accept", + }, + { + Transport: "repository", + Name: "quay.io/multi-signed", + RepoName: "quay.io/multi-signed", + Type: "signed", + SignatureStore: "", + GPGId: "1, 2, 3", + }, + { + Transport: "repository", + Name: "registry.redhat.io", + RepoName: "registry.redhat.io", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat", + }, + }, + }, + } { + policyJSON, err := json.Marshal(c.policy) + require.NoError(t, err) + err = os.WriteFile(policyPath, policyJSON, 0600) + require.NoError(t, err) + + res, err := policyDescriptionWithGPGIDReader(policyPath, "./testdata", idReader) + require.NoError(t, err) + assert.Equal(t, c.expected, res) + } +} -- cgit v1.2.3-54-g00ecf From d4c5217280a420aa28e9f9d116989f419dc427a1 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 19:56:37 +0200 Subject: Recognize the new lookaside names for simple signing sigstore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- pkg/trust/registries.go | 6 ++++-- pkg/trust/testdata/quay.io.yaml | 3 +++ pkg/trust/trust.go | 6 +++++- pkg/trust/trust_test.go | 2 +- 4 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 pkg/trust/testdata/quay.io.yaml (limited to 'pkg') diff --git a/pkg/trust/registries.go b/pkg/trust/registries.go index da2e7eb42..23de8b1e3 100644 --- a/pkg/trust/registries.go +++ b/pkg/trust/registries.go @@ -22,8 +22,10 @@ type registryConfiguration struct { // registryNamespace defines lookaside locations for a single namespace. type registryNamespace struct { - SigStore string `json:"sigstore"` // For reading, and if SigStoreStaging is not present, for writing. - SigStoreStaging string `json:"sigstore-staging"` // For writing only. + Lookaside string `json:"lookaside"` // For reading, and if LookasideStaging is not present, for writing. + LookasideStaging string `json:"lookaside-staging"` // For writing only. + SigStore string `json:"sigstore"` // For reading, and if SigStoreStaging is not present, for writing. + SigStoreStaging string `json:"sigstore-staging"` // For writing only. } // systemRegistriesDirPath is the path to registries.d. diff --git a/pkg/trust/testdata/quay.io.yaml b/pkg/trust/testdata/quay.io.yaml new file mode 100644 index 000000000..80071596d --- /dev/null +++ b/pkg/trust/testdata/quay.io.yaml @@ -0,0 +1,3 @@ +docker: + quay.io/multi-signed: + lookaside: https://quay.example.com/sigstore diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 9dd6878f9..aaddcf93e 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -91,7 +91,11 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP registryNamespace := haveMatchRegistry(repo, registryConfigs) if registryNamespace != nil { - tempTrustShowOutput.SignatureStore = registryNamespace.SigStore + if registryNamespace.Lookaside != "" { + tempTrustShowOutput.SignatureStore = registryNamespace.Lookaside + } else { // incl. registryNamespace.SigStore == "" + tempTrustShowOutput.SignatureStore = registryNamespace.SigStore + } } output = append(output, &tempTrustShowOutput) } diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index fc906572d..3ee49cc47 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -66,7 +66,7 @@ func TestPolicyDescription(t *testing.T) { Name: "quay.io/multi-signed", RepoName: "quay.io/multi-signed", Type: "signed", - SignatureStore: "", + SignatureStore: "https://quay.example.com/sigstore", GPGId: "1, 2, 3", }, { -- cgit v1.2.3-54-g00ecf From 51064acc49127daf1e945b19fe859bcc67d840ba Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 20:07:18 +0200 Subject: Split descriptionsOfPolicyRequirements out of getPolicyShowOutput MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will evetually allow us to use it for the default scope as well, which currently uses a simplified version. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 52 +++++++++++++++------------ pkg/trust/trust_test.go | 95 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 23 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index aaddcf93e..2d6f1fb87 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -72,33 +72,39 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP sort.Strings(scopes) for _, repo := range scopes { repoval := transval[repo] - tempTrustShowOutput := Policy{ + template := Policy{ + Transport: transport, Name: repo, RepoName: repo, - Transport: transport, - Type: trustTypeDescription(repoval[0].Type), } - uids := []string{} - for _, repoele := range repoval { - if len(repoele.KeyPath) > 0 { - uids = append(uids, idReader(repoele.KeyPath)...) - } - if len(repoele.KeyData) > 0 { - uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) - } - } - tempTrustShowOutput.GPGId = strings.Join(uids, ", ") - - registryNamespace := haveMatchRegistry(repo, registryConfigs) - if registryNamespace != nil { - if registryNamespace.Lookaside != "" { - tempTrustShowOutput.SignatureStore = registryNamespace.Lookaside - } else { // incl. registryNamespace.SigStore == "" - tempTrustShowOutput.SignatureStore = registryNamespace.SigStore - } - } - output = append(output, &tempTrustShowOutput) + output = append(output, descriptionsOfPolicyRequirements(repoval, template, registryConfigs, repo, idReader)...) } } return output, nil } + +// descriptionsOfPolicyRequirements turns reqs into user-readable policy entries, with Transport/Name/Reponame coming from template, potentially looking up scope in registryConfigs. +func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, registryConfigs *registryConfiguration, scope string, idReader gpgIDReader) []*Policy { + tempTrustShowOutput := template + tempTrustShowOutput.Type = trustTypeDescription(reqs[0].Type) + uids := []string{} + for _, repoele := range reqs { + if len(repoele.KeyPath) > 0 { + uids = append(uids, idReader(repoele.KeyPath)...) + } + if len(repoele.KeyData) > 0 { + uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) + } + } + tempTrustShowOutput.GPGId = strings.Join(uids, ", ") + + registryNamespace := haveMatchRegistry(scope, registryConfigs) + if registryNamespace != nil { + if registryNamespace.Lookaside != "" { + tempTrustShowOutput.SignatureStore = registryNamespace.Lookaside + } else { // incl. registryNamespace.SigStore == "" + tempTrustShowOutput.SignatureStore = registryNamespace.SigStore + } + } + return []*Policy{&tempTrustShowOutput} +} diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index 3ee49cc47..ef2d10061 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -90,3 +90,98 @@ func TestPolicyDescription(t *testing.T) { assert.Equal(t, c.expected, res) } } + +func TestDescriptionsOfPolicyRequirements(t *testing.T) { + // Override getGPGIdFromKeyPath because we don't want to bother with (and spend the unit-test time on) generating valid GPG keys, and running the real GPG binary. + // Instead of reading the files at all, just expect file names like /id1,id2,...,idN.pub + idReader := func(keyPath string) []string { + require.True(t, strings.HasPrefix(keyPath, "/")) + require.True(t, strings.HasSuffix(keyPath, ".pub")) + return strings.Split(keyPath[1:len(keyPath)-4], ",") + } + + template := Policy{ + Transport: "transport", + Name: "name", + RepoName: "repoName", + } + registryConfigs, err := loadAndMergeConfig("./testdata") + require.NoError(t, err) + + for _, c := range []struct { + scope string + reqs signature.PolicyRequirements + expected []*Policy + }{ + { + "", + signature.PolicyRequirements{ + signature.NewPRReject(), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "reject", + }, + }, + }, + { + "quay.io/accepted", + signature.PolicyRequirements{ + signature.NewPRInsecureAcceptAnything(), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "accept", + }, + }, + }, + { + "registry.redhat.io", + signature.PolicyRequirements{ + xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat", + }, + }, + }, + { + "quay.io/multi-signed", + signature.PolicyRequirements{ + xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://quay.example.com/sigstore", + GPGId: "1, 2, 3", + }, + }, + }, + } { + reqsJSON, err := json.Marshal(c.reqs) + require.NoError(t, err) + var parsedRegs []repoContent + err = json.Unmarshal(reqsJSON, &parsedRegs) + require.NoError(t, err) + + res := descriptionsOfPolicyRequirements(parsedRegs, template, registryConfigs, c.scope, idReader) + assert.Equal(t, c.expected, res) + } +} -- cgit v1.2.3-54-g00ecf From 1a97c4d9fa8d991625c2a6b5d9f353ef9cd5f6ab Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 20:07:54 +0200 Subject: Rename tempTrustShowOutput to entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that it is the primary return value of a small function, the long name only makes reading harder. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 2d6f1fb87..dd4262648 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -85,8 +85,8 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP // descriptionsOfPolicyRequirements turns reqs into user-readable policy entries, with Transport/Name/Reponame coming from template, potentially looking up scope in registryConfigs. func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, registryConfigs *registryConfiguration, scope string, idReader gpgIDReader) []*Policy { - tempTrustShowOutput := template - tempTrustShowOutput.Type = trustTypeDescription(reqs[0].Type) + entry := template + entry.Type = trustTypeDescription(reqs[0].Type) uids := []string{} for _, repoele := range reqs { if len(repoele.KeyPath) > 0 { @@ -96,15 +96,15 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) } } - tempTrustShowOutput.GPGId = strings.Join(uids, ", ") + entry.GPGId = strings.Join(uids, ", ") registryNamespace := haveMatchRegistry(scope, registryConfigs) if registryNamespace != nil { if registryNamespace.Lookaside != "" { - tempTrustShowOutput.SignatureStore = registryNamespace.Lookaside + entry.SignatureStore = registryNamespace.Lookaside } else { // incl. registryNamespace.SigStore == "" - tempTrustShowOutput.SignatureStore = registryNamespace.SigStore + entry.SignatureStore = registryNamespace.SigStore } } - return []*Policy{&tempTrustShowOutput} + return []*Policy{&entry} } -- cgit v1.2.3-54-g00ecf From b15afce551a521b6224cf0a0c5a29beb89556e91 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 20:27:44 +0200 Subject: Rename haveMatchRegistry to registriesDConfigurationForScope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just so that we don't have a boolean-named function returning a struct. Also reorder the parameters to have the container first, and the lookup key second. Shoud not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/registries.go | 18 +++++++++--------- pkg/trust/trust.go | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/registries.go b/pkg/trust/registries.go index 23de8b1e3..e179b61ac 100644 --- a/pkg/trust/registries.go +++ b/pkg/trust/registries.go @@ -101,22 +101,22 @@ func loadAndMergeConfig(dirPath string) (*registryConfiguration, error) { return &mergedConfig, nil } -// haveMatchRegistry returns configuration from registryConfigs that is configured for key. -func haveMatchRegistry(key string, registryConfigs *registryConfiguration) *registryNamespace { - searchKey := key - if !strings.Contains(searchKey, "/") { - val, exists := registryConfigs.Docker[searchKey] +// registriesDConfigurationForScope returns registries.d configuration for the provided scope. +func registriesDConfigurationForScope(registryConfigs *registryConfiguration, scope string) *registryNamespace { + searchScope := scope + if !strings.Contains(searchScope, "/") { + val, exists := registryConfigs.Docker[searchScope] if exists { return &val } } - for range strings.Split(key, "/") { - val, exists := registryConfigs.Docker[searchKey] + for range strings.Split(scope, "/") { + val, exists := registryConfigs.Docker[searchScope] if exists { return &val } - if strings.Contains(searchKey, "/") { - searchKey = searchKey[:strings.LastIndex(searchKey, "/")] + if strings.Contains(searchScope, "/") { + searchScope = searchScope[:strings.LastIndex(searchScope, "/")] } } return registryConfigs.DefaultDocker diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index dd4262648..a9ce99dd3 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -98,7 +98,7 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis } entry.GPGId = strings.Join(uids, ", ") - registryNamespace := haveMatchRegistry(scope, registryConfigs) + registryNamespace := registriesDConfigurationForScope(registryConfigs, scope) if registryNamespace != nil { if registryNamespace.Lookaside != "" { entry.SignatureStore = registryNamespace.Lookaside -- cgit v1.2.3-54-g00ecf From 2f6c145e86027da7ecf352331db70f5e688701b6 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 20:28:14 +0200 Subject: Use the full descriptionsOfPolicyRequirements for the default scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... instead of taking a shortcut, e.g. not listing any keys if they are required. Signed-off-by: Miloslav Trmač --- pkg/trust/registries.go | 27 +++++++++++++++------------ pkg/trust/trust.go | 7 +++---- pkg/trust/trust_test.go | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 16 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/registries.go b/pkg/trust/registries.go index e179b61ac..0adc38232 100644 --- a/pkg/trust/registries.go +++ b/pkg/trust/registries.go @@ -102,21 +102,24 @@ func loadAndMergeConfig(dirPath string) (*registryConfiguration, error) { } // registriesDConfigurationForScope returns registries.d configuration for the provided scope. +// scope can be "" to return only the global default configuration entry. func registriesDConfigurationForScope(registryConfigs *registryConfiguration, scope string) *registryNamespace { searchScope := scope - if !strings.Contains(searchScope, "/") { - val, exists := registryConfigs.Docker[searchScope] - if exists { - return &val - } - } - for range strings.Split(scope, "/") { - val, exists := registryConfigs.Docker[searchScope] - if exists { - return &val + if searchScope != "" { + if !strings.Contains(searchScope, "/") { + val, exists := registryConfigs.Docker[searchScope] + if exists { + return &val + } } - if strings.Contains(searchScope, "/") { - searchScope = searchScope[:strings.LastIndex(searchScope, "/")] + for range strings.Split(scope, "/") { + val, exists := registryConfigs.Docker[searchScope] + if exists { + return &val + } + if strings.Contains(searchScope, "/") { + searchScope = searchScope[:strings.LastIndex(searchScope, "/")] + } } } return registryConfigs.DefaultDocker diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index a9ce99dd3..7412fab20 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -44,13 +44,12 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP } if len(policyContentStruct.Default) > 0 { - defaultPolicyStruct := Policy{ + template := Policy{ Transport: "all", Name: "* (default)", RepoName: "default", - Type: trustTypeDescription(policyContentStruct.Default[0].Type), } - output = append(output, &defaultPolicyStruct) + output = append(output, descriptionsOfPolicyRequirements(policyContentStruct.Default, template, registryConfigs, "", idReader)...) } // FIXME: This should use x/exp/maps.Keys after we update to Go 1.18. transports := []string{} @@ -83,7 +82,7 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP return output, nil } -// descriptionsOfPolicyRequirements turns reqs into user-readable policy entries, with Transport/Name/Reponame coming from template, potentially looking up scope in registryConfigs. +// descriptionsOfPolicyRequirements turns reqs into user-readable policy entries, with Transport/Name/Reponame coming from template, potentially looking up scope (which may be "") in registryConfigs. func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, registryConfigs *registryConfiguration, scope string, idReader gpgIDReader) []*Policy { entry := template entry.Type = trustTypeDescription(reqs[0].Type) diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index ef2d10061..d04e9f211 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -79,6 +79,24 @@ func TestPolicyDescription(t *testing.T) { }, }, }, + { + &signature.Policy{ + Default: signature.PolicyRequirements{ + xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + }, + []*Policy{ + { + Transport: "all", + Name: "* (default)", + RepoName: "default", + Type: "signed", + SignatureStore: "", + GPGId: "1, 2, 3", + }, + }, + }, } { policyJSON, err := json.Marshal(c.policy) require.NoError(t, err) -- cgit v1.2.3-54-g00ecf From bba306788aba723d8555281eb07edd90a5890e64 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 20:36:14 +0200 Subject: Reorganize descriptionsOfPolicyRequirements a bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do the registries.d lookup once, separately from building an entry, so that we can share it across entries. Also prepare a separate res to allow adding multiple entries. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 7412fab20..7b1b798ca 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -84,6 +84,18 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP // descriptionsOfPolicyRequirements turns reqs into user-readable policy entries, with Transport/Name/Reponame coming from template, potentially looking up scope (which may be "") in registryConfigs. func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, registryConfigs *registryConfiguration, scope string, idReader gpgIDReader) []*Policy { + res := []*Policy{} + + var lookasidePath string + registryNamespace := registriesDConfigurationForScope(registryConfigs, scope) + if registryNamespace != nil { + if registryNamespace.Lookaside != "" { + lookasidePath = registryNamespace.Lookaside + } else { // incl. registryNamespace.SigStore == "" + lookasidePath = registryNamespace.SigStore + } + } + entry := template entry.Type = trustTypeDescription(reqs[0].Type) uids := []string{} @@ -96,14 +108,9 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis } } entry.GPGId = strings.Join(uids, ", ") + entry.SignatureStore = lookasidePath - registryNamespace := registriesDConfigurationForScope(registryConfigs, scope) - if registryNamespace != nil { - if registryNamespace.Lookaside != "" { - entry.SignatureStore = registryNamespace.Lookaside - } else { // incl. registryNamespace.SigStore == "" - entry.SignatureStore = registryNamespace.SigStore - } - } - return []*Policy{&entry} + res = append(res, &entry) + + return res } -- cgit v1.2.3-54-g00ecf From b36a1d1b79d7579738430adfd0696c324c3dacc0 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 20:45:57 +0200 Subject: BREAKING CHANGE: Change how (podman image trust show) represents multiple requirements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently - the output uses the first entry's type, even if the requirements are different (notably signedBy + sigstoreSIgned) - all public keys IDs are collected to a single line, even if some of them are interchangeable, and some are required (e.g. two signedBy requirements could require an image to be signed by (redhatProd OR redhatBeta) AND (vendor1 OR vendor2) So, stop collapsing the requirements, and return a separate entry for each one. Multiple GPG IDs on a single line used to mean AND or OR, now they always mean AND. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 14 ++++----- pkg/trust/trust_test.go | 80 +++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 84 insertions(+), 10 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 7b1b798ca..5f292083f 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -96,21 +96,21 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis } } - entry := template - entry.Type = trustTypeDescription(reqs[0].Type) - uids := []string{} for _, repoele := range reqs { + entry := template + entry.Type = trustTypeDescription(repoele.Type) + + uids := []string{} if len(repoele.KeyPath) > 0 { uids = append(uids, idReader(repoele.KeyPath)...) } if len(repoele.KeyData) > 0 { uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) } + entry.GPGId = strings.Join(uids, ", ") + entry.SignatureStore = lookasidePath + res = append(res, &entry) } - entry.GPGId = strings.Join(uids, ", ") - entry.SignatureStore = lookasidePath - - res = append(res, &entry) return res } diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index d04e9f211..edafeb5c1 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -67,7 +67,15 @@ func TestPolicyDescription(t *testing.T) { RepoName: "quay.io/multi-signed", Type: "signed", SignatureStore: "https://quay.example.com/sigstore", - GPGId: "1, 2, 3", + GPGId: "1", + }, + { + Transport: "repository", + Name: "quay.io/multi-signed", + RepoName: "quay.io/multi-signed", + Type: "signed", + SignatureStore: "https://quay.example.com/sigstore", + GPGId: "2, 3", }, { Transport: "repository", @@ -93,7 +101,15 @@ func TestPolicyDescription(t *testing.T) { RepoName: "default", Type: "signed", SignatureStore: "", - GPGId: "1, 2, 3", + GPGId: "1", + }, + { + Transport: "all", + Name: "* (default)", + RepoName: "default", + Type: "signed", + SignatureStore: "", + GPGId: "2, 3", }, }, }, @@ -188,7 +204,65 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { RepoName: "repoName", Type: "signed", SignatureStore: "https://quay.example.com/sigstore", - GPGId: "1, 2, 3", + GPGId: "1", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://quay.example.com/sigstore", + GPGId: "2, 3", + }, + }, + }, + { // Multiple kinds of requirements are represented individually. + "registry.redhat.io", + signature.PolicyRequirements{ + signature.NewPRReject(), + signature.NewPRInsecureAcceptAnything(), + xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + Type: "reject", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + Type: "accept", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "1", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "2, 3", }, }, }, -- cgit v1.2.3-54-g00ecf From 752eceaecc979627e998bee2dba8ee9ce47aa5cf Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 20:51:13 +0200 Subject: Support (image trust show) for sigstoreSigned entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sigstoreSigned does not have GPG IDs, so we add N/A in that column. NOTE: this does not show the use-sigstore-attachments value from registries.d. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 2 +- pkg/trust/trust.go | 24 ++++++++++++------- pkg/trust/trust_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 9 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 7f32e2afc..085f0076a 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -130,7 +130,7 @@ func getPolicy(policyPath string) (policyContent, error) { return policyContentStruct, nil } -var typeDescription = map[string]string{"insecureAcceptAnything": "accept", "signedBy": "signed", "reject": "reject"} +var typeDescription = map[string]string{"insecureAcceptAnything": "accept", "signedBy": "signed", "sigstoreSigned": "sigstoreSigned", "reject": "reject"} func trustTypeDescription(trustType string) string { trustDescription, exist := typeDescription[trustType] diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 5f292083f..a27ce5a85 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -100,15 +100,23 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis entry := template entry.Type = trustTypeDescription(repoele.Type) - uids := []string{} - if len(repoele.KeyPath) > 0 { - uids = append(uids, idReader(repoele.KeyPath)...) - } - if len(repoele.KeyData) > 0 { - uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) + var gpgIDString string + switch repoele.Type { + case "signedBy": + uids := []string{} + if len(repoele.KeyPath) > 0 { + uids = append(uids, idReader(repoele.KeyPath)...) + } + if len(repoele.KeyData) > 0 { + uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) + } + gpgIDString = strings.Join(uids, ", ") + + case "sigstoreSigned": + gpgIDString = "N/A" // We could potentially return key fingerprints here, but they would not be _GPG_ fingerprints. } - entry.GPGId = strings.Join(uids, ", ") - entry.SignatureStore = lookasidePath + entry.GPGId = gpgIDString + entry.SignatureStore = lookasidePath // We do this even for sigstoreSigned and things like type: reject, to show that the sigstore is being read. res = append(res, &entry) } diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index edafeb5c1..58394e77b 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -45,6 +45,10 @@ func TestPolicyDescription(t *testing.T) { xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), }, + "quay.io/sigstore-signed": { + xNewPRSigstoreSignedKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSigstoreSignedKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, }, }, }, @@ -77,6 +81,22 @@ func TestPolicyDescription(t *testing.T) { SignatureStore: "https://quay.example.com/sigstore", GPGId: "2, 3", }, + { + Transport: "repository", + Name: "quay.io/sigstore-signed", + RepoName: "quay.io/sigstore-signed", + Type: "sigstoreSigned", + SignatureStore: "", + GPGId: "N/A", + }, + { + Transport: "repository", + Name: "quay.io/sigstore-signed", + RepoName: "quay.io/sigstore-signed", + Type: "sigstoreSigned", + SignatureStore: "", + GPGId: "N/A", + }, { Transport: "repository", Name: "registry.redhat.io", @@ -215,6 +235,30 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { GPGId: "2, 3", }, }, + }, { + "quay.io/sigstore-signed", + signature.PolicyRequirements{ + xNewPRSigstoreSignedKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSigstoreSignedKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "sigstoreSigned", + SignatureStore: "", + GPGId: "N/A", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "sigstoreSigned", + SignatureStore: "", + GPGId: "N/A", + }, + }, }, { // Multiple kinds of requirements are represented individually. "registry.redhat.io", @@ -224,6 +268,8 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSigstoreSignedKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSigstoreSignedKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), }, []*Policy{ { @@ -264,6 +310,22 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { SignatureStore: "https://registry.redhat.io/containers/sigstore", GPGId: "2, 3", }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "sigstoreSigned", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "N/A", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "sigstoreSigned", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "N/A", + }, }, }, } { -- cgit v1.2.3-54-g00ecf From a7e88c8dacd56d266173d3fa04c66eb1a964b332 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Thu, 25 Aug 2022 01:02:13 +0200 Subject: Add support for showing keyPaths in (podman image trust show) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 1 + pkg/trust/policy_test.go | 7 +++++++ pkg/trust/testdata/redhat.yaml | 2 ++ pkg/trust/trust.go | 3 +++ pkg/trust/trust_test.go | 35 +++++++++++++++++++++++++++++++++++ 5 files changed, 48 insertions(+) (limited to 'pkg') diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 085f0076a..a41982c13 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -29,6 +29,7 @@ type repoContent struct { Type string `json:"type"` KeyType string `json:"keyType,omitempty"` KeyPath string `json:"keyPath,omitempty"` + KeyPaths []string `json:"keyPaths,omitempty"` KeyData string `json:"keyData,omitempty"` SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` } diff --git a/pkg/trust/policy_test.go b/pkg/trust/policy_test.go index c2c2d93be..0f9721722 100644 --- a/pkg/trust/policy_test.go +++ b/pkg/trust/policy_test.go @@ -117,6 +117,13 @@ func xNewPRSignedByKeyPath(t *testing.T, keyPath string, signedIdentity signatur return pr } +// xNewPRSignedByKeyPaths is a wrapper for NewPRSignedByKeyPaths which must not fail. +func xNewPRSignedByKeyPaths(t *testing.T, keyPaths []string, signedIdentity signature.PolicyReferenceMatch) signature.PolicyRequirement { + pr, err := signature.NewPRSignedByKeyPaths(signature.SBKeyTypeGPGKeys, keyPaths, signedIdentity) + require.NoError(t, err) + return pr +} + // xNewPRSigstoreSignedKeyPath is a wrapper for NewPRSigstoreSignedKeyPath which must not fail. func xNewPRSigstoreSignedKeyPath(t *testing.T, keyPath string, signedIdentity signature.PolicyReferenceMatch) signature.PolicyRequirement { pr, err := signature.NewPRSigstoreSignedKeyPath(keyPath, signedIdentity) diff --git a/pkg/trust/testdata/redhat.yaml b/pkg/trust/testdata/redhat.yaml index 35f2c611c..8e40a4174 100644 --- a/pkg/trust/testdata/redhat.yaml +++ b/pkg/trust/testdata/redhat.yaml @@ -1,3 +1,5 @@ docker: registry.redhat.io: sigstore: https://registry.redhat.io/containers/sigstore + registry.access.redhat.com: + sigstore: https://registry.redhat.io/containers/sigstore diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index a27ce5a85..07d144bc1 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -107,6 +107,9 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis if len(repoele.KeyPath) > 0 { uids = append(uids, idReader(repoele.KeyPath)...) } + for _, path := range repoele.KeyPaths { + uids = append(uids, idReader(path)...) + } if len(repoele.KeyData) > 0 { uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) } diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index 58394e77b..22b780fc9 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -41,6 +41,9 @@ func TestPolicyDescription(t *testing.T) { "registry.redhat.io": { xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), }, + "registry.access.redhat.com": { + xNewPRSignedByKeyPaths(t, []string{"/redhat.pub", "/redhat-beta.pub"}, signature.NewPRMMatchRepoDigestOrExact()), + }, "quay.io/multi-signed": { xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), @@ -98,6 +101,13 @@ func TestPolicyDescription(t *testing.T) { GPGId: "N/A", }, { + Transport: "repository", + Name: "registry.access.redhat.com", + RepoName: "registry.access.redhat.com", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat, redhat-beta", + }, { Transport: "repository", Name: "registry.redhat.io", RepoName: "registry.redhat.io", @@ -211,6 +221,22 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { }, }, }, + { + "registry.access.redhat.com", + signature.PolicyRequirements{ + xNewPRSignedByKeyPaths(t, []string{"/redhat.pub", "/redhat-beta.pub"}, signature.NewPRMMatchRepoDigestOrExact()), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat, redhat-beta", + }, + }, + }, { "quay.io/multi-signed", signature.PolicyRequirements{ @@ -266,6 +292,7 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { signature.NewPRReject(), signature.NewPRInsecureAcceptAnything(), xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPaths(t, []string{"/redhat.pub", "/redhat-beta.pub"}, signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSigstoreSignedKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), @@ -294,6 +321,14 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { SignatureStore: "https://registry.redhat.io/containers/sigstore", GPGId: "redhat", }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat, redhat-beta", + }, { Transport: "transport", Name: "name", -- cgit v1.2.3-54-g00ecf From ad0c785f8e0846112b301872ffc8a7ea276c82a5 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Thu, 25 Aug 2022 01:11:07 +0200 Subject: Reorganize the types in policy.go a bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... to go from top to bottom. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index a41982c13..daa7698b2 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -23,6 +23,12 @@ type policyContent struct { Transports transportsContent `json:"transports,omitempty"` } +// transportsContent contains policies for individual transports (= c/image/v5/signature.Policy.Transports) +type transportsContent map[string]repoMap + +// repoMap maps a scope name to requirements that apply to that scope (= c/image/v5/signature.PolicyTransportScopes) +type repoMap map[string][]repoContent + // repoContent is a single policy requirement (one of possibly several for a scope), representing all of the individual alternatives in a single merged struct // (= c/image/v5/signature.{PolicyRequirement,pr*}) type repoContent struct { @@ -34,12 +40,6 @@ type repoContent struct { SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` } -// repoMap maps a scope name to requirements that apply to that scope (= c/image/v5/signature.PolicyTransportScopes) -type repoMap map[string][]repoContent - -// transportsContent contains policies for individual transports (= c/image/v5/signature.Policy.Transports) -type transportsContent map[string]repoMap - // DefaultPolicyPath returns a path to the default policy of the system. func DefaultPolicyPath(sys *types.SystemContext) string { systemDefaultPolicyPath := "/etc/containers/policy.json" -- cgit v1.2.3-54-g00ecf From 61fe95bb4fa7b6f83cd2a013877664db90cd773b Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Thu, 25 Aug 2022 01:25:08 +0200 Subject: Preserve all unknown PolicyRequirement fields on (podman image trust set) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We are unmarshaling and re-marshaling JSON, which can _silently_ drop data with the Go design decision.data. Try harder, by using json.RawMessage at least for the data we care about. Alternatively, this could use json.Decoder.DisallowUnknownFields. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 30 +++++++++++++++++------ pkg/trust/policy_test.go | 64 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 7 deletions(-) (limited to 'pkg') diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index daa7698b2..326fe17af 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -40,6 +40,18 @@ type repoContent struct { SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` } +// genericPolicyContent is the overall structure of a policy.json file (= c/image/v5/signature.Policy), using generic data for individual requirements. +type genericPolicyContent struct { + Default json.RawMessage `json:"default"` + Transports genericTransportsContent `json:"transports,omitempty"` +} + +// genericTransportsContent contains policies for individual transports (= c/image/v5/signature.Policy.Transports), using generic data for individual requirements. +type genericTransportsContent map[string]genericRepoMap + +// genericRepoMap maps a scope name to requirements that apply to that scope (= c/image/v5/signature.PolicyTransportScopes) +type genericRepoMap map[string]json.RawMessage + // DefaultPolicyPath returns a path to the default policy of the system. func DefaultPolicyPath(sys *types.SystemContext) string { systemDefaultPolicyPath := "/etc/containers/policy.json" @@ -152,7 +164,7 @@ type AddPolicyEntriesInput struct { // AddPolicyEntries adds one or more policy entries necessary to implement AddPolicyEntriesInput. func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { var ( - policyContentStruct policyContent + policyContentStruct genericPolicyContent newReposContent []repoContent ) trustType := input.Type @@ -188,8 +200,12 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { default: return fmt.Errorf("unknown trust type %q", input.Type) } + newReposJSON, err := json.Marshal(newReposContent) + if err != nil { + return err + } - _, err := os.Stat(policyPath) + _, err = os.Stat(policyPath) if !os.IsNotExist(err) { policyContent, err := ioutil.ReadFile(policyPath) if err != nil { @@ -200,7 +216,7 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { } } if input.Scope == "default" { - policyContentStruct.Default = newReposContent + policyContentStruct.Default = json.RawMessage(newReposJSON) } else { if len(policyContentStruct.Default) == 0 { return errors.New("default trust policy must be set") @@ -209,18 +225,18 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { for transport, transportval := range policyContentStruct.Transports { _, registryExists = transportval[input.Scope] if registryExists { - policyContentStruct.Transports[transport][input.Scope] = newReposContent + policyContentStruct.Transports[transport][input.Scope] = json.RawMessage(newReposJSON) break } } if !registryExists { if policyContentStruct.Transports == nil { - policyContentStruct.Transports = make(map[string]repoMap) + policyContentStruct.Transports = make(map[string]genericRepoMap) } if policyContentStruct.Transports["docker"] == nil { - policyContentStruct.Transports["docker"] = make(map[string][]repoContent) + policyContentStruct.Transports["docker"] = make(map[string]json.RawMessage) } - policyContentStruct.Transports["docker"][input.Scope] = append(policyContentStruct.Transports["docker"][input.Scope], newReposContent...) + policyContentStruct.Transports["docker"][input.Scope] = json.RawMessage(newReposJSON) } } diff --git a/pkg/trust/policy_test.go b/pkg/trust/policy_test.go index 0f9721722..3952b72c3 100644 --- a/pkg/trust/policy_test.go +++ b/pkg/trust/policy_test.go @@ -108,6 +108,70 @@ func TestAddPolicyEntries(t *testing.T) { }, }, }, parsedPolicy) + + // Test that completely unknown JSON is preserved + jsonWithUnknownData := `{ + "default": [ + { + "type": "this is unknown", + "unknown field": "should be preserved" + } + ], + "transports": + { + "docker-daemon": + { + "": [{ + "type":"this is unknown 2", + "unknown field 2": "should be preserved 2" + }] + } + } +}` + err = os.WriteFile(policyPath, []byte(jsonWithUnknownData), 0600) + require.NoError(t, err) + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ + Scope: "quay.io/innocuous", + Type: "signedBy", + PubKeyFiles: []string{"/1.pub"}, + }) + require.NoError(t, err) + updatedJSONWithUnknownData, err := os.ReadFile(policyPath) + require.NoError(t, err) + // Decode updatedJSONWithUnknownData so that this test does not depend on details of the encoding. + // To reduce noise in the constants below: + type a = []interface{} + type m = map[string]interface{} + var parsedUpdatedJSON m + err = json.Unmarshal(updatedJSONWithUnknownData, &parsedUpdatedJSON) + require.NoError(t, err) + assert.Equal(t, m{ + "default": a{ + m{ + "type": "this is unknown", + "unknown field": "should be preserved", + }, + }, + "transports": m{ + "docker-daemon": m{ + "": a{ + m{ + "type": "this is unknown 2", + "unknown field 2": "should be preserved 2", + }, + }, + }, + "docker": m{ + "quay.io/innocuous": a{ + m{ + "type": "signedBy", + "keyType": "GPGKeys", + "keyPath": "/1.pub", + }, + }, + }, + }, + }, parsedUpdatedJSON) } // xNewPRSignedByKeyPath is a wrapper for NewPRSignedByKeyPath which must not fail. -- cgit v1.2.3-54-g00ecf From 9553f3bafad264367a8a642a17239c0d87c18090 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Thu, 25 Aug 2022 16:31:53 -0400 Subject: Run codespell Signed-off-by: Daniel J Walsh --- Makefile | 2 +- libpod/container_internal_unsupported.go | 8 ++++---- libpod/define/exec_codes.go | 4 ++-- pkg/machine/config.go | 2 +- pkg/systemd/notifyproxy/notifyproxy_test.go | 2 +- test/e2e/restart_test.go | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) (limited to 'pkg') diff --git a/Makefile b/Makefile index 4818ee122..d10c9cf19 100644 --- a/Makefile +++ b/Makefile @@ -267,7 +267,7 @@ test/version/version: version/version.go .PHONY: codespell codespell: - codespell -S bin,vendor,.git,go.sum,.cirrus.yml,"RELEASE_NOTES.md,*.xz,*.gz,*.ps1,*.tar,swagger.yaml,*.tgz,bin2img,*ico,*.png,*.1,*.5,copyimg,*.orig,apidoc.go" -L pullrequest,uint,iff,od,seeked,splitted,marge,erro,hist,ether -w + codespell -S bin,vendor,.git,go.sum,.cirrus.yml,"RELEASE_NOTES.md,*.xz,*.gz,*.ps1,*.tar,swagger.yaml,*.tgz,bin2img,*ico,*.png,*.1,*.5,copyimg,*.orig,apidoc.go" -L clos,ans,pullrequest,uint,iff,od,seeked,splitted,marge,erro,hist,ether -w .PHONY: validate validate: lint .gitvalidation validate.completions man-page-check swagger-check tests-included tests-expect-exit pr-removes-fixed-skips diff --git a/libpod/container_internal_unsupported.go b/libpod/container_internal_unsupported.go index de92ff260..074aeee47 100644 --- a/libpod/container_internal_unsupported.go +++ b/libpod/container_internal_unsupported.go @@ -69,21 +69,21 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti // getHostsEntries returns the container ip host entries for the correct netmode func (c *Container) getHostsEntries() (etchosts.HostEntries, error) { - return nil, errors.New("unspported (*Container) getHostsEntries") + return nil, errors.New("unsupported (*Container) getHostsEntries") } // Fix ownership and permissions of the specified volume if necessary. func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error { - return errors.New("unspported (*Container) fixVolumePermissions") + return errors.New("unsupported (*Container) fixVolumePermissions") } func (c *Container) expectPodCgroup() (bool, error) { - return false, errors.New("unspported (*Container) expectPodCgroup") + return false, errors.New("unsupported (*Container) expectPodCgroup") } // Get cgroup path in a format suitable for the OCI spec func (c *Container) getOCICgroupPath() (string, error) { - return "", errors.New("unspported (*Container) getOCICgroupPath") + return "", errors.New("unsupported (*Container) getOCICgroupPath") } func getLocalhostHostEntry(c *Container) etchosts.HostEntries { diff --git a/libpod/define/exec_codes.go b/libpod/define/exec_codes.go index 3f2da4910..a84730e72 100644 --- a/libpod/define/exec_codes.go +++ b/libpod/define/exec_codes.go @@ -11,8 +11,8 @@ const ( // ExecErrorCodeGeneric is the default error code to return from an exec session if libpod failed // prior to calling the runtime ExecErrorCodeGeneric = 125 - // ExecErrorCodeCannotInvoke is the error code to return when the runtime fails to invoke a command - // an example of this can be found by trying to execute a directory: + // ExecErrorCodeCannotInvoke is the error code to return when the runtime fails to invoke a command. + // An example of this can be found by trying to execute a directory: // `podman exec -l /etc` ExecErrorCodeCannotInvoke = 126 // ExecErrorCodeNotFound is the error code to return when a command cannot be found diff --git a/pkg/machine/config.go b/pkg/machine/config.go index 5162006db..54aa9e1b7 100644 --- a/pkg/machine/config.go +++ b/pkg/machine/config.go @@ -175,7 +175,7 @@ func (rc RemoteConnectionType) MakeSSHURL(host, path, port, userName string) url return uri } -// GetCacheDir returns the dir where VM images are downladed into when pulled +// GetCacheDir returns the dir where VM images are downloaded into when pulled func GetCacheDir(vmType string) (string, error) { dataDir, err := GetDataDir(vmType) if err != nil { diff --git a/pkg/systemd/notifyproxy/notifyproxy_test.go b/pkg/systemd/notifyproxy/notifyproxy_test.go index edad95659..ce63fc9cd 100644 --- a/pkg/systemd/notifyproxy/notifyproxy_test.go +++ b/pkg/systemd/notifyproxy/notifyproxy_test.go @@ -37,7 +37,7 @@ func TestWaitAndClose(t *testing.T) { time.Sleep(250 * time.Millisecond) select { case err := <-ch: - t.Fatalf("Should stil be waiting but received %v", err) + t.Fatalf("Should still be waiting but received %v", err) default: } diff --git a/test/e2e/restart_test.go b/test/e2e/restart_test.go index dd0070f54..9df884292 100644 --- a/test/e2e/restart_test.go +++ b/test/e2e/restart_test.go @@ -228,7 +228,7 @@ var _ = Describe("Podman restart", func() { Expect(beforeRestart.OutputToString()).To(Equal(afterRestart.OutputToString())) }) - It("podman restart all stoped containers with --all", func() { + It("podman restart all stopped containers with --all", func() { session := podmanTest.RunTopContainer("") session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) -- cgit v1.2.3-54-g00ecf From c7fda06f669ab1be6b36aa3312c6c6d732e06c9b Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 25 Aug 2022 14:10:49 -0400 Subject: Compat API image remove events now have 'delete' status Change only the compat API, so we don't force a breaking change on Libpod API users. Partial fix for #15485 Signed-off-by: Matthew Heon --- pkg/api/handlers/compat/events.go | 6 ++++++ test/apiv2/10-images.at | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/events.go b/pkg/api/handlers/compat/events.go index 18fb35966..105404a0d 100644 --- a/pkg/api/handlers/compat/events.go +++ b/pkg/api/handlers/compat/events.go @@ -89,6 +89,12 @@ func GetEvents(w http.ResponseWriter, r *http.Request) { } e := entities.ConvertToEntitiesEvent(*evt) + // Some events differ between Libpod and Docker endpoints. + // Handle these differences for Docker-compat. + if !utils.IsLibpodRequest(r) && e.Type == "image" && e.Status == "remove" { + e.Status = "delete" + e.Action = "delete" + } if !utils.IsLibpodRequest(r) && e.Status == "died" { e.Status = "die" e.Action = "die" diff --git a/test/apiv2/10-images.at b/test/apiv2/10-images.at index 4fd954e37..86ee2a1f5 100644 --- a/test/apiv2/10-images.at +++ b/test/apiv2/10-images.at @@ -239,4 +239,23 @@ fi cleanBuildTest +# compat API vs libpod API event differences: +# on image removal, libpod produces 'remove' events. +# compat produces 'delete' events. +podman image build -t test:test -< Date: Thu, 25 Aug 2022 12:10:53 +0530 Subject: remote: fix implementation of build with --userns=auto for API `podman-remote` and Libpod API does not supports build with `--userns=auto` since `IDMappingOptions` were not implemented for API and bindings, following PR implements passing `IDMappingOptions` via bindings to API. Closes: https://github.com/containers/podman/issues/15476 Signed-off-by: Aditya R --- pkg/api/handlers/compat/images_build.go | 10 ++++++++++ pkg/bindings/images/build.go | 7 +++++++ test/e2e/build/Containerfile.userns-auto | 2 ++ test/e2e/run_userns_test.go | 30 ++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+) create mode 100644 test/e2e/build/Containerfile.userns-auto (limited to 'pkg') diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index 020991cc7..7ba1029a7 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -101,6 +101,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { ForceRm bool `schema:"forcerm"` From string `schema:"from"` HTTPProxy bool `schema:"httpproxy"` + IDMappingOptions string `schema:"idmappingoptions"` IdentityLabel bool `schema:"identitylabel"` Ignore bool `schema:"ignore"` Isolation string `schema:"isolation"` @@ -389,6 +390,14 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { } } + var idMappingOptions buildahDefine.IDMappingOptions + if _, found := r.URL.Query()["idmappingoptions"]; found { + if err := json.Unmarshal([]byte(query.IDMappingOptions), &idMappingOptions); err != nil { + utils.BadRequest(w, "idmappingoptions", query.IDMappingOptions, err) + return + } + } + var cacheFrom reference.Named if _, found := r.URL.Query()["cachefrom"]; found { cacheFrom, err = parse.RepoNameToNamedReference(query.CacheFrom) @@ -644,6 +653,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { Excludes: excludes, ForceRmIntermediateCtrs: query.ForceRm, From: fromImage, + IDMappingOptions: &idMappingOptions, IgnoreUnrecognizedInstructions: query.Ignore, Isolation: isolation, Jobs: &jobs, diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go index 2615bc516..8348ac54b 100644 --- a/pkg/bindings/images/build.go +++ b/pkg/bindings/images/build.go @@ -88,6 +88,13 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO } params.Set("additionalbuildcontexts", string(additionalBuildContextMap)) } + if options.IDMappingOptions != nil { + idmappingsOptions, err := jsoniter.Marshal(options.IDMappingOptions) + if err != nil { + return nil, err + } + params.Set("idmappingoptions", string(idmappingsOptions)) + } if buildArgs := options.Args; len(buildArgs) > 0 { bArgs, err := jsoniter.MarshalToString(buildArgs) if err != nil { diff --git a/test/e2e/build/Containerfile.userns-auto b/test/e2e/build/Containerfile.userns-auto new file mode 100644 index 000000000..921610982 --- /dev/null +++ b/test/e2e/build/Containerfile.userns-auto @@ -0,0 +1,2 @@ +FROM alpine +RUN cat /proc/self/uid_map diff --git a/test/e2e/run_userns_test.go b/test/e2e/run_userns_test.go index f247b2dac..62e512d3a 100644 --- a/test/e2e/run_userns_test.go +++ b/test/e2e/run_userns_test.go @@ -8,6 +8,7 @@ import ( "strings" . "github.com/containers/podman/v4/test/utils" + "github.com/containers/storage" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gexec" @@ -42,6 +43,33 @@ var _ = Describe("Podman UserNS support", func() { }) + // Note: Lot of tests for build with --userns=auto are already there in buildah + // but they are skipped in podman CI because bud tests are executed in rootfull + // environment ( where mappings for the `containers` user is not present in /etc/subuid ) + // causing them to skip hence this is a redundant test for sanity to make sure + // we don't break this feature for podman-remote. + It("podman build with --userns=auto", func() { + u, err := user.Current() + Expect(err).To(BeNil()) + name := u.Name + if name == "root" { + name = "containers" + } + content, err := ioutil.ReadFile("/etc/subuid") + if err != nil { + Skip("cannot read /etc/subuid") + } + if !strings.Contains(string(content), name) { + Skip("cannot find mappings for the current user") + } + session := podmanTest.Podman([]string{"build", "-f", "build/Containerfile.userns-auto", "-t", "test", "--userns=auto"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + // `1024` is the default size or length of the range of user IDs + // that is mapped between the two user namespaces by --userns=auto. + Expect(session.OutputToString()).To(ContainSubstring(fmt.Sprintf("%d", storage.AutoUserNsMinSize))) + }) + It("podman uidmapping and gidmapping", func() { session := podmanTest.Podman([]string{"run", "--uidmap=0:100:5000", "--gidmap=0:200:5000", "alpine", "echo", "hello"}) session.WaitWithDefaultTimeout() @@ -157,6 +185,8 @@ var _ = Describe("Podman UserNS support", func() { session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) l := session.OutputToString() + // `1024` is the default size or length of the range of user IDs + // that is mapped between the two user namespaces by --userns=auto. Expect(l).To(ContainSubstring("1024")) m[l] = l } -- cgit v1.2.3-54-g00ecf From 0b3184a5acb68043a6cf44b7c223d84d8badd930 Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Fri, 26 Aug 2022 15:32:52 +0100 Subject: pkg/domain: Add terminal support for FreeBSD This just moves the code to files which can be shared with freebsd. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson --- pkg/domain/infra/abi/terminal/sigproxy_commn.go | 63 ++++++++++++ pkg/domain/infra/abi/terminal/sigproxy_linux.go | 60 ----------- pkg/domain/infra/abi/terminal/terminal_common.go | 113 +++++++++++++++++++++ pkg/domain/infra/abi/terminal/terminal_linux.go | 110 -------------------- .../infra/abi/terminal/terminal_unsupported.go | 4 +- 5 files changed, 178 insertions(+), 172 deletions(-) create mode 100644 pkg/domain/infra/abi/terminal/sigproxy_commn.go delete mode 100644 pkg/domain/infra/abi/terminal/sigproxy_linux.go create mode 100644 pkg/domain/infra/abi/terminal/terminal_common.go delete mode 100644 pkg/domain/infra/abi/terminal/terminal_linux.go (limited to 'pkg') diff --git a/pkg/domain/infra/abi/terminal/sigproxy_commn.go b/pkg/domain/infra/abi/terminal/sigproxy_commn.go new file mode 100644 index 000000000..3a0132ef3 --- /dev/null +++ b/pkg/domain/infra/abi/terminal/sigproxy_commn.go @@ -0,0 +1,63 @@ +//go:build linux || freebsd +// +build linux freebsd + +package terminal + +import ( + "errors" + "os" + "syscall" + + "github.com/containers/podman/v4/libpod" + "github.com/containers/podman/v4/libpod/define" + "github.com/containers/podman/v4/libpod/shutdown" + "github.com/containers/podman/v4/pkg/signal" + "github.com/sirupsen/logrus" +) + +// Make sure the signal buffer is sufficiently big. +// runc is using the same value. +const signalBufferSize = 2048 + +// ProxySignals ... +func ProxySignals(ctr *libpod.Container) { + // Stop catching the shutdown signals (SIGINT, SIGTERM) - they're going + // to the container now. + shutdown.Stop() //nolint: errcheck + + sigBuffer := make(chan os.Signal, signalBufferSize) + signal.CatchAll(sigBuffer) + + logrus.Debugf("Enabling signal proxying") + + go func() { + for s := range sigBuffer { + // Ignore SIGCHLD and SIGPIPE - these are mostly likely + // intended for the podman command itself. + // SIGURG was added because of golang 1.14 and its preemptive changes + // causing more signals to "show up". + // https://github.com/containers/podman/issues/5483 + if s == syscall.SIGCHLD || s == syscall.SIGPIPE || s == syscall.SIGURG { + continue + } + + if err := ctr.Kill(uint(s.(syscall.Signal))); err != nil { + if errors.Is(err, define.ErrCtrStateInvalid) { + logrus.Infof("Ceasing signal forwarding to container %s as it has stopped", ctr.ID()) + } else { + logrus.Errorf("forwarding signal %d to container %s: %v", s, ctr.ID(), err) + } + // If the container dies, and we find out here, + // we need to forward that one signal to + // ourselves so that it is not lost, and then + // we terminate the proxy and let the defaults + // play out. + signal.StopCatch(sigBuffer) + if err := syscall.Kill(syscall.Getpid(), s.(syscall.Signal)); err != nil { + logrus.Errorf("Failed to kill pid %d", syscall.Getpid()) + } + return + } + } + }() +} diff --git a/pkg/domain/infra/abi/terminal/sigproxy_linux.go b/pkg/domain/infra/abi/terminal/sigproxy_linux.go deleted file mode 100644 index 16d345f06..000000000 --- a/pkg/domain/infra/abi/terminal/sigproxy_linux.go +++ /dev/null @@ -1,60 +0,0 @@ -package terminal - -import ( - "errors" - "os" - "syscall" - - "github.com/containers/podman/v4/libpod" - "github.com/containers/podman/v4/libpod/define" - "github.com/containers/podman/v4/libpod/shutdown" - "github.com/containers/podman/v4/pkg/signal" - "github.com/sirupsen/logrus" -) - -// Make sure the signal buffer is sufficiently big. -// runc is using the same value. -const signalBufferSize = 2048 - -// ProxySignals ... -func ProxySignals(ctr *libpod.Container) { - // Stop catching the shutdown signals (SIGINT, SIGTERM) - they're going - // to the container now. - shutdown.Stop() //nolint: errcheck - - sigBuffer := make(chan os.Signal, signalBufferSize) - signal.CatchAll(sigBuffer) - - logrus.Debugf("Enabling signal proxying") - - go func() { - for s := range sigBuffer { - // Ignore SIGCHLD and SIGPIPE - these are mostly likely - // intended for the podman command itself. - // SIGURG was added because of golang 1.14 and its preemptive changes - // causing more signals to "show up". - // https://github.com/containers/podman/issues/5483 - if s == syscall.SIGCHLD || s == syscall.SIGPIPE || s == syscall.SIGURG { - continue - } - - if err := ctr.Kill(uint(s.(syscall.Signal))); err != nil { - if errors.Is(err, define.ErrCtrStateInvalid) { - logrus.Infof("Ceasing signal forwarding to container %s as it has stopped", ctr.ID()) - } else { - logrus.Errorf("forwarding signal %d to container %s: %v", s, ctr.ID(), err) - } - // If the container dies, and we find out here, - // we need to forward that one signal to - // ourselves so that it is not lost, and then - // we terminate the proxy and let the defaults - // play out. - signal.StopCatch(sigBuffer) - if err := syscall.Kill(syscall.Getpid(), s.(syscall.Signal)); err != nil { - logrus.Errorf("Failed to kill pid %d", syscall.Getpid()) - } - return - } - } - }() -} diff --git a/pkg/domain/infra/abi/terminal/terminal_common.go b/pkg/domain/infra/abi/terminal/terminal_common.go new file mode 100644 index 000000000..afae2c085 --- /dev/null +++ b/pkg/domain/infra/abi/terminal/terminal_common.go @@ -0,0 +1,113 @@ +//go:build linux || freebsd +// +build linux freebsd + +package terminal + +import ( + "bufio" + "context" + "fmt" + "os" + + "github.com/containers/common/pkg/resize" + "github.com/containers/podman/v4/libpod" + "github.com/containers/podman/v4/libpod/define" + "github.com/sirupsen/logrus" + "golang.org/x/term" +) + +// ExecAttachCtr execs and attaches to a container +func ExecAttachCtr(ctx context.Context, ctr *libpod.Container, execConfig *libpod.ExecConfig, streams *define.AttachStreams) (int, error) { + var resizechan chan resize.TerminalSize + haveTerminal := term.IsTerminal(int(os.Stdin.Fd())) + + // Check if we are attached to a terminal. If we are, generate resize + // events, and set the terminal to raw mode + if haveTerminal && execConfig.Terminal { + resizechan = make(chan resize.TerminalSize) + cancel, oldTermState, err := handleTerminalAttach(ctx, resizechan) + if err != nil { + return -1, err + } + defer cancel() + defer func() { + if err := restoreTerminal(oldTermState); err != nil { + logrus.Errorf("Unable to restore terminal: %q", err) + } + }() + } + return ctr.Exec(execConfig, streams, resizechan) +} + +// StartAttachCtr starts and (if required) attaches to a container +// if you change the signature of this function from os.File to io.Writer, it will trigger a downstream +// error. we may need to just lint disable this one. +func StartAttachCtr(ctx context.Context, ctr *libpod.Container, stdout, stderr, stdin *os.File, detachKeys string, sigProxy bool, startContainer bool) error { //nolint: interfacer + resize := make(chan resize.TerminalSize) + + haveTerminal := term.IsTerminal(int(os.Stdin.Fd())) + + // Check if we are attached to a terminal. If we are, generate resize + // events, and set the terminal to raw mode + if haveTerminal && ctr.Spec().Process.Terminal { + cancel, oldTermState, err := handleTerminalAttach(ctx, resize) + if err != nil { + return err + } + defer func() { + if err := restoreTerminal(oldTermState); err != nil { + logrus.Errorf("Unable to restore terminal: %q", err) + } + }() + defer cancel() + } + + streams := new(define.AttachStreams) + streams.OutputStream = stdout + streams.ErrorStream = stderr + streams.InputStream = bufio.NewReader(stdin) + streams.AttachOutput = true + streams.AttachError = true + streams.AttachInput = true + + if stdout == nil { + logrus.Debugf("Not attaching to stdout") + streams.AttachOutput = false + } + if stderr == nil { + logrus.Debugf("Not attaching to stderr") + streams.AttachError = false + } + if stdin == nil { + logrus.Debugf("Not attaching to stdin") + streams.AttachInput = false + } + + if !startContainer { + if sigProxy { + ProxySignals(ctr) + } + + return ctr.Attach(streams, detachKeys, resize) + } + + attachChan, err := ctr.StartAndAttach(ctx, streams, detachKeys, resize, true) + if err != nil { + return err + } + + if sigProxy { + ProxySignals(ctr) + } + + if stdout == nil && stderr == nil { + fmt.Printf("%s\n", ctr.ID()) + } + + err = <-attachChan + if err != nil { + return fmt.Errorf("error attaching to container %s: %w", ctr.ID(), err) + } + + return nil +} diff --git a/pkg/domain/infra/abi/terminal/terminal_linux.go b/pkg/domain/infra/abi/terminal/terminal_linux.go deleted file mode 100644 index 222590871..000000000 --- a/pkg/domain/infra/abi/terminal/terminal_linux.go +++ /dev/null @@ -1,110 +0,0 @@ -package terminal - -import ( - "bufio" - "context" - "fmt" - "os" - - "github.com/containers/common/pkg/resize" - "github.com/containers/podman/v4/libpod" - "github.com/containers/podman/v4/libpod/define" - "github.com/sirupsen/logrus" - "golang.org/x/term" -) - -// ExecAttachCtr execs and attaches to a container -func ExecAttachCtr(ctx context.Context, ctr *libpod.Container, execConfig *libpod.ExecConfig, streams *define.AttachStreams) (int, error) { - var resizechan chan resize.TerminalSize - haveTerminal := term.IsTerminal(int(os.Stdin.Fd())) - - // Check if we are attached to a terminal. If we are, generate resize - // events, and set the terminal to raw mode - if haveTerminal && execConfig.Terminal { - resizechan = make(chan resize.TerminalSize) - cancel, oldTermState, err := handleTerminalAttach(ctx, resizechan) - if err != nil { - return -1, err - } - defer cancel() - defer func() { - if err := restoreTerminal(oldTermState); err != nil { - logrus.Errorf("Unable to restore terminal: %q", err) - } - }() - } - return ctr.Exec(execConfig, streams, resizechan) -} - -// StartAttachCtr starts and (if required) attaches to a container -// if you change the signature of this function from os.File to io.Writer, it will trigger a downstream -// error. we may need to just lint disable this one. -func StartAttachCtr(ctx context.Context, ctr *libpod.Container, stdout, stderr, stdin *os.File, detachKeys string, sigProxy bool, startContainer bool) error { //nolint: interfacer - resize := make(chan resize.TerminalSize) - - haveTerminal := term.IsTerminal(int(os.Stdin.Fd())) - - // Check if we are attached to a terminal. If we are, generate resize - // events, and set the terminal to raw mode - if haveTerminal && ctr.Spec().Process.Terminal { - cancel, oldTermState, err := handleTerminalAttach(ctx, resize) - if err != nil { - return err - } - defer func() { - if err := restoreTerminal(oldTermState); err != nil { - logrus.Errorf("Unable to restore terminal: %q", err) - } - }() - defer cancel() - } - - streams := new(define.AttachStreams) - streams.OutputStream = stdout - streams.ErrorStream = stderr - streams.InputStream = bufio.NewReader(stdin) - streams.AttachOutput = true - streams.AttachError = true - streams.AttachInput = true - - if stdout == nil { - logrus.Debugf("Not attaching to stdout") - streams.AttachOutput = false - } - if stderr == nil { - logrus.Debugf("Not attaching to stderr") - streams.AttachError = false - } - if stdin == nil { - logrus.Debugf("Not attaching to stdin") - streams.AttachInput = false - } - - if !startContainer { - if sigProxy { - ProxySignals(ctr) - } - - return ctr.Attach(streams, detachKeys, resize) - } - - attachChan, err := ctr.StartAndAttach(ctx, streams, detachKeys, resize, true) - if err != nil { - return err - } - - if sigProxy { - ProxySignals(ctr) - } - - if stdout == nil && stderr == nil { - fmt.Printf("%s\n", ctr.ID()) - } - - err = <-attachChan - if err != nil { - return fmt.Errorf("error attaching to container %s: %w", ctr.ID(), err) - } - - return nil -} diff --git a/pkg/domain/infra/abi/terminal/terminal_unsupported.go b/pkg/domain/infra/abi/terminal/terminal_unsupported.go index 8fe325736..21ed6c8d4 100644 --- a/pkg/domain/infra/abi/terminal/terminal_unsupported.go +++ b/pkg/domain/infra/abi/terminal/terminal_unsupported.go @@ -1,5 +1,5 @@ -//go:build !linux -// +build !linux +//go:build !linux && !freebsd +// +build !linux,!freebsd package terminal -- cgit v1.2.3-54-g00ecf