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.

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

Allow for wildcards in k8s ingress host, fixes #792 #1029

Merged
merged 2 commits into from
Jan 20, 2017

Conversation

sheerun
Copy link
Contributor

@sheerun sheerun commented Jan 9, 2017

Example ingress:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  namespace: deis
  name: deis
spec:
  rules:
  - host: "*.example.com"
    http:
      paths:
      - path: /
        backend:
          serviceName: deis-router
          servicePort: 80

rule := "Host:" + r.Host

if strings.Contains(r.Host, "*") {
rule = "HostRegexp:" + strings.Replace(r.Host, "*", "{subdomain:[a-z]+}", 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

The subdomain rule you implemented here does not allow for capital letters, numbers, or dashes/underscores. This means that ww2.example.com will not have a properly matched rule.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dtomcej I've changed it to allow dashes and capital letter, but I believe underscores are disallowed in domain names, aren't they?

@dtomcej
Copy link
Contributor

dtomcej commented Jan 9, 2017

http://stackoverflow.com/a/2183140

The commenter is correct, many services include underscores in the subdomains.

@sheerun
Copy link
Contributor Author

sheerun commented Jan 9, 2017

@dtomcej fixed

@dtomcej
Copy link
Contributor

dtomcej commented Jan 9, 2017

Why not just "{subdomain:[A-Za-z0-9-_]*}"? The underscore may be the first character in the subdomain:

_test.example.com

@sheerun
Copy link
Contributor Author

sheerun commented Jan 11, 2017

@dtomcej fixed

@dtomcej
Copy link
Contributor

dtomcej commented Jan 11, 2017

LGTM :shipit:
Ping @containous/traefik

Copy link
Contributor

@errm errm left a comment

Choose a reason for hiding this comment

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

LGTM

@emilevauge
Copy link
Member

emilevauge commented Jan 12, 2017

If we allow _ then we may use the same regex \\da-z\\.-_ in https://github.com/containous/traefik/blob/master/server.go#L754 :)

PS: I edited my comment, read too quickly ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants