HashiCorp key vault does not override application.properties

[bardsoleim]

Describe the bug
HashiCorp vault does not override properties in application.properties

Expected behavior
Values in application.properties that share the same name described in vault should be overridden.

Actual behavior
Only the values in application.properties gets assigned.

To Reproduce

https://quarkus.io/guides/vault
follow this guide while having a value for "a-private-key" in application.properties

Steps to reproduce the behavior:

1. Create a secret in vault
2. Use the same secret with diffrent value in appliction.properties
   icable, add screenshots to help explain your problem.)

**Environment **
Quarkus 1.11.1.Final

[ghost]

/cc @vsevel

[ebullient]

related to #14707 ?

[radcortez]

related to #14707 ?

No. The VaultConfigSource is set with a lower ordinal (150) vs the application.properties file (250).

@vsevel was that on purpose?

[vsevel]

what that on purpose?

No. I don't remember where I got this value. Some quarkus code or an example somewhere. What would an appropriate value?

[vsevel]

I think I got it from the zookeeper config source
I see that the consul config source has ordinal 270 with comment: this is higher than the file system or jar ordinals, but lower than env vars

for the kubernetes config we have:

    CONFIG_MAP(
            270, // this is higher than the file system or jar ordinals, but lower than env vars
            284 // this is one less than the ordinal of Secret
    ),

    SECRET(
            285, // this is one less than the ordinal of ConfigMap
            299 // this is one less than env vars
    );

@geoand what do you think? should we go with 270 for vault? or 285?

[geoand]

Both are just fine.

The Kubernetes uses both of them in order to ensure that secrets override properties, but for vault I assume you don't have 2 different kinds of sources

[radcortez]

By the way, per the spec, a ConfigSource may set the property config_ordinal to override its own ordinal, but I believe we don't have that implemented.

We may want to add that capability to the VaultConfigSource and also review all the other sources.

[bardsoleim]

After changing the value from 150 to 285 it works as expected.

[kny78]

When @bardsoleim tested with ordinal=285, he modified the Quarkus source.

Is it possible to override this runtime or with @QuarkusMain ?

[radcortez]

When @bardsoleim tested with ordinal=285, he modified the Quarkus source.

Is it possible to override this runtime or with @QuarkusMain ?

Not with a simple configuration as far as I'm aware.

Right now, the possible options I see is to create a new provider / instance that is able to override the ordinal, but since it requires some internal Quarkus config it may not be so easy. Another option may be to cast the Config object to SmallRyeConfig and call io.smallrye.config.SmallRyeConfig#addConfigSourcewith a new source built from the current one and with the right ordinal. Be careful, because this is deprecated API and will be removed Config 2.0. Hopefully we will have this sorted out before :)

[vsevel]

By the way, per the spec, a ConfigSource may set the property config_ordinal to override its own ordinal, but I believe we don't have that implemented.

@radcortez you mean we should honor a property quarkus.vault.config_ordinal?

[radcortez]

By the way, per the spec, a ConfigSource may set the property config_ordinal to override its own ordinal, but I believe we don't have that implemented.

@radcortez you mean we should honor a property quarkus.vault.config_ordinal?

No, just config_ordinal. This property should not be exposed, but something internal to the ConfigSource itself. In the case of VaultConfigSource, this property must be retrieved from Vault and used to configure the ordinal.

Please check:

• https://github.com/eclipse/microprofile-config/blob/562ece3bed9396513351599c943b8ab90075fb00/api/src/main/java/org/eclipse/microprofile/config/spi/ConfigSource.java#L163-L166
• https://github.com/smallrye/smallrye-config/blob/master/common/src/main/java/io/smallrye/config/common/MapBackedConfigSource.java#L49
• https://github.com/smallrye/smallrye-config/blob/master/common/src/main/java/io/smallrye/config/common/utils/ConfigSourceUtil.java#L74-L84

[vsevel]

@radcortez it looks a bit strange to put this information into the vault, because that is something that's going to be quarkus specific. whereas vault is a central point for many workloads and tools. we would have to figure out a path in the kv secret engine such as secret/quarkus, but then this would apply to all quarkus apps in the enterprise, which is not going to work out in most organizations. so we would need to take into account the name of the app, or better provide a quarkus.vault.path_to_override_config_ordinal=secret/myorg/mydivision/mytechnologies/quarkus/mpconfig/defaults. somebody should not forget also about providing read access to this path in the app's policy.
it sounds a lot of work, and actually not very practical. my gut feeling would be that 270 (and it would be nice to have a constant in quarkus for that value DEFAULT_CUSTOM_CONFIG_SOURCE_ORDINAL) would work for many if not all apps, and we may want to provide an override for a particular app, but I would put this in the app config, not in vault.

[radcortez]

Don't you already take into account the app config in the quarkus.vault.secret-config-kv-path config? So, couldn't the user add a config_ordinal in the path and then use it to set the ordinal?

[vsevel]

yes we could. note that quarkus.vault.secret-config-kv-path is actually multiple paths, such as:

quarkus.vault.secret-config-kv-path=myapp/path1,myapp/path2

somebody would have to decide what to do if config_ordinal was found if both of these paths. we could choose the first one. but then the behavior of the app would change if we were to reorder those paths, which could be unexpected.
since there is only one vault config source, it would seem more natural to me if that parameter was with the others, mainly secret-config-kv-path.
it is going to be the same question for the kubernetes config because I assume it can fetch properties from multiple secrets and config maps.
now if we want to adhere to the spec, we can certainly do this.

[radcortez]

We can opt out of it if we don't find a suitable way to do it. I guess the spec did not foreseen these cases, so this is somehow uncharted territory. We can certainly bend the rules a little bit and point to a different property, other than config_ordinal due to the limitations of the implemented source.

[vsevel]

so would you be fine with a property such as quarkus.vault.config_ordinal=300?

[radcortez]

Yes. We can even add some of the reasoning why we picked that approach. Thanks!