Support Vault agent auth config for AWS/GCP CA provider auth #15970
Conversation
github-actions
bot
added
pr/dependencies
| mount := authMethod.Type | ||
| if authMethod.MountPath != "" { | ||
| mount = authMethod.MountPath | ||
| } |
There was a problem hiding this comment.
nit/minor: configureVaultAuthMethod also has the "default mount path to type" logic. Is it easy to have this defaulting logic in just one place?
There was a problem hiding this comment.
Yes, good call. I think it is fair to assume that the caller sets the MountPath correctly.
| var _ VaultAuthenticator = (*gcp.GCPAuth)(nil) | ||
|
|
||
| // NewGCPAuthClient returns a VaultAuthenticator that can log into Vault using the GCP auth method. | ||
| func NewGCPAuthClient(authMethod *structs.VaultAuthMethod) (VaultAuthenticator, error) { |
There was a problem hiding this comment.
nit: It would be nice to put the GCP stuff here in its own file, and maybe the AWS stuff in its own file. I guess, provider_vault_auth_gcp.go, provider_vault_auth_aws.go? What do you think?
There was a problem hiding this comment.
Agreed. I think this makes sense.
| } | ||
|
|
||
| authMethod.Params["jwt"] = string(serviceAccountToken) | ||
| } | ||
| loginPath = fmt.Sprintf("auth/%s/login", authMethod.MountPath) | ||
| return NewVaultAPIAuthClient(authMethod, loginPath), nil |
There was a problem hiding this comment.
non-blocking: You don't have to do this here, but for organizational purposes, I wouldn't mind if we added functions for the other auth-method types that have non-default config: NewK8sAuthClient, NewOCIAuthClient, NewAuthClientWithUsername(?) - where those could check/set params and call NewVaultAPIAuthClient.
There was a problem hiding this comment.
Yes I fully agree. I've added a new issue to our backlog for this.
|
|
||
| if c.isExplicit { | ||
| // in this case a JWT is provided so we'll call the login API directly using a VaultAuthClient. | ||
| _ = auth.(*VaultAuthClient) |
There was a problem hiding this comment.
nit: We could use require.IsType for these type assertions
| // NewCredentialsConfig creates an awsutil.CredentialsConfig from the given set of parameters. | ||
| // It is mostly copied from the awsutil package because that package does not currently provide | ||
| // an easy way to configure all the available options. | ||
| func NewCredentialsConfig(config map[string]interface{}) (*awsutil.CredentialsConfig, string, error) { |
There was a problem hiding this comment.
If we're not namespacing each auth provider with its own package, prefixing this with AWS might help discover related functions together
| func NewCredentialsConfig(config map[string]interface{}) (*awsutil.CredentialsConfig, string, error) { | |
| func NewAWSCredentialsConfig(config map[string]interface{}) (*awsutil.CredentialsConfig, string, error) { |
There was a problem hiding this comment.
Or if this is an implementation detail, we could simply unexport it?
There was a problem hiding this comment.
Good call. I've added AWS and unexported it.
| // LoginDataGen derives the parameter map containing the data for the login API request. | ||
| // For some auth methods this is needed to transform the AuthMethod.Params into the login data | ||
| // needed for the API request. | ||
| LoginDataGen LoginDataGenerator |
There was a problem hiding this comment.
My understanding is that we either use AuthMethod.Params directly or transform it with LoginDataGen to ultimately produce a map[string]interface{}.
Could this simply be func(*structs.VaultAuthMethod) map[string]any with a default that returns AuthMethod.Params? The LoginDataGenerator feels like a level of indirection which isn't entirely necessary.
There was a problem hiding this comment.
Yes, I like this suggestion. We don't really need an interface for this. I've updated the type to a func definition as suggested.
Description
This PR addresses #11921 and adds support for supplying an existing Vault agent auto-auth configuration as the provider configuration when using the Vault CA provider. Support has been added for the AWS and GCP auth methods.
The changes have been made such that backwards compatibility is maintained for existing AWS/GCP CA auth configurations.
Testing & Reproduction steps
PR Checklist