Merge pull request #118547 from thockin/fix-dup-env-var-warn
Fix warnings on "duplicate" env vars
This commit is contained in:
		| @@ -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) | ||||
| 				} | ||||
|   | ||||
| @@ -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"`, | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Kubernetes Prow Robot
					Kubernetes Prow Robot