Skip to content

Commit 180011e

Browse files
committed
Fix admission webhook host/path overlap detection not working for multiple rules
1 parent 8f2593b commit 180011e

File tree

2 files changed

+35
-7
lines changed

2 files changed

+35
-7
lines changed

internal/ingress/controller/controller.go

+4-7
Original file line numberDiff line numberDiff line change
@@ -1821,16 +1821,14 @@ func checkOverlap(ing *networking.Ingress, servers []*ingress.Server) error {
18211821
continue
18221822
}
18231823

1824-
// same ingress
1824+
// path overlap. Check if one of the ingresses has a canary annotation
1825+
isCanaryEnabled, annotationErr := parser.GetBoolAnnotation("canary", ing, canary.CanaryAnnotations.Annotations)
18251826
for _, existing := range existingIngresses {
18261827
if existing.ObjectMeta.Namespace == ing.ObjectMeta.Namespace && existing.ObjectMeta.Name == ing.ObjectMeta.Name {
1827-
return nil
1828+
// same ingress
1829+
continue
18281830
}
1829-
}
18301831

1831-
// path overlap. Check if one of the ingresses has a canary annotation
1832-
isCanaryEnabled, annotationErr := parser.GetBoolAnnotation("canary", ing, canary.CanaryAnnotations.Annotations)
1833-
for _, existing := range existingIngresses {
18341832
isExistingCanaryEnabled, existingAnnotationErr := parser.GetBoolAnnotation("canary", existing, canary.CanaryAnnotations.Annotations)
18351833

18361834
if isCanaryEnabled && isExistingCanaryEnabled {
@@ -1843,7 +1841,6 @@ func checkOverlap(ing *networking.Ingress, servers []*ingress.Server) error {
18431841
}
18441842

18451843
// no overlap
1846-
return nil
18471844
}
18481845
}
18491846

test/e2e/admission/admission.go

+31
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,37 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
6060
assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with the same host and path should return an error")
6161
})
6262

63+
ginkgo.It("should not allow overlaps of host and paths without canary annotations in any rule", func() {
64+
host := admissionTestHost
65+
66+
firstIngress := framework.NewSingleIngressWithMultiplePaths(
67+
"first-ingress",
68+
[]string{"/safe-path-1", "/conflict-path"},
69+
host,
70+
f.Namespace,
71+
framework.EchoService,
72+
80,
73+
nil)
74+
_, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), firstIngress, metav1.CreateOptions{})
75+
assert.Nil(ginkgo.GinkgoT(), err, "creating ingress")
76+
77+
f.WaitForNginxServer(host,
78+
func(server string) bool {
79+
return strings.Contains(server, fmt.Sprintf("server_name %v", host))
80+
})
81+
82+
secondIngress := framework.NewSingleIngressWithMultiplePaths(
83+
"second-ingress",
84+
[]string{"/safe-path-2", "/conflict-path"},
85+
host,
86+
f.Namespace,
87+
framework.EchoService,
88+
80,
89+
nil)
90+
_, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), secondIngress, metav1.CreateOptions{})
91+
assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with the same host and path should return an error")
92+
})
93+
6394
ginkgo.It("should allow overlaps of host and paths with canary annotation", func() {
6495
host := admissionTestHost
6596

0 commit comments

Comments
 (0)