Skip to content

Conversation

radcortez
Copy link
Member

@radcortez radcortez commented Aug 20, 2025

This exposes only the defaults of AuthRuntimeConfig and CORSConfig in a new mapping DefaultsAuthConfig.

This uses @ConfigMapping @WithDefaults @WithUnnamedKey with Map to populate defaults for any key. See https://smallrye.io/smallrye-config/Latest/config/mappings/#maps

The runtime configuration is available in the Map associated with the null key. A lookup to the Map with any other keys, yields the config object with the defaults. It is still possible to influence these defaults by using the named key defaults as part of the path of quarkus.http.auth.defaults.* and quarkus.http.cors.defaults.*. This is only valid for DefaultsAuthConfig.

@radcortez
Copy link
Member Author

@michalvavrik, here is an alternate proposal.

There is no need for a dummy implementation of VertxHttpConfg, and you could use AuthDefaultsConfig directly if you prefer.

@michalvavrik
Copy link
Member

@michalvavrik, here is an alternate proposal.

There is no need for a dummy implementation of VertxHttpConfg, and you could use AuthDefaultsConfig directly if you prefer.

Thanks @radcortez , I think this is better. Yes, we will use AuthDefaultConfig directly. If you want, we can get in this fix and I'll adapt security code to drop the dummy implementation of VertxHttpConfig. Whatever works for you. I checked and we only need the auth and cors so this is safe change.

This comment has been minimized.

Copy link

github-actions bot commented Aug 20, 2025

😭 Deploy PR Preview failed.

@michalvavrik
Copy link
Member

FYI #49647 fixes the reason why the CORS builder was called so many times. But this PR is proper fix for the linked issue.

Copy link
Member

@michalvavrik michalvavrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the AuthDefaultConfig yields the result we need (config defaults) and should be more effective. It is a new class, but we need these defaults, so I am +1. Let's see CI as we have test scenarios verifying this.

This comment has been minimized.

@radcortez radcortez force-pushed the fix-49642-alternative branch from 62231d9 to 9514076 Compare August 20, 2025 22:14
@radcortez
Copy link
Member Author

Thanks @radcortez , I think this is better. Yes, we will use AuthDefaultConfig directly. If you want, we can get in this fix and I'll adapt security code to drop the dummy implementation of VertxHttpConfig. Whatever works for you. I checked and we only need the auth and cors so this is safe change.

No worries, I've dropped the dummy class and using the new mapping directly.

@radcortez
Copy link
Member Author

Also renamed AuthDefaultsConfig to DefaultsAuthConfig

Copy link

quarkus-bot bot commented Aug 20, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 9514076.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented Aug 21, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 9514076.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
Native Tests - Data3 ⚠️ Check → Logs Raw logs 🚧

You can consult the Develocity build scans.

Copy link
Member

@michalvavrik michalvavrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sberyozkin sberyozkin merged commit ba0134f into quarkusio:main Aug 21, 2025
58 of 59 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.28 - main milestone Aug 21, 2025
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.

Logs are flooded with SRCFG01008 warning in dev mode
3 participants