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>

11 files changed, 46 insertions, 29 deletions

diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go
index 53f4a8fa2..7086dc839 100644
--- a/cmd/podman/common/create.go
+++ b/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",
 	)
diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go
index c275b1674..8b38e3b47 100644
--- a/cmd/podman/common/create_opts.go
+++ b/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
diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go
index 3e9772576..9a2345064 100644
--- a/cmd/podman/common/specgen.go
+++ b/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)
-		if err != nil {
-			return err
-		}
-		env = envLib.Join(env, parsedEnv)
+	parsedEnv, err := envLib.ParseSlice(c.Env)
+	if err != nil {
+		return err
 	}
-	s.Env = env
+
+	s.Env = envLib.Join(env, parsedEnv)
 
 	// LABEL VARIABLES
 	labels, err := parse.GetAllLabels(c.LabelFile, c.Label)
diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go
index 7927da04d..2ecdda2e0 100644
--- a/cmd/podman/containers/create.go
+++ b/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).
diff --git a/cmd/podman/containers/run.go b/cmd/podman/containers/run.go
index f72446cb6..5f3ea9ef4 100644
--- a/cmd/podman/containers/run.go
+++ b/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() {
diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md
index f0494ca7d..2fd8512a6 100644
--- a/docs/source/markdown/podman-create.1.md
+++ b/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:
diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md
index b21eb9da9..dd221590d 100644
--- a/docs/source/markdown/podman-run.1.md
+++ b/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**.
 
diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go
index 92a2b4d35..e4bd2991a 100644
--- a/pkg/specgen/generate/container.go
+++ b/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)
-		if err != nil {
-			return err
-		}
-		for k, v := range envs {
-			if _, exists := s.Env[k]; !exists {
-				s.Env[v] = k
-			}
-		}
+	envs, err := envLib.ParseSlice(imageEnvs)
+	if err != nil {
+		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 {
diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go
index 4ad6dd6fb..bb01a5d14 100644
--- a/pkg/specgen/specgen.go
+++ b/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"`
diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go
index 76651283a..9e41fd231 100644
--- a/test/e2e/build_test.go
+++ b/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",
diff --git a/test/e2e/run_env_test.go b/test/e2e/run_env_test.go
index 867913a08..2adf21171 100644
--- a/test/e2e/run_env_test.go
+++ b/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()