Add missing parameters to vault_mount
#2235
Conversation
There was a problem hiding this comment.
I had one question about this change, other than that it looks pretty straightforward.
It does look like the tests are not passing e.g: https://github.com/hashicorp/terraform-provider-vault/actions/runs/8994620751/job/24708374987?pr=2235#step:9:1766
Will you also be doing the docs update under this PR?
| @@ -236,8 +305,32 @@ func updateMount(d *schema.ResourceData, meta interface{}, excludeType bool) err | |||
| path = newPath | |||
| } | |||
|
|
|||
| if d.HasChange("allowed_managed_keys") { | |||
| config.AllowedManagedKeys = expandStringSlice(d.Get("allowed_managed_keys").(*schema.Set).List()) | |||
| if d.HasChange(consts.FieldAllowedManagedKeys) { | |||
There was a problem hiding this comment.
Out of curiosity, is there a reason we are conditionally setting these new parameters, rather than always setting them?
There was a problem hiding this comment.
It's mostly because they all have varied minimum version requirements. We can't always set them since some fields are only enabled on certain Vault versions.
The idea is that all these params will only be added in if the user provides them / makes a change to them. The expectation is that if a user is providing us a field to use, then they are aware of their Vault Server and its API features/limitations. We can also individually add version checks for all the older versions to the parameters, I chose this route for simplicity and to avoid annoying conditionals like "If this greater than this version but less than that version do this" etc.
Good catch! Docs slipped my mind and I will add them in. And yes! The tests failures are expected, looks like the TFVP helped uncover a bug in a newly added Vault 1.16.x field. There is a fix PR up on the Vault repo. For now, I'll add a |
Description
Adds parameters to the resource that were missing, including newly 1.16 added fields.
Checklist
Output from acceptance testing: