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

[resteasy-reactive] unrecognized configuration key "quarkus.security.jaxrs.deny-unannotated-endpoints" #25591

Closed
PhilippParis opened this issue May 16, 2022 · 11 comments · Fixed by #26967
Assignees
Labels
area/config kind/bug Something isn't working
Milestone

Comments

@PhilippParis
Copy link

Describe the bug

when using resteasy-reactive in combination with the configuration quarkus.security.jaxrs.deny-unannotated-endpoints quarkus prints an "unrecognized configuration key" warning on startup
altough the configuration is recognized and works as expected.

when using resteasy-classic, no such warning is printed

Expected behavior

quarkus starts without "unrecognized configuration key" warning

Actual behavior

warning on startup:

Unrecognized configuration key "quarkus.security.jaxrs.deny-unannotated-endpoints" was provided; it will be ignored; verify that the dependency extension for this configuration is set or that you did not make a typo

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

openjdk version "11.0.11" 2021-04-20

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.9.0-Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.6.0 Maven home: /usr/share/maven Java version: 11.0.11, vendor: Ubuntu, runtime: /usr/lib/jvm/java-11-openjdk-amd64

Additional information

No response

@PhilippParis PhilippParis added the kind/bug Something isn't working label May 16, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented May 16, 2022

@geoand
Copy link
Contributor

geoand commented May 16, 2022

@radcortez to fix this, we would need a new build item that would remove items from unknownBuildProperties in BuildTimeConfigurationReader. If that is acceptable, I can open a PR

@radcortez
Copy link
Member

Hum, do we have a different rule or behaviour for this particular configuration property between classic and reactive?

@geoand
Copy link
Contributor

geoand commented May 16, 2022

No, the issue with this is that we have the exact same configuration class for both RESTEasy Classic and Reactive, but we cannot have a Quarkus config class for both, because otherwise an application that mixes RESTEasy Classic and Reactive will get errors that are totally non-actionable (as opposed to now, where there is a great error message stating that the two can't be mixed).

@radcortez
Copy link
Member

Ah, I see this here:

// we do this in order to avoid having 'io.quarkus.resteasy.reactive.common.runtime.JaxRsSecurityConfig' conflict with 'io.quarkus.resteasy.runtime.JaxRsSecurityConfig'
Optional<Boolean> denyUnannotatedEndpointsConfig = config
.getOptionalValue("quarkus.security.jaxrs.deny-unannotated-endpoints", Boolean.class);
Optional<List<String>> defaultRolesAllowedConfig = config
.getOptionalValues("quarkus.security.jaxrs.default-roles-allowed", String.class);

Do we have something that prevents double mapping in the ConfigRoot? Can't remember, I'll have to check. I don't see a reason for that. Did you tried that before? Did you got an error? Do you remember?

@geoand
Copy link
Contributor

geoand commented May 16, 2022

The exception looks something like:

java.lang.RuntimeException: java.lang.IllegalArgumentException: Multiple matching properties for name "security.jaxrs.deny-unannotated-endpoints" property was matched by both public boolean io.quarkus.resteasy.reactive.common.runtime.JaxRsSecurityConfig.denyJaxRs and public boolean io.quarkus.resteasy.runtime.JaxRsSecurityConfig.denyJaxRs. This is likely because you have an incompatible combination of extensions that both define the same properties (e.g. including both reactive and blocking database extensions)

(an example reported can be found here)

@radcortez
Copy link
Member

I understand that we would add the build step to remove such cases. I'm just not sure if this is something that we want to add since it will create an exception to our current configuration rules.

I'm wondering if we should lift the restriction of the double mapping. I believe it should be ok. I think it is perfectible acceptable for different extensions to refer to the same configuration names (especially considering that you can get the values programmatically anyway).

@geoand
Copy link
Contributor

geoand commented May 16, 2022

I'm wondering if we should lift the restriction of the double mapping. I believe it should be ok. I think it is perfectible acceptable for different extensions to refer to the same configuration names (especially considering that you can get the values programmatically anyway).

In sounds fine in theory :)

@radcortez
Copy link
Member

Ok, I'll have a look into this.

@geoand
Copy link
Contributor

geoand commented May 16, 2022

👍🏼

@radcortez
Copy link
Member

Lifting the @ConfigRoot restriction for the double mapping requires some work (because internally, the backing classes do not support it). @ConfigMapping does not have that restriction, so I use that instead to avoid the warnings.

@quarkus-bot quarkus-bot bot added this to the 2.12 - main milestone Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants