Annotations cannot being empty

internal/ingress/annotations/authreq/main.go

@@ -123,9 +123,6 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) {
 	if err != nil {
 		return nil, err
 	}
-	if urlString == "" {
-		return nil, ing_errors.NewLocationDenied("an empty string is not a valid URL")
-	}
 
 	authURL, err := url.Parse(urlString)
 	if err != nil {

internal/ingress/annotations/authreq/main_test.go

@@ -105,6 +105,10 @@ func TestAnnotations(t *testing.T) {
 			}
 			continue
 		}
+		if err != nil {
+			t.Errorf("%v: unexpected error: %v", test.title, err)
+		}
+
 		u, ok := i.(*Config)
 		if !ok {
 			t.Errorf("%v: expected an External type", test.title)

internal/ingress/annotations/authtls/main.go

@@ -95,10 +95,6 @@ func (a authTLS) Parse(ing *extensions.Ingress) (interface{}, error) {
 		return &Config{}, err
 	}
 
-	if tlsauthsecret == "" {
-		return &Config{}, ing_errors.NewLocationDenied("an empty string is not a valid secret name")
-	}
-
 	_, _, err = k8s.ParseNameNS(tlsauthsecret)
 	if err != nil {
 		return &Config{}, ing_errors.NewLocationDenied(err.Error())
@@ -122,7 +118,7 @@ func (a authTLS) Parse(ing *extensions.Ingress) (interface{}, error) {
 	}
 
 	config.ErrorPage, err = parser.GetStringAnnotation("auth-tls-error-page", ing)
-	if err != nil || config.ErrorPage == "" {
+	if err != nil {
 		config.ErrorPage = ""
 	}
 

internal/ingress/annotations/connection/main_test.go

@@ -38,7 +38,6 @@ func TestParse(t *testing.T) {
 		annotations map[string]string
 		expected    *Config
 	}{
-		{map[string]string{annotation: ""}, &Config{Enabled: true, Header: ""}},
 		{map[string]string{annotation: "keep-alive"}, &Config{Enabled: true, Header: "keep-alive"}},
 		{map[string]string{}, &Config{Enabled: false}},
 		{nil, &Config{Enabled: false}},

internal/ingress/annotations/cors/main.go

@@ -106,17 +106,17 @@ func (c cors) Parse(ing *extensions.Ingress) (interface{}, error) {
 	}
 
 	config.CorsAllowOrigin, err = parser.GetStringAnnotation("cors-allow-origin", ing)
-	if err != nil || config.CorsAllowOrigin == "" || !corsOriginRegex.MatchString(config.CorsAllowOrigin) {
+	if err != nil || !corsOriginRegex.MatchString(config.CorsAllowOrigin) {
 		config.CorsAllowOrigin = "*"
 	}
 
 	config.CorsAllowHeaders, err = parser.GetStringAnnotation("cors-allow-headers", ing)
-	if err != nil || config.CorsAllowHeaders == "" || !corsHeadersRegex.MatchString(config.CorsAllowHeaders) {
+	if err != nil || !corsHeadersRegex.MatchString(config.CorsAllowHeaders) {
 		config.CorsAllowHeaders = defaultCorsHeaders
 	}
 
 	config.CorsAllowMethods, err = parser.GetStringAnnotation("cors-allow-methods", ing)
-	if err != nil || config.CorsAllowMethods == "" || !corsMethodsRegex.MatchString(config.CorsAllowMethods) {
+	if err != nil || !corsMethodsRegex.MatchString(config.CorsAllowMethods) {
 		config.CorsAllowMethods = defaultCorsMethods
 	}
 

internal/ingress/annotations/parser/main.go

@@ -52,6 +52,10 @@ func (a ingAnnotations) parseBool(name string) (bool, error) {
 func (a ingAnnotations) parseString(name string) (string, error) {
 	val, ok := a[name]
 	if ok {
+		if len(val) == 0 {
+			return "", errors.NewInvalidAnnotationContent(name, val)
+		}
+
 		return val, nil
 	}
 	return "", errors.ErrMissingAnnotations
@@ -97,6 +101,7 @@ func GetStringAnnotation(name string, ing *extensions.Ingress) (string, error) {
 	if err != nil {
 		return "", err
 	}
+
 	return ingAnnotations(ing.GetAnnotations()).parseString(v)
 }
 

internal/ingress/annotations/parser/main_test.go

@@ -79,7 +79,7 @@ func TestGetStringAnnotation(t *testing.T) {
 
 	_, err := GetStringAnnotation("", nil)
 	if err == nil {
-		t.Errorf("expected error but retuned nil")
+		t.Errorf("expected error but none returned")
 	}
 
 	tests := []struct {
@@ -91,6 +91,7 @@ func TestGetStringAnnotation(t *testing.T) {
 	}{
 		{"valid - A", "string", "A", "A", false},
 		{"valid - B", "string", "B", "B", false},
+		{"empty", "string", "", "", true},
 	}
 
 	data := map[string]string{}
@@ -102,7 +103,7 @@ func TestGetStringAnnotation(t *testing.T) {
 		s, err := GetStringAnnotation(test.field, ing)
 		if test.expErr {
 			if err == nil {
-				t.Errorf("%v: expected error but retuned nil", test.name)
+				t.Errorf("%v: expected error but none returned", test.name)
 			}
 			continue
 		}

internal/ingress/annotations/sessionaffinity/main.go

@@ -86,7 +86,7 @@ func (a affinity) cookieAffinityParse(ing *extensions.Ingress) *Cookie {
 	cookie := &Cookie{}
 
 	cookie.Name, err = parser.GetStringAnnotation(annotationAffinityCookieName, ing)
-	if err != nil || cookie.Name == "" {
+	if err != nil {
 		glog.V(3).Infof("Ingress %v: No value found in annotation %v. Using the default %v", ing.Name, annotationAffinityCookieName, defaultAffinityCookieName)
 		cookie.Name = defaultAffinityCookieName
 	}
@@ -110,7 +110,7 @@ func (a affinity) cookieAffinityParse(ing *extensions.Ingress) *Cookie {
 	}
 
 	cookie.Path, err = parser.GetStringAnnotation(annotationAffinityCookiePath, ing)
-	if err != nil || cookie.Path == "" {
+	if err != nil {
 		glog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Ignoring it", ing.Name, annotationAffinityCookieMaxAge)
 	}
 

internal/ingress/controller/store/store.go

@@ -310,7 +310,7 @@ func New(checkOCSP bool,
 			}
 			recorder.Eventf(ing, corev1.EventTypeNormal, "CREATE", fmt.Sprintf("Ingress %s/%s", ing.Namespace, ing.Name))
 
-			store.extractAnnotations(ing)
+			store.syncIngress(ing)
 			store.updateSecretIngressMap(ing)
 			store.syncSecrets(ing)
 
@@ -365,7 +365,7 @@ func New(checkOCSP bool,
 				recorder.Eventf(curIng, corev1.EventTypeNormal, "UPDATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name))
 			}
 
-			store.extractAnnotations(curIng)
+			store.syncIngress(curIng)
 			store.updateSecretIngressMap(curIng)
 			store.syncSecrets(curIng)
 
@@ -394,7 +394,7 @@ func New(checkOCSP bool,
 						glog.Errorf("could not find Ingress %v in local store", ingKey)
 						continue
 					}
-					store.extractAnnotations(ing)
+					store.syncIngress(ing)
 					store.syncSecrets(ing)
 				}
 				updateCh.In() <- Event{
@@ -421,7 +421,7 @@ func New(checkOCSP bool,
 							glog.Errorf("could not find Ingress %v in local store", ingKey)
 							continue
 						}
-						store.extractAnnotations(ing)
+						store.syncIngress(ing)
 						store.syncSecrets(ing)
 					}
 					updateCh.In() <- Event{
@@ -460,7 +460,7 @@ func New(checkOCSP bool,
 						glog.Errorf("could not find Ingress %v in local store", ingKey)
 						continue
 					}
-					store.extractAnnotations(ing)
+					store.syncIngress(ing)
 				}
 				updateCh.In() <- Event{
 					Type: DeleteEvent,
@@ -530,7 +530,7 @@ func New(checkOCSP bool,
 							glog.Errorf("could not find Ingress %v in local store: %v", key, err)
 							continue
 						}
-						store.extractAnnotations(ing)
+						store.syncIngress(ing)
 					}
 
 					updateCh.In() <- Event{
@@ -588,9 +588,9 @@ func New(checkOCSP bool,
 	return store
 }
 
-// extractAnnotations parses ingress annotations converting the value of the
-// annotation to a go struct and also information about the referenced secrets
-func (s *k8sStore) extractAnnotations(ing *extensions.Ingress) {
+// syncIngress parses ingress annotations converting the value of the
+// annotation to a go struct
+func (s *k8sStore) syncIngress(ing *extensions.Ingress) {
 	key := k8s.MetaNamespaceKey(ing)
 	glog.V(3).Infof("updating annotations information for ingress %v", key)
 
@@ -665,7 +665,7 @@ func (s *k8sStore) updateSecretIngressMap(ing *extensions.Ingress) {
 // 'namespace/name' key from the given annotation name.
 func objectRefAnnotationNsKey(ann string, ing *extensions.Ingress) (string, error) {
 	annValue, err := parser.GetStringAnnotation(ann, ing)
-	if annValue == "" {
+	if err != nil {
 		return "", err
 	}