Add --env and --unsetenv to podman update.

The --env is used to add new environment variable to container or
override the existing one. The --unsetenv is used to remove
the environment variable.

It is done by sharing "env" and "unsetenv" flags between both
"update" and "create" commands and later handling these flags
in the "update" command handler.

The list of environment variables to add/remove is stored
in newly added variables in the ContainerUpdateOptions.

The Container.Update API call is refactored to take
the ContainerUpdateOptions as an input to limit the number of its
arguments.

The Env and UnsetEnv lists are later handled using the envLib
package and the Container is updated.

The remote API is also extended to handle Env and EnvUnset.

Fixes: #24875

Signed-off-by: Jan Kaluza <jkaluza@redhat.com>

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,

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

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.

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.

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
 

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.

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
 		}
 	}

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
 	}

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

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 {

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 {

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() {

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

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="))
+	})
 })