From 2303301d38a9ff59f5846f384890dd8aaee9ac95 Mon Sep 17 00:00:00 2001 From: Daniel Tomcej Date: Fri, 6 Jul 2018 02:06:04 -0600 Subject: [PATCH] Add annotation to allow modifiers to be used properly in kubernetes --- docs/configuration/backends/kubernetes.md | 3 +- provider/kubernetes/annotations.go | 1 + provider/kubernetes/kubernetes.go | 55 ++++++ provider/kubernetes/kubernetes_test.go | 216 ++++++++++++++++++++-- 4 files changed, 261 insertions(+), 14 deletions(-) diff --git a/docs/configuration/backends/kubernetes.md b/docs/configuration/backends/kubernetes.md index 3a49d97ea..5dc699b4c 100644 --- a/docs/configuration/backends/kubernetes.md +++ b/docs/configuration/backends/kubernetes.md @@ -154,7 +154,8 @@ The following general annotations are applicable on the Ingress object: | `traefik.ingress.kubernetes.io/redirect-regex: ^http://localhost/(.*)` | Redirect to another URL for that frontend. Must be set with `traefik.ingress.kubernetes.io/redirect-replacement`. | | `traefik.ingress.kubernetes.io/redirect-replacement: http://mydomain/$1` | Redirect to another URL for that frontend. Must be set with `traefik.ingress.kubernetes.io/redirect-regex`. | | `traefik.ingress.kubernetes.io/rewrite-target: /users` | Replaces each matched Ingress path with the specified one, and adds the old path to the `X-Replaced-Path` header. | -| `traefik.ingress.kubernetes.io/rule-type: PathPrefixStrip` | Override the default frontend rule type. Default: `PathPrefix`. | +| `traefik.ingress.kubernetes.io/rule-type: PathPrefixStrip` | Override the default frontend rule type. Only path related matchers can be used [(`Path`, `PathPrefix`, `PathStrip`, `PathPrefixStrip`)](/basics/#path-matcher-usage-guidelines). Note: ReplacePath is deprecated in this annotation, use the `traefik.ingress.kubernetes.io/request-modifier` annotation instead. Default: `PathPrefix`. | +| `traefik.ingress.kubernetes.io/request-modifier: AddPath: /users` | Add a [request modifier](/basics/#modifiers) to the backend request. | | `traefik.ingress.kubernetes.io/whitelist-source-range: "1.2.3.0/24, fe80::/16"` | A comma-separated list of IP ranges permitted for access (6). | | `ingress.kubernetes.io/whitelist-x-forwarded-for: "true"` | Use `X-Forwarded-For` header as valid source of IP for the white list. | | `traefik.ingress.kubernetes.io/app-root: "/index.html"` | Redirects all requests for `/` to the defined path. (4) | diff --git a/provider/kubernetes/annotations.go b/provider/kubernetes/annotations.go index a12b25061..f56213704 100644 --- a/provider/kubernetes/annotations.go +++ b/provider/kubernetes/annotations.go @@ -38,6 +38,7 @@ const ( annotationKubernetesBuffering = "ingress.kubernetes.io/buffering" annotationKubernetesAppRoot = "ingress.kubernetes.io/app-root" annotationKubernetesServiceWeights = "ingress.kubernetes.io/service-weights" + annotationKubernetesRequestModifier = "ingress.kubernetes.io/request-modifier" annotationKubernetesSSLForceHost = "ingress.kubernetes.io/ssl-force-host" annotationKubernetesSSLRedirect = "ingress.kubernetes.io/ssl-redirect" diff --git a/provider/kubernetes/kubernetes.go b/provider/kubernetes/kubernetes.go index d12fd3185..8f2a019d3 100644 --- a/provider/kubernetes/kubernetes.go +++ b/provider/kubernetes/kubernetes.go @@ -32,8 +32,13 @@ import ( var _ provider.Provider = (*Provider)(nil) const ( + ruleTypePath = "Path" ruleTypePathPrefix = "PathPrefix" + ruleTypePathStrip = "PathStrip" + ruleTypePathPrefixStrip = "PathPrefixStrip" + ruleTypeAddPrefix = "AddPrefix" ruleTypeReplacePath = "ReplacePath" + ruleTypeReplacePathRegex = "ReplacePathRegex" traefikDefaultRealm = "traefik" traefikDefaultIngressClass = "traefik" defaultBackendName = "global-default-backend" @@ -511,6 +516,17 @@ func getRuleForPath(pa extensionsv1beta1.HTTPIngressPath, i *extensionsv1beta1.I } ruleType := getStringValue(i.Annotations, annotationKubernetesRuleType, ruleTypePathPrefix) + + switch ruleType { + case ruleTypePath, ruleTypePathPrefix, ruleTypePathStrip, ruleTypePathPrefixStrip: + case ruleTypeReplacePath: + log.Warnf("Using %s as %s will be deprecated in the future. Please use the %s annotation instead", ruleType, annotationKubernetesRuleType, annotationKubernetesRequestModifier) + case "": + return "", errors.New("cannot use empty rule") + default: + return "", fmt.Errorf("cannot use non-matcher rule: %q", ruleType) + } + rules := []string{ruleType + ":" + pa.Path} var pathReplaceAnnotation string @@ -532,9 +548,48 @@ func getRuleForPath(pa extensionsv1beta1.HTTPIngressPath, i *extensionsv1beta1.I } rules = append(rules, ruleTypeReplacePath+":"+rootPath) } + + if requestModifier := getStringValue(i.Annotations, annotationKubernetesRequestModifier, ""); requestModifier != "" { + rule, err := parseRequestModifier(requestModifier, ruleType) + if err != nil { + return "", err + } + + rules = append(rules, rule) + } return strings.Join(rules, ";"), nil } +func parseRequestModifier(requestModifier, ruleType string) (string, error) { + trimmedRequestModifier := strings.TrimRight(requestModifier, " :") + if trimmedRequestModifier == "" { + return "", fmt.Errorf("rule %q is empty", requestModifier) + } + + // Split annotation to determine modifier type + modifierParts := strings.Split(trimmedRequestModifier, ":") + if len(modifierParts) < 2 { + return "", fmt.Errorf("rule %q is missing type or value", requestModifier) + } + + modifier := strings.TrimSpace(modifierParts[0]) + value := strings.TrimSpace(modifierParts[1]) + + switch modifier { + case ruleTypeAddPrefix, ruleTypeReplacePath, ruleTypeReplacePathRegex: + if ruleType == ruleTypeReplacePath { + return "", fmt.Errorf("cannot use '%s: %s' and '%s: %s', as this leads to rule duplication, and unintended behavior", + annotationKubernetesRuleType, ruleTypeReplacePath, annotationKubernetesRequestModifier, modifier) + } + case "": + return "", errors.New("cannot use empty rule") + default: + return "", fmt.Errorf("cannot use non-modifier rule: %q", modifier) + } + + return modifier + ":" + value, nil +} + func getRuleForHost(host string) string { if strings.Contains(host, "*") { return "HostRegexp:" + strings.Replace(host, "*", "{subdomain:[A-Za-z0-9-_]+}", 1) diff --git a/provider/kubernetes/kubernetes_test.go b/provider/kubernetes/kubernetes_test.go index 037283b67..eed8f7252 100644 --- a/provider/kubernetes/kubernetes_test.go +++ b/provider/kubernetes/kubernetes_test.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "strings" "testing" "time" @@ -350,7 +351,7 @@ func TestLoadGlobalIngressWithHttpsPortNames(t *testing.T) { } func TestRuleType(t *testing.T) { - tests := []struct { + testCases := []struct { desc string ingressRuleType string frontendRuleType string @@ -371,13 +372,13 @@ func TestRuleType(t *testing.T) { frontendRuleType: "PathStrip", }, { - desc: "PathStripPrefix rule type annotation set", - ingressRuleType: "PathStripPrefix", - frontendRuleType: "PathStripPrefix", + desc: "PathPrefixStrip rule type annotation set", + ingressRuleType: "PathPrefixStrip", + frontendRuleType: "PathPrefixStrip", }, } - for _, test := range tests { + for _, test := range testCases { test := test t.Run(test.desc, func(t *testing.T) { t.Parallel() @@ -389,10 +390,8 @@ func TestRuleType(t *testing.T) { ), ))) - if test.ingressRuleType != "" { - ingress.Annotations = map[string]string{ - annotationKubernetesRuleType: test.ingressRuleType, - } + ingress.Annotations = map[string]string{ + annotationKubernetesRuleType: test.ingressRuleType, } service := buildService( @@ -423,6 +422,197 @@ func TestRuleType(t *testing.T) { } } +func TestRuleFails(t *testing.T) { + testCases := []struct { + desc string + ruletypeAnnotation string + requestModifierAnnotation string + }{ + { + desc: "Rule-type using unknown rule", + ruletypeAnnotation: "Foo: /bar", + }, + { + desc: "Rule type full of spaces", + ruletypeAnnotation: " ", + }, + { + desc: "Rule type missing both parts of rule", + ruletypeAnnotation: " : ", + }, + { + desc: "Rule type combined with replacepath modifier", + ruletypeAnnotation: "ReplacePath", + requestModifierAnnotation: "ReplacePath:/foo", + }, + { + desc: "Rule type combined with replacepathregex modifier", + ruletypeAnnotation: "ReplacePath", + requestModifierAnnotation: "ReplacePathRegex:/foo /bar", + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + ingress := buildIngress(iRules(iRule( + iHost("host"), + iPaths( + onePath(iPath("/path"), iBackend("service", intstr.FromInt(80))), + ), + ))) + + ingress.Annotations = map[string]string{ + annotationKubernetesRuleType: test.ruletypeAnnotation, + annotationKubernetesRequestModifier: test.requestModifierAnnotation, + } + + _, err := getRuleForPath(extensionsv1beta1.HTTPIngressPath{Path: "/path"}, ingress) + assert.Error(t, err) + }) + } +} + +func TestModifierType(t *testing.T) { + testCases := []struct { + desc string + requestModifierAnnotation string + expectedModifierRule string + }{ + { + desc: "Request modifier annotation missing", + requestModifierAnnotation: "", + expectedModifierRule: "", + }, + { + desc: "AddPrefix modifier annotation", + requestModifierAnnotation: " AddPrefix: /foo", + expectedModifierRule: "AddPrefix:/foo", + }, + { + desc: "ReplacePath modifier annotation", + requestModifierAnnotation: " ReplacePath: /foo", + expectedModifierRule: "ReplacePath:/foo", + }, + { + desc: "ReplacePathRegex modifier annotation", + requestModifierAnnotation: " ReplacePathRegex: /foo /bar", + expectedModifierRule: "ReplacePathRegex:/foo /bar", + }, + { + desc: "AddPrefix modifier annotation", + requestModifierAnnotation: "AddPrefix:/foo", + expectedModifierRule: "AddPrefix:/foo", + }, + { + desc: "ReplacePath modifier annotation", + requestModifierAnnotation: "ReplacePath:/foo", + expectedModifierRule: "ReplacePath:/foo", + }, + { + desc: "ReplacePathRegex modifier annotation", + requestModifierAnnotation: "ReplacePathRegex:/foo /bar", + expectedModifierRule: "ReplacePathRegex:/foo /bar", + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + ingress := buildIngress(iRules(iRule( + iHost("host"), + iPaths( + onePath(iPath("/path"), iBackend("service", intstr.FromInt(80))), + ), + ))) + + ingress.Annotations = map[string]string{ + annotationKubernetesRequestModifier: test.requestModifierAnnotation, + } + + service := buildService( + sName("service"), + sUID("1"), + sSpec(sPorts(sPort(801, "http"))), + ) + + watchChan := make(chan interface{}) + client := clientMock{ + ingresses: []*extensionsv1beta1.Ingress{ingress}, + services: []*corev1.Service{service}, + watchChan: watchChan, + } + + provider := Provider{DisablePassHostHeaders: true} + + actualConfig, err := provider.loadIngresses(client) + require.NoError(t, err, "error loading ingresses") + + expectedRules := []string{"PathPrefix:/path"} + if len(test.expectedModifierRule) > 0 { + expectedRules = append(expectedRules, test.expectedModifierRule) + } + + expected := buildFrontends(frontend("host/path", + routes( + route("/path", strings.Join(expectedRules, ";")), + route("host", "Host:host")), + )) + + assert.Equal(t, expected, actualConfig.Frontends) + }) + } +} + +func TestModifierFails(t *testing.T) { + testCases := []struct { + desc string + requestModifierAnnotation string + }{ + { + desc: "Request modifier missing part of annotation", + requestModifierAnnotation: "AddPrefix: ", + }, + { + desc: "Request modifier full of spaces annotation", + requestModifierAnnotation: " ", + }, + { + desc: "Request modifier missing both parts of annotation", + requestModifierAnnotation: " : ", + }, + { + desc: "Request modifier using unknown rule", + requestModifierAnnotation: "Foo: /bar", + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + ingress := buildIngress(iRules(iRule( + iHost("host"), + iPaths( + onePath(iPath("/path"), iBackend("service", intstr.FromInt(80))), + ), + ))) + + ingress.Annotations = map[string]string{ + annotationKubernetesRequestModifier: test.requestModifierAnnotation, + } + + _, err := getRuleForPath(extensionsv1beta1.HTTPIngressPath{Path: "/path"}, ingress) + assert.Error(t, err) + }) + } +} + func TestGetPassHostHeader(t *testing.T) { ingresses := []*extensionsv1beta1.Ingress{ buildIngress( @@ -2095,7 +2285,7 @@ func TestLoadIngressesForwardAuthWithTLSSecret(t *testing.T) { } func TestLoadIngressesForwardAuthWithTLSSecretFailures(t *testing.T) { - tests := []struct { + testCases := []struct { desc string secretName string certName string @@ -2189,7 +2379,7 @@ func TestLoadIngressesForwardAuthWithTLSSecretFailures(t *testing.T) { ), } - for _, test := range tests { + for _, test := range testCases { test := test t.Run(test.desc, func(t *testing.T) { t.Parallel() @@ -2365,7 +2555,7 @@ func TestGetTLS(t *testing.T) { ), ) - tests := []struct { + testCases := []struct { desc string ingress *extensionsv1beta1.Ingress client Client @@ -2515,7 +2705,7 @@ func TestGetTLS(t *testing.T) { }, } - for _, test := range tests { + for _, test := range testCases { test := test t.Run(test.desc, func(t *testing.T) { t.Parallel()