Fix warnings on "duplicate" env vars

Some use-cases are not actually wrong
This commit is contained in:
Tim Hockin 2023-06-07 15:55:31 -07:00
parent c4f44b3de3
commit 0ddaa7f7c9
No known key found for this signature in database
2 changed files with 43 additions and 11 deletions

View File

@ -246,7 +246,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)
}

View File

@ -272,21 +272,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"`,
},
},
{