From f2344db40adf86b7242cbeab12a265236f0e102e Mon Sep 17 00:00:00 2001 From: akolomentsev Date: Wed, 19 Dec 2018 14:50:28 -0800 Subject: [PATCH] do not mutate defaults in replaceOrAppendEnvValues Signed-off-by: Andrey Kolomentsev --- oci/spec_opts.go | 16 +++++++++------- oci/spec_opts_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/oci/spec_opts.go b/oci/spec_opts.go index 2fd2295bb..b9949f837 100644 --- a/oci/spec_opts.go +++ b/oci/spec_opts.go @@ -141,8 +141,10 @@ func WithEnv(environmentVariables []string) SpecOpts { // replaced by env key or appended to the list func replaceOrAppendEnvValues(defaults, overrides []string) []string { cache := make(map[string]int, len(defaults)) + results := make([]string, 0, len(defaults)) for i, e := range defaults { parts := strings.SplitN(e, "=", 2) + results = append(results, e) cache[parts[0]] = i } @@ -150,7 +152,7 @@ func replaceOrAppendEnvValues(defaults, overrides []string) []string { // Values w/o = means they want this env to be removed/unset. if !strings.Contains(value, "=") { if i, exists := cache[value]; exists { - defaults[i] = "" // Used to indicate it should be removed + results[i] = "" // Used to indicate it should be removed } continue } @@ -158,21 +160,21 @@ func replaceOrAppendEnvValues(defaults, overrides []string) []string { // Just do a normal set/update parts := strings.SplitN(value, "=", 2) if i, exists := cache[parts[0]]; exists { - defaults[i] = value + results[i] = value } else { - defaults = append(defaults, value) + results = append(results, value) } } // Now remove all entries that we want to "unset" - for i := 0; i < len(defaults); i++ { - if defaults[i] == "" { - defaults = append(defaults[:i], defaults[i+1:]...) + for i := 0; i < len(results); i++ { + if results[i] == "" { + results = append(results[:i], results[i+1:]...) i-- } } - return defaults + return results } // WithProcessArgs replaces the args on the generated spec diff --git a/oci/spec_opts_test.go b/oci/spec_opts_test.go index 1bacce817..9cbc7a55d 100644 --- a/oci/spec_opts_test.go +++ b/oci/spec_opts_test.go @@ -31,6 +31,49 @@ import ( specs "github.com/opencontainers/runtime-spec/specs-go" ) +func TestReplaceOrAppendEnvValues(t *testing.T) { + t.Parallel() + + defaults := []string{ + "o=ups", "p=$e", "x=foo", "y=boo", "z", "t=", + } + overrides := []string{ + "x=bar", "y", "a=42", "o=", "e", "s=", + } + expected := []string{ + "o=", "p=$e", "x=bar", "z", "t=", "a=42", "s=", + } + + defaultsOrig := make([]string, len(defaults)) + copy(defaultsOrig, defaults) + overridesOrig := make([]string, len(overrides)) + copy(overridesOrig, overrides) + + results := replaceOrAppendEnvValues(defaults, overrides) + + for i, x := range defaultsOrig { + if defaults[i] != x { + t.Fatal("defaults slice was mutated") + } + } + + for i, x := range overridesOrig { + if overrides[i] != x { + t.Fatal("overrides slice was mutated") + } + } + + if len(expected) != len(results) { + t.Fatalf("expected %s, but found %s", expected, results) + } + + for i, x := range expected { + if results[i] != x { + t.Fatalf("expected %s, but found %s", expected, results) + } + } +} + func TestWithEnv(t *testing.T) { t.Parallel()