VaultConfigSource in quarkus-vault cannot be configured with properties requiring property expansion

[knutwannheden]

Describe the bug
The VaultConfigSource code doesn't use the Config#getValue() API to read its own config properties. Instead it uses Config#getConfigSources() and manually extracts the config values from these ConfigSources. (Presumably this is because there would otherwise be an endless recursion which would have to be addressed.) As a consequence, however, the corresponding config properties can not be configured with values which require property expansion, since VaultConfigSource doesn't contain the logic to do the property expansion.

I think that VaultConfigSource should try to directly use Config#getValue(). That would probably require logic to detect and avoid an endless recursion in VaultConfigSource#getValue(String). Or can SmallRyeConfigBuilder be used (subclassed?) somehow to avoid the recursion?

Expected behavior
VaultConfigSource should also be configurable with property values requiring property expansion. E.g.

my-vault-host=localhost
quarkus.vault.url=http://${my-vault-host}:8200

Actual behavior
Specifying config values which require expansion doesn't work.

To Reproduce
See above.

Configuration
See above.

[sberyozkin]

CC @vsevel @radcortez (Roberto, may be you can suggest some latest config feature)

[radcortez]

Yes, we just added a feature in SmallRye Config that allows a ConfigSource to configure itself:
https://smallrye.io/docs/smallrye-config/config-sources/config-sources.html

https://github.com/smallrye/smallrye-config/blob/master/implementation/src/main/java/io/smallrye/config/ConfigSourceFactory.java

This allows you to create the ConfigSource and access a ConfigSourceContext object that has access to the ConfigSources already initialized. These are the ConfigSources with higher priority than the Factory.

[radcortez]

This is not released yet, so we may need a few more days to release and update Quarkus.

[knutwannheden]

That sounds interesting. Certainly something we could also use in a custom ConfigSource implementation in our project.

[sberyozkin]

@radcortez Super

[vsevel]

smallrye/smallrye-config#196 was fixed by smallrye/smallrye-config#313 as part of milestone 1.9.0. waiting for the release.

[radcortez]

We are finally here :)

Sorry for taking so much time, we were working on another big feature that took longer to complete. PR is opened for 1.9.0. When we are done with that, you can configure ConfigSources in this way:
https://smallrye.io/docs/smallrye-config/config-sources/config-sources.html

Or if you prefer to see some code, I've updated the ZooKeeper source to use this:
https://github.com/smallrye/smallrye-config/tree/master/sources/zookeeper/src/main/java/io/smallrye/config/source/zookeeper

[knutwannheden]

Very cool!

[vsevel]

@radcortez hello. I have started to look at the new feature. I am missing the capability to list all properties given a particular prefix (e.g. quarkus.vault.), for instance when building the list of vault paths.
is there a better way to do it? if not do you think that is something that could be added?
from what I see however, this would still require the VaultConfigSourceFactory to recode the logic of building the config objects that are normally built by quarkus. in that sense, #4848 would have been a much better option I believe.

[radcortez]

We already have that, is just not exposed in the ConfigSourceContext API. It should be fairly easy to add the list of properties as well.

Yes, the Factory will handle the init code. I haven't tried to use it as a build step, but we should be able to record it as a ServiceProvider. We need to try that.

[radcortez]

@vsevel let me know if this works for you: smallrye/smallrye-config#402