-
Notifications
You must be signed in to change notification settings - Fork 583
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 redirect failures from apache client 5.x #5996
Fix redirect failures from apache client 5.x #5996
Conversation
Provide followRedirect to remove Authorization. Fixes spring-cloud#5989
Provide followRedirect to remove Authorization. Fixes spring-cloud#5989
...org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
Outdated
Show resolved
Hide resolved
...org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
Outdated
Show resolved
Hide resolved
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.
I like the switch to use Netty client but I think we are missing a huge chunk of logic that used to reside in the drop header strategy. Am I missing something?
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.
Excellent work @corneil .
The move to Reactor Netty "is the way" and has greatly reduced the complexity of this class. I am glad you stayed firm on getting this in - worth the wait!
A few minor comments/suggestions - but after those are resolved we should be good to go as far as I'm concerned.
...org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
Outdated
Show resolved
Hide resolved
...org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
Outdated
Show resolved
Hide resolved
...org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
Show resolved
Hide resolved
...org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
Outdated
Show resolved
Hide resolved
...org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
Outdated
Show resolved
Hide resolved
...org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
Outdated
Show resolved
Hide resolved
...org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
Show resolved
Hide resolved
.../dataflow/container/registry/authorization/DropAuthorizationHeaderOnContainerTestManual.java
Outdated
Show resolved
Hide resolved
.../dataflow/container/registry/authorization/DropAuthorizationHeaderOnContainerTestManual.java
Outdated
Show resolved
Hide resolved
.../dataflow/container/registry/authorization/DropAuthorizationHeaderOnContainerTestManual.java
Show resolved
Hide resolved
…on on using for a manual test. Updated ContainerImageRestTemplateFactory with documentation on extra and improved formatting. Refactored CacheKey in ContainerImageRestTemplateFactory to be a Java Record and include extra to ensure correct behaviour for RestTemplate obtained.
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.
Great work @corneil - LGTM!
Provided alternative by using Reactor Netty HttpClient that does provide proper redirect handling.
Reason for redirect handler was in deleted
DropAuthorizationHeaderRequestRedirectStrategy