-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix secret-config-kv-prefix-path property #13300
Conversation
integration-tests/vault/src/test/resources/application-vault-multi-path.properties
Outdated
Show resolved
Hide resolved
...time/src/main/java/io/quarkus/vault/runtime/config/VaultSecretConfigKvPrefixPathsConfig.java
Outdated
Show resolved
Hide resolved
hello all, I have squashed commits and rebased. I think the result is looking good thanks for @machi1990 's help. |
Hi @vsevel, @machi1990 Thanks, it looks good; @machi1990 thanks for your help :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vsevel, Gradle test is failing, most likely it is unrelated, but please rebase one more time
You are welcome :-) |
@sberyozkin build failed again, this time on the Microprofile TCK |
@sberyozkin failed build again because of the global timeout set on the maven build (3h). not too sure what I should do. |
@vsevel IMHO it is safe to merge this PR as all the Vault tests pass |
thanks to all of you for your assistance in the review and figuring out how to implement the best solution |
This changes property
quarkus.vault.secret-config-kv-path.singer=multi/singer1,multi/singer2
intoquarkus.vault.secret-config-kv-prefix.singer.paths=multi/singer1,multi/singer2
(configuration breaking change)The original approach (reusing the same name for the prefixed properties and non prefixed properties) seemed to work, but only because the re-parsing of configuration done in the
VaultConfigSource
was doing the right thing.Fixes #13276