String List Maps and String Maps properties are not mapped in BOOTSTRAP phase

[vsevel]

Describe the bug
Configuration supports mapping from properties files to complex types such as:

• public Map<String, String> stringMap;
• public Map<String, List<String>> stringListMap;
• public List<String> stringList;

However, when those properties are used in a config loaded during the BOOTSTRAP phase, only the stringList is correctly mapped. The other two stringMap and stringListMap are initialized to an empty map, and there are no warns saying that the properties are unknown or unexpected.

If we change the phase to be BUILD_AND_RUN_TIME_FIXED instead of BOOTSTRAP, those properties get mapped appropriately.

Expected behavior
stringMap and stringListMap should be mapped, just as stringList.

To Reproduce
Steps to reproduce the behavior:

1. get my branch from [WIP] Move VaultRuntimeConfig to BOOTSTRAP phase #9989
2. ./mvnw clean install -o -Dvault-test-extension.use-tls=true -Dtest-vault -pl extensions/vault/deployment -pl extensions/vault/runtime -pl integration-tests/vault -Dit.test=VaultMultiPathConfigITCase
3. you should see:

DEBUG VAULT ===> stringListMap = {}
DEBUG VAULT ===> stringMap = {}
DEBUG VAULT ===> stringList = [value1, value2]

4. change phase to BUILD_AND_RUN_TIME_FIXED in VaultRuntimeConfig, and rerun the test, you should see:

DEBUG VAULT ===> stringListMap = {key1=[value1, value2, value3], key2=[value4, value5], key3=[value6]}
DEBUG VAULT ===> stringMap = {key1=value1, key2=value2, key3=value3}
DEBUG VAULT ===> stringList = [value1, value2]

Configuration

quarkus.vault.string-list-map.key1=value1,value2,value3
quarkus.vault.string-list-map.key2=value4,value5
quarkus.vault.string-list-map.key3=value6
quarkus.vault.string-map.key1=value1
quarkus.vault.string-map.key2=value2
quarkus.vault.string-map.key3=value3
quarkus.vault.string-list=value1,value2

Environment (please complete the following information):
Running from HEAD

[geoand]

Before I start looking into it, @dmlloyd any quick idea why Map isn't working in BOOTSTRAP config?

[dmlloyd]

I'd check to see if it works with RUNTIME before reaching any conclusion. But I might guess that the configuration is being read before the run time configuration is available.

Also we're already overusing BOOTSTRAP. It will lead to trouble...

[geoand]

Also we're already overusing BOOTSTRAP. It will lead to trouble...

What do you have in mind?

[dmlloyd]

SmallRye Config has recently gained the ability to configure one source from another source (using the interceptors infrastructure). This allows everything to stay in RUNTIME, while still enabling some configuration properties to be used to set up subsequent sources.

[geoand]

Gotcha, thanks!

@vsevel maybe you want to hold off this PR for the time being? If we can completely do away with BOOTSTRAP config soon, that would be great :)

[vsevel]

@geoand sure. is there any issue I could relate to, so that I can catch the notification and know when something is ready?

@dmlloyd in vault's case (and I suspect for consul also), the goal is not to use a config source to configure another, but instead to use an extension (e.g. vault) from a config source (e.g. vault config source). and that extension should be able to be implemented using regular cdi beans.
will SmallRye Config allow that type of use cases?

this would be a great simplification for the extension itself. right now I have to recode by hand the logic to fetch the vault properties.

[geoand]

@geoand sure. is there any issue I could relate to, so that I can catch the notification and know when something is ready?

@radcortez would know more

[radcortez]

@vsevel @geoand this is still dependent on #9184

the goal is not to use a config source to configure another, but instead to use an extension (e.g. vault) from a config source (e.g. vault config source). and that extension should be able to be implemented using regular cdi beans.
will SmallRye Config allow that type of use cases?

@vsevel not quite sure if I understood correctly what you want to do. Can you provide me more details?

[vsevel]

hello @radcortez
I agree my statement was a bit mysterious and confusing ;)
let's try to make it clearer:

the vault extensions can be used as:

• a config source: if you have foo=bar in vault at secret/myapp, then you will be able to inject property ${foo} from within the app
• a programmatic api through managed beans, such as VaultKVSecretEngine

Obviously those 2 use cases share the code to read from the vault through its rest api. and they both need to be configured somehow, at minimum the vault url, represented by the quarkus.vault.url property.

the issue I faced when writing the config source was that I could not benefit from property objects such as VaultRuntimeConfig at the time the vault config source gets called on public String getValue(String propertyName). As I understand it, config sources are called too early to benefit from other config sources, or simply said there is no dependency that we can set up between the vault config source and the application properties config source for instance. As a result I had to reimplement property loading for the vault config source use case. and that hurts.

in the original vault PR, @gsmet made some comments about this, reached out to @dmlloyd, who gave some hint about what could be done in this #2897 (comment)
I ended up creating #4848, which was closed by @geoand because the BOOSTRAP phase was seen as a solution.

so ideally I would have a way to create a VaultConfigSource that gets injected with a VaultRuntimeConfig, which gets loaded from other lower level config sources (e.g. system property, environment variables, application.properties).

if we go a step further, in an ideal world, the system would:

• load low level config sources, which will allow building a VaultRuntimeConfig
• initialize the vault extension, and specifically the VaultKVSecretEngine
• initialize the vault config source, which would access vault using the VaultKVSecretEngine from the extension itself

that way I would not have to come up with a VaultManager singleton, that is used by the vault config source..

so we are talking about 2 challenges:

• avoid recoding property loading logic in the vault config source
• allow the vault config source to use cdi managed objects from the extension itself

solving just the first one would be a nice improvement in itself.

may be I have missed somethign in the way config objects, config sources and managed objects get created. and we could do already a lot better. if so let me know.

[radcortez]

Ah, now I understand. Thank you.

I think I have a solution for you :)

In SmallRye Config (which Quarkus uses as the Config implementation), we added support for ConfigSources to configure themselves. This is how it looks:
https://smallrye.io/docs/smallrye-config/config-sources/config-sources.html

It should be straightforward. You provide your ConfigSource via a ConfigSourceFactory, which gives you access to a ConfigSourceContext where you can call getValue to retrieve configuration from already instantiated sources.

This is not released in SmallRye Config, so is Quarkus does not have access to it either, but we should push a release soon and then update Quarkus. Let me know if this works for you. There is still chance to improve it or add things if required.

[vsevel]

@radcortez sounds promising.
I suppose however that I will not get a VaultRuntime, but only get the ability to do some getValue as you say. so I think we are talking about being able to drop my reimplementation of getVaultProperty, but not my loadRuntimeConfig in particular.
I do not think you can do better in smallrye config; it would have to be done in quarkus, because it is the one doing the mapping between the config sources and the mapped config objects.
I will give it a try as soon as it is available in quarkus.

[radcortez]

I suppose however that I will not get a VaultRuntime, but only get the ability to do some getValue as you say. so I think we are talking about being able to drop my reimplementation of getVaultProperty, but not my loadRuntimeConfig in particular.

Yes. But not only that, you can remove the init code, since you will have access to the Config Context at the constructor level, so no need to play around with Atomic values :)

I do not think you can do better in smallrye config; it would have to be done in quarkus, because it is the one doing the mapping between the config sources and the mapped config objects.

Actually, we can :) This is being worked on at the moment:
smallrye/smallrye-config#333

But probably still a few weeks away.

I will give it a try as soon as it is available in quarkus.

Thank you!

[vsevel]

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

[vsevel]

the issue was in the vault extension. see #13300