Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add annotation to allow modifiers to be used properly in kubernetes #3481

Merged
merged 2 commits into from Jul 6, 2018
Merged

Add annotation to allow modifiers to be used properly in kubernetes #3481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d1de74a
base commit
dtomcej Jun 11, 2018
54a9013
review cleanup
mmatur Jul 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/configuration/backends/kubernetes.md
View file
Open in desktop
Expand Up @@ -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. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there is a type - should'n it be AddPrefix instead of AddPath?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tompson thanks. After changing to AddPrefix my ingress started working :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ferrarimarco can you maybe share your ingress configuration? I still cannot get mine working

Copy link

@ferrarimarco ferrarimarco Jul 12, 2018 •
edited by ldez

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tompson

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: a12
  labels:
    app: a1
    chart: a1
    release: a1
    heritage: Tiller
  annotations:
    kubernetes.io/ingress.class: traefik
    traefik.ingress.kubernetes.io/preserve-host: "true"
    traefik.ingress.kubernetes.io/app-root: "/index.html"
    traefik.ingress.kubernetes.io/request-modifier: "AddPrefix: /a1"
spec:
  rules:
    - host: host1.tld
      http:
        paths:
          - path: /test/aaaa
            backend:
              serviceName: service-name
              servicePort: http

Copy link
Contributor

@tompson tompson Jul 12, 2018 •
edited by ldez

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much!

Unfortunately I am still not able to get my desired configuration to work.

I got a backend that is available under /app and I want a traefik configuration that allows user to call it with /app or with /oldapp - both ways should work exactly the same.

I thought my config should look something like

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: default
  annotations:
    kubernetes.io/ingress.class: traefik
spec:
  rules:
  - host: httpecho.kube.troii.net
    http: 
      paths:
      - path: /app
        backend:
          serviceName: echo-server
          servicePort: 80
---
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: default
  annotations:
    kubernetes.io/ingress.class: traefik
    traefik.ingress.kubernetes.io/request-modifier: "AddPrefix: /app"
spec:
  rules:
  - host: httpecho.kube.troii.net
    http: 
      paths:
      - path: /oldapp
        backend:
          serviceName: echo-server
          servicePort: 80
---

Copy link
Contributor Author

@dtomcej dtomcej Jul 12, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: default
  annotations:
    kubernetes.io/ingress.class: traefik
    traefik.ingress.kubernetes.io/rule-type: PathPrefixStrip
    traefik.ingress.kubernetes.io/request-modifier: "AddPrefix: /app"
spec:
  rules:
  - host: httpecho.kube.troii.net
    http: 
      paths:
      - path: /oldapp
        backend:
          serviceName: echo-server
          servicePort: 80
      - path: /app
        backend:
          serviceName: echo-server
          servicePort: 80

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @dtomcej ! should this already work with v1.6.5 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into the commit history and it seems like this will be in included in 1.7.0 but is not part of 1.6.5 - so I will have to wait to try it

| `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) |
Expand Down
1 change: 1 addition & 0 deletions provider/kubernetes/annotations.go
View file
Open in desktop
Expand Up @@ -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"
Expand Down
55 changes: 55 additions & 0 deletions provider/kubernetes/kubernetes.go
View file
Open in desktop
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -532,9 +548,48 @@ func getRuleForPath(pa extensionsv1beta1.HTTPIngressPath, i *extensionsv1beta1.I
}
rules = append(rules, ruleTypeReplacePath+":"+rootPath)
}

if requestModifier := getStringValue(i.Annotations, annotationKubernetesRequestModifier, ""); requestModifier != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the annotation consists of all whitespaces only?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rude

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, added tests to include a bunch of other oddities

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)
Expand Down
216 changes: 203 additions & 13 deletions provider/kubernetes/kubernetes_test.go
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"os"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -350,7 +351,7 @@ func TestLoadGlobalIngressWithHttpsPortNames(t *testing.T) {
}

func TestRuleType(t *testing.T) {
tests := []struct {
testCases := []struct {
desc string
ingressRuleType string
frontendRuleType string
Expand All @@ -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()
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -2095,7 +2285,7 @@ func TestLoadIngressesForwardAuthWithTLSSecret(t *testing.T) {
}

func TestLoadIngressesForwardAuthWithTLSSecretFailures(t *testing.T) {
tests := []struct {
testCases := []struct {
desc string
secretName string
certName string
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -2365,7 +2555,7 @@ func TestGetTLS(t *testing.T) {
),
)

tests := []struct {
testCases := []struct {
desc string
ingress *extensionsv1beta1.Ingress
client Client
Expand Down Expand Up @@ -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()
Expand Down