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

Validate that the injected config class has to be the entry point of the @ConfigMapping #39431

Open
michalvavrik opened this issue Mar 14, 2024 · 8 comments
Labels
area/config kind/enhancement New feature or request

Comments

@michalvavrik
Copy link
Member

michalvavrik commented Mar 14, 2024

Describe the bug

I'm trying to migrate Elytron Security extensions and I can see that inside buildstep io.quarkus.elytron.security.properties.deployment.ElytronPropertiesProcessor#configureFileRealmAuthConfig values are fine, but when I put breakpoint inside recorded method io.quarkus.elytron.security.runtime.ElytronPropertiesFileRecorder#loadRealm(io.quarkus.runtime.RuntimeValue<org.wildfly.security.auth.server.SecurityRealm>, io.quarkus.elytron.security.runtime.PropertiesRealmConfig) values are gone.

This is commit that causes the issue 840ac57

UPDATE: see Roberto explanation below.

Expected behavior

Worked for config classes.

Actual behavior

Passed config interface is not null, however when methods are called, they provide none or false (for booleans). I tried to add the RuntimeConfigSetupCompleteBuildItem without success. Inside buildstep, values are correct.

How to Reproduce?

Reproducer:

  1. git clone
  2. cd quarkus
  3. git checkout feature/migrate-elytron-config-classes
  4. mvn -Dquickly
  5. mvn clean test -f extensions/elytron-security-properties-file/deployment/ -Dtest=BasicAuthTestCase

Please note there are other failures as result of config migration when you run all the tests in the deployment module, but I'm yet to analyze them. I checked code several times and I can't see what is wrong in my migration.

Output of uname -a or ver

Fedora 38

Output of java -version

21

Quarkus version or git rev

999-SNAPSHOT

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

Apache Maven 3.9.3

Additional information

No response

@michalvavrik michalvavrik added kind/bug Something isn't working area/config labels Mar 14, 2024
@michalvavrik
Copy link
Member Author

cc @radcortez

@radcortez
Copy link
Member

I'll have a look.

@michalvavrik
Copy link
Member Author

Thanks. I hope I didn't overlook something in my changes, but it was really weird.

@radcortez
Copy link
Member

I had a look, and the issue here is that @ConfigMapping cannot be recorded like the old @ConfigRoots, meaning that they have to be reconstructed from the Config instance in the recorder.

This does work, but the reference you need to use is the actual @ConfigMapping class. In this case, you are using a @ConfigGroup, which could be recorded because it was a plain class. When you change that to an interface, we don't know how to reconstruct it. This could be fixed by just using this method signature in the recorder:

public Runnable loadRealm(RuntimeValue<SecurityRealm> realm, SecurityUsersConfig securityUsersConfig) {

I guess we should throw a meaningful error if you try to inject a config class that is not the entry point to the mapping.

@michalvavrik
Copy link
Member Author

+1 for validation error

@michalvavrik
Copy link
Member Author

and thanks for analysis

@radcortez
Copy link
Member

Changing this to an enhancement to validate that the injected config class has to be the entry point of the mapping.

@michalvavrik
Copy link
Member Author

@michalvavrik michalvavrik changed the title Build time and runtime fixed @ConfigMapping has values inside buildstep but nulls inside recorded method Validate that the injected config class has to be the entry point of the @ConfigMapping Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants