From f055022e58e7c295e4bf2950584ed56cc2a99bf9 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 23 Nov 2017 13:46:23 -0300 Subject: [PATCH] Simplify annotations --- Makefile | 10 +++--- cmd/nginx/flags.go | 4 ++- internal/ingress/annotations/alias/main.go | 2 +- .../ingress/annotations/alias/main_test.go | 3 +- .../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 | 8 ++--- .../ingress/annotations/authreq/main_test.go | 13 ++++---- internal/ingress/annotations/authtls/main.go | 10 +++--- .../annotations/clientbodybuffersize/main.go | 2 +- .../clientbodybuffersize/main_test.go | 3 +- internal/ingress/annotations/cors/main.go | 10 +++--- .../ingress/annotations/cors/main_test.go | 11 ++++--- .../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 | 2 +- .../annotations/serversnippet/main_test.go | 3 +- .../annotations/serviceupstream/main.go | 2 +- .../annotations/serviceupstream/main_test.go | 5 +-- .../annotations/sessionaffinity/main.go | 6 ++-- .../annotations/sessionaffinity/main_test.go | 8 ++--- internal/ingress/annotations/snippet/main.go | 2 +- .../ingress/annotations/snippet/main_test.go | 3 +- .../annotations/sslpassthrough/main.go | 2 +- .../annotations/sslpassthrough/main_test.go | 3 +- .../annotations/upstreamhashby/main.go | 2 +- .../annotations/upstreamhashby/main_test.go | 3 +- .../ingress/annotations/upstreamvhost/main.go | 2 +- .../ingress/annotations/vtsfilterkey/main.go | 2 +- internal/ingress/controller/backend_ssl.go | 2 +- internal/ingress/controller/controller.go | 2 +- internal/ingress/controller/listers.go | 2 +- internal/ingress/controller/nginx.go | 2 +- test/e2e/defaultbackend/default_backend.go | 2 -- 51 files changed, 195 insertions(+), 175 deletions(-) diff --git a/Makefile b/Makefile index 73ccea0c2..59bd12a4b 100644 --- a/Makefile +++ b/Makefile @@ -133,12 +133,12 @@ endif clean: $(DOCKER) rmi -f $(MULTI_ARCH_IMG):$(TAG) || true -.PHONE: gobindata -gobindata: +.PHONE: code-generator +code-generator: go-bindata -o internal/file/bindata.go -prefix="rootfs" -pkg=file -ignore=Dockerfile -ignore=".DS_Store" rootfs/... .PHONY: build -build: clean gobindata +build: clean CGO_ENABLED=0 GOOS=${GOOS} GOARCH=${GOARCH} go build -a -installsuffix cgo \ -ldflags "-s -w -X ${PKG}/version.RELEASE=${TAG} -X ${PKG}/version.COMMIT=${COMMIT} -X ${PKG}/version.REPO=${REPO_INFO}" \ -o ${TEMP_DIR}/rootfs/nginx-ingress-controller ${PKG}/cmd/nginx @@ -154,7 +154,7 @@ lint: @go list -f '{{if len .TestGoFiles}}"golint {{.Dir}}/..."{{end}}' $(shell go list ${PKG}/... | grep -v vendor | grep -v '/test/e2e') | xargs -L 1 sh -c .PHONY: test -test: fmt lint vet gobindata +test: fmt lint vet @echo "+ $@" @go test -v -race -tags "$(BUILDTAGS) cgo" $(shell go list ${PKG}/... | grep -v vendor | grep -v '/test/e2e') @@ -169,7 +169,7 @@ e2e-test: @KUBECONFIG=${HOME}/.kube/config INGRESSNGINXCONFIG=${HOME}/.kube/config ./e2e-tests .PHONY: cover -cover: gobindata +cover: @echo "+ $@" @go list -f '{{if len .TestGoFiles}}"go test -coverprofile={{.Dir}}/.coverprofile {{.ImportPath}}"{{end}}' $(shell go list ${PKG}/... | grep -v vendor | grep -v '/test/e2e') | xargs -L 1 sh -c gover diff --git a/cmd/nginx/flags.go b/cmd/nginx/flags.go index b2446f821..d80afb714 100644 --- a/cmd/nginx/flags.go +++ b/cmd/nginx/flags.go @@ -28,6 +28,7 @@ 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" @@ -156,6 +157,8 @@ func parseFlags() (bool, *controller.Configuration, error) { 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) @@ -187,7 +190,6 @@ func parseFlags() (bool, *controller.Configuration, error) { } config := &controller.Configuration{ - AnnotationsPrefix: *annotationsPrefix, APIServerHost: *apiserverHost, KubeConfigFile: *kubeConfigFile, UpdateStatus: *updateStatus, diff --git a/internal/ingress/annotations/alias/main.go b/internal/ingress/annotations/alias/main.go index 2fb81b2a4..449d64b11 100644 --- a/internal/ingress/annotations/alias/main.go +++ b/internal/ingress/annotations/alias/main.go @@ -35,5 +35,5 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // 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_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..43f936138 100644 --- a/internal/ingress/annotations/authreq/main.go +++ b/internal/ingress/annotations/authreq/main.go @@ -112,7 +112,7 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // 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 +121,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 +138,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..fc9b1e9ae 100644 --- a/internal/ingress/annotations/authreq/main_test.go +++ b/internal/ingress/annotations/authreq/main_test.go @@ -24,6 +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/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/resolver" "k8s.io/apimachinery/pkg/util/intstr" @@ -87,9 +88,9 @@ 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) if test.expErr { @@ -137,9 +138,9 @@ 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) if test.expErr { 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/clientbodybuffersize/main.go b/internal/ingress/annotations/clientbodybuffersize/main.go index 6ff3e070a..41cce30ba 100644 --- a/internal/ingress/annotations/clientbodybuffersize/main.go +++ b/internal/ingress/annotations/clientbodybuffersize/main.go @@ -35,5 +35,5 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // 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..cc261fb63 100644 --- a/internal/ingress/annotations/clientbodybuffersize/main_test.go +++ b/internal/ingress/annotations/clientbodybuffersize/main_test.go @@ -22,11 +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/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/resolver" ) func TestParse(t *testing.T) { - annotation := "nginx/client-body-buffer-size" + annotation := parser.GetAnnotationWithPrefix("client-body-buffer-size") ap := NewParser(&resolver.Mock{}) 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..a23a1c5be 100644 --- a/internal/ingress/annotations/cors/main.go +++ b/internal/ingress/annotations/cors/main.go @@ -92,27 +92,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..c8a195294 100644 --- a/internal/ingress/annotations/cors/main_test.go +++ b/internal/ingress/annotations/cors/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/resolver" ) @@ -65,11 +66,11 @@ 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) 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..82bed533f 100644 --- a/internal/ingress/annotations/serversnippet/main.go +++ b/internal/ingress/annotations/serversnippet/main.go @@ -36,5 +36,5 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // 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..1e18228f0 100644 --- a/internal/ingress/annotations/serversnippet/main_test.go +++ b/internal/ingress/annotations/serversnippet/main_test.go @@ -22,11 +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/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/resolver" ) func TestParse(t *testing.T) { - annotation := "nginx/server-snippet" + annotation := parser.GetAnnotationWithPrefix("server-snippet") ap := NewParser(&resolver.Mock{}) if ap == nil { diff --git a/internal/ingress/annotations/serviceupstream/main.go b/internal/ingress/annotations/serviceupstream/main.go index a8386edb6..9f9a41a35 100644 --- a/internal/ingress/annotations/serviceupstream/main.go +++ b/internal/ingress/annotations/serviceupstream/main.go @@ -33,5 +33,5 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { } 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..6c2bcc2b4 100644 --- a/internal/ingress/annotations/serviceupstream/main_test.go +++ b/internal/ingress/annotations/serviceupstream/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/resolver" ) @@ -65,7 +66,7 @@ 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) @@ -84,7 +85,7 @@ 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) 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..c21fbad4d 100644 --- a/internal/ingress/annotations/snippet/main.go +++ b/internal/ingress/annotations/snippet/main.go @@ -36,5 +36,5 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // 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..e197547cb 100644 --- a/internal/ingress/annotations/snippet/main_test.go +++ b/internal/ingress/annotations/snippet/main_test.go @@ -22,11 +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/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/resolver" ) func TestParse(t *testing.T) { - annotation := "nginx/configuration-snippet" + annotation := parser.GetAnnotationWithPrefix("configuration-snippet") ap := NewParser(&resolver.Mock{}) if ap == nil { diff --git a/internal/ingress/annotations/sslpassthrough/main.go b/internal/ingress/annotations/sslpassthrough/main.go index 82b69a170..8ab5ab066 100644 --- a/internal/ingress/annotations/sslpassthrough/main.go +++ b/internal/ingress/annotations/sslpassthrough/main.go @@ -40,5 +40,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..575d33894 100644 --- a/internal/ingress/annotations/sslpassthrough/main_test.go +++ b/internal/ingress/annotations/sslpassthrough/main_test.go @@ -22,6 +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/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/resolver" "k8s.io/apimachinery/pkg/util/intstr" @@ -51,7 +52,7 @@ func TestParseAnnotations(t *testing.T) { } 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) diff --git a/internal/ingress/annotations/upstreamhashby/main.go b/internal/ingress/annotations/upstreamhashby/main.go index b543070a0..bfeb5afa5 100644 --- a/internal/ingress/annotations/upstreamhashby/main.go +++ b/internal/ingress/annotations/upstreamhashby/main.go @@ -36,5 +36,5 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // 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..0f4cee518 100644 --- a/internal/ingress/annotations/upstreamhashby/main_test.go +++ b/internal/ingress/annotations/upstreamhashby/main_test.go @@ -22,11 +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/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/resolver" ) func TestParse(t *testing.T) { - annotation := "nginx/upstream-hash-by" + annotation := parser.GetAnnotationWithPrefix("upstream-hash-by") ap := NewParser(&resolver.Mock{}) if ap == nil { diff --git a/internal/ingress/annotations/upstreamvhost/main.go b/internal/ingress/annotations/upstreamvhost/main.go index 02c1e96cf..e11fe7914 100644 --- a/internal/ingress/annotations/upstreamvhost/main.go +++ b/internal/ingress/annotations/upstreamvhost/main.go @@ -36,5 +36,5 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // 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..e349c36a6 100644 --- a/internal/ingress/annotations/vtsfilterkey/main.go +++ b/internal/ingress/annotations/vtsfilterkey/main.go @@ -36,5 +36,5 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // used to indicate 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/backend_ssl.go b/internal/ingress/controller/backend_ssl.go index 9f95bc911..c48a34d54 100644 --- a/internal/ingress/controller/backend_ssl.go +++ b/internal/ingress/controller/backend_ssl.go @@ -187,7 +187,7 @@ func (ic *NGINXController) checkMissingSecrets() { } } - key, _ := parser.GetStringAnnotation("auth-tls-secret", ing, ic) + key, _ := parser.GetStringAnnotation("auth-tls-secret", ing) if key == "" { continue } diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 252406277..f766913e9 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -1159,7 +1159,7 @@ func (n *NGINXController) readSecrets(ing *extensions.Ingress) { n.syncSecret(key) } - key, _ := parser.GetStringAnnotation("auth-tls-secret", ing, n) + key, _ := parser.GetStringAnnotation("auth-tls-secret", ing) if key == "" { return } diff --git a/internal/ingress/controller/listers.go b/internal/ingress/controller/listers.go index a225b7b3e..568158c98 100644 --- a/internal/ingress/controller/listers.go +++ b/internal/ingress/controller/listers.go @@ -66,7 +66,7 @@ func (n *NGINXController) createListers(stopCh chan struct{}) (*ingress.StoreLis AddFunc: func(obj interface{}) { addIng := obj.(*extensions.Ingress) if !class.IsValid(addIng) { - a, _ := parser.GetStringAnnotation(class.IngressKey, addIng, n) + 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 } diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 2d7ababb7..b1850bea4 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -257,7 +257,7 @@ func (n *NGINXController) Start() { ing := obj.(*extensions.Ingress) if !class.IsValid(ing) { - a, _ := parser.GetStringAnnotation(class.IngressKey, ing, n) + a, _ := parser.GetStringAnnotation(class.IngressKey, ing) glog.Infof("ignoring add for ingress %v based on annotation %v with value %v", ing.Name, class.IngressKey, a) continue } diff --git a/test/e2e/defaultbackend/default_backend.go b/test/e2e/defaultbackend/default_backend.go index 824a70609..4e552630d 100644 --- a/test/e2e/defaultbackend/default_backend.go +++ b/test/e2e/defaultbackend/default_backend.go @@ -77,7 +77,6 @@ var _ = framework.IngressNginxDescribe("Default backend", func() { switch test.Scheme { case framework.HTTP: cm = request.CustomMethod(test.Method, f.NginxHTTPURL) - break case framework.HTTPS: cm = request.CustomMethod(test.Method, f.NginxHTTPSURL) // the default backend uses a self generated certificate @@ -86,7 +85,6 @@ var _ = framework.IngressNginxDescribe("Default backend", func() { InsecureSkipVerify: true, }, } - break default: Fail("Unexpected request scheme") }