Skip to content

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 force-pushed the fix-rewrite-target-rule branch from fbca00e to 6fa6491 Compare November 27, 2017 10:09
@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