Skip to content

Commit 987b331

Browse files
committed
fix(validation): support simultaneous ALB and NGINX traffic routing validation
Previously, when both ALB and NGINX traffic routing were configured simultaneously, the validation logic incorrectly applied NGINX validation rules to ALB ingresses, causing validation errors like: 'ingress has no rules using service sample-api backend' This fix modifies the ValidateIngress function to: - Check if an ingress belongs to NGINX configuration first - Check if an ingress belongs to ALB configuration second - Only validate ingresses against their respective traffic routing rules Added helper functions isNginxIngress() and isAlbIngress() to properly identify which traffic routing configuration an ingress belongs to. Fixes #4452 Signed-off-by: puretension <[email protected]>
1 parent d77b05b commit 987b331

File tree

2 files changed

+90
-5
lines changed

2 files changed

+90
-5
lines changed

pkg/apis/rollouts/validation/validation_references.go

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,59 @@ func ValidateIngress(rollout *v1alpha1.Rollout, ingress *ingressutil.Ingress) fi
222222
fldPath := field.NewPath("spec", "strategy", "canary", "trafficRouting")
223223
canary := rollout.Spec.Strategy.Canary
224224

225+
// Check if this ingress belongs to NGINX configuration
225226
if canary.TrafficRouting.Nginx != nil {
226-
return validateNginxIngress(canary, ingress, fldPath)
227-
} else if canary.TrafficRouting.ALB != nil {
228-
return validateAlbIngress(canary, ingress, fldPath)
229-
} else {
230-
return allErrs
227+
if isNginxIngress(canary, ingress.GetName()) {
228+
return validateNginxIngress(canary, ingress, fldPath)
229+
}
230+
}
231+
232+
// Check if this ingress belongs to ALB configuration
233+
if canary.TrafficRouting.ALB != nil {
234+
if isAlbIngress(canary, ingress.GetName()) {
235+
return validateAlbIngress(canary, ingress, fldPath)
236+
}
237+
}
238+
239+
return allErrs
240+
}
241+
242+
func isNginxIngress(canary *v1alpha1.CanaryStrategy, ingressName string) bool {
243+
nginx := canary.TrafficRouting.Nginx
244+
if nginx == nil {
245+
return false
246+
}
247+
248+
// Check single stable ingress
249+
if nginx.StableIngress == ingressName {
250+
return true
251+
}
252+
253+
// Check multiple stable ingresses
254+
if nginx.StableIngresses != nil && slices.Contains(nginx.StableIngresses, ingressName) {
255+
return true
256+
}
257+
258+
return false
259+
}
260+
261+
func isAlbIngress(canary *v1alpha1.CanaryStrategy, ingressName string) bool {
262+
alb := canary.TrafficRouting.ALB
263+
if alb == nil {
264+
return false
265+
}
266+
267+
// Check single ingress
268+
if alb.Ingress == ingressName {
269+
return true
231270
}
271+
272+
// Check multiple ingresses
273+
if alb.Ingresses != nil && slices.Contains(alb.Ingresses, ingressName) {
274+
return true
275+
}
276+
277+
return false
232278
}
233279

234280
func validateNginxIngress(canary *v1alpha1.CanaryStrategy, ingress *ingressutil.Ingress, fldPath *field.Path) field.ErrorList {

pkg/apis/rollouts/validation/validation_references_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,45 @@ func TestValidateAlbIngress(t *testing.T) {
846846
})
847847
}
848848

849+
func TestValidateIngressSimultaneousAlbNginx(t *testing.T) {
850+
t.Run("validate simultaneous ALB and NGINX ingresses - success", func(t *testing.T) {
851+
// Create a rollout with both ALB and NGINX traffic routing
852+
rollout := getAlbRollout("alb-ingress")
853+
rollout.Spec.Strategy.Canary.TrafficRouting.Nginx = &v1alpha1.NginxTrafficRouting{
854+
StableIngresses: []string{"nginx-ingress-1", "nginx-ingress-2"},
855+
}
856+
857+
// Test ALB ingress validation
858+
albIngressObj := getIngress()
859+
albIngressObj.Name = "alb-ingress"
860+
albIngress := ingressutil.NewLegacyIngress(albIngressObj)
861+
allErrs := ValidateIngress(rollout, albIngress)
862+
assert.Empty(t, allErrs)
863+
864+
// Test NGINX ingress validation
865+
nginxIngressObj := getIngress()
866+
nginxIngressObj.Name = "nginx-ingress-1"
867+
nginxIngress := ingressutil.NewLegacyIngress(nginxIngressObj)
868+
allErrs = ValidateIngress(rollout, nginxIngress)
869+
assert.Empty(t, allErrs)
870+
})
871+
872+
t.Run("validate simultaneous ALB and NGINX ingresses - ALB ingress not validated against NGINX rules", func(t *testing.T) {
873+
// Create a rollout with both ALB and NGINX traffic routing
874+
rollout := getAlbRollout("alb-ingress")
875+
rollout.Spec.Strategy.Canary.TrafficRouting.Nginx = &v1alpha1.NginxTrafficRouting{
876+
StableIngresses: []string{"nginx-ingress-1", "nginx-ingress-2"},
877+
}
878+
879+
// Test that ALB ingress is not validated against NGINX rules
880+
albIngressObj := getIngress()
881+
albIngressObj.Name = "alb-ingress"
882+
albIngress := ingressutil.NewLegacyIngress(albIngressObj)
883+
allErrs := ValidateIngress(rollout, albIngress)
884+
assert.Empty(t, allErrs, "ALB ingress should not be validated against NGINX rules")
885+
})
886+
}
887+
849888
func TestValidateRolloutNginxIngressesConfig(t *testing.T) {
850889
var emptyStableIngress string
851890
var emptyStableIngresses []string

0 commit comments

Comments
 (0)