provider/vault: vault_mount resource #14456
Conversation
There was a problem hiding this comment.
Looks good to me, @wjam. Thanks!
I left a comment inline for your information; I'm going to hold off on merging this for the moment in case you'd like to do another pass to support remounts, but if you'd rather leave this here and have that added in a separate PR that's fine by me.
| path := d.Id() | ||
|
|
||
| log.Printf("[DEBUG] Updating mount %s in Vault", path) | ||
|
|
There was a problem hiding this comment.
It is actually valid to call d.SetId from inside the update function, so in principle it should be fine to support remount here and just be careful do call d.SetId if and only if the remount succeeds.
This seems desirable to allow moving a mount without losing its internal state, though if you'd rather save that for a followup PR I think that's fine.
|
Okay, thanks for that @apparentlymart. I've just pushed the changes to add support for remounting mounts. The extra attributes are still missing due to the Vault API issue. |
|
Hi @apparentlymart. Is this okay to be merged? I'm keen to get this into the next release... |
|
Just realised that mounts can be created through the |
|
Just realised that using |
|
ping! Can this merge before the next release? |
| @@ -90,6 +90,7 @@ func Provider() terraform.ResourceProvider { | |||
| "vault_auth_backend": authBackendResource(), | |||
| "vault_generic_secret": genericSecretResource(), | |||
| "vault_policy": policyResource(), | |||
| "vault_mount": mountResource(), | |||
There was a problem hiding this comment.
There is an existing vault_auth_backend; maybe vault_secret_backend is more consistent with the lingo. A vault_audit_backend would be nice too.
There was a problem hiding this comment.
This is a good thought, @bmoylan! I think though that since the term "mount" is what appears in Vault's UI it makes sense to follow that here so that it's easier to understand how the Terraform resources map onto the Vault CLI operations.
|
Thanks for this, @wjam, and sorry for the delay in getting it merged. |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Part of #12167
Note that remounting hasn't been implemented because the
pathis being used as the ID. The attributesforce_no_cache&localare not able to be set as the Vault API doesn't expose these values.