Keyvault storage permissions

[Lucretius]

Resolves #2405

I've also updated the documentation to reflect that all permissions blocks are optional. They are marked so in the code but the documentation said some of them were required.

Tests have all been updated. Also created the resource using local binary and saw the expected storage permissions when reading them out with the data source.

[tombuildsstuff]

hey @Lucretius

Thanks for this PR :)

I've taken a look through and left a couple of comments inline but this is otherwise looking pretty good to me; if we can fix those up we should be able to get this merged 👍

Thanks!

[tombuildsstuff]

(as above) could we remove this?

[Lucretius]

Did you want to remove the v0_1_all test case? Or just the two storage ones that I had added? I will remove the two storage ones since its a new property and there will be no state to migrate. I can change the all test so that it doesnt try to handle the all property value for storage since it never existed.

[tombuildsstuff]

thinking about this some more, could we revert the changes in resource_arm_key_vault_migration_test.go? since there's no need to test for them in the state migration

[Lucretius]

I attempted to revert the test file but tests began failing. It appears that the migration tests, at minimum, all expect a terraform output of
access_policy.0.storage_permissions.#:0. Right now, the tests just include a single storage permission, and that storage permission does not change before or after the state migration (showing its not affected by it). But I think we need it there due to how the tests need to read the policies from the resource.

[Lucretius]

@tombuildsstuff I've gone and swapped the migration test to be the bare minimum of expecting 0 storage permissions for each migration test so that the terraform diff works. I don't think we can remove it entirely as per my comment above.

[katbyte]

Thanks for the changes @Lucretius, LGTM 👍

[ghost]

This has been released in version 1.24.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
	version = "~> 1.24.0"
}
# ... other configuration ...

[ghost]

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 👉 [email protected]. Thanks!