Add stand-alone resource to edit Key Vault access policies #1052
Conversation
… creating/destroying the underlying KV instance.
|
Ping... Any way to get a review going on this PR? |
|
I didn't notice this PR when I opened #1149 this weekend. I didn't see any PRs associated with #754 which is the issue i was looking at. I just want this feature added, so lets pool resources. I see several issues with this PR and will comment inline pointing them out. To avoid duplicate work lets collaborate here. |
| // schema with some tweaks so that unneed properties need not be | ||
| // specified in the template. | ||
|
|
||
| kvapSchema := resourceArmKeyVault() |
There was a problem hiding this comment.
Doing this will make it look just like a Key Vault resource so the examples you have wouldn't follow this pater. This resource would have to look like
resource "azurerm_key_vault" "test" {
name = "testvault"
resource_group_name = "${azurerm_resource_group.test.name}"
tenant_id = "d6e396d0-5584-41dc-9fc0-268df99bc610"
access_policy {
tenant_id = "d6e396d0-5584-41dc-9fc0-268df99bc610"
object_id = "d746815a-0433-4a21-b95d-fc437d2d475b"
key_permissions = [
"get",
]
secret_permissions = [
"get",
]
}
Which wouldn't really be allowing you to attach it to an existing keyvault.
There was a problem hiding this comment.
I missed that the SDK has the UpdateAccessPolicy function. I therefore intentionally reused the KV resource with the idea of reading all its existing properties and editing only the access policies. Given that UpdateAccessPolicy is there, I will quickly reassess approaches and write back.
| Tags: expandTags(tags), | ||
| } | ||
|
|
||
| _, err = client.CreateOrUpdate(ctx, resGroup, name, parameters) |
There was a problem hiding this comment.
There is a call in the sdk to only update the policy
client.UpdateAccessPolicy(ctx, resGroup, name, action, parameters)
| *# Remaining scaleset configuration omitted for brevity...* | ||
| } | ||
|
|
||
| resource "azurerm_key_vault_access_policy" "test" { |
There was a problem hiding this comment.
As noted above, the written code would allow for resources that look like this.
| Access policies can be imported from any Key Vault using the `resource id`, e.g. | ||
|
|
||
| ```shell | ||
| terraform import azurerm_key_vault.test /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/mygroup1/providers/Microsoft.KeyVault/vaults/vault1 |
There was a problem hiding this comment.
Probably shouldn't import the entire vault for just the access policies.
| } | ||
| } | ||
|
|
||
| resource access_policy { |
There was a problem hiding this comment.
This terraform is malformed and the tests fail due to this, should be
resource "azurerm_key_vault_access_policy" "test"
Or something like that
| }) | ||
| } | ||
|
|
||
| func testCheckAzureRMKeyVaultAccessPolicyDestroy(s *terraform.State) error { |
There was a problem hiding this comment.
This should be used to ensure that resources setup by your acceptance tests are detroyed.
|
|
||
| func TestAccAzureRMKeyVaultAccessPolicy_update(t *testing.T) { | ||
| ri := acctest.RandInt() | ||
| resourceName := "azurerm_key_vault.test" |
There was a problem hiding this comment.
Should reference your tests policy resource
| ) | ||
|
|
||
| func TestAccAzureRMKeyVaultAccessPolicy(t *testing.T) { | ||
| resourceName := "azurerm_key_vault.test" |
There was a problem hiding this comment.
Should reference your test policy resource
|
Jeff, this is simple--your implementation is correct, this one is not. I will close this PR in favor of yours. I missed that the Go SDK has a function to update access policies, so I implemented a workaround. I reviewed and tested your code, and it looks great. |
|
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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
This new resource allows access policies to be set on existing Key Vaults without creating/destroying the KV instance. This allows new resources (e.g. VMs) to be given access to keys, secrets and certificates stored in KV.
This PR was created in order to allow new VMs (stand-alone or in a scale set) to read/write secrets and certificates from/to an existing Key Vault.