From f3598e6b0ff356d3a6cedea9b386f9846068c6d0 Mon Sep 17 00:00:00 2001 From: Timo Reimann Date: Tue, 7 Feb 2017 00:04:30 +0100 Subject: [PATCH] Refactor k8s rule type annotation parsing/retrieval. - Move annotation logic into function. - Constantify strings. - Refactor TestRuleType. - Add test for GetRuleTypeFromAnnotations. --- provider/kubernetes.go | 54 ++++-- provider/kubernetes_test.go | 370 +++++++++++++++--------------------- 2 files changed, 194 insertions(+), 230 deletions(-) diff --git a/provider/kubernetes.go b/provider/kubernetes.go index 6fe6ca848..a7dce2a01 100644 --- a/provider/kubernetes.go +++ b/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) { - case "pathprefixstrip": - ruleType = "PathPrefixStrip" - case "pathstrip": - ruleType = "PathStrip" - case "path": - ruleType = "Path" - case "pathprefix": - 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" + if len(pa.Path) > 0 { + ruleType, unknown := getRuleTypeFromAnnotation(i.Annotations) + switch { + case unknown: + log.Warnf("Unknown RuleType '%s' for Ingress %s/%s, falling back to PathPrefix", ruleType, i.ObjectMeta.Namespace, i.ObjectMeta.Name) + fallthrough + case ruleType == "": + ruleType = ruleTypePathPrefix } 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 +} diff --git a/provider/kubernetes_test.go b/provider/kubernetes_test.go index 132fdf90e..7213d548e 100644 --- a/provider/kubernetes_test.go +++ b/provider/kubernetes_test.go @@ -2,7 +2,9 @@ package provider import ( "encoding/json" + "fmt" "reflect" + "strings" "testing" "github.com/containous/traefik/provider/k8s" @@ -328,230 +330,111 @@ func TestLoadIngresses(t *testing.T) { } func TestRuleType(t *testing.T) { - ingresses := []*v1beta1.Ingress{ + tests := []struct { + desc string + ingressRuleType string + frontendRuleType string + }{ { - ObjectMeta: v1.ObjectMeta{ - Annotations: map[string]string{"traefik.frontend.rule.type": "PathPrefixStrip"}, //camel case - }, - Spec: v1beta1.IngressSpec{ - Rules: []v1beta1.IngressRule{ - { - Host: "foo1", - IngressRuleValue: v1beta1.IngressRuleValue{ - HTTP: &v1beta1.HTTPIngressRuleValue{ - Paths: []v1beta1.HTTPIngressPath{ - { - Path: "/bar1", - Backend: v1beta1.IngressBackend{ - ServiceName: "service1", - ServicePort: intstr.FromInt(801), - }, - }, - }, - }, - }, - }, - }, - }, + desc: "implicit default", + ingressRuleType: "", + frontendRuleType: ruleTypePathPrefix, }, { - 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), - }, - }, - }, - }, - }, - }, - }, - }, + desc: "unknown ingress / explicit default", + ingressRuleType: "unknown", + frontendRuleType: ruleTypePathPrefix, }, { - 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), - }, - }, - }, - }, - }, - }, - }, - }, + desc: "explicit ingress", + ingressRuleType: ruleTypePath, + frontendRuleType: ruleTypePath, }, - { - 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), - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - services := []*v1.Service{ - { - ObjectMeta: v1.ObjectMeta{ - Name: "service1", - UID: "1", - }, - Spec: v1.ServiceSpec{ - ClusterIP: "10.0.0.1", - Ports: []v1.ServicePort{ - { - Name: "http", - Port: 801, - }, - }, - }, - }, - } - watchChan := make(chan interface{}) - client := clientMock{ - ingresses: ingresses, - services: services, - watchChan: watchChan, - } - provider := Kubernetes{DisablePassHostHeaders: true} - actualConfig, err := provider.loadIngresses(client) - actual := actualConfig.Frontends - if err != nil { - t.Fatalf("error %+v", err) } - expected := map[string]*types.Frontend{ - "foo1/bar1": { - Backend: "foo1/bar1", - Priority: len("/bar1"), - Routes: map[string]types.Route{ - "/bar1": { - Rule: "PathPrefixStrip:/bar1", + 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: "host", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Path: "/path", + Backend: v1beta1.IngressBackend{ + ServiceName: "service", + ServicePort: intstr.FromInt(80), + }, + }, + }, + }, + }, + }, + }, }, - "foo1": { - Rule: "Host:foo1", - }, - }, - }, - "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) { - t.Fatalf("expected %+v, got %+v", string(expectedJSON), string(actualJSON)) + if test.ingressRuleType != "" { + ingress.ObjectMeta.Annotations = map[string]string{ + annotationFrontendRuleType: test.ingressRuleType, + } + } + + service := &v1.Service{ + ObjectMeta: v1.ObjectMeta{ + Name: "service", + UID: "1", + }, + Spec: v1.ServiceSpec{ + ClusterIP: "10.0.0.1", + Ports: []v1.ServicePort{ + { + Name: "http", + Port: 801, + }, + }, + }, + } + + watchChan := make(chan interface{}) + client := clientMock{ + ingresses: []*v1beta1.Ingress{ingress}, + services: []*v1.Service{service}, + watchChan: watchChan, + } + provider := Kubernetes{DisablePassHostHeaders: true} + actualConfig, err := provider.loadIngresses(client) + if err != nil { + t.Fatalf("error loading ingresses: %+v", err) + } + + actual := actualConfig.Frontends + expected := map[string]*types.Frontend{ + "host/path": { + Backend: "host/path", + Priority: len("/path"), + Routes: map[string]types.Route{ + "/path": { + Rule: fmt.Sprintf("%s:/path", test.frontendRuleType), + }, + "host": { + Rule: "Host:host", + }, + }, + }, + } + + if !reflect.DeepEqual(expected, actual) { + expectedJSON, _ := json.Marshal(expected) + actualJSON, _ := json.Marshal(actual) + t.Fatalf("expected %+v, got %+v", string(expectedJSON), string(actualJSON)) + } + }) } } @@ -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