Correct Mergeable Types misspelling and optimize blacklists

Corrects incorrect spelling of 'mergeable' in different locations.
Also optimizes filtering and merging functions by converting the
blacklists to maps instead of arrays.

Fixes nginx#269

Author: Fernando Diaz

examples/mergable-ingress-types/README.md renamed to examples/mergeable-ingress-types/README.md

@@ -5,7 +5,7 @@ for easier management when using a large amount of paths.
 
 ## Syntax and Rules
 
-A Master is declared using `nginx.org/mergible-ingress-type: master`. A Master will process all configurations at the
+A Master is declared using `nginx.org/mergeable-ingress-type: master`. A Master will process all configurations at the
 host level, which includes the TLS configuration, and any annotations which will be applied for the complete host. There
 can only be one ingress resource on a unique host that contains the master value. Paths cannot be part of the
 ingress resource.
@@ -16,7 +16,7 @@ Masters cannot contain the following annotations:
 * nginx.org/websocket-services
 * nginx.com/sticky-cookie-services
 
-A Minion is declared using `nginx.org/mergible-ingress-type: minion`. A Minion will be used to append different
+A Minion is declared using `nginx.org/mergeable-ingress-type: minion`. A Minion will be used to append different
 locations to an ingress resource with the Master value. TLS configurations are not allowed. Multiple minions can be
 applied per master as long as they do not have conflicting paths. If a conflicting path is present then the path defined
 on the oldest minion will be used.
@@ -42,7 +42,7 @@ Note: Ingress Resources with more than one host cannot be used.
 ## Example
 
 In this example we deploy the NGINX Ingress controller, a simple web application and then configure
-load balancing for that application using Ingress resources with the `nginx.org/mergible-ingress-type` annotations.
+load balancing for that application using Ingress resources with the `nginx.org/mergeable-ingress-type` annotations.
 
 ## Running the Example
 

examples/mergable-ingress-types/cafe-master.yaml renamed to examples/mergeable-ingress-types/cafe-master.yaml

@@ -4,7 +4,7 @@ metadata:
   name: cafe-ingress-master
   annotations:
     kubernetes.io/ingress.class: "nginx"
-    nginx.org/mergible-ingress-type: "master"
+    nginx.org/mergeable-ingress-type: "master"
 spec:
   tls:
   - hosts:

examples/mergable-ingress-types/cafe-secret.yaml renamed to examples/mergeable-ingress-types/cafe-secret.yaml

@@ -4,7 +4,7 @@ metadata:
   name: cafe-ingress-coffee-minion
   annotations:
     kubernetes.io/ingress.class: "nginx"
-    nginx.org/mergible-ingress-type: "minion"
+    nginx.org/mergeable-ingress-type: "minion"
 spec:
   rules:
   - host: cafe.example.com

examples/mergable-ingress-types/cafe.yaml renamed to examples/mergeable-ingress-types/cafe.yaml

@@ -4,7 +4,7 @@ metadata:
   name: cafe-ingress-tea-minion
   annotations:
     kubernetes.io/ingress.class: "nginx"
-    nginx.org/mergible-ingress-type: "minion"
+    nginx.org/mergeable-ingress-type: "minion"
 spec:
   rules:
   - host: cafe.example.com

examples/mergable-ingress-types/coffee-minion.yaml renamed to examples/mergeable-ingress-types/coffee-minion.yaml

@@ -783,21 +783,21 @@ func (lbc *LoadBalancerController) syncIng(task Task) {
 		ing := obj.(*extensions.Ingress)
 
 		if isMaster(ing) {
-			mergableIngExs, err := lbc.createMergableIngresses(ing)
+			mergeableIngExs, err := lbc.createMergableIngresses(ing)
 			if err != nil {
 				lbc.syncQueue.requeueAfter(task, err, 5*time.Second)
 				lbc.recorder.Eventf(ing, api_v1.EventTypeWarning, "Rejected", "%v was rejected: %v", key, err)
 				return
 			}
-			err = lbc.cnf.AddOrUpdateMergableIngress(mergableIngExs)
+			err = lbc.cnf.AddOrUpdateMergableIngress(mergeableIngExs)
 			if err != nil {
 				lbc.recorder.Eventf(ing, api_v1.EventTypeWarning, "AddedOrUpdatedWithError", "Configuration for %v(Master) was added or updated, but not applied: %v", key, err)
-				for _, minion := range mergableIngExs.Minions {
+				for _, minion := range mergeableIngExs.Minions {
 					lbc.recorder.Eventf(ing, api_v1.EventTypeWarning, "AddedOrUpdatedWithError", "Configuration for %v/%v(Minion) was added or updated, but not applied: %v", minion.Ingress.Namespace, minion.Ingress.Name, err)
 				}
 			} else {
 				lbc.recorder.Eventf(ing, api_v1.EventTypeNormal, "AddedOrUpdated", "Configuration for %v(Master) was added or updated", key)
-				for _, minion := range mergableIngExs.Minions {
+				for _, minion := range mergeableIngExs.Minions {
 					lbc.recorder.Eventf(ing, api_v1.EventTypeNormal, "AddedOrUpdated", "Configuration for %v/%v(Minion) was added or updated", minion.Ingress.Namespace, minion.Ingress.Name)
 				}
 			}
@@ -1233,18 +1233,18 @@ func (lbc *LoadBalancerController) getMinionsForMaster(master *nginx.IngressEx)
 			continue
 		}
 		if len(ings.Items[i].Spec.Rules) != 1 {
-			glog.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergible-ingress-type' annotation must contain only one host", ings.Items[i].Namespace, ings.Items[i].Name)
+			glog.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergeable-ingress-type' annotation must contain only one host", ings.Items[i].Namespace, ings.Items[i].Name)
 			continue
 		}
 		if ings.Items[i].Spec.Rules[0].HTTP == nil {
-			glog.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergible-ingress-type' annotation set to 'minion' must contain a Path", ings.Items[i].Namespace, ings.Items[i].Name)
+			glog.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergeable-ingress-type' annotation set to 'minion' must contain a Path", ings.Items[i].Namespace, ings.Items[i].Name)
 			continue
 		}
 
 		uniquePaths := []extensions.HTTPIngressPath{}
 		for _, path := range ings.Items[i].Spec.Rules[0].HTTP.Paths {
 			if val, ok := minionPaths[path.Path]; ok {
-				glog.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergible-ingress-type' annotation set to 'minion' cannot contain the same path as another ingress resource, %v/%v.",
+				glog.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergeable-ingress-type' annotation set to 'minion' cannot contain the same path as another ingress resource, %v/%v.",
 					ings.Items[i].Namespace, ings.Items[i].Name, val.Namespace, val.Name)
 				glog.Errorf("Path %s for Ingress Resource %v/%v will be ignored", path.Path, val.Namespace, val.Name)
 			} else {
@@ -1260,7 +1260,7 @@ func (lbc *LoadBalancerController) getMinionsForMaster(master *nginx.IngressEx)
 			continue
 		}
 		if len(ingEx.TLSSecrets) > 0 || ingEx.JWTKey != nil {
-			glog.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergible-ingress-type' annotation set to 'minion' cannot contain TLSSecrets or JWTKeys", ingEx.Ingress.Namespace, ingEx.Ingress.Name)
+			glog.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergeable-ingress-type' annotation set to 'minion' cannot contain TLSSecrets or JWTKeys", ingEx.Ingress.Namespace, ingEx.Ingress.Name)
 			continue
 		}
 		minions = append(minions, ingEx)
@@ -1300,15 +1300,15 @@ func (lbc *LoadBalancerController) createMergableIngresses(master *extensions.In
 	mergeableIngresses := nginx.MergeableIngresses{}
 
 	if len(master.Spec.Rules) != 1 {
-		err := fmt.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergible-ingress-type' annotation must contain only one host", master.Namespace, master.Name)
+		err := fmt.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergeable-ingress-type' annotation must contain only one host", master.Namespace, master.Name)
 		return &mergeableIngresses, err
 	}
 
 	var empty extensions.HTTPIngressRuleValue
 	if master.Spec.Rules[0].HTTP != nil {
 		if master.Spec.Rules[0].HTTP != &empty {
 			if len(master.Spec.Rules[0].HTTP.Paths) != 0 {
-				err := fmt.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergible-ingress-type' annotation set to 'master' cannot contain Paths", master.Namespace, master.Name)
+				err := fmt.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergeable-ingress-type' annotation set to 'master' cannot contain Paths", master.Namespace, master.Name)
 				return &mergeableIngresses, err
 			}
 		}
@@ -1337,14 +1337,14 @@ func (lbc *LoadBalancerController) createMergableIngresses(master *extensions.In
 }
 
 func isMinion(ing *extensions.Ingress) bool {
-	if ing.Annotations["nginx.org/mergible-ingress-type"] == "minion" {
+	if ing.Annotations["nginx.org/mergeable-ingress-type"] == "minion" {
 		return true
 	}
 	return false
 }
 
 func isMaster(ing *extensions.Ingress) bool {
-	if ing.Annotations["nginx.org/mergible-ingress-type"] == "master" {
+	if ing.Annotations["nginx.org/mergeable-ingress-type"] == "master" {
 		return true
 	}
 	return false

examples/mergable-ingress-types/tea-minion.yaml renamed to examples/mergeable-ingress-types/tea-minion.yaml

@@ -161,21 +161,21 @@ func TestCreateMergableIngresses(t *testing.T) {
 	lbc.ingLister.Add(&coffeeMinion)
 	lbc.ingLister.Add(&teaMinion)
 
-	mergableIngresses, err := lbc.createMergableIngresses(&cafeMaster)
+	mergeableIngresses, err := lbc.createMergableIngresses(&cafeMaster)
 	if err != nil {
 		t.Errorf("Error creating Mergable Ingresses: %v", err)
 	}
-	if mergableIngresses.Master.Ingress.Name != cafeMaster.Name && mergableIngresses.Master.Ingress.Namespace != cafeMaster.Namespace {
+	if mergeableIngresses.Master.Ingress.Name != cafeMaster.Name && mergeableIngresses.Master.Ingress.Namespace != cafeMaster.Namespace {
 		t.Errorf("Master %s not set properly", cafeMaster.Name)
 	}
 
-	if len(mergableIngresses.Minions) != 2 {
-		t.Errorf("Invalid amount of minions in mergableIngresses: %v", mergableIngresses.Minions)
+	if len(mergeableIngresses.Minions) != 2 {
+		t.Errorf("Invalid amount of minions in mergeableIngresses: %v", mergeableIngresses.Minions)
 	}
 
 	coffeeCount := 0
 	teaCount := 0
-	for _, minion := range mergableIngresses.Minions {
+	for _, minion := range mergeableIngresses.Minions {
 		if minion.Ingress.Name == coffeeMinion.Name {
 			coffeeCount++
 		} else if minion.Ingress.Name == teaMinion.Name {
@@ -220,7 +220,7 @@ func TestCreateMergableIngressesInvalidMaster(t *testing.T) {
 	}
 	lbc.ingLister.Add(&cafeMaster)
 
-	expected := fmt.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergible-ingress-type' annotation set to 'master' cannot contain Paths", cafeMaster.Namespace, cafeMaster.Name)
+	expected := fmt.Errorf("Ingress Resource %v/%v with the 'nginx.org/mergeable-ingress-type' annotation set to 'master' cannot contain Paths", cafeMaster.Namespace, cafeMaster.Name)
 	_, err := lbc.createMergableIngresses(&cafeMaster)
 	if !reflect.DeepEqual(err, expected) {
 		t.Errorf("Error Validating the Ingress Resource: \n Expected: %s \n Obtained: %s", expected, err)
@@ -467,8 +467,8 @@ func getMergableDefaults() (cafeMaster, coffeeMinion, teaMinion extensions.Ingre
 			Name:      "cafe-master",
 			Namespace: "default",
 			Annotations: map[string]string{
-				"kubernetes.io/ingress.class":     "nginx",
-				"nginx.org/mergible-ingress-type": "master",
+				"kubernetes.io/ingress.class":      "nginx",
+				"nginx.org/mergeable-ingress-type": "master",
 			},
 		},
 		Spec: extensions.IngressSpec{
@@ -486,8 +486,8 @@ func getMergableDefaults() (cafeMaster, coffeeMinion, teaMinion extensions.Ingre
 			Name:      "coffee-minion",
 			Namespace: "default",
 			Annotations: map[string]string{
-				"kubernetes.io/ingress.class":     "nginx",
-				"nginx.org/mergible-ingress-type": "minion",
+				"kubernetes.io/ingress.class":      "nginx",
+				"nginx.org/mergeable-ingress-type": "minion",
 			},
 		},
 		Spec: extensions.IngressSpec{
@@ -520,8 +520,8 @@ func getMergableDefaults() (cafeMaster, coffeeMinion, teaMinion extensions.Ingre
 			Name:      "tea-minion",
 			Namespace: "default",
 			Annotations: map[string]string{
-				"kubernetes.io/ingress.class":     "nginx",
-				"nginx.org/mergible-ingress-type": "minion",
+				"kubernetes.io/ingress.class":      "nginx",
+				"nginx.org/mergeable-ingress-type": "minion",
 			},
 		},
 		Spec: extensions.IngressSpec{

nginx-controller/controller/controller.go

@@ -87,7 +87,7 @@ func (cnf *Configurator) addOrUpdateMergableIngress(mergeableIngs *MergeableIngr
 
 	removedAnnotations = filterMasterAnnotations(mergeableIngs.Master.Ingress.Annotations)
 	if len(removedAnnotations) != 0 {
-		glog.Errorf("Ingress Resource %v/%v with the annotation 'nginx.com/mergeable-ingress-type' set to 'master' cannot contain the '%v' annotation(s). They will be ignored",
+		glog.Errorf("Ingress Resource %v/%v with the annotation 'nginx.org/mergeable-ingress-type' set to 'master' cannot contain the '%v' annotation(s). They will be ignored",
 			mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, strings.Join(removedAnnotations, ","))
 	}
 
@@ -116,7 +116,7 @@ func (cnf *Configurator) addOrUpdateMergableIngress(mergeableIngs *MergeableIngr
 
 		removedAnnotations = filterMinionAnnotations(minion.Ingress.Annotations)
 		if len(removedAnnotations) != 0 {
-			glog.Errorf("Ingress Resource %v/%v with the annotation 'nginx.com/mergeable-ingress-type' set to 'minion' cannot contain the %v annotation(s). They will be ignored",
+			glog.Errorf("Ingress Resource %v/%v with the annotation 'nginx.org/mergeable-ingress-type' set to 'minion' cannot contain the %v annotation(s). They will be ignored",
 				minion.Ingress.Namespace, minion.Ingress.Name, strings.Join(removedAnnotations, ","))
 		}
 
@@ -807,10 +807,10 @@ func (cnf *Configurator) updatePlusEndpoints(ingEx *IngressEx) error {
 func filterMasterAnnotations(annotations map[string]string) []string {
 	var removedAnnotations []string
 
-	for _, blacklistAnn := range masterBlacklist {
-		if _, ok := annotations[blacklistAnn]; ok {
-			removedAnnotations = append(removedAnnotations, blacklistAnn)
-			delete(annotations, blacklistAnn)
+	for key, _ := range annotations {
+		if _, ok := masterBlacklist[key]; ok {
+			removedAnnotations = append(removedAnnotations, key)
+			delete(annotations, key)
 		}
 	}
 
@@ -820,10 +820,10 @@ func filterMasterAnnotations(annotations map[string]string) []string {
 func filterMinionAnnotations(annotations map[string]string) []string {
 	var removedAnnotations []string
 
-	for _, blacklistAnn := range minionBlacklist {
-		if _, ok := annotations[blacklistAnn]; ok {
-			removedAnnotations = append(removedAnnotations, blacklistAnn)
-			delete(annotations, blacklistAnn)
+	for key, _ := range annotations {
+		if _, ok := minionBlacklist[key]; ok {
+			removedAnnotations = append(removedAnnotations, key)
+			delete(annotations, key)
 		}
 	}
 
@@ -832,14 +832,8 @@ func filterMinionAnnotations(annotations map[string]string) []string {
 
 func mergeMasterAnnotationsIntoMinion(minionAnnotations map[string]string, masterAnnotations map[string]string) {
 	for key, val := range masterAnnotations {
-		isBlacklisted := false
 		if _, ok := minionAnnotations[key]; !ok {
-			for _, blacklistAnn := range minionBlacklist {
-				if blacklistAnn == key {
-					isBlacklisted = true
-				}
-			}
-			if !isBlacklisted {
+			if _, ok := minionBlacklist[key]; !ok {
 				minionAnnotations[key] = val
 			}
 		}

nginx-controller/controller/controller_test.go

@@ -2,6 +2,7 @@ package nginx
 
 import (
 	"reflect"
+	"sort"
 	"testing"
 )
 
@@ -83,6 +84,9 @@ func TestFilterMasterAnnotations(t *testing.T) {
 		"nginx.org/ssl-services",
 	}
 
+	sort.Strings(removedAnnotations)
+	sort.Strings(expectedRemovedAnnotations)
+
 	if !reflect.DeepEqual(expectedfilteredMasterAnnotations, masterAnnotations) {
 		t.Errorf("filterMasterAnnotations returned %v, but expected %v", masterAnnotations, expectedfilteredMasterAnnotations)
 	}
@@ -111,6 +115,9 @@ func TestFilterMinionAnnotations(t *testing.T) {
 		"nginx.org/hsts-include-subdomains",
 	}
 
+	sort.Strings(removedAnnotations)
+	sort.Strings(expectedRemovedAnnotations)
+
 	if !reflect.DeepEqual(expectedfilteredMinionAnnotations, minionAnnotations) {
 		t.Errorf("filterMinionAnnotations returned %v, but expected %v", minionAnnotations, expectedfilteredMinionAnnotations)
 	}

nginx-controller/nginx/configurator.go

@@ -19,26 +19,26 @@ type MergeableIngresses struct {
 	Minions []*IngressEx
 }
 
-var masterBlacklist = []string{
-	"nginx.org/rewrites",
-	"nginx.org/ssl-services",
-	"nginx.org/websocket-services",
-	"nginx.com/sticky-cookie-services",
+var masterBlacklist = map[string]bool{
+	"nginx.org/rewrites":               true,
+	"nginx.org/ssl-services":           true,
+	"nginx.org/websocket-services":     true,
+	"nginx.com/sticky-cookie-services": true,
 }
 
-var minionBlacklist = []string{
-	"nginx.org/proxy-hide-headers",
-	"nginx.org/proxy-pass-headers",
-	"nginx.org/redirect-to-https",
-	"ingress.kubernetes.io/ssl-redirect",
-	"nginx.org/hsts",
-	"nginx.org/hsts-max-age",
-	"nginx.org/hsts-include-subdomains",
-	"nginx.org/server-tokens",
-	"nginx.org/listen-ports",
-	"nginx.org/listen-ports-ssl",
-	"nginx.com/jwt-key",
-	"nginx.com/jwt-realm",
-	"nginx.com/jwt-token",
-	"nginx.com/jwt-login-url",
+var minionBlacklist = map[string]bool{
+	"nginx.org/proxy-hide-headers":       true,
+	"nginx.org/proxy-pass-headers":       true,
+	"nginx.org/redirect-to-https":        true,
+	"ingress.kubernetes.io/ssl-redirect": true,
+	"nginx.org/hsts":                     true,
+	"nginx.org/hsts-max-age":             true,
+	"nginx.org/hsts-include-subdomains":  true,
+	"nginx.org/server-tokens":            true,
+	"nginx.org/listen-ports":             true,
+	"nginx.org/listen-ports-ssl":         true,
+	"nginx.com/jwt-key":                  true,
+	"nginx.com/jwt-realm":                true,
+	"nginx.com/jwt-token":                true,
+	"nginx.com/jwt-login-url":            true,
 }