Skip to content

Commit 2445eb6

Browse files
jkhelilcursoragent
authored andcommitted
fix(common): apply proxy settings to StatefulSets too
ApplyProxySettings only handled Deployment resources, so pods running as StatefulSets (e.g. when statefulset-ordinals is enabled) never inherited the cluster-wide proxy environment variables. Extend the transformer to also cover the StatefulSet kind, and harden the containers lookup to tolerate a null containers field instead of erroring out. Fixes SRVKP-12831 Signed-off-by: Jawed khelil <jkhelil@redhat.com> Assisted-by: Claude Sonnet 5 (via Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent b1dab6c commit 2445eb6

3 files changed

Lines changed: 142 additions & 4 deletions

File tree

pkg/reconciler/common/proxy.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ import (
2424
)
2525

2626
// ApplyProxySettings is a transformer that propagate any proxy environment variables
27-
// set on the operator deployment to the underlying deployment.
27+
// set on the operator deployment to the underlying deployment or statefulset.
2828
func ApplyProxySettings(u *unstructured.Unstructured) error {
29-
if u.GetKind() != "Deployment" {
30-
// Don't do anything on something else than Deployment
29+
if u.GetKind() != "Deployment" && u.GetKind() != "StatefulSet" {
30+
// Don't do anything on something else than Deployment or StatefulSet
3131
return nil
3232
}
3333

@@ -48,7 +48,7 @@ func ApplyProxySettings(u *unstructured.Unstructured) error {
4848
return err
4949
}
5050
if !found {
51-
// No containers in the deployment, it is weird but let's not fail
51+
// No containers in the resource, it is weird but let's not fail
5252
return nil
5353
}
5454
for _, c := range containers {

pkg/reconciler/common/proxy_test.go

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,69 @@ func TestApplyProxySettingsRemovingProxy(t *testing.T) {
108108
assert.DeepEqual(t, actual, expected)
109109
}
110110

111+
func TestApplyProxySettingsStatefulSetNoProxy(t *testing.T) {
112+
actual := unstructuredStatefulSet(t)
113+
expected := unstructuredStatefulSet(t)
114+
115+
if err := ApplyProxySettings(actual); err != nil {
116+
t.Fatal(err)
117+
}
118+
119+
assert.DeepEqual(t, actual, expected)
120+
}
121+
122+
func TestApplyProxySettingsStatefulSetWithProxy(t *testing.T) {
123+
proxyEnv := map[string]string{
124+
"HTTP_PROXY": "http://1.2.3.4:30001",
125+
"HTTPS_PROXY": "http://1.2.3.4:30002",
126+
"NO_PROXY": "index.docker.io",
127+
}
128+
actual := unstructuredStatefulSet(t, withStatefulSetEnv(extraEnvVars))
129+
expected := unstructuredStatefulSet(t, withStatefulSetEnv(toEnvVar(proxyEnv), extraEnvVars))
130+
131+
defer env.PatchAll(t, proxyEnv)()
132+
if err := ApplyProxySettings(actual); err != nil {
133+
t.Fatal(err)
134+
}
135+
136+
assert.DeepEqual(t, actual, expected)
137+
}
138+
139+
func TestApplyProxySettingsOtherKindIgnored(t *testing.T) {
140+
svc := &corev1.Service{
141+
ObjectMeta: metav1.ObjectMeta{
142+
Namespace: "foo",
143+
Name: "registry",
144+
},
145+
}
146+
svc.SetGroupVersionKind(schema.GroupVersionKind{
147+
Group: corev1.SchemeGroupVersion.Group,
148+
Version: corev1.SchemeGroupVersion.Version,
149+
Kind: "Service",
150+
})
151+
b, err := json.Marshal(svc)
152+
if err != nil {
153+
t.Fatal(err)
154+
}
155+
actual := &unstructured.Unstructured{}
156+
if err := json.Unmarshal(b, actual); err != nil {
157+
t.Fatal(err)
158+
}
159+
expected := actual.DeepCopy()
160+
161+
proxyEnv := map[string]string{
162+
"HTTP_PROXY": "http://1.2.3.4:30001",
163+
}
164+
defer env.PatchAll(t, proxyEnv)()
165+
if err := ApplyProxySettings(actual); err != nil {
166+
t.Fatal(err)
167+
}
168+
169+
assert.DeepEqual(t, actual, expected)
170+
}
171+
111172
type deploymentModifier func(*appsv1.Deployment)
173+
type statefulSetModifier func(*appsv1.StatefulSet)
112174

113175
func unstructuredDeployment(t *testing.T, modifiers ...deploymentModifier) *unstructured.Unstructured {
114176
deploy := &appsv1.Deployment{
@@ -158,6 +220,69 @@ func unstructuredDeployment(t *testing.T, modifiers ...deploymentModifier) *unst
158220
return ud
159221
}
160222

223+
func unstructuredStatefulSet(t *testing.T, modifiers ...statefulSetModifier) *unstructured.Unstructured {
224+
ss := &appsv1.StatefulSet{
225+
ObjectMeta: metav1.ObjectMeta{
226+
Namespace: "foo",
227+
Name: "registry",
228+
},
229+
Spec: appsv1.StatefulSetSpec{
230+
ServiceName: "registry",
231+
Selector: &metav1.LabelSelector{
232+
MatchLabels: map[string]string{
233+
"app": "registry",
234+
},
235+
},
236+
Template: corev1.PodTemplateSpec{
237+
ObjectMeta: metav1.ObjectMeta{
238+
Labels: map[string]string{
239+
"app": "registry",
240+
},
241+
},
242+
Spec: corev1.PodSpec{
243+
Containers: []corev1.Container{{
244+
Name: "registry",
245+
Image: "registry",
246+
}},
247+
},
248+
},
249+
},
250+
}
251+
252+
for _, modifier := range modifiers {
253+
modifier(ss)
254+
}
255+
256+
ss.SetGroupVersionKind(schema.GroupVersionKind{
257+
Group: appsv1.SchemeGroupVersion.Group,
258+
Version: appsv1.SchemeGroupVersion.Version,
259+
Kind: "StatefulSet",
260+
})
261+
b, err := json.Marshal(ss)
262+
if err != nil {
263+
t.Fatal(err)
264+
}
265+
ud := &unstructured.Unstructured{}
266+
if err := json.Unmarshal(b, ud); err != nil {
267+
t.Fatal(err)
268+
}
269+
return ud
270+
}
271+
272+
func withStatefulSetEnv(envs ...[]corev1.EnvVar) func(*appsv1.StatefulSet) {
273+
return func(ss *appsv1.StatefulSet) {
274+
for i, c := range ss.Spec.Template.Spec.Containers {
275+
for _, env := range envs {
276+
c.Env = append(c.Env, env...)
277+
}
278+
sort.Slice(c.Env, func(i, j int) bool {
279+
return c.Env[i].Name < c.Env[j].Name
280+
})
281+
ss.Spec.Template.Spec.Containers[i] = c
282+
}
283+
}
284+
}
285+
161286
func withEnv(envs ...[]corev1.EnvVar) func(*appsv1.Deployment) {
162287
return func(d *appsv1.Deployment) {
163288
for i, c := range d.Spec.Template.Spec.Containers {

pkg/reconciler/kubernetes/tektoninstallerset/install_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ var (
163163
Spec: appsv1.StatefulSetSpec{
164164
Replicas: ptr.Int32(1),
165165
ServiceName: "test",
166+
Template: statefulsetPodTemplate,
166167
},
167168
Status: appsv1.StatefulSetStatus{
168169
Replicas: 1,
@@ -181,12 +182,24 @@ var (
181182
Spec: appsv1.StatefulSetSpec{
182183
Replicas: ptr.Int32(1),
183184
ServiceName: "test",
185+
Template: statefulsetPodTemplate,
184186
},
185187
Status: appsv1.StatefulSetStatus{
186188
Replicas: 1,
187189
ReadyReplicas: 1,
188190
},
189191
}
192+
// statefulsetPodTemplate gives the StatefulSet fixtures above a real container
193+
// so that spec.template.spec.containers isn't a JSON null, matching what a
194+
// real manifest looks like once ApplyProxySettings processes it.
195+
statefulsetPodTemplate = corev1.PodTemplateSpec{
196+
Spec: corev1.PodSpec{
197+
Containers: []corev1.Container{{
198+
Name: "test",
199+
Image: "test",
200+
}},
201+
},
202+
}
190203
readyControllerDeployment = &appsv1.Deployment{
191204
ObjectMeta: metav1.ObjectMeta{
192205
Namespace: "test",

0 commit comments

Comments
 (0)