Add AWS secret data source. #20
Conversation
This is a fully-functioning data source for retrieving AWS access tokens and secret tokens from Vault. While the generic vault data source _could_ be used, this adds more affordances for users (named attributes, required attributes, marking things as sensitive, generated paths instead of forcing users to deal with API details, etc.) and allows us to add some behaviour that helps mitigate AWS' eventual consistency, which has caused problems for users in the past (credentials are returned, but when users attempt to use them, they're "not valid"). The approach to this is fairly naive at the moment (wait for the credentials to make three successful API calls before returning them), but should reduce the number of problems users see. Also add the aws_secret_backend resource, which sets up an AWS secret backend in Vault. We can't use vault_mount and vault_generic_secret to set it up, because it needs special configuration (access key and secret key) written that can't be deleted once written--the way to remove it is to unmount. So the new resource mounts and writes to the location, and when deleting, unmounts. Also, add the aws_secret_role resource, which sets up an AWS secret backend role. While vault_generic_secret *could* be used to manage roles, we get a lot of affordances (named attributes instead of a lump of JSON, required attributes, hiding API paths from users) by turning them into a resource. Finally, add some helpers for dealing with JSON. Adding the resources all at once isn't ideal, obviously, and I wish I had a smaller PR here. Unfortunately, I approached the problem from the data source end, and had to build the stuff I needed to write the tests and make them pass, and I underestimated how much stuff that would be. :(
Plans aren't perpetually non-empty anymore on data source changes (or at least are reporting they're not). Also, ensure network requests get surrounded by log statements.
There was a problem hiding this comment.
Implementation-wise this looks fine. Had a minor quibble with naming inline, but I'm happy to be overridden on that if you chose this name for some consistency with something in Vault I don't know about.
Whether you decide to change the name or not, should be fine to go ahead and merge this without another round of review from me. 👍
vault/provider.go
Outdated
| "vault_policy": policyResource(), | ||
| "vault_mount": mountResource(), | ||
| "vault_aws_secret_backend": awsSecretBackendResource(), | ||
| "vault_aws_secret_role": awsSecretRoleResource(), |
There was a problem hiding this comment.
If this is aligning with some terminology in Vault land then all of this comment is moot and ignorable, but...
"Secret Role" doesn't really speak to me as a what it seems to be here. I think this name would ideally have "credentials" in it to make it clear that what we're issuing here is credentials, rather than just generically a "secret" (which could, for example, be an AWS KMS thing, or an IAM server certificate key, etc).
I took a quick skim of the Vault docs related to this backend, and based on that (it looks like they use the terminology "access credentials" there) my suggestion would be vault_aws_access_credentials, to make it very explicit that it's access credentials that we're issuing.
There was a problem hiding this comment.
So for the line you specifically commented on, the relevant Vault docs say:
The next step is to configure a role. A role is a logical name that maps to a policy used to generated those credentials. You can either supply a user inline policy (via the policy argument), or provide a reference to an existing AWS policy by supplying the full ARN reference (via the arn argument).
For example, lets first create a "deploy" role using an user inline policy as an example:
So I think vault_aws_access_credentials doesn't quite capture the idea of it, if that's what you were suggesting get renamed. Though, looking at the pattern established in later resources, I renamed it vault_aws_secret_backend_role instead of vault_aws_secret_role, which feels a bit clearer.
Similarly, for vault_aws_secret_backend, the docs say:
The first step to using the aws backend is to mount it. Unlike the kv backend, the aws backend is not mounted by default.
{code_snippet}
Next, we must configure the credentials that Vault uses to manage the IAM credentials generated by this secret backend:
This is encapsulated in the vault_aws_secret_backend resource. I didn't call it vault_aws_secret_backend_credentials because it takes care of both the mounting and the credentials configuration, so it seemed clearer to have the name imply we were configuring the entire backend, not just the credentials. The reason the two API calls are combined like that is because removing the credentials once they're configured is impossible, and setting them to an invalid value breaks things, so I was struggling with the Delete function. The canonical way to remove configured credentials, as far as I can tell, is to unmount the backend. So I combined mounting/unmounting the backend and configuring the credentials so it all maps nicely to the CRUD format Terraform expects.
All that said, I can see an argument for renaming the vault_aws_secret data source to vault_aws_access_credentials, if that's what you're referencing. The _secret part largely just came from trying to match the pre-existing generic_secret, to make it obvious that that's generic and this is the aws-specific flavor. But on further consideration, I do like vault_aws_access_credentials better, so I've updated the name to reflect that. If any of this is unclear or does not make sense, or you disagree with it, do let me know.
website/docs/d/aws_secret.html.md
Outdated
| EOT | ||
| } | ||
|
|
||
| data "vault_aws_secret" "creds" { |
There was a problem hiding this comment.
Maybe worth mentioning in a comment here that in practice the remainder of this would probably be in a separate config altogether, since it'd be weird to configure vault with credentials and then immediately read back again in the same config. 😀
There was a problem hiding this comment.
I can see use cases (e.g., write one set of creds to vault, have it issue a more restricted set of creds, have Terraform use those to set up your provider) but I can also see it causing confusion, so I added a comment. :)
Also, update names on resources.
|
@paddycarver looks like this isn't solving that issue in #9 |
|
I still have to run
before plan can run |
|
Let's re-open #9 |
|
Hmm, looking like this may be related to stored state |
…ta_source Add AWS secret data source.
This is a fully-functioning data source for retrieving AWS access tokens
and secret tokens from Vault. While the generic vault data source
could be used, this adds more affordances for users (named attributes,
required attributes, marking things as sensitive, generated paths
instead of forcing users to deal with API details, etc.) and allows us
to add some behaviour that helps mitigate AWS' eventual consistency,
which has caused problems for users in the past (credentials are
returned, but when users attempt to use them, they're "not valid"). The
approach to this is fairly naive at the moment (wait for the credentials
to make three successful API calls before returning them), but should
reduce the number of problems users see.
Also add the aws_secret_backend resource, which sets up an AWS secret
backend in Vault. We can't use vault_mount and vault_generic_secret to
set it up, because it needs special configuration (access key and secret
key) written that can't be deleted once written--the way to remove it is
to unmount. So the new resource mounts and writes to the location, and
when deleting, unmounts.
Also, add the aws_secret_role resource, which sets up an AWS secret
backend role. While vault_generic_secret could be used to manage
roles, we get a lot of affordances (named attributes instead of a lump
of JSON, required attributes, hiding API paths from users) by turning
them into a resource.
Finally, add some helpers for dealing with JSON.
Adding the resources all at once isn't ideal, obviously, and I wish I
had a smaller PR here. Unfortunately, I approached the problem from the
data source end, and had to build the stuff I needed to write the tests
and make them pass, and I underestimated how much stuff that would be.
:(