From 597a0e691a128a971632246699865e2dd960734b Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 29 Dec 2016 17:02:06 -0300 Subject: [PATCH] Deny location mapping in case of specific errors --- controllers/nginx/pkg/template/template.go | 11 ++ .../rootfs/etc/nginx/template/nginx.tmpl | 7 + core/pkg/cache/main_test.go | 92 +++++++++++++ core/pkg/ingress/annotations/auth/main.go | 77 ++++++----- .../pkg/ingress/annotations/auth/main_test.go | 42 +++++- core/pkg/ingress/annotations/authreq/main.go | 33 +++-- .../ingress/annotations/authreq/main_test.go | 8 +- core/pkg/ingress/annotations/authtls/main.go | 34 +++-- core/pkg/ingress/annotations/cors/main.go | 18 ++- .../ingress/annotations/healthcheck/main.go | 22 ++- .../annotations/healthcheck/main_test.go | 19 ++- .../ingress/annotations/ipwhitelist/main.go | 34 +++-- .../annotations/ipwhitelist/main_test.go | 47 +++++-- core/pkg/ingress/annotations/parser/main.go | 70 +++++----- .../ingress/annotations/parser/main_test.go | 6 + core/pkg/ingress/annotations/proxy/main.go | 33 ++--- .../ingress/annotations/proxy/main_test.go | 29 ++-- .../pkg/ingress/annotations/ratelimit/main.go | 21 ++- .../annotations/ratelimit/main_test.go | 26 ++-- core/pkg/ingress/annotations/rewrite/main.go | 24 ++-- .../ingress/annotations/rewrite/main_test.go | 39 ++++-- .../annotations/secureupstream/main.go | 12 +- .../annotations/secureupstream/main_test.go | 4 +- .../annotations/sslpassthrough/main.go | 21 +-- .../annotations/sslpassthrough/main_test.go | 18 +-- core/pkg/ingress/controller/annotations.go | 114 ++++++++++++++++ .../ingress/controller/annotations_test.go | 92 +++++++++++++ core/pkg/ingress/controller/controller.go | 128 +++++------------- core/pkg/ingress/controller/util.go | 16 +++ core/pkg/ingress/errors/errors.go | 89 ++++++++++++ core/pkg/ingress/errors/errors_test.go | 52 +++++++ core/pkg/ingress/resolver/main.go | 43 ++++++ core/pkg/ingress/types.go | 3 + core/pkg/task/queue_test.go | 17 +-- 34 files changed, 968 insertions(+), 333 deletions(-) create mode 100644 core/pkg/cache/main_test.go create mode 100644 core/pkg/ingress/controller/annotations.go create mode 100644 core/pkg/ingress/controller/annotations_test.go create mode 100644 core/pkg/ingress/errors/errors.go create mode 100644 core/pkg/ingress/errors/errors_test.go create mode 100644 core/pkg/ingress/resolver/main.go diff --git a/controllers/nginx/pkg/template/template.go b/controllers/nginx/pkg/template/template.go index 43b62d2bd..8dc2b9073 100644 --- a/controllers/nginx/pkg/template/template.go +++ b/controllers/nginx/pkg/template/template.go @@ -137,6 +137,7 @@ var ( "buildRateLimit": buildRateLimit, "buildSSPassthroughUpstreams": buildSSPassthroughUpstreams, "buildResolvers": buildResolvers, + "isLocationAllowed": isLocationAllowed, "contains": strings.Contains, "hasPrefix": strings.HasPrefix, @@ -352,3 +353,13 @@ func buildRateLimit(input interface{}) []string { return limits } + +func isLocationAllowed(input interface{}) bool { + loc, ok := input.(*ingress.Location) + if !ok { + glog.Errorf("expected an ingress.Location type but %T was returned", input) + return false + } + + return loc.Denied == nil +} diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index f18b6a365..ef478fb2a 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl @@ -240,6 +240,7 @@ http { {{ end }} location {{ $path }} { + {{ if isLocationAllowed $location }} {{ if gt (len $location.Whitelist.CIDR) 0 }} {{ range $ip := $location.Whitelist.CIDR }} allow {{ $ip }};{{ end }} @@ -312,6 +313,10 @@ http { set $proxy_upstream_name "{{ $location.Backend }}"; {{ buildProxyPass $backends $location }} + {{ else }} + #{{ $location.Denied }} + return 503; + {{ end }} } {{ end }} @@ -326,6 +331,7 @@ http { # with an external software (like sysdig) location /nginx_status { allow 127.0.0.1; + allow ::1; deny all; access_log off; @@ -365,6 +371,7 @@ http { # TODO: enable extraction for vts module. location /internal_nginx_status { allow 127.0.0.1; + allow ::1; deny all; access_log off; diff --git a/core/pkg/cache/main_test.go b/core/pkg/cache/main_test.go new file mode 100644 index 000000000..72dc7617e --- /dev/null +++ b/core/pkg/cache/main_test.go @@ -0,0 +1,92 @@ +/* +Copyright 2015 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cache + +import ( + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/apis/extensions" + "k8s.io/kubernetes/pkg/client/cache" + "k8s.io/kubernetes/pkg/util/sets" +) + +func TestStoreToIngressLister(t *testing.T) { + store := cache.NewStore(cache.MetaNamespaceKeyFunc) + ids := sets.NewString("foo", "bar", "baz") + for id := range ids { + store.Add(&extensions.Ingress{ObjectMeta: api.ObjectMeta{Name: id}}) + } + sml := StoreToIngressLister{store} + + gotIngress := sml.List() + got := make([]string, len(gotIngress)) + for ix := range gotIngress { + ing, ok := gotIngress[ix].(*extensions.Ingress) + if !ok { + t.Errorf("expected an Ingress type") + } + got[ix] = ing.Name + } + if !ids.HasAll(got...) || len(got) != len(ids) { + t.Errorf("expected %v, got %v", ids, got) + } +} + +func TestStoreToSecretsLister(t *testing.T) { + store := cache.NewStore(cache.MetaNamespaceKeyFunc) + ids := sets.NewString("foo", "bar", "baz") + for id := range ids { + store.Add(&api.Secret{ObjectMeta: api.ObjectMeta{Name: id}}) + } + sml := StoreToSecretsLister{store} + + gotIngress := sml.List() + got := make([]string, len(gotIngress)) + for ix := range gotIngress { + s, ok := gotIngress[ix].(*api.Secret) + if !ok { + t.Errorf("expected a Secret type") + } + got[ix] = s.Name + } + if !ids.HasAll(got...) || len(got) != len(ids) { + t.Errorf("expected %v, got %v", ids, got) + } +} + +func TestStoreToConfigmapLister(t *testing.T) { + store := cache.NewStore(cache.MetaNamespaceKeyFunc) + ids := sets.NewString("foo", "bar", "baz") + for id := range ids { + store.Add(&api.ConfigMap{ObjectMeta: api.ObjectMeta{Name: id}}) + } + sml := StoreToConfigmapLister{store} + + gotIngress := sml.List() + got := make([]string, len(gotIngress)) + for ix := range gotIngress { + m, ok := gotIngress[ix].(*api.ConfigMap) + if !ok { + t.Errorf("expected an Ingress type") + } + got[ix] = m.Name + } + if !ids.HasAll(got...) || len(got) != len(ids) { + t.Errorf("expected %v, got %v", ids, got) + } +} diff --git a/core/pkg/ingress/annotations/auth/main.go b/core/pkg/ingress/annotations/auth/main.go index 70acde2e1..2921a550c 100644 --- a/core/pkg/ingress/annotations/auth/main.go +++ b/core/pkg/ingress/annotations/auth/main.go @@ -17,44 +17,31 @@ limitations under the License. package auth import ( - "errors" "fmt" "io/ioutil" "os" "regexp" + "github.com/pkg/errors" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/ingress/core/pkg/ingress/annotations/parser" + ing_errors "k8s.io/ingress/core/pkg/ingress/errors" + "k8s.io/ingress/core/pkg/ingress/resolver" ) const ( authType = "ingress.kubernetes.io/auth-type" authSecret = "ingress.kubernetes.io/auth-secret" authRealm = "ingress.kubernetes.io/auth-realm" - - // DefAuthDirectory default directory used to store files - // to authenticate request - DefAuthDirectory = "/etc/ingress-controller/auth" ) -func init() { - // TODO: check permissions required - os.MkdirAll(DefAuthDirectory, 0655) -} - var ( authTypeRegex = regexp.MustCompile(`basic|digest`) - - // ErrInvalidAuthType is return in case of unsupported authentication type - ErrInvalidAuthType = errors.New("invalid authentication type") - - // ErrMissingSecretName is returned when the name of the secret is missing - ErrMissingSecretName = errors.New("secret name is missing") - - // ErrMissingAuthInSecret is returned when there is no auth key in secret data - ErrMissingAuthInSecret = errors.New("the secret does not contains the auth key") + // AuthDirectory default directory used to store files + // to authenticate request + AuthDirectory = "/etc/ingress-controller/auth" ) // BasicDigest returns authentication configuration for an Ingress rule @@ -65,40 +52,53 @@ type BasicDigest struct { Secured bool `json:"secured"` } -// ParseAnnotations parses the annotations contained in the ingress +type auth struct { + secretResolver resolver.Secret + authDirectory string +} + +// NewParser creates a new authentication annotation parser +func NewParser(authDirectory string, sr resolver.Secret) parser.IngressAnnotation { + // TODO: check permissions required + os.MkdirAll(authDirectory, 0655) + return auth{sr, authDirectory} +} + +// Parse parses the annotations contained in the ingress // rule used to add authentication in the paths defined in the rule // and generated an htpasswd compatible file to be used as source // during the authentication process -func ParseAnnotations(ing *extensions.Ingress, authDir string, fn func(string) (*api.Secret, error)) (*BasicDigest, error) { - if ing.GetAnnotations() == nil { - return &BasicDigest{}, parser.ErrMissingAnnotations - } - +func (a auth) Parse(ing *extensions.Ingress) (interface{}, error) { at, err := parser.GetStringAnnotation(authType, ing) if err != nil { - return &BasicDigest{}, err + return nil, err } if !authTypeRegex.MatchString(at) { - return &BasicDigest{}, ErrInvalidAuthType + return nil, ing_errors.NewLocationDenied("invalid authentication type") } s, err := parser.GetStringAnnotation(authSecret, ing) if err != nil { - return &BasicDigest{}, err + return nil, ing_errors.LocationDenied{ + Reason: errors.Wrap(err, "error reading secret name from annotation"), + } } - secret, err := fn(fmt.Sprintf("%v/%v", ing.Namespace, s)) + name := fmt.Sprintf("%v/%v", ing.Namespace, s) + secret, err := a.secretResolver.GetSecret(name) if err != nil { - return &BasicDigest{}, err + return nil, ing_errors.LocationDenied{ + Reason: errors.Wrapf(err, "unexpected error reading secret %v", name), + } } realm, _ := parser.GetStringAnnotation(authRealm, ing) - passFile := fmt.Sprintf("%v/%v-%v.passwd", authDir, ing.GetNamespace(), ing.GetName()) + passFile := fmt.Sprintf("%v/%v-%v.passwd", a.authDirectory, ing.GetNamespace(), ing.GetName()) err = dumpSecret(passFile, secret) if err != nil { - return &BasicDigest{}, err + return nil, err } return &BasicDigest{ @@ -114,9 +114,18 @@ func ParseAnnotations(ing *extensions.Ingress, authDir string, fn func(string) ( func dumpSecret(filename string, secret *api.Secret) error { val, ok := secret.Data["auth"] if !ok { - return ErrMissingAuthInSecret + return ing_errors.LocationDenied{ + Reason: errors.Errorf("the secret %v does not contains a key with value auth", secret.Name), + } } // TODO: check permissions required - return ioutil.WriteFile(filename, val, 0777) + err := ioutil.WriteFile(filename, val, 0777) + if err != nil { + return ing_errors.LocationDenied{ + Reason: errors.Wrap(err, "unexpected error creating password file"), + } + } + + return nil } diff --git a/core/pkg/ingress/annotations/auth/main_test.go b/core/pkg/ingress/annotations/auth/main_test.go index bb6999018..6d4027313 100644 --- a/core/pkg/ingress/annotations/auth/main_test.go +++ b/core/pkg/ingress/annotations/auth/main_test.go @@ -23,6 +23,7 @@ import ( "testing" "time" + "github.com/pkg/errors" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/util/intstr" @@ -63,7 +64,14 @@ func buildIngress() *extensions.Ingress { } } -func mockSecret(name string) (*api.Secret, error) { +type mockSecret struct { +} + +func (m mockSecret) GetSecret(name string) (*api.Secret, error) { + if name != "default/demo-secret" { + return nil, errors.Errorf("there is no secret with name %v", name) + } + return &api.Secret{ ObjectMeta: api.ObjectMeta{ Namespace: api.NamespaceDefault, @@ -72,9 +80,12 @@ func mockSecret(name string) (*api.Secret, error) { Data: map[string][]byte{"auth": []byte("foo:$apr1$OFG3Xybp$ckL0FHDAkoXYIlH9.cysT0")}, }, nil } + func TestIngressWithoutAuth(t *testing.T) { ing := buildIngress() - _, err := ParseAnnotations(ing, "", mockSecret) + _, dir, _ := dummySecretContent(t) + defer os.RemoveAll(dir) + _, err := NewParser(dir, mockSecret{}).Parse(ing) if err == nil { t.Error("Expected error with ingress without annotations") } @@ -92,11 +103,14 @@ func TestIngressAuth(t *testing.T) { _, dir, _ := dummySecretContent(t) defer os.RemoveAll(dir) - auth, err := ParseAnnotations(ing, dir, mockSecret) + i, err := NewParser(dir, mockSecret{}).Parse(ing) if err != nil { t.Errorf("Uxpected error with ingress: %v", err) } - + auth, ok := i.(*BasicDigest) + if !ok { + t.Errorf("expected a BasicDigest type") + } if auth.Type != "basic" { t.Errorf("Expected basic as auth type but returned %s", auth.Type) } @@ -108,6 +122,24 @@ func TestIngressAuth(t *testing.T) { } } +func TestIngressAuthWithoutSecret(t *testing.T) { + ing := buildIngress() + + data := map[string]string{} + data[authType] = "basic" + data[authSecret] = "invalid-secret" + data[authRealm] = "-realm-" + ing.SetAnnotations(data) + + _, dir, _ := dummySecretContent(t) + defer os.RemoveAll(dir) + + _, err := NewParser(dir, mockSecret{}).Parse(ing) + if err == nil { + t.Errorf("expected an error with invalid secret name") + } +} + func dummySecretContent(t *testing.T) (string, string, *api.Secret) { dir, err := ioutil.TempDir("", fmt.Sprintf("%v", time.Now().Unix())) if err != nil { @@ -119,7 +151,7 @@ func dummySecretContent(t *testing.T) (string, string, *api.Secret) { t.Error(err) } defer tmpfile.Close() - s, _ := mockSecret("demo") + s, _ := mockSecret{}.GetSecret("default/demo-secret") return tmpfile.Name(), dir, s } diff --git a/core/pkg/ingress/annotations/authreq/main.go b/core/pkg/ingress/annotations/authreq/main.go index 3e5f33c7c..560a73868 100644 --- a/core/pkg/ingress/annotations/authreq/main.go +++ b/core/pkg/ingress/annotations/authreq/main.go @@ -17,13 +17,13 @@ limitations under the License. package authreq import ( - "fmt" "net/url" "strings" "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/ingress/core/pkg/ingress/annotations/parser" + ing_errors "k8s.io/ingress/core/pkg/ingress/errors" ) const ( @@ -57,44 +57,49 @@ func validMethod(method string) bool { return false } +type authReq struct { +} + +// NewParser creates a new authentication request annotation parser +func NewParser() parser.IngressAnnotation { + return authReq{} +} + // ParseAnnotations parses the annotations contained in the ingress // rule used to use an external URL as source for authentication -func ParseAnnotations(ing *extensions.Ingress) (External, error) { - if ing.GetAnnotations() == nil { - return External{}, parser.ErrMissingAnnotations - } - +func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) { str, err := parser.GetStringAnnotation(authURL, ing) if err != nil { - return External{}, err + return nil, err } + if str == "" { - return External{}, fmt.Errorf("an empty string is not a valid URL") + return nil, ing_errors.NewLocationDenied("an empty string is not a valid URL") } ur, err := url.Parse(str) if err != nil { - return External{}, err + return nil, err } if ur.Scheme == "" { - return External{}, fmt.Errorf("url scheme is empty") + return nil, ing_errors.NewLocationDenied("url scheme is empty") } if ur.Host == "" { - return External{}, fmt.Errorf("url host is empty") + return nil, ing_errors.NewLocationDenied("url host is empty") } if strings.Contains(ur.Host, "..") { - return External{}, fmt.Errorf("invalid url host") + return nil, ing_errors.NewLocationDenied("invalid url host") } m, _ := parser.GetStringAnnotation(authMethod, ing) if len(m) != 0 && !validMethod(m) { - return External{}, fmt.Errorf("invalid HTTP method") + return nil, ing_errors.NewLocationDenied("invalid HTTP method") } sb, _ := parser.GetBoolAnnotation(authBody, ing) - return External{ + return &External{ URL: str, Method: m, SendBody: sb, diff --git a/core/pkg/ingress/annotations/authreq/main_test.go b/core/pkg/ingress/annotations/authreq/main_test.go index 57f182599..696d8bdc0 100644 --- a/core/pkg/ingress/annotations/authreq/main_test.go +++ b/core/pkg/ingress/annotations/authreq/main_test.go @@ -87,15 +87,17 @@ func TestAnnotations(t *testing.T) { data[authBody] = fmt.Sprintf("%v", test.sendBody) data[authMethod] = fmt.Sprintf("%v", test.method) - u, err := ParseAnnotations(ing) - + i, err := NewParser().Parse(ing) if test.expErr { if err == nil { t.Errorf("%v: expected error but retuned nil", test.title) } continue } - + u, ok := i.(*External) + if !ok { + t.Errorf("%v: expected an External type", test.title) + } if u.URL != test.url { t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.url, u.URL) } diff --git a/core/pkg/ingress/annotations/authtls/main.go b/core/pkg/ingress/annotations/authtls/main.go index 3d80e0deb..79d4b22d5 100644 --- a/core/pkg/ingress/annotations/authtls/main.go +++ b/core/pkg/ingress/annotations/authtls/main.go @@ -17,11 +17,10 @@ limitations under the License. package authtls import ( - "fmt" - "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/ingress/core/pkg/ingress/annotations/parser" + ing_errors "k8s.io/ingress/core/pkg/ingress/errors" "k8s.io/ingress/core/pkg/k8s" ) @@ -30,6 +29,13 @@ const ( authTLSSecret = "ingress.kubernetes.io/auth-tls-secret" ) +// AuthCertificate has a method that searchs for a secret +// that contains a SSL certificate. +// The secret must contain 3 keys named: +type AuthCertificate interface { + GetAuthCertificate(string) (*SSLCert, error) +} + // SSLCert returns external authentication configuration for an Ingress rule type SSLCert struct { Secret string `json:"secret"` @@ -39,27 +45,31 @@ type SSLCert struct { PemSHA string `json:"pemSha"` } +type authTLS struct { + certResolver AuthCertificate +} + +// NewParser creates a new TLS authentication annotation parser +func NewParser(resolver AuthCertificate) parser.IngressAnnotation { + return authTLS{resolver} +} + // ParseAnnotations parses the annotations contained in the ingress // rule used to use an external URL as source for authentication -func ParseAnnotations(ing *extensions.Ingress, - fn func(secret string) (*SSLCert, error)) (*SSLCert, error) { - if ing.GetAnnotations() == nil { - return &SSLCert{}, parser.ErrMissingAnnotations - } - +func (a authTLS) Parse(ing *extensions.Ingress) (interface{}, error) { str, err := parser.GetStringAnnotation(authTLSSecret, ing) if err != nil { - return &SSLCert{}, err + return nil, err } if str == "" { - return &SSLCert{}, fmt.Errorf("an empty string is not a valid secret name") + return nil, ing_errors.NewLocationDenied("an empty string is not a valid secret name") } _, _, err = k8s.ParseNameNS(str) if err != nil { - return &SSLCert{}, err + return nil, ing_errors.NewLocationDenied("an empty string is not a valid secret name") } - return fn(str) + return a.certResolver.GetAuthCertificate(str) } diff --git a/core/pkg/ingress/annotations/cors/main.go b/core/pkg/ingress/annotations/cors/main.go index 53d9cca6d..71195863f 100644 --- a/core/pkg/ingress/annotations/cors/main.go +++ b/core/pkg/ingress/annotations/cors/main.go @@ -23,11 +23,19 @@ import ( ) const ( - cors = "ingress.kubernetes.io/enable-cors" + annotation = "ingress.kubernetes.io/enable-cors" ) -// ParseAnnotations parses the annotations contained in the ingress -// rule used to indicate if the location/s should allows CORS -func ParseAnnotations(ing *extensions.Ingress) (bool, error) { - return parser.GetBoolAnnotation(cors, ing) +type cors struct { +} + +// NewParser creates a new CORS annotation parser +func NewParser() parser.IngressAnnotation { + return cors{} +} + +// Parse parses the annotations contained in the ingress +// rule used to indicate if the location/s should allows CORS +func (a cors) Parse(ing *extensions.Ingress) (interface{}, error) { + return parser.GetBoolAnnotation(annotation, ing) } diff --git a/core/pkg/ingress/annotations/healthcheck/main.go b/core/pkg/ingress/annotations/healthcheck/main.go index 9dfc2050a..493195154 100644 --- a/core/pkg/ingress/annotations/healthcheck/main.go +++ b/core/pkg/ingress/annotations/healthcheck/main.go @@ -20,7 +20,7 @@ import ( "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/ingress/core/pkg/ingress/annotations/parser" - "k8s.io/ingress/core/pkg/ingress/defaults" + "k8s.io/ingress/core/pkg/ingress/resolver" ) const ( @@ -35,22 +35,32 @@ type Upstream struct { FailTimeout int `json:"failTimeout"` } +type healthCheck struct { + backendResolver resolver.DefaultBackend +} + +// NewParser creates a new health check annotation parser +func NewParser(br resolver.DefaultBackend) parser.IngressAnnotation { + return healthCheck{br} +} + // ParseAnnotations parses the annotations contained in the ingress // rule used to configure upstream check parameters -func ParseAnnotations(cfg defaults.Backend, ing *extensions.Ingress) *Upstream { +func (a healthCheck) Parse(ing *extensions.Ingress) (interface{}, error) { + defBackend := a.backendResolver.GetDefaultBackend() if ing.GetAnnotations() == nil { - return &Upstream{cfg.UpstreamMaxFails, cfg.UpstreamFailTimeout} + return &Upstream{defBackend.UpstreamMaxFails, defBackend.UpstreamFailTimeout}, nil } mf, err := parser.GetIntAnnotation(upsMaxFails, ing) if err != nil { - mf = cfg.UpstreamMaxFails + mf = defBackend.UpstreamMaxFails } ft, err := parser.GetIntAnnotation(upsFailTimeout, ing) if err != nil { - ft = cfg.UpstreamFailTimeout + ft = defBackend.UpstreamFailTimeout } - return &Upstream{mf, ft} + return &Upstream{mf, ft}, nil } diff --git a/core/pkg/ingress/annotations/healthcheck/main_test.go b/core/pkg/ingress/annotations/healthcheck/main_test.go index aba9c7385..21166cbe0 100644 --- a/core/pkg/ingress/annotations/healthcheck/main_test.go +++ b/core/pkg/ingress/annotations/healthcheck/main_test.go @@ -61,6 +61,13 @@ func buildIngress() *extensions.Ingress { } } +type mockBackend struct { +} + +func (m mockBackend) GetDefaultBackend() defaults.Backend { + return defaults.Backend{UpstreamFailTimeout: 1} +} + func TestIngressHealthCheck(t *testing.T) { ing := buildIngress() @@ -68,15 +75,17 @@ func TestIngressHealthCheck(t *testing.T) { data[upsMaxFails] = "2" ing.SetAnnotations(data) - cfg := defaults.Backend{UpstreamFailTimeout: 1} - - nginxHz := ParseAnnotations(cfg, ing) + hzi, _ := NewParser(mockBackend{}).Parse(ing) + nginxHz, ok := hzi.(*Upstream) + if !ok { + t.Errorf("expected a Upstream type") + } if nginxHz.MaxFails != 2 { - t.Errorf("Expected 2 as max-fails but returned %v", nginxHz.MaxFails) + t.Errorf("expected 2 as max-fails but returned %v", nginxHz.MaxFails) } if nginxHz.FailTimeout != 1 { - t.Errorf("Expected 0 as fail-timeout but returned %v", nginxHz.FailTimeout) + t.Errorf("expected 0 as fail-timeout but returned %v", nginxHz.FailTimeout) } } diff --git a/core/pkg/ingress/annotations/ipwhitelist/main.go b/core/pkg/ingress/annotations/ipwhitelist/main.go index 06a22f84d..0e44ff9d6 100644 --- a/core/pkg/ingress/annotations/ipwhitelist/main.go +++ b/core/pkg/ingress/annotations/ipwhitelist/main.go @@ -17,51 +17,55 @@ limitations under the License. package ipwhitelist import ( - "errors" "sort" "strings" + "github.com/pkg/errors" "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/util/net/sets" "k8s.io/ingress/core/pkg/ingress/annotations/parser" - "k8s.io/ingress/core/pkg/ingress/defaults" + ing_errors "k8s.io/ingress/core/pkg/ingress/errors" + "k8s.io/ingress/core/pkg/ingress/resolver" ) const ( whitelist = "ingress.kubernetes.io/whitelist-source-range" ) -var ( - // ErrInvalidCIDR returned error when the whitelist annotation does not - // contains a valid IP or network address - ErrInvalidCIDR = errors.New("the annotation does not contains a valid IP address or network") -) - // SourceRange returns the CIDR type SourceRange struct { CIDR []string `json:"cidr"` } +type ipwhitelist struct { + backendResolver resolver.DefaultBackend +} + +// NewParser creates a new whitelist annotation parser +func NewParser(br resolver.DefaultBackend) parser.IngressAnnotation { + return ipwhitelist{br} +} + // ParseAnnotations parses the annotations contained in the ingress // rule used to limit access to certain client addresses or networks. // Multiple ranges can specified using commas as separator // e.g. `18.0.0.0/8,56.0.0.0/8` -func ParseAnnotations(cfg defaults.Backend, ing *extensions.Ingress) (*SourceRange, error) { - sort.Strings(cfg.WhitelistSourceRange) - if ing.GetAnnotations() == nil { - return &SourceRange{CIDR: cfg.WhitelistSourceRange}, parser.ErrMissingAnnotations - } +func (a ipwhitelist) Parse(ing *extensions.Ingress) (interface{}, error) { + defBackend := a.backendResolver.GetDefaultBackend() + sort.Strings(defBackend.WhitelistSourceRange) val, err := parser.GetStringAnnotation(whitelist, ing) if err != nil { - return &SourceRange{CIDR: cfg.WhitelistSourceRange}, err + return &SourceRange{CIDR: defBackend.WhitelistSourceRange}, err } values := strings.Split(val, ",") ipnets, err := sets.ParseIPNets(values...) if err != nil { - return &SourceRange{CIDR: cfg.WhitelistSourceRange}, ErrInvalidCIDR + return &SourceRange{CIDR: defBackend.WhitelistSourceRange}, ing_errors.LocationDenied{ + Reason: errors.Wrap(err, "the annotation does not contains a valid IP address or network"), + } } cidrs := []string{} diff --git a/core/pkg/ingress/annotations/ipwhitelist/main_test.go b/core/pkg/ingress/annotations/ipwhitelist/main_test.go index 257547ddf..d3f60adc1 100644 --- a/core/pkg/ingress/annotations/ipwhitelist/main_test.go +++ b/core/pkg/ingress/annotations/ipwhitelist/main_test.go @@ -62,6 +62,13 @@ func buildIngress() *extensions.Ingress { } } +type mockBackend struct { +} + +func (m mockBackend) GetDefaultBackend() defaults.Backend { + return defaults.Backend{} +} + func TestParseAnnotations(t *testing.T) { // TODO: convert test cases to tables ing := buildIngress() @@ -77,38 +84,56 @@ func TestParseAnnotations(t *testing.T) { CIDR: enet, } - sr, err := ParseAnnotations(defaults.Backend{}, ing) + p := NewParser(mockBackend{}) + + i, err := p.Parse(ing) if err != nil { - t.Errorf("Unexpected error: %v", err) + t.Errorf("unexpected error: %v", err) + } + sr, ok := i.(*SourceRange) + if !ok { + t.Errorf("expected a SourceRange type") } if !reflect.DeepEqual(sr, expected) { - t.Errorf("Expected %v but returned %s", sr, expected) + t.Errorf("expected %v but returned %s", sr, expected) } data[whitelist] = "www" - _, err = ParseAnnotations(defaults.Backend{}, ing) + _, err = p.Parse(ing) if err == nil { - t.Errorf("Expected error parsing an invalid cidr") + t.Errorf("expected error parsing an invalid cidr") } delete(data, whitelist) ing.SetAnnotations(data) - sr, err = ParseAnnotations(defaults.Backend{}, ing) + i, err = p.Parse(ing) + sr, ok = i.(*SourceRange) + if !ok { + t.Errorf("expected a SourceRange type") + } if err == nil { - t.Errorf("Expected error parsing an invalid cidr") + t.Errorf("expected error parsing an invalid cidr") } if !strsEquals(sr.CIDR, []string{}) { - t.Errorf("Expected empty CIDR but %v returned", sr.CIDR) + t.Errorf("expected empty CIDR but %v returned", sr.CIDR) } - sr, _ = ParseAnnotations(defaults.Backend{}, &extensions.Ingress{}) + i, _ = p.Parse(&extensions.Ingress{}) + sr, ok = i.(*SourceRange) + if !ok { + t.Errorf("expected a SourceRange type") + } if !strsEquals(sr.CIDR, []string{}) { - t.Errorf("Expected empty CIDR but %v returned", sr.CIDR) + t.Errorf("expected empty CIDR but %v returned", sr.CIDR) } data[whitelist] = "2.2.2.2/32,1.1.1.1/32,3.3.3.0/24" - sr, _ = ParseAnnotations(defaults.Backend{}, ing) + i, _ = p.Parse(ing) + sr, ok = i.(*SourceRange) + if !ok { + t.Errorf("expected a SourceRange type") + } ecidr := []string{"1.1.1.1/32", "2.2.2.2/32", "3.3.3.0/24"} if !strsEquals(sr.CIDR, ecidr) { t.Errorf("Expected %v CIDR but %v returned", ecidr, sr.CIDR) diff --git a/core/pkg/ingress/annotations/parser/main.go b/core/pkg/ingress/annotations/parser/main.go index 31736cf82..eb505ae74 100644 --- a/core/pkg/ingress/annotations/parser/main.go +++ b/core/pkg/ingress/annotations/parser/main.go @@ -17,32 +17,30 @@ limitations under the License. package parser import ( - "errors" - "fmt" "strconv" "k8s.io/kubernetes/pkg/apis/extensions" + + "k8s.io/ingress/core/pkg/ingress/errors" ) -var ( - // ErrMissingAnnotations is returned when the ingress rule - // does not contains annotations related with rate limit - ErrMissingAnnotations = errors.New("Ingress rule without annotations") - - // ErrInvalidName ... - ErrInvalidName = errors.New("invalid annotation name") -) +// IngressAnnotation has a method to parse annotations located in Ingress +type IngressAnnotation interface { + Parse(ing *extensions.Ingress) (interface{}, error) +} type ingAnnotations map[string]string func (a ingAnnotations) parseBool(name string) (bool, error) { val, ok := a[name] if ok { - if b, err := strconv.ParseBool(val); err == nil { - return b, nil + b, err := strconv.ParseBool(val) + if err != nil { + return false, errors.NewInvalidAnnotationContent(name) } + return b, nil } - return false, ErrMissingAnnotations + return false, errors.ErrMissingAnnotations } func (a ingAnnotations) parseString(name string) (string, error) { @@ -50,7 +48,7 @@ func (a ingAnnotations) parseString(name string) (string, error) { if ok { return val, nil } - return "", ErrMissingAnnotations + return "", errors.ErrMissingAnnotations } func (a ingAnnotations) parseInt(name string) (int, error) { @@ -58,45 +56,47 @@ func (a ingAnnotations) parseInt(name string) (int, error) { if ok { i, err := strconv.Atoi(val) if err != nil { - return 0, fmt.Errorf("invalid annotations value: %v", err) + return 0, errors.NewInvalidAnnotationContent(name) } return i, nil } - return 0, ErrMissingAnnotations + return 0, errors.ErrMissingAnnotations } -// GetBoolAnnotation ... -func GetBoolAnnotation(name string, ing *extensions.Ingress) (bool, error) { - if ing == nil || ing.GetAnnotations() == nil { - return false, ErrMissingAnnotations +func checkAnnotation(name string, ing *extensions.Ingress) error { + if ing == nil || len(ing.GetAnnotations()) == 0 { + return errors.ErrMissingAnnotations } if name == "" { - return false, ErrInvalidName + return errors.ErrInvalidAnnotationName } + return nil +} + +// GetBoolAnnotation extracts a boolean from an Ingress annotation +func GetBoolAnnotation(name string, ing *extensions.Ingress) (bool, error) { + err := checkAnnotation(name, ing) + if err != nil { + return false, err + } return ingAnnotations(ing.GetAnnotations()).parseBool(name) } -// GetStringAnnotation ... +// GetStringAnnotation extracts a string from an Ingress annotation func GetStringAnnotation(name string, ing *extensions.Ingress) (string, error) { - if ing == nil || ing.GetAnnotations() == nil { - return "", ErrMissingAnnotations + err := checkAnnotation(name, ing) + if err != nil { + return "", err } - if name == "" { - return "", ErrInvalidName - } - return ingAnnotations(ing.GetAnnotations()).parseString(name) } -// GetIntAnnotation ... +// GetIntAnnotation extracts an int from an Ingress annotation func GetIntAnnotation(name string, ing *extensions.Ingress) (int, error) { - if ing == nil || ing.GetAnnotations() == nil { - return 0, ErrMissingAnnotations + err := checkAnnotation(name, ing) + if err != nil { + return 0, err } - if name == "" { - return 0, ErrInvalidName - } - return ingAnnotations(ing.GetAnnotations()).parseInt(name) } diff --git a/core/pkg/ingress/annotations/parser/main_test.go b/core/pkg/ingress/annotations/parser/main_test.go index 1c756cdc2..099e76556 100644 --- a/core/pkg/ingress/annotations/parser/main_test.go +++ b/core/pkg/ingress/annotations/parser/main_test.go @@ -70,6 +70,8 @@ func TestGetBoolAnnotation(t *testing.T) { if u != test.exp { t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.name, test.exp, u) } + + delete(data, test.field) } } @@ -110,6 +112,8 @@ func TestGetStringAnnotation(t *testing.T) { if s != test.exp { t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.name, test.exp, s) } + + delete(data, test.field) } } @@ -150,5 +154,7 @@ func TestGetIntAnnotation(t *testing.T) { if s != test.exp { t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.name, test.exp, s) } + + delete(data, test.field) } } diff --git a/core/pkg/ingress/annotations/proxy/main.go b/core/pkg/ingress/annotations/proxy/main.go index d749f1849..97f3dddc0 100644 --- a/core/pkg/ingress/annotations/proxy/main.go +++ b/core/pkg/ingress/annotations/proxy/main.go @@ -20,7 +20,7 @@ import ( "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/ingress/core/pkg/ingress/annotations/parser" - "k8s.io/ingress/core/pkg/ingress/defaults" + "k8s.io/ingress/core/pkg/ingress/resolver" ) const ( @@ -38,37 +38,38 @@ type Configuration struct { BufferSize string `json:"bufferSize"` } +type proxy struct { + backendResolver resolver.DefaultBackend +} + +// NewParser creates a new reverse proxy configuration annotation parser +func NewParser(br resolver.DefaultBackend) parser.IngressAnnotation { + return proxy{br} +} + // ParseAnnotations parses the annotations contained in the ingress // rule used to configure upstream check parameters -func ParseAnnotations(cfg defaults.Backend, ing *extensions.Ingress) *Configuration { - if ing == nil || ing.GetAnnotations() == nil { - return &Configuration{ - cfg.ProxyConnectTimeout, - cfg.ProxySendTimeout, - cfg.ProxyReadTimeout, - cfg.ProxyBufferSize, - } - } - +func (a proxy) Parse(ing *extensions.Ingress) (interface{}, error) { + defBackend := a.backendResolver.GetDefaultBackend() ct, err := parser.GetIntAnnotation(connect, ing) if err != nil { - ct = cfg.ProxyConnectTimeout + ct = defBackend.ProxyConnectTimeout } st, err := parser.GetIntAnnotation(send, ing) if err != nil { - st = cfg.ProxySendTimeout + st = defBackend.ProxySendTimeout } rt, err := parser.GetIntAnnotation(read, ing) if err != nil { - rt = cfg.ProxyReadTimeout + rt = defBackend.ProxyReadTimeout } bs, err := parser.GetStringAnnotation(bufferSize, ing) if err != nil || bs == "" { - bs = cfg.ProxyBufferSize + bs = defBackend.ProxyBufferSize } - return &Configuration{ct, st, rt, bs} + return &Configuration{ct, st, rt, bs}, nil } diff --git a/core/pkg/ingress/annotations/proxy/main_test.go b/core/pkg/ingress/annotations/proxy/main_test.go index 337408d68..6b39a2d12 100644 --- a/core/pkg/ingress/annotations/proxy/main_test.go +++ b/core/pkg/ingress/annotations/proxy/main_test.go @@ -61,7 +61,14 @@ func buildIngress() *extensions.Ingress { } } -func TestIngressHealthCheck(t *testing.T) { +type mockBackend struct { +} + +func (m mockBackend) GetDefaultBackend() defaults.Backend { + return defaults.Backend{UpstreamFailTimeout: 1} +} + +func TestProxy(t *testing.T) { ing := buildIngress() data := map[string]string{} @@ -71,20 +78,24 @@ func TestIngressHealthCheck(t *testing.T) { data[bufferSize] = "1k" ing.SetAnnotations(data) - cfg := defaults.Backend{UpstreamFailTimeout: 1} - - p := ParseAnnotations(cfg, ing) - + i, err := NewParser(mockBackend{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error parsing a valid") + } + p, ok := i.(*Configuration) + if !ok { + t.Errorf("expected a Configuration type") + } if p.ConnectTimeout != 1 { - t.Errorf("Expected 1 as connect-timeout but returned %v", p.ConnectTimeout) + t.Errorf("expected 1 as connect-timeout but returned %v", p.ConnectTimeout) } if p.SendTimeout != 2 { - t.Errorf("Expected 2 as send-timeout but returned %v", p.SendTimeout) + t.Errorf("expected 2 as send-timeout but returned %v", p.SendTimeout) } if p.ReadTimeout != 3 { - t.Errorf("Expected 3 as read-timeout but returned %v", p.ReadTimeout) + t.Errorf("expected 3 as read-timeout but returned %v", p.ReadTimeout) } if p.BufferSize != "1k" { - t.Errorf("Expected 1k as buffer-size but returned %v", p.BufferSize) + t.Errorf("expected 1k as buffer-size but returned %v", p.BufferSize) } } diff --git a/core/pkg/ingress/annotations/ratelimit/main.go b/core/pkg/ingress/annotations/ratelimit/main.go index 774bf6e07..83ce4d2c0 100644 --- a/core/pkg/ingress/annotations/ratelimit/main.go +++ b/core/pkg/ingress/annotations/ratelimit/main.go @@ -17,7 +17,6 @@ limitations under the License. package ratelimit import ( - "errors" "fmt" "k8s.io/kubernetes/pkg/apis/extensions" @@ -37,11 +36,6 @@ const ( defSharedSize = 5 ) -var ( - // ErrInvalidRateLimit is returned when the annotation caontains invalid values - ErrInvalidRateLimit = errors.New("invalid rate limit value. Must be > 0") -) - // RateLimit returns rate limit configuration for an Ingress rule // Is possible to limit the number of connections per IP address or // connections per second. @@ -63,12 +57,17 @@ type Zone struct { SharedSize int `json:"sharedSize"` } +type ratelimit struct { +} + +// NewParser creates a new ratelimit annotation parser +func NewParser() parser.IngressAnnotation { + return ratelimit{} +} + // ParseAnnotations parses the annotations contained in the ingress // rule used to rewrite the defined paths -func ParseAnnotations(ing *extensions.Ingress) (*RateLimit, error) { - if ing.GetAnnotations() == nil { - return &RateLimit{}, parser.ErrMissingAnnotations - } +func (a ratelimit) Parse(ing *extensions.Ingress) (interface{}, error) { rps, _ := parser.GetIntAnnotation(limitRPS, ing) conn, _ := parser.GetIntAnnotation(limitIP, ing) @@ -77,7 +76,7 @@ func ParseAnnotations(ing *extensions.Ingress) (*RateLimit, error) { return &RateLimit{ Connections: Zone{}, RPS: Zone{}, - }, ErrInvalidRateLimit + }, nil } zoneName := fmt.Sprintf("%v_%v", ing.GetNamespace(), ing.GetName()) diff --git a/core/pkg/ingress/annotations/ratelimit/main_test.go b/core/pkg/ingress/annotations/ratelimit/main_test.go index 3095662ce..46ad25d54 100644 --- a/core/pkg/ingress/annotations/ratelimit/main_test.go +++ b/core/pkg/ingress/annotations/ratelimit/main_test.go @@ -61,9 +61,9 @@ func buildIngress() *extensions.Ingress { func TestWithoutAnnotations(t *testing.T) { ing := buildIngress() - _, err := ParseAnnotations(ing) - if err == nil { - t.Error("Expected error with ingress without annotations") + _, err := NewParser().Parse(ing) + if err != nil { + t.Error("unexpected error with ingress without annotations") } } @@ -75,9 +75,9 @@ func TestBadRateLimiting(t *testing.T) { data[limitRPS] = "0" ing.SetAnnotations(data) - _, err := ParseAnnotations(ing) - if err == nil { - t.Errorf("Expected error with invalid limits (0)") + _, err := NewParser().Parse(ing) + if err != nil { + t.Errorf("unexpected error with invalid limits (0)") } data = map[string]string{} @@ -85,16 +85,18 @@ func TestBadRateLimiting(t *testing.T) { data[limitRPS] = "100" ing.SetAnnotations(data) - rateLimit, err := ParseAnnotations(ing) + i, err := NewParser().Parse(ing) if err != nil { - t.Errorf("Uxpected error: %v", err) + t.Errorf("unexpected error: %v", err) + } + rateLimit, ok := i.(*RateLimit) + if !ok { + t.Errorf("expected a RateLimit type") } - if rateLimit.Connections.Limit != 5 { - t.Errorf("Expected 5 in limit by ip but %v was returend", rateLimit.Connections) + t.Errorf("expected 5 in limit by ip but %v was returend", rateLimit.Connections) } - if rateLimit.RPS.Limit != 100 { - t.Errorf("Expected 100 in limit by rps but %v was returend", rateLimit.RPS) + t.Errorf("expected 100 in limit by rps but %v was returend", rateLimit.RPS) } } diff --git a/core/pkg/ingress/annotations/rewrite/main.go b/core/pkg/ingress/annotations/rewrite/main.go index 33ff7bcad..c7e57c253 100644 --- a/core/pkg/ingress/annotations/rewrite/main.go +++ b/core/pkg/ingress/annotations/rewrite/main.go @@ -17,12 +17,10 @@ limitations under the License. package rewrite import ( - "errors" - "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/ingress/core/pkg/ingress/annotations/parser" - "k8s.io/ingress/core/pkg/ingress/defaults" + "k8s.io/ingress/core/pkg/ingress/resolver" ) const ( @@ -42,19 +40,27 @@ type Redirect struct { SSLRedirect bool `json:"sslRedirect"` } +type rewrite struct { + backendResolver resolver.DefaultBackend +} + +// NewParser creates a new reqrite annotation parser +func NewParser(br resolver.DefaultBackend) parser.IngressAnnotation { + return rewrite{br} +} + // ParseAnnotations parses the annotations contained in the ingress // rule used to rewrite the defined paths -func ParseAnnotations(cfg defaults.Backend, ing *extensions.Ingress) (*Redirect, error) { - if ing.GetAnnotations() == nil { - return &Redirect{}, errors.New("no annotations present") +func (a rewrite) Parse(ing *extensions.Ingress) (interface{}, error) { + rt, err := parser.GetStringAnnotation(rewriteTo, ing) + if err != nil { + return nil, err } sslRe, err := parser.GetBoolAnnotation(sslRedirect, ing) if err != nil { - sslRe = cfg.SSLRedirect + sslRe = a.backendResolver.GetDefaultBackend().SSLRedirect } - - rt, _ := parser.GetStringAnnotation(rewriteTo, ing) abu, _ := parser.GetBoolAnnotation(addBaseURL, ing) return &Redirect{ Target: rt, diff --git a/core/pkg/ingress/annotations/rewrite/main_test.go b/core/pkg/ingress/annotations/rewrite/main_test.go index c20919fd9..56ba6a9b7 100644 --- a/core/pkg/ingress/annotations/rewrite/main_test.go +++ b/core/pkg/ingress/annotations/rewrite/main_test.go @@ -65,9 +65,17 @@ func buildIngress() *extensions.Ingress { } } +type mockBackend struct { + redirect bool +} + +func (m mockBackend) GetDefaultBackend() defaults.Backend { + return defaults.Backend{SSLRedirect: m.redirect} +} + func TestWithoutAnnotations(t *testing.T) { ing := buildIngress() - _, err := ParseAnnotations(defaults.Backend{}, ing) + _, err := NewParser(mockBackend{}).Parse(ing) if err == nil { t.Error("Expected error with ingress without annotations") } @@ -80,11 +88,14 @@ func TestRedirect(t *testing.T) { data[rewriteTo] = defRoute ing.SetAnnotations(data) - redirect, err := ParseAnnotations(defaults.Backend{}, ing) + i, err := NewParser(mockBackend{}).Parse(ing) if err != nil { - t.Errorf("Uxpected error with ingress: %v", err) + t.Errorf("Unexpected error with ingress: %v", err) + } + redirect, ok := i.(*Redirect) + if !ok { + t.Errorf("expected a Redirect type") } - if redirect.Target != defRoute { t.Errorf("Expected %v as redirect but returned %s", defRoute, redirect.Target) } @@ -93,13 +104,18 @@ func TestRedirect(t *testing.T) { func TestSSLRedirect(t *testing.T) { ing := buildIngress() - cfg := defaults.Backend{SSLRedirect: true} - data := map[string]string{} - + data[rewriteTo] = defRoute ing.SetAnnotations(data) - redirect, _ := ParseAnnotations(cfg, ing) + i, _ := NewParser(mockBackend{true}).Parse(ing) + redirect, ok := i.(*Redirect) + if !ok { + t.Errorf("expected a Redirect type") + } + if !redirect.SSLRedirect { + t.Errorf("Expected true but returned false") + } if !redirect.SSLRedirect { t.Errorf("Expected true but returned false") @@ -108,8 +124,11 @@ func TestSSLRedirect(t *testing.T) { data[sslRedirect] = "false" ing.SetAnnotations(data) - redirect, _ = ParseAnnotations(cfg, ing) - + i, _ = NewParser(mockBackend{false}).Parse(ing) + redirect, ok = i.(*Redirect) + if !ok { + t.Errorf("expected a Redirect type") + } if redirect.SSLRedirect { t.Errorf("Expected false but returned true") } diff --git a/core/pkg/ingress/annotations/secureupstream/main.go b/core/pkg/ingress/annotations/secureupstream/main.go index 32240df32..732b577cd 100644 --- a/core/pkg/ingress/annotations/secureupstream/main.go +++ b/core/pkg/ingress/annotations/secureupstream/main.go @@ -26,8 +26,16 @@ const ( secureUpstream = "ingress.kubernetes.io/secure-backends" ) -// ParseAnnotations parses the annotations contained in the ingress +type su struct { +} + +// NewParser creates a new secure upstream annotation parser +func NewParser() parser.IngressAnnotation { + return su{} +} + +// Parse parses the annotations contained in the ingress // rule used to indicate if the upstream servers should use SSL -func ParseAnnotations(ing *extensions.Ingress) (bool, error) { +func (a su) Parse(ing *extensions.Ingress) (interface{}, error) { return parser.GetBoolAnnotation(secureUpstream, ing) } diff --git a/core/pkg/ingress/annotations/secureupstream/main_test.go b/core/pkg/ingress/annotations/secureupstream/main_test.go index eeb319600..a841e28c7 100644 --- a/core/pkg/ingress/annotations/secureupstream/main_test.go +++ b/core/pkg/ingress/annotations/secureupstream/main_test.go @@ -65,7 +65,7 @@ func TestAnnotations(t *testing.T) { data[secureUpstream] = "true" ing.SetAnnotations(data) - _, err := ParseAnnotations(ing) + _, err := NewParser().Parse(ing) if err != nil { t.Error("Expected error with ingress without annotations") } @@ -73,7 +73,7 @@ func TestAnnotations(t *testing.T) { func TestWithoutAnnotations(t *testing.T) { ing := buildIngress() - _, err := ParseAnnotations(ing) + _, err := NewParser().Parse(ing) if err == nil { t.Error("Expected error with ingress without annotations") } diff --git a/core/pkg/ingress/annotations/sslpassthrough/main.go b/core/pkg/ingress/annotations/sslpassthrough/main.go index 765cca227..99fcd0ab3 100644 --- a/core/pkg/ingress/annotations/sslpassthrough/main.go +++ b/core/pkg/ingress/annotations/sslpassthrough/main.go @@ -17,28 +17,29 @@ limitations under the License. package sslpassthrough import ( - "fmt" - "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/ingress/core/pkg/ingress/annotations/parser" - "k8s.io/ingress/core/pkg/ingress/defaults" + ing_errors "k8s.io/ingress/core/pkg/ingress/errors" ) const ( passthrough = "ingress.kubernetes.io/ssl-passthrough" ) +type sslpt struct { +} + +// NewParser creates a new SSL passthrough annotation parser +func NewParser() parser.IngressAnnotation { + return sslpt{} +} + // ParseAnnotations parses the annotations contained in the ingress // rule used to indicate if is required to configure -func ParseAnnotations(cfg defaults.Backend, ing *extensions.Ingress) (bool, error) { - +func (a sslpt) Parse(ing *extensions.Ingress) (interface{}, error) { if ing.GetAnnotations() == nil { - return false, parser.ErrMissingAnnotations - } - - if len(ing.Spec.TLS) == 0 { - return false, fmt.Errorf("ingres rule %v/%v does not contains a TLS section", ing.Name, ing.Namespace) + return false, ing_errors.ErrMissingAnnotations } return parser.GetBoolAnnotation(passthrough, ing) diff --git a/core/pkg/ingress/annotations/sslpassthrough/main_test.go b/core/pkg/ingress/annotations/sslpassthrough/main_test.go index 1ae6d64b3..3f1f6286b 100644 --- a/core/pkg/ingress/annotations/sslpassthrough/main_test.go +++ b/core/pkg/ingress/annotations/sslpassthrough/main_test.go @@ -22,8 +22,6 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/util/intstr" - - "k8s.io/ingress/core/pkg/ingress/defaults" ) func buildIngress() *extensions.Ingress { @@ -44,7 +42,7 @@ func buildIngress() *extensions.Ingress { func TestParseAnnotations(t *testing.T) { ing := buildIngress() - _, err := ParseAnnotations(defaults.Backend{}, ing) + _, err := NewParser().Parse(ing) if err == nil { t.Errorf("unexpected error: %v", err) } @@ -53,9 +51,9 @@ func TestParseAnnotations(t *testing.T) { data[passthrough] = "true" ing.SetAnnotations(data) // test ingress using the annotation without a TLS section - val, err := ParseAnnotations(defaults.Backend{}, ing) - if err == nil { - t.Errorf("expected error parsing an invalid cidr") + _, err = NewParser().Parse(ing) + if err != nil { + t.Errorf("unexpected error parsing ingress with sslpassthrough") } // test with a valid host @@ -64,9 +62,13 @@ func TestParseAnnotations(t *testing.T) { Hosts: []string{"foo.bar.com"}, }, } - val, err = ParseAnnotations(defaults.Backend{}, ing) + i, err := NewParser().Parse(ing) if err != nil { - t.Errorf("expected error parsing an invalid cidr") + t.Errorf("expected error parsing ingress with sslpassthrough") + } + val, ok := i.(bool) + if !ok { + t.Errorf("expected a bool type") } if !val { t.Errorf("expected true but false returned") diff --git a/core/pkg/ingress/controller/annotations.go b/core/pkg/ingress/controller/annotations.go new file mode 100644 index 000000000..27c0baf1a --- /dev/null +++ b/core/pkg/ingress/controller/annotations.go @@ -0,0 +1,114 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "github.com/golang/glog" + + "k8s.io/kubernetes/pkg/apis/extensions" + + "k8s.io/ingress/core/pkg/ingress/annotations/auth" + "k8s.io/ingress/core/pkg/ingress/annotations/authreq" + "k8s.io/ingress/core/pkg/ingress/annotations/authtls" + "k8s.io/ingress/core/pkg/ingress/annotations/cors" + "k8s.io/ingress/core/pkg/ingress/annotations/healthcheck" + "k8s.io/ingress/core/pkg/ingress/annotations/ipwhitelist" + "k8s.io/ingress/core/pkg/ingress/annotations/parser" + "k8s.io/ingress/core/pkg/ingress/annotations/proxy" + "k8s.io/ingress/core/pkg/ingress/annotations/ratelimit" + "k8s.io/ingress/core/pkg/ingress/annotations/rewrite" + "k8s.io/ingress/core/pkg/ingress/annotations/secureupstream" + "k8s.io/ingress/core/pkg/ingress/annotations/sslpassthrough" + "k8s.io/ingress/core/pkg/ingress/errors" + "k8s.io/ingress/core/pkg/ingress/resolver" +) + +type extractorConfig interface { + resolver.AuthCertificate + resolver.DefaultBackend + resolver.Secret +} + +type annotationExtractor struct { + annotations map[string]parser.IngressAnnotation +} + +func newAnnotationExtractor(cfg extractorConfig) annotationExtractor { + return annotationExtractor{ + map[string]parser.IngressAnnotation{ + "BasicDigestAuth": auth.NewParser(auth.AuthDirectory, cfg), + "ExternalAuth": authreq.NewParser(), + "CertificateAuth": authtls.NewParser(cfg), + "EnableCORS": cors.NewParser(), + "HealthCheck": healthcheck.NewParser(cfg), + "Whitelist": ipwhitelist.NewParser(cfg), + "Proxy": proxy.NewParser(cfg), + "RateLimit": ratelimit.NewParser(), + "Redirect": rewrite.NewParser(cfg), + "SecureUpstream": secureupstream.NewParser(), + "SSLPassthrough": sslpassthrough.NewParser(), + }, + } +} + +func (e *annotationExtractor) Extract(ing *extensions.Ingress) map[string]interface{} { + anns := make(map[string]interface{}, 0) + for name, annotationParser := range e.annotations { + val, err := annotationParser.Parse(ing) + glog.V(5).Infof("annotation %v in Ingress %v/%v: %v", name, ing.GetNamespace(), ing.GetName(), val) + if err != nil { + _, de := anns["Denied"] + if errors.IsLocationDenied(err) && !de { + anns["Denied"] = err + glog.Errorf("error reading %v annotation in Ingress %v/%v: %v", name, ing.GetNamespace(), ing.GetName(), err) + continue + } + if !errors.IsMissingAnnotations(err) { + glog.Errorf("error reading %v annotation in Ingress %v/%v: %v", name, ing.GetNamespace(), ing.GetName(), err) + continue + } + glog.V(5).Infof("error reading %v annotation in Ingress %v/%v: %v", name, ing.GetNamespace(), ing.GetName(), err) + } + + if val != nil { + anns[name] = val + } + } + + return anns +} + +const ( + secureUpstream = "SecureUpstream" + healthCheck = "HealthCheck" + sslPassthrough = "SSLPassthrough" +) + +func (e *annotationExtractor) SecureUpstream(ing *extensions.Ingress) bool { + val, _ := e.annotations[secureUpstream].Parse(ing) + return val.(bool) +} + +func (e *annotationExtractor) HealthCheck(ing *extensions.Ingress) *healthcheck.Upstream { + val, _ := e.annotations[healthCheck].Parse(ing) + return val.(*healthcheck.Upstream) +} + +func (e *annotationExtractor) SSLPassthrough(ing *extensions.Ingress) bool { + val, _ := e.annotations[sslPassthrough].Parse(ing) + return val.(bool) +} diff --git a/core/pkg/ingress/controller/annotations_test.go b/core/pkg/ingress/controller/annotations_test.go new file mode 100644 index 000000000..431e3c1a8 --- /dev/null +++ b/core/pkg/ingress/controller/annotations_test.go @@ -0,0 +1,92 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/apis/extensions" + "k8s.io/kubernetes/pkg/util/intstr" + + "k8s.io/ingress/core/pkg/ingress/annotations/authtls" + "k8s.io/ingress/core/pkg/ingress/defaults" +) + +type mockCfg struct { +} + +func (m mockCfg) GetDefaultBackend() defaults.Backend { + return defaults.Backend{} +} + +func (m mockCfg) GetSecret(string) (*api.Secret, error) { + return nil, nil +} + +func (m mockCfg) GetAuthCertificate(string) (*authtls.SSLCert, error) { + return nil, nil +} + +func TestAnnotationExtractor(t *testing.T) { + ec := newAnnotationExtractor(mockCfg{}) + ing := buildIngress() + + m := ec.Extract(ing) + // the map at least should contains HealthCheck and Proxy information (defaults) + if _, ok := m["HealthCheck"]; !ok { + t.Error("expected HealthCheck annotation") + } + if _, ok := m["Proxy"]; !ok { + t.Error("expected Proxy annotation") + } +} + +func buildIngress() *extensions.Ingress { + defaultBackend := extensions.IngressBackend{ + ServiceName: "default-backend", + ServicePort: intstr.FromInt(80), + } + + return &extensions.Ingress{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + Namespace: api.NamespaceDefault, + }, + Spec: extensions.IngressSpec{ + Backend: &extensions.IngressBackend{ + ServiceName: "default-backend", + ServicePort: intstr.FromInt(80), + }, + Rules: []extensions.IngressRule{ + { + Host: "foo.bar.com", + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: "/foo", + Backend: defaultBackend, + }, + }, + }, + }, + }, + }, + }, + } +} diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index 46679ee99..1851bb9b5 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -39,18 +39,11 @@ import ( cache_store "k8s.io/ingress/core/pkg/cache" "k8s.io/ingress/core/pkg/ingress" - "k8s.io/ingress/core/pkg/ingress/annotations/auth" - "k8s.io/ingress/core/pkg/ingress/annotations/authreq" "k8s.io/ingress/core/pkg/ingress/annotations/authtls" - "k8s.io/ingress/core/pkg/ingress/annotations/cors" "k8s.io/ingress/core/pkg/ingress/annotations/healthcheck" - "k8s.io/ingress/core/pkg/ingress/annotations/ipwhitelist" "k8s.io/ingress/core/pkg/ingress/annotations/proxy" - "k8s.io/ingress/core/pkg/ingress/annotations/ratelimit" - "k8s.io/ingress/core/pkg/ingress/annotations/rewrite" - "k8s.io/ingress/core/pkg/ingress/annotations/secureupstream" "k8s.io/ingress/core/pkg/ingress/annotations/service" - "k8s.io/ingress/core/pkg/ingress/annotations/sslpassthrough" + "k8s.io/ingress/core/pkg/ingress/defaults" "k8s.io/ingress/core/pkg/ingress/status" "k8s.io/ingress/core/pkg/k8s" local_strings "k8s.io/ingress/core/pkg/strings" @@ -90,6 +83,8 @@ type GenericController struct { secrLister cache_store.StoreToSecretsLister mapLister cache_store.StoreToConfigmapLister + annotations annotationExtractor + recorder record.EventRecorder syncQueue *task.Queue @@ -268,6 +263,8 @@ func newIngressController(config *Configuration) *GenericController { IngressLister: ic.ingLister, }) + ic.annotations = newAnnotationExtractor(ic) + return &ic } @@ -289,8 +286,13 @@ func (ic GenericController) IngressClass() string { return ic.cfg.IngressClass } -// getSecret searchs for a secret in the local secrets Store -func (ic *GenericController) getSecret(name string) (*api.Secret, error) { +// GetDefaultBackend returns the default backend +func (ic GenericController) GetDefaultBackend() defaults.Backend { + return ic.cfg.Backend.BackendDefaults() +} + +// GetSecret searchs for a secret in the local secrets Store +func (ic GenericController) GetSecret(name string) (*api.Secret, error) { s, exists, err := ic.secrLister.Store.GetByKey(name) if err != nil { return nil, err @@ -551,53 +553,10 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress upstreams := ic.createUpstreams(ings) servers := ic.createServers(ings, upstreams) - upsDefaults := ic.cfg.Backend.BackendDefaults() - for _, ingIf := range ings { ing := ingIf.(*extensions.Ingress) - nginxAuth, err := auth.ParseAnnotations(ing, auth.DefAuthDirectory, ic.getSecret) - glog.V(5).Infof("auth annotation: %v", nginxAuth) - if err != nil { - glog.V(5).Infof("error reading authentication in Ingress %v/%v: %v", ing.GetNamespace(), ing.GetName(), err) - } - - rl, err := ratelimit.ParseAnnotations(ing) - glog.V(5).Infof("rate limit annotation: %v", rl) - if err != nil { - glog.V(5).Infof("error reading rate limit annotation in Ingress %v/%v: %v", ing.GetNamespace(), ing.GetName(), err) - } - - locRew, err := rewrite.ParseAnnotations(upsDefaults, ing) - if err != nil { - glog.V(5).Infof("error parsing rewrite annotations for Ingress rule %v/%v: %v", ing.GetNamespace(), ing.GetName(), err) - } - - wl, err := ipwhitelist.ParseAnnotations(upsDefaults, ing) - glog.V(5).Infof("white list annotation: %v", wl) - if err != nil { - glog.V(5).Infof("error reading white list annotation in Ingress %v/%v: %v", ing.GetNamespace(), ing.GetName(), err) - } - - eCORS, err := cors.ParseAnnotations(ing) - if err != nil { - glog.V(5).Infof("error reading CORS annotation in Ingress %v/%v: %v", ing.GetNamespace(), ing.GetName(), err) - } - - ra, err := authreq.ParseAnnotations(ing) - glog.V(5).Infof("auth request annotation: %v", ra) - if err != nil { - glog.V(5).Infof("error reading auth request annotation in Ingress %v/%v: %v", ing.GetNamespace(), ing.GetName(), err) - } - - prx := proxy.ParseAnnotations(upsDefaults, ing) - glog.V(5).Infof("proxy timeouts annotation: %v", prx) - - certAuth, err := authtls.ParseAnnotations(ing, ic.getAuthCertificate) - glog.V(5).Infof("auth request annotation: %v", certAuth) - if err != nil { - glog.V(5).Infof("error reading certificate auth annotation in Ingress %v/%v: %v", ing.GetNamespace(), ing.GetName(), err) - } + anns := ic.annotations.Extract(ing) for _, rule := range ing.Spec.Rules { host := rule.Host @@ -664,34 +623,21 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress glog.V(3).Infof("replacing ingress rule %v/%v location %v upstream %v (%v)", ing.Namespace, ing.Name, loc.Path, ups.Name, loc.Backend) loc.Backend = ups.Name loc.IsDefBackend = false - loc.BasicDigestAuth = *nginxAuth - loc.RateLimit = *rl - loc.Redirect = *locRew - loc.Whitelist = *wl loc.Backend = ups.Name - loc.EnableCORS = eCORS - loc.ExternalAuth = ra - loc.Proxy = *prx - loc.CertificateAuth = *certAuth + mergeLocationAnnotations(loc, anns) break } } // is a new location if addLoc { glog.V(3).Infof("adding location %v in ingress rule %v/%v upstream %v", nginxPath, ing.Namespace, ing.Name, ups.Name) - server.Locations = append(server.Locations, &ingress.Location{ - Path: nginxPath, - Backend: ups.Name, - IsDefBackend: false, - BasicDigestAuth: *nginxAuth, - RateLimit: *rl, - Redirect: *locRew, - Whitelist: *wl, - EnableCORS: eCORS, - ExternalAuth: ra, - Proxy: *prx, - CertificateAuth: *certAuth, - }) + loc := &ingress.Location{ + Path: nginxPath, + Backend: ups.Name, + IsDefBackend: false, + } + mergeLocationAnnotations(loc, anns) + server.Locations = append(server.Locations, loc) } } } @@ -721,7 +667,8 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress return aUpstreams, aServers } -func (ic *GenericController) getAuthCertificate(secretName string) (*authtls.SSLCert, error) { +// GetAuthCertificate ... +func (ic GenericController) GetAuthCertificate(secretName string) (*authtls.SSLCert, error) { bc, exists := ic.sslCertTracker.Get(secretName) if !exists { return &authtls.SSLCert{}, fmt.Errorf("secret %v does not exists", secretName) @@ -741,16 +688,11 @@ func (ic *GenericController) createUpstreams(data []interface{}) map[string]*ing upstreams := make(map[string]*ingress.Backend) upstreams[defUpstreamName] = ic.getDefaultUpstream() - upsDefaults := ic.cfg.Backend.BackendDefaults() for _, ingIf := range data { ing := ingIf.(*extensions.Ingress) - secUpstream, err := secureupstream.ParseAnnotations(ing) - if err != nil { - glog.V(5).Infof("error reading secure upstream in Ingress %v/%v: %v", ing.GetNamespace(), ing.GetName(), err) - } - - hz := healthcheck.ParseAnnotations(upsDefaults, ing) + secUpstream := ic.annotations.SecureUpstream(ing) + hz := ic.annotations.HealthCheck(ing) var defBackend string if ing.Spec.Backend != nil { @@ -843,9 +785,16 @@ func (ic *GenericController) serviceEndpoints(svcKey, backendPort string, func (ic *GenericController) createServers(data []interface{}, upstreams map[string]*ingress.Backend) map[string]*ingress.Server { servers := make(map[string]*ingress.Server) - ngxProxy := *proxy.ParseAnnotations(ic.cfg.Backend.BackendDefaults(), nil) - upsDefaults := ic.cfg.Backend.BackendDefaults() + bdef := ic.GetDefaultBackend() + ngxProxy := proxy.Configuration{ + ConnectTimeout: bdef.ProxyConnectTimeout, + SendTimeout: bdef.ProxySendTimeout, + ReadTimeout: bdef.ProxyReadTimeout, + BufferSize: bdef.ProxyBufferSize, + } + + dun := ic.getDefaultUpstream().Name // default server servers[defServerName] = &ingress.Server{ @@ -854,7 +803,7 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str { Path: rootLocation, IsDefBackend: true, - Backend: ic.getDefaultUpstream().Name, + Backend: dun, Proxy: ngxProxy, }, }} @@ -863,10 +812,7 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str for _, ingIf := range data { ing := ingIf.(*extensions.Ingress) // check if ssl passthrough is configured - sslpt, err := sslpassthrough.ParseAnnotations(upsDefaults, ing) - if err != nil { - glog.V(5).Infof("error reading ssl passthrough annotation in Ingress %v/%v: %v", ing.GetNamespace(), ing.GetName(), err) - } + sslpt := ic.annotations.SSLPassthrough(ing) for _, rule := range ing.Spec.Rules { host := rule.Host @@ -883,7 +829,7 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str { Path: rootLocation, IsDefBackend: true, - Backend: ic.getDefaultUpstream().Name, + Backend: dun, Proxy: ngxProxy, }, }, SSLPassthrough: sslpt} diff --git a/core/pkg/ingress/controller/util.go b/core/pkg/ingress/controller/util.go index c3e4f249c..57b31dba3 100644 --- a/core/pkg/ingress/controller/util.go +++ b/core/pkg/ingress/controller/util.go @@ -19,6 +19,9 @@ package controller import ( "strings" + "github.com/golang/glog" + "github.com/imdario/mergo" + "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/ingress/core/pkg/ingress" @@ -93,3 +96,16 @@ func IsValidClass(ing *extensions.Ingress, class string) bool { return cc == class } + +const denied = "Denied" + +func mergeLocationAnnotations(loc *ingress.Location, anns map[string]interface{}) { + if _, ok := anns[denied]; ok { + loc.Denied = anns[denied].(error) + } + delete(anns, denied) + err := mergo.Map(loc, anns) + if err != nil { + glog.Errorf("unexpected error merging extracted annotations in location type: %v", err) + } +} diff --git a/core/pkg/ingress/errors/errors.go b/core/pkg/ingress/errors/errors.go new file mode 100644 index 000000000..924753963 --- /dev/null +++ b/core/pkg/ingress/errors/errors.go @@ -0,0 +1,89 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package errors + +import ( + "fmt" + + "github.com/pkg/errors" +) + +var ( + // ErrMissingAnnotations the ingress rule does not contains annotations + // This is an error only when annotations are being parsed + ErrMissingAnnotations = errors.New("ingress rule without annotations") + + // ErrInvalidAnnotationName the ingress rule does contains an invalid + // annotation name + ErrInvalidAnnotationName = errors.New("invalid annotation name") + + // ErrInvalidAnnotationContent the ingress rule annotation content is + // invalid + ErrInvalidAnnotationContent = errors.New("invalid annotation content") +) + +// NewInvalidAnnotationContent returns a new InvalidContent error +func NewInvalidAnnotationContent(name string) error { + return InvalidContent{ + Name: fmt.Sprintf("the annotation %v does not contains a valid value", name), + } +} + +// NewLocationDenied returns a new LocationDenied error +func NewLocationDenied(reason string) error { + return LocationDenied{ + Reason: errors.Errorf("Location denied, reason: %v", reason), + } +} + +// InvalidContent error +type InvalidContent struct { + Name string +} + +func (e InvalidContent) Error() string { + return e.Name +} + +// LocationDenied error +type LocationDenied struct { + Reason error +} + +func (e LocationDenied) Error() string { + return e.Reason.Error() +} + +// IsLocationDenied checks if the err is an error which +// indicates a location should return HTTP code 503 +func IsLocationDenied(e error) bool { + _, ok := e.(LocationDenied) + return ok +} + +// IsMissingAnnotations checks if the err is an error which +// indicates the ingress does not contains annotations +func IsMissingAnnotations(e error) bool { + return e == ErrMissingAnnotations +} + +// IsInvalidContent checks if the err is an error which +// indicates an annotations value is not valid +func IsInvalidContent(e error) bool { + _, ok := e.(InvalidContent) + return ok +} diff --git a/core/pkg/ingress/errors/errors_test.go b/core/pkg/ingress/errors/errors_test.go new file mode 100644 index 000000000..c0bd52406 --- /dev/null +++ b/core/pkg/ingress/errors/errors_test.go @@ -0,0 +1,52 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package errors + +import "testing" + +func TestIsLocationDenied(t *testing.T) { + err := NewLocationDenied("demo") + if !IsLocationDenied(err) { + t.Error("expected true") + } + if IsLocationDenied(nil) { + t.Error("expected false") + } +} + +func TestIsMissingAnnotations(t *testing.T) { + if !IsMissingAnnotations(ErrMissingAnnotations) { + t.Error("expected true") + } +} + +func TestInvalidContent(t *testing.T) { + if IsInvalidContent(ErrMissingAnnotations) { + t.Error("expected false") + } + err := NewInvalidAnnotationContent("demo") + if !IsInvalidContent(err) { + t.Error("expected true") + } + if IsInvalidContent(nil) { + t.Error("expected false") + } + err = NewLocationDenied("demo") + if IsInvalidContent(err) { + t.Error("expected false") + } +} diff --git a/core/pkg/ingress/resolver/main.go b/core/pkg/ingress/resolver/main.go new file mode 100644 index 000000000..c345ad289 --- /dev/null +++ b/core/pkg/ingress/resolver/main.go @@ -0,0 +1,43 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resolver + +import ( + "k8s.io/kubernetes/pkg/api" + + "k8s.io/ingress/core/pkg/ingress/annotations/authtls" + "k8s.io/ingress/core/pkg/ingress/defaults" +) + +// DefaultBackend has a method that returns the backend +// that must be used as default +type DefaultBackend interface { + GetDefaultBackend() defaults.Backend +} + +// Secret has a method that searchs for secrets contenating +// the namespace and name using a the character / +type Secret interface { + GetSecret(string) (*api.Secret, error) +} + +// AuthCertificate has a method that searchs for a secret +// that contains a SSL certificate. +// The secret must contain 3 keys named: +type AuthCertificate interface { + GetAuthCertificate(string) (*authtls.SSLCert, error) +} diff --git a/core/pkg/ingress/types.go b/core/pkg/ingress/types.go index 7a0f5929a..e26b1e9c5 100644 --- a/core/pkg/ingress/types.go +++ b/core/pkg/ingress/types.go @@ -203,6 +203,9 @@ type Location struct { // an Ingress rule. // +optional BasicDigestAuth auth.BasicDigest `json:"basicDigestAuth,omitempty"` + // Denied returns an error when this location cannot not be allowed + // Requesting a denied location should return HTTP code 403. + Denied error // EnableCORS indicates if path must support CORS // +optional EnableCORS bool `json:"enableCors,omitempty"` diff --git a/core/pkg/task/queue_test.go b/core/pkg/task/queue_test.go index 295c62637..5d2cf2f41 100644 --- a/core/pkg/task/queue_test.go +++ b/core/pkg/task/queue_test.go @@ -18,11 +18,12 @@ package task import ( "fmt" + "sync/atomic" "testing" "time" ) -var sr int = 0 +var sr uint32 type mockEnqueueObj struct { k string @@ -31,7 +32,7 @@ type mockEnqueueObj struct { func mockSynFn(interface{}) error { // sr will be plus one times after enqueue - sr++ + atomic.AddUint32(&sr, 1) return nil } @@ -60,7 +61,7 @@ func TestShutdown(t *testing.T) { func TestEnqueueSuccess(t *testing.T) { // initialize result - sr = 0 + atomic.StoreUint32(&sr, 0) q := NewCustomTaskQueue(mockSynFn, mockKeyFn) stopCh := make(chan struct{}) // run queue @@ -73,7 +74,7 @@ func TestEnqueueSuccess(t *testing.T) { q.Enqueue(mo) // wait for 'mockSynFn' time.Sleep(time.Millisecond * 10) - if sr != 1 { + if atomic.LoadUint32(&sr) != 1 { t.Errorf("sr should be 1, but is %d", sr) } @@ -83,7 +84,7 @@ func TestEnqueueSuccess(t *testing.T) { func TestEnqueueFailed(t *testing.T) { // initialize result - sr = 0 + atomic.StoreUint32(&sr, 0) q := NewCustomTaskQueue(mockSynFn, mockKeyFn) stopCh := make(chan struct{}) // run queue @@ -102,14 +103,14 @@ func TestEnqueueFailed(t *testing.T) { // wait for 'mockSynFn' time.Sleep(time.Millisecond * 10) // queue is shutdown, so mockSynFn should not be executed, so the result should be 0 - if sr != 0 { + if atomic.LoadUint32(&sr) != 0 { t.Errorf("queue has been shutdown, so sr should be 0, but is %d", sr) } } func TestEnqueueKeyError(t *testing.T) { // initialize result - sr = 0 + atomic.StoreUint32(&sr, 0) q := NewCustomTaskQueue(mockSynFn, mockErrorKeyFn) stopCh := make(chan struct{}) // run queue @@ -124,7 +125,7 @@ func TestEnqueueKeyError(t *testing.T) { // wait for 'mockSynFn' time.Sleep(time.Millisecond * 10) // key error, so the result should be 0 - if sr != 0 { + if atomic.LoadUint32(&sr) != 0 { t.Errorf("error occurs while get key, so sr should be 0, but is %d", sr) } // shutdown queue before exit