Skip to content

Commit 14aa9c6

Browse files
sjbermannowjean
authored andcommitted
Fix index out of bounds error when building status (nginx#3513)
Problem: There's a specific scenario where we can hit an out of bounds index while building route statuses. I don't know the exact scenario, but it involves a parentRef without a sectionName, on a Gateway with a single listener. Could possibly involve having multiple implementations being deployed as well. Solution: Adjust the logic to ensure we don't access indices that are out of bounds. ADD Nginx Gateway HPA(nginx#3447)
1 parent 4043a5e commit 14aa9c6

File tree

4 files changed

+13
-14
lines changed

4 files changed

+13
-14
lines changed

charts/nginx-gateway-fabric/templates/hpa.yaml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
apiVersion: {{ ternary "autoscaling/v2" "autoscaling/v2beta2" (.Capabilities.APIVersions.Has "autoscaling/v2") }}
33
kind: HorizontalPodAutoscaler
44
metadata:
5-
{{- with .Values.autoscaling.annotations }}
5+
{{- with .Values.nginxGateway.autoscaling.annotations }}
66
annotations: {{ toYaml . | nindent 4 }}
77
{{- end }}
88
labels:
@@ -17,18 +17,18 @@ spec:
1717
apiVersion: apps/v1
1818
kind: Deployment
1919
name: {{ include "nginx-gateway.fullname" . }}
20-
minReplicas: {{ .Values.autoscaling.minReplicas }}
21-
maxReplicas: {{ .Values.autoscaling.maxReplicas }}
20+
minReplicas: {{ .Values.nginxGateway.autoscaling.minReplicas }}
21+
maxReplicas: {{ .Values.nginxGateway.autoscaling.maxReplicas }}
2222
metrics:
23-
{{- with .Values.autoscaling.targetMemoryUtilizationPercentage }}
23+
{{- with .Values.nginxGateway.autoscaling.targetMemoryUtilizationPercentage }}
2424
- type: Resource
2525
resource:
2626
name: memory
2727
target:
2828
type: Utilization
2929
averageUtilization: {{ . }}
3030
{{- end }}
31-
{{- with .Values.autoscaling.targetCPUUtilizationPercentage }}
31+
{{- with .Values.nginxGateway.autoscaling.targetCPUUtilizationPercentage }}
3232
- type: Resource
3333
resource:
3434
name: cpu
@@ -39,7 +39,7 @@ spec:
3939
{{- with .Values.autoscalingTemplate }}
4040
{{- toYaml . | nindent 2 }}
4141
{{- end }}
42-
{{- with .Values.autoscaling.behavior }}
42+
{{- with .Values.nginxGateway.autoscaling.behavior }}
4343
behavior:
4444
{{- toYaml . | nindent 4 }}
4545
{{- end }}

charts/nginx-gateway-fabric/values.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ nginxGateway:
157157
topologySpreadConstraints: []
158158

159159
autoscaling:
160+
# Enable or disable Horizontal Pod Autoscaler
160161
enabled: false
161162
annotations: {}
162163
minReplicas: 1
@@ -252,8 +253,6 @@ nginx:
252253
# plane will copy these secrets into any namespace where NGINX is deployed.
253254
imagePullSecrets: []
254255

255-
resources: {}
256-
257256
# Configuration for NGINX Plus usage reporting.
258257
usage:
259258
# -- The name of the Secret containing the JWT for NGINX Plus usage reporting. Must exist in the same namespace

internal/controller/status/prepare_requests.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,11 @@ func removeDuplicateIndexParentRefs(parentRefs []graph.ParentRef) []graph.Parent
103103
idxToParentRef[ref.Idx] = append(idxToParentRef[ref.Idx], ref)
104104
}
105105

106-
results := make([]graph.ParentRef, len(idxToParentRef))
106+
results := make([]graph.ParentRef, 0, len(idxToParentRef))
107107

108108
for idx, refs := range idxToParentRef {
109109
if len(refs) == 1 {
110-
results[idx] = refs[0]
110+
results = append(results, refs[0])
111111
continue
112112
}
113113

@@ -124,7 +124,7 @@ func removeDuplicateIndexParentRefs(parentRefs []graph.ParentRef) []graph.Parent
124124
}
125125
}
126126
}
127-
results[idx] = winningParentRef
127+
results = append(results, winningParentRef)
128128
}
129129

130130
return results

internal/controller/status/prepare_requests_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ func TestBuildHTTPRouteStatuses(t *testing.T) {
445445

446446
err := k8sClient.Get(context.Background(), nsname, &hr)
447447
g.Expect(err).ToNot(HaveOccurred())
448-
g.Expect(helpers.Diff(expected, hr.Status)).To(BeEmpty())
448+
g.Expect(expected.RouteStatus.Parents).To(ConsistOf(hr.Status.Parents))
449449
}
450450
}
451451

@@ -524,7 +524,7 @@ func TestBuildGRPCRouteStatuses(t *testing.T) {
524524

525525
err := k8sClient.Get(context.Background(), nsname, &hr)
526526
g.Expect(err).ToNot(HaveOccurred())
527-
g.Expect(helpers.Diff(expected, hr.Status)).To(BeEmpty())
527+
g.Expect(expected.RouteStatus.Parents).To(ConsistOf(hr.Status.Parents))
528528
}
529529
}
530530

@@ -601,7 +601,7 @@ func TestBuildTLSRouteStatuses(t *testing.T) {
601601

602602
err := k8sClient.Get(context.Background(), nsname, &hr)
603603
g.Expect(err).ToNot(HaveOccurred())
604-
g.Expect(helpers.Diff(expected, hr.Status)).To(BeEmpty())
604+
g.Expect(expected.RouteStatus.Parents).To(ConsistOf(hr.Status.Parents))
605605
}
606606
}
607607

0 commit comments

Comments
 (0)