diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index ae18cee3d0..ff0865b821 100644 --- a/cmd/podman/common/create.go +++ b/cmd/podman/common/create.go @@ -114,21 +114,6 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions, ) _ = cmd.RegisterFlagCompletionFunc(envMergeFlagName, completion.AutocompleteNone) - envFlagName := "env" - createFlags.StringArrayP( - envFlagName, "e", Env(), - "Set environment variables in container", - ) - _ = cmd.RegisterFlagCompletionFunc(envFlagName, completion.AutocompleteNone) - - unsetenvFlagName := "unsetenv" - createFlags.StringArrayVar( - &cf.UnsetEnv, - unsetenvFlagName, []string{}, - "Unset environment default variables in container", - ) - _ = cmd.RegisterFlagCompletionFunc(unsetenvFlagName, completion.AutocompleteNone) - createFlags.BoolVar( &cf.UnsetEnvAll, "unsetenv-all", false, @@ -555,6 +540,21 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions, "Disable healthchecks on container", ) + envFlagName := "env" + createFlags.StringArrayP( + envFlagName, "e", Env(), + "Set environment variables in container", + ) + _ = cmd.RegisterFlagCompletionFunc(envFlagName, completion.AutocompleteNone) + + unsetenvFlagName := "unsetenv" + createFlags.StringArrayVar( + &cf.UnsetEnv, + unsetenvFlagName, []string{}, + "Unset environment default variables in container", + ) + _ = cmd.RegisterFlagCompletionFunc(unsetenvFlagName, completion.AutocompleteNone) + healthCmdFlagName := "health-cmd" createFlags.StringVar( &cf.HealthCmd, diff --git a/cmd/podman/containers/update.go b/cmd/podman/containers/update.go index 429bd96821..6f6ed362c4 100644 --- a/cmd/podman/containers/update.go +++ b/cmd/podman/containers/update.go @@ -163,6 +163,22 @@ func update(cmd *cobra.Command, args []string) error { } } + if cmd.Flags().Changed("env") { + env, err := cmd.Flags().GetStringArray("env") + if err != nil { + return err + } + opts.Env = env + } + + if cmd.Flags().Changed("unsetenv") { + env, err := cmd.Flags().GetStringArray("unsetenv") + if err != nil { + return err + } + opts.UnsetEnv = env + } + rep, err := registry.ContainerEngine().ContainerUpdate(context.Background(), opts) if err != nil { return err diff --git a/docs/source/markdown/options/env.update.md b/docs/source/markdown/options/env.update.md new file mode 100644 index 0000000000..dc7af0e491 --- /dev/null +++ b/docs/source/markdown/options/env.update.md @@ -0,0 +1,13 @@ +####> This option file is used in: +####> podman update +####> If file is edited, make sure the changes +####> are applicable to all of those. +#### **--env**, **-e**=*env* + +Add a value (e.g. env=*value*) to the container. Can be used multiple times. +If the value already exists in the container, it is overridden. +To remove an environment variable from the container, use the `--unsetenv` +option. + +Note that the env updates only affect the main container process after +the next start. diff --git a/docs/source/markdown/options/unsetenv.update.md b/docs/source/markdown/options/unsetenv.update.md new file mode 100644 index 0000000000..62ef47f0fb --- /dev/null +++ b/docs/source/markdown/options/unsetenv.update.md @@ -0,0 +1,10 @@ +####> This option file is used in: +####> podman update +####> If file is edited, make sure the changes +####> are applicable to all of those. +#### **--unsetenv**=*env* + +Unset environment variables from the container. + +Note that the env updates only affect the main container process after +the next start. diff --git a/docs/source/markdown/podman-update.1.md.in b/docs/source/markdown/podman-update.1.md.in index de8a7dc252..68b713f760 100644 --- a/docs/source/markdown/podman-update.1.md.in +++ b/docs/source/markdown/podman-update.1.md.in @@ -42,6 +42,8 @@ Updates the configuration of an existing container, allowing changes to resource @@option device-write-iops +@@option env.update + @@option health-cmd @@option health-interval @@ -90,6 +92,8 @@ Changing this setting resets the timer, depending on the state of the container. @@option restart +@@option unsetenv.update + ## EXAMPLEs diff --git a/libpod/container_api.go b/libpod/container_api.go index ddaa6ed950..47e1797a5f 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -14,8 +14,8 @@ import ( "github.com/containers/common/pkg/resize" "github.com/containers/podman/v5/libpod/define" "github.com/containers/podman/v5/libpod/events" + "github.com/containers/podman/v5/pkg/domain/entities" "github.com/containers/storage/pkg/archive" - spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -106,10 +106,10 @@ func (c *Container) Start(ctx context.Context, recursive bool) error { // Update updates the given container. // Either resource limits, restart policies, or HealthCheck configuration can be updated. -// Either resources, restartPolicy or changedHealthCheckConfiguration must not be nil. +// Either resources, restartPolicy or changedHealthCheckConfiguration must not be nil in the updateOptions. // If restartRetries is not nil, restartPolicy must be set and must be "on-failure". // Nil values of changedHealthCheckConfiguration are not updated. -func (c *Container) Update(resources *spec.LinuxResources, restartPolicy *string, restartRetries *uint, changedHealthCheckConfiguration *define.UpdateHealthCheckConfig) error { +func (c *Container) Update(updateOptions *entities.ContainerUpdateOptions) error { if !c.batched { c.lock.Lock() defer c.lock.Unlock() @@ -123,7 +123,7 @@ func (c *Container) Update(resources *spec.LinuxResources, restartPolicy *string return fmt.Errorf("container %s is being removed, cannot update: %w", c.ID(), define.ErrCtrStateInvalid) } - healthCheckConfig, changedHealthCheck, err := GetNewHealthCheckConfig(&HealthCheckConfig{Schema2HealthConfig: c.HealthCheckConfig()}, *changedHealthCheckConfiguration) + healthCheckConfig, changedHealthCheck, err := GetNewHealthCheckConfig(&HealthCheckConfig{Schema2HealthConfig: c.HealthCheckConfig()}, *updateOptions.ChangedHealthCheckConfiguration) if err != nil { return err } @@ -136,7 +136,7 @@ func (c *Container) Update(resources *spec.LinuxResources, restartPolicy *string } } - startupHealthCheckConfig, changedStartupHealthCheck, err := GetNewHealthCheckConfig(&StartupHealthCheckConfig{StartupHealthCheck: c.Config().StartupHealthCheckConfig}, *changedHealthCheckConfiguration) + startupHealthCheckConfig, changedStartupHealthCheck, err := GetNewHealthCheckConfig(&StartupHealthCheckConfig{StartupHealthCheck: c.Config().StartupHealthCheckConfig}, *updateOptions.ChangedHealthCheckConfiguration) if err != nil { return err } @@ -149,7 +149,7 @@ func (c *Container) Update(resources *spec.LinuxResources, restartPolicy *string } } - globalHealthCheckOptions, err := changedHealthCheckConfiguration.GetNewGlobalHealthCheck() + globalHealthCheckOptions, err := updateOptions.ChangedHealthCheckConfiguration.GetNewGlobalHealthCheck() if err != nil { return err } @@ -158,7 +158,7 @@ func (c *Container) Update(resources *spec.LinuxResources, restartPolicy *string } defer c.newContainerEvent(events.Update) - return c.update(resources, restartPolicy, restartRetries) + return c.update(updateOptions) } // Attach to a container. diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 7ec58be578..51be31196a 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -34,6 +34,8 @@ import ( "github.com/containers/podman/v5/libpod/events" "github.com/containers/podman/v5/libpod/shutdown" "github.com/containers/podman/v5/pkg/ctime" + "github.com/containers/podman/v5/pkg/domain/entities" + envLib "github.com/containers/podman/v5/pkg/env" "github.com/containers/podman/v5/pkg/lookup" "github.com/containers/podman/v5/pkg/rootless" "github.com/containers/podman/v5/pkg/selinux" @@ -2793,11 +2795,11 @@ func (c *Container) extractSecretToCtrStorage(secr *ContainerSecret) error { // Update a container's resources or restart policy after creation. // At least one of resources or restartPolicy must not be nil. -func (c *Container) update(resources *spec.LinuxResources, restartPolicy *string, restartRetries *uint) error { - if resources == nil && restartPolicy == nil { +func (c *Container) update(updateOptions *entities.ContainerUpdateOptions) error { + if updateOptions.Resources == nil && updateOptions.RestartPolicy == nil { return fmt.Errorf("must provide at least one of resources and restartPolicy to update a container: %w", define.ErrInvalidArg) } - if restartRetries != nil && restartPolicy == nil { + if updateOptions.RestartRetries != nil && updateOptions.RestartPolicy == nil { return fmt.Errorf("must provide restart policy if updating restart retries: %w", define.ErrInvalidArg) } @@ -2810,38 +2812,50 @@ func (c *Container) update(resources *spec.LinuxResources, restartPolicy *string oldRestart := c.config.RestartPolicy oldRetries := c.config.RestartRetries - if restartPolicy != nil { - if err := define.ValidateRestartPolicy(*restartPolicy); err != nil { + if updateOptions.RestartPolicy != nil { + if err := define.ValidateRestartPolicy(*updateOptions.RestartPolicy); err != nil { return err } - if restartRetries != nil { - if *restartPolicy != define.RestartPolicyOnFailure { + if updateOptions.RestartRetries != nil { + if *updateOptions.RestartPolicy != define.RestartPolicyOnFailure { return fmt.Errorf("cannot set restart policy retries unless policy is on-failure: %w", define.ErrInvalidArg) } } - c.config.RestartPolicy = *restartPolicy - if restartRetries != nil { - c.config.RestartRetries = *restartRetries + c.config.RestartPolicy = *updateOptions.RestartPolicy + if updateOptions.RestartRetries != nil { + c.config.RestartRetries = *updateOptions.RestartRetries } else { c.config.RestartRetries = 0 } } - if resources != nil { + if updateOptions.Resources != nil { if c.config.Spec.Linux == nil { c.config.Spec.Linux = new(spec.Linux) } - resourcesToUpdate, err := json.Marshal(resources) + resourcesToUpdate, err := json.Marshal(updateOptions.Resources) if err != nil { return err } if err := json.Unmarshal(resourcesToUpdate, c.config.Spec.Linux.Resources); err != nil { return err } - resources = c.config.Spec.Linux.Resources + updateOptions.Resources = c.config.Spec.Linux.Resources + } + + if len(updateOptions.Env) != 0 { + c.config.Spec.Process.Env = envLib.Slice(envLib.Join(envLib.Map(c.config.Spec.Process.Env), envLib.Map(updateOptions.Env))) + } + + if len(updateOptions.UnsetEnv) != 0 { + envMap := envLib.Map(c.config.Spec.Process.Env) + for _, e := range updateOptions.UnsetEnv { + delete(envMap, e) + } + c.config.Spec.Process.Env = envLib.Slice(envMap) } if err := c.runtime.state.SafeRewriteContainerConfig(c, "", "", c.config); err != nil { @@ -2852,22 +2866,28 @@ func (c *Container) update(resources *spec.LinuxResources, restartPolicy *string return err } - if c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStatePaused) && resources != nil { + if c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStatePaused) && + (updateOptions.Resources != nil || updateOptions.Env != nil || updateOptions.UnsetEnv != nil) { // So `podman inspect` on running containers sources its OCI spec from disk. // To keep inspect accurate we need to update the on-disk OCI spec. onDiskSpec, err := c.specFromState() if err != nil { return fmt.Errorf("retrieving on-disk OCI spec to update: %w", err) } - if onDiskSpec.Linux == nil { - onDiskSpec.Linux = new(spec.Linux) + if updateOptions.Resources != nil { + if onDiskSpec.Linux == nil { + onDiskSpec.Linux = new(spec.Linux) + } + onDiskSpec.Linux.Resources = updateOptions.Resources + } + if len(updateOptions.Env) != 0 || len(updateOptions.UnsetEnv) != 0 { + onDiskSpec.Process.Env = c.config.Spec.Process.Env } - onDiskSpec.Linux.Resources = resources if err := c.saveSpec(onDiskSpec); err != nil { logrus.Errorf("Unable to update container %s OCI spec - `podman inspect` may not be accurate until container is restarted: %v", c.ID(), err) } - if err := c.ociRuntime.UpdateContainer(c, resources); err != nil { + if err := c.ociRuntime.UpdateContainer(c, updateOptions.Resources); err != nil { return err } } diff --git a/pkg/api/handlers/compat/containers.go b/pkg/api/handlers/compat/containers.go index 0a26735bd1..e6fae78e84 100644 --- a/pkg/api/handlers/compat/containers.go +++ b/pkg/api/handlers/compat/containers.go @@ -782,7 +782,14 @@ func UpdateContainer(w http.ResponseWriter, r *http.Request) { restartRetries = &localRetries } - if err := ctr.Update(resources, restartPolicy, restartRetries, &define.UpdateHealthCheckConfig{}); err != nil { + updateOptions := &entities.ContainerUpdateOptions{ + Resources: resources, + ChangedHealthCheckConfiguration: &define.UpdateHealthCheckConfig{}, + RestartPolicy: restartPolicy, + RestartRetries: restartRetries, + } + + if err := ctr.Update(updateOptions); err != nil { utils.Error(w, http.StatusInternalServerError, fmt.Errorf("updating container: %w", err)) return } diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go index bd6162c2ce..a7ed9283b3 100644 --- a/pkg/api/handlers/libpod/containers.go +++ b/pkg/api/handlers/libpod/containers.go @@ -455,7 +455,16 @@ func UpdateContainer(w http.ResponseWriter, r *http.Request) { return } - err = ctr.Update(resourceLimits, restartPolicy, restartRetries, &options.UpdateHealthCheckConfig) + updateOptions := &entities.ContainerUpdateOptions{ + Resources: resourceLimits, + ChangedHealthCheckConfiguration: &options.UpdateHealthCheckConfig, + RestartPolicy: restartPolicy, + RestartRetries: restartRetries, + Env: options.Env, + UnsetEnv: options.UnsetEnv, + } + + err = ctr.Update(updateOptions) if err != nil { utils.InternalServerError(w, err) return diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go index 278fa2316a..2d85470854 100644 --- a/pkg/api/handlers/types.go +++ b/pkg/api/handlers/types.go @@ -79,6 +79,8 @@ type UpdateEntities struct { specs.LinuxResources define.UpdateHealthCheckConfig define.UpdateContainerDevicesLimits + Env []string + UnsetEnv []string } type Info struct { diff --git a/pkg/bindings/containers/update.go b/pkg/bindings/containers/update.go index 786802434e..1279095b92 100644 --- a/pkg/bindings/containers/update.go +++ b/pkg/bindings/containers/update.go @@ -30,6 +30,8 @@ func Update(ctx context.Context, options *types.ContainerUpdateOptions) (string, LinuxResources: *options.Resources, UpdateHealthCheckConfig: *options.ChangedHealthCheckConfiguration, UpdateContainerDevicesLimits: *options.DevicesLimits, + Env: options.Env, + UnsetEnv: options.UnsetEnv, } requestData, err := jsoniter.MarshalToString(updateEntities) if err != nil { diff --git a/pkg/domain/entities/types/containers.go b/pkg/domain/entities/types/containers.go index 1f16b54d23..a02a700cde 100644 --- a/pkg/domain/entities/types/containers.go +++ b/pkg/domain/entities/types/containers.go @@ -51,12 +51,16 @@ type ContainerUpdateOptions struct { // - DevicesLimits to Limit device // - RestartPolicy to change restart policy // - RestartRetries to change restart retries + // - Env to change the environment variables. + // - UntsetEnv to unset the environment variables. Specgen *specgen.SpecGenerator Resources *specs.LinuxResources DevicesLimits *define.UpdateContainerDevicesLimits ChangedHealthCheckConfiguration *define.UpdateHealthCheckConfig RestartPolicy *string RestartRetries *uint + Env []string + UnsetEnv []string } func (u *ContainerUpdateOptions) ProcessSpecgen() { diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index ff1d44ce35..52ff8797c6 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -1803,12 +1803,12 @@ func (ic *ContainerEngine) ContainerUpdate(ctx context.Context, updateOptions *e } container := containers[0].Container - resourceLimits, err := specgenutil.UpdateMajorAndMinorNumbers(updateOptions.Resources, updateOptions.DevicesLimits) + updateOptions.Resources, err = specgenutil.UpdateMajorAndMinorNumbers(updateOptions.Resources, updateOptions.DevicesLimits) if err != nil { return "", err } - if err = container.Update(resourceLimits, updateOptions.RestartPolicy, updateOptions.RestartRetries, updateOptions.ChangedHealthCheckConfiguration); err != nil { + if err = container.Update(updateOptions); err != nil { return "", err } return containers[0].ID(), nil diff --git a/test/e2e/update_test.go b/test/e2e/update_test.go index 04a8532209..7950b1ffe6 100644 --- a/test/e2e/update_test.go +++ b/test/e2e/update_test.go @@ -8,6 +8,7 @@ import ( "github.com/containers/storage/pkg/fileutils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gexec" ) var _ = Describe("Podman update", func() { @@ -243,4 +244,65 @@ var _ = Describe("Podman update", func() { podmanTest.CheckContainerSingleField(testCtr, restartPolicyName, "always") podmanTest.CheckContainerSingleField(testCtr, restartPolicyRetries, "0") }) + + It("podman update sets/unsets environment variables", func() { + testCtr := "test-ctr-name" + + // Test that the variable is not set. + ctr1 := podmanTest.Podman([]string{"run", "-t", "--name", testCtr, ALPINE, "printenv", "FOO"}) + ctr1.WaitWithDefaultTimeout() + Expect(ctr1).Should(Exit(1)) + + // Test that variable can be set and existing variables are not overridden. + update := podmanTest.Podman([]string{"update", "--env", "FOO=BAR", testCtr}) + update.WaitWithDefaultTimeout() + Expect(update).Should(ExitCleanly()) + + session := podmanTest.Podman([]string{"start", "--attach", testCtr}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + env := session.OutputToString() + Expect(env).To(ContainSubstring("BAR")) + + session = podmanTest.Podman([]string{"inspect", testCtr, "--format", "{{.Config.Env}}"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + env = session.OutputToString() + Expect(env).To(ContainSubstring("FOO=BAR")) + Expect(env).To(ContainSubstring("PATH=")) + + // Test that variable can be updated. + update = podmanTest.Podman([]string{"update", "--env", "FOO=RAB", testCtr}) + update.WaitWithDefaultTimeout() + Expect(update).Should(ExitCleanly()) + + session = podmanTest.Podman([]string{"start", "--attach", testCtr}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + env = session.OutputToString() + Expect(env).To(ContainSubstring("RAB")) + + session = podmanTest.Podman([]string{"inspect", testCtr, "--format", "{{.Config.Env}}"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + env = session.OutputToString() + Expect(env).To(ContainSubstring("FOO=RAB")) + Expect(env).To(ContainSubstring("PATH=")) + + // Test that variable can be unset. + update = podmanTest.Podman([]string{"update", "--unsetenv", "FOO", testCtr}) + update.WaitWithDefaultTimeout() + Expect(update).Should(ExitCleanly()) + + session = podmanTest.Podman([]string{"start", "--attach", testCtr}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(1)) + + session = podmanTest.Podman([]string{"inspect", testCtr, "--format", "{{.Config.Env}}"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + env = session.OutputToString() + Expect(env).ToNot(ContainSubstring("FOO")) + Expect(env).To(ContainSubstring("PATH=")) + }) })