From ad8e0e5e49a96146d52cfa41945afbe973ba30af Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Wed, 19 Feb 2020 15:55:14 +0100
Subject: consolidate env handling into pkg/env

Env-variable related code is scattered across several packages making it
hard to maintain and extend.  Consolidate the code into a new pkg/env
package.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
---
 cmd/podman/shared/create.go      | 71 ++++++++++++++++++++++------------------
 cmd/podman/shared/parse/parse.go | 19 ++---------
 2 files changed, 42 insertions(+), 48 deletions(-)

(limited to 'cmd/podman/shared')

diff --git a/cmd/podman/shared/create.go b/cmd/podman/shared/create.go
index 0814eeba3..0ce578bef 100644
--- a/cmd/podman/shared/create.go
+++ b/cmd/podman/shared/create.go
@@ -18,6 +18,7 @@ import (
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/libpod/image"
 	ann "github.com/containers/libpod/pkg/annotations"
+	envLib "github.com/containers/libpod/pkg/env"
 	"github.com/containers/libpod/pkg/errorhandling"
 	"github.com/containers/libpod/pkg/inspect"
 	ns "github.com/containers/libpod/pkg/namespaces"
@@ -473,19 +474,49 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 	}
 
 	// ENVIRONMENT VARIABLES
-	env := EnvVariablesFromData(data)
+	//
+	// Precedence order (higher index wins):
+	//  1) env-host, 2) image data, 3) env-file, 4) env
+	var env map[string]string
+
+	// Start with env-host
 	if c.Bool("env-host") {
-		for _, e := range os.Environ() {
-			pair := strings.SplitN(e, "=", 2)
-			if _, ok := env[pair[0]]; !ok {
-				if len(pair) > 1 {
-					env[pair[0]] = pair[1]
-				}
+		osEnv, err := envLib.ParseSlice(os.Environ())
+		if err != nil {
+			return nil, errors.Wrap(err, "error parsing host environment variables")
+		}
+		env = osEnv
+	}
+
+	// Image data overrides any previous variables
+	if data != nil {
+		configEnv, err := envLib.ParseSlice(data.Config.Env)
+		if err != nil {
+			return nil, errors.Wrap(err, "error pasing image environment variables")
+		}
+		env = envLib.Join(env, configEnv)
+	}
+
+	// env-file overrides any previous variables
+	if c.IsSet("env-file") {
+		for _, f := range c.StringSlice("env-file") {
+			fileEnv, err := envLib.ParseFile(f)
+			if err != nil {
+				return nil, err
 			}
+			// File env is overridden by env.
+			env = envLib.Join(env, fileEnv)
 		}
 	}
-	if err := parse.ReadKVStrings(env, c.StringSlice("env-file"), c.StringArray("env")); err != nil {
-		return nil, errors.Wrapf(err, "unable to process environment variables")
+
+	// env overrides any previous variables
+	cmdlineEnv := c.StringSlice("env")
+	if len(cmdlineEnv) > 0 {
+		parsedEnv, err := envLib.ParseSlice(cmdlineEnv)
+		if err != nil {
+			return nil, err
+		}
+		env = envLib.Join(env, parsedEnv)
 	}
 
 	// LABEL VARIABLES
@@ -822,28 +853,6 @@ func CreateContainerFromCreateConfig(r *libpod.Runtime, createConfig *cc.CreateC
 	return ctr, nil
 }
 
-var defaultEnvVariables = map[string]string{
-	"PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
-	"TERM": "xterm",
-}
-
-// EnvVariablesFromData gets sets the default environment variables
-// for containers, and reads the variables from the image data, if present.
-func EnvVariablesFromData(data *inspect.ImageData) map[string]string {
-	env := defaultEnvVariables
-	if data != nil {
-		for _, e := range data.Config.Env {
-			split := strings.SplitN(e, "=", 2)
-			if len(split) > 1 {
-				env[split[0]] = split[1]
-			} else {
-				env[split[0]] = ""
-			}
-		}
-	}
-	return env
-}
-
 func makeHealthCheckFromCli(c *GenericCLIResults) (*manifest.Schema2HealthConfig, error) {
 	inCommand := c.String("healthcheck-command")
 	inInterval := c.String("healthcheck-interval")
diff --git a/cmd/podman/shared/parse/parse.go b/cmd/podman/shared/parse/parse.go
index 79449029d..03cda268c 100644
--- a/cmd/podman/shared/parse/parse.go
+++ b/cmd/podman/shared/parse/parse.go
@@ -90,6 +90,8 @@ func GetAllLabels(labelFile, inputLabels []string) (map[string]string, error) {
 		// all environment variables, even those sourced from files, but
 		// that would require a substantial rework.
 		if err := parseEnvFile(labels, file); err != nil {
+			// FIXME: parseEnvFile is using parseEnv, so we need to add extra
+			// logic for labels.
 			return nil, err
 		}
 	}
@@ -107,23 +109,6 @@ func GetAllLabels(labelFile, inputLabels []string) (map[string]string, error) {
 	return labels, nil
 }
 
-// reads a file of line terminated key=value pairs, and overrides any keys
-// present in the file with additional pairs specified in the override parameter
-// for env-file and labels-file flags
-func ReadKVStrings(env map[string]string, files []string, override []string) error {
-	for _, ef := range files {
-		if err := parseEnvFile(env, ef); err != nil {
-			return err
-		}
-	}
-	for _, line := range override {
-		if err := parseEnv(env, line); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 func parseEnv(env map[string]string, line string) error {
 	data := strings.SplitN(line, "=", 2)
 
-- 
cgit v1.2.3-54-g00ecf