Extend https redirection tests, and fix incorrect behavior #3742
Conversation
ldez
added
status/2-needs-review
kind/bug/fix
|
Many thanks for putting this together @dtomcej! Does this PR also fix a double-trailing slash when the ingress path ends with a slash? #3741 (comment) I looked through your changes, but could not find a new test case for this, so am wondering. Sorry in advance if that's a lame question – I don't know Go, so asking is the only option I have 😆 In any case, I'll be keen to test the result once the PR is merged! Is |
|
@kachkaev Yessir. The trailing slash with a This means that in the new test suite, anything referring to This PR increases the number of tests from ~15 to ~55 to improve coverage. If you would like to test out the code right away, I have pushed the image to |
|
I have just tried Many thanks for your work on this @dtomcej 🙌 💯 |
integration/https_test.go
Outdated
| @@ -751,114 +752,92 @@ func (s *HTTPSSuite) TestEntrypointHttpsRedirectAndPathModification(c *check.C) | |||
|
|
|||
| testCases := []struct { | |||
| desc string | |||
| host string | |||
| host []string | |||
There was a problem hiding this comment.
Could you please rename host into hosts
integration/https_test.go
Outdated
| }, | ||
| } | ||
|
|
||
| for _, test := range testCases { | ||
| test := test | ||
| for _, subtest := range test.host { |
There was a problem hiding this comment.
Could you please rename subtest into host
integration/https_test.go
Outdated
| c.Assert(err, checker.IsNil) | ||
| defer resp.Body.Close() | ||
| location := resp.Header.Get("Location") | ||
| expected := strings.Replace(test.expectedURL, "<HOST>", subtest, 1) |
There was a problem hiding this comment.
You should maybe replace by
expected := fmt.Sprintf(test.expectedURL, host)And replace all <HOST> occurrences in expectedURL by %s
| @@ -740,7 +741,7 @@ func (s *HTTPSSuite) TestEntrypointHttpsRedirectAndPathModification(c *check.C) | |||
| defer cmd.Process.Kill() | |||
|
|
|||
| // wait for Traefik | |||
| err = try.GetRequest("http://127.0.0.1:8080/api/providers", 500*time.Millisecond, try.BodyContains("Host: example.com")) | |||
| err = try.GetRequest("http://127.0.0.1:8080/api/providers", 1000*time.Millisecond, try.BodyContains("Host: example.com")) | |||
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
traefiker
added
bot/merge-retry-1
status/4-merge-in-progress
and removed
bot/merge-retry-1
labels
What does this PR do?
Rebuilds https redirection paths after being modified by middlewares
Motivation
Fixes #3741
More
replacePathto contextreplacePathRegexto contextstripPathPrefix