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

Fix kubernetes path prefix rule with rewrite-target #2461

Merged
merged 1 commit into from
Nov 27, 2017

Conversation

cheungpat
Copy link
Contributor

When rewrite-target is specified, any existing PathPrefix rule is
removed. Hence an ingress with rewrite-target is always matched
for the same host regardless of path prefix.

@timoreimann
Copy link
Contributor

I think this makes sense.

@dtomcej @emilevauge WDYT?

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.

Good catch @cheungpat!

LGTM
:shipit:

@dtomcej
Copy link
Contributor

dtomcej commented Nov 25, 2017

I approved because I can't think that the code would be intended to function the way it was written :P I assume that @cheungpat's PR was what was intended.

@ldez ldez added this to the 1.5 milestone Nov 25, 2017
Copy link
Contributor

@m3co-code m3co-code left a comment

Choose a reason for hiding this comment

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

LGTM with same reasoning as @dtomcej said.

@ldez ldez added kind/bug/fix a bug fix and removed kind/enhancement a new or improved feature. labels Nov 25, 2017
@timoreimann
Copy link
Contributor

@ldez do we plan to make another 1.4 patch release, or is it okay to let the fix go into the upcoming 1.5 / master?

@ldez ldez modified the milestones: 1.5, 1.4 Nov 27, 2017
When rewrite-target is specified, any existing PathPrefix rule is
removed. Hence an ingress with rewrite-target is always matched
for the same host regardless of path prefix.
@ldez ldez changed the base branch from master to v1.4 November 27, 2017 10:09
@traefiker traefiker merged commit 0f09551 into traefik:v1.4 Nov 27, 2017
@cheungpat
Copy link
Contributor Author

Thanks for merging the pull request!

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.

6 participants