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

Remove rule type path list. #1630

Merged
merged 2 commits into from
May 23, 2017

Conversation

timoreimann
Copy link
Contributor

Instead of doing sanity checks in the Kubernetes provider, we just accept any non-empty value from the annotation and rely on the server part to filter out unknown rules.

This allows us to automatically stay in sync with the currently supported Path matchers/modifiers.

Fixes #1627.

@timoreimann
Copy link
Contributor Author

@emilevauge and others: I think this one should still go into 1.3. If you believe that master is the better target, please let me know and I'll rebase.

@ldez ldez added this to the 1.3 milestone May 18, 2017
Copy link
Member

@emilevauge emilevauge left a comment

Choose a reason for hiding this comment

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

@timoreimann great move ;)
Could you add some tests on PathPrefixStrip PathStrip & Path rules?
This is linked to the previous PR #1151.

@timoreimann
Copy link
Contributor Author

timoreimann commented May 19, 2017

@emilevauge Pushed more tests. Not sure if that's what you had in mind though since the new implementation doesn't care what value we put into the annotation (the validation now happens in server.go), and we already had a test for one explicit value (Path).

Let me know. :-)

Copy link
Member

@emilevauge emilevauge left a comment

Choose a reason for hiding this comment

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

Thanks @timoreimann
LGTM

@emilevauge
Copy link
Member

Ping @errm @dtomcej, could you give your thoughts on this one ?

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.

Very nice LGTM

Copy link
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

LGTM

:shipit:

Instead of doing sanity checks in the Kubernetes provider, we just
accept any non-empty value from the annotation and rely on the server
part to filter out unknown rules.

This allows us to automatically stay in sync with the currently
supported Path matchers/modifiers.
@ldez ldez force-pushed the kubernetes-remove-path-rules branch from 2d74241 to 1757671 Compare May 23, 2017 15:08
@ldez ldez merged commit b392023 into traefik:v1.3 May 23, 2017
@ldez ldez changed the title [k8s] Remove rule type path list. Remove rule type path list. May 25, 2017
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