`azurerm_api_management` fix to pass all acctests #7603

yupwei68 · 2020-07-07T06:41:32Z

developer_portal certificate keeps being updated, because in read function, when it gets d.Get("hostname_configuration") using key is strings.Lower("DeveloperPortal") instead "developer_portal"
identity.0.type=None causes plan diff not empty, because it'll not be returned back. Fulfill the identity type in schema setting when identity.0.type=None.
When VirtualNetworkType = VirtualNetworkTypeInternal, call sign_in, sign_up, policy service api causes error.
One default custom domain has been ignored because it causes plan refresh diff
key vault acctest needs to separate the steps because it depends on the permission assignment. But permission assignment depends on its identity. So it'll be a cycle dependency. Thus it needs to be separated into 3 steps

test:
=== RUN TestAccAzureRMApiManagement_basic
=== PAUSE TestAccAzureRMApiManagement_basic
=== CONT TestAccAzureRMApiManagement_basic
--- PASS: TestAccAzureRMApiManagement_basic (2751.90s)
=== RUN TestAccAzureRMApiManagement_requiresImport
=== PAUSE TestAccAzureRMApiManagement_requiresImport
=== CONT TestAccAzureRMApiManagement_requiresImport
--- PASS: TestAccAzureRMApiManagement_requiresImport (2500.16s)
=== RUN TestAccAzureRMApiManagement_customProps
=== PAUSE TestAccAzureRMApiManagement_customProps
=== CONT TestAccAzureRMApiManagement_customProps
--- PASS: TestAccAzureRMApiManagement_customProps (2551.24s)
=== RUN TestAccAzureRMApiManagement_complete
=== PAUSE TestAccAzureRMApiManagement_complete
=== CONT TestAccAzureRMApiManagement_complete
--- PASS: TestAccAzureRMApiManagement_complete (1451.06s)
=== RUN TestAccAzureRMApiManagement_signInSignUpSettings
=== PAUSE TestAccAzureRMApiManagement_signInSignUpSettings
=== CONT TestAccAzureRMApiManagement_signInSignUpSettings
--- PASS: TestAccAzureRMApiManagement_signInSignUpSettings (2473.22s)
=== RUN TestAccAzureRMApiManagement_policy
=== PAUSE TestAccAzureRMApiManagement_policy
=== CONT TestAccAzureRMApiManagement_policy
--- PASS: TestAccAzureRMApiManagement_policy (2488.46s)
=== RUN TestAccAzureRMApiManagement_virtualNetworkInternal
=== PAUSE TestAccAzureRMApiManagement_virtualNetworkInternal
=== CONT TestAccAzureRMApiManagement_virtualNetworkInternal
--- PASS: TestAccAzureRMApiManagement_virtualNetworkInternal (2593.72s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId (3016.47s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId (2984.86s)
=== RUN TestAccAzureRMApiManagement_identityUserAssigned
=== PAUSE TestAccAzureRMApiManagement_identityUserAssigned
=== CONT TestAccAzureRMApiManagement_identityUserAssigned
--- PASS: TestAccAzureRMApiManagement_identityUserAssigned (2584.16s)
=== RUN TestAccAzureRMApiManagement_identityNoneUpdateUserAssigned
=== PAUSE TestAccAzureRMApiManagement_identityNoneUpdateUserAssigned
=== CONT TestAccAzureRMApiManagement_identityNoneUpdateUserAssigned
--- PASS: TestAccAzureRMApiManagement_identityNoneUpdateUserAssigned (4731.82s)
=== RUN TestAccAzureRMApiManagement_identityUserAssignedUpdateNone
=== PAUSE TestAccAzureRMApiManagement_identityUserAssignedUpdateNone
=== CONT TestAccAzureRMApiManagement_identityUserAssignedUpdateNone
--- PASS: TestAccAzureRMApiManagement_identityUserAssignedUpdateNone (4973.29s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssigned
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssigned
=== CONT TestAccAzureRMApiManagement_identitySystemAssigned
--- PASS: TestAccAzureRMApiManagement_identitySystemAssigned (2417.97s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUpdateNone
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUpdateNone
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUpdateNone
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUpdateNone (4725.02s)
=== RUN TestAccAzureRMApiManagement_identityNoneUpdateSystemAssigned
=== PAUSE TestAccAzureRMApiManagement_identityNoneUpdateSystemAssigned
=== CONT TestAccAzureRMApiManagement_identityNoneUpdateSystemAssigned
--- PASS: TestAccAzureRMApiManagement_identityNoneUpdateSystemAssigned (4483.66s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateNone
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateNone
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateNone
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateNone (5097.89s)
=== RUN TestAccAzureRMApiManagement_identityNoneUpdateSystemAssignedUserAssigned
=== PAUSE TestAccAzureRMApiManagement_identityNoneUpdateSystemAssignedUserAssigned
=== CONT TestAccAzureRMApiManagement_identityNoneUpdateSystemAssignedUserAssigned
--- PASS: TestAccAzureRMApiManagement_identityNoneUpdateSystemAssignedUserAssigned (4758.50s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUserAssigned
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUserAssigned
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUserAssigned
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUserAssigned (2421.46s)

tombuildsstuff

Left a few comments inline but this otherwise LGTM 👍

azurerm/internal/services/apimanagement/api_management_resource.go

tombuildsstuff · 2020-07-07T09:38:45Z

azurerm/internal/services/apimanagement/api_management_resource.go

@@ -820,6 +829,11 @@ func flattenApiManagementHostnameConfigurations(input *[]apimanagement.HostnameC
 			output["host_name"] = *config.HostName
 		}

+		// There'll always be a default custom domain with hostName "apim_name.azure-api.net" and Type "Proxy", which should be ignored
+		if *config.HostName == strings.ToLower(name)+".azure-api.net" && config.Type == apimanagement.HostnameTypeProxy {


azure-api.net is only valid in Azure Public - looks like this'll need to be fetched/added to the environments list in Azure/go-autorest

Upstream issue opened: Azure/azure-rest-api-specs#10065

tombuildsstuff · 2020-07-10T09:55:20Z

Dependent on Azure/go-autorest#540

EklipZgit · 2020-09-15T21:01:23Z

@tombuildsstuff hey can this get merged? This bug is annoying.

yupwei68 · 2020-10-10T05:40:10Z

@tombuildsstuff I've rebased to the latest master branch. Thanks for your comments. All tests have passed. Please continue reviewing.

=== RUN TestAccAzureRMApiManagement_basic
=== PAUSE TestAccAzureRMApiManagement_basic
=== CONT TestAccAzureRMApiManagement_basic
--- PASS: TestAccAzureRMApiManagement_basic (2430.77s)
=== RUN TestAccAzureRMApiManagement_requiresImport
=== PAUSE TestAccAzureRMApiManagement_requiresImport
=== CONT TestAccAzureRMApiManagement_requiresImport
--- PASS: TestAccAzureRMApiManagement_requiresImport (2640.95s)
=== RUN TestAccAzureRMApiManagement_customProps
=== PAUSE TestAccAzureRMApiManagement_customProps
=== CONT TestAccAzureRMApiManagement_customProps
--- PASS: TestAccAzureRMApiManagement_customProps (2205.83s)
=== RUN TestAccAzureRMApiManagement_complete
=== PAUSE TestAccAzureRMApiManagement_complete
=== CONT TestAccAzureRMApiManagement_complete
--- PASS: TestAccAzureRMApiManagement_complete (1588.05s)
=== RUN TestAccAzureRMApiManagement_signInSignUpSettings
=== PAUSE TestAccAzureRMApiManagement_signInSignUpSettings
=== CONT TestAccAzureRMApiManagement_signInSignUpSettings
--- PASS: TestAccAzureRMApiManagement_signInSignUpSettings (2177.03s)
=== RUN TestAccAzureRMApiManagement_policy
=== PAUSE TestAccAzureRMApiManagement_policy
=== CONT TestAccAzureRMApiManagement_policy
--- PASS: TestAccAzureRMApiManagement_policy (2793.47s)
=== RUN TestAccAzureRMApiManagement_virtualNetworkInternal
=== PAUSE TestAccAzureRMApiManagement_virtualNetworkInternal
=== CONT TestAccAzureRMApiManagement_virtualNetworkInternal
--- PASS: TestAccAzureRMApiManagement_virtualNetworkInternal (3318.60s)
=== RUN TestAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation
=== PAUSE TestAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation
=== CONT TestAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation
--- PASS: TestAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation (2541.65s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId (2978.04s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId (3141.35s)
=== RUN TestAccAzureRMApiManagement_identityUserAssigned
=== PAUSE TestAccAzureRMApiManagement_identityUserAssigned
=== CONT TestAccAzureRMApiManagement_identityUserAssigned
--- PASS: TestAccAzureRMApiManagement_identityUserAssigned (2513.36s)
=== RUN TestAccAzureRMApiManagement_identityNoneUpdateUserAssigned
=== PAUSE TestAccAzureRMApiManagement_identityNoneUpdateUserAssigned
=== CONT TestAccAzureRMApiManagement_identityNoneUpdateUserAssigned
--- PASS: TestAccAzureRMApiManagement_identityNoneUpdateUserAssigned (3054.05s)
=== RUN TestAccAzureRMApiManagement_identityUserAssignedUpdateNone
=== PAUSE TestAccAzureRMApiManagement_identityUserAssignedUpdateNone
=== CONT TestAccAzureRMApiManagement_identityUserAssignedUpdateNone
--- PASS: TestAccAzureRMApiManagement_identityUserAssignedUpdateNone (2514.02s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssigned
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssigned
=== CONT TestAccAzureRMApiManagement_identitySystemAssigned
--- PASS: TestAccAzureRMApiManagement_identitySystemAssigned (2429.85s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUpdateNone
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUpdateNone
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUpdateNone
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUpdateNone (2552.19s)
=== RUN TestAccAzureRMApiManagement_identityNoneUpdateSystemAssigned
=== PAUSE TestAccAzureRMApiManagement_identityNoneUpdateSystemAssigned
=== CONT TestAccAzureRMApiManagement_identityNoneUpdateSystemAssigned
--- PASS: TestAccAzureRMApiManagement_identityNoneUpdateSystemAssigned (2967.79s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUserAssigned
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUserAssigned
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUserAssigned
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUserAssigned (2692.14s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateNone
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateNone
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateNone
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateNone (2380.81s)
=== RUN TestAccAzureRMApiManagement_identityNoneUpdateSystemAssignedUserAssigned
=== PAUSE TestAccAzureRMApiManagement_identityNoneUpdateSystemAssignedUserAssigned
=== CONT TestAccAzureRMApiManagement_identityNoneUpdateSystemAssignedUserAssigned
--- PASS: TestAccAzureRMApiManagement_identityNoneUpdateSystemAssignedUserAssigned (2512.43s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateSystemAssigned
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateSystemAssigned
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateSystemAssigned
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateSystemAssigned (2740.42s)
=== RUN TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateUserAssigned
=== PAUSE TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateUserAssigned
=== CONT TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateUserAssigned
--- PASS: TestAccAzureRMApiManagement_identitySystemAssignedUserAssignedUpdateUserAssigned (2392.75s)

yupwei68 · 2020-11-13T03:05:33Z

@manicminer All tests have passed. Please continue reviewing.

manicminer

@yupwei68 Thanks, this looks great. I have a couple suggestions regarding the virtual_network_type attribute and tidying up the tests a little.

Since the outputs of testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId(), testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultIdUpdateCD(), testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId() and testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultIdUpdateCD() are almost identical, could we consolidate these into a testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurations_template() function and only embed the differences in the other funcs? It would make the tests easier to reason about.

Aside from that, this looks good to me!

manicminer · 2020-11-13T12:34:34Z

azurerm/internal/services/apimanagement/api_management_resource.go

@@ -121,6 +121,9 @@ func resourceArmApiManagementService() *schema.Resource {
 				},
 			},

+			// Here we could not remove the `ForceNew` of the vnet type. Once we update the vnet type from `Internal` `External` to `None`, the destroy will fail because the subnet is still in use.
+			// In the above case, it doesn't remove the link to subnet immediately. It'll be removed within 3 hours.
+			// But if we destroy the APIM service with VNET type `Internal` or `External`, the subnet could be destroyed


If we can safely change from virtual_network_type = "None" to one of the other values, it might be worth conditionally enforcing ForceNew?

e.g.

CustomizeDiff: customdiff.All( customdiff.ForceNewIfChange("virtual_network_type", func(old, new, meta interface{}) bool { return (old.(string) == string(apimanagement.VirtualNetworkTypeExternal) || old.(string) == string(apimanagement.VirtualNetworkTypeInternal)) && new.(string) == string(apimanagement.VirtualNetworkTypeNone) }), ),

manicminer · 2020-11-18T12:16:32Z

@yupwei68 is this ready for re-review?

yupwei68 · 2020-11-24T01:52:52Z

@manicminer Thanks for comments. Please continue reviewing.
=== RUN TestAccAzureRMApiManagement_virtualNetworkInternalUpdate
=== PAUSE TestAccAzureRMApiManagement_virtualNetworkInternalUpdate
=== CONT TestAccAzureRMApiManagement_virtualNetworkInternalUpdate
--- PASS: TestAccAzureRMApiManagement_virtualNetworkInternalUpdate (5153.10s)

manicminer

Many thanks @yupwei68, this LGTM 🚀

Test results:

ghost · 2020-11-27T01:08:49Z

This has been released in version 2.38.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 = "~> 2.38.0"
}
# ... other configuration ...

ghost · 2020-12-25T14:50:12Z

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!

ghost added the size/XL label Jul 7, 2020

tombuildsstuff approved these changes Jul 7, 2020

View reviewed changes

tombuildsstuff added service/api-management tests waiting-response labels Jul 7, 2020

tombuildsstuff added this to the v2.18.0 milestone Jul 7, 2020

yupwei68 closed this Jul 8, 2020

yupwei68 reopened this Jul 8, 2020

yupwei68 force-pushed the wyp-apim-apim branch from a49d48b to 8273a75 Compare July 9, 2020 05:19

katbyte added the upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR label Jul 9, 2020

katbyte modified the milestones: v2.18.0, Blocked Jul 9, 2020

yupwei68 mentioned this pull request Jul 10, 2020

Add ApiManagementHostManagementSuffix in environment variables Azure/go-autorest#540

Merged

4 tasks

tombuildsstuff added the sdk/requires-upgrade This is dependent upon upgrading an SDK label Jul 10, 2020

yupwei68 force-pushed the wyp-apim-apim branch from 0876e7f to 7a86f5a Compare August 11, 2020 03:03

sschmeck mentioned this pull request Sep 1, 2020

azurerm_api_management add sku_name Consumption_0 #6868

Merged

ghost removed the waiting-response label Sep 15, 2020

yupwei68 added 3 commits October 9, 2020 11:10

merge upstream

16a2544

update

abdac9e

update

aa5f726

yupwei68 force-pushed the wyp-apim-apim branch from 7a86f5a to aa5f726 Compare October 10, 2020 01:54

ghost added the documentation label Oct 10, 2020

yupwei68 added 2 commits October 10, 2020 10:09

update

08c72c0

update

7366921

yupwei68 requested a review from tombuildsstuff October 10, 2020 05:40

update

e535094

tombuildsstuff removed the upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR label Oct 21, 2020

update

da36e67

ghost removed the waiting-response label Nov 13, 2020

manicminer suggested changes Nov 13, 2020

View reviewed changes

manicminer added the waiting-response label Nov 13, 2020

yupwei68 added 3 commits November 16, 2020 17:00

update

3522bfa

update

031d557

update

cdf3927

update

211da89

yupwei68 closed this Nov 19, 2020

yupwei68 reopened this Nov 19, 2020

yupwei68 closed this Nov 19, 2020

yupwei68 reopened this Nov 19, 2020

ghost removed the waiting-response label Nov 19, 2020

yupwei68 requested a review from manicminer November 19, 2020 02:37

yupwei68 closed this Nov 23, 2020

yupwei68 reopened this Nov 23, 2020

yupwei68 added 3 commits November 23, 2020 17:11

update

b16eea3

update

af4522e

fmt

9aaf7c8

manicminer approved these changes Nov 25, 2020

View reviewed changes

manicminer added this to the v2.38.0 milestone Nov 25, 2020

manicminer merged commit c5e9867 into hashicorp:master Nov 25, 2020

manicminer added a commit that referenced this pull request Nov 25, 2020

Update changelog for #7603

8f612fd

ghost locked as resolved and limited conversation to collaborators Dec 25, 2020

yupwei68 deleted the wyp-apim-apim branch June 7, 2021 06:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_api_management` fix to pass all acctests #7603

`azurerm_api_management` fix to pass all acctests #7603

yupwei68 commented Jul 7, 2020

tombuildsstuff left a comment

tombuildsstuff Jul 7, 2020

yupwei68 Jul 8, 2020

tombuildsstuff commented Jul 10, 2020

EklipZgit commented Sep 15, 2020

yupwei68 commented Oct 10, 2020

yupwei68 commented Nov 13, 2020

manicminer left a comment

manicminer Nov 13, 2020 •

edited

Loading

manicminer commented Nov 18, 2020

yupwei68 commented Nov 24, 2020

manicminer left a comment •

edited

Loading

ghost commented Nov 27, 2020

ghost commented Dec 25, 2020

azurerm_api_management fix to pass all acctests #7603

azurerm_api_management fix to pass all acctests #7603

Conversation

yupwei68 commented Jul 7, 2020

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Jul 7, 2020

Choose a reason for hiding this comment

yupwei68 Jul 8, 2020

Choose a reason for hiding this comment

tombuildsstuff commented Jul 10, 2020

EklipZgit commented Sep 15, 2020

yupwei68 commented Oct 10, 2020

yupwei68 commented Nov 13, 2020

manicminer left a comment

Choose a reason for hiding this comment

manicminer Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

manicminer commented Nov 18, 2020

yupwei68 commented Nov 24, 2020

manicminer left a comment • edited Loading

Choose a reason for hiding this comment

ghost commented Nov 27, 2020

ghost commented Dec 25, 2020

`azurerm_api_management` fix to pass all acctests #7603

`azurerm_api_management` fix to pass all acctests #7603

manicminer Nov 13, 2020 •

edited

Loading

manicminer left a comment •

edited

Loading