From 57599f0075ccab859d4158f7ee891b9b971c731f Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 7 Nov 2017 10:03:46 -0500 Subject: Fix up handling of environment variables The way docker works is if a user specifies a non `-e Name=Value`, IE just a `-e Name`, then the environment variable Name from the clients OS.ENV is used. Also by default Docker containers run with the HOSTNAME environment set to the HOSTNAME specified for the container. Signed-off-by: Daniel J Walsh Closes: #21 Approved by: baude --- cmd/kpod/create.go | 31 ++++++++++++---------- cmd/kpod/create_cli.go | 32 +---------------------- cmd/kpod/create_cli_test.go | 29 --------------------- cmd/kpod/parse.go | 63 +++++++++++++++++++++++++-------------------- cmd/kpod/spec.go | 9 +++++-- 5 files changed, 60 insertions(+), 104 deletions(-) (limited to 'cmd/kpod') diff --git a/cmd/kpod/create.go b/cmd/kpod/create.go index 0a0e31c40..73959ed02 100644 --- a/cmd/kpod/create.go +++ b/cmd/kpod/create.go @@ -27,7 +27,10 @@ const ( ) var ( - defaultEnvVariables = []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "TERM=xterm"} + defaultEnvVariables = map[string]string{ + "PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "TERM": "xterm", + } ) type createResourceConfig struct { @@ -64,16 +67,16 @@ type createConfig struct { cidFile string cgroupParent string // cgroup-parent command []string - detach bool // detach - devices []*pb.Device // device - dnsOpt []string //dns-opt - dnsSearch []string //dns-search - dnsServers []string //dns - entrypoint string //entrypoint - env []string //env - expose []string //expose - groupAdd []uint32 // group-add - hostname string //hostname + detach bool // detach + devices []*pb.Device // device + dnsOpt []string //dns-opt + dnsSearch []string //dns-search + dnsServers []string //dns + entrypoint string //entrypoint + env map[string]string //env + expose []string //expose + groupAdd []uint32 // group-add + hostname string //hostname image string interactive bool //interactive ip6Address string //ipv6 @@ -264,8 +267,8 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime) (*createConfig, er return &createConfig{}, errors.Wrapf(err, "unable to process labels") } // ENVIRONMENT VARIABLES - env, err := getAllEnvironmentVariables(c.StringSlice("env-file"), c.StringSlice("env")) - if err != nil { + env := defaultEnvVariables + if err := readKVStrings(env, c.StringSlice("env-file"), c.StringSlice("env")); err != nil { return &createConfig{}, errors.Wrapf(err, "unable to process environment variables") } @@ -338,7 +341,7 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime) (*createConfig, er dnsServers: c.StringSlice("dns"), entrypoint: c.String("entrypoint"), env: env, - expose: c.StringSlice("env"), + expose: c.StringSlice("expose"), groupAdd: groupAdd, hostname: c.String("hostname"), image: image, diff --git a/cmd/kpod/create_cli.go b/cmd/kpod/create_cli.go index eaad46591..91e984785 100644 --- a/cmd/kpod/create_cli.go +++ b/cmd/kpod/create_cli.go @@ -7,44 +7,14 @@ import ( ) func getAllLabels(labelFile, inputLabels []string) (map[string]string, error) { - var labelValues []string labels := make(map[string]string) - labelValues, labelErr := readKVStrings(labelFile, inputLabels) + labelErr := readKVStrings(labels, labelFile, inputLabels) if labelErr != nil { return labels, errors.Wrapf(labelErr, "unable to process labels from --label and label-file") } - // Process KEY=VALUE stringslice in string map for WithLabels func - if len(labelValues) > 0 { - for _, i := range labelValues { - spliti := strings.Split(i, "=") - if len(spliti) < 2 { - return labels, errors.Errorf("labels must be in KEY=VALUE format: %s is invalid", i) - } - labels[spliti[0]] = spliti[1] - } - } return labels, nil } -func getAllEnvironmentVariables(envFiles, envInput []string) ([]string, error) { - env, err := readKVStrings(envFiles, envInput) - if err != nil { - return []string{}, errors.Wrapf(err, "unable to process variables from --env and --env-file") - } - // Add default environment variables if nothing defined - if len(env) == 0 { - env = append(env, defaultEnvVariables...) - } - // Each environment variable must be in the K=V format - for _, i := range env { - spliti := strings.Split(i, "=") - if len(spliti) != 2 { - return env, errors.Errorf("environment variables must be in the format KEY=VALUE: %s is invalid", i) - } - } - return env, nil -} - func convertStringSliceToMap(strSlice []string, delimiter string) (map[string]string, error) { sysctl := make(map[string]string) for _, inputSysctl := range strSlice { diff --git a/cmd/kpod/create_cli_test.go b/cmd/kpod/create_cli_test.go index af5c5afae..63a1e5dd3 100644 --- a/cmd/kpod/create_cli_test.go +++ b/cmd/kpod/create_cli_test.go @@ -68,32 +68,3 @@ func TestGetAllLabelsFile(t *testing.T) { result, _ := getAllLabels(fileLabels, Var1) assert.Equal(t, len(result), 3) } - -func TestGetAllEnvironmentVariables(t *testing.T) { - fileEnvs := []string{} - result, _ := getAllEnvironmentVariables(fileEnvs, Var1) - assert.Equal(t, len(result), 2) -} - -func TestGetAllEnvironmentVariablesBadKeyValue(t *testing.T) { - inEnvs := []string{"ONE1", "TWO=2"} - fileEnvs := []string{} - _, err := getAllEnvironmentVariables(fileEnvs, inEnvs) - assert.Error(t, err, assert.AnError) -} - -func TestGetAllEnvironmentVariablesBadEnvFile(t *testing.T) { - fileEnvs := []string{"/foobar5001/be"} - _, err := getAllEnvironmentVariables(fileEnvs, Var1) - assert.Error(t, err, assert.AnError) -} - -func TestGetAllEnvironmentVariablesFile(t *testing.T) { - content := []byte("THREE=3") - tFile, err := createTmpFile(content) - defer os.Remove(tFile) - assert.NoError(t, err) - fileEnvs := []string{tFile} - result, _ := getAllEnvironmentVariables(fileEnvs, Var1) - assert.Equal(t, len(result), 3) -} diff --git a/cmd/kpod/parse.go b/cmd/kpod/parse.go index e3143a793..7f6fc78df 100644 --- a/cmd/kpod/parse.go +++ b/cmd/kpod/parse.go @@ -327,55 +327,62 @@ func doesEnvExist(name string) bool { // 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(files []string, override []string) ([]string, error) { - envVariables := []string{} +func readKVStrings(env map[string]string, files []string, override []string) error { for _, ef := range files { - parsedVars, err := parseEnvFile(ef) - if err != nil { - return nil, err + if err := parseEnvFile(env, ef); err != nil { + return err + } + } + for _, line := range override { + if err := parseEnv(env, line); err != nil { + return err } - envVariables = append(envVariables, parsedVars...) } - // parse the '-e' and '--env' after, to allow override - envVariables = append(envVariables, override...) + return nil +} + +func parseEnv(env map[string]string, line string) error { + data := strings.SplitN(line, "=", 2) + + // trim the front of a variable, but nothing else + name := strings.TrimLeft(data[0], whiteSpaces) + if strings.ContainsAny(name, whiteSpaces) { + return errors.Errorf("name %q has white spaces, poorly formatted name", name) + } - return envVariables, nil + if len(data) > 1 { + env[name] = data[1] + } else { + // if only a pass-through variable is given, clean it up. + val, exists := os.LookupEnv(name) + if !exists { + return errors.Errorf("environment variable %q does not exist", name) + } + env[name] = val + } + return nil } // parseEnvFile reads a file with environment variables enumerated by lines -func parseEnvFile(filename string) ([]string, error) { +func parseEnvFile(env map[string]string, filename string) error { fh, err := os.Open(filename) if err != nil { - return []string{}, err + return err } defer fh.Close() - lines := []string{} scanner := bufio.NewScanner(fh) for scanner.Scan() { // trim the line from all leading whitespace first line := strings.TrimLeft(scanner.Text(), whiteSpaces) // line is not empty, and not starting with '#' if len(line) > 0 && !strings.HasPrefix(line, "#") { - data := strings.SplitN(line, "=", 2) - - // trim the front of a variable, but nothing else - variable := strings.TrimLeft(data[0], whiteSpaces) - if strings.ContainsAny(variable, whiteSpaces) { - return []string{}, errors.Errorf("variable %q has white spaces, poorly formatted environment", variable) - } - - if len(data) > 1 { - - // pass the value through, no trimming - lines = append(lines, fmt.Sprintf("%s=%s", variable, data[1])) - } else { - // if only a pass-through variable is given, clean it up. - lines = append(lines, fmt.Sprintf("%s=%s", strings.TrimSpace(line), os.Getenv(line))) + if err := parseEnv(env, line); err != nil { + return err } } } - return lines, scanner.Err() + return scanner.Err() } // NsIpc represents the container ipc stack. diff --git a/cmd/kpod/spec.go b/cmd/kpod/spec.go index 611a3cc56..d31f9c8ed 100644 --- a/cmd/kpod/spec.go +++ b/cmd/kpod/spec.go @@ -52,6 +52,9 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) { } g.SetRootReadonly(config.readOnlyRootfs) g.SetHostname(config.hostname) + if config.hostname != "" { + g.AddProcessEnv("HOSTNAME", config.hostname) + } for _, sysctl := range config.sysctl { s := strings.SplitN(sysctl, "=", 2) @@ -124,6 +127,10 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) { g.AddTmpfsMount(spliti[0], options) } + for name, val := range config.env { + g.AddProcessEnv(name, val) + } + configSpec := g.Spec() if config.seccompProfilePath != "" && config.seccompProfilePath != "unconfined" { @@ -138,8 +145,6 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) { configSpec.Linux.Seccomp = &seccompConfig } - configSpec.Process.Env = config.env - // BIND MOUNTS configSpec.Mounts = append(configSpec.Mounts, config.GetVolumeMounts()...) -- cgit v1.2.3-54-g00ecf