YAML configuration ignored for JAX-RS Dynamic Features

[essobedo]

Describe the bug

In my projects, I use the YAML extension and after migrating to Quarkus 2.0, I realized that my YAML configuration is no more taken into account in case of JAX-RS Dynamic Features.

Indeed in my projects, I use a DynamicFeature to manage dynamically the authentication / authorization, and this feature is configured indirectly thanks to a couple of @ConfigProperty whose value is located into YAML configuration file. With migrating to Quarkus 1.13 and before, the value of the properties could be retrieved from the YAML file, since Quarkus 2.0, I end up with an error of type java.util.NoSuchElementException: SRCFG00014: The config property <property name> is required but it could not be found in any config source

Expected behavior

The value of my properties can be retrieved from the YAML as before

Actual behavior

The value of my properties can not be retrieved from the YAML file since this change because the initialization of the DynamicFeatures is done during the static init phase while "Discovered Sources" (sources retrieved thanks to the ServiceLoader via ConfigSource or ConfigSourceProvider) are now only added at startup time such that the YAML files are not taken into account to initialize the DynamicFeature's instance.

To Reproduce

Link to a small reproducer (preferably a Maven project if the issue is not Gradle-specific).

Or attach an archive containing the reproducer to the issue.

Steps to reproduce the behavior:

1. Adds the yaml extension to your project
2. Adds a DynamicFeature containing a field annotated with a @ConfigProperty
3. Adds a file application.yaml containing a value of the property defined in step 2

Environment (please complete the following information):

Output of uname -a or ver

Darwin MBP-A36950NF 18.7.0 Darwin Kernel Version 18.7.0: Mon May 3 20:41:19 PDT 2021; root:xnu-4903.278.68~1/RELEASE_X86_64 x86_64

Output of java -version

openjdk version "11.0.10" 2021-01-19
OpenJDK Runtime Environment (build 11.0.10+9)
OpenJDK 64-Bit Server VM (build 11.0.10+9, mixed mode)

Quarkus version or git rev

2.0.0.Final

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

Maven home: /usr/local/Cellar/maven/3.6.3_1/libexec
Java version: 11.0.10, vendor: Oracle Corporation, runtime: /usr/local/Cellar/openjdk@11/11.0.10/libexec/openjdk.jdk/Contents/Home
Default locale: fr_FR, platform encoding: UTF-8
OS name: "mac os x", version: "10.14.6", arch: "x86_64", family: "mac"

[essobedo]

I added a PR to show the use case #18238, the fix proposal simply defines the YAML ConfigSourceProviders as AdditionalStaticInitConfigSourceProviderBuildItem, but I don't think that it is the expected fix.

Hi @radcortez WDYT of this ticket? Do you consider it as a bug? If not how to workaround it?

[radcortez]

We intentionally removed this due to #8145.

Ideally, I would like to remove all sources, because static init is not a good place to discover or add new sources.

First, there is one issue where ConfigRoot extensions objects may get their value overridden, even they are only build time (we record all build-time values to later reconstructs the ConfigRoots, but in static init the value may be overridden, which shouldn't happen). Second, not all sources are going to work in static init. YAML will work, but sources like Consul or Vault will not, because the required dependencies are not available yet. For me, this is a huge consistency issue that I would like to avoid.

I've been slowly doing a lot of changes to be able to reach that goal. The next one will be to actually rewrite the RESTEasy sources due to #5492.

Can you please provide me an example of your DinamicFeature? I'm guessing that the configuration you pass cannot be build time, right?

[essobedo]

We intentionally removed this due to #8145.

Ideally, I would like to remove all sources, because static init is not a good place to discover or add new sources.

First, there is one issue where ConfigRoot extensions objects may get their value overridden, even they are only build time (we record all build-time values to later reconstructs the ConfigRoots, but in static init the value may be overridden, which shouldn't happen). Second, not all sources are going to work in static init. YAML will work, but sources like Consul or Vault will not, because the required dependencies are not available yet. For me, this is a huge consistency issue that I would like to avoid.

I've been slowly doing a lot of changes to be able to reach that goal. The next one will be to actually rewrite the RESTEasy sources due to #5492.

I see, I guess that the right fix could be to make sure that the the DynamicFeatures are not initialized during the static init phase which means that there is probably some work to be done in Resteasy

Can you please provide me an example of your DynamicFeature?

You have an example in the PR #18238

I'm guessing that the configuration you pass cannot be build time, right?

That's right, my configuration is more a runtime configuration set with environment variables provided to my docker container in GCP.

---

Do you agree that it is something that should be fixed? And if so in which version of Quarkus could I expect to have a fix?

As it seems to be complex to have a proper fix, what do you think of the temporary fix/workaround in my PR? It is not ideal for sure as it has some limitations (##8145 and static init of DynamicFeature which can be a problem in native mode) but at least it helps on most use cases.

BTW: If you need help on finding a better fix (even in Resteasy), please tell me what you have in mind, I can try to propose a better fix if you want and/or if it can be helpful

[radcortez]

I see, I guess that the right fix could be to make sure that the the DynamicFeatures are not initialized during the static init phase which means that there is probably some work to be done in Resteasy

I'm not sure. I need to have a look, but If I remember correctly, the features are supplied as providers during build time and RESTEasy starts at static init. I don't believe there is a way to supply additional providers afterward.

Do you agree that it is something that should be fixed? And if so in which version of Quarkus could I expect to have a fix?

We can try to find a way to make it work, but I also want to keep consistency. Right now, we may allow the YAML source to work, but as I mentioned other sources will not work and I believe this will be confusing for other users.

As it seems to be complex to have a proper fix, what do you think of the temporary fix/workaround in my PR? It is not ideal for sure as it has some limitations (##8145 and static init of DynamicFeature which can be a problem in native mode) but at least it helps on most use cases.

A possible workaround is to just call the programmatic API, and retrieve the value dynamically. The Provider / Feature even has other limitations like the one reported in #7607. Again this is a case where you need to know how to call the API, which is not great for user experience.

BTW: If you need help on finding a better fix (even in Resteasy), please tell me what you have in mind, I can try to propose a better fix if you want and/or if it can be helpful

We probably need to brainstorm a bit about this. My initial thought is to investigate if it is possible to register additional providers at runtime. If it is, maybe we can defer the registration of providers that contain config injection to that phase, but we may still have some chicken / egg issues (sources that require the rest client to retrieve values, will not have the configurable provided registered).

[ekasumov]

@radcortez This bug doesn't seem limited to DynamicFeatures.

I have the following configuration in application.yml

jwt:
  secret:
    key: 1234567890

when I attempt to retrieve it with ConfigProvider, I get a an error "The config property jwt.secret.key is required but it could not be found in any config source"
String secret = ConfigProvider.getConfig().getValue("jwt.secret.key", String.class);

If I use ConfigProperty(name = "jwt.secret.key"), I get a null value.

If I add an application.properties file, which contains only jwt.secret.key=1234567890, everything works fine.

This appears to be an overall regression of YAML config parsing.

[radcortez]

@ekasumov

The explanation is that you are calling that code on static init, where all sources were being added. We removed the discovery and kept them at the very minimum for compatibility reasons, but the expectation is that we remove the reads from application.properties as well.

I guess that we can readd the application.yaml to quickly fix what you and @essobedo are experiencing. You need to be aware that you may have non-specified behavior between configuration in static init vs runtime. Most likely you never experienced this because you stick with the default sources, but we have seen this behavior when other users add their own custom sources. For instance, a rest-based config source can never work during static init because you get a chicken and egg problem.

@ekasumov, the idea is to find some alternate ways for things that require configuration at static init to be lazily initialized during runtime. There is a lot of work to do in that area, so is not something that we can do in a single PR, so I understand the frustration.

If possible, can you please let me know what is your use case? What are you trying to configure?

[ekasumov]

@ekasumov

The explanation is that you are calling that code on static init, where all sources were being added. We removed the discovery and kept them at the very minimum for compatibility reasons, but the expectation is that we remove the reads from application.properties as well.

I guess that we can readd the application.yaml to quickly fix what you and @essobedo are experiencing. You need to be aware that you may have non-specified behavior between configuration in static init vs runtime. Most likely you never experienced this because you stick with the default sources, but we have seen this behavior when other users add their own custom sources. For instance, a rest-based config source can never work during static init because you get a chicken and egg problem.

@ekasumov, the idea is to find some alternate ways for things that require configuration at static init to be lazily initialized during runtime. There is a lot of work to do in that area, so is not something that we can do in a single PR, so I understand the frustration.

If possible, can you please let me know what is your use case? What are you trying to configure?

@radcortez We're just retrieving a string token. There are several simple configurations for a %test profile, and something as simple as running ./gradlew test fails, because the YAML configuration isn't getting detected.