Expanding ${keycloak.url}/realms/quarkus/ when keycloak.url is not defined leads to null value

[gsmet]

I stumbled upon this behavior when looking at #41326 and I was surprised by it.

I want to discuss this with Roberto when he's back. Creating this as a reminder.

[quarkus-bot]

/cc @pedroigor (keycloak), @sberyozkin (keycloak)

[radcortez]

Hum, that does not make much sense to me.

If the Config system cannot expand a partial expression, we throw an exception:
https://github.com/smallrye/smallrye-config/blob/d447b0c346e39a09c6d07c7d5ce8186bd4154037/implementation/src/test/java/io/smallrye/config/ExpressionConfigSourceInterceptorTest.java#L136-L142

I'll see what is going on.

[radcortez]

Ah, maybe the confusion is about how ConfigValue works, which is being used here in the method:

quarkus/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigUtils.java

Lines 119 to 122 in 78045ea

	public static boolean isPropertyNonEmpty(String propertyName) {
	ConfigValue configValue = ConfigProvider.getConfig().getConfigValue(propertyName);
	return configValue.getValue() != null && !configValue.getValue().isEmpty();
	}

For ConfigValue it is expected that getValue is null if the expression cannot be expanded.

[radcortez]

If I remember correctly, when we added isPropertyPresent we ignored the expansion part, because we are evaluating runtime properties during build, and of course, the expansion wouldn't be valid at that point, but it may be valid at runtime.

For Dev Services, we used that check, so we assumed that if a property was there (no matter what), someone would provide the expected value and the Dev Service will not start.

[gsmet]

But shouldn't the behavior be consistent with what will be done at runtime?

TBH, I'm not exactly happy with the fact that we had to special case the dev services case. I don't see a way out but still it's weird to have to put a default value there.

[gsmet]

Basically, wondering it we could find a way to start the test resources first and then make sure the config is out there when we are testing if the property is present. But in some cases, you might want the test resource to be started after the dev services.
Maybe we should have two stages, I don't know.

[radcortez]

But shouldn't the behavior be consistent with what will be done at runtime?

How so? The problem is that we are looking for runtime configurations that may or may not be available at build time.

Basically, wondering it we could find a way to start the test resources first and then make sure the config is out there when we are testing if the property is present. But in some cases, you might want the test resource to be started after the dev services.

While #41326 fixed #41128, it made us less consistent, and I would probably argue that it was not the proper fix.

The way we implemented Dev Services is to check if a connection or URL property is set to either start or skip the Dev Service. These properties are usually runtime, and Dev Services are set up during build time. The problem is that there is no reliable way to check if a runtime property is available at build time (obviously).

The old implementation checked only if the property existed and then started the Dev Service. Even if the property was not available at build time, the runtime property could be available via a runtime-only source (for instance, a Vault or Database source). This could cause a Dev Service to be started without being required. Ultimately, this is not a big issue because when you run in Dev or Test, it is unlikely to provide such properties in this way.

The new implementation not only checks for empty but also requires expressions to be expanded, effectively making the Dev Services check properties mandatory if they exist during build time. While it worked for the original user request, it uncovered that issue with Keycloak, and it is likely to cause breaks to existent user configuration. In the Keycloak test, the test resource supplies the configuration for runtime, and of course, it is only available at runtime. Because we are now checking the value, the expansion needs to be valid, so we need to supply a default value for ${keycloak.url}. I could make that piece work, but we never know where the expansion originated. If the expansion comes from a runtime-only source, then there is nothing we can do, and we require a default value for the expansion at build time.

So what can we do?

My recommendation for #41128 would be to keep everything as is and have the user create specific profiles.

Be less smart, and add properties to force the start of the Dev Services, complementing the old behavior.