azurerm_key_vault_key - rotation_policy block added

[aristosvo]

Depends on #18576

Fixes #14471

Issues:

• Contains a workaround for now, ignoring the policy read unauthorized error if unauthorised. Maybe something like a features flag would be the best solution?
• Made the policy read/create/update authorization error explicit and added an Acceptance Test to confirm this behaviour.

[katbyte]

THanks @aristosvo - couple comments on property names, mainly expire_time seems less then idea for clarity on what it means. WDYT?

[aristosvo]

@katbyte It seems the troubles of implementing this as an inline resource are bubbling up:

testcase.go:110: Step 1/1 error: Error running apply: exit status 1
Error: keyvault.BaseClient#GetKeyRotationPolicy: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="Forbidden" Message="The user, group or application 'appid=*******;oid=3aa04c8c-5a75-4e5e-9117-1b7cf6f33e21;numgroups=9;iss=https://sts.windows.net/*******/' does not have keys getrotationpolicy permission on key vault 'acctestkv-vecj9;location=westeurope'. For help resolving this issue, please see https://go.microsoft.com/fwlink/?linkid=2125287" InnerError={"code":"ForbiddenByPolicy"}
with azurerm_key_vault_key.test,

This means we would not have backwards compatibility and we'd need more permissions on key_vault than strictly necessary for a non-keyvaultkeypolicy user.. 😢 Especially in a resource like KV, not something we want.., WDYT?

Edit: Added a workaround for now, ignoring the policy if unauthorised. Maybe something like a features flag would be the best solution:

provider "azurerm" {
  features {
    key_vault {
      ignore_missing_key_vault_key_policy_permissions = true
    }
  }
}

[katbyte]

@aristosvo - would those permissions not still be needed by the separate resources?

i imagine you could infer if the user is trying to set them from the existence of the block or not, if not set + read fails ignore, if set + read fails error out, but that does seem less then ideal 😅

[aristosvo]

@aristosvo - would those permissions not still be needed by the separate resources?

Yes it would, but then it is not breaking azurerm_key_vault_key for existing user base with limited permissions

i imagine you could infer if the user is trying to set them from the existence of the block or not, if not set + read fails ignore, if set + read fails error out, but that does seem less then ideal 😅

Yeah, doing something like that now, I set it conditionally and ignore read fails. Could finetune that indeed a bit, but it still isn't ideal. Should've stopped thinking earlier and just implemented it separately 😅

Reopened and improved #18603 to have options. Both are working and tested, each with their own compromise I'd say.

[katbyte]

this is waiting on #18576 which is waiting on Azure/azure-rest-api-specs#21137

[aristosvo]

Added in SDK v7.4, but this one is not available yet, causing all tests to fail.

[tombuildsstuff]

@aristosvo SDK v7.4 is now available in tombuildsstuff/kermit, which is being updated in #20649 - so once that's in we should be able to rebase this FYI :)

[aristosvo]

Rebased and replaced with the kermit SDK!!

[katbyte]

Thanks @aristosvo ! LGTM 🌴

[MattoHopkins]

Out of curiosity, will v3.46.0 come out the 2nd @katbyte? Not normally a github follower, but I noticed the 'due date' was the 2nd and wasn't sure if those were hard or soft dates

Ty ty!

[ziyeqf]

Hey @aristosvo @katbyte , want to confirm the final decision on "ignore read fails", it seems it's always failing but with a more friendly error message when read fails without enough permission now?

https://github.com/hashicorp/terraform-provider-azurerm/blob/main/internal/services/keyvault/key_vault_key_resource.go#L590

[aristosvo]

@ziyeqf Although it has been a while since implementing it, I believe that is indeed the case, discussed that with @katbyte privately.

[github-actions]

This functionality has been released in v3.46.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[markor002002]

This functionality has been released but no documentation has bene provided.

[aristosvo]

This functionality has been released but no documentation has bene provided.

website/docs/r/key_vault_key.html.markdown contains updates in this PR on the rotation_policy block, can you explain what you are looking for?

[markor002002]

https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/key_vault_key my bad sorry all is ok,

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.