diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index 41e93153487..d86a754363b 100644 --- a/pkg/api/pod/warnings.go +++ b/pkg/api/pod/warnings.go @@ -249,7 +249,25 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta items := sets.NewString() for i, item := range c.Env { if items.Has(item.Name) { - warnings = append(warnings, fmt.Sprintf("%s: duplicate name %q", p.Child("env").Index(i).Child("name"), item.Name)) + // a previous value exists, but it might be OK + bad := false + ref := fmt.Sprintf("$(%s)", item.Name) // what does a ref to this name look like + // if we are replacing it with a valueFrom, warn + if item.ValueFrom != nil { + bad = true + } + // if this is X="$(X)", warn + if item.Value == ref { + bad = true + } + // if the new value does not contain a reference to the old + // value (e.g. X="abc"; X="$(X)123"), warn + if !strings.Contains(item.Value, ref) { + bad = true + } + if bad { + warnings = append(warnings, fmt.Sprintf("%s: hides previous definition of %q", p.Child("env").Index(i), item.Name)) + } } else { items.Insert(item.Name) } diff --git a/pkg/api/pod/warnings_test.go b/pkg/api/pod/warnings_test.go index 98a88419f04..b2ca9a2ada9 100644 --- a/pkg/api/pod/warnings_test.go +++ b/pkg/api/pod/warnings_test.go @@ -281,21 +281,35 @@ func TestWarnings(t *testing.T) { name: "duplicate env", template: &api.PodTemplateSpec{Spec: api.PodSpec{ InitContainers: []api.Container{{Env: []api.EnvVar{ - {Name: "a"}, - {Name: "a"}, - {Name: "a"}, + {Name: "a", Value: "a"}, + {Name: "a", Value: "a"}, + {Name: "a", Value: "other"}, + {Name: "a", Value: ""}, + {Name: "a", Value: "$(a)"}, + {Name: "a", ValueFrom: &api.EnvVarSource{}}, + {Name: "a", Value: "$(a) $(a)"}, // no warning }}}, Containers: []api.Container{{Env: []api.EnvVar{ - {Name: "b"}, - {Name: "b"}, - {Name: "b"}, + {Name: "b", Value: "b"}, + {Name: "b", Value: "b"}, + {Name: "b", Value: "other"}, + {Name: "b", Value: ""}, + {Name: "b", Value: "$(b)"}, + {Name: "b", ValueFrom: &api.EnvVarSource{}}, + {Name: "b", Value: "$(b) $(b)"}, // no warning }}}, }}, expected: []string{ - `spec.initContainers[0].env[1].name: duplicate name "a"`, - `spec.initContainers[0].env[2].name: duplicate name "a"`, - `spec.containers[0].env[1].name: duplicate name "b"`, - `spec.containers[0].env[2].name: duplicate name "b"`, + `spec.initContainers[0].env[1]: hides previous definition of "a"`, + `spec.initContainers[0].env[2]: hides previous definition of "a"`, + `spec.initContainers[0].env[3]: hides previous definition of "a"`, + `spec.initContainers[0].env[4]: hides previous definition of "a"`, + `spec.initContainers[0].env[5]: hides previous definition of "a"`, + `spec.containers[0].env[1]: hides previous definition of "b"`, + `spec.containers[0].env[2]: hides previous definition of "b"`, + `spec.containers[0].env[3]: hides previous definition of "b"`, + `spec.containers[0].env[4]: hides previous definition of "b"`, + `spec.containers[0].env[5]: hides previous definition of "b"`, }, }, {