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 redirect failures from apache client 5.x #5996

Merged
merged 7 commits into from
Nov 7, 2024

Conversation

corneil
Copy link
Contributor

@corneil corneil commented Oct 18, 2024

Provided alternative by using Reactor Netty HttpClient that does provide proper redirect handling.

Reason for redirect handler was in deleted DropAuthorizationHeaderRequestRedirectStrategy

Provide followRedirect to remove Authorization.

Fixes spring-cloud#5989
Provide followRedirect to remove Authorization.

Fixes spring-cloud#5989
@corneil corneil requested review from cppwfs and onobc October 18, 2024 15:05
@corneil corneil added this to the 3.0.x milestone Oct 22, 2024
Copy link
Contributor

@onobc onobc left a 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?

@corneil corneil requested a review from onobc November 1, 2024 14:38
@onobc onobc added the review/all-required All reviewers need to sign off. label Nov 6, 2024
@onobc onobc changed the title fix redirect failures from apache client 5.x Fix redirect failures from apache client 5.x Nov 6, 2024
Copy link
Contributor

@onobc onobc left a 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.

…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.
@corneil corneil requested a review from onobc November 7, 2024 11:28
Copy link
Contributor

@onobc onobc left a 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!

@onobc onobc merged commit 08c082d into spring-cloud:main Nov 7, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review/all-required All reviewers need to sign off.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants