Skip to content

azurerm_cognitive_account : fix crash when key_vault_key_id is nil#28368

Merged
catriona-m merged 3 commits intohashicorp:mainfrom
sinbai:cognitive/fix_28357
Mar 19, 2025
Merged

azurerm_cognitive_account : fix crash when key_vault_key_id is nil#28368
catriona-m merged 3 commits intohashicorp:mainfrom
sinbai:cognitive/fix_28357

Conversation

@sinbai
Copy link
Contributor

@sinbai sinbai commented Dec 23, 2024

Fix plugin crash when key_vault_key_id is nil, fix #28357.

Copy link
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

Thanks @sinbai - could we add a test here which switches from using a customer managed key to microsoft managed to confirm that this will fix the linked issue?

@sinbai
Copy link
Contributor Author

sinbai commented Mar 14, 2025

which switches from using a customer managed key to microsoft managed

Hi @catriona-m thanks for your feedback. When I added a test to switch from using customer managed keys to Microsoft managed keys, I found that the current TF implementation cannot make the switch. Here's why:

When creating azurerm_cognitive_account, customer_managed_key was not specified, and the API did not return the default Encryption type(corresponding to the keySource in the API. According to the document "By default, Microsoft creates and manages your resources in a Microsoft-owned Azure subscription and uses a Microsoft-managed key to encrypt the data.", the default Encryption type should be Microsoft Managed Keys (Corresponding to Microsoft.CognitiveServices. And what is actually displayed on the portal is Microsoft Managed Keys).

However, when updating resource azurerm_cognitive_account (switching from using customer managed keys to Microsoft managed keys), deleting customer_managed_key (setting Encryption = nil), the API does not return an error, but the switch is actually unsuccessful. After verifying the API behavior, it was found that the API needs to explicitly specify the API property keySource as Microsoft.CognitiveServices in order to update successfully.

So while adding the test, I updated the code (when sending the API request without specifying customer_managed_key, specify the keysource value as Microsoft.CognitiveServices) so that the update can succeed. Could you please take another look?

Test result:
PASS: TestAccCognitiveAccount_customerManagedKey (440.27s)

Copy link
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

Thanks for updating this @sinbai. It looks like there are some test failures related to this, but once those are addressed I can take another look. Thanks!

------- Stdout: -------
=== RUN   TestAccCognitiveAccount_basic
=== PAUSE TestAccCognitiveAccount_basic
=== CONT  TestAccCognitiveAccount_basic
    testcase.go:173: Step 1/3 error: Error running apply: exit status 1
        Error: creating Account (Subscription: "*******"
        Resource Group Name: "acctestRG-cognitive-250314103054145747"
        Account Name: "acctestcogacc-250314103054145747"): unexpected status 400 (400 Bad Request) with error: BringOwnFeatureNotEnabled: Bring your own feature is not enabled for Subscription/SKU/Kind.
          with azurerm_cognitive_account.test,
          on terraform_plugin_test.tf line 40, in resource "azurerm_cognitive_account" "test":
          40: resource "azurerm_cognitive_account" "test" {
--- FAIL: TestAccCognitiveAccount_basic (109.26s)
FAIL

@sinbai
Copy link
Contributor Author

sinbai commented Mar 19, 2025

using a customer managed key to microsoft managed

Thanks for updating this @sinbai. It looks like there are some test failures related to this, but once those are addressed I can take another look. Thanks!

------- Stdout: -------
=== RUN   TestAccCognitiveAccount_basic
=== PAUSE TestAccCognitiveAccount_basic
=== CONT  TestAccCognitiveAccount_basic
    testcase.go:173: Step 1/3 error: Error running apply: exit status 1
        Error: creating Account (Subscription: "*******"
        Resource Group Name: "acctestRG-cognitive-250314103054145747"
        Account Name: "acctestcogacc-250314103054145747"): unexpected status 400 (400 Bad Request) with error: BringOwnFeatureNotEnabled: Bring your own feature is not enabled for Subscription/SKU/Kind.
          with azurerm_cognitive_account.test,
          on terraform_plugin_test.tf line 40, in resource "azurerm_cognitive_account" "test":
          40: resource "azurerm_cognitive_account" "test" {
--- FAIL: TestAccCognitiveAccount_basic (109.26s)
FAIL

Hi @catriona-m thanks for your feedback. I have updated the code, could you please take another look?

Copy link
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

Thanks @sinbai LGTM!

@catriona-m catriona-m merged commit de3c839 into hashicorp:main Mar 19, 2025
30 checks passed
@github-actions github-actions bot added this to the v4.24.0 milestone Mar 19, 2025
catriona-m added a commit that referenced this pull request Mar 19, 2025
jackofallops added a commit that referenced this pull request Mar 21, 2025
* CHANGELOG.md for v4.24.0

* Update CHANGELOG.md #28173

* Update CHANGELOG.md #28964

* Update CHANGELOG.md #28569

* Update CHANGELOG.md for #28670

* Update CHANGELOG.md #29081

* Update CHANGELOG.md for #29082

* Update CHANGELOG.md #28888

* Update CHANGELOG.md #29012

* Update CHANGELOG.md #28485

* Update CHANGELOG.md for #29094

* Update CHANGELOG.md #29107

* Update CHANGELOG.md #28368

* Update for #29113 #28884 #28739 #28920 #29018 #28822 #28977 #29096 #29040

* Update for #29088

* Update CHANGELOG.md #29079

* Update CHANGELOG.md #28992

* prep for release

---------

Co-authored-by: sreallymatt <106555974+sreallymatt@users.noreply.github.com>
Co-authored-by: Wyatt Fry <wyattfry@gmail.com>
Co-authored-by: jackofallops <11830746+jackofallops@users.noreply.github.com>
Co-authored-by: Matthew Frahry <mbfrahry@gmail.com>
Co-authored-by: stephybun <steph@hashicorp.com>
Co-authored-by: jackofallops <ste@hashicorp.com>
@github-actions
Copy link
Contributor

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 19, 2025
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.

Cognitive Account - Runtime error: Plugin crashed!

3 participants