Merge pull request #25626 from jankaluza/24875

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

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