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

ConfigMapping interface containing nested Map not being initialized properly if using VaultConfigSource #38869

Closed
wiebeck opened this issue Feb 19, 2024 · 9 comments
Labels

Comments

@wiebeck
Copy link
Contributor

wiebeck commented Feb 19, 2024

Describe the bug

Assume you have a config interface like this:

@ConfigMapping(prefix = "mycfg")
public interface MyConfig {

    @WithParentName
    Map<String, InnerConfig> innerConfigs();

    interface InnerConfig {
        String value();
    }

}

When specifying a config like this:

mycfg.x.value=d
mycfg.y.value=e
mycfg.z.value=f

I should get an innerConfigs map of size 3 containing the values d, e and f.

This works as expected if the config comes from e.g. a local properties file. It does not work as expected if the config comes from e.g. Vault provided by a VaultConfigSource.

Expected behavior

I assume that it should not matter what the source of the properties is and still get a fully populated MyConfig class offering access to all three instances of InnerConfig.

Actual behavior

When using Vault as config source the innerConfigs map is empty although I can still access the properties via

@ConfigProperty(name = "mycfg.x.value")
String xValue;

So they can be read successfully from Vault. It's just the mapping to the Map<String, InnerConfig> that seems to fail.

How to Reproduce?

I have a reproducer ready at https://github.com/wiebeck/quarkus-configmap-issue with more details.

Output of uname -a or ver

Darwin Kernel Version 23.2.0: Wed Nov 15 21:53:18 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6000 arm64

Output of java -version

openjdk version "21.0.1" 2023-10-17 LTS OpenJDK Runtime Environment Temurin-21.0.1+12 (build 21.0.1+12-LTS) OpenJDK 64-Bit Server VM Temurin-21.0.1+12 (build 21.0.1+12-LTS, mixed mode)

Quarkus version or git rev

3.7.3

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

Gradle 8.5

Additional information

The environment shouldn't matter since we encoutered the issue in an app running on Kubernetes in a Linux container.

@wiebeck wiebeck added the kind/bug Something isn't working label Feb 19, 2024
Copy link

quarkus-bot bot commented Feb 19, 2024

/cc @geoand (kubernetes), @iocanel (kubernetes), @vsevel (vault)

@vsevel
Copy link
Contributor

vsevel commented Feb 20, 2024

/cc @radcortez

@sberyozkin
Copy link
Member

Hi @wiebeck @vsevel, is it a duplicate of quarkiverse/quarkus-vault#231 ?

@radcortez
Copy link
Member

Hi @wiebeck @vsevel, is it a duplicate of quarkiverse/quarkus-vault#231 ?

No, this is something different.

@wiebeck
Copy link
Contributor Author

wiebeck commented Feb 20, 2024

Hi @wiebeck @vsevel, is it a duplicate of quarkiverse/quarkus-vault#231 ?

This issue is due to the more thorough validation of properties containing dashes ("-") usually passed as environment variables since Quarkus 3.7. Workaround is to use quarkus.config.mapping.validate-unknown=false.

@radcortez
Copy link
Member

This is caused by the VaultConfigSource not implementing getPropertyNames: https://github.com/quarkiverse/quarkus-vault/blob/81d276215926b3d106cd1d9af4476421103c704e/runtime/src/main/java/io/quarkus/vault/runtime/config/VaultConfigSource.java#L58-L61. Why is this important for Maps?

Consider:

@ConfigMapping(prefix = "acme")
interface MyConfig {
  String value();

  Map<String, String> map();
}

When populating the value(), the config name is well know: we take the prefix and convert the method name to generate acme.value and then we query the Config system for that name directly.

To populate a Map, things are a bit different, because Map keys are not known, due to the dynamic part of the key name, so the Config system has to query for all known properties under acme.map.*. This is where getPropertiesNames comes in. Because the Vault source does not return to use any known names, there is no way for us to known what to query for.

Ideally, all sources should implement getPropertyNames, but this may not be possible depending on the nature of the source itself. I'm not even sure if Vault provides an API to query for all names, and even if it does, you may now want to expose that for security reasons.

In such cases, you can provide the empty names in source with a lower ordinal (like application.properties), so the Config system is aware of them, and when they are queried directly, you will get the Vault value for each name. The downside is that you need to know beforehand the Map key names.

@radcortez radcortez closed this as not planned Won't fix, can't repro, duplicate, stale Feb 20, 2024
@wiebeck
Copy link
Contributor Author

wiebeck commented Feb 20, 2024

@radcortez Thanks for the explanation. This actually makes sense (btw. debugging smallrye-config is HELL!). I cannot provide the property names in application.properties as I don't know their names in advance but in my case it's possible to provide them as environment variable (they're not really secrets actually) which is working as expected.

@radcortez
Copy link
Member

it's possible to provide them as environment variable (they're not really secrets actually) which is working as expected.

Yes, Env vars or system properties work, too.

(btw. debugging smallrye-config is HELL!)

Sorry about that. There was a lot of generated code before (there still is), but I did a major rewrite of the mappings implementation and everything should be abstracted now and easier to debug. The generated code is mainly calling the ObjectCreator API. Please check:

https://github.com/smallrye/smallrye-config/blob/cd8d6eca210ee2339caffd47977cf28437f65c31/implementation/src/main/java/io/smallrye/config/ConfigMappingContext.java#L251

And you can even simulate the generated code:

https://github.com/smallrye/smallrye-config/blob/51245a1cd50ec50c98c1252519361247c879fd99/implementation/src/test/java/io/smallrye/config/ObjectCreatorTest.java#L110-L265

@radcortez
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants