Skip to content

Fix CORS configuration timing issue with RedisRouteDefinitionRepository and RefreshRoutesEvent #3778

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

Conversation

PeterMue
Copy link
Contributor

CORS configuration is based on outdated RouteDefinitions when using Redis and RefreshRoutesEvent, see gh-3774.

Changing the event type in CorsGatewayFilterApplicationListener to use the RefreshRoutesResultEvent ensures the CORS configuration happens after the reload of the definitions completed and the RouteLocator contains the latest RouteDefinitions.

@PeterMue PeterMue marked this pull request as ready for review April 22, 2025 16:07
@PeterMue PeterMue force-pushed the gh-3774-cors-configuration-on-redis-refresh branch from f9a1115 to 6ae1a4c Compare May 5, 2025 08:38
@ryanjbaxter
Copy link
Contributor

Could you submit this PR agains the 4.1.x branch?

@PeterMue PeterMue force-pushed the gh-3774-cors-configuration-on-redis-refresh branch from 6ae1a4c to 3f18f6c Compare May 13, 2025 07:18
@PeterMue PeterMue changed the base branch from main to 4.1.x May 13, 2025 07:20
@PeterMue
Copy link
Contributor Author

PeterMue commented May 13, 2025

Could you submit this PR agains the 4.1.x branch?

Sure, it's done.
Could you re-run the DCO check? IMO this should be fine already and the fail comes from the base branch change after the push.

@ryanjbaxter
Copy link
Contributor

Can you merge in the latest changes from main?

@PeterMue PeterMue force-pushed the gh-3774-cors-configuration-on-redis-refresh branch from 3f18f6c to 3787cf4 Compare May 14, 2025 08:52
@PeterMue
Copy link
Contributor Author

I'm not sure if that was what you wanted me to do. I rebased against 4.1.x and merged into the main branch. Doesn't seem quite right - does it?

@spencergibb
Copy link
Member

You have to force push

@ryanjbaxter
Copy link
Contributor

I realize your branch was still based on main, and luckily the changes merge cleanly into 4.1.x. I just wanted you to pull in the latest changes from main into your fork so the build passes and we make sure nothing else broke.

Configure CORS after refresh routes completed on RefreshRoutesResultEvent

Signed-off-by: PeterMue <[email protected]>
@PeterMue PeterMue force-pushed the gh-3774-cors-configuration-on-redis-refresh branch from 3787cf4 to 8a98421 Compare May 14, 2025 15:04
@PeterMue
Copy link
Contributor Author

I again rebased mine one the upstream spring-cloud:4.1.x branch.
Now the PR looks like i would expect it to look.

If that's still not right, you have to explain me what i should do, i'm a little confused right now.

@spencergibb
Copy link
Member

looks better thanks

@ryanjbaxter ryanjbaxter merged commit d1af099 into spring-cloud:4.1.x May 14, 2025
2 checks passed
@PeterMue PeterMue deleted the gh-3774-cors-configuration-on-redis-refresh branch May 15, 2025 06:30
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.

CORS configuration is based on outdated RouteDefinitions when using Redis and RefreshRoutesEvent
4 participants