From a8d2f0244ea0f7a1d9ded61a22add3bd7dc5c1c0 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 19 Nov 2017 16:38:11 -0300 Subject: [PATCH] Refactoring of cache, informers and store helpers --- .travis.yml | 4 + cmd/nginx/flags.go | 14 +- cmd/nginx/main.go | 1 - internal/ingress/annotations/alias/main.go | 5 +- .../ingress/annotations/alias/main_test.go | 3 +- internal/ingress/annotations/annotations.go | 20 +- .../ingress/annotations/annotations_test.go | 31 +- internal/ingress/annotations/auth/main.go | 6 +- .../ingress/annotations/auth/main_test.go | 13 +- internal/ingress/annotations/authreq/main.go | 14 +- .../ingress/annotations/authreq/main_test.go | 18 +- internal/ingress/annotations/authtls/main.go | 10 +- internal/ingress/annotations/class/main.go | 16 +- .../ingress/annotations/class/main_test.go | 14 +- .../annotations/clientbodybuffersize/main.go | 11 +- .../clientbodybuffersize/main_test.go | 6 +- internal/ingress/annotations/cors/main.go | 19 +- .../ingress/annotations/cors/main_test.go | 14 +- .../annotations/defaultbackend/main.go | 2 +- .../ingress/annotations/healthcheck/main.go | 4 +- .../annotations/healthcheck/main_test.go | 3 +- .../ingress/annotations/ipwhitelist/main.go | 2 +- .../annotations/ipwhitelist/main_test.go | 5 +- internal/ingress/annotations/parser/main.go | 24 +- .../ingress/annotations/parser/main_test.go | 26 +- .../annotations/portinredirect/main.go | 2 +- .../annotations/portinredirect/main_test.go | 3 +- internal/ingress/annotations/proxy/main.go | 24 +- .../ingress/annotations/proxy/main_test.go | 17 +- .../ingress/annotations/ratelimit/main.go | 12 +- .../annotations/ratelimit/main_test.go | 17 +- .../ingress/annotations/redirect/redirect.go | 6 +- internal/ingress/annotations/rewrite/main.go | 12 +- .../ingress/annotations/rewrite/main_test.go | 13 +- .../annotations/secureupstream/main.go | 4 +- .../annotations/secureupstream/main_test.go | 13 +- .../ingress/annotations/serversnippet/main.go | 11 +- .../annotations/serversnippet/main_test.go | 6 +- .../annotations/serviceupstream/main.go | 11 +- .../annotations/serviceupstream/main_test.go | 12 +- .../annotations/sessionaffinity/main.go | 6 +- .../annotations/sessionaffinity/main_test.go | 8 +- internal/ingress/annotations/snippet/main.go | 13 +- .../ingress/annotations/snippet/main_test.go | 6 +- .../annotations/sslpassthrough/main.go | 11 +- .../annotations/sslpassthrough/main_test.go | 10 +- .../annotations/upstreamhashby/main.go | 11 +- .../annotations/upstreamhashby/main_test.go | 6 +- .../ingress/annotations/upstreamvhost/main.go | 11 +- .../ingress/annotations/vtsfilterkey/main.go | 15 +- internal/ingress/controller/controller.go | 190 ++----- internal/ingress/controller/listers.go | 228 -------- internal/ingress/controller/nginx.go | 158 +++--- internal/ingress/controller/process/nginx.go | 2 +- internal/ingress/resolver/main.go | 3 - internal/ingress/status/status.go | 99 ++-- internal/ingress/status/status_test.go | 53 +- .../{controller => store}/backend_ssl.go | 104 ++-- .../{controller => store}/backend_ssl_test.go | 26 +- internal/ingress/store/configmap.go | 41 ++ internal/ingress/store/endpoint.go | 40 ++ internal/ingress/store/ingress.go | 41 ++ internal/ingress/store/ingress_annotation.go | 26 + internal/ingress/store/local_secret.go | 30 ++ internal/ingress/store/local_secret_test.go | 39 ++ internal/ingress/store/main.go | 113 ---- internal/ingress/store/secret.go | 41 ++ internal/ingress/store/service.go | 41 ++ internal/ingress/store/store.go | 502 ++++++++++++++++++ internal/ingress/store/store_test.go | 315 +++++++++++ internal/ingress/types.go | 12 - internal/k8s/main.go | 13 + test/e2e/up.sh | 44 -- test/e2e/wait-for-nginx.sh | 61 +++ 74 files changed, 1700 insertions(+), 1067 deletions(-) delete mode 100644 internal/ingress/controller/listers.go rename internal/ingress/{controller => store}/backend_ssl.go (69%) rename internal/ingress/{controller => store}/backend_ssl_test.go (96%) create mode 100644 internal/ingress/store/configmap.go create mode 100644 internal/ingress/store/endpoint.go create mode 100644 internal/ingress/store/ingress.go create mode 100644 internal/ingress/store/ingress_annotation.go create mode 100644 internal/ingress/store/local_secret.go create mode 100644 internal/ingress/store/local_secret_test.go delete mode 100644 internal/ingress/store/main.go create mode 100644 internal/ingress/store/secret.go create mode 100644 internal/ingress/store/service.go create mode 100644 internal/ingress/store/store.go create mode 100644 internal/ingress/store/store_test.go create mode 100755 test/e2e/wait-for-nginx.sh diff --git a/.travis.yml b/.travis.yml index c40926a84..cd453b96a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -34,6 +34,9 @@ jobs: - go get github.com/golang/lint/golint - make fmt lint vet - stage: Coverage + before_script: + # start minikube + - test/e2e/up.sh script: - go get github.com/mattn/goveralls - go get github.com/modocache/gover @@ -44,6 +47,7 @@ jobs: before_script: - make e2e-image - test/e2e/up.sh + - test/e2e/wait-for-nginx.sh script: - make e2e-test # split builds to avoid job timeouts diff --git a/cmd/nginx/flags.go b/cmd/nginx/flags.go index e58230cc0..d80afb714 100644 --- a/cmd/nginx/flags.go +++ b/cmd/nginx/flags.go @@ -27,15 +27,13 @@ import ( apiv1 "k8s.io/api/core/v1" + "k8s.io/ingress-nginx/internal/ingress/annotations/class" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/controller" ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" ing_net "k8s.io/ingress-nginx/internal/net" ) -const ( - defIngressClass = "nginx" -) - func parseFlags() (bool, *controller.Configuration, error) { var ( flags = pflag.NewFlagSet("", pflag.ExitOnError) @@ -152,11 +150,15 @@ func parseFlags() (bool, *controller.Configuration, error) { if *ingressClass != "" { glog.Infof("Watching for ingress class: %s", *ingressClass) - if *ingressClass != defIngressClass { + if *ingressClass != class.DefaultClass { glog.Warningf("only Ingress with class \"%v\" will be processed by this ingress controller", *ingressClass) } + + class.IngressClass = *ingressClass } + parser.AnnotationsPrefix = *annotationsPrefix + // check port collisions if !ing_net.IsPortAvailable(*httpPort) { return false, nil, fmt.Errorf("Port %v is already in use. Please check the flag --http-port", *httpPort) @@ -188,7 +190,6 @@ func parseFlags() (bool, *controller.Configuration, error) { } config := &controller.Configuration{ - AnnotationsPrefix: *annotationsPrefix, APIServerHost: *apiserverHost, KubeConfigFile: *kubeConfigFile, UpdateStatus: *updateStatus, @@ -198,7 +199,6 @@ func parseFlags() (bool, *controller.Configuration, error) { EnableSSLChainCompletion: *enableSSLChainCompletion, ResyncPeriod: *resyncPeriod, DefaultService: *defaultSvc, - IngressClass: *ingressClass, Namespace: *watchNamespace, ConfigMapName: *configMap, TCPConfigMapName: *tcpConfigMapName, diff --git a/cmd/nginx/main.go b/cmd/nginx/main.go index caab12aca..2e5e677c6 100644 --- a/cmd/nginx/main.go +++ b/cmd/nginx/main.go @@ -128,7 +128,6 @@ func main() { conf.FakeCertificateSHA = c.PemSHA conf.Client = kubeClient - conf.DefaultIngressClass = defIngressClass ngx := controller.NewNGINXController(conf) diff --git a/internal/ingress/annotations/alias/main.go b/internal/ingress/annotations/alias/main.go index 2fb81b2a4..7e664e345 100644 --- a/internal/ingress/annotations/alias/main.go +++ b/internal/ingress/annotations/alias/main.go @@ -24,16 +24,15 @@ import ( ) type alias struct { - r resolver.Resolver } // NewParser creates a new Alias annotation parser func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return alias{r} + return alias{} } // Parse parses the annotations contained in the ingress rule // used to add an alias to the provided hosts func (a alias) Parse(ing *extensions.Ingress) (interface{}, error) { - return parser.GetStringAnnotation("server-alias", ing, a.r) + return parser.GetStringAnnotation("server-alias", ing) } diff --git a/internal/ingress/annotations/alias/main_test.go b/internal/ingress/annotations/alias/main_test.go index 579ed83f4..b1dba57c7 100644 --- a/internal/ingress/annotations/alias/main_test.go +++ b/internal/ingress/annotations/alias/main_test.go @@ -22,10 +22,11 @@ import ( api "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/resolver" ) -const annotation = "nginx/server-alias" +var annotation = parser.GetAnnotationWithPrefix("server-alias") func TestParse(t *testing.T) { ap := NewParser(&resolver.Mock{}) diff --git a/internal/ingress/annotations/annotations.go b/internal/ingress/annotations/annotations.go index 2bbd8814d..47f709a4f 100644 --- a/internal/ingress/annotations/annotations.go +++ b/internal/ingress/annotations/annotations.go @@ -95,25 +95,25 @@ func NewAnnotationExtractor(cfg resolver.Resolver) Extractor { "Alias": alias.NewParser(cfg), "BasicDigestAuth": auth.NewParser(auth.AuthDirectory, cfg), "CertificateAuth": authtls.NewParser(cfg), - "ClientBodyBufferSize": clientbodybuffersize.NewParser(cfg), - "ConfigurationSnippet": snippet.NewParser(cfg), - "CorsConfig": cors.NewParser(cfg), + "ClientBodyBufferSize": clientbodybuffersize.NewParser(), + "ConfigurationSnippet": snippet.NewParser(), + "CorsConfig": cors.NewParser(), "DefaultBackend": defaultbackend.NewParser(cfg), - "ExternalAuth": authreq.NewParser(cfg), + "ExternalAuth": authreq.NewParser(), "HealthCheck": healthcheck.NewParser(cfg), "Proxy": proxy.NewParser(cfg), "RateLimit": ratelimit.NewParser(cfg), "Redirect": redirect.NewParser(cfg), "Rewrite": rewrite.NewParser(cfg), "SecureUpstream": secureupstream.NewParser(cfg), - "ServerSnippet": serversnippet.NewParser(cfg), - "ServiceUpstream": serviceupstream.NewParser(cfg), + "ServerSnippet": serversnippet.NewParser(), + "ServiceUpstream": serviceupstream.NewParser(), "SessionAffinity": sessionaffinity.NewParser(cfg), - "SSLPassthrough": sslpassthrough.NewParser(cfg), + "SSLPassthrough": sslpassthrough.NewParser(), "UsePortInRedirects": portinredirect.NewParser(cfg), - "UpstreamHashBy": upstreamhashby.NewParser(cfg), - "UpstreamVhost": upstreamvhost.NewParser(cfg), - "VtsFilterKey": vtsfilterkey.NewParser(cfg), + "UpstreamHashBy": upstreamhashby.NewParser(), + "UpstreamVhost": upstreamvhost.NewParser(), + "VtsFilterKey": vtsfilterkey.NewParser(), "Whitelist": ipwhitelist.NewParser(cfg), }, } diff --git a/internal/ingress/annotations/annotations_test.go b/internal/ingress/annotations/annotations_test.go index fd086d43d..4657de41e 100644 --- a/internal/ingress/annotations/annotations_test.go +++ b/internal/ingress/annotations/annotations_test.go @@ -24,27 +24,28 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/resolver" ) -const ( - annotationSecureUpstream = "nginx/secure-backends" - annotationSecureVerifyCACert = "nginx/secure-verify-ca-secret" - annotationUpsMaxFails = "nginx/upstream-max-fails" - annotationUpsFailTimeout = "nginx/upstream-fail-timeout" - annotationPassthrough = "nginx/ssl-passthrough" - annotationAffinityType = "nginx/affinity" - annotationCorsEnabled = "nginx/enable-cors" - annotationCorsAllowOrigin = "nginx/cors-allow-origin" - annotationCorsAllowMethods = "nginx/cors-allow-methods" - annotationCorsAllowHeaders = "nginx/cors-allow-headers" - annotationCorsAllowCredentials = "nginx/cors-allow-credentials" +var ( + annotationSecureUpstream = parser.GetAnnotationWithPrefix("secure-backends") + annotationSecureVerifyCACert = parser.GetAnnotationWithPrefix("secure-verify-ca-secret") + annotationUpsMaxFails = parser.GetAnnotationWithPrefix("upstream-max-fails") + annotationUpsFailTimeout = parser.GetAnnotationWithPrefix("upstream-fail-timeout") + annotationPassthrough = parser.GetAnnotationWithPrefix("ssl-passthrough") + annotationAffinityType = parser.GetAnnotationWithPrefix("affinity") + annotationCorsEnabled = parser.GetAnnotationWithPrefix("enable-cors") + annotationCorsAllowOrigin = parser.GetAnnotationWithPrefix("cors-allow-origin") + annotationCorsAllowMethods = parser.GetAnnotationWithPrefix("cors-allow-methods") + annotationCorsAllowHeaders = parser.GetAnnotationWithPrefix("cors-allow-headers") + annotationCorsAllowCredentials = parser.GetAnnotationWithPrefix("cors-allow-credentials") defaultCorsMethods = "GET, PUT, POST, DELETE, PATCH, OPTIONS" defaultCorsHeaders = "DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Authorization" - annotationAffinityCookieName = "nginx/session-cookie-name" - annotationAffinityCookieHash = "nginx/session-cookie-hash" - annotationUpstreamHashBy = "nginx/upstream-hash-by" + annotationAffinityCookieName = parser.GetAnnotationWithPrefix("session-cookie-name") + annotationAffinityCookieHash = parser.GetAnnotationWithPrefix("session-cookie-hash") + annotationUpstreamHashBy = parser.GetAnnotationWithPrefix("upstream-hash-by") ) type mockCfg struct { diff --git a/internal/ingress/annotations/auth/main.go b/internal/ingress/annotations/auth/main.go index 0b2187368..1dd885f04 100644 --- a/internal/ingress/annotations/auth/main.go +++ b/internal/ingress/annotations/auth/main.go @@ -102,7 +102,7 @@ func NewParser(authDirectory string, r resolver.Resolver) parser.IngressAnnotati // and generated an htpasswd compatible file to be used as source // during the authentication process func (a auth) Parse(ing *extensions.Ingress) (interface{}, error) { - at, err := parser.GetStringAnnotation("auth-type", ing, a.r) + at, err := parser.GetStringAnnotation("auth-type", ing) if err != nil { return nil, err } @@ -111,7 +111,7 @@ func (a auth) Parse(ing *extensions.Ingress) (interface{}, error) { return nil, ing_errors.NewLocationDenied("invalid authentication type") } - s, err := parser.GetStringAnnotation("auth-secret", ing, a.r) + s, err := parser.GetStringAnnotation("auth-secret", ing) if err != nil { return nil, ing_errors.LocationDenied{ Reason: errors.Wrap(err, "error reading secret name from annotation"), @@ -126,7 +126,7 @@ func (a auth) Parse(ing *extensions.Ingress) (interface{}, error) { } } - realm, _ := parser.GetStringAnnotation("auth-realm", ing, a.r) + realm, _ := parser.GetStringAnnotation("auth-realm", ing) passFile := fmt.Sprintf("%v/%v-%v.passwd", a.authDirectory, ing.GetNamespace(), ing.GetName()) err = dumpSecret(passFile, secret) diff --git a/internal/ingress/annotations/auth/main_test.go b/internal/ingress/annotations/auth/main_test.go index c93dddb67..3546bd025 100644 --- a/internal/ingress/annotations/auth/main_test.go +++ b/internal/ingress/annotations/auth/main_test.go @@ -29,6 +29,7 @@ import ( extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -99,9 +100,9 @@ func TestIngressAuth(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/auth-type"] = "basic" - data["nginx/auth-secret"] = "demo-secret" - data["nginx/auth-realm"] = "-realm-" + data[parser.GetAnnotationWithPrefix("auth-type")] = "basic" + data[parser.GetAnnotationWithPrefix("auth-secret")] = "demo-secret" + data[parser.GetAnnotationWithPrefix("auth-realm")] = "-realm-" ing.SetAnnotations(data) _, dir, _ := dummySecretContent(t) @@ -130,9 +131,9 @@ func TestIngressAuthWithoutSecret(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/auth-type"] = "basic" - data["nginx/auth-secret"] = "invalid-secret" - data["nginx/auth-realm"] = "-realm-" + data[parser.GetAnnotationWithPrefix("auth-type")] = "basic" + data[parser.GetAnnotationWithPrefix("auth-secret")] = "invalid-secret" + data[parser.GetAnnotationWithPrefix("auth-realm")] = "-realm-" ing.SetAnnotations(data) _, dir, _ := dummySecretContent(t) diff --git a/internal/ingress/annotations/authreq/main.go b/internal/ingress/annotations/authreq/main.go index dbc9f51a2..b8059de95 100644 --- a/internal/ingress/annotations/authreq/main.go +++ b/internal/ingress/annotations/authreq/main.go @@ -25,7 +25,6 @@ import ( "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ing_errors "k8s.io/ingress-nginx/internal/ingress/errors" - "k8s.io/ingress-nginx/internal/ingress/resolver" ) // Config returns external authentication configuration for an Ingress rule @@ -101,18 +100,17 @@ func validHeader(header string) bool { } type authReq struct { - r resolver.Resolver } // NewParser creates a new authentication request annotation parser -func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return authReq{r} +func NewParser() parser.IngressAnnotation { + return authReq{} } // ParseAnnotations parses the annotations contained in the ingress // rule used to use an Config URL as source for authentication func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) { - str, err := parser.GetStringAnnotation("auth-url", ing, a.r) + str, err := parser.GetStringAnnotation("auth-url", ing) if err != nil { return nil, err } @@ -121,7 +119,7 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) { return nil, ing_errors.NewLocationDenied("an empty string is not a valid URL") } - signin, _ := parser.GetStringAnnotation("auth-signin", ing, a.r) + signin, _ := parser.GetStringAnnotation("auth-signin", ing) ur, err := url.Parse(str) if err != nil { @@ -138,13 +136,13 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) { return nil, ing_errors.NewLocationDenied("invalid url host") } - m, _ := parser.GetStringAnnotation("auth-method", ing, a.r) + m, _ := parser.GetStringAnnotation("auth-method", ing) if len(m) != 0 && !validMethod(m) { return nil, ing_errors.NewLocationDenied("invalid HTTP method") } h := []string{} - hstr, _ := parser.GetStringAnnotation("auth-response-headers", ing, a.r) + hstr, _ := parser.GetStringAnnotation("auth-response-headers", ing) if len(hstr) != 0 { harr := strings.Split(hstr, ",") diff --git a/internal/ingress/annotations/authreq/main_test.go b/internal/ingress/annotations/authreq/main_test.go index 2208cc24e..dc9e3948c 100644 --- a/internal/ingress/annotations/authreq/main_test.go +++ b/internal/ingress/annotations/authreq/main_test.go @@ -24,7 +24,7 @@ import ( api "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/ingress-nginx/internal/ingress/resolver" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -87,11 +87,11 @@ func TestAnnotations(t *testing.T) { } for _, test := range tests { - data["nginx/auth-url"] = test.url - data["nginx/auth-signin"] = test.signinURL - data["nginx/auth-method"] = fmt.Sprintf("%v", test.method) + data[parser.GetAnnotationWithPrefix("auth-url")] = test.url + data[parser.GetAnnotationWithPrefix("auth-signin")] = test.signinURL + data[parser.GetAnnotationWithPrefix("auth-method")] = fmt.Sprintf("%v", test.method) - i, err := NewParser(&resolver.Mock{}).Parse(ing) + i, err := NewParser().Parse(ing) if test.expErr { if err == nil { t.Errorf("%v: expected error but retuned nil", test.title) @@ -137,11 +137,11 @@ func TestHeaderAnnotations(t *testing.T) { } for _, test := range tests { - data["nginx/auth-url"] = test.url - data["nginx/auth-response-headers"] = test.headers - data["nginx/auth-method"] = "GET" + data[parser.GetAnnotationWithPrefix("auth-url")] = test.url + data[parser.GetAnnotationWithPrefix("auth-response-headers")] = test.headers + data[parser.GetAnnotationWithPrefix("auth-method")] = "GET" - i, err := NewParser(&resolver.Mock{}).Parse(ing) + i, err := NewParser().Parse(ing) if test.expErr { if err == nil { t.Errorf("%v: expected error but retuned nil", err.Error()) diff --git a/internal/ingress/annotations/authtls/main.go b/internal/ingress/annotations/authtls/main.go index 59862f159..7c608bb53 100644 --- a/internal/ingress/annotations/authtls/main.go +++ b/internal/ingress/annotations/authtls/main.go @@ -87,7 +87,7 @@ type authTLS struct { // rule used to use a Certificate as authentication method func (a authTLS) Parse(ing *extensions.Ingress) (interface{}, error) { - tlsauthsecret, err := parser.GetStringAnnotation(a.r.GetAnnotationWithPrefix("auth-tls-secret"), ing, a.r) + tlsauthsecret, err := parser.GetStringAnnotation("auth-tls-secret", ing) if err != nil { return &Config{}, err } @@ -101,12 +101,12 @@ func (a authTLS) Parse(ing *extensions.Ingress) (interface{}, error) { return &Config{}, ing_errors.NewLocationDenied(err.Error()) } - tlsVerifyClient, err := parser.GetStringAnnotation("auth-tls-verify-client", ing, a.r) + tlsVerifyClient, err := parser.GetStringAnnotation("auth-tls-verify-client", ing) if err != nil || !authVerifyClientRegex.MatchString(tlsVerifyClient) { tlsVerifyClient = defaultAuthVerifyClient } - tlsdepth, err := parser.GetIntAnnotation("auth-tls-verify-depth", ing, a.r) + tlsdepth, err := parser.GetIntAnnotation("auth-tls-verify-depth", ing) if err != nil || tlsdepth == 0 { tlsdepth = defaultAuthTLSDepth } @@ -118,12 +118,12 @@ func (a authTLS) Parse(ing *extensions.Ingress) (interface{}, error) { } } - errorpage, err := parser.GetStringAnnotation("auth-tls-error-page", ing, a.r) + errorpage, err := parser.GetStringAnnotation("auth-tls-error-page", ing) if err != nil || errorpage == "" { errorpage = "" } - passCert, err := parser.GetBoolAnnotation("auth-tls-pass-certificate-to-upstream", ing, a.r) + passCert, err := parser.GetBoolAnnotation("auth-tls-pass-certificate-to-upstream", ing) if err != nil { passCert = false } diff --git a/internal/ingress/annotations/class/main.go b/internal/ingress/annotations/class/main.go index 33c7c7629..812c63d91 100644 --- a/internal/ingress/annotations/class/main.go +++ b/internal/ingress/annotations/class/main.go @@ -28,10 +28,20 @@ const ( IngressKey = "kubernetes.io/ingress.class" ) +var ( + // DefaultClass defines the default class used in the nginx ingres controller + DefaultClass = "nginx" + + // IngressClass sets the runtime ingress class to use + // An empty string means accept all ingresses without + // annotation and the ones configured with class nginx + IngressClass = "" +) + // IsValid returns true if the given Ingress either doesn't specify // the ingress.class annotation, or it's set to the configured in the // ingress controller. -func IsValid(ing *extensions.Ingress, controller, defClass string) bool { +func IsValid(ing *extensions.Ingress) bool { ingress, ok := ing.GetAnnotations()[IngressKey] if !ok { glog.V(3).Infof("annotation %v is not present in ingress %v/%v", IngressKey, ing.Namespace, ing.Name) @@ -44,9 +54,9 @@ func IsValid(ing *extensions.Ingress, controller, defClass string) bool { // and 2 invalid combinations // 3 - ingress with default class | fixed annotation on ingress // 4 - ingress with specific class | different annotation on ingress - if ingress == "" && controller == defClass { + if ingress == "" && IngressClass == DefaultClass { return true } - return ingress == controller + return ingress == IngressClass } diff --git a/internal/ingress/annotations/class/main_test.go b/internal/ingress/annotations/class/main_test.go index 45f7c02e7..bb8d2db65 100644 --- a/internal/ingress/annotations/class/main_test.go +++ b/internal/ingress/annotations/class/main_test.go @@ -25,6 +25,14 @@ import ( ) func TestIsValidClass(t *testing.T) { + dc := DefaultClass + ic := IngressClass + // restore original values after the tests + defer func() { + DefaultClass = dc + IngressClass = ic + }() + tests := []struct { ingress string controller string @@ -51,7 +59,11 @@ func TestIsValidClass(t *testing.T) { ing.SetAnnotations(data) for _, test := range tests { ing.Annotations[IngressKey] = test.ingress - b := IsValid(ing, test.controller, test.defClass) + + IngressClass = test.controller + DefaultClass = test.defClass + + b := IsValid(ing) if b != test.isValid { t.Errorf("test %v - expected %v but %v was returned", test, test.isValid, b) } diff --git a/internal/ingress/annotations/clientbodybuffersize/main.go b/internal/ingress/annotations/clientbodybuffersize/main.go index 6ff3e070a..13d11658c 100644 --- a/internal/ingress/annotations/clientbodybuffersize/main.go +++ b/internal/ingress/annotations/clientbodybuffersize/main.go @@ -20,20 +20,17 @@ import ( extensions "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" - "k8s.io/ingress-nginx/internal/ingress/resolver" ) -type clientBodyBufferSize struct { - r resolver.Resolver -} +type clientBodyBufferSize struct{} // NewParser creates a new clientBodyBufferSize annotation parser -func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return clientBodyBufferSize{r} +func NewParser() parser.IngressAnnotation { + return clientBodyBufferSize{} } // Parse parses the annotations contained in the ingress rule // used to add an client-body-buffer-size to the provided locations func (cbbs clientBodyBufferSize) Parse(ing *extensions.Ingress) (interface{}, error) { - return parser.GetStringAnnotation("client-body-buffer-size", ing, cbbs.r) + return parser.GetStringAnnotation("client-body-buffer-size", ing) } diff --git a/internal/ingress/annotations/clientbodybuffersize/main_test.go b/internal/ingress/annotations/clientbodybuffersize/main_test.go index b47231498..a3a43235c 100644 --- a/internal/ingress/annotations/clientbodybuffersize/main_test.go +++ b/internal/ingress/annotations/clientbodybuffersize/main_test.go @@ -22,12 +22,12 @@ import ( api "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/ingress-nginx/internal/ingress/resolver" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ) func TestParse(t *testing.T) { - annotation := "nginx/client-body-buffer-size" - ap := NewParser(&resolver.Mock{}) + annotation := parser.GetAnnotationWithPrefix("client-body-buffer-size") + ap := NewParser() if ap == nil { t.Fatalf("expected a parser.IngressAnnotation but returned nil") } diff --git a/internal/ingress/annotations/cors/main.go b/internal/ingress/annotations/cors/main.go index ac77c250a..8fdd2bd81 100644 --- a/internal/ingress/annotations/cors/main.go +++ b/internal/ingress/annotations/cors/main.go @@ -22,7 +22,6 @@ import ( extensions "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" - "k8s.io/ingress-nginx/internal/ingress/resolver" ) const ( @@ -44,9 +43,7 @@ var ( corsHeadersRegex = regexp.MustCompile(`^([A-Za-z0-9\-\_]+,?\s?)+$`) ) -type cors struct { - r resolver.Resolver -} +type cors struct{} // Config contains the Cors configuration to be used in the Ingress type Config struct { @@ -58,8 +55,8 @@ type Config struct { } // NewParser creates a new CORS annotation parser -func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return cors{r} +func NewParser() parser.IngressAnnotation { + return cors{} } // Equal tests for equality between two External types @@ -92,27 +89,27 @@ func (c1 *Config) Equal(c2 *Config) bool { // Parse parses the annotations contained in the ingress // rule used to indicate if the location/s should allows CORS func (c cors) Parse(ing *extensions.Ingress) (interface{}, error) { - corsenabled, err := parser.GetBoolAnnotation("enable-cors", ing, c.r) + corsenabled, err := parser.GetBoolAnnotation("enable-cors", ing) if err != nil { corsenabled = false } - corsalloworigin, err := parser.GetStringAnnotation("cors-allow-origin", ing, c.r) + corsalloworigin, err := parser.GetStringAnnotation("cors-allow-origin", ing) if err != nil || corsalloworigin == "" || !corsOriginRegex.MatchString(corsalloworigin) { corsalloworigin = "*" } - corsallowheaders, err := parser.GetStringAnnotation("cors-allow-headers", ing, c.r) + corsallowheaders, err := parser.GetStringAnnotation("cors-allow-headers", ing) if err != nil || corsallowheaders == "" || !corsHeadersRegex.MatchString(corsallowheaders) { corsallowheaders = defaultCorsHeaders } - corsallowmethods, err := parser.GetStringAnnotation("cors-allow-methods", ing, c.r) + corsallowmethods, err := parser.GetStringAnnotation("cors-allow-methods", ing) if err != nil || corsallowmethods == "" || !corsMethodsRegex.MatchString(corsallowmethods) { corsallowmethods = defaultCorsMethods } - corsallowcredentials, err := parser.GetBoolAnnotation("cors-allow-credentials", ing, c.r) + corsallowcredentials, err := parser.GetBoolAnnotation("cors-allow-credentials", ing) if err != nil { corsallowcredentials = true } diff --git a/internal/ingress/annotations/cors/main_test.go b/internal/ingress/annotations/cors/main_test.go index 2eda3a0fa..779799fbe 100644 --- a/internal/ingress/annotations/cors/main_test.go +++ b/internal/ingress/annotations/cors/main_test.go @@ -23,7 +23,7 @@ import ( extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/ingress-nginx/internal/ingress/resolver" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ) func buildIngress() *extensions.Ingress { @@ -65,14 +65,14 @@ func TestIngressCorsConfig(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/enable-cors"] = "true" - data["nginx/cors-allow-headers"] = "DNT,X-CustomHeader, Keep-Alive,User-Agent" - data["nginx/cors-allow-credentials"] = "false" - data["nginx/cors-allow-methods"] = "PUT, GET,OPTIONS, PATCH, $nginx_version" - data["nginx/cors-allow-origin"] = "https://origin123.test.com:4443" + data[parser.GetAnnotationWithPrefix("enable-cors")] = "true" + data[parser.GetAnnotationWithPrefix("cors-allow-headers")] = "DNT,X-CustomHeader, Keep-Alive,User-Agent" + data[parser.GetAnnotationWithPrefix("cors-allow-credentials")] = "false" + data[parser.GetAnnotationWithPrefix("cors-allow-methods")] = "PUT, GET,OPTIONS, PATCH, $nginx_version" + data[parser.GetAnnotationWithPrefix("cors-allow-origin")] = "https://origin123.test.com:4443" ing.SetAnnotations(data) - corst, _ := NewParser(&resolver.Mock{}).Parse(ing) + corst, _ := NewParser().Parse(ing) nginxCors, ok := corst.(*Config) if !ok { t.Errorf("expected a Config type") diff --git a/internal/ingress/annotations/defaultbackend/main.go b/internal/ingress/annotations/defaultbackend/main.go index 1d4be720f..8b4112a3e 100644 --- a/internal/ingress/annotations/defaultbackend/main.go +++ b/internal/ingress/annotations/defaultbackend/main.go @@ -38,7 +38,7 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // Parse parses the annotations contained in the ingress to use // a custom default backend func (db backend) Parse(ing *extensions.Ingress) (interface{}, error) { - s, err := parser.GetStringAnnotation("default-backend", ing, db.r) + s, err := parser.GetStringAnnotation("default-backend", ing) if err != nil { return nil, err } diff --git a/internal/ingress/annotations/healthcheck/main.go b/internal/ingress/annotations/healthcheck/main.go index 44c5b3602..24c65e0a2 100644 --- a/internal/ingress/annotations/healthcheck/main.go +++ b/internal/ingress/annotations/healthcheck/main.go @@ -47,12 +47,12 @@ func (hc healthCheck) Parse(ing *extensions.Ingress) (interface{}, error) { return &Config{defBackend.UpstreamMaxFails, defBackend.UpstreamFailTimeout}, nil } - mf, err := parser.GetIntAnnotation("upstream-max-fails", ing, hc.r) + mf, err := parser.GetIntAnnotation("upstream-max-fails", ing) if err != nil { mf = defBackend.UpstreamMaxFails } - ft, err := parser.GetIntAnnotation("upstream-fail-timeout", ing, hc.r) + ft, err := parser.GetIntAnnotation("upstream-fail-timeout", ing) if err != nil { ft = defBackend.UpstreamFailTimeout } diff --git a/internal/ingress/annotations/healthcheck/main_test.go b/internal/ingress/annotations/healthcheck/main_test.go index 1654aa0c2..6a407f273 100644 --- a/internal/ingress/annotations/healthcheck/main_test.go +++ b/internal/ingress/annotations/healthcheck/main_test.go @@ -24,6 +24,7 @@ import ( meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -75,7 +76,7 @@ func TestIngressHealthCheck(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/upstream-max-fails"] = "2" + data[parser.GetAnnotationWithPrefix("upstream-max-fails")] = "2" ing.SetAnnotations(data) hzi, _ := NewParser(mockBackend{}).Parse(ing) diff --git a/internal/ingress/annotations/ipwhitelist/main.go b/internal/ingress/annotations/ipwhitelist/main.go index 2681f7136..0d76a2e00 100644 --- a/internal/ingress/annotations/ipwhitelist/main.go +++ b/internal/ingress/annotations/ipwhitelist/main.go @@ -81,7 +81,7 @@ func (a ipwhitelist) Parse(ing *extensions.Ingress) (interface{}, error) { defBackend := a.r.GetDefaultBackend() sort.Strings(defBackend.WhitelistSourceRange) - val, err := parser.GetStringAnnotation("whitelist-source-range", ing, a.r) + val, err := parser.GetStringAnnotation("whitelist-source-range", ing) // A missing annotation is not a problem, just use the default if err == ing_errors.ErrMissingAnnotations { return &SourceRange{CIDR: defBackend.WhitelistSourceRange}, nil diff --git a/internal/ingress/annotations/ipwhitelist/main_test.go b/internal/ingress/annotations/ipwhitelist/main_test.go index 2e7d54f5c..3ffc59959 100644 --- a/internal/ingress/annotations/ipwhitelist/main_test.go +++ b/internal/ingress/annotations/ipwhitelist/main_test.go @@ -23,6 +23,7 @@ import ( extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -94,7 +95,7 @@ func TestParseAnnotations(t *testing.T) { for testName, test := range tests { data := map[string]string{} - data["nginx/whitelist-source-range"] = test.net + data[parser.GetAnnotationWithPrefix("whitelist-source-range")] = test.net ing.SetAnnotations(data) p := NewParser(&resolver.Mock{}) i, err := p.Parse(ing) @@ -166,7 +167,7 @@ func TestParseAnnotationsWithDefaultConfig(t *testing.T) { for testName, test := range tests { data := map[string]string{} - data["nginx/whitelist-source-range"] = test.net + data[parser.GetAnnotationWithPrefix("whitelist-source-range")] = test.net ing.SetAnnotations(data) p := NewParser(mockBackend) i, err := p.Parse(ing) diff --git a/internal/ingress/annotations/parser/main.go b/internal/ingress/annotations/parser/main.go index d0e8dca71..f167e83b6 100644 --- a/internal/ingress/annotations/parser/main.go +++ b/internal/ingress/annotations/parser/main.go @@ -17,12 +17,17 @@ limitations under the License. package parser import ( + "fmt" "strconv" extensions "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-nginx/internal/ingress/errors" - "k8s.io/ingress-nginx/internal/ingress/resolver" +) + +var ( + // AnnotationsPrefix defines the common prefix used in the nginx ingress controller + AnnotationsPrefix = "nginx.ingress.kubernetes.io" ) // IngressAnnotation has a method to parse annotations located in Ingress @@ -76,8 +81,8 @@ func checkAnnotation(name string, ing *extensions.Ingress) error { } // GetBoolAnnotation extracts a boolean from an Ingress annotation -func GetBoolAnnotation(name string, ing *extensions.Ingress, r resolver.Resolver) (bool, error) { - v := r.GetAnnotationWithPrefix(name) +func GetBoolAnnotation(name string, ing *extensions.Ingress) (bool, error) { + v := GetAnnotationWithPrefix(name) err := checkAnnotation(v, ing) if err != nil { return false, err @@ -86,8 +91,8 @@ func GetBoolAnnotation(name string, ing *extensions.Ingress, r resolver.Resolver } // GetStringAnnotation extracts a string from an Ingress annotation -func GetStringAnnotation(name string, ing *extensions.Ingress, r resolver.Resolver) (string, error) { - v := r.GetAnnotationWithPrefix(name) +func GetStringAnnotation(name string, ing *extensions.Ingress) (string, error) { + v := GetAnnotationWithPrefix(name) err := checkAnnotation(v, ing) if err != nil { return "", err @@ -96,11 +101,16 @@ func GetStringAnnotation(name string, ing *extensions.Ingress, r resolver.Resolv } // GetIntAnnotation extracts an int from an Ingress annotation -func GetIntAnnotation(name string, ing *extensions.Ingress, r resolver.Resolver) (int, error) { - v := r.GetAnnotationWithPrefix(name) +func GetIntAnnotation(name string, ing *extensions.Ingress) (int, error) { + v := GetAnnotationWithPrefix(name) err := checkAnnotation(v, ing) if err != nil { return 0, err } return ingAnnotations(ing.GetAnnotations()).parseInt(v) } + +// GetAnnotationWithPrefix returns the prefix of ingress annotations +func GetAnnotationWithPrefix(suffix string) string { + return fmt.Sprintf("%v/%v", AnnotationsPrefix, suffix) +} diff --git a/internal/ingress/annotations/parser/main_test.go b/internal/ingress/annotations/parser/main_test.go index b04f0d722..f65fbdbad 100644 --- a/internal/ingress/annotations/parser/main_test.go +++ b/internal/ingress/annotations/parser/main_test.go @@ -17,13 +17,11 @@ limitations under the License. package parser import ( - "fmt" "testing" api "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/ingress-nginx/internal/ingress/resolver" ) func buildIngress() *extensions.Ingress { @@ -37,11 +35,9 @@ func buildIngress() *extensions.Ingress { } func TestGetBoolAnnotation(t *testing.T) { - r := &resolver.Mock{} - ing := buildIngress() - _, err := GetBoolAnnotation("", nil, r) + _, err := GetBoolAnnotation("", nil) if err == nil { t.Errorf("expected error but retuned nil") } @@ -61,9 +57,9 @@ func TestGetBoolAnnotation(t *testing.T) { ing.SetAnnotations(data) for _, test := range tests { - data[fmt.Sprintf("nginx/%v", test.field)] = test.value + data[GetAnnotationWithPrefix(test.field)] = test.value - u, err := GetBoolAnnotation(test.field, ing, r) + u, err := GetBoolAnnotation(test.field, ing) if test.expErr { if err == nil { t.Errorf("%v: expected error but retuned nil", test.name) @@ -79,11 +75,9 @@ func TestGetBoolAnnotation(t *testing.T) { } func TestGetStringAnnotation(t *testing.T) { - r := &resolver.Mock{} - ing := buildIngress() - _, err := GetStringAnnotation("", nil, r) + _, err := GetStringAnnotation("", nil) if err == nil { t.Errorf("expected error but retuned nil") } @@ -103,9 +97,9 @@ func TestGetStringAnnotation(t *testing.T) { ing.SetAnnotations(data) for _, test := range tests { - data[fmt.Sprintf("nginx/%v", test.field)] = test.value + data[GetAnnotationWithPrefix(test.field)] = test.value - s, err := GetStringAnnotation(test.field, ing, r) + s, err := GetStringAnnotation(test.field, ing) if test.expErr { if err == nil { t.Errorf("%v: expected error but retuned nil", test.name) @@ -121,11 +115,9 @@ func TestGetStringAnnotation(t *testing.T) { } func TestGetIntAnnotation(t *testing.T) { - r := &resolver.Mock{} - ing := buildIngress() - _, err := GetIntAnnotation("", nil, r) + _, err := GetIntAnnotation("", nil) if err == nil { t.Errorf("expected error but retuned nil") } @@ -145,9 +137,9 @@ func TestGetIntAnnotation(t *testing.T) { ing.SetAnnotations(data) for _, test := range tests { - data[fmt.Sprintf("nginx/%v", test.field)] = test.value + data[GetAnnotationWithPrefix(test.field)] = test.value - s, err := GetIntAnnotation(test.field, ing, r) + s, err := GetIntAnnotation(test.field, ing) if test.expErr { if err == nil { t.Errorf("%v: expected error but retuned nil", test.name) diff --git a/internal/ingress/annotations/portinredirect/main.go b/internal/ingress/annotations/portinredirect/main.go index 459878069..c091f1530 100644 --- a/internal/ingress/annotations/portinredirect/main.go +++ b/internal/ingress/annotations/portinredirect/main.go @@ -35,7 +35,7 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // Parse parses the annotations contained in the ingress // rule used to indicate if the redirects must func (a portInRedirect) Parse(ing *extensions.Ingress) (interface{}, error) { - up, err := parser.GetBoolAnnotation("use-port-in-redirects", ing, a.r) + up, err := parser.GetBoolAnnotation("use-port-in-redirects", ing) if err != nil { return a.r.GetDefaultBackend().UsePortInRedirects, nil } diff --git a/internal/ingress/annotations/portinredirect/main_test.go b/internal/ingress/annotations/portinredirect/main_test.go index 9bd0e4f31..c4e0e9179 100644 --- a/internal/ingress/annotations/portinredirect/main_test.go +++ b/internal/ingress/annotations/portinredirect/main_test.go @@ -25,6 +25,7 @@ import ( meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -92,7 +93,7 @@ func TestPortInRedirect(t *testing.T) { data := map[string]string{} if test.usePort != nil { - data["nginx/use-port-in-redirects"] = fmt.Sprintf("%v", *test.usePort) + data[parser.GetAnnotationWithPrefix("use-port-in-redirects")] = fmt.Sprintf("%v", *test.usePort) } ing.SetAnnotations(data) diff --git a/internal/ingress/annotations/proxy/main.go b/internal/ingress/annotations/proxy/main.go index 67f4581db..8171bdbbb 100644 --- a/internal/ingress/annotations/proxy/main.go +++ b/internal/ingress/annotations/proxy/main.go @@ -100,62 +100,62 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // rule used to configure upstream check parameters func (a proxy) Parse(ing *extensions.Ingress) (interface{}, error) { defBackend := a.r.GetDefaultBackend() - ct, err := parser.GetIntAnnotation("proxy-connect-timeout", ing, a.r) + ct, err := parser.GetIntAnnotation("proxy-connect-timeout", ing) if err != nil { ct = defBackend.ProxyConnectTimeout } - st, err := parser.GetIntAnnotation("proxy-send-timeout", ing, a.r) + st, err := parser.GetIntAnnotation("proxy-send-timeout", ing) if err != nil { st = defBackend.ProxySendTimeout } - rt, err := parser.GetIntAnnotation("proxy-read-timeout", ing, a.r) + rt, err := parser.GetIntAnnotation("proxy-read-timeout", ing) if err != nil { rt = defBackend.ProxyReadTimeout } - bufs, err := parser.GetStringAnnotation("proxy-buffer-size", ing, a.r) + bufs, err := parser.GetStringAnnotation("proxy-buffer-size", ing) if err != nil || bufs == "" { bufs = defBackend.ProxyBufferSize } - cp, err := parser.GetStringAnnotation("proxy-cookie-path", ing, a.r) + cp, err := parser.GetStringAnnotation("proxy-cookie-path", ing) if err != nil || cp == "" { cp = defBackend.ProxyCookiePath } - cd, err := parser.GetStringAnnotation("proxy-cookie-domain", ing, a.r) + cd, err := parser.GetStringAnnotation("proxy-cookie-domain", ing) if err != nil || cd == "" { cd = defBackend.ProxyCookieDomain } - bs, err := parser.GetStringAnnotation("proxy-body-size", ing, a.r) + bs, err := parser.GetStringAnnotation("proxy-body-size", ing) if err != nil || bs == "" { bs = defBackend.ProxyBodySize } - nu, err := parser.GetStringAnnotation("proxy-next-upstream", ing, a.r) + nu, err := parser.GetStringAnnotation("proxy-next-upstream", ing) if err != nil || nu == "" { nu = defBackend.ProxyNextUpstream } - pp, err := parser.GetStringAnnotation("proxy-pass-params", ing, a.r) + pp, err := parser.GetStringAnnotation("proxy-pass-params", ing) if err != nil || pp == "" { pp = defBackend.ProxyPassParams } - rb, err := parser.GetStringAnnotation("proxy-request-buffering", ing, a.r) + rb, err := parser.GetStringAnnotation("proxy-request-buffering", ing) if err != nil || rb == "" { rb = defBackend.ProxyRequestBuffering } - prf, err := parser.GetStringAnnotation("proxy-redirect-from", ing, a.r) + prf, err := parser.GetStringAnnotation("proxy-redirect-from", ing) if err != nil || rb == "" { prf = defBackend.ProxyRedirectFrom } - prt, err := parser.GetStringAnnotation("proxy-redirect-to", ing, a.r) + prt, err := parser.GetStringAnnotation("proxy-redirect-to", ing) if err != nil || rb == "" { prt = defBackend.ProxyRedirectTo } diff --git a/internal/ingress/annotations/proxy/main_test.go b/internal/ingress/annotations/proxy/main_test.go index c83dc9ef1..558d140f3 100644 --- a/internal/ingress/annotations/proxy/main_test.go +++ b/internal/ingress/annotations/proxy/main_test.go @@ -24,6 +24,7 @@ import ( meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -85,14 +86,14 @@ func TestProxy(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/proxy-connect-timeout"] = "1" - data["nginx/proxy-send-timeout"] = "2" - data["nginx/proxy-read-timeout"] = "3" - data["nginx/proxy-buffer-size"] = "1k" - data["nginx/proxy-body-size"] = "2k" - data["nginx/proxy-next-upstream"] = "off" - data["nginx/proxy-pass-params"] = "smax=5 max=10" - data["nginx/proxy-request-buffering"] = "off" + data[parser.GetAnnotationWithPrefix("proxy-connect-timeout")] = "1" + data[parser.GetAnnotationWithPrefix("proxy-send-timeout")] = "2" + data[parser.GetAnnotationWithPrefix("proxy-read-timeout")] = "3" + data[parser.GetAnnotationWithPrefix("proxy-buffer-size")] = "1k" + data[parser.GetAnnotationWithPrefix("proxy-body-size")] = "2k" + data[parser.GetAnnotationWithPrefix("proxy-next-upstream")] = "off" + data[parser.GetAnnotationWithPrefix("proxy-pass-params")] = "smax=5 max=10" + data[parser.GetAnnotationWithPrefix("proxy-request-buffering")] = "off" ing.SetAnnotations(data) i, err := NewParser(mockBackend{}).Parse(ing) diff --git a/internal/ingress/annotations/ratelimit/main.go b/internal/ingress/annotations/ratelimit/main.go index 624d4aefd..44e1a46c9 100644 --- a/internal/ingress/annotations/ratelimit/main.go +++ b/internal/ingress/annotations/ratelimit/main.go @@ -157,20 +157,20 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // rule used to rewrite the defined paths func (a ratelimit) Parse(ing *extensions.Ingress) (interface{}, error) { defBackend := a.r.GetDefaultBackend() - lr, err := parser.GetIntAnnotation("limit-rate", ing, a.r) + lr, err := parser.GetIntAnnotation("limit-rate", ing) if err != nil { lr = defBackend.LimitRate } - lra, err := parser.GetIntAnnotation("limit-rate-after", ing, a.r) + lra, err := parser.GetIntAnnotation("limit-rate-after", ing) if err != nil { lra = defBackend.LimitRateAfter } - rpm, _ := parser.GetIntAnnotation("limit-rpm", ing, a.r) - rps, _ := parser.GetIntAnnotation("limit-rps", ing, a.r) - conn, _ := parser.GetIntAnnotation("limit-connections", ing, a.r) + rpm, _ := parser.GetIntAnnotation("limit-rpm", ing) + rps, _ := parser.GetIntAnnotation("limit-rps", ing) + conn, _ := parser.GetIntAnnotation("limit-connections", ing) - val, _ := parser.GetStringAnnotation("limit-whitelist", ing, a.r) + val, _ := parser.GetStringAnnotation("limit-whitelist", ing) cidrs, err := parseCIDRs(val) if err != nil { diff --git a/internal/ingress/annotations/ratelimit/main_test.go b/internal/ingress/annotations/ratelimit/main_test.go index a470bbb47..06ced468b 100644 --- a/internal/ingress/annotations/ratelimit/main_test.go +++ b/internal/ingress/annotations/ratelimit/main_test.go @@ -24,6 +24,7 @@ import ( meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -86,9 +87,9 @@ func TestBadRateLimiting(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/limit-connections"] = "0" - data["nginx/limit-rps"] = "0" - data["nginx/limit-rpm"] = "0" + data[parser.GetAnnotationWithPrefix("limit-connections")] = "0" + data[parser.GetAnnotationWithPrefix("limit-rps")] = "0" + data[parser.GetAnnotationWithPrefix("limit-rpm")] = "0" ing.SetAnnotations(data) _, err := NewParser(mockBackend{}).Parse(ing) @@ -97,11 +98,11 @@ func TestBadRateLimiting(t *testing.T) { } data = map[string]string{} - data["nginx/limit-connections"] = "5" - data["nginx/limit-rps"] = "100" - data["nginx/limit-rpm"] = "10" - data["nginx/limit-rate-after"] = "100" - data["nginx/limit-rate"] = "10" + data[parser.GetAnnotationWithPrefix("limit-connections")] = "5" + data[parser.GetAnnotationWithPrefix("limit-rps")] = "100" + data[parser.GetAnnotationWithPrefix("limit-rpm")] = "10" + data[parser.GetAnnotationWithPrefix("limit-rate-after")] = "100" + data[parser.GetAnnotationWithPrefix("limit-rate")] = "10" ing.SetAnnotations(data) diff --git a/internal/ingress/annotations/redirect/redirect.go b/internal/ingress/annotations/redirect/redirect.go index 255763ef6..d94ede184 100644 --- a/internal/ingress/annotations/redirect/redirect.go +++ b/internal/ingress/annotations/redirect/redirect.go @@ -49,9 +49,9 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // If the Ingress contains both annotations the execution order is // temporal and then permanent func (a redirect) Parse(ing *extensions.Ingress) (interface{}, error) { - r3w, _ := parser.GetBoolAnnotation("from-to-www-redirect", ing, a.r) + r3w, _ := parser.GetBoolAnnotation("from-to-www-redirect", ing) - tr, err := parser.GetStringAnnotation("temporal-redirect", ing, a.r) + tr, err := parser.GetStringAnnotation("temporal-redirect", ing) if err != nil && !errors.IsMissingAnnotations(err) { return nil, err } @@ -68,7 +68,7 @@ func (a redirect) Parse(ing *extensions.Ingress) (interface{}, error) { }, nil } - pr, err := parser.GetStringAnnotation("permanent-redirect", ing, a.r) + pr, err := parser.GetStringAnnotation("permanent-redirect", ing) if err != nil && !errors.IsMissingAnnotations(err) { return nil, err } diff --git a/internal/ingress/annotations/rewrite/main.go b/internal/ingress/annotations/rewrite/main.go index 227cba446..2f222a509 100644 --- a/internal/ingress/annotations/rewrite/main.go +++ b/internal/ingress/annotations/rewrite/main.go @@ -82,18 +82,18 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // ParseAnnotations parses the annotations contained in the ingress // rule used to rewrite the defined paths func (a rewrite) Parse(ing *extensions.Ingress) (interface{}, error) { - rt, _ := parser.GetStringAnnotation("rewrite-target", ing, a.r) - sslRe, err := parser.GetBoolAnnotation("ssl-redirect", ing, a.r) + rt, _ := parser.GetStringAnnotation("rewrite-target", ing) + sslRe, err := parser.GetBoolAnnotation("ssl-redirect", ing) if err != nil { sslRe = a.r.GetDefaultBackend().SSLRedirect } - fSslRe, err := parser.GetBoolAnnotation("force-ssl-redirect", ing, a.r) + fSslRe, err := parser.GetBoolAnnotation("force-ssl-redirect", ing) if err != nil { fSslRe = a.r.GetDefaultBackend().ForceSSLRedirect } - abu, _ := parser.GetBoolAnnotation("add-base-url", ing, a.r) - bus, _ := parser.GetStringAnnotation("base-url-scheme", ing, a.r) - ar, _ := parser.GetStringAnnotation("app-root", ing, a.r) + abu, _ := parser.GetBoolAnnotation("add-base-url", ing) + bus, _ := parser.GetStringAnnotation("base-url-scheme", ing) + ar, _ := parser.GetStringAnnotation("app-root", ing) return &Config{ Target: rt, diff --git a/internal/ingress/annotations/rewrite/main_test.go b/internal/ingress/annotations/rewrite/main_test.go index 7b0f9c7c1..b54e3bfdc 100644 --- a/internal/ingress/annotations/rewrite/main_test.go +++ b/internal/ingress/annotations/rewrite/main_test.go @@ -24,6 +24,7 @@ import ( meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -88,7 +89,7 @@ func TestRedirect(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/rewrite-target"] = defRoute + data[parser.GetAnnotationWithPrefix("rewrite-target")] = defRoute ing.SetAnnotations(data) i, err := NewParser(mockBackend{}).Parse(ing) @@ -108,7 +109,7 @@ func TestSSLRedirect(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/rewrite-target"] = defRoute + data[parser.GetAnnotationWithPrefix("rewrite-target")] = defRoute ing.SetAnnotations(data) i, _ := NewParser(mockBackend{redirect: true}).Parse(ing) @@ -120,7 +121,7 @@ func TestSSLRedirect(t *testing.T) { t.Errorf("Expected true but returned false") } - data["nginx/ssl-redirect"] = "false" + data[parser.GetAnnotationWithPrefix("ssl-redirect")] = "false" ing.SetAnnotations(data) i, _ = NewParser(mockBackend{redirect: false}).Parse(ing) @@ -137,7 +138,7 @@ func TestForceSSLRedirect(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/rewrite-target"] = defRoute + data[parser.GetAnnotationWithPrefix("rewrite-target")] = defRoute ing.SetAnnotations(data) i, _ := NewParser(mockBackend{redirect: true}).Parse(ing) @@ -149,7 +150,7 @@ func TestForceSSLRedirect(t *testing.T) { t.Errorf("Expected false but returned true") } - data["nginx/force-ssl-redirect"] = "true" + data[parser.GetAnnotationWithPrefix("force-ssl-redirect")] = "true" ing.SetAnnotations(data) i, _ = NewParser(mockBackend{redirect: false}).Parse(ing) @@ -165,7 +166,7 @@ func TestAppRoot(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/app-root"] = "/app1" + data[parser.GetAnnotationWithPrefix("app-root")] = "/app1" ing.SetAnnotations(data) i, _ := NewParser(mockBackend{redirect: true}).Parse(ing) diff --git a/internal/ingress/annotations/secureupstream/main.go b/internal/ingress/annotations/secureupstream/main.go index c2d5082e7..e973dacbc 100644 --- a/internal/ingress/annotations/secureupstream/main.go +++ b/internal/ingress/annotations/secureupstream/main.go @@ -44,8 +44,8 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // Parse parses the annotations contained in the ingress // rule used to indicate if the upstream servers should use SSL func (a su) Parse(ing *extensions.Ingress) (interface{}, error) { - s, _ := parser.GetBoolAnnotation("secure-backends", ing, a.r) - ca, _ := parser.GetStringAnnotation("secure-verify-ca-secret", ing, a.r) + s, _ := parser.GetBoolAnnotation("secure-backends", ing) + ca, _ := parser.GetStringAnnotation("secure-verify-ca-secret", ing) secure := &Config{ Secure: s, CACert: resolver.AuthSSLCert{}, diff --git a/internal/ingress/annotations/secureupstream/main_test.go b/internal/ingress/annotations/secureupstream/main_test.go index 6563c8c6e..13a7c4e2c 100644 --- a/internal/ingress/annotations/secureupstream/main_test.go +++ b/internal/ingress/annotations/secureupstream/main_test.go @@ -25,6 +25,7 @@ import ( meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -78,8 +79,8 @@ func (cfg mockCfg) GetAuthCertificate(secret string) (*resolver.AuthSSLCert, err func TestAnnotations(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/secure-backends"] = "true" - data["nginx/secure-verify-ca-secret"] = "secure-verify-ca" + data[parser.GetAnnotationWithPrefix("secure-backends")] = "true" + data[parser.GetAnnotationWithPrefix("secure-verify-ca-secret")] = "secure-verify-ca" ing.SetAnnotations(data) _, err := NewParser(mockCfg{ @@ -95,8 +96,8 @@ func TestAnnotations(t *testing.T) { func TestSecretNotFound(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/secure-backends"] = "true" - data["nginx/secure-verify-ca-secret"] = "secure-verify-ca" + data[parser.GetAnnotationWithPrefix("secure-backends")] = "true" + data[parser.GetAnnotationWithPrefix("secure-verify-ca-secret")] = "secure-verify-ca" ing.SetAnnotations(data) _, err := NewParser(mockCfg{}).Parse(ing) if err == nil { @@ -107,8 +108,8 @@ func TestSecretNotFound(t *testing.T) { func TestSecretOnNonSecure(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/secure-backends"] = "false" - data["nginx/secure-verify-ca-secret"] = "secure-verify-ca" + data[parser.GetAnnotationWithPrefix("secure-backends")] = "false" + data[parser.GetAnnotationWithPrefix("secure-verify-ca-secret")] = "secure-verify-ca" ing.SetAnnotations(data) _, err := NewParser(mockCfg{ certs: map[string]resolver.AuthSSLCert{ diff --git a/internal/ingress/annotations/serversnippet/main.go b/internal/ingress/annotations/serversnippet/main.go index d6830d8fe..00fc3850c 100644 --- a/internal/ingress/annotations/serversnippet/main.go +++ b/internal/ingress/annotations/serversnippet/main.go @@ -20,21 +20,18 @@ import ( extensions "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" - "k8s.io/ingress-nginx/internal/ingress/resolver" ) -type serverSnippet struct { - r resolver.Resolver -} +type serverSnippet struct{} // NewParser creates a new server snippet annotation parser -func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return serverSnippet{r} +func NewParser() parser.IngressAnnotation { + return serverSnippet{} } // Parse parses the annotations contained in the ingress rule // used to indicate if the location/s contains a fragment of // configuration to be included inside the paths of the rules func (a serverSnippet) Parse(ing *extensions.Ingress) (interface{}, error) { - return parser.GetStringAnnotation("server-snippet", ing, a.r) + return parser.GetStringAnnotation("server-snippet", ing) } diff --git a/internal/ingress/annotations/serversnippet/main_test.go b/internal/ingress/annotations/serversnippet/main_test.go index 4d17e5e49..0d699e2cb 100644 --- a/internal/ingress/annotations/serversnippet/main_test.go +++ b/internal/ingress/annotations/serversnippet/main_test.go @@ -22,13 +22,13 @@ import ( api "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/ingress-nginx/internal/ingress/resolver" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ) func TestParse(t *testing.T) { - annotation := "nginx/server-snippet" + annotation := parser.GetAnnotationWithPrefix("server-snippet") - ap := NewParser(&resolver.Mock{}) + ap := NewParser() if ap == nil { t.Fatalf("expected a parser.IngressAnnotation but returned nil") } diff --git a/internal/ingress/annotations/serviceupstream/main.go b/internal/ingress/annotations/serviceupstream/main.go index a8386edb6..3408ed978 100644 --- a/internal/ingress/annotations/serviceupstream/main.go +++ b/internal/ingress/annotations/serviceupstream/main.go @@ -20,18 +20,15 @@ import ( extensions "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" - "k8s.io/ingress-nginx/internal/ingress/resolver" ) -type serviceUpstream struct { - r resolver.Resolver -} +type serviceUpstream struct{} // NewParser creates a new serviceUpstream annotation parser -func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return serviceUpstream{r} +func NewParser() parser.IngressAnnotation { + return serviceUpstream{} } func (s serviceUpstream) Parse(ing *extensions.Ingress) (interface{}, error) { - return parser.GetBoolAnnotation("service-upstream", ing, s.r) + return parser.GetBoolAnnotation("service-upstream", ing) } diff --git a/internal/ingress/annotations/serviceupstream/main_test.go b/internal/ingress/annotations/serviceupstream/main_test.go index 0b196ca0f..c651c1adb 100644 --- a/internal/ingress/annotations/serviceupstream/main_test.go +++ b/internal/ingress/annotations/serviceupstream/main_test.go @@ -23,7 +23,7 @@ import ( extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/ingress-nginx/internal/ingress/resolver" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ) func buildIngress() *extensions.Ingress { @@ -65,10 +65,10 @@ func TestIngressAnnotationServiceUpstreamEnabled(t *testing.T) { ing := buildIngress() data := map[string]string{} - data["nginx/service-upstream"] = "true" + data[parser.GetAnnotationWithPrefix("service-upstream")] = "true" ing.SetAnnotations(data) - val, _ := NewParser(&resolver.Mock{}).Parse(ing) + val, _ := NewParser().Parse(ing) enabled, ok := val.(bool) if !ok { t.Errorf("expected a bool type") @@ -84,10 +84,10 @@ func TestIngressAnnotationServiceUpstreamSetFalse(t *testing.T) { // Test with explicitly set to false data := map[string]string{} - data["nginx/service-upstream"] = "false" + data[parser.GetAnnotationWithPrefix("service-upstream")] = "false" ing.SetAnnotations(data) - val, _ := NewParser(&resolver.Mock{}).Parse(ing) + val, _ := NewParser().Parse(ing) enabled, ok := val.(bool) if !ok { t.Errorf("expected a bool type") @@ -101,7 +101,7 @@ func TestIngressAnnotationServiceUpstreamSetFalse(t *testing.T) { data = map[string]string{} ing.SetAnnotations(data) - val, _ = NewParser(&resolver.Mock{}).Parse(ing) + val, _ = NewParser().Parse(ing) enabled, ok = val.(bool) if !ok { t.Errorf("expected a bool type") diff --git a/internal/ingress/annotations/sessionaffinity/main.go b/internal/ingress/annotations/sessionaffinity/main.go index fd4bceeb4..49de25d33 100644 --- a/internal/ingress/annotations/sessionaffinity/main.go +++ b/internal/ingress/annotations/sessionaffinity/main.go @@ -63,14 +63,14 @@ type Cookie struct { // cookieAffinityParse gets the annotation values related to Cookie Affinity // It also sets default values when no value or incorrect value is found func (a affinity) cookieAffinityParse(ing *extensions.Ingress) *Cookie { - sn, err := parser.GetStringAnnotation(annotationAffinityCookieName, ing, a.r) + sn, err := parser.GetStringAnnotation(annotationAffinityCookieName, ing) if err != nil || sn == "" { glog.V(3).Infof("Ingress %v: No value found in annotation %v. Using the default %v", ing.Name, annotationAffinityCookieName, defaultAffinityCookieName) sn = defaultAffinityCookieName } - sh, err := parser.GetStringAnnotation(annotationAffinityCookieHash, ing, a.r) + sh, err := parser.GetStringAnnotation(annotationAffinityCookieHash, ing) if err != nil || !affinityCookieHashRegex.MatchString(sh) { glog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Setting it to default %v", ing.Name, annotationAffinityCookieHash, defaultAffinityCookieHash) @@ -97,7 +97,7 @@ type affinity struct { func (a affinity) Parse(ing *extensions.Ingress) (interface{}, error) { cookie := &Cookie{} // Check the type of affinity that will be used - at, err := parser.GetStringAnnotation(annotationAffinityType, ing, a.r) + at, err := parser.GetStringAnnotation(annotationAffinityType, ing) if err != nil { at = "" } diff --git a/internal/ingress/annotations/sessionaffinity/main_test.go b/internal/ingress/annotations/sessionaffinity/main_test.go index 464435117..3db53a2f7 100644 --- a/internal/ingress/annotations/sessionaffinity/main_test.go +++ b/internal/ingress/annotations/sessionaffinity/main_test.go @@ -17,13 +17,13 @@ limitations under the License. package sessionaffinity import ( - "fmt" "testing" api "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -66,9 +66,9 @@ func TestIngressAffinityCookieConfig(t *testing.T) { ing := buildIngress() data := map[string]string{} - data[fmt.Sprintf("nginx/%v", annotationAffinityType)] = "cookie" - data[fmt.Sprintf("nginx/%v", annotationAffinityCookieHash)] = "sha123" - data[fmt.Sprintf("nginx/%v", annotationAffinityCookieName)] = "INGRESSCOOKIE" + data[parser.GetAnnotationWithPrefix(annotationAffinityType)] = "cookie" + data[parser.GetAnnotationWithPrefix(annotationAffinityCookieHash)] = "sha123" + data[parser.GetAnnotationWithPrefix(annotationAffinityCookieName)] = "INGRESSCOOKIE" ing.SetAnnotations(data) affin, _ := NewParser(&resolver.Mock{}).Parse(ing) diff --git a/internal/ingress/annotations/snippet/main.go b/internal/ingress/annotations/snippet/main.go index b93dbf63b..41352b073 100644 --- a/internal/ingress/annotations/snippet/main.go +++ b/internal/ingress/annotations/snippet/main.go @@ -20,21 +20,18 @@ import ( extensions "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" - "k8s.io/ingress-nginx/internal/ingress/resolver" ) -type snippet struct { - r resolver.Resolver -} +type snippet struct{} -// NewParser creates a new CORS annotation parser -func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return snippet{r} +// NewParser creates a new snippet annotation parser +func NewParser() parser.IngressAnnotation { + return snippet{} } // Parse parses the annotations contained in the ingress rule // used to indicate if the location/s contains a fragment of // configuration to be included inside the paths of the rules func (a snippet) Parse(ing *extensions.Ingress) (interface{}, error) { - return parser.GetStringAnnotation("configuration-snippet", ing, a.r) + return parser.GetStringAnnotation("configuration-snippet", ing) } diff --git a/internal/ingress/annotations/snippet/main_test.go b/internal/ingress/annotations/snippet/main_test.go index 30943379f..17ffe80a3 100644 --- a/internal/ingress/annotations/snippet/main_test.go +++ b/internal/ingress/annotations/snippet/main_test.go @@ -22,13 +22,13 @@ import ( api "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/ingress-nginx/internal/ingress/resolver" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ) func TestParse(t *testing.T) { - annotation := "nginx/configuration-snippet" + annotation := parser.GetAnnotationWithPrefix("configuration-snippet") - ap := NewParser(&resolver.Mock{}) + ap := NewParser() if ap == nil { t.Fatalf("expected a parser.IngressAnnotation but returned nil") } diff --git a/internal/ingress/annotations/sslpassthrough/main.go b/internal/ingress/annotations/sslpassthrough/main.go index 82b69a170..ad75b7bdd 100644 --- a/internal/ingress/annotations/sslpassthrough/main.go +++ b/internal/ingress/annotations/sslpassthrough/main.go @@ -21,16 +21,13 @@ import ( "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ing_errors "k8s.io/ingress-nginx/internal/ingress/errors" - "k8s.io/ingress-nginx/internal/ingress/resolver" ) -type sslpt struct { - r resolver.Resolver -} +type sslpt struct{} // NewParser creates a new SSL passthrough annotation parser -func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return sslpt{r} +func NewParser() parser.IngressAnnotation { + return sslpt{} } // ParseAnnotations parses the annotations contained in the ingress @@ -40,5 +37,5 @@ func (a sslpt) Parse(ing *extensions.Ingress) (interface{}, error) { return false, ing_errors.ErrMissingAnnotations } - return parser.GetBoolAnnotation("ssl-passthrough", ing, a.r) + return parser.GetBoolAnnotation("ssl-passthrough", ing) } diff --git a/internal/ingress/annotations/sslpassthrough/main_test.go b/internal/ingress/annotations/sslpassthrough/main_test.go index 0320c007e..b53e06bad 100644 --- a/internal/ingress/annotations/sslpassthrough/main_test.go +++ b/internal/ingress/annotations/sslpassthrough/main_test.go @@ -22,7 +22,7 @@ import ( api "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/ingress-nginx/internal/ingress/resolver" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -45,16 +45,16 @@ func buildIngress() *extensions.Ingress { func TestParseAnnotations(t *testing.T) { ing := buildIngress() - _, err := NewParser(&resolver.Mock{}).Parse(ing) + _, err := NewParser().Parse(ing) if err == nil { t.Errorf("unexpected error: %v", err) } data := map[string]string{} - data["nginx/ssl-passthrough"] = "true" + data[parser.GetAnnotationWithPrefix("ssl-passthrough")] = "true" ing.SetAnnotations(data) // test ingress using the annotation without a TLS section - _, err = NewParser(&resolver.Mock{}).Parse(ing) + _, err = NewParser().Parse(ing) if err != nil { t.Errorf("unexpected error parsing ingress with sslpassthrough") } @@ -65,7 +65,7 @@ func TestParseAnnotations(t *testing.T) { Hosts: []string{"foo.bar.com"}, }, } - i, err := NewParser(&resolver.Mock{}).Parse(ing) + i, err := NewParser().Parse(ing) if err != nil { t.Errorf("expected error parsing ingress with sslpassthrough") } diff --git a/internal/ingress/annotations/upstreamhashby/main.go b/internal/ingress/annotations/upstreamhashby/main.go index b543070a0..ff102232c 100644 --- a/internal/ingress/annotations/upstreamhashby/main.go +++ b/internal/ingress/annotations/upstreamhashby/main.go @@ -20,21 +20,18 @@ import ( extensions "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" - "k8s.io/ingress-nginx/internal/ingress/resolver" ) -type upstreamhashby struct { - r resolver.Resolver -} +type upstreamhashby struct{} // NewParser creates a new CORS annotation parser -func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return upstreamhashby{r} +func NewParser() parser.IngressAnnotation { + return upstreamhashby{} } // Parse parses the annotations contained in the ingress rule // used to indicate if the location/s contains a fragment of // configuration to be included inside the paths of the rules func (a upstreamhashby) Parse(ing *extensions.Ingress) (interface{}, error) { - return parser.GetStringAnnotation("upstream-hash-by", ing, a.r) + return parser.GetStringAnnotation("upstream-hash-by", ing) } diff --git a/internal/ingress/annotations/upstreamhashby/main_test.go b/internal/ingress/annotations/upstreamhashby/main_test.go index 5507a8c7f..a3a34bb3f 100644 --- a/internal/ingress/annotations/upstreamhashby/main_test.go +++ b/internal/ingress/annotations/upstreamhashby/main_test.go @@ -22,13 +22,13 @@ import ( api "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/ingress-nginx/internal/ingress/resolver" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ) func TestParse(t *testing.T) { - annotation := "nginx/upstream-hash-by" + annotation := parser.GetAnnotationWithPrefix("upstream-hash-by") - ap := NewParser(&resolver.Mock{}) + ap := NewParser() if ap == nil { t.Fatalf("expected a parser.IngressAnnotation but returned nil") } diff --git a/internal/ingress/annotations/upstreamvhost/main.go b/internal/ingress/annotations/upstreamvhost/main.go index 02c1e96cf..8ecc472f2 100644 --- a/internal/ingress/annotations/upstreamvhost/main.go +++ b/internal/ingress/annotations/upstreamvhost/main.go @@ -20,21 +20,18 @@ import ( extensions "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" - "k8s.io/ingress-nginx/internal/ingress/resolver" ) -type upstreamVhost struct { - r resolver.Resolver -} +type upstreamVhost struct{} // NewParser creates a new upstream VHost annotation parser -func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return upstreamVhost{r} +func NewParser() parser.IngressAnnotation { + return upstreamVhost{} } // Parse parses the annotations contained in the ingress rule // used to indicate if the location/s contains a fragment of // configuration to be included inside the paths of the rules func (a upstreamVhost) Parse(ing *extensions.Ingress) (interface{}, error) { - return parser.GetStringAnnotation("upstream-vhost", ing, a.r) + return parser.GetStringAnnotation("upstream-vhost", ing) } diff --git a/internal/ingress/annotations/vtsfilterkey/main.go b/internal/ingress/annotations/vtsfilterkey/main.go index 41288e291..8979cf3ce 100644 --- a/internal/ingress/annotations/vtsfilterkey/main.go +++ b/internal/ingress/annotations/vtsfilterkey/main.go @@ -20,21 +20,18 @@ import ( extensions "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" - "k8s.io/ingress-nginx/internal/ingress/resolver" ) -type vtsFilterKey struct { - r resolver.Resolver -} +type vtsFilterKey struct{} // NewParser creates a new vts filter key annotation parser -func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return vtsFilterKey{r} +func NewParser() parser.IngressAnnotation { + return vtsFilterKey{} } -// Parse parses the annotations contained in the ingress rule -// used to indicate if the location/s contains a fragment of +// (Parse parses the annotations contained in the ingress rule +// used toindicate if the location/s contains a fragment of // configuration to be included inside the paths of the rules func (a vtsFilterKey) Parse(ing *extensions.Ingress) (interface{}, error) { - return parser.GetStringAnnotation("vts-filter-key", ing, a.r) + return parser.GetStringAnnotation("vts-filter-key", ing) } diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 9da69ad59..d9cd90b7a 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -37,16 +37,10 @@ import ( clientset "k8s.io/client-go/kubernetes" "k8s.io/ingress-nginx/internal/ingress" - "k8s.io/ingress-nginx/internal/ingress/annotations" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" "k8s.io/ingress-nginx/internal/ingress/annotations/healthcheck" - "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/annotations/proxy" ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" - "k8s.io/ingress-nginx/internal/ingress/defaults" - "k8s.io/ingress-nginx/internal/ingress/resolver" "k8s.io/ingress-nginx/internal/k8s" - "k8s.io/ingress-nginx/internal/task" ) const ( @@ -66,8 +60,6 @@ func init() { // Configuration contains all the settings required by an Ingress controller type Configuration struct { - AnnotationsPrefix string - APIServerHost string KubeConfigFile string Client clientset.Interface @@ -76,7 +68,6 @@ type Configuration struct { ConfigMapName string DefaultService string - IngressClass string Namespace string ForceNamespaceIsolation bool @@ -87,7 +78,6 @@ type Configuration struct { UDPConfigMapName string DefaultHealthzURL string - DefaultIngressClass string DefaultSSLCertificate string // optional @@ -112,14 +102,9 @@ type Configuration struct { FakeCertificateSHA string } -// GetDefaultBackend returns the default backend -func (n NGINXController) GetDefaultBackend() defaults.Backend { - return n.backendDefaults -} - // GetPublishService returns the configured service used to set ingress status func (n NGINXController) GetPublishService() *apiv1.Service { - s, err := n.listers.Service.GetByName(n.cfg.PublishService) + s, err := n.storeLister.GetService(n.cfg.PublishService) if err != nil { return nil } @@ -127,21 +112,6 @@ func (n NGINXController) GetPublishService() *apiv1.Service { return s } -// GetSecret searches for a secret in the local secrets Store -func (n NGINXController) GetSecret(name string) (*apiv1.Secret, error) { - return n.listers.Secret.GetByName(name) -} - -// GetService searches for a service in the local secrets Store -func (n NGINXController) GetService(name string) (*apiv1.Service, error) { - return n.listers.Service.GetByName(name) -} - -// GetAnnotationWithPrefix returns the prefix of ingress annotations -func (n NGINXController) GetAnnotationWithPrefix(suffix string) string { - return fmt.Sprintf("%v/%v", n.cfg.AnnotationsPrefix, suffix) -} - // sync collects all the pieces required to assemble the configuration file and // then sends the content to the backend (OnUpdate) receiving the populated // template as response reloading the backend if is required. @@ -152,34 +122,14 @@ func (n *NGINXController) syncIngress(item interface{}) error { return nil } - if element, ok := item.(task.Element); ok { - if name, ok := element.Key.(string); ok { - if obj, exists, _ := n.listers.Ingress.GetByKey(name); exists { - ing := obj.(*extensions.Ingress) - n.readSecrets(ing) - } - } - } - // Sort ingress rules using the ResourceVersion field - ings := n.listers.Ingress.List() - sort.SliceStable(ings, func(i, j int) bool { - ir := ings[i].(*extensions.Ingress).ResourceVersion - jr := ings[j].(*extensions.Ingress).ResourceVersion + ingresses := n.storeLister.ListIngresses() + sort.SliceStable(ingresses, func(i, j int) bool { + ir := ingresses[i].ResourceVersion + jr := ingresses[j].ResourceVersion return ir < jr }) - // filter ingress rules - var ingresses []*extensions.Ingress - for _, ingIf := range ings { - ing := ingIf.(*extensions.Ingress) - if !class.IsValid(ing, n.cfg.IngressClass, n.cfg.DefaultIngressClass) { - continue - } - - ingresses = append(ingresses, ing) - } - upstreams, servers := n.getBackendServers(ingresses) var passUpstreams []*ingress.SSLPassthroughBackend @@ -248,7 +198,7 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr return []ingress.L4Service{} } - configmap, err := n.listers.ConfigMap.GetByName(configmapName) + configmap, err := n.storeLister.GetConfigMap(configmapName) if err != nil { glog.Errorf("unexpected error reading configmap %v: %v", configmapName, err) return []ingress.L4Service{} @@ -306,19 +256,12 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr continue } - svcObj, svcExists, err := n.listers.Service.GetByKey(nsName) + svc, err := n.storeLister.GetService(nsName) if err != nil { glog.Warningf("error getting service %v: %v", nsName, err) continue } - if !svcExists { - glog.Warningf("service %v was not found", nsName) - continue - } - - svc := svcObj.(*apiv1.Service) - var endps []ingress.Endpoint targetPort, err := strconv.Atoi(svcPort) if err != nil { @@ -375,20 +318,13 @@ func (n *NGINXController) getDefaultUpstream() *ingress.Backend { Name: defUpstreamName, } svcKey := n.cfg.DefaultService - svcObj, svcExists, err := n.listers.Service.GetByKey(svcKey) + svc, err := n.storeLister.GetService(svcKey) if err != nil { glog.Warningf("unexpected error searching the default backend %v: %v", n.cfg.DefaultService, err) upstream.Endpoints = append(upstream.Endpoints, n.DefaultEndpoint()) return upstream } - if !svcExists { - glog.Warningf("service %v does not exist", svcKey) - upstream.Endpoints = append(upstream.Endpoints, n.DefaultEndpoint()) - return upstream - } - - svc := svcObj.(*apiv1.Service) endps := n.getEndpoints(svc, &svc.Spec.Ports[0], apiv1.ProtocolTCP, &healthcheck.Config{}) if len(endps) == 0 { glog.Warningf("service %v does not have any active endpoints", svcKey) @@ -408,7 +344,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] servers := n.createServers(ingresses, upstreams, du) for _, ing := range ingresses { - anns := n.getIngressAnnotations(ing) + anns, _ := n.storeLister.GetIngressAnnotations(ing) for _, rule := range ing.Spec.Rules { host := rule.Host @@ -620,29 +556,6 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] return aUpstreams, aServers } -// GetAuthCertificate is used by the auth-tls annotations to get a cert from a secret -func (n NGINXController) GetAuthCertificate(name string) (*resolver.AuthSSLCert, error) { - if _, exists := n.sslCertTracker.Get(name); !exists { - n.syncSecret(name) - } - - _, err := n.listers.Secret.GetByName(name) - if err != nil { - return &resolver.AuthSSLCert{}, fmt.Errorf("unexpected error: %v", err) - } - - bc, exists := n.sslCertTracker.Get(name) - if !exists { - return &resolver.AuthSSLCert{}, fmt.Errorf("secret %v does not exist", name) - } - cert := bc.(*ingress.SSLCert) - return &resolver.AuthSSLCert{ - Secret: name, - CAFileName: cert.CAFileName, - PemSHA: cert.PemSHA, - }, nil -} - // createUpstreams creates the NGINX upstreams for each service referenced in // Ingress rules. The servers inside the upstream are endpoints. func (n *NGINXController) createUpstreams(data []*extensions.Ingress, du *ingress.Backend) map[string]*ingress.Backend { @@ -650,7 +563,7 @@ func (n *NGINXController) createUpstreams(data []*extensions.Ingress, du *ingres upstreams[defUpstreamName] = du for _, ing := range data { - anns := n.getIngressAnnotations(ing) + anns, _ := n.storeLister.GetIngressAnnotations(ing) var defBackend string if ing.Spec.Backend != nil { @@ -737,13 +650,13 @@ func (n *NGINXController) createUpstreams(data []*extensions.Ingress, du *ingres upstreams[name].Endpoints = endp } - s, err := n.listers.Service.GetByName(svcKey) + svc, err := n.storeLister.GetService(svcKey) if err != nil { glog.Warningf("error obtaining service: %v", err) continue } - upstreams[name].Service = s + upstreams[name].Service = svc } } } @@ -752,13 +665,11 @@ func (n *NGINXController) createUpstreams(data []*extensions.Ingress, du *ingres } func (n *NGINXController) getServiceClusterEndpoint(svcKey string, backend *extensions.IngressBackend) (endpoint ingress.Endpoint, err error) { - svcObj, svcExists, err := n.listers.Service.GetByKey(svcKey) - - if !svcExists { - return endpoint, fmt.Errorf("service %v does not exist", svcKey) + svc, err := n.storeLister.GetService(svcKey) + if err != nil { + return endpoint, err } - svc := svcObj.(*apiv1.Service) if svc.Spec.ClusterIP == "" || svc.Spec.ClusterIP == "None" { return endpoint, fmt.Errorf("No ClusterIP found for service %s", svcKey) } @@ -790,7 +701,7 @@ func (n *NGINXController) getServiceClusterEndpoint(svcKey string, backend *exte // to a service. func (n *NGINXController) serviceEndpoints(svcKey, backendPort string, hz *healthcheck.Config) ([]ingress.Endpoint, error) { - svc, err := n.listers.Service.GetByName(svcKey) + svc, err := n.storeLister.GetService(svcKey) var upstreams []ingress.Endpoint if err != nil { @@ -872,7 +783,7 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, // remove the alias to avoid conflicts. aliases := make(map[string]string, len(data)) - bdef := n.GetDefaultBackend() + bdef := n.storeLister.GetDefaultBackend() ngxProxy := proxy.Config{ BodySize: bdef.ProxyBodySize, ConnectTimeout: bdef.ProxyConnectTimeout, @@ -886,16 +797,18 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, ProxyRedirectFrom: bdef.ProxyRedirectFrom, } - // generated on Start() with createDefaultSSLCertificate() + // self generated certificate on start. defaultPemFileName := n.cfg.FakeCertificatePath defaultPemSHA := n.cfg.FakeCertificateSHA // Tries to fetch the default Certificate from nginx configuration. // If it does not exists, use the ones generated on Start() - defaultCertificate, err := n.getPemCertificate(n.cfg.DefaultSSLCertificate) - if err == nil { - defaultPemFileName = defaultCertificate.PemFileName - defaultPemSHA = defaultCertificate.PemSHA + if n.cfg.DefaultSSLCertificate != "" { + defaultCertificate, err := n.storeLister.GetLocalSecret(n.cfg.DefaultSSLCertificate) + if err == nil { + defaultPemFileName = defaultCertificate.PemFileName + defaultPemSHA = defaultCertificate.PemSHA + } } // initialize the default server @@ -915,7 +828,7 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, // initialize all the servers for _, ing := range data { - anns := n.getIngressAnnotations(ing) + anns, _ := n.storeLister.GetIngressAnnotations(ing) // default upstream server un := du.Name @@ -966,7 +879,7 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, // configure default location, alias, and SSL for _, ing := range data { - anns := n.getIngressAnnotations(ing) + anns, _ := n.storeLister.GetIngressAnnotations(ing) for _, rule := range ing.Spec.Rules { host := rule.Host @@ -1031,13 +944,12 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, } key := fmt.Sprintf("%v/%v", ing.Namespace, tlsSecretName) - bc, exists := n.sslCertTracker.Get(key) - if !exists { - glog.Warningf("ssl certificate \"%v\" does not exist in local store", key) + cert, err := n.storeLister.GetLocalSecret(key) + if err != nil { + glog.Warning(err) continue } - cert := bc.(*ingress.SSLCert) err = cert.Certificate.VerifyHostname(host) if err != nil { glog.Warningf("ssl certificate %v does not contain a Common Name or Subject Alternative Name for host %v", key, host) @@ -1107,7 +1019,7 @@ func (n *NGINXController) getEndpoints( } glog.V(3).Infof("getting endpoints for service %v/%v and port %v", s.Namespace, s.Name, servicePort.String()) - ep, err := n.listers.Endpoint.GetServiceEndpoints(s) + ep, err := n.storeLister.GetServiceEndpoints(s) if err != nil { glog.Warningf("unexpected error obtaining service endpoints: %v", err) return upsServers @@ -1156,24 +1068,6 @@ func (n *NGINXController) getEndpoints( return upsServers } -// readSecrets extracts information about secrets from an Ingress rule -func (n *NGINXController) readSecrets(ing *extensions.Ingress) { - for _, tls := range ing.Spec.TLS { - if tls.SecretName == "" { - continue - } - - key := fmt.Sprintf("%v/%v", ing.Namespace, tls.SecretName) - n.syncSecret(key) - } - - key, _ := parser.GetStringAnnotation("auth-tls-secret", ing, n) - if key == "" { - return - } - n.syncSecret(key) -} - func (n *NGINXController) isForceReload() bool { return atomic.LoadInt32(&n.forceReload) != 0 } @@ -1183,28 +1077,8 @@ func (n *NGINXController) SetForceReload(shouldReload bool) { if shouldReload { atomic.StoreInt32(&n.forceReload, 1) n.syncQueue.Enqueue(&extensions.Ingress{}) - } else { - atomic.StoreInt32(&n.forceReload, 0) + return } -} -func (n *NGINXController) extractAnnotations(ing *extensions.Ingress) { - anns := n.annotations.Extract(ing) - glog.V(3).Infof("updating annotations information for ingres %v/%v", anns.Namespace, anns.Name) - n.listers.IngressAnnotation.Update(anns) -} - -// getByIngress returns the parsed annotations from an Ingress -func (n *NGINXController) getIngressAnnotations(ing *extensions.Ingress) *annotations.Ingress { - key := fmt.Sprintf("%v/%v", ing.Namespace, ing.Name) - item, exists, err := n.listers.IngressAnnotation.GetByKey(key) - if err != nil { - glog.Errorf("unexpected error getting ingress annotation %v: %v", key, err) - return &annotations.Ingress{} - } - if !exists { - glog.Errorf("ingress annotation %v was not found", key) - return &annotations.Ingress{} - } - return item.(*annotations.Ingress) + atomic.StoreInt32(&n.forceReload, 0) } diff --git a/internal/ingress/controller/listers.go b/internal/ingress/controller/listers.go deleted file mode 100644 index adfa71982..000000000 --- a/internal/ingress/controller/listers.go +++ /dev/null @@ -1,228 +0,0 @@ -/* -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 ( - "fmt" - "reflect" - - "github.com/golang/glog" - - apiv1 "k8s.io/api/core/v1" - extensions "k8s.io/api/extensions/v1beta1" - "k8s.io/apimachinery/pkg/fields" - "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/client-go/tools/cache" - cache_client "k8s.io/client-go/tools/cache" - - "k8s.io/ingress-nginx/internal/ingress" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" - "k8s.io/ingress-nginx/internal/ingress/annotations/parser" -) - -type cacheController struct { - Ingress cache.Controller - Endpoint cache.Controller - Service cache.Controller - Secret cache.Controller - Configmap cache.Controller -} - -func (c *cacheController) Run(stopCh chan struct{}) { - go c.Ingress.Run(stopCh) - go c.Endpoint.Run(stopCh) - go c.Service.Run(stopCh) - go c.Secret.Run(stopCh) - go c.Configmap.Run(stopCh) - - // Wait for all involved caches to be synced, before processing items from the queue is started - if !cache.WaitForCacheSync(stopCh, - c.Ingress.HasSynced, - c.Endpoint.HasSynced, - c.Service.HasSynced, - c.Secret.HasSynced, - c.Configmap.HasSynced, - ) { - runtime.HandleError(fmt.Errorf("Timed out waiting for caches to sync")) - } -} - -func (n *NGINXController) createListers(stopCh chan struct{}) (*ingress.StoreLister, *cacheController) { - ingEventHandler := cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { - addIng := obj.(*extensions.Ingress) - if !class.IsValid(addIng, n.cfg.IngressClass, defIngressClass) { - a, _ := parser.GetStringAnnotation(class.IngressKey, addIng, n) - glog.Infof("ignoring add for ingress %v based on annotation %v with value %v", addIng.Name, class.IngressKey, a) - return - } - - n.extractAnnotations(addIng) - n.recorder.Eventf(addIng, apiv1.EventTypeNormal, "CREATE", fmt.Sprintf("Ingress %s/%s", addIng.Namespace, addIng.Name)) - n.syncQueue.Enqueue(obj) - }, - DeleteFunc: func(obj interface{}) { - delIng, ok := obj.(*extensions.Ingress) - if !ok { - // If we reached here it means the ingress was deleted but its final state is unrecorded. - tombstone, ok := obj.(cache.DeletedFinalStateUnknown) - if !ok { - glog.Errorf("couldn't get object from tombstone %#v", obj) - return - } - delIng, ok = tombstone.Obj.(*extensions.Ingress) - if !ok { - glog.Errorf("Tombstone contained object that is not an Ingress: %#v", obj) - return - } - } - if !class.IsValid(delIng, n.cfg.IngressClass, defIngressClass) { - glog.Infof("ignoring delete for ingress %v based on annotation %v", delIng.Name, class.IngressKey) - return - } - n.recorder.Eventf(delIng, apiv1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", delIng.Namespace, delIng.Name)) - n.listers.IngressAnnotation.Delete(delIng) - n.syncQueue.Enqueue(obj) - }, - UpdateFunc: func(old, cur interface{}) { - oldIng := old.(*extensions.Ingress) - curIng := cur.(*extensions.Ingress) - validOld := class.IsValid(oldIng, n.cfg.IngressClass, defIngressClass) - validCur := class.IsValid(curIng, n.cfg.IngressClass, defIngressClass) - if !validOld && validCur { - glog.Infof("creating ingress %v based on annotation %v", curIng.Name, class.IngressKey) - n.recorder.Eventf(curIng, apiv1.EventTypeNormal, "CREATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) - } else if validOld && !validCur { - glog.Infof("removing ingress %v based on annotation %v", curIng.Name, class.IngressKey) - n.recorder.Eventf(curIng, apiv1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) - } else if validCur && !reflect.DeepEqual(old, cur) { - n.recorder.Eventf(curIng, apiv1.EventTypeNormal, "UPDATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) - } - - n.extractAnnotations(curIng) - n.syncQueue.Enqueue(cur) - }, - } - - secrEventHandler := cache.ResourceEventHandlerFuncs{ - UpdateFunc: func(old, cur interface{}) { - if !reflect.DeepEqual(old, cur) { - sec := cur.(*apiv1.Secret) - key := fmt.Sprintf("%v/%v", sec.Namespace, sec.Name) - _, exists := n.sslCertTracker.Get(key) - if exists { - n.syncSecret(key) - } - } - }, - DeleteFunc: func(obj interface{}) { - sec, ok := obj.(*apiv1.Secret) - if !ok { - // If we reached here it means the secret was deleted but its final state is unrecorded. - tombstone, ok := obj.(cache.DeletedFinalStateUnknown) - if !ok { - glog.Errorf("couldn't get object from tombstone %#v", obj) - return - } - sec, ok = tombstone.Obj.(*apiv1.Secret) - if !ok { - glog.Errorf("Tombstone contained object that is not a Secret: %#v", obj) - return - } - } - key := fmt.Sprintf("%v/%v", sec.Namespace, sec.Name) - n.sslCertTracker.Delete(key) - n.syncQueue.Enqueue(key) - }, - } - - eventHandler := cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { - n.syncQueue.Enqueue(obj) - }, - DeleteFunc: func(obj interface{}) { - n.syncQueue.Enqueue(obj) - }, - UpdateFunc: func(old, cur interface{}) { - oep := old.(*apiv1.Endpoints) - ocur := cur.(*apiv1.Endpoints) - if !reflect.DeepEqual(ocur.Subsets, oep.Subsets) { - n.syncQueue.Enqueue(cur) - } - }, - } - - mapEventHandler := cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { - upCmap := obj.(*apiv1.ConfigMap) - mapKey := fmt.Sprintf("%s/%s", upCmap.Namespace, upCmap.Name) - if mapKey == n.cfg.ConfigMapName { - glog.V(2).Infof("adding configmap %v to backend", mapKey) - n.SetConfig(upCmap) - n.SetForceReload(true) - } - }, - UpdateFunc: func(old, cur interface{}) { - if !reflect.DeepEqual(old, cur) { - upCmap := cur.(*apiv1.ConfigMap) - mapKey := fmt.Sprintf("%s/%s", upCmap.Namespace, upCmap.Name) - if mapKey == n.cfg.ConfigMapName { - glog.V(2).Infof("updating configmap backend (%v)", mapKey) - n.SetConfig(upCmap) - n.SetForceReload(true) - } - // updates to configuration configmaps can trigger an update - if mapKey == n.cfg.ConfigMapName || mapKey == n.cfg.TCPConfigMapName || mapKey == n.cfg.UDPConfigMapName { - n.recorder.Eventf(upCmap, apiv1.EventTypeNormal, "UPDATE", fmt.Sprintf("ConfigMap %v", mapKey)) - n.syncQueue.Enqueue(cur) - } - } - }, - } - - watchNs := apiv1.NamespaceAll - if n.cfg.ForceNamespaceIsolation && n.cfg.Namespace != apiv1.NamespaceAll { - watchNs = n.cfg.Namespace - } - - lister := &ingress.StoreLister{} - lister.IngressAnnotation.Store = cache_client.NewStore(cache_client.DeletionHandlingMetaNamespaceKeyFunc) - - controller := &cacheController{} - - lister.Ingress.Store, controller.Ingress = cache.NewInformer( - cache.NewListWatchFromClient(n.cfg.Client.ExtensionsV1beta1().RESTClient(), "ingresses", n.cfg.Namespace, fields.Everything()), - &extensions.Ingress{}, n.cfg.ResyncPeriod, ingEventHandler) - - lister.Endpoint.Store, controller.Endpoint = cache.NewInformer( - cache.NewListWatchFromClient(n.cfg.Client.CoreV1().RESTClient(), "endpoints", n.cfg.Namespace, fields.Everything()), - &apiv1.Endpoints{}, n.cfg.ResyncPeriod, eventHandler) - - lister.Secret.Store, controller.Secret = cache.NewInformer( - cache.NewListWatchFromClient(n.cfg.Client.CoreV1().RESTClient(), "secrets", watchNs, fields.Everything()), - &apiv1.Secret{}, n.cfg.ResyncPeriod, secrEventHandler) - - lister.ConfigMap.Store, controller.Configmap = cache.NewInformer( - cache.NewListWatchFromClient(n.cfg.Client.CoreV1().RESTClient(), "configmaps", watchNs, fields.Everything()), - &apiv1.ConfigMap{}, n.cfg.ResyncPeriod, mapEventHandler) - - lister.Service.Store, controller.Service = cache.NewInformer( - cache.NewListWatchFromClient(n.cfg.Client.CoreV1().RESTClient(), "services", n.cfg.Namespace, fields.Everything()), - &apiv1.Service{}, n.cfg.ResyncPeriod, cache.ResourceEventHandlerFuncs{}) - - return lister, controller -} diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index b74c05cb0..6ead782ff 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -36,21 +36,14 @@ import ( apiv1 "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" - "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/client-go/kubernetes/scheme" - v1core "k8s.io/client-go/kubernetes/typed/core/v1" - "k8s.io/client-go/tools/record" "k8s.io/client-go/util/flowcontrol" "k8s.io/kubernetes/pkg/util/filesystem" "k8s.io/ingress-nginx/internal/ingress" - "k8s.io/ingress-nginx/internal/ingress/annotations" "k8s.io/ingress-nginx/internal/ingress/annotations/class" - "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" "k8s.io/ingress-nginx/internal/ingress/controller/process" ngx_template "k8s.io/ingress-nginx/internal/ingress/controller/template" - "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/status" "k8s.io/ingress-nginx/internal/ingress/store" ing_net "k8s.io/ingress-nginx/internal/net" @@ -69,10 +62,9 @@ const ( ) var ( - tmplPath = "/etc/nginx/template/nginx.tmpl" - cfgPath = "/etc/nginx/nginx.conf" - nginxBinary = "/usr/sbin/nginx" - defIngressClass = "nginx" + tmplPath = "/etc/nginx/template/nginx.tmpl" + cfgPath = "/etc/nginx/nginx.conf" + nginxBinary = "/usr/sbin/nginx" ) // NewNGINXController creates a new NGINX Ingress controller. @@ -84,56 +76,67 @@ func NewNGINXController(config *Configuration) *NGINXController { ngx = nginxBinary } - eventBroadcaster := record.NewBroadcaster() - eventBroadcaster.StartLogging(glog.Infof) - eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{ - Interface: config.Client.CoreV1().Events(config.Namespace), - }) - h, err := dns.GetSystemNameServers() if err != nil { glog.Warningf("unexpected error reading system nameservers: %v", err) } n := &NGINXController{ - backendDefaults: ngx_config.NewDefault().Backend, - binary: ngx, + binary: ngx, configmap: &apiv1.ConfigMap{}, isIPV6Enabled: ing_net.IsIPv6Enabled(), - resolver: h, - cfg: config, - sslCertTracker: store.NewSSLCertTracker(), + resolver: h, + cfg: config, + syncRateLimiter: flowcontrol.NewTokenBucketRateLimiter(0.3, 1), - recorder: eventBroadcaster.NewRecorder(scheme.Scheme, apiv1.EventSource{ - Component: "nginx-ingress-controller", - }), - stopCh: make(chan struct{}), + updateCh: make(chan store.Event), + stopLock: &sync.Mutex{}, fileSystem: filesystem.DefaultFs{}, } - n.listers, n.controllers = n.createListers(n.stopCh) - - n.stats = newStatsCollector(config.Namespace, config.IngressClass, n.binary, n.cfg.ListenPorts.Status) + n.stats = newStatsCollector(config.Namespace, class.IngressClass, n.binary, n.cfg.ListenPorts.Status) n.syncQueue = task.NewTaskQueue(n.syncIngress) - n.annotations = annotations.NewAnnotationExtractor(n) + // start goroutine to process events + // from changes in objects from kubernetes + go func(updateCh chan store.Event) { + for evt := range updateCh { + switch obj := evt.Obj.(type) { + case *apiv1.ConfigMap: + // update configration configmap + n.SetConfig(obj) + } + + // any other change could trigger an update + n.syncQueue.Enqueue(evt.Obj) + } + }(n.updateCh) + + n.storeLister = store.New( + n.cfg.EnableSSLChainCompletion, + n.cfg.Namespace, + n.cfg.ConfigMapName, + n.cfg.TCPConfigMapName, + n.cfg.UDPConfigMapName, + n.cfg.ResyncPeriod, + n.cfg.Client, + n.updateCh, + ) if config.UpdateStatus { n.syncStatus = status.NewStatusSyncer(status.Config{ Client: config.Client, PublishService: config.PublishService, - IngressLister: n.listers.Ingress, + IngressLister: n.storeLister.ListIngresses, ElectionID: config.ElectionID, - IngressClass: config.IngressClass, - DefaultIngressClass: config.DefaultIngressClass, UpdateStatusOnShutdown: config.UpdateStatusOnShutdown, UseNodeInternalIP: config.UseNodeInternalIP, }) @@ -174,21 +177,10 @@ Error loading new template : %v type NGINXController struct { cfg *Configuration - listers *ingress.StoreLister - controllers *cacheController - - annotations annotations.Extractor - - recorder record.EventRecorder - syncQueue *task.Queue syncStatus status.Sync - // local store of SSL certificates - // (only certificates used in ingress) - sslCertTracker *store.SSLCertTracker - syncRateLimiter flowcontrol.RateLimiter // stopLock is used to enforce only a single call to Stop is active. @@ -196,8 +188,12 @@ type NGINXController struct { // allowing concurrent stoppers leads to stack traces. stopLock *sync.Mutex + // stopCh channel used to stop informer controllers stopCh chan struct{} + // updateCh channel used to process events from api server + updateCh chan store.Event + // ngxErrCh channel used to detect errors with the nginx processes ngxErrCh chan error @@ -210,7 +206,7 @@ type NGINXController struct { configmap *apiv1.ConfigMap - storeLister *ingress.StoreLister + storeLister store.Storer binary string resolver []net.IP @@ -221,16 +217,14 @@ type NGINXController struct { // returns true if IPV6 is enabled in the pod isIPV6Enabled bool - // returns true if proxy protocol es enabled - IsProxyProtocolEnabled bool - isSSLPassthroughEnabled bool isShuttingDown bool - Proxy *TCPProxy + // returns true if proxy protocol es enabled + IsProxyProtocolEnabled bool - backendDefaults defaults.Backend + Proxy *TCPProxy fileSystem filesystem.Filesystem } @@ -239,34 +233,14 @@ type NGINXController struct { func (n *NGINXController) Start() { glog.Infof("starting Ingress controller") - n.controllers.Run(n.stopCh) - - // initial sync of secrets to avoid unnecessary reloads - glog.Info("running initial sync of secrets") - for _, obj := range n.listers.Ingress.List() { - ing := obj.(*extensions.Ingress) - - if !class.IsValid(ing, n.cfg.IngressClass, n.cfg.DefaultIngressClass) { - a, _ := parser.GetStringAnnotation(class.IngressKey, ing, n) - glog.Infof("ignoring add for ingress %v based on annotation %v with value %v", ing.Name, class.IngressKey, a) - continue - } - - n.readSecrets(ing) - } + n.storeLister.Run(n.stopCh) go n.syncQueue.Run(time.Second, n.stopCh) - if n.cfg.EnableSSLChainCompletion { - go wait.Until(n.checkSSLChainIssues, 60*time.Second, n.stopCh) - } - if n.syncStatus != nil { go n.syncStatus.Run(n.stopCh) } - go wait.Until(n.checkMissingSecrets, 30*time.Second, n.stopCh) - done := make(chan error, 1) cmd := exec.Command(n.binary, "-c", cfgPath) @@ -276,7 +250,6 @@ func (n *NGINXController) Start() { Setpgid: true, Pgid: 0, } - glog.Info("starting NGINX process...") n.start(cmd) @@ -365,7 +338,8 @@ func (n *NGINXController) start(cmd *exec.Cmd) { }() } -// DefaultEndpoint returns the default endpoint to be use as default server that returns 404. +// DefaultEndpoint returns the default endpoint to be use as +// default server that returns 404. func (n NGINXController) DefaultEndpoint() ingress.Endpoint { return ingress.Endpoint{ Address: "127.0.0.1", @@ -374,8 +348,8 @@ func (n NGINXController) DefaultEndpoint() ingress.Endpoint { } } -// testTemplate checks if the NGINX configuration inside the byte array is valid -// running the command "nginx -t" using a temporal file. +// 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 { if len(cfg) == 0 { return fmt.Errorf("invalid nginx configuration (empty)") @@ -391,7 +365,8 @@ func (n NGINXController) testTemplate(cfg []byte) error { } out, err := exec.Command(n.binary, "-t", "-c", tmpfile.Name()).CombinedOutput() if err != nil { - // this error is different from the rest because it must be clear why nginx is not working + // this error is different from the rest because it must be clear + // why nginx is not working oe := fmt.Sprintf(` ------------------------------------------------------------------------------- Error: %v @@ -406,6 +381,7 @@ Error: %v } // SetConfig sets the configured configmap +// TODO: refactor func (n *NGINXController) SetConfig(cmap *apiv1.ConfigMap) { n.configmap = cmap n.IsProxyProtocolEnabled = false @@ -433,7 +409,7 @@ func (n *NGINXController) SetConfig(cmap *apiv1.ConfigMap) { ioutil.WriteFile("/etc/nginx/tickets.key", d, 0644) } - n.backendDefaults = c.Backend + n.storeLister.SetDefaultBackend(c.Backend) } // OnUpdate is called periodically by syncQueue to keep the configuration in sync. @@ -555,38 +531,34 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error { setHeaders := map[string]string{} if cfg.ProxySetHeaders != "" { - cmap, exists, err := n.storeLister.ConfigMap.GetByKey(cfg.ProxySetHeaders) - if err != nil { - glog.Warningf("unexpected error reading configmap %v: %v", cfg.ProxySetHeaders, err) - } - - if exists { - setHeaders = cmap.(*apiv1.ConfigMap).Data + cmap, err := n.storeLister.GetConfigMap(cfg.ProxySetHeaders) + if err == nil { + setHeaders = cmap.Data + } else { + glog.Warningf("unexpected error reading configmap %v: %v", cfg.AddHeaders, err) } } addHeaders := map[string]string{} if cfg.AddHeaders != "" { - cmap, exists, err := n.storeLister.ConfigMap.GetByKey(cfg.AddHeaders) - if err != nil { + cmap, err := n.storeLister.GetConfigMap(cfg.AddHeaders) + if err == nil { + addHeaders = cmap.Data + } else { glog.Warningf("unexpected error reading configmap %v: %v", cfg.AddHeaders, err) } - - if exists { - addHeaders = cmap.(*apiv1.ConfigMap).Data - } } + // TODO: refactor this to avoid creating the file on update sslDHParam := "" if cfg.SSLDHParam != "" { secretName := cfg.SSLDHParam - s, exists, err := n.storeLister.Secret.GetByKey(secretName) + secret, err := n.storeLister.GetSecret(secretName) if err != nil { glog.Warningf("unexpected error reading secret %v: %v", secretName, err) } - if exists { - secret := s.(*apiv1.Secret) + if secret != nil { nsSecName := strings.Replace(secretName, "/", "-", -1) dh, ok := secret.Data["dhparam.pem"] diff --git a/internal/ingress/controller/process/nginx.go b/internal/ingress/controller/process/nginx.go index 4a925a3e1..b8fdcd669 100644 --- a/internal/ingress/controller/process/nginx.go +++ b/internal/ingress/controller/process/nginx.go @@ -45,8 +45,8 @@ NGINX master process died (%v): %v return true } +// WaitUntilPortIsAvailable waits until no workers is listening in a port func WaitUntilPortIsAvailable(port int) { - // we wait until the workers are killed for { conn, err := net.DialTimeout("tcp", fmt.Sprintf("0.0.0.0:%v", port), 1*time.Second) if err != nil { diff --git a/internal/ingress/resolver/main.go b/internal/ingress/resolver/main.go index 9fd43828f..b73144866 100644 --- a/internal/ingress/resolver/main.go +++ b/internal/ingress/resolver/main.go @@ -37,9 +37,6 @@ type Resolver interface { // GetService searches for services contenating the namespace and name using a the character / GetService(string) (*apiv1.Service, error) - - // GetAnnotationWithPrefix returns the prefix of the Ingress annotations - GetAnnotationWithPrefix(suffix string) string } // AuthSSLCert contains the necessary information to do certificate based diff --git a/internal/ingress/status/status.go b/internal/ingress/status/status.go index 8c18f9c7c..27cd070b6 100644 --- a/internal/ingress/status/status.go +++ b/internal/ingress/status/status.go @@ -25,9 +25,7 @@ import ( "time" "github.com/golang/glog" - "github.com/pkg/errors" - pool "gopkg.in/go-playground/pool.v3" apiv1 "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -41,7 +39,6 @@ import ( "k8s.io/kubernetes/pkg/kubelet/util/sliceutils" "k8s.io/ingress-nginx/internal/ingress/annotations/class" - "k8s.io/ingress-nginx/internal/ingress/store" "k8s.io/ingress-nginx/internal/k8s" "k8s.io/ingress-nginx/internal/task" ) @@ -68,10 +65,7 @@ type Config struct { UseNodeInternalIP bool - IngressLister store.IngressLister - - DefaultIngressClass string - IngressClass string + IngressLister func() []*extensions.Ingress } // statusSync keeps the status IP in each Ingress rule updated executing a periodic check @@ -178,11 +172,15 @@ func NewStatusSyncer(config Config) Sync { } st.syncQueue = task.NewCustomTaskQueue(st.sync, st.keyfunc) + if config.ElectionID == "" { + config.ElectionID = "ingress-controller-leader" + } + // we need to use the defined ingress class to allow multiple leaders // in order to update information about ingress status - electionID := fmt.Sprintf("%v-%v", config.ElectionID, config.DefaultIngressClass) - if config.IngressClass != "" { - electionID = fmt.Sprintf("%v-%v", config.ElectionID, config.IngressClass) + electionID := fmt.Sprintf("%v-%v", config.ElectionID, class.DefaultClass) + if class.IngressClass != "" { + electionID = fmt.Sprintf("%v-%v", config.ElectionID, class.IngressClass) } callbacks := leaderelection.LeaderCallbacks{ @@ -304,59 +302,44 @@ func sliceToStatus(endpoints []string) []apiv1.LoadBalancerIngress { // updateStatus changes the status information of Ingress rules func (s *statusSync) updateStatus(newIngressPoint []apiv1.LoadBalancerIngress) { - ings := s.IngressLister.List() + // max number of goroutines to be used in the update process + max := 10 + running := make(chan struct{}, max) - p := pool.NewLimited(10) - defer p.Close() + for _, ing := range s.IngressLister() { + running <- struct{}{} // waits for a free slot + go func(ing *extensions.Ingress, + status []apiv1.LoadBalancerIngress, + client clientset.Interface) { + defer func() { + <-running // releases slot + }() - batch := p.Batch() + sort.SliceStable(status, lessLoadBalancerIngress(status)) + curIPs := ing.Status.LoadBalancer.Ingress + sort.SliceStable(curIPs, lessLoadBalancerIngress(curIPs)) - for _, cur := range ings { - ing := cur.(*extensions.Ingress) + if ingressSliceEqual(status, curIPs) { + glog.V(3).Infof("skipping update of Ingress %v/%v (no change)", ing.Namespace, ing.Name) + return + } - if !class.IsValid(ing, s.Config.IngressClass, s.Config.DefaultIngressClass) { - continue - } + // we cannot assume/trust the local informer is up to date + // request a fresh copy where we are doing the update + ingClient := client.Extensions().Ingresses(ing.Namespace) + currIng, err := ingClient.Get(ing.Name, metav1.GetOptions{}) + if err != nil { + glog.Errorf("unexpected error searching Ingress %v/%v: %v", ing.Namespace, ing.Name, err) + return + } - batch.Queue(runUpdate(ing, newIngressPoint, s.Client)) - } - - batch.QueueComplete() - batch.WaitAll() -} - -func runUpdate(ing *extensions.Ingress, status []apiv1.LoadBalancerIngress, - client clientset.Interface) pool.WorkFunc { - return func(wu pool.WorkUnit) (interface{}, error) { - if wu.IsCancelled() { - return nil, nil - } - - sort.SliceStable(status, lessLoadBalancerIngress(status)) - - curIPs := ing.Status.LoadBalancer.Ingress - sort.SliceStable(curIPs, lessLoadBalancerIngress(curIPs)) - - if ingressSliceEqual(status, curIPs) { - glog.V(3).Infof("skipping update of Ingress %v/%v (no change)", ing.Namespace, ing.Name) - return true, nil - } - - ingClient := client.ExtensionsV1beta1().Ingresses(ing.Namespace) - - currIng, err := ingClient.Get(ing.Name, metav1.GetOptions{}) - if err != nil { - return nil, errors.Wrap(err, fmt.Sprintf("unexpected error searching Ingress %v/%v", ing.Namespace, ing.Name)) - } - - glog.Infof("updating Ingress %v/%v status to %v", currIng.Namespace, currIng.Name, status) - currIng.Status.LoadBalancer.Ingress = status - _, err = ingClient.UpdateStatus(currIng) - if err != nil { - glog.Warningf("error updating ingress rule: %v", err) - } - - return true, nil + glog.Infof("updating Ingress %v/%v status to %v", currIng.Namespace, currIng.Name, status) + currIng.Status.LoadBalancer.Ingress = status + _, err = ingClient.UpdateStatus(currIng) + if err != nil { + glog.Warningf("error updating ingress rule: %v", err) + } + }(ing, newIngressPoint, s.Client) } } diff --git a/internal/ingress/status/status_test.go b/internal/ingress/status/status_test.go index 77bfb6969..2042f0216 100644 --- a/internal/ingress/status/status_test.go +++ b/internal/ingress/status/status_test.go @@ -25,11 +25,9 @@ import ( extensions "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" testclient "k8s.io/client-go/kubernetes/fake" - "k8s.io/client-go/tools/cache" "k8s.io/kubernetes/pkg/api" "k8s.io/ingress-nginx/internal/ingress/annotations/class" - "k8s.io/ingress-nginx/internal/ingress/store" "k8s.io/ingress-nginx/internal/k8s" "k8s.io/ingress-nginx/internal/task" ) @@ -213,26 +211,25 @@ func buildExtensionsIngresses() []extensions.Ingress { } } -func buildIngressListener() store.IngressLister { - s := cache.NewStore(cache.MetaNamespaceKeyFunc) - s.Add(&extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo_ingress_non_01", - Namespace: apiv1.NamespaceDefault, - }}) - s.Add(&extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo_ingress_1", - Namespace: apiv1.NamespaceDefault, - }, - Status: extensions.IngressStatus{ - LoadBalancer: apiv1.LoadBalancerStatus{ - Ingress: buildLoadBalancerIngressByIP(), +func buildIngressListener() []*extensions.Ingress { + return []*extensions.Ingress{ + &extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo_ingress_non_01", + Namespace: apiv1.NamespaceDefault, + }}, + &extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo_ingress_1", + Namespace: apiv1.NamespaceDefault, + }, + Status: extensions.IngressStatus{ + LoadBalancer: apiv1.LoadBalancerStatus{ + Ingress: buildLoadBalancerIngressByIP(), + }, }, }, - }) - - return store.IngressLister{Store: s} + } } func buildStatusSync() statusSync { @@ -248,7 +245,7 @@ func buildStatusSync() statusSync { Config: Config{ Client: buildSimpleClientSet(), PublishService: apiv1.NamespaceDefault + "/" + "foo", - IngressLister: buildIngressListener(), + IngressLister: buildIngressListener, }, } } @@ -260,9 +257,7 @@ func TestStatusActions(t *testing.T) { c := Config{ Client: buildSimpleClientSet(), PublishService: "", - IngressLister: buildIngressListener(), - DefaultIngressClass: "nginx", - IngressClass: "", + IngressLister: buildIngressListener, UpdateStatusOnShutdown: true, } // create object @@ -285,7 +280,7 @@ func TestStatusActions(t *testing.T) { newIPs := []apiv1.LoadBalancerIngress{{ IP: "11.0.0.2", }} - fooIngress1, err1 := fk.Client.ExtensionsV1beta1().Ingresses(apiv1.NamespaceDefault).Get("foo_ingress_1", metav1.GetOptions{}) + fooIngress1, err1 := fk.Client.Extensions().Ingresses(apiv1.NamespaceDefault).Get("foo_ingress_1", metav1.GetOptions{}) if err1 != nil { t.Fatalf("unexpected error") } @@ -298,7 +293,7 @@ func TestStatusActions(t *testing.T) { fk.Shutdown() // ingress should be empty newIPs2 := []apiv1.LoadBalancerIngress{} - fooIngress2, err2 := fk.Client.ExtensionsV1beta1().Ingresses(apiv1.NamespaceDefault).Get("foo_ingress_1", metav1.GetOptions{}) + fooIngress2, err2 := fk.Client.Extensions().Ingresses(apiv1.NamespaceDefault).Get("foo_ingress_1", metav1.GetOptions{}) if err2 != nil { t.Fatalf("unexpected error") } @@ -307,7 +302,7 @@ func TestStatusActions(t *testing.T) { t.Fatalf("returned %v but expected %v", fooIngress2CurIPs, newIPs2) } - oic, err := fk.Client.ExtensionsV1beta1().Ingresses(api.NamespaceDefault).Get("foo_ingress_different_class", metav1.GetOptions{}) + oic, err := fk.Client.Extensions().Ingresses(api.NamespaceDefault).Get("foo_ingress_different_class", metav1.GetOptions{}) if err != nil { t.Fatalf("unexpected error") } @@ -367,8 +362,6 @@ func TestRunningAddresessWithPods(t *testing.T) { } } -/* -TODO: this test requires a refactoring func TestUpdateStatus(t *testing.T) { fk := buildStatusSync() newIPs := buildLoadBalancerIngressByIP() @@ -392,7 +385,7 @@ func TestUpdateStatus(t *testing.T) { t.Fatalf("returned %v but expected %v", fooIngress2CurIPs, []apiv1.LoadBalancerIngress{}) } } -*/ + func TestSliceToStatus(t *testing.T) { fkEndpoints := []string{ "10.0.0.1", diff --git a/internal/ingress/controller/backend_ssl.go b/internal/ingress/store/backend_ssl.go similarity index 69% rename from internal/ingress/controller/backend_ssl.go rename to internal/ingress/store/backend_ssl.go index a4f3f138f..a86e54930 100644 --- a/internal/ingress/controller/backend_ssl.go +++ b/internal/ingress/store/backend_ssl.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package controller +package store import ( "fmt" @@ -28,50 +28,50 @@ import ( extensions "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-nginx/internal/ingress" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/k8s" "k8s.io/ingress-nginx/internal/net/ssl" ) // syncSecret keeps in sync Secrets used by Ingress rules with the files on // disk to allow copy of the content of the secret to disk to be used // by external processes. -func (ic *NGINXController) syncSecret(key string) { +func (s k8sStore) syncSecret(key string) { glog.V(3).Infof("starting syncing of secret %v", key) - cert, err := ic.getPemCertificate(key) + // TODO: getPemCertificate should not write to disk to avoid unnecessary overhead + cert, err := s.getPemCertificate(key) if err != nil { glog.Warningf("error obtaining PEM from secret %v: %v", key, err) return } // create certificates and add or update the item in the store - cur, exists := ic.sslCertTracker.Get(key) - if exists { - s := cur.(*ingress.SSLCert) - if s.Equal(cert) { + cur, err := s.GetLocalSecret(key) + if err == nil { + if cur.Equal(cert) { // no need to update return } glog.Infof("updating secret %v in the local store", key) - ic.sslCertTracker.Update(key, cert) + s.sslStore.Update(key, cert) // this update must trigger an update // (like an update event from a change in Ingress) - ic.syncQueue.Enqueue(&extensions.Ingress{}) + //ic.syncQueue.Enqueue(&extensions.Ingress{}) return } glog.Infof("adding secret %v to the local store", key) - ic.sslCertTracker.Add(key, cert) + s.sslStore.Add(key, cert) // this update must trigger an update // (like an update event from a change in Ingress) - ic.syncQueue.Enqueue(&extensions.Ingress{}) + //ic.syncQueue.Enqueue(&extensions.Ingress{}) } // getPemCertificate receives a secret, and creates a ingress.SSLCert as return. // It parses the secret and verifies if it's a keypair, or a 'ca.crt' secret only. -func (ic *NGINXController) getPemCertificate(secretName string) (*ingress.SSLCert, error) { - secret, err := ic.listers.Secret.GetByName(secretName) +func (s k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error) { + secret, err := s.listers.Secret.ByKey(secretName) if err != nil { return nil, fmt.Errorf("error retrieving secret %v: %v", secretName, err) } @@ -83,7 +83,7 @@ func (ic *NGINXController) getPemCertificate(secretName string) (*ingress.SSLCer // namespace/secretName -> namespace-secretName nsSecName := strings.Replace(secretName, "/", "-", -1) - var s *ingress.SSLCert + var sslCert *ingress.SSLCert if okcert && okkey { if cert == nil { return nil, fmt.Errorf("secret %v has no 'tls.crt'", secretName) @@ -94,18 +94,17 @@ func (ic *NGINXController) getPemCertificate(secretName string) (*ingress.SSLCer // If 'ca.crt' is also present, it will allow this secret to be used in the // 'nginx.ingress.kubernetes.io/auth-tls-secret' annotation - s, err = ssl.AddOrUpdateCertAndKey(nsSecName, cert, key, ca) + sslCert, err = ssl.AddOrUpdateCertAndKey(nsSecName, cert, key, ca) if err != nil { return nil, fmt.Errorf("unexpected error creating pem file: %v", err) } - glog.V(3).Infof("found 'tls.crt' and 'tls.key', configuring %v as a TLS Secret (CN: %v)", secretName, s.CN) + glog.V(3).Infof("found 'tls.crt' and 'tls.key', configuring %v as a TLS Secret (CN: %v)", secretName, sslCert.CN) if ca != nil { glog.V(3).Infof("found 'ca.crt', secret %v can also be used for Certificate Authentication", secretName) } - } else if ca != nil { - s, err = ssl.AddCertAuth(nsSecName, ca) + sslCert, err = ssl.AddCertAuth(nsSecName, ca) if err != nil { return nil, fmt.Errorf("unexpected error creating pem file: %v", err) @@ -119,15 +118,19 @@ func (ic *NGINXController) getPemCertificate(secretName string) (*ingress.SSLCer return nil, fmt.Errorf("no keypair or CA cert could be found in %v", secretName) } - s.Name = secret.Name - s.Namespace = secret.Namespace - return s, nil + sslCert.Name = secret.Name + sslCert.Namespace = secret.Namespace + + return sslCert, nil } -func (ic *NGINXController) checkSSLChainIssues() { - for _, secretName := range ic.sslCertTracker.ListKeys() { - s, _ := ic.sslCertTracker.Get(secretName) - secret := s.(*ingress.SSLCert) +func (s k8sStore) checkSSLChainIssues() { + for _, item := range s.ListLocalSecrets() { + secretName := k8s.MetaNamespaceKey(item) + secret, err := s.GetLocalSecret(secretName) + if err != nil { + continue + } if secret.FullChainPemFileName != "" { // chain already checked @@ -158,42 +161,53 @@ func (ic *NGINXController) checkSSLChainIssues() { dst.FullChainPemFileName = fullChainPemFileName glog.Infof("updating local copy of ssl certificate %v with missing intermediate CA certs", secretName) - ic.sslCertTracker.Update(secretName, dst) + s.sslStore.Update(secretName, dst) // this update must trigger an update // (like an update event from a change in Ingress) - ic.syncQueue.Enqueue(&extensions.Ingress{}) + //ic.syncQueue.Enqueue(&extensions.Ingress{}) } } -// checkMissingSecrets verify if one or more ingress rules contains a reference -// to a secret that is not present in the local secret store. -// In this case we call syncSecret. -func (ic *NGINXController) checkMissingSecrets() { - for _, obj := range ic.listers.Ingress.List() { - ing := obj.(*extensions.Ingress) - - if !class.IsValid(ing, ic.cfg.IngressClass, ic.cfg.DefaultIngressClass) { - continue - } - +// checkMissingSecrets verifies if one or more ingress rules contains +// a reference to a secret that is not present in the local secret store. +func (s k8sStore) checkMissingSecrets() { + for _, ing := range s.ListIngresses() { for _, tls := range ing.Spec.TLS { if tls.SecretName == "" { continue } key := fmt.Sprintf("%v/%v", ing.Namespace, tls.SecretName) - if _, ok := ic.sslCertTracker.Get(key); !ok { - ic.syncSecret(key) + if _, ok := s.sslStore.Get(key); !ok { + s.syncSecret(key) } } - key, _ := parser.GetStringAnnotation("auth-tls-secret", ing, ic) + key, _ := parser.GetStringAnnotation("auth-tls-secret", ing) if key == "" { - continue + return } - if _, ok := ic.sslCertTracker.Get(key); !ok { - ic.syncSecret(key) + if _, ok := s.sslStore.Get(key); !ok { + s.syncSecret(key) } } } + +// readSecrets extracts information about secrets from an Ingress rule +func (s k8sStore) readSecrets(ing *extensions.Ingress) { + for _, tls := range ing.Spec.TLS { + if tls.SecretName == "" { + continue + } + + key := fmt.Sprintf("%v/%v", ing.Namespace, tls.SecretName) + s.syncSecret(key) + } + + key, _ := parser.GetStringAnnotation("auth-tls-secret", ing) + if key == "" { + return + } + s.syncSecret(key) +} diff --git a/internal/ingress/controller/backend_ssl_test.go b/internal/ingress/store/backend_ssl_test.go similarity index 96% rename from internal/ingress/controller/backend_ssl_test.go rename to internal/ingress/store/backend_ssl_test.go index 16892da62..31304bd56 100644 --- a/internal/ingress/controller/backend_ssl_test.go +++ b/internal/ingress/store/backend_ssl_test.go @@ -14,24 +14,20 @@ See the License for the specific language governing permissions and limitations under the License. */ -package controller +package store import ( "encoding/base64" "fmt" "io/ioutil" - "testing" apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" testclient "k8s.io/client-go/kubernetes/fake" cache_client "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/flowcontrol" + "k8s.io/kubernetes/pkg/api" "k8s.io/ingress-nginx/internal/ingress" - "k8s.io/ingress-nginx/internal/ingress/store" - "k8s.io/ingress-nginx/internal/task" - "k8s.io/kubernetes/pkg/api" ) const ( @@ -66,8 +62,8 @@ func buildSimpleClientSetForBackendSSL() *testclient.Clientset { return testclient.NewSimpleClientset() } -func buildIngListenerForBackendSSL() store.IngressLister { - ingLister := store.IngressLister{} +func buildIngListenerForBackendSSL() IngressLister { + ingLister := IngressLister{} ingLister.Store = cache_client.NewStore(cache_client.DeletionHandlingMetaNamespaceKeyFunc) return ingLister } @@ -81,20 +77,21 @@ func buildSecretForBackendSSL() *apiv1.Secret { } } -func buildSecrListerForBackendSSL() store.SecretLister { - secrLister := store.SecretLister{} +func buildSecrListerForBackendSSL() SecretLister { + secrLister := SecretLister{} secrLister.Store = cache_client.NewStore(cache_client.DeletionHandlingMetaNamespaceKeyFunc) return secrLister } +/* func buildListers() *ingress.StoreLister { sl := &ingress.StoreLister{} sl.Ingress.Store = buildIngListenerForBackendSSL() sl.Secret.Store = buildSecrListerForBackendSSL() return sl } - +*/ func buildControllerForBackendSSL() cache_client.Controller { cfg := &cache_client.Config{ Queue: &MockQueue{Synced: true}, @@ -103,6 +100,7 @@ func buildControllerForBackendSSL() cache_client.Controller { return cache_client.New(cfg) } +/* func buildGenericControllerForBackendSSL() *NGINXController { gc := &NGINXController{ syncRateLimiter: flowcontrol.NewTokenBucketRateLimiter(0.3, 1), @@ -110,13 +108,13 @@ func buildGenericControllerForBackendSSL() *NGINXController { Client: buildSimpleClientSetForBackendSSL(), }, listers: buildListers(), - sslCertTracker: store.NewSSLCertTracker(), + sslCertTracker: NewSSLCertTracker(), } gc.syncQueue = task.NewTaskQueue(gc.syncIngress) return gc } - +*/ func buildCrtKeyAndCA() ([]byte, []byte, []byte, error) { // prepare td, err := ioutil.TempDir("", "ssl") @@ -140,6 +138,7 @@ func buildCrtKeyAndCA() ([]byte, []byte, []byte, error) { return dCrt, dKey, dCa, nil } +/* func TestSyncSecret(t *testing.T) { // prepare for test dCrt, dKey, dCa, err := buildCrtKeyAndCA() @@ -232,3 +231,4 @@ func TestGetPemCertificate(t *testing.T) { }) } } +*/ diff --git a/internal/ingress/store/configmap.go b/internal/ingress/store/configmap.go new file mode 100644 index 000000000..d679a86c4 --- /dev/null +++ b/internal/ingress/store/configmap.go @@ -0,0 +1,41 @@ +/* +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 store + +import ( + "fmt" + + apiv1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/cache" +) + +// ConfigMapLister makes a Store that lists Configmaps. +type ConfigMapLister struct { + cache.Store +} + +// ByKey searches for a configmap in the local configmaps Store +func (cml *ConfigMapLister) ByKey(key string) (*apiv1.ConfigMap, error) { + s, exists, err := cml.GetByKey(key) + if err != nil { + return nil, err + } + if !exists { + return nil, fmt.Errorf("configmap %v was not found", key) + } + return s.(*apiv1.ConfigMap), nil +} diff --git a/internal/ingress/store/endpoint.go b/internal/ingress/store/endpoint.go new file mode 100644 index 000000000..c464e98b5 --- /dev/null +++ b/internal/ingress/store/endpoint.go @@ -0,0 +1,40 @@ +/* +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 store + +import ( + "fmt" + + apiv1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/cache" +) + +// EndpointLister makes a Store that lists Endpoints. +type EndpointLister struct { + cache.Store +} + +// GetServiceEndpoints returns the endpoints of a service, matched on service name. +func (s *EndpointLister) GetServiceEndpoints(svc *apiv1.Service) (*apiv1.Endpoints, error) { + for _, m := range s.Store.List() { + ep := m.(*apiv1.Endpoints) + if svc.Name == ep.Name && svc.Namespace == ep.Namespace { + return ep, nil + } + } + return nil, fmt.Errorf("could not find endpoints for service: %v", svc.Name) +} diff --git a/internal/ingress/store/ingress.go b/internal/ingress/store/ingress.go new file mode 100644 index 000000000..67a7b6c53 --- /dev/null +++ b/internal/ingress/store/ingress.go @@ -0,0 +1,41 @@ +/* +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 store + +import ( + "fmt" + + extensions "k8s.io/api/extensions/v1beta1" + "k8s.io/client-go/tools/cache" +) + +// IngressLister makes a Store that lists Ingress. +type IngressLister struct { + cache.Store +} + +// ByKey searches for an ingress in the local ingress Store +func (il IngressLister) ByKey(key string) (*extensions.Ingress, error) { + i, exists, err := il.GetByKey(key) + if err != nil { + return nil, err + } + if !exists { + return nil, fmt.Errorf("ingress %v was not found", key) + } + return i.(*extensions.Ingress), nil +} diff --git a/internal/ingress/store/ingress_annotation.go b/internal/ingress/store/ingress_annotation.go new file mode 100644 index 000000000..676875aca --- /dev/null +++ b/internal/ingress/store/ingress_annotation.go @@ -0,0 +1,26 @@ +/* +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 store + +import ( + "k8s.io/client-go/tools/cache" +) + +// IngressAnnotationsLister makes a Store that lists annotations in Ingress rules. +type IngressAnnotationsLister struct { + cache.Store +} diff --git a/internal/ingress/store/local_secret.go b/internal/ingress/store/local_secret.go new file mode 100644 index 000000000..0eb4ebc03 --- /dev/null +++ b/internal/ingress/store/local_secret.go @@ -0,0 +1,30 @@ +package store + +import ( + "fmt" + + "k8s.io/client-go/tools/cache" + + "k8s.io/ingress-nginx/internal/ingress" +) + +// SSLCertTracker holds a store of referenced Secrets in Ingress rules +type SSLCertTracker struct { + cache.ThreadSafeStore +} + +// NewSSLCertTracker creates a new SSLCertTracker store +func NewSSLCertTracker() *SSLCertTracker { + return &SSLCertTracker{ + cache.NewThreadSafeStore(cache.Indexers{}, cache.Indices{}), + } +} + +// ByKey searches for an ingress in the local ingress Store +func (s SSLCertTracker) ByKey(key string) (*ingress.SSLCert, error) { + cert, exists := s.Get(key) + if !exists { + return nil, fmt.Errorf("local SSL certificate %v was not found", key) + } + return cert.(*ingress.SSLCert), nil +} diff --git a/internal/ingress/store/local_secret_test.go b/internal/ingress/store/local_secret_test.go new file mode 100644 index 000000000..0b532c41d --- /dev/null +++ b/internal/ingress/store/local_secret_test.go @@ -0,0 +1,39 @@ +/* +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 store + +import "testing" + +func TestSSLCertTracker(t *testing.T) { + tracker := NewSSLCertTracker() + + items := len(tracker.List()) + if items != 0 { + t.Errorf("expected 0 items in the store but %v returned", items) + } + + tracker.Add("key", "value") + items = len(tracker.List()) + if items != 1 { + t.Errorf("expected 1 item in the store but %v returned", items) + } + + item, exists := tracker.Get("key") + if !exists || item == nil { + t.Errorf("expected an item from the store but none returned") + } +} diff --git a/internal/ingress/store/main.go b/internal/ingress/store/main.go deleted file mode 100644 index 299f54c0b..000000000 --- a/internal/ingress/store/main.go +++ /dev/null @@ -1,113 +0,0 @@ -/* -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 store - -import ( - "fmt" - - apiv1 "k8s.io/api/core/v1" - "k8s.io/client-go/tools/cache" -) - -// IngressLister makes a Store that lists Ingress. -type IngressLister struct { - cache.Store -} - -// IngressAnnotationsLister makes a Store that lists annotations in Ingress rules. -type IngressAnnotationsLister struct { - cache.Store -} - -// SecretLister makes a Store that lists Secrets. -type SecretLister struct { - cache.Store -} - -// GetByName searches for a secret in the local secrets Store -func (sl *SecretLister) GetByName(name string) (*apiv1.Secret, error) { - s, exists, err := sl.GetByKey(name) - if err != nil { - return nil, err - } - if !exists { - return nil, fmt.Errorf("secret %v was not found", name) - } - return s.(*apiv1.Secret), nil -} - -// ConfigMapLister makes a Store that lists Configmaps. -type ConfigMapLister struct { - cache.Store -} - -// GetByName searches for a configmap in the local configmaps Store -func (cml *ConfigMapLister) GetByName(name string) (*apiv1.ConfigMap, error) { - s, exists, err := cml.GetByKey(name) - if err != nil { - return nil, err - } - if !exists { - return nil, fmt.Errorf("configmap %v was not found", name) - } - return s.(*apiv1.ConfigMap), nil -} - -// ServiceLister makes a Store that lists Services. -type ServiceLister struct { - cache.Store -} - -// GetByName searches for a service in the local secrets Store -func (sl *ServiceLister) GetByName(name string) (*apiv1.Service, error) { - s, exists, err := sl.GetByKey(name) - if err != nil { - return nil, err - } - if !exists { - return nil, fmt.Errorf("service %v was not found", name) - } - return s.(*apiv1.Service), nil -} - -// EndpointLister makes a Store that lists Endpoints. -type EndpointLister struct { - cache.Store -} - -// GetServiceEndpoints returns the endpoints of a service, matched on service name. -func (s *EndpointLister) GetServiceEndpoints(svc *apiv1.Service) (*apiv1.Endpoints, error) { - for _, m := range s.Store.List() { - ep := m.(*apiv1.Endpoints) - if svc.Name == ep.Name && svc.Namespace == ep.Namespace { - return ep, nil - } - } - return nil, fmt.Errorf("could not find endpoints for service: %v", svc.Name) -} - -// SSLCertTracker holds a store of referenced Secrets in Ingress rules -type SSLCertTracker struct { - cache.ThreadSafeStore -} - -// NewSSLCertTracker creates a new SSLCertTracker store -func NewSSLCertTracker() *SSLCertTracker { - return &SSLCertTracker{ - cache.NewThreadSafeStore(cache.Indexers{}, cache.Indices{}), - } -} diff --git a/internal/ingress/store/secret.go b/internal/ingress/store/secret.go new file mode 100644 index 000000000..54774e8e9 --- /dev/null +++ b/internal/ingress/store/secret.go @@ -0,0 +1,41 @@ +/* +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 store + +import ( + "fmt" + + apiv1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/cache" +) + +// SecretLister makes a Store that lists Secrets. +type SecretLister struct { + cache.Store +} + +// ByKey searches for a secret in the local secrets Store +func (sl *SecretLister) ByKey(key string) (*apiv1.Secret, error) { + s, exists, err := sl.GetByKey(key) + if err != nil { + return nil, err + } + if !exists { + return nil, fmt.Errorf("secret %v was not found", key) + } + return s.(*apiv1.Secret), nil +} diff --git a/internal/ingress/store/service.go b/internal/ingress/store/service.go new file mode 100644 index 000000000..44d235558 --- /dev/null +++ b/internal/ingress/store/service.go @@ -0,0 +1,41 @@ +/* +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 store + +import ( + "fmt" + + apiv1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/cache" +) + +// ServiceLister makes a Store that lists Services. +type ServiceLister struct { + cache.Store +} + +// ByKey searches for a service in the local secrets Store +func (sl *ServiceLister) ByKey(key string) (*apiv1.Service, error) { + s, exists, err := sl.GetByKey(key) + if err != nil { + return nil, err + } + if !exists { + return nil, fmt.Errorf("service %v was not found", key) + } + return s.(*apiv1.Service), nil +} diff --git a/internal/ingress/store/store.go b/internal/ingress/store/store.go new file mode 100644 index 000000000..70faccd2f --- /dev/null +++ b/internal/ingress/store/store.go @@ -0,0 +1,502 @@ +/* +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 store + +import ( + "fmt" + "reflect" + "time" + + "github.com/golang/glog" + + apiv1 "k8s.io/api/core/v1" + extensions "k8s.io/api/extensions/v1beta1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/wait" + clientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/scheme" + v1core "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/client-go/tools/cache" + cache_client "k8s.io/client-go/tools/cache" + "k8s.io/client-go/tools/record" + + "k8s.io/ingress-nginx/internal/ingress" + "k8s.io/ingress-nginx/internal/ingress/annotations" + "k8s.io/ingress-nginx/internal/ingress/annotations/class" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/ingress/defaults" + "k8s.io/ingress-nginx/internal/ingress/resolver" + "k8s.io/ingress-nginx/internal/k8s" +) + +// Storer is the interface that wraps the required methods to gather information +// about ingresses, services, secrets and ingress annotations. +type Storer interface { + // GetConfigMap returns a ConfigmMap using the namespace and name as key + GetConfigMap(key string) (*apiv1.ConfigMap, error) + + // GetSecret returns a Secret using the namespace and name as key + GetSecret(key string) (*apiv1.Secret, error) + + // GetService returns a Service using the namespace and name as key + GetService(key string) (*apiv1.Service, error) + + GetServiceEndpoints(svc *apiv1.Service) (*apiv1.Endpoints, error) + + // GetSecret returns an Ingress using the namespace and name as key + GetIngress(key string) (*extensions.Ingress, error) + + // ListIngresses returns the list of Ingresses + ListIngresses() []*extensions.Ingress + + // GetIngressAnnotations returns the annotations associated to an Ingress + GetIngressAnnotations(ing *extensions.Ingress) (*annotations.Ingress, error) + + // GetLocalSecret returns the local copy of a Secret + GetLocalSecret(name string) (*ingress.SSLCert, error) + + // ListLocalSecrets returns the list of local Secrets + ListLocalSecrets() []*ingress.SSLCert + + // GetAuthCertificate resolves a given secret name into an SSL certificate. + // The secret must contain 3 keys named: + // ca.crt: contains the certificate chain used for authentication + GetAuthCertificate(string) (*resolver.AuthSSLCert, error) + + // GetDefaultBackend returns the default backend configuration + GetDefaultBackend() defaults.Backend + + // SetDefaultBackend sets the default backend configuration + SetDefaultBackend(defaults.Backend) + + // Run initiates the synchronization of the controllers + Run(stopCh chan struct{}) +} + +// EventType type of event associated with an informer +type EventType string + +const ( + // CreateEvent event associated with new objects in an informer + CreateEvent EventType = "CREATE" + // UpdateEvent event associated with an object update in an informer + UpdateEvent EventType = "UPDATE" + // DeleteEvent event associated when an object is removed from an informer + DeleteEvent EventType = "DELETE" +) + +// Event holds the context of an event +type Event struct { + Type EventType + Obj interface{} +} + +// Lister returns the stores for ingresses, services, endpoints, secrets and configmaps. +type Lister struct { + Ingress IngressLister + Service ServiceLister + Endpoint EndpointLister + Secret SecretLister + ConfigMap ConfigMapLister + IngressAnnotation IngressAnnotationsLister +} + +// Controller defines the required controllers that interact agains the api server +type Controller struct { + Ingress cache.Controller + Endpoint cache.Controller + Service cache.Controller + Secret cache.Controller + Configmap cache.Controller +} + +// Run initiates the synchronization of the controllers against the api server +func (c *Controller) Run(stopCh chan struct{}) { + go c.Ingress.Run(stopCh) + go c.Endpoint.Run(stopCh) + go c.Service.Run(stopCh) + go c.Secret.Run(stopCh) + go c.Configmap.Run(stopCh) + + // wait for all involved caches to be synced, before processing items + // from the queue is started + if !cache.WaitForCacheSync(stopCh, + c.Ingress.HasSynced, + c.Endpoint.HasSynced, + c.Service.HasSynced, + c.Secret.HasSynced, + c.Configmap.HasSynced, + ) { + runtime.HandleError(fmt.Errorf("Timed out waiting for caches to sync")) + } +} + +// k8sStore internal Storer implementation using informers and thread safe stores +type k8sStore struct { + isOCSPCheckEnabled bool + + backendDefaults defaults.Backend + + cache *Controller + // listers + listers *Lister + + // sslStore local store of SSL certificates (certificates used in ingress) + // this is required because the certificates must be present in the + // container filesystem + sslStore *SSLCertTracker + + annotations annotations.Extractor +} + +// New creates a new object store to be used in the ingress controller +func New(checkOCSP bool, + namespace, configmap, tcp, udp string, + resyncPeriod time.Duration, + client clientset.Interface, + updateCh chan Event) Storer { + + store := &k8sStore{ + isOCSPCheckEnabled: checkOCSP, + cache: &Controller{}, + listers: &Lister{}, + sslStore: NewSSLCertTracker(), + } + + eventBroadcaster := record.NewBroadcaster() + eventBroadcaster.StartLogging(glog.Infof) + eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{ + Interface: client.CoreV1().Events(namespace), + }) + recorder := eventBroadcaster.NewRecorder(scheme.Scheme, apiv1.EventSource{ + Component: "nginx-ingress-controller", + }) + + // k8sStore fulfils resolver.Resolver interface + store.annotations = annotations.NewAnnotationExtractor(store) + + ingEventHandler := cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + addIng := obj.(*extensions.Ingress) + if !class.IsValid(addIng) { + a, _ := parser.GetStringAnnotation(class.IngressKey, addIng) + glog.Infof("ignoring add for ingress %v based on annotation %v with value %v", addIng.Name, class.IngressKey, a) + return + } + + store.extractAnnotations(addIng) + recorder.Eventf(addIng, apiv1.EventTypeNormal, "CREATE", fmt.Sprintf("Ingress %s/%s", addIng.Namespace, addIng.Name)) + updateCh <- Event{ + Type: CreateEvent, + Obj: obj, + } + }, + DeleteFunc: func(obj interface{}) { + delIng, ok := obj.(*extensions.Ingress) + if !ok { + // If we reached here it means the ingress was deleted but its final state is unrecorded. + tombstone, ok := obj.(cache.DeletedFinalStateUnknown) + if !ok { + glog.Errorf("couldn't get object from tombstone %#v", obj) + return + } + delIng, ok = tombstone.Obj.(*extensions.Ingress) + if !ok { + glog.Errorf("Tombstone contained object that is not an Ingress: %#v", obj) + return + } + } + if !class.IsValid(delIng) { + glog.Infof("ignoring delete for ingress %v based on annotation %v", delIng.Name, class.IngressKey) + return + } + recorder.Eventf(delIng, apiv1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", delIng.Namespace, delIng.Name)) + store.listers.IngressAnnotation.Delete(delIng) + updateCh <- Event{ + Type: DeleteEvent, + Obj: obj, + } + }, + UpdateFunc: func(old, cur interface{}) { + oldIng := old.(*extensions.Ingress) + curIng := cur.(*extensions.Ingress) + validOld := class.IsValid(oldIng) + validCur := class.IsValid(curIng) + if !validOld && validCur { + glog.Infof("creating ingress %v based on annotation %v", curIng.Name, class.IngressKey) + recorder.Eventf(curIng, apiv1.EventTypeNormal, "CREATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) + } else if validOld && !validCur { + glog.Infof("removing ingress %v based on annotation %v", curIng.Name, class.IngressKey) + recorder.Eventf(curIng, apiv1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) + } else if validCur && !reflect.DeepEqual(old, cur) { + recorder.Eventf(curIng, apiv1.EventTypeNormal, "UPDATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) + } + + store.extractAnnotations(curIng) + updateCh <- Event{ + Type: UpdateEvent, + Obj: cur, + } + }, + } + + secrEventHandler := cache.ResourceEventHandlerFuncs{ + UpdateFunc: func(old, cur interface{}) { + if !reflect.DeepEqual(old, cur) { + sec := cur.(*apiv1.Secret) + _, exists := store.sslStore.Get(k8s.MetaNamespaceKey(sec)) + if exists { + updateCh <- Event{ + Type: UpdateEvent, + Obj: cur, + } + } + } + }, + DeleteFunc: func(obj interface{}) { + sec, ok := obj.(*apiv1.Secret) + if !ok { + // If we reached here it means the secret was deleted but its final state is unrecorded. + tombstone, ok := obj.(cache.DeletedFinalStateUnknown) + if !ok { + glog.Errorf("couldn't get object from tombstone %#v", obj) + return + } + sec, ok = tombstone.Obj.(*apiv1.Secret) + if !ok { + glog.Errorf("Tombstone contained object that is not a Secret: %#v", obj) + return + } + } + store.sslStore.Delete(k8s.MetaNamespaceKey(sec)) + updateCh <- Event{ + Type: DeleteEvent, + Obj: obj, + } + }, + } + + eventHandler := cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + updateCh <- Event{ + Type: CreateEvent, + Obj: obj, + } + }, + DeleteFunc: func(obj interface{}) { + updateCh <- Event{ + Type: DeleteEvent, + Obj: obj, + } + }, + UpdateFunc: func(old, cur interface{}) { + oep := old.(*apiv1.Endpoints) + ocur := cur.(*apiv1.Endpoints) + if !reflect.DeepEqual(ocur.Subsets, oep.Subsets) { + updateCh <- Event{ + Type: UpdateEvent, + Obj: cur, + } + } + }, + } + + mapEventHandler := cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + upCmap := obj.(*apiv1.ConfigMap) + mapKey := fmt.Sprintf("%s/%s", upCmap.Namespace, upCmap.Name) + if mapKey == configmap { + glog.V(2).Infof("adding configmap %v to backend", mapKey) + updateCh <- Event{ + Type: CreateEvent, + Obj: obj, + } + } + }, + UpdateFunc: func(old, cur interface{}) { + if !reflect.DeepEqual(old, cur) { + upCmap := cur.(*apiv1.ConfigMap) + mapKey := fmt.Sprintf("%s/%s", upCmap.Namespace, upCmap.Name) + if mapKey == configmap { + glog.V(2).Infof("updating configmap backend (%v)", mapKey) + updateCh <- Event{ + Type: UpdateEvent, + Obj: cur, + } + } + // updates to configuration configmaps can trigger an update + if mapKey == configmap || mapKey == tcp || mapKey == udp { + recorder.Eventf(upCmap, apiv1.EventTypeNormal, "UPDATE", fmt.Sprintf("ConfigMap %v", mapKey)) + updateCh <- Event{ + Type: UpdateEvent, + Obj: cur, + } + } + } + }, + } + + store.listers.IngressAnnotation.Store = cache_client.NewStore(cache_client.DeletionHandlingMetaNamespaceKeyFunc) + + store.listers.Ingress.Store, store.cache.Ingress = cache.NewInformer( + cache.NewListWatchFromClient(client.ExtensionsV1beta1().RESTClient(), "ingresses", namespace, fields.Everything()), + &extensions.Ingress{}, resyncPeriod, ingEventHandler) + + store.listers.Endpoint.Store, store.cache.Endpoint = cache.NewInformer( + cache.NewListWatchFromClient(client.CoreV1().RESTClient(), "endpoints", namespace, fields.Everything()), + &apiv1.Endpoints{}, resyncPeriod, eventHandler) + + store.listers.Secret.Store, store.cache.Secret = cache.NewInformer( + cache.NewListWatchFromClient(client.CoreV1().RESTClient(), "secrets", namespace, fields.Everything()), + &apiv1.Secret{}, resyncPeriod, secrEventHandler) + + store.listers.ConfigMap.Store, store.cache.Configmap = cache.NewInformer( + cache.NewListWatchFromClient(client.CoreV1().RESTClient(), "configmaps", namespace, fields.Everything()), + &apiv1.ConfigMap{}, resyncPeriod, mapEventHandler) + + store.listers.Service.Store, store.cache.Service = cache.NewInformer( + cache.NewListWatchFromClient(client.CoreV1().RESTClient(), "services", namespace, fields.Everything()), + &apiv1.Service{}, resyncPeriod, cache.ResourceEventHandlerFuncs{}) + + return store +} + +func (s k8sStore) extractAnnotations(ing *extensions.Ingress) { + anns := s.annotations.Extract(ing) + glog.V(3).Infof("updating annotations information for ingres %v/%v", anns.Namespace, anns.Name) + err := s.listers.IngressAnnotation.Update(anns) + if err != nil { + glog.Error(err) + } +} + +// GetSecret returns a Secret using the namespace and name as key +func (s k8sStore) GetSecret(key string) (*apiv1.Secret, error) { + return s.listers.Secret.ByKey(key) +} + +// ListLocalSecrets returns the list of local Secrets +func (s k8sStore) ListLocalSecrets() []*ingress.SSLCert { + var certs []*ingress.SSLCert + for _, item := range s.sslStore.List() { + if s, ok := item.(*ingress.SSLCert); ok { + certs = append(certs, s) + } + } + + return certs +} + +// GetService returns a Service using the namespace and name as key +func (s k8sStore) GetService(key string) (*apiv1.Service, error) { + return s.listers.Service.ByKey(key) +} + +// GetSecret returns an Ingress using the namespace and name as key +func (s k8sStore) GetIngress(key string) (*extensions.Ingress, error) { + return s.listers.Ingress.ByKey(key) +} + +// ListIngresses returns the list of Ingresses +func (s k8sStore) ListIngresses() []*extensions.Ingress { + // filter ingress rules + var ingresses []*extensions.Ingress + for _, item := range s.listers.Ingress.List() { + ing := item.(*extensions.Ingress) + if !class.IsValid(ing) { + continue + } + + ingresses = append(ingresses, ing) + } + + return ingresses +} + +// GetIngressAnnotations returns the annotations associated to an Ingress +func (s k8sStore) GetIngressAnnotations(ing *extensions.Ingress) (*annotations.Ingress, error) { + key := fmt.Sprintf("%v/%v", ing.Namespace, ing.Name) + item, exists, err := s.listers.IngressAnnotation.GetByKey(key) + if err != nil { + return nil, fmt.Errorf("unexpected error getting ingress annotation %v: %v", key, err) + } + if !exists { + return nil, fmt.Errorf("ingress annotation %v was not found", key) + } + return item.(*annotations.Ingress), nil +} + +// GetLocalSecret returns the local copy of a Secret +func (s k8sStore) GetLocalSecret(key string) (*ingress.SSLCert, error) { + return s.sslStore.ByKey(key) +} + +func (s k8sStore) GetConfigMap(key string) (*apiv1.ConfigMap, error) { + return s.listers.ConfigMap.ByKey(key) +} + +func (s k8sStore) GetServiceEndpoints(svc *apiv1.Service) (*apiv1.Endpoints, error) { + return s.listers.Endpoint.GetServiceEndpoints(svc) +} + +// GetAuthCertificate is used by the auth-tls annotations to get a cert from a secret +func (s k8sStore) GetAuthCertificate(name string) (*resolver.AuthSSLCert, error) { + if _, err := s.GetLocalSecret(name); err != nil { + s.syncSecret(name) + } + + cert, err := s.GetLocalSecret(name) + if err != nil { + return nil, err + } + + return &resolver.AuthSSLCert{ + Secret: name, + CAFileName: cert.CAFileName, + PemSHA: cert.PemSHA, + }, nil +} + +// GetDefaultBackend returns the default backend +func (s k8sStore) GetDefaultBackend() defaults.Backend { + return s.backendDefaults +} + +func (s *k8sStore) SetDefaultBackend(bd defaults.Backend) { + s.backendDefaults = bd +} + +// Run initiates the synchronization of the controllers +// and the initial synchronization of the secrets. +func (s k8sStore) Run(stopCh chan struct{}) { + // start controllers + s.cache.Run(stopCh) + + // initial sync of secrets to avoid unnecessary reloads + glog.Info("running initial sync of secrets") + for _, ing := range s.ListIngresses() { + s.readSecrets(ing) + } + + // start goroutine to check for missing local secrets + go wait.Until(s.checkMissingSecrets, 30*time.Second, stopCh) + + if s.isOCSPCheckEnabled { + go wait.Until(s.checkSSLChainIssues, 60*time.Second, stopCh) + } +} diff --git a/internal/ingress/store/store_test.go b/internal/ingress/store/store_test.go new file mode 100644 index 000000000..d0d4c2c2d --- /dev/null +++ b/internal/ingress/store/store_test.go @@ -0,0 +1,315 @@ +/* +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 store + +import ( + "fmt" + "os" + "sync/atomic" + "testing" + "time" + + apiv1 "k8s.io/api/core/v1" + "k8s.io/api/extensions/v1beta1" + extensions "k8s.io/api/extensions/v1beta1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/kubernetes" + + "k8s.io/ingress-nginx/test/e2e/framework" +) + +func TestStore(t *testing.T) { + // TODO: find a way to avoid the need to use a real api server + home := os.Getenv("HOME") + kubeConfigFile := fmt.Sprintf("%v/.kube/config", home) + kubeContext := "" + + kubeConfig, err := framework.LoadConfig(kubeConfigFile, kubeContext) + if err != nil { + t.Errorf("unexpected error loading kubeconfig file: %v", err) + } + + clientSet, err := kubernetes.NewForConfig(kubeConfig) + if err != nil { + t.Errorf("unexpected error creating ingress client: %v", err) + } + + t.Run("should return an error searching for non existing objects", func(t *testing.T) { + ns := createNamespace(clientSet, t) + defer deleteNamespace(ns, clientSet, t) + + stopCh := make(chan struct{}) + defer close(stopCh) + + updateCh := make(chan Event) + defer close(updateCh) + + go func(ch chan Event) { + for { + <-ch + } + }(updateCh) + + storer := New(true, + ns.Name, + fmt.Sprintf("%v/config", ns.Name), + fmt.Sprintf("%v/tcp", ns.Name), + fmt.Sprintf("%v/udp", ns.Name), + 10*time.Minute, + clientSet, + updateCh) + + storer.Run(stopCh) + + key := fmt.Sprintf("%v/anything", ns.Name) + ing, err := storer.GetIngress(key) + if err == nil { + t.Errorf("expected an error but none returned") + } + if ing != nil { + t.Errorf("expected an Ingres but none returned") + } + + ls, err := storer.GetLocalSecret(key) + if err == nil { + t.Errorf("expected an error but none returned") + } + if ls != nil { + t.Errorf("expected an Ingres but none returned") + } + + s, err := storer.GetSecret(key) + if err == nil { + t.Errorf("expected an error but none returned") + } + if s != nil { + t.Errorf("expected an Ingres but none returned") + } + + svc, err := storer.GetService(key) + if err == nil { + t.Errorf("expected an error but none returned") + } + if svc != nil { + t.Errorf("expected an Ingres but none returned") + } + }) + + t.Run("should return ingress one event for add, update and delete", func(t *testing.T) { + ns := createNamespace(clientSet, t) + defer deleteNamespace(ns, clientSet, t) + + stopCh := make(chan struct{}) + defer close(stopCh) + + updateCh := make(chan Event) + defer close(updateCh) + + var add uint64 + var upd uint64 + var del uint64 + + go func(ch chan Event) { + for { + e := <-ch + if e.Obj == nil { + continue + } + if _, ok := e.Obj.(*extensions.Ingress); !ok { + t.Errorf("expected an Ingress type but %T returned", e.Obj) + } + switch e.Type { + case CreateEvent: + atomic.AddUint64(&add, 1) + break + case UpdateEvent: + atomic.AddUint64(&upd, 1) + break + case DeleteEvent: + atomic.AddUint64(&del, 1) + break + } + } + }(updateCh) + + storer := New(true, + ns.Name, + fmt.Sprintf("%v/config", ns.Name), + fmt.Sprintf("%v/tcp", ns.Name), + fmt.Sprintf("%v/udp", ns.Name), + 10*time.Minute, + clientSet, + updateCh) + + storer.Run(stopCh) + + ing, err := ensureIngress(&v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy", + Namespace: ns.Name, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: "dummy", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Path: "/", + Backend: v1beta1.IngressBackend{ + ServiceName: "http-svc", + ServicePort: intstr.FromInt(80), + }, + }, + }, + }, + }, + }, + }, + }, + }, clientSet) + if err != nil { + t.Errorf("unexpected error creating ingress: %v", err) + } + + // create an invalid ingress (different class) + _, err = ensureIngress(&v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "custom-class", + Namespace: ns.Name, + Annotations: map[string]string{ + "kubernetes.io/ingress.class": "something", + }, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: "dummy", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Path: "/", + Backend: v1beta1.IngressBackend{ + ServiceName: "http-svc", + ServicePort: intstr.FromInt(80), + }, + }, + }, + }, + }, + }, + }, + }, + }, clientSet) + if err != nil { + t.Errorf("unexpected error creating ingress: %v", err) + } + + ni := ing.DeepCopy() + ni.Spec.Rules[0].Host = "update-dummy" + _, err = ensureIngress(ni, clientSet) + if err != nil { + t.Errorf("unexpected error creating ingress: %v", err) + } + + err = clientSet.ExtensionsV1beta1(). + Ingresses(ni.Namespace). + Delete(ni.Name, &metav1.DeleteOptions{}) + if err != nil { + t.Errorf("unexpected error creating ingress: %v", err) + } + + waitForNoIngressInNamespace(clientSet, ni.Namespace, ni.Name) + + if atomic.LoadUint64(&add) != 1 { + t.Errorf("expected 1 event of type Create but %v ocurred", add) + } + if atomic.LoadUint64(&upd) != 1 { + t.Errorf("expected 1 event of type Update but %v ocurred", upd) + } + if atomic.LoadUint64(&del) != 1 { + t.Errorf("expected 1 event of type Delete but %v ocurred", del) + } + }) + + // test add secret no referenced from ingress + // test add ingress with secret it doesn't exists + // test add ingress with secret it doesn't exists and then add secret + // check secret is generated on fs + // check ocsp + // check invalid secret (missing crt) + // check invalid secret (missing key) + // check invalid secret (missing ca) +} + +func createNamespace(clientSet *kubernetes.Clientset, t *testing.T) *apiv1.Namespace { + t.Log("creating temporal namespace") + ns, err := framework.CreateKubeNamespace("store-test", clientSet) + if err != nil { + t.Errorf("unexpected error creating ingress client: %v", err) + } + t.Logf("temporal namespace %v created", ns.Name) + + return ns +} + +func deleteNamespace(ns *apiv1.Namespace, clientSet *kubernetes.Clientset, t *testing.T) { + t.Logf("deleting temporal namespace %v created", ns.Name) + err := framework.DeleteKubeNamespace(clientSet, ns.Name) + if err != nil { + t.Errorf("unexpected error creating ingress client: %v", err) + } + t.Logf("temporal namespace %v deleted", ns.Name) +} + +func ensureIngress(ingress *extensions.Ingress, clientSet *kubernetes.Clientset) (*extensions.Ingress, error) { + s, err := clientSet.ExtensionsV1beta1().Ingresses(ingress.Namespace).Update(ingress) + if err != nil { + if k8sErrors.IsNotFound(err) { + return clientSet.ExtensionsV1beta1().Ingresses(ingress.Namespace).Create(ingress) + } + return nil, err + } + return s, nil +} + +func waitForNoIngressInNamespace(c kubernetes.Interface, namespace, name string) error { + return wait.PollImmediate(1*time.Second, time.Minute*2, noIngressInNamespace(c, namespace, name)) +} + +func noIngressInNamespace(c kubernetes.Interface, namespace, name string) wait.ConditionFunc { + return func() (bool, error) { + ing, err := c.ExtensionsV1beta1().Ingresses(namespace).Get(name, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + return true, nil + } + if err != nil { + return false, err + } + + if ing == nil { + return true, nil + } + return false, nil + } +} diff --git a/internal/ingress/types.go b/internal/ingress/types.go index d9da68b2f..6798a49d0 100644 --- a/internal/ingress/types.go +++ b/internal/ingress/types.go @@ -33,7 +33,6 @@ import ( "k8s.io/ingress-nginx/internal/ingress/annotations/redirect" "k8s.io/ingress-nginx/internal/ingress/annotations/rewrite" "k8s.io/ingress-nginx/internal/ingress/resolver" - "k8s.io/ingress-nginx/internal/ingress/store" ) var ( @@ -44,17 +43,6 @@ var ( DefaultSSLDirectory = "/ingress-controller/ssl" ) -// StoreLister returns the configured stores for ingresses, services, -// endpoints, secrets and configmaps. -type StoreLister struct { - Ingress store.IngressLister - Service store.ServiceLister - Endpoint store.EndpointLister - Secret store.SecretLister - ConfigMap store.ConfigMapLister - IngressAnnotation store.IngressAnnotationsLister -} - // Configuration holds the definition of all the parts required to describe all // ingresses reachable by the ingress controller (using a filter by namespace) type Configuration struct { diff --git a/internal/k8s/main.go b/internal/k8s/main.go index b391cad69..7a1f2f9c2 100644 --- a/internal/k8s/main.go +++ b/internal/k8s/main.go @@ -21,9 +21,12 @@ import ( "os" "strings" + "github.com/golang/glog" + apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/cache" ) // ParseNameNS parses a string searching a namespace and name @@ -96,3 +99,13 @@ func GetPodDetails(kubeClient clientset.Interface) (*PodInfo, error) { Labels: pod.GetLabels(), }, nil } + +// MetaNamespaceKey knows how to make keys for API objects which implement meta.Interface. +func MetaNamespaceKey(obj interface{}) string { + key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(obj) + if err != nil { + glog.Warning(err) + } + + return key +} diff --git a/test/e2e/up.sh b/test/e2e/up.sh index fd4839fb9..e98b155d6 100755 --- a/test/e2e/up.sh +++ b/test/e2e/up.sh @@ -35,47 +35,3 @@ until kubectl get nodes -o jsonpath="$JSONPATH" 2>&1 | grep -q "Ready=True"; do sleep 1; done - -echo "deploying NGINX Ingress controller" -cat deploy/namespace.yaml | kubectl apply -f - -cat deploy/default-backend.yaml | kubectl apply -f - -cat deploy/configmap.yaml | kubectl apply -f - -cat deploy/tcp-services-configmap.yaml | kubectl apply -f - -cat deploy/udp-services-configmap.yaml | kubectl apply -f - -cat deploy/without-rbac.yaml | kubectl apply -f - -cat deploy/provider/baremetal/service-nodeport.yaml | kubectl apply -f - - -echo "updating image..." -kubectl set image \ - deployments \ - --namespace ingress-nginx \ - --selector app=ingress-nginx \ - nginx-ingress-controller=quay.io/kubernetes-ingress-controller/nginx-ingress-controller:test - -sleep 5 - -echo "waiting NGINX ingress pod..." - -function waitForPod() { - until kubectl get pods -n ingress-nginx -l app=ingress-nginx -o jsonpath="$JSONPATH" 2>&1 | grep -q "Ready=True"; - do - sleep 1; - done -} - -export -f waitForPod - -timeout 10s bash -c waitForPod - -if kubectl get pods -n ingress-nginx -l app=ingress-nginx -o jsonpath="$JSONPATH" 2>&1 | grep -q "Ready=True"; -then - echo "Kubernetes deployments started" -else - echo "Kubernetes deployments with issues:" - kubectl get pods -n ingress-nginx - - echo "Reason:" - kubectl describe pods -n ingress-nginx - kubectl logs -n ingress-nginx -l app=ingress-nginx - exit 1 -fi diff --git a/test/e2e/wait-for-nginx.sh b/test/e2e/wait-for-nginx.sh new file mode 100755 index 000000000..19e4d5ebb --- /dev/null +++ b/test/e2e/wait-for-nginx.sh @@ -0,0 +1,61 @@ +#!/bin/bash + +# 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. + +export JSONPATH='{range .items[*]}{@.metadata.name}:{range @.status.conditions[*]}{@.type}={@.status};{end}{end}' + +echo "deploying NGINX Ingress controller" +cat deploy/namespace.yaml | kubectl apply -f - +cat deploy/default-backend.yaml | kubectl apply -f - +cat deploy/configmap.yaml | kubectl apply -f - +cat deploy/tcp-services-configmap.yaml | kubectl apply -f - +cat deploy/udp-services-configmap.yaml | kubectl apply -f - +cat deploy/without-rbac.yaml | kubectl apply -f - +cat deploy/provider/baremetal/service-nodeport.yaml | kubectl apply -f - + +echo "updating image..." +kubectl set image \ + deployments \ + --namespace ingress-nginx \ + --selector app=ingress-nginx \ + nginx-ingress-controller=quay.io/kubernetes-ingress-controller/nginx-ingress-controller:test + +sleep 5 + +echo "waiting NGINX ingress pod..." + +function waitForPod() { + until kubectl get pods -n ingress-nginx -l app=ingress-nginx -o jsonpath="$JSONPATH" 2>&1 | grep -q "Ready=True"; + do + sleep 1; + done +} + +export -f waitForPod + +timeout 10s bash -c waitForPod + +if kubectl get pods -n ingress-nginx -l app=ingress-nginx -o jsonpath="$JSONPATH" 2>&1 | grep -q "Ready=True"; +then + echo "Kubernetes deployments started" +else + echo "Kubernetes deployments with issues:" + kubectl get pods -n ingress-nginx + + echo "Reason:" + kubectl describe pods -n ingress-nginx + kubectl logs -n ingress-nginx -l app=ingress-nginx + exit 1 +fi