PostgreSQL backend passwordless authentication in cloud #30681
Conversation
There was a problem hiding this comment.
Hi @tauhid621
Thanks for this PR. Overall I like this functionality and you've done a great job of updating documentation etc.
Overall I think this looks good but a couple of questions:
- Initially I assumed
pgmultiauthlibrary was a third party library that presumably is already well used and well tested. It seems like it's a pretty recent library that we wrote ourselves though and I'm guessing hasn't had time in production yet? Could you talk through the testing process for that library? Does it have reasonable integration tests with each provider? - Are there a plans to continue adding new providers to it? One concern with previous libraries like this (see go-discover) is that they ended up adding 100s of MB of bloat to our binaries because they tend to pull in entire SDK for each cloud even though they only hit a single endpoint. I'd be curious what the plans are for broader support for other clouds and whether this increases Vault's binary size any. (It probably won't a lot as I think we probably already pull in these three SDKs).
Actually....
There is a bigger concern that could be a blocker. I don't think that library is in compliance with our internal controls and so can't be included in our releases yet. I'll work with you on this out of band.
We should not merge this until we confirm this.
| @@ -174,6 +176,47 @@ func TestConnectionURL(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| // TestGetAuthConfig - We only test the standard flow as we would get errors for cloud based auth while fetching token | |||
There was a problem hiding this comment.
Is there any way to stub out the actual endpoint pgmulti-auth uses for each cloud to test those? I'm not sure it's necessary if it's a huge lift, but it seems like it could be reasonable if they allow you to override the URL for testing.
There was a problem hiding this comment.
I did think of it initially and realised that it could be very tricky.
I will try to explore this a bit more.
Co-authored-by: Paul Banks <banks@banksco.de>
|
@banks . Answering your questions
I will work on the compliance part. We did had a security review and got the repo added to |
There was a problem hiding this comment.
Hi, we've confirmed necessary compliance controls are in place now.
I'm going to approve this on the basis of your answers above - thanks!
I do think additional testing especially since it's a new library and there are several different integrations should be prioritised, however for now I think we can accept and unblock you.
* PostgreSQL backend passwordless authentication in cloud * updated changelog file name * Update the changelog Co-authored-by: Paul Banks <banks@banksco.de> * fix image spec sha --------- Co-authored-by: Paul Banks <banks@banksco.de>
Description
What does this PR do?
This PR adds support to authenticate PostgreSQL Backend server with cloud based identities(AWS IAM, Azure MSI and GCP IAM). This will allow users to avoid password based auth in cloud environment.
Note: The changes were merged earlier and were reverted for another round of review.
Slack conversation: https://hashicorp.slack.com/archives/C0287E435NE/p1747085455400359
How
We have replaced the sql driver with
pgmultiauth's implementation. pgmultiauth handles cloud authentication and is a wrapper aroundpgxdriver.Note: The PR also updates some deprecated docker types used in tests.
Added configs
auth_mode- Defaults to standard auth. Can be set toaws_iam,azure_msi,gcp_iamorstandard.aws_db_region- Specifies the AWS region where DB is situated. Required whenauth_modeis set toaws_iamazure_client_id- Client ID of a user-assigned Managed Service Identity. System-assigned Managed Service Identity is used ifazure_client_idis not provided andauth_modeis set toazure_msi.References
Jira: https://hashicorp.atlassian.net/browse/IND-2963
RFC: https://go.hashi.co/rfc/tf-1050
Testing
TODO only if you're a HashiCorp employee
backport/label that matches the desired release branch. Note that in the CE repo, the latest release branch will look likebackport/x.x.x, but older release branches will bebackport/ent/x.x.x+ent.of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
in the PR description, commit message, or branch name.
description. Also, make sure the changelog is in this PR, not in your ENT PR.