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
support not follow redirect for HTTP client #702
Conversation
2c2bf91
to
9d33b3a
Compare
Please add a test case with redirect set to false for a given client using example/example-gateway/config/*.yaml |
9d33b3a
to
4ebe1eb
Compare
4ebe1eb
to
0b5485a
Compare
baseURL, | ||
map[string]string{}, | ||
time.Second, | ||
false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rpatali does this test case address your ask? in this test case, it creates the gateway via exampleGateway and use example gateway's config files. And this test case is to verify the client call doesn't follow the redirect, so the caller will receive 303 status code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is fine but additional also set the config value for clients.*.followRedirects = false
for one of the clients in the .json to test the behavior since that is what you are going to need.
runtime/client_http_response_test.go
Outdated
assert.NoError(t, err) | ||
} | ||
|
||
fakeNormal := func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't seem to be used.
runtime/client_http_response_test.go
Outdated
} | ||
|
||
bgateway.HTTPBackends()["bar"].HandleFunc("POST", "/bar/echo", fakeEcho) | ||
bgateway.HTTPBackends()["bar"].HandleFunc("GET", "/bar-path", fakeNormal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't seem to be used, remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sense, since we don't follow the redirect, we don't need it anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Enable HTTP clients to have a config to choose whether it wants to follow HTTP redirect or not. In other words, if the HTTP client does not follow the redirect, the caller will receive 30* Status with Location header