Merge pull request #1745 from aledbf/simplify-annotations

Simplify annotations

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

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,

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)
 }

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{})

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 {

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)

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)

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, ",")

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 {

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
 	}

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)
 }

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")

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
 	}

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)

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
 	}

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
 	}