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

Migrate Elytron Security extensions from config classes to @ConfigMapping #39518

Merged

Conversation

michalvavrik
Copy link
Member

@michalvavrik michalvavrik commented Mar 18, 2024

Following extensions fits criteria for migration of security extensions from config classes to the @ConfigMapping, they are not used by Quarkiverse extensions and if users are injecting these config clasess, it must be very edge case (because I can't think of reasons it's needed after inspecting their reference guides):

Situation is more complex with the Elytron Security Properties File extension, it is used by Quarkiverse extensions, Kogito and Camel:

However it is used for integration and unit testing and they are not working with config classes there. For usages, I've cloned open source code and checked following projects:

Implementation note: @ConfigMappings are passed to recorded methods instead of config sub-classes due to #39431

PR note: feel free to squash the commits, I thought it would be better to separate commits in case we detect any issue with one of migrations

Copy link

quarkus-bot bot commented Mar 18, 2024

Status for workflow Quarkus CI

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

✅ 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.

@sberyozkin sberyozkin merged commit 7ce6f8d into quarkusio:main Mar 18, 2024
24 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.10 - main milestone Mar 18, 2024
@michalvavrik michalvavrik deleted the feature/migrate-elytron-config-classes branch March 18, 2024 12:14
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.

2 participants