Quarkus: CredentialsProvider should be part of the datasource extension

[overthetop]

Quarkus: 1.3.2.Final

I'm trying to implement a custom credentials provider and as @gsmet has suggested on stackoverflow (https://stackoverflow.com/questions/61407783/quarkus-data-source-custom-credentials-provider) the CredentialsProvider interface is part of the Vault extension and that should be moved to the datasource extension.

I still haven't managed to implement a working custom credentials provider. Please help a little in the stack overflow thread. Thx

[machi1990]

/cc @vsevel

[sberyozkin]

@overthetop This interface is part of the Vault SPI, it does not need to be moved. Please see https://github.com/quarkusio/quarkus/issues/6896#issuecomment-581014674

[gsmet]

It was me asking @overthetop to create this issue.

If this SPI has nothing to do with Vault and can be more generally used, it shouldn't be in a Vault SPI module. It should be somewhere in the datasource implementation.

[sberyozkin]

@gsmet IMHO it would not be a good idea. Even though that interface is used by Agroal right now it does not belong to Agroal alone. Some other module may need it.
I haven't said it has nothing to do with Vault - its goal is to let the modules which may need a Vault functionality integrate with any Vault, not only HashiCorp.
There is absolutely no need IMHO to move that interface out of the vault-spi - the fact this interface is there does not prevent anyone from plugging in something else.

[sberyozkin]

Perhaps Agroal needs a dedicated interface if the credentials have to be pulled in from something other than (HashiCorp) vault

[sberyozkin]

Sorry, pressed closed by accident

[gsmet]

Ah, yeah, I could see how it could be useful for others too.

But typically, it's very useful even without Vault so maybe we just need to rename the module to make it totally agnostic of any technology.

Not high priority though.

[sberyozkin]

But typically, it's very useful even without Vault so maybe we just need to rename the module to make it totally agnostic of any technology.

Interesting idea. And then we can have several credential providers loaded at the same time, etc... Thanks

[vsevel]

I agree with the last 2 comments. It should not be in either agroal or vault.
there should be a notion of credentials consumer (agroal being one of those) and credentials provider (vault being one of those).
anybody should be able to consume credentials.
and anybody should be able to produce credentials.

[vsevel]

@overthetop check https://github.com/quarkusio/quarkus/issues/6896#issuecomment-581014674. tell us if it works for you.

@gsmet @sberyozkin
if those needs start to pop up (and that is the second one with https://github.com/quarkusio/quarkus/issues/6896), we should rename vault-spi to credentials-spi and make it a public documented module on its own.

[machi1990]

@gsmet @sberyozkin
if those needs start to pop up (and that is the second one with #6896), we should rename vault-spi to credentials-spi and make it a public documented module on its own.

+1 for this renaming. If we could also take care of #8828 in one go will be awesome.

[gsmet]

I provided a working example to @overthetop on SO.

[vsevel]

@gsmet this looks nice and concise.
one alternative is to keep the username in the ds config, and only return the password from the provider:

quarkus.datasource.db-kind=postgresql
quarkus.datasource.credentials-provider=custom
quarkus.datasource.jdbc.url=jdbc:postgresql://localhost:5431/hibernate_orm_test
quarkus.datasource.username=hibernate_orm_test

Properties properties = new Properties();
properties.put(PASSWORD_PROPERTY_NAME, "hibernate_orm_test");
return properties;

choice depends on personal preference, and whether or not you store username beside passwords in your custom vault.

the 2 other things that are worth considering:

• provider configuration using the credentialsProviderName, such as in here
• distinguishing between providers when several are available in the project using property quarkus.datasource.credentials-provider-type

once we rename the module, we need to document properly all of this.

[sberyozkin]

@vsevel FYI, there was a query about getting the credentials from Azure KeyVault: https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/datasource.20credentials.20from.20Azure.20KeyVault

[overthetop]

@gsmet Thank you for the working example. Now I see why this is not working in my case. I'm using the reactive postgres driver and the custom credentials provider is never called. I've created a PR for the example repo so that you can see what I got. The whole project is reactive and I can't switch to the jdbc driver :(
https://github.com/gsmet/quarkus-credentials-provider/pull/1
Is there a way to make this working?

[vsevel]

the logic in the reactive pg client is not shared with agroal.

it might be just a matter of adding this block of code from agroal with minor adjustments.

[vsevel]

But typically, it's very useful even without Vault so maybe we just need to rename the module to make it totally agnostic of any technology.

where should it go?
vault-spi is not an extension in itself.
all spi modules seem to be associated with an extension today.
should we move th sole interface CredentialsProvider to core/runtime in package io.quarkus.runtime.security.credentials for instance?
somewhere else?

[vsevel]

closed by #9032