Merge pull request #1151 from timoreimann/refactor-k8s-rule-type-annotation-logic

Refactor k8s rule type annotation parsing/retrieval.

provider/kubernetes.go

@@ -19,6 +19,14 @@ import (
 
 var _ Provider = (*Kubernetes)(nil)
 
+const (
+	annotationFrontendRuleType = "traefik.frontend.rule.type"
+	ruleTypePathPrefixStrip    = "PathPrefixStrip"
+	ruleTypePathStrip          = "PathStrip"
+	ruleTypePath               = "Path"
+	ruleTypePathPrefix         = "PathPrefix"
+)
+
 // Kubernetes holds configurations of the Kubernetes provider.
 type Kubernetes struct {
 	BaseProvider           `mapstructure:",squash"`
@@ -157,29 +165,22 @@ func (provider *Kubernetes) loadIngresses(k8sClient k8s.Client) (*types.Configur
 						}
 					}
 				}
-				if len(pa.Path) > 0 {
-					ruleType := i.Annotations["traefik.frontend.rule.type"]
 
-					switch strings.ToLower(ruleType) {
+				if len(pa.Path) > 0 {
-					case "pathprefixstrip":
+					ruleType, unknown := getRuleTypeFromAnnotation(i.Annotations)
-						ruleType = "PathPrefixStrip"
+					switch {
-					case "pathstrip":
+					case unknown:
-						ruleType = "PathStrip"
+						log.Warnf("Unknown RuleType '%s' for Ingress %s/%s, falling back to PathPrefix", ruleType, i.ObjectMeta.Namespace, i.ObjectMeta.Name)
-					case "path":
+						fallthrough
-						ruleType = "Path"
+					case ruleType == "":
-					case "pathprefix":
+						ruleType = ruleTypePathPrefix
-						ruleType = "PathPrefix"
-					case "":
-						ruleType = "PathPrefix"
-					default:
-						log.Warnf("Unknown RuleType %s for %s/%s, falling back to PathPrefix", ruleType, i.ObjectMeta.Namespace, i.ObjectMeta.Name)
-						ruleType = "PathPrefix"
 					}
 
 					templateObjects.Frontends[r.Host+pa.Path].Routes[pa.Path] = types.Route{
 						Rule: ruleType + ":" + pa.Path,
 					}
 				}
+
 				service, exists, err := k8sClient.GetService(i.ObjectMeta.Namespace, pa.Backend.ServiceName)
 				if err != nil || !exists {
 					log.Warnf("Error retrieving service %s/%s: %v", i.ObjectMeta.Namespace, pa.Backend.ServiceName, err)
@@ -295,3 +296,24 @@ func (provider *Kubernetes) loadConfig(templateObjects types.Configuration) *typ
 	}
 	return configuration
 }
+
+func getRuleTypeFromAnnotation(annotations map[string]string) (ruleType string, unknown bool) {
+	ruleType = annotations[annotationFrontendRuleType]
+	for _, knownRuleType := range []string{
+		ruleTypePathPrefixStrip,
+		ruleTypePathStrip,
+		ruleTypePath,
+		ruleTypePathPrefix,
+	} {
+		if strings.ToLower(ruleType) == strings.ToLower(knownRuleType) {
+			return knownRuleType, false
+		}
+	}
+
+	if ruleType != "" {
+		// Annotation is set but does not match anything we know.
+		unknown = true
+	}
+
+	return ruleType, unknown
+}

provider/kubernetes_test.go

@@ -2,7 +2,9 @@ package provider
 
 import (
 	"encoding/json"
+	"fmt"
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/containous/traefik/provider/k8s"
@@ -328,124 +330,45 @@ func TestLoadIngresses(t *testing.T) {
 }
 
 func TestRuleType(t *testing.T) {
-	ingresses := []*v1beta1.Ingress{
+	tests := []struct {
+		desc             string
+		ingressRuleType  string
+		frontendRuleType string
+	}{
 		{
-			ObjectMeta: v1.ObjectMeta{
+			desc:             "implicit default",
-				Annotations: map[string]string{"traefik.frontend.rule.type": "PathPrefixStrip"}, //camel case
+			ingressRuleType:  "",
+			frontendRuleType: ruleTypePathPrefix,
 		},
+		{
+			desc:             "unknown ingress / explicit default",
+			ingressRuleType:  "unknown",
+			frontendRuleType: ruleTypePathPrefix,
+		},
+		{
+			desc:             "explicit ingress",
+			ingressRuleType:  ruleTypePath,
+			frontendRuleType: ruleTypePath,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+			ingress := &v1beta1.Ingress{
 				Spec: v1beta1.IngressSpec{
 					Rules: []v1beta1.IngressRule{
 						{
-						Host: "foo1",
+							Host: "host",
 							IngressRuleValue: v1beta1.IngressRuleValue{
 								HTTP: &v1beta1.HTTPIngressRuleValue{
 									Paths: []v1beta1.HTTPIngressPath{
 										{
-										Path: "/bar1",
+											Path: "/path",
 											Backend: v1beta1.IngressBackend{
-											ServiceName: "service1",
+												ServiceName: "service",
-											ServicePort: intstr.FromInt(801),
+												ServicePort: intstr.FromInt(80),
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-		},
-		{
-			ObjectMeta: v1.ObjectMeta{
-				Annotations: map[string]string{"traefik.frontend.rule.type": "path"}, //lower case
-			},
-			Spec: v1beta1.IngressSpec{
-				Rules: []v1beta1.IngressRule{
-					{
-						Host: "foo1",
-						IngressRuleValue: v1beta1.IngressRuleValue{
-							HTTP: &v1beta1.HTTPIngressRuleValue{
-								Paths: []v1beta1.HTTPIngressPath{
-									{
-										Path: "/bar2",
-										Backend: v1beta1.IngressBackend{
-											ServiceName: "service1",
-											ServicePort: intstr.FromInt(801),
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-		},
-		{
-			ObjectMeta: v1.ObjectMeta{
-				Annotations: map[string]string{"traefik.frontend.rule.type": "PathPrefix"}, //path prefix
-			},
-			Spec: v1beta1.IngressSpec{
-				Rules: []v1beta1.IngressRule{
-					{
-						Host: "foo2",
-						IngressRuleValue: v1beta1.IngressRuleValue{
-							HTTP: &v1beta1.HTTPIngressRuleValue{
-								Paths: []v1beta1.HTTPIngressPath{
-									{
-										Path: "/bar1",
-										Backend: v1beta1.IngressBackend{
-											ServiceName: "service1",
-											ServicePort: intstr.FromInt(801),
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-		},
-		{
-			ObjectMeta: v1.ObjectMeta{
-				Annotations: map[string]string{"traefik.frontend.rule.type": "PathStrip"}, //path strip
-			},
-			Spec: v1beta1.IngressSpec{
-				Rules: []v1beta1.IngressRule{
-					{
-						Host: "foo2",
-						IngressRuleValue: v1beta1.IngressRuleValue{
-							HTTP: &v1beta1.HTTPIngressRuleValue{
-								Paths: []v1beta1.HTTPIngressPath{
-									{
-										Path: "/bar2",
-										Backend: v1beta1.IngressBackend{
-											ServiceName: "service1",
-											ServicePort: intstr.FromInt(801),
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-		},
-		{
-			ObjectMeta: v1.ObjectMeta{
-				Annotations: map[string]string{"traefik.frontend.rule.type": "PathXXStrip"}, //wrong rule
-			},
-			Spec: v1beta1.IngressSpec{
-				Rules: []v1beta1.IngressRule{
-					{
-						Host: "foo1",
-						IngressRuleValue: v1beta1.IngressRuleValue{
-							HTTP: &v1beta1.HTTPIngressRuleValue{
-								Paths: []v1beta1.HTTPIngressPath{
-									{
-										Path: "/bar3",
-										Backend: v1beta1.IngressBackend{
-											ServiceName: "service1",
-											ServicePort: intstr.FromInt(801),
-										},
 											},
 										},
 									},
@@ -455,10 +378,16 @@ func TestRuleType(t *testing.T) {
 					},
 				},
 			}
-	services := []*v1.Service{
+
-		{
+			if test.ingressRuleType != "" {
+				ingress.ObjectMeta.Annotations = map[string]string{
+					annotationFrontendRuleType: test.ingressRuleType,
+				}
+			}
+
+			service := &v1.Service{
 				ObjectMeta: v1.ObjectMeta{
-				Name: "service1",
+					Name: "service",
 					UID:  "1",
 				},
 				Spec: v1.ServiceSpec{
@@ -470,89 +399,43 @@ func TestRuleType(t *testing.T) {
 						},
 					},
 				},
-		},
 			}
+
 			watchChan := make(chan interface{})
 			client := clientMock{
-		ingresses: ingresses,
+				ingresses: []*v1beta1.Ingress{ingress},
-		services:  services,
+				services:  []*v1.Service{service},
 				watchChan: watchChan,
 			}
 			provider := Kubernetes{DisablePassHostHeaders: true}
 			actualConfig, err := provider.loadIngresses(client)
-	actual := actualConfig.Frontends
 			if err != nil {
-		t.Fatalf("error %+v", err)
+				t.Fatalf("error loading ingresses: %+v", err)
 			}
 
+			actual := actualConfig.Frontends
 			expected := map[string]*types.Frontend{
-		"foo1/bar1": {
+				"host/path": {
-			Backend:  "foo1/bar1",
+					Backend:  "host/path",
-			Priority: len("/bar1"),
+					Priority: len("/path"),
 					Routes: map[string]types.Route{
-				"/bar1": {
+						"/path": {
-					Rule: "PathPrefixStrip:/bar1",
+							Rule: fmt.Sprintf("%s:/path", test.frontendRuleType),
 						},
-				"foo1": {
+						"host": {
-					Rule: "Host:foo1",
+							Rule: "Host:host",
-				},
-			},
-		},
-		"foo1/bar2": {
-			Backend:  "foo1/bar2",
-			Priority: len("/bar2"),
-			Routes: map[string]types.Route{
-				"/bar2": {
-					Rule: "Path:/bar2",
-				},
-				"foo1": {
-					Rule: "Host:foo1",
-				},
-			},
-		},
-		"foo2/bar1": {
-			Backend:  "foo2/bar1",
-			Priority: len("/bar1"),
-			Routes: map[string]types.Route{
-				"/bar1": {
-					Rule: "PathPrefix:/bar1",
-				},
-				"foo2": {
-					Rule: "Host:foo2",
-				},
-			},
-		},
-		"foo2/bar2": {
-			Backend:  "foo2/bar2",
-			Priority: len("/bar2"),
-			Routes: map[string]types.Route{
-				"/bar2": {
-					Rule: "PathStrip:/bar2",
-				},
-				"foo2": {
-					Rule: "Host:foo2",
-				},
-			},
-		},
-		"foo1/bar3": {
-			Backend:  "foo1/bar3",
-			Priority: len("/bar3"),
-			Routes: map[string]types.Route{
-				"/bar3": {
-					Rule: "PathPrefix:/bar3",
-				},
-				"foo1": {
-					Rule: "Host:foo1",
 						},
 					},
 				},
 			}
-	actualJSON, _ := json.Marshal(actual)
-	expectedJSON, _ := json.Marshal(expected)
 
-	if !reflect.DeepEqual(actual, expected) {
+			if !reflect.DeepEqual(expected, actual) {
+				expectedJSON, _ := json.Marshal(expected)
+				actualJSON, _ := json.Marshal(actual)
 				t.Fatalf("expected %+v, got %+v", string(expectedJSON), string(actualJSON))
 			}
+		})
+	}
 }
 
 func TestGetPassHostHeader(t *testing.T) {
@@ -1843,6 +1726,65 @@ func TestInvalidPassHostHeaderValue(t *testing.T) {
 	}
 }
 
+func TestGetRuleTypeFromAnnotation(t *testing.T) {
+	tests := []struct {
+		in            string
+		wantedUnknown bool
+	}{
+		{
+			in:            ruleTypePathPrefixStrip,
+			wantedUnknown: false,
+		},
+		{
+			in:            ruleTypePathStrip,
+			wantedUnknown: false,
+		},
+		{
+			in:            ruleTypePath,
+			wantedUnknown: false,
+		},
+		{
+			in:            ruleTypePathPrefix,
+			wantedUnknown: false,
+		},
+		{
+			wantedUnknown: false,
+		},
+		{
+			in:            "Unknown",
+			wantedUnknown: true,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		inputs := []string{test.in, strings.ToLower(test.in)}
+		if inputs[0] == inputs[1] {
+			// Lower-casing makes no difference -- truncating to single case.
+			inputs = inputs[:1]
+		}
+		for _, input := range inputs {
+			t.Run(fmt.Sprintf("in='%s'", input), func(t *testing.T) {
+				t.Parallel()
+				annotations := map[string]string{}
+				if test.in != "" {
+					annotations[annotationFrontendRuleType] = test.in
+				}
+
+				gotRuleType, gotUnknown := getRuleTypeFromAnnotation(annotations)
+
+				if gotUnknown != test.wantedUnknown {
+					t.Errorf("got unknown '%t', wanted '%t'", gotUnknown, test.wantedUnknown)
+				}
+
+				if gotRuleType != test.in {
+					t.Errorf("got rule type '%s', wanted '%s'", gotRuleType, test.in)
+				}
+			})
+		}
+	}
+}
+
 type clientMock struct {
 	ingresses []*v1beta1.Ingress
 	services  []*v1.Service