podman: split env variables in env and overrides

There are three different priorities for applying env variables:

1) environment/config file environment variables
2) image's config
3) user overrides (--env)

The third kind are known to the client, while the default config and image's
config is handled by the backend.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

cmd/podman/common/create.go

@@ -156,8 +156,7 @@ func GetCreateFlags(cf *ContainerCLIOpts) *pflag.FlagSet {
 	createFlags.String("entrypoint", "",
 		"Overwrite the default ENTRYPOINT of the image",
 	)
-	createFlags.StringArrayVarP(
-		&cf.env,
+	createFlags.StringArrayP(
 		"env", "e", containerConfig.Env(),
 		"Set environment variables in container",
 	)

cmd/podman/common/create_opts.go

@@ -32,7 +32,7 @@ type ContainerCLIOpts struct {
 	DeviceWriteBPs    []string
 	DeviceWriteIOPs   []string
 	Entrypoint        *string
-	env               []string
+	Env               []string
 	EnvHost           bool
 	EnvFile           []string
 	Expose            []string

cmd/podman/common/specgen.go

@@ -335,15 +335,12 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
 		env = envLib.Join(env, fileEnv)
 	}
 
-	// env overrides any previous variables
-	if cmdLineEnv := c.env; len(cmdLineEnv) > 0 {
-		parsedEnv, err := envLib.ParseSlice(cmdLineEnv)
+	parsedEnv, err := envLib.ParseSlice(c.Env)
 	if err != nil {
 		return err
 	}
-		env = envLib.Join(env, parsedEnv)
-	}
-	s.Env = env
+
+	s.Env = envLib.Join(env, parsedEnv)
 
 	// LABEL VARIABLES
 	labels, err := parse.GetAllLabels(c.LabelFile, c.Label)

cmd/podman/containers/create.go

@@ -55,6 +55,11 @@ func createFlags(flags *pflag.FlagSet) {
 	flags.AddFlagSet(common.GetCreateFlags(&cliVals))
 	flags.AddFlagSet(common.GetNetFlags())
 	flags.SetNormalizeFunc(common.AliasFlags)
+	if registry.IsRemote() {
+		_ = flags.MarkHidden("authfile")
+		_ = flags.MarkHidden("env-host")
+		_ = flags.MarkHidden("http-proxy")
+	}
 }
 
 func init() {
@@ -170,6 +175,13 @@ func createInit(c *cobra.Command) error {
 		val := c.Flag("entrypoint").Value.String()
 		cliVals.Entrypoint = &val
 	}
+	if c.Flags().Changed("env") {
+		env, err := c.Flags().GetStringArray("env")
+		if err != nil {
+			return errors.Wrapf(err, "retrieve env flag")
+		}
+		cliVals.Env = env
+	}
 
 	// Docker-compatibility: the "-h" flag for run/create is reserved for
 	// the hostname (see https://github.com/containers/libpod/issues/1367).

cmd/podman/containers/run.go

@@ -60,6 +60,8 @@ func runFlags(flags *pflag.FlagSet) {
 	flags.BoolVar(&runRmi, "rmi", false, "Remove container image unless used by other containers")
 	if registry.IsRemote() {
 		_ = flags.MarkHidden("authfile")
+		_ = flags.MarkHidden("env-host")
+		_ = flags.MarkHidden("http-proxy")
 	}
 }
 func init() {

docs/source/markdown/podman-create.1.md

@@ -278,7 +278,7 @@ See [**Environment**](#environment) note below for precedence and examples.
 
 **--env-host**=*true|false*
 
-Use host environment inside of the container. See **Environment** note below for precedence.
+Use host environment inside of the container. See **Environment** note below for precedence. (Not available for remote commands)
 
 **--env-file**=*file*
 
@@ -347,7 +347,7 @@ the container should not use any proxy.  Proxy environment variables specified
 for the container in any other way will override the values that would have
 been passed through from the host.  (Other ways to specify the proxy for the
 container include passing the values with the `--env` flag, or hard coding the
-proxy environment at container build time.)
+proxy environment at container build time.)  (Not available for remote commands)
 
 For example, to disable passing these environment variables from host to
 container:

docs/source/markdown/podman-run.1.md

@@ -294,7 +294,7 @@ See [**Environment**](#environment) note below for precedence and examples.
 
 **--env-host**=**true**|**false**
 
-Use host environment inside of the container. See **Environment** note below for precedence.
+Use host environment inside of the container. See **Environment** note below for precedence. (Not available for remote commands)
 
 **--env-file**=*file*
 
@@ -363,7 +363,7 @@ the container should not use any proxy.  Proxy environment variables specified
 for the container in any other way will override the values that would have
 been passed through from the host.  (Other ways to specify the proxy for the
 container include passing the values with the **--env** flag, or hard coding the
-proxy environment at container build time.)
+proxy environment at container build time.) (Not available for remote commands)
 
 Defaults to **true**.
 

pkg/specgen/generate/container.go

@@ -9,6 +9,7 @@ import (
 	envLib "github.com/containers/libpod/pkg/env"
 	"github.com/containers/libpod/pkg/signal"
 	"github.com/containers/libpod/pkg/specgen"
+	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 )
 
@@ -48,24 +49,28 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 		s.StopSignal = &sig
 	}
 
+	rtc, err := r.GetConfig()
+	if err != nil {
+		return err
+	}
+	// Get Default Environment
+	defaultEnvs, err := envLib.ParseSlice(rtc.Containers.Env)
+	if err != nil {
+		return errors.Wrap(err, "Env fields in containers.conf failed to parse")
+	}
+
 	// Image envs from the image if they don't exist
-	// already
-	env, err := newImage.Env(ctx)
+	// already, overriding the default environments
+	imageEnvs, err := newImage.Env(ctx)
 	if err != nil {
 		return err
 	}
 
-	if len(env) > 0 {
-		envs, err := envLib.ParseSlice(env)
+	envs, err := envLib.ParseSlice(imageEnvs)
 	if err != nil {
-			return err
-		}
-		for k, v := range envs {
-			if _, exists := s.Env[k]; !exists {
-				s.Env[v] = k
-			}
-		}
+		return errors.Wrap(err, "Env fields from image failed to parse")
 	}
+	s.Env = envLib.Join(envLib.Join(defaultEnvs, envs), s.Env)
 
 	labels, err := newImage.Labels(ctx)
 	if err != nil {

pkg/specgen/specgen.go

@@ -47,6 +47,7 @@ type ContainerBasicConfig struct {
 	// Optional.
 	Env map[string]string `json:"env,omitempty"`
 	// Terminal is whether the container will create a PTY.
+	// Optional.
 	Terminal bool `json:"terminal,omitempty"`
 	// Stdin is whether the container will keep its STDIN open.
 	Stdin bool `json:"stdin,omitempty"`

test/e2e/build_test.go

@@ -177,7 +177,6 @@ var _ = Describe("Podman build", func() {
 	})
 
 	It("podman Test PATH in built image", func() {
-		Skip(v2fail) // Run error - we don't set data from the image (i.e., PATH) yet
 		path := "/tmp:/bin:/usr/bin:/usr/sbin"
 		session := podmanTest.PodmanNoCache([]string{
 			"build", "-f", "build/basicalpine/Containerfile.path", "-t", "test-path",

test/e2e/run_env_test.go

@@ -91,7 +91,8 @@ var _ = Describe("Podman run", func() {
 		Expect(match).Should(BeTrue())
 	})
 
-	It("podman run --host-env environment test", func() {
+	It("podman run --env-host environment test", func() {
+		SkipIfRemote()
 		env := append(os.Environ(), "FOO=BAR")
 		session := podmanTest.PodmanAsUser([]string{"run", "--rm", "--env-host", ALPINE, "/bin/printenv", "FOO"}, 0, 0, "", env)
 
@@ -109,6 +110,7 @@ var _ = Describe("Podman run", func() {
 	})
 
 	It("podman run --http-proxy test", func() {
+		SkipIfRemote()
 		os.Setenv("http_proxy", "1.2.3.4")
 		session := podmanTest.Podman([]string{"run", "--rm", ALPINE, "printenv", "http_proxy"})
 		session.WaitWithDefaultTimeout()