From ec326d589f8917afdd81f340a64e3a5101e775f6 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Tue, 4 Aug 2026 17:30:19 +0100 Subject: [PATCH] Validate inject-proxy even without proxy env vars We were previously validating that the inject-proxy annotation referred only to valid container names, but only when proxy variables were defined. This meant we were not usually catching invalid manifests during presubmits, as these jobs rarely run with a cluster proxy. --- lib/resourcebuilder/podspec.go | 28 +++++++++++------- lib/resourcebuilder/podspec_test.go | 45 ++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/lib/resourcebuilder/podspec.go b/lib/resourcebuilder/podspec.go index b4b4fa83e5..80d854f062 100644 --- a/lib/resourcebuilder/podspec.go +++ b/lib/resourcebuilder/podspec.go @@ -2,6 +2,7 @@ package resourcebuilder import ( "fmt" + "strings" corev1 "k8s.io/api/core/v1" ) @@ -10,9 +11,8 @@ import ( // matching the container names. func updatePodSpecWithProxy(podSpec *corev1.PodSpec, containerNames []string, httpProxy, httpsProxy, noProxy string) error { hasProxy := len(httpsProxy) > 0 || len(httpProxy) > 0 || len(noProxy) > 0 - if !hasProxy { - return nil - } + + var notFoundContainerNames []string for _, containerName := range containerNames { found := false @@ -22,9 +22,11 @@ func updatePodSpecWithProxy(podSpec *corev1.PodSpec, containerNames []string, ht } found = true - podSpec.Containers[i].Env = append(podSpec.Containers[i].Env, corev1.EnvVar{Name: "HTTP_PROXY", Value: httpProxy}) - podSpec.Containers[i].Env = append(podSpec.Containers[i].Env, corev1.EnvVar{Name: "HTTPS_PROXY", Value: httpsProxy}) - podSpec.Containers[i].Env = append(podSpec.Containers[i].Env, corev1.EnvVar{Name: "NO_PROXY", Value: noProxy}) + if hasProxy { + podSpec.Containers[i].Env = append(podSpec.Containers[i].Env, corev1.EnvVar{Name: "HTTP_PROXY", Value: httpProxy}) + podSpec.Containers[i].Env = append(podSpec.Containers[i].Env, corev1.EnvVar{Name: "HTTPS_PROXY", Value: httpsProxy}) + podSpec.Containers[i].Env = append(podSpec.Containers[i].Env, corev1.EnvVar{Name: "NO_PROXY", Value: noProxy}) + } } for i := range podSpec.InitContainers { if podSpec.InitContainers[i].Name != containerName { @@ -32,16 +34,22 @@ func updatePodSpecWithProxy(podSpec *corev1.PodSpec, containerNames []string, ht } found = true - podSpec.InitContainers[i].Env = append(podSpec.InitContainers[i].Env, corev1.EnvVar{Name: "HTTP_PROXY", Value: httpProxy}) - podSpec.InitContainers[i].Env = append(podSpec.InitContainers[i].Env, corev1.EnvVar{Name: "HTTPS_PROXY", Value: httpsProxy}) - podSpec.InitContainers[i].Env = append(podSpec.InitContainers[i].Env, corev1.EnvVar{Name: "NO_PROXY", Value: noProxy}) + if hasProxy { + podSpec.InitContainers[i].Env = append(podSpec.InitContainers[i].Env, corev1.EnvVar{Name: "HTTP_PROXY", Value: httpProxy}) + podSpec.InitContainers[i].Env = append(podSpec.InitContainers[i].Env, corev1.EnvVar{Name: "HTTPS_PROXY", Value: httpsProxy}) + podSpec.InitContainers[i].Env = append(podSpec.InitContainers[i].Env, corev1.EnvVar{Name: "NO_PROXY", Value: noProxy}) + } } if !found { - return fmt.Errorf("requested injection for non-existent container: %q", containerName) + notFoundContainerNames = append(notFoundContainerNames, containerName) } } + if len(notFoundContainerNames) > 0 { + return fmt.Errorf("requested injection for non-existent containers: %s", strings.Join(notFoundContainerNames, ", ")) + } + return nil } diff --git a/lib/resourcebuilder/podspec_test.go b/lib/resourcebuilder/podspec_test.go index 248c05c488..37a5c19bd9 100644 --- a/lib/resourcebuilder/podspec_test.go +++ b/lib/resourcebuilder/podspec_test.go @@ -89,6 +89,49 @@ func TestUpdatePodSpecWithProxy(t *testing.T) { }, }, }, + { + name: "proxy info, invalid container name", + containerNames: []string{"foo", "invalid"}, + httpsProxy: "httpsProxy-val", + noProxy: "noProxy-val", + input: &corev1.PodSpec{ + InitContainers: []corev1.Container{ + {Name: "init-foo"}, + }, + Containers: []corev1.Container{ + {Name: "foo"}, + }, + }, + expectedErr: "requested injection for non-existent containers: invalid", + }, + { + name: "no proxy info, invalid container name", + containerNames: []string{"foo", "invalid"}, + input: &corev1.PodSpec{ + InitContainers: []corev1.Container{ + {Name: "init-foo"}, + }, + Containers: []corev1.Container{ + {Name: "foo"}, + }, + }, + expectedErr: "requested injection for non-existent containers: invalid", + }, + { + name: "proxy info, multiple invalid container names", + containerNames: []string{"foo", "invalid1", "invalid2"}, + httpsProxy: "httpsProxy-val", + noProxy: "noProxy-val", + input: &corev1.PodSpec{ + InitContainers: []corev1.Container{ + {Name: "init-foo"}, + }, + Containers: []corev1.Container{ + {Name: "foo"}, + }, + }, + expectedErr: "requested injection for non-existent containers: invalid1, invalid2", + }, } for _, test := range tests { @@ -104,7 +147,7 @@ func TestUpdatePodSpecWithProxy(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(test.input, test.expected) { + if test.expectedErr == "" && !reflect.DeepEqual(test.input, test.expected) { t.Error(cmp.Diff(test.input, test.expected)) } })