Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_automation_account: encryption.key_vault_key_id should be optional #20433

Closed
wants to merge 4 commits into from

Conversation

wuxu92
Copy link
Contributor

@wuxu92 wuxu92 commented Feb 13, 2023

It's no need to provide key_vault_key_id where key_source is Microsoft.Keyvault.

Fixes: #20421

--- PASS: TestAccAutomationAccount_encryption (395.20s)
PASS

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a failing test:

------- Stdout: -------
=== RUN   TestAccAutomationAccount_encryption
=== PAUSE TestAccAutomationAccount_encryption
=== CONT  TestAccAutomationAccount_encryption
    testcase.go:110: Step 1/4 error: Error running apply: exit status 1
        
        Error: current client lacks permissions to read Key Rotation Policy for Key "acckvkey-230307040523881231" ("Vault: (Name \"vault230307040523881231\" / Resource Group \"acctestRG-auto-230307040523881231\")", Vault url: "https://vault230307040523881231.vault.azure.net/"), please update this as described here: https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/key_vault_key#example-usage : 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 'vault230307040523881231;location=eastus2'. For help resolving this issue, please see https://go.microsoft.com/fwlink/?linkid=2125287" InnerError={"code":"ForbiddenByPolicy"}
        
          with azurerm_key_vault_key.test,
          on terraform_plugin_test.tf line 88, in resource "azurerm_key_vault_key" "test":
          88: resource "azurerm_key_vault_key" "test" {

@wuxu92
Copy link
Contributor Author

wuxu92 commented Mar 8, 2023

they are passed now @katbyte
image

@tombuildsstuff
Copy link
Contributor

hey @wuxu92

Thanks for this PR - taking a look through here unfortunately we're going to need to take a different approach here, and so I've opened #21041 which makes these changes.

In version 3.0 of the Provider we've consolidated resources to using the presence/omission of a block to signify that these should be configured - for identity and encryption this means that when a Managed Identity or Customer Managed Key is configured for the resource, the identity or encryption block must be specified in the configuration - and when it's not it should be omitted. For the encryption block (which in some resources is called customer_managed_key), this includes inferring the value for the key_source field, meaning that we can deprecate and remove this field, since it can be inferred from the presence of the encryption block.

As such I hope you don't mind but I'm going to close this PR in favour of #21041 which fixes this issue, by bringing this behaviourally into line with other resources in the Provider.

Thanks!

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
@wuxu92 wuxu92 deleted the auto/fixkvidoptional branch August 21, 2023 05:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azurerm_automation_account - encryption - key_vault_key_id optional when key_source is Microsoft.Automation
3 participants