From 5c9bf126487bc43cede6eec3c5a5f30b72ec6deb Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Tue, 14 Feb 2017 11:02:23 -0300 Subject: [PATCH] Fix error getting class information from Ingress annotations --- controllers/nginx/pkg/cmd/controller/nginx.go | 6 ++++++ core/pkg/ingress/annotations/authreq/main.go | 6 +++++- core/pkg/ingress/controller/controller.go | 19 +++++++++++++++++-- core/pkg/ingress/controller/launch.go | 2 ++ core/pkg/ingress/controller/util.go | 6 +++++- core/pkg/ingress/controller/util_test.go | 8 +++++++- core/pkg/ingress/types.go | 4 ++++ 7 files changed, 46 insertions(+), 5 deletions(-) diff --git a/controllers/nginx/pkg/cmd/controller/nginx.go b/controllers/nginx/pkg/cmd/controller/nginx.go index 24a9a7046f..ffc04e21b9 100644 --- a/controllers/nginx/pkg/cmd/controller/nginx.go +++ b/controllers/nginx/pkg/cmd/controller/nginx.go @@ -30,6 +30,7 @@ import ( "github.com/golang/glog" "github.com/mitchellh/mapstructure" + "github.com/spf13/pflag" "k8s.io/kubernetes/pkg/api" @@ -251,6 +252,11 @@ func (n NGINXController) Info() *ingress.BackendInfo { } } +// OverrideFlags customize NGINX controller flags +func (n NGINXController) OverrideFlags(flags *pflag.FlagSet) { + flags.Set("ingress-class", "nginx") +} + // testTemplate checks if the NGINX configuration inside the byte array is valid // running the command "nginx -t" using a temporal file. func (n NGINXController) testTemplate(cfg []byte) error { diff --git a/core/pkg/ingress/annotations/authreq/main.go b/core/pkg/ingress/annotations/authreq/main.go index 560a738684..31c2085072 100644 --- a/core/pkg/ingress/annotations/authreq/main.go +++ b/core/pkg/ingress/annotations/authreq/main.go @@ -92,7 +92,11 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) { return nil, ing_errors.NewLocationDenied("invalid url host") } - m, _ := parser.GetStringAnnotation(authMethod, ing) + m, err := parser.GetStringAnnotation(authMethod, ing) + if err != nil { + return nil, err + } + if len(m) != 0 && !validMethod(m) { return nil, ing_errors.NewLocationDenied("invalid HTTP method") } diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index 5c979dcce9..87fbdbaa4b 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -173,7 +173,7 @@ func newIngressController(config *Configuration) *GenericController { DeleteFunc: func(obj interface{}) { delIng := obj.(*extensions.Ingress) if !IsValidClass(delIng, config.IngressClass) { - glog.Infof("ignoring add for ingress %v based on annotation %v", delIng.Name, ingressClassKey) + glog.Infof("ignoring delete for ingress %v based on annotation %v", delIng.Name, ingressClassKey) return } ic.recorder.Eventf(delIng, api.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", delIng.Namespace, delIng.Name)) @@ -182,7 +182,7 @@ func newIngressController(config *Configuration) *GenericController { UpdateFunc: func(old, cur interface{}) { oldIng := old.(*extensions.Ingress) curIng := cur.(*extensions.Ingress) - if !IsValidClass(curIng, config.IngressClass) { + if !IsValidClass(curIng, config.IngressClass) && !IsValidClass(oldIng, config.IngressClass) { return } @@ -564,6 +564,10 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress for _, ingIf := range ings { ing := ingIf.(*extensions.Ingress) + if !IsValidClass(ing, ic.cfg.IngressClass) { + continue + } + anns := ic.annotations.Extract(ing) for _, rule := range ing.Spec.Rules { @@ -698,6 +702,10 @@ func (ic *GenericController) createUpstreams(data []interface{}) map[string]*ing for _, ingIf := range data { ing := ingIf.(*extensions.Ingress) + if !IsValidClass(ing, ic.cfg.IngressClass) { + continue + } + secUpstream := ic.annotations.SecureUpstream(ing) hz := ic.annotations.HealthCheck(ing) @@ -840,6 +848,10 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str // initialize all the servers for _, ingIf := range data { ing := ingIf.(*extensions.Ingress) + if !IsValidClass(ing, ic.cfg.IngressClass) { + continue + } + // check if ssl passthrough is configured sslpt := ic.annotations.SSLPassthrough(ing) @@ -868,6 +880,9 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str // configure default location and SSL for _, ingIf := range data { ing := ingIf.(*extensions.Ingress) + if !IsValidClass(ing, ic.cfg.IngressClass) { + continue + } for _, rule := range ing.Spec.Rules { host := rule.Host diff --git a/core/pkg/ingress/controller/launch.go b/core/pkg/ingress/controller/launch.go index a1f325d4fb..ce23f9a5c8 100644 --- a/core/pkg/ingress/controller/launch.go +++ b/core/pkg/ingress/controller/launch.go @@ -84,6 +84,8 @@ func NewIngressController(backend ingress.Controller) *GenericController { ingress controller should update the Ingress status IP/hostname. Default is true`) ) + backend.OverrideFlags(flags) + flags.AddGoFlagSet(flag.CommandLine) flags.Parse(os.Args) diff --git a/core/pkg/ingress/controller/util.go b/core/pkg/ingress/controller/util.go index 439526cbaf..7ff154d8fb 100644 --- a/core/pkg/ingress/controller/util.go +++ b/core/pkg/ingress/controller/util.go @@ -26,6 +26,7 @@ import ( "k8s.io/ingress/core/pkg/ingress" "k8s.io/ingress/core/pkg/ingress/annotations/parser" + "k8s.io/ingress/core/pkg/ingress/errors" ) // DeniedKeyName name of the key that contains the reason to deny a location @@ -92,7 +93,10 @@ func IsValidClass(ing *extensions.Ingress, class string) bool { return true } - cc, _ := parser.GetStringAnnotation(ingressClassKey, ing) + cc, err := parser.GetStringAnnotation(ingressClassKey, ing) + if err != nil && !errors.IsMissingAnnotations(err) { + glog.Warningf("unexpected error reading ingress annotation: %v", err) + } if cc == "" { return true } diff --git a/core/pkg/ingress/controller/util_test.go b/core/pkg/ingress/controller/util_test.go index 3707145334..8ec84785af 100644 --- a/core/pkg/ingress/controller/util_test.go +++ b/core/pkg/ingress/controller/util_test.go @@ -19,6 +19,8 @@ package controller import ( "testing" + "reflect" + "k8s.io/ingress/core/pkg/ingress" "k8s.io/ingress/core/pkg/ingress/annotations/auth" "k8s.io/ingress/core/pkg/ingress/annotations/authreq" @@ -29,7 +31,6 @@ import ( "k8s.io/ingress/core/pkg/ingress/resolver" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" - "reflect" ) type fakeError struct{} @@ -54,6 +55,7 @@ func TestIsValidClass(t *testing.T) { data := map[string]string{} data[ingressClassKey] = "custom" ing.SetAnnotations(data) + b = IsValidClass(ing, "custom") if !b { t.Errorf("Expected valid class but %v returned", b) @@ -62,6 +64,10 @@ func TestIsValidClass(t *testing.T) { if b { t.Errorf("Expected invalid class but %v returned", b) } + b = IsValidClass(ing, "") + if !b { + t.Errorf("Expected invalid class but %v returned", b) + } } func TestIsHostValid(t *testing.T) { diff --git a/core/pkg/ingress/types.go b/core/pkg/ingress/types.go index e619b5dc65..f9aad07ab0 100644 --- a/core/pkg/ingress/types.go +++ b/core/pkg/ingress/types.go @@ -17,6 +17,8 @@ limitations under the License. package ingress import ( + "github.com/spf13/pflag" + "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/healthz" @@ -86,6 +88,8 @@ type Controller interface { BackendDefaults() defaults.Backend // Info returns information about the ingress controller Info() *BackendInfo + // OverrideFlags allow the customization of the flags in the backend + OverrideFlags(*pflag.FlagSet) } // BackendInfo returns information about the backend.