Skip to content

Commit

Permalink
Fix --disable-catch-all
Browse files Browse the repository at this point in the history
  • Loading branch information
Alex Kursell committed Jan 25, 2019
1 parent bd74dce commit ca74960
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 4 deletions.
14 changes: 10 additions & 4 deletions internal/ingress/controller/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ func New(checkOCSP bool,
klog.Infof("ignoring delete for ingress %v based on annotation %v", ing.Name, class.IngressKey)
return
}
if ing.Spec.Backend != nil && disableCatchAll {
if isCatchAllIngress(ing.Spec) && disableCatchAll {
klog.Infof("ignoring delete for catch-all ingress %v/%v because of --disable-catch-all", ing.Namespace, ing.Name)
return
}
Expand All @@ -348,7 +348,7 @@ func New(checkOCSP bool,
klog.Infof("ignoring add for ingress %v based on annotation %v with value %v", ing.Name, class.IngressKey, a)
return
}
if ing.Spec.Backend != nil && disableCatchAll {
if isCatchAllIngress(ing.Spec) && disableCatchAll {
klog.Infof("ignoring add for catch-all ingress %v/%v because of --disable-catch-all", ing.Namespace, ing.Name)
return
}
Expand All @@ -370,7 +370,7 @@ func New(checkOCSP bool,
validOld := class.IsValid(oldIng)
validCur := class.IsValid(curIng)
if !validOld && validCur {
if curIng.Spec.Backend != nil && disableCatchAll {
if isCatchAllIngress(curIng.Spec) && disableCatchAll {
klog.Infof("ignoring update for catch-all ingress %v/%v because of --disable-catch-all", curIng.Namespace, curIng.Name)
return
}
Expand All @@ -382,7 +382,7 @@ func New(checkOCSP bool,
ingDeleteHandler(old)
return
} else if validCur && !reflect.DeepEqual(old, cur) {
if curIng.Spec.Backend != nil && disableCatchAll {
if isCatchAllIngress(curIng.Spec) && disableCatchAll {
klog.Infof("ignoring update for catch-all ingress %v/%v and delete old one because of --disable-catch-all", curIng.Namespace, curIng.Name)
ingDeleteHandler(old)
return
Expand Down Expand Up @@ -617,6 +617,12 @@ func New(checkOCSP bool,
return store
}

// isCatchAllIngress returns whether or not an ingress produces a
// catch-all server, and so should be ignored when --disable-catch-all is set
func isCatchAllIngress(spec extensions.IngressSpec) bool {
return spec.Backend != nil && len(spec.Rules) == 0
}

// syncIngress parses ingress annotations converting the value of the
// annotation to a go struct
func (s *k8sStore) syncIngress(ing *extensions.Ingress) {
Expand Down
30 changes: 30 additions & 0 deletions test/e2e/framework/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,36 @@ func newSingleIngressWithRules(name, path, host, ns, service string, port int, a
return newSingleIngress(name, ns, annotations, spec)
}

// NewSingleIngressWithBackendAndRules creates an ingress with both a default backend and a rule
func NewSingleIngressWithBackendAndRules(name, path, host, ns, defaultService string, defaultPort int, service string, port int, annotations *map[string]string) *extensions.Ingress {
spec := extensions.IngressSpec{
Backend: &extensions.IngressBackend{
ServiceName: defaultService,
ServicePort: intstr.FromInt(defaultPort),
},
Rules: []extensions.IngressRule{
{
Host: host,
IngressRuleValue: extensions.IngressRuleValue{
HTTP: &extensions.HTTPIngressRuleValue{
Paths: []extensions.HTTPIngressPath{
{
Path: path,
Backend: extensions.IngressBackend{
ServiceName: service,
ServicePort: intstr.FromInt(port),
},
},
},
},
},
},
},
}

return newSingleIngress(name, ns, annotations, spec)
}

// NewSingleCatchAllIngress creates a simple ingress with a catch-all backend
func NewSingleCatchAllIngress(name, ns, service string, port int, annotations *map[string]string) *extensions.Ingress {
spec := extensions.IngressSpec{
Expand Down
20 changes: 20 additions & 0 deletions test/e2e/settings/disable_catch_all.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,4 +109,24 @@ var _ = framework.IngressNginxDescribe("Disabled catch-all", func() {
Expect(errs).To(BeNil())
Expect(resp.StatusCode).Should(Equal(http.StatusNotFound))
})

It("should allow Ingress with both a default backend and rules", func() {
host := "foo"

ing := framework.NewSingleIngressWithBackendAndRules("not-catch-all", "/rulepath", host, f.IngressController.Namespace, "http-svc", 80, "http-svc", 80, nil)
f.EnsureIngress(ing)

f.WaitForNginxServer(host, func(cfg string) bool {
return strings.Contains(cfg, "server_name foo")
})

resp, _, errs := gorequest.New().
Get(f.IngressController.HTTPURL).
Set("Host", host).
End()

Expect(errs).To(BeNil())
Expect(resp.StatusCode).Should(Equal(http.StatusOK))

})
})

0 comments on commit ca74960

Please sign in to comment.