From 16a25440ebb6199278d558861731ca565d5abe02 Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Fri, 9 Oct 2020 11:10:47 +0800 Subject: [PATCH 01/14] merge upstream --- .../apimanagement/api_management_resource.go | 121 +++++---- .../tests/api_management_resource_test.go | 229 ++++++++++++++++-- 2 files changed, 287 insertions(+), 63 deletions(-) diff --git a/azurerm/internal/services/apimanagement/api_management_resource.go b/azurerm/internal/services/apimanagement/api_management_resource.go index 170c9b0b65f5..31d1b7300a98 100644 --- a/azurerm/internal/services/apimanagement/api_management_resource.go +++ b/azurerm/internal/services/apimanagement/api_management_resource.go @@ -88,9 +88,10 @@ func resourceArmApiManagementService() *schema.Resource { "type": { Type: schema.TypeString, Optional: true, - Default: string(apimanagement.None), + // None will not be returned, which will cause refresh diff + //Default: string(apimanagement.None), ValidateFunc: validation.StringInSlice([]string{ - string(apimanagement.None), + //string(apimanagement.None), string(apimanagement.SystemAssigned), string(apimanagement.UserAssigned), string(apimanagement.SystemAssignedUserAssigned), @@ -570,28 +571,32 @@ func resourceArmApiManagementServiceCreateUpdate(d *schema.ResourceData, meta in d.SetId(*read.ID) - signInSettingsRaw := d.Get("sign_in").([]interface{}) - signInSettings := expandApiManagementSignInSettings(signInSettingsRaw) - signInClient := meta.(*clients.Client).ApiManagement.SignInClient - if _, err := signInClient.CreateOrUpdate(ctx, resourceGroup, name, signInSettings, ""); err != nil { - return fmt.Errorf(" setting Sign In settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) + if d.HasChange("sign_in") { + signInSettingsRaw := d.Get("sign_in").([]interface{}) + signInSettings := expandApiManagementSignInSettings(signInSettingsRaw) + signInClient := meta.(*clients.Client).ApiManagement.SignInClient + if _, err := signInClient.CreateOrUpdate(ctx, resourceGroup, name, signInSettings, ""); err != nil { + return fmt.Errorf(" setting Sign In settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) + } } - signUpSettingsRaw := d.Get("sign_up").([]interface{}) - signUpSettings := expandApiManagementSignUpSettings(signUpSettingsRaw) - signUpClient := meta.(*clients.Client).ApiManagement.SignUpClient - if _, err := signUpClient.CreateOrUpdate(ctx, resourceGroup, name, signUpSettings, ""); err != nil { - return fmt.Errorf(" setting Sign Up settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) - } + if d.HasChange("sign_up") { + signUpSettingsRaw := d.Get("sign_up").([]interface{}) + signUpSettings := expandApiManagementSignUpSettings(signUpSettingsRaw) + signUpClient := meta.(*clients.Client).ApiManagement.SignUpClient - policyClient := meta.(*clients.Client).ApiManagement.PolicyClient - policiesRaw := d.Get("policy").([]interface{}) - policy, err := expandApiManagementPolicies(policiesRaw) - if err != nil { - return err + if _, err := signUpClient.CreateOrUpdate(ctx, resourceGroup, name, signUpSettings, ""); err != nil { + return fmt.Errorf(" setting Sign Up settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) + } } if d.HasChange("policy") { + policyClient := meta.(*clients.Client).ApiManagement.PolicyClient + policiesRaw := d.Get("policy").([]interface{}) + policy, err := expandApiManagementPolicies(policiesRaw) + if err != nil { + return err + } // remove the existing policy if resp, err := policyClient.Delete(ctx, resourceGroup, name, ""); err != nil { if !utils.ResponseWasNotFound(resp) { @@ -634,23 +639,36 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{ return fmt.Errorf("making Read request on API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) } - signInClient := meta.(*clients.Client).ApiManagement.SignInClient - signInSettings, err := signInClient.Get(ctx, resourceGroup, name) - if err != nil { - return fmt.Errorf("retrieving Sign In Settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) - } + if resp.VirtualNetworkType != apimanagement.VirtualNetworkTypeInternal { + signInClient := meta.(*clients.Client).ApiManagement.SignInClient + signInSettings, err := signInClient.Get(ctx, resourceGroup, name) + if err != nil { + return fmt.Errorf("retrieving Sign In Settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) + } - signUpClient := meta.(*clients.Client).ApiManagement.SignUpClient - signUpSettings, err := signUpClient.Get(ctx, resourceGroup, name) - if err != nil { - return fmt.Errorf("retrieving Sign Up Settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) - } + if err := d.Set("sign_in", flattenApiManagementSignInSettings(signInSettings)); err != nil { + return fmt.Errorf("setting `sign_in`: %+v", err) + } - policyClient := meta.(*clients.Client).ApiManagement.PolicyClient - policy, err := policyClient.Get(ctx, resourceGroup, name, apimanagement.PolicyExportFormatXML) - if err != nil { - if !utils.ResponseWasNotFound(policy.Response) { - return fmt.Errorf("retrieving Policy for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) + signUpClient := meta.(*clients.Client).ApiManagement.SignUpClient + signUpSettings, err := signUpClient.Get(ctx, resourceGroup, name) + if err != nil { + return fmt.Errorf("retrieving Sign Up Settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) + } + + if err := d.Set("sign_up", flattenApiManagementSignUpSettings(signUpSettings)); err != nil { + return fmt.Errorf("setting `sign_up`: %+v", err) + } + + policyClient := meta.(*clients.Client).ApiManagement.PolicyClient + policy, err := policyClient.Get(ctx, resourceGroup, name, apimanagement.PolicyExportFormatXML) + if err != nil { + if !utils.ResponseWasNotFound(policy.Response) { + return fmt.Errorf("retrieving Policy for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) + } + } + if err := d.Set("policy", flattenApiManagementPolicies(d, policy)); err != nil { + return fmt.Errorf("setting `policy`: %+v", err) } } @@ -688,7 +706,7 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{ return fmt.Errorf("setting `protocols`: %+v", err) } - hostnameConfigs := flattenApiManagementHostnameConfigurations(props.HostnameConfigurations, d) + hostnameConfigs := flattenApiManagementHostnameConfigurations(props.HostnameConfigurations, d, name) if err := d.Set("hostname_configuration", hostnameConfigs); err != nil { return fmt.Errorf("setting `hostname_configuration`: %+v", err) } @@ -706,18 +724,6 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{ return fmt.Errorf("setting `sku_name`: %+v", err) } - if err := d.Set("sign_in", flattenApiManagementSignInSettings(signInSettings)); err != nil { - return fmt.Errorf("setting `sign_in`: %+v", err) - } - - if err := d.Set("sign_up", flattenApiManagementSignUpSettings(signUpSettings)); err != nil { - return fmt.Errorf("setting `sign_up`: %+v", err) - } - - if err := d.Set("policy", flattenApiManagementPolicies(d, policy)); err != nil { - return fmt.Errorf("setting `policy`: %+v", err) - } - return tags.FlattenAndSet(d, resp.Tags) } @@ -829,7 +835,7 @@ func expandApiManagementCommonHostnameConfiguration(input map[string]interface{} return output } -func flattenApiManagementHostnameConfigurations(input *[]apimanagement.HostnameConfiguration, d *schema.ResourceData) []interface{} { +func flattenApiManagementHostnameConfigurations(input *[]apimanagement.HostnameConfiguration, d *schema.ResourceData, name string) []interface{} { results := make([]interface{}, 0) if input == nil { return results @@ -848,6 +854,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 { + continue + } + if config.NegotiateClientCertificate != nil { output["negotiate_client_certificate"] = *config.NegotiateClientCertificate } @@ -863,7 +874,7 @@ func flattenApiManagementHostnameConfigurations(input *[]apimanagement.HostnameC if len(existingHostnames) > 0 { v := existingHostnames[0].(map[string]interface{}) - if valsRaw, ok := v[strings.ToLower(string(config.Type))]; ok { + if valsRaw, ok := v[mapAzureRmApiManagementHostNameType(config.Type)]; ok { vals := valsRaw.([]interface{}) for _, val := range vals { oldConfig := val.(map[string]interface{}) @@ -898,6 +909,10 @@ func flattenApiManagementHostnameConfigurations(input *[]apimanagement.HostnameC } } + if len(managementResults) == 0 && len(portalResults) == 0 && len(developerPortalResults) == 0 && len(proxyResults) == 0 && len(scmResults) == 0 { + return []interface{}{} + } + return []interface{}{ map[string]interface{}{ "management": managementResults, @@ -909,6 +924,16 @@ func flattenApiManagementHostnameConfigurations(input *[]apimanagement.HostnameC } } +func mapAzureRmApiManagementHostNameType(t apimanagement.HostnameType) string { + switch t { + case apimanagement.HostnameTypeDeveloperPortal: + return "developer_portal" + default: + return strings.ToLower(string(t)) + } + +} + func expandAzureRmApiManagementCertificates(d *schema.ResourceData) *[]apimanagement.CertificateConfiguration { vs := d.Get("certificate").([]interface{}) diff --git a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go index 976e06b4097a..2ccd673f5ec7 100644 --- a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go +++ b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go @@ -227,6 +227,13 @@ func TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurati ), }, data.ImportStep(), + { + Config: testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultIdUpdateCD(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMApiManagementExists(data.ResourceName), + ), + }, + data.ImportStep(), }, }) } @@ -253,6 +260,13 @@ func TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurati ), }, data.ImportStep(), + { + Config: testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultIdUpdateCD(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMApiManagementExists(data.ResourceName), + ), + }, + data.ImportStep(), }, }) } @@ -1016,7 +1030,7 @@ resource "azurerm_api_management" "test" { ] } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger) +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomIntOfLength(12)) } func testAccAzureRMApiManagement_identitySystemAssigned(data acceptance.TestData) string { @@ -1025,11 +1039,11 @@ provider "azurerm" { features {} } resource "azurerm_resource_group" "test" { - name = "acctestRG-%d" - location = "%s" + name = "acctestRG-%[1]d" + location = "%[2]s" } resource "azurerm_api_management" "test" { - name = "acctestAM-%d" + name = "acctestAM-%[3]d" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name publisher_name = "pub1" @@ -1039,7 +1053,7 @@ resource "azurerm_api_management" "test" { type = "SystemAssigned" } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +`, data.RandomInteger, data.Locations.Primary, data.RandomIntOfLength(12)) } func testAccAzureRMApiManagement_identitySystemAssignedUserAssigned(data acceptance.TestData) string { @@ -1075,33 +1089,116 @@ resource "azurerm_api_management" "test" { ] } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger) +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomIntOfLength(12)) } -func testAccAzureRMApiManagement_identityNone(data acceptance.TestData) string { +func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { features {} } - resource "azurerm_resource_group" "test" { - name = "acctestRG-%d" - location = "%s" + name = "acctestRG-%[1]d" + location = "%[2]s" +} +data "azurerm_client_config" "current" {} +resource "azurerm_key_vault" "test" { + name = "acctestKV-%[3]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + tenant_id = data.azurerm_client_config.current.tenant_id + sku_name = "standard" +} +resource "azurerm_key_vault_access_policy" "test" { + key_vault_id = azurerm_key_vault.test.id + tenant_id = data.azurerm_client_config.current.tenant_id + object_id = data.azurerm_client_config.current.object_id + certificate_permissions = [ + "Create", + "Delete", + "Deleteissuers", + "Get", + "Getissuers", + "Import", + "List", + "Listissuers", + "Managecontacts", + "Manageissuers", + "Setissuers", + "Update", + ] + secret_permissions = [ + "Delete", + "Get", + "List", + "Purge", + ] +} +resource "azurerm_key_vault_access_policy" "test2" { + key_vault_id = azurerm_key_vault.test.id + tenant_id = azurerm_api_management.test.identity[0].tenant_id + object_id = azurerm_api_management.test.identity[0].principal_id + secret_permissions = [ + "Get", + "List", + ] +} +resource "azurerm_key_vault_certificate" "test" { + depends_on = [azurerm_key_vault_access_policy.test] + name = "acctestKVCert-%[3]d" + key_vault_id = azurerm_key_vault.test.id + certificate_policy { + issuer_parameters { + name = "Self" + } + key_properties { + exportable = true + key_size = 2048 + key_type = "RSA" + reuse_key = true + } + secret_properties { + content_type = "application/x-pkcs12" + } + x509_certificate_properties { + # Server Authentication = 1.3.6.1.5.5.7.3.1 + # Client Authentication = 1.3.6.1.5.5.7.3.2 + extended_key_usage = ["1.3.6.1.5.5.7.3.1"] + key_usage = [ + "cRLSign", + "dataEncipherment", + "digitalSignature", + "keyAgreement", + "keyCertSign", + "keyEncipherment", + ] + subject_alternative_names { + dns_names = ["api.terraform.io"] + } + subject = "CN=api.terraform.io" + validity_in_months = 1 + } + } } - resource "azurerm_api_management" "test" { - name = "acctestAM-%d" + name = "acctestAM-%[3]d" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name publisher_name = "pub1" publisher_email = "pub1@email.com" - sku_name = "Developer_1" +sku_name = "Developer_1" + + identity { + type = "SystemAssigned" + } + + } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +`, data.RandomInteger, data.Locations.Primary, data.RandomIntOfLength(12)) } -func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId(data acceptance.TestData) string { +func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultIdUpdateCD(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { features {} @@ -1303,6 +1400,108 @@ resource "azurerm_key_vault_certificate" "test" { } } } +resource "azurerm_api_management" "test" { + name = "acctestAM-%[3]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + publisher_name = "pub1" + publisher_email = "pub1@email.com" + sku_name = "Developer_1" + identity { + type = "SystemAssigned" + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomIntOfLength(12)) +} + +func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultIdUpdateCD(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} +resource "azurerm_resource_group" "test" { + name = "acctestRG-%[1]d" + location = "%[2]s" +} +data "azurerm_client_config" "current" {} +resource "azurerm_key_vault" "test" { + name = "acctestKV-%[3]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + tenant_id = data.azurerm_client_config.current.tenant_id + sku_name = "standard" +} +resource "azurerm_key_vault_access_policy" "test" { + key_vault_id = azurerm_key_vault.test.id + tenant_id = data.azurerm_client_config.current.tenant_id + object_id = data.azurerm_client_config.current.object_id + certificate_permissions = [ + "Create", + "Delete", + "Deleteissuers", + "Get", + "Getissuers", + "Import", + "List", + "Listissuers", + "Managecontacts", + "Manageissuers", + "Setissuers", + "Update", + ] + secret_permissions = [ + "Delete", + "Get", + "List", + "Purge", + ] +} +resource "azurerm_key_vault_access_policy" "test2" { + key_vault_id = azurerm_key_vault.test.id + tenant_id = azurerm_api_management.test.identity[0].tenant_id + object_id = azurerm_api_management.test.identity[0].principal_id + secret_permissions = [ + "Get", + "List", + ] +} +resource "azurerm_key_vault_certificate" "test" { + depends_on = [azurerm_key_vault_access_policy.test] + name = "acctestKVCert-%[3]d" + key_vault_id = azurerm_key_vault.test.id + certificate_policy { + issuer_parameters { + name = "Self" + } + key_properties { + exportable = true + key_size = 2048 + key_type = "RSA" + reuse_key = true + } + secret_properties { + content_type = "application/x-pkcs12" + } + x509_certificate_properties { + # Server Authentication = 1.3.6.1.5.5.7.3.1 + # Client Authentication = 1.3.6.1.5.5.7.3.2 + extended_key_usage = ["1.3.6.1.5.5.7.3.1"] + key_usage = [ + "cRLSign", + "dataEncipherment", + "digitalSignature", + "keyAgreement", + "keyCertSign", + "keyEncipherment", + ] + subject_alternative_names { + dns_names = ["api.terraform.io"] + } + subject = "CN=api.terraform.io" + validity_in_months = 1 + } + } +} resource "azurerm_api_management" "test" { name = "acctestAM-%[3]d" location = azurerm_resource_group.test.location From abdac9ead9601d244f771c2aabf5920b80dfcd76 Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Thu, 2 Jul 2020 15:35:22 +0800 Subject: [PATCH 02/14] update --- .../apimanagement/api_management_resource.go | 25 +++++-- .../tests/api_management_resource_test.go | 66 ++++++++++++++++--- 2 files changed, 78 insertions(+), 13 deletions(-) diff --git a/azurerm/internal/services/apimanagement/api_management_resource.go b/azurerm/internal/services/apimanagement/api_management_resource.go index 31d1b7300a98..97576d438453 100644 --- a/azurerm/internal/services/apimanagement/api_management_resource.go +++ b/azurerm/internal/services/apimanagement/api_management_resource.go @@ -88,8 +88,7 @@ func resourceArmApiManagementService() *schema.Resource { "type": { Type: schema.TypeString, Optional: true, - // None will not be returned, which will cause refresh diff - //Default: string(apimanagement.None), + Default: string(apimanagement.None), ValidateFunc: validation.StringInSlice([]string{ //string(apimanagement.None), string(apimanagement.SystemAssigned), @@ -639,7 +638,6 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{ return fmt.Errorf("making Read request on API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) } - if resp.VirtualNetworkType != apimanagement.VirtualNetworkTypeInternal { signInClient := meta.(*clients.Client).ApiManagement.SignInClient signInSettings, err := signInClient.Get(ctx, resourceGroup, name) if err != nil { @@ -666,7 +664,6 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{ if !utils.ResponseWasNotFound(policy.Response) { return fmt.Errorf("retrieving Policy for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) } - } if err := d.Set("policy", flattenApiManagementPolicies(d, policy)); err != nil { return fmt.Errorf("setting `policy`: %+v", err) } @@ -724,6 +721,18 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{ return fmt.Errorf("setting `sku_name`: %+v", err) } + if err := d.Set("sign_in", flattenApiManagementSignInSettings(signInSettings)); err != nil { + return fmt.Errorf("setting `sign_in`: %+v", err) + } + + if err := d.Set("sign_up", flattenApiManagementSignUpSettings(signUpSettings)); err != nil { + return fmt.Errorf("setting `sign_up`: %+v", err) + } + + if err := d.Set("policy", flattenApiManagementPolicies(d, policy)); err != nil { + return fmt.Errorf("setting `policy`: %+v", err) + } + return tags.FlattenAndSet(d, resp.Tags) } @@ -1064,6 +1073,14 @@ func expandAzureRmApiManagementIdentity(vs []interface{}) (*apimanagement.Servic } func flattenAzureRmApiManagementMachineIdentity(identity *apimanagement.ServiceIdentity) []interface{} { + // service api will not return identity when type = `None` + if v, ok := d.GetOk("identity.0.type"); ok && v.(string) == string(apimanagement.None) && identity == nil { + return []interface{}{ + map[string]interface{}{ + "type": v.(string), + }, + } + } if identity == nil || identity.Type == apimanagement.None { return make([]interface{}, 0) } diff --git a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go index 2ccd673f5ec7..dbfc2b73b78f 100644 --- a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go +++ b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go @@ -162,6 +162,9 @@ func TestAccAzureRMApiManagement_policy(t *testing.T) { }) } +// Here we could not update the APIM service out of VNET. The destroy will fail because the subnet is still in use. +// If we move the service out of VNET, it doesn't remove the link to subnet immediately. It'll be removed within 3 hours. +// But if we destroy the APIM service, the subnet could be destroyed func TestAccAzureRMApiManagement_virtualNetworkInternal(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_api_management", "test") @@ -170,6 +173,13 @@ func TestAccAzureRMApiManagement_virtualNetworkInternal(t *testing.T) { Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMApiManagementDestroy, Steps: []resource.TestStep{ + { + Config: testAccAzureRMApiManagement_basic(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMApiManagementExists(data.ResourceName), + ), + }, + data.ImportStep(), { Config: testAccAzureRMApiManagement_virtualNetworkInternal(data), Check: resource.ComposeTestCheckFunc( @@ -867,11 +877,10 @@ resource "azurerm_api_management" "test" { certificate_password = "terraform" } - #developer_portal { - # host_name = "developer-portal.terraform.io" - # certificate = filebase64("testdata/api_management_developer_portal_test.pfx") - # certificate_password = "terraform" - #} + developer_portal { + host_name = "developer-portal.terraform.io" + certificate = filebase64("testdata/api_management_developer_portal_test.pfx") + } } sku_name = "Premium_1" @@ -898,21 +907,60 @@ resource "azurerm_resource_group" "test" { } resource "azurerm_virtual_network" "test" { - name = "acctestVNET-%d" + name = "acctestVNET-%[1]d" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name address_space = ["10.0.0.0/16"] } resource "azurerm_subnet" "test" { - name = "acctestSNET-%d" + name = "acctestSNET-%[1]d" resource_group_name = azurerm_resource_group.test.name virtual_network_name = azurerm_virtual_network.test.name - address_prefix = "10.0.1.0/24" + address_prefixes = ["10.0.1.0/24"] +} + +resource "azurerm_network_security_group" "test" { + name = "acctest-NSG-%[1]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name +} + +resource "azurerm_subnet_network_security_group_association" "test" { + subnet_id = azurerm_subnet.test.id + network_security_group_id = azurerm_network_security_group.test.id +} + +resource "azurerm_network_security_rule" "port_3443" { + name = "Port_3443" + priority = 100 + direction = "Inbound" + access = "Allow" + protocol = "Tcp" + source_port_range = "*" + destination_port_range = "3443" + source_address_prefix = "ApiManagement" + destination_address_prefix = "VirtualNetwork" + resource_group_name = azurerm_resource_group.test.name + network_security_group_name = azurerm_network_security_group.test.name +} + +resource "azurerm_network_security_rule" "port_443_1433" { + name = "Port_443_1433" + priority = 100 + direction = "Outbound" + access = "Allow" + protocol = "Tcp" + source_port_range = "*" + destination_port_ranges = ["443", "1433"] + source_address_prefix = "VirtualNetwork" + destination_address_prefix = "VirtualNetwork" + resource_group_name = azurerm_resource_group.test.name + network_security_group_name = azurerm_network_security_group.test.name } resource "azurerm_api_management" "test" { - name = "acctestAM-%d" + name = "acctestAM-%[1]d" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name publisher_name = "pub1" From aa5f7268317713ecf9530c1af4248fef710dcd2f Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Sat, 10 Oct 2020 09:52:33 +0800 Subject: [PATCH 03/14] update --- .../apimanagement/api_management_resource.go | 108 +++++----- .../tests/api_management_resource_test.go | 192 ++++++++++++------ .../api_management_developer_portal_test.pfx | Bin 2301 -> 2660 bytes website/allowed-subcategories | 2 +- 4 files changed, 184 insertions(+), 118 deletions(-) diff --git a/azurerm/internal/services/apimanagement/api_management_resource.go b/azurerm/internal/services/apimanagement/api_management_resource.go index 97576d438453..9d7462ff50c4 100644 --- a/azurerm/internal/services/apimanagement/api_management_resource.go +++ b/azurerm/internal/services/apimanagement/api_management_resource.go @@ -90,7 +90,7 @@ func resourceArmApiManagementService() *schema.Resource { Optional: true, Default: string(apimanagement.None), ValidateFunc: validation.StringInSlice([]string{ - //string(apimanagement.None), + string(apimanagement.None), string(apimanagement.SystemAssigned), string(apimanagement.UserAssigned), string(apimanagement.SystemAssignedUserAssigned), @@ -117,6 +117,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 "virtual_network_type": { Type: schema.TypeString, Optional: true, @@ -570,32 +573,28 @@ func resourceArmApiManagementServiceCreateUpdate(d *schema.ResourceData, meta in d.SetId(*read.ID) - if d.HasChange("sign_in") { - signInSettingsRaw := d.Get("sign_in").([]interface{}) - signInSettings := expandApiManagementSignInSettings(signInSettingsRaw) - signInClient := meta.(*clients.Client).ApiManagement.SignInClient - if _, err := signInClient.CreateOrUpdate(ctx, resourceGroup, name, signInSettings, ""); err != nil { - return fmt.Errorf(" setting Sign In settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) - } + signInSettingsRaw := d.Get("sign_in").([]interface{}) + signInSettings := expandApiManagementSignInSettings(signInSettingsRaw) + signInClient := meta.(*clients.Client).ApiManagement.SignInClient + if _, err := signInClient.CreateOrUpdate(ctx, resourceGroup, name, signInSettings, ""); err != nil { + return fmt.Errorf(" setting Sign In settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) } - if d.HasChange("sign_up") { - signUpSettingsRaw := d.Get("sign_up").([]interface{}) - signUpSettings := expandApiManagementSignUpSettings(signUpSettingsRaw) - signUpClient := meta.(*clients.Client).ApiManagement.SignUpClient + signUpSettingsRaw := d.Get("sign_up").([]interface{}) + signUpSettings := expandApiManagementSignUpSettings(signUpSettingsRaw) + signUpClient := meta.(*clients.Client).ApiManagement.SignUpClient + if _, err := signUpClient.CreateOrUpdate(ctx, resourceGroup, name, signUpSettings, ""); err != nil { + return fmt.Errorf(" setting Sign Up settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) + } - if _, err := signUpClient.CreateOrUpdate(ctx, resourceGroup, name, signUpSettings, ""); err != nil { - return fmt.Errorf(" setting Sign Up settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) - } + policyClient := meta.(*clients.Client).ApiManagement.PolicyClient + policiesRaw := d.Get("policy").([]interface{}) + policy, err := expandApiManagementPolicies(policiesRaw) + if err != nil { + return err } if d.HasChange("policy") { - policyClient := meta.(*clients.Client).ApiManagement.PolicyClient - policiesRaw := d.Get("policy").([]interface{}) - policy, err := expandApiManagementPolicies(policiesRaw) - if err != nil { - return err - } // remove the existing policy if resp, err := policyClient.Delete(ctx, resourceGroup, name, ""); err != nil { if !utils.ResponseWasNotFound(resp) { @@ -616,6 +615,7 @@ func resourceArmApiManagementServiceCreateUpdate(d *schema.ResourceData, meta in func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).ApiManagement.ServiceClient + environment := meta.(*clients.Client).Account.Environment ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() @@ -638,34 +638,23 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{ return fmt.Errorf("making Read request on API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) } - signInClient := meta.(*clients.Client).ApiManagement.SignInClient - signInSettings, err := signInClient.Get(ctx, resourceGroup, name) - if err != nil { - return fmt.Errorf("retrieving Sign In Settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) - } - - if err := d.Set("sign_in", flattenApiManagementSignInSettings(signInSettings)); err != nil { - return fmt.Errorf("setting `sign_in`: %+v", err) - } - - signUpClient := meta.(*clients.Client).ApiManagement.SignUpClient - signUpSettings, err := signUpClient.Get(ctx, resourceGroup, name) - if err != nil { - return fmt.Errorf("retrieving Sign Up Settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) - } + signInClient := meta.(*clients.Client).ApiManagement.SignInClient + signInSettings, err := signInClient.Get(ctx, resourceGroup, name) + if err != nil { + return fmt.Errorf("retrieving Sign In Settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) + } - if err := d.Set("sign_up", flattenApiManagementSignUpSettings(signUpSettings)); err != nil { - return fmt.Errorf("setting `sign_up`: %+v", err) - } + signUpClient := meta.(*clients.Client).ApiManagement.SignUpClient + signUpSettings, err := signUpClient.Get(ctx, resourceGroup, name) + if err != nil { + return fmt.Errorf("retrieving Sign Up Settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) + } - policyClient := meta.(*clients.Client).ApiManagement.PolicyClient - policy, err := policyClient.Get(ctx, resourceGroup, name, apimanagement.PolicyExportFormatXML) - if err != nil { - if !utils.ResponseWasNotFound(policy.Response) { - return fmt.Errorf("retrieving Policy for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) - } - if err := d.Set("policy", flattenApiManagementPolicies(d, policy)); err != nil { - return fmt.Errorf("setting `policy`: %+v", err) + policyClient := meta.(*clients.Client).ApiManagement.PolicyClient + policy, err := policyClient.Get(ctx, resourceGroup, name, apimanagement.PolicyExportFormatXML) + if err != nil { + if !utils.ResponseWasNotFound(policy.Response) { + return fmt.Errorf("retrieving Policy for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err) } } @@ -676,7 +665,7 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{ d.Set("location", azure.NormalizeLocation(*location)) } - identity := flattenAzureRmApiManagementMachineIdentity(resp.Identity) + identity := flattenAzureRmApiManagementMachineIdentity(resp.Identity, d) if err := d.Set("identity", identity); err != nil { return fmt.Errorf("setting `identity`: %+v", err) } @@ -703,7 +692,8 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{ return fmt.Errorf("setting `protocols`: %+v", err) } - hostnameConfigs := flattenApiManagementHostnameConfigurations(props.HostnameConfigurations, d, name) + apimHostNameSuffix := environment.APIManagementHostNameSuffix + hostnameConfigs := flattenApiManagementHostnameConfigurations(props.HostnameConfigurations, d, name, apimHostNameSuffix) if err := d.Set("hostname_configuration", hostnameConfigs); err != nil { return fmt.Errorf("setting `hostname_configuration`: %+v", err) } @@ -844,7 +834,7 @@ func expandApiManagementCommonHostnameConfiguration(input map[string]interface{} return output } -func flattenApiManagementHostnameConfigurations(input *[]apimanagement.HostnameConfiguration, d *schema.ResourceData, name string) []interface{} { +func flattenApiManagementHostnameConfigurations(input *[]apimanagement.HostnameConfiguration, d *schema.ResourceData, name, apimHostNameSuffix string) []interface{} { results := make([]interface{}, 0) if input == nil { return results @@ -864,7 +854,7 @@ func flattenApiManagementHostnameConfigurations(input *[]apimanagement.HostnameC } // 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 { + if *config.HostName == strings.ToLower(name)+"."+apimHostNameSuffix && config.Type == apimanagement.HostnameTypeProxy { continue } @@ -1072,15 +1062,15 @@ func expandAzureRmApiManagementIdentity(vs []interface{}) (*apimanagement.Servic return &managedServiceIdentity, nil } -func flattenAzureRmApiManagementMachineIdentity(identity *apimanagement.ServiceIdentity) []interface{} { +func flattenAzureRmApiManagementMachineIdentity(identity *apimanagement.ServiceIdentity, d *schema.ResourceData) []interface{} { // service api will not return identity when type = `None` - if v, ok := d.GetOk("identity.0.type"); ok && v.(string) == string(apimanagement.None) && identity == nil { - return []interface{}{ - map[string]interface{}{ - "type": v.(string), - }, - } - } + //if v, ok := d.GetOk("identity.0.type"); ok && v.(string) == string(apimanagement.None) && identity == nil { + // return []interface{}{ + // map[string]interface{}{ + // "type": v.(string), + // }, + // } + //} if identity == nil || identity.Type == apimanagement.None { return make([]interface{}, 0) } diff --git a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go index dbfc2b73b78f..0ca8f72f0b85 100644 --- a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go +++ b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go @@ -162,9 +162,6 @@ func TestAccAzureRMApiManagement_policy(t *testing.T) { }) } -// Here we could not update the APIM service out of VNET. The destroy will fail because the subnet is still in use. -// If we move the service out of VNET, it doesn't remove the link to subnet immediately. It'll be removed within 3 hours. -// But if we destroy the APIM service, the subnet could be destroyed func TestAccAzureRMApiManagement_virtualNetworkInternal(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_api_management", "test") @@ -173,13 +170,6 @@ func TestAccAzureRMApiManagement_virtualNetworkInternal(t *testing.T) { Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMApiManagementDestroy, Steps: []resource.TestStep{ - { - Config: testAccAzureRMApiManagement_basic(data), - Check: resource.ComposeTestCheckFunc( - testCheckAzureRMApiManagementExists(data.ResourceName), - ), - }, - data.ImportStep(), { Config: testAccAzureRMApiManagement_virtualNetworkInternal(data), Check: resource.ComposeTestCheckFunc( @@ -215,6 +205,10 @@ func TestAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation(t *tes }) } +// Api Management doesn't support hostname keyvault using UserAssigned Identity +// There will be a inevitable dependency cycle here when using SystemAssigned Identity +// 1. create SystemAssigned Identity, grant the identity certificate access +// 2. Update the hostname configuration of the keyvault certificate func TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_api_management", "test") @@ -223,13 +217,6 @@ func TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurati Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMApiManagementDestroy, Steps: []resource.TestStep{ - { - Config: testAccAzureRMApiManagement_identitySystemAssigned(data), - Check: resource.ComposeTestCheckFunc( - testCheckAzureRMApiManagementExists(data.ResourceName), - ), - }, - data.ImportStep(), { Config: testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId(data), Check: resource.ComposeTestCheckFunc( @@ -256,13 +243,6 @@ func TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurati Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMApiManagementDestroy, Steps: []resource.TestStep{ - { - Config: testAccAzureRMApiManagement_identitySystemAssigned(data), - Check: resource.ComposeTestCheckFunc( - testCheckAzureRMApiManagementExists(data.ResourceName), - ), - }, - data.ImportStep(), { Config: testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId(data), Check: resource.ComposeTestCheckFunc( @@ -902,8 +882,8 @@ provider "azurerm" { } resource "azurerm_resource_group" "test" { - name = "acctestRG-%d" - location = "%s" + name = "acctestRG-%[1]d" + location = "%[2]s" } resource "azurerm_virtual_network" "test" { @@ -983,45 +963,126 @@ provider "azurerm" { } resource "azurerm_resource_group" "test1" { - name = "acctestRG1-%d" - location = "%s" + name = "acctestRG1-%[1]d" + location = "%[2]s" } resource "azurerm_resource_group" "test2" { - name = "acctestRG2-%d" - location = "%s" + name = "acctestRG2-%[1]d" + location = "%[3]s" } +// subnet1 from the first location resource "azurerm_virtual_network" "test1" { - name = "acctestVNET1-%d" + name = "acctestVNET1-%[1]d" location = azurerm_resource_group.test1.location resource_group_name = azurerm_resource_group.test1.name address_space = ["10.0.0.0/16"] } resource "azurerm_subnet" "test1" { - name = "acctestSNET1-%d" + name = "acctestSNET1-%[1]d" resource_group_name = azurerm_resource_group.test1.name virtual_network_name = azurerm_virtual_network.test1.name address_prefix = "10.0.1.0/24" } +resource "azurerm_network_security_group" "test1" { + name = "acctest-NSG1-%[1]d" + location = azurerm_resource_group.test1.location + resource_group_name = azurerm_resource_group.test1.name +} + +resource "azurerm_subnet_network_security_group_association" "test1" { + subnet_id = azurerm_subnet.test1.id + network_security_group_id = azurerm_network_security_group.test1.id +} + +resource "azurerm_network_security_rule" "port_3443" { + name = "Port_3443" + priority = 100 + direction = "Inbound" + access = "Allow" + protocol = "Tcp" + source_port_range = "*" + destination_port_range = "3443" + source_address_prefix = "ApiManagement" + destination_address_prefix = "VirtualNetwork" + resource_group_name = azurerm_resource_group.test1.name + network_security_group_name = azurerm_network_security_group.test1.name +} + +resource "azurerm_network_security_rule" "port_443_1433" { + name = "Port_443_1433" + priority = 100 + direction = "Outbound" + access = "Allow" + protocol = "Tcp" + source_port_range = "*" + destination_port_ranges = ["443", "1433"] + source_address_prefix = "VirtualNetwork" + destination_address_prefix = "VirtualNetwork" + resource_group_name = azurerm_resource_group.test1.name + network_security_group_name = azurerm_network_security_group.test1.name +} + +// subnet2 from the second location resource "azurerm_virtual_network" "test2" { - name = "acctestVNET2-%d" + name = "acctestVNET2-%[1]d" location = azurerm_resource_group.test2.location resource_group_name = azurerm_resource_group.test2.name address_space = ["10.1.0.0/16"] } resource "azurerm_subnet" "test2" { - name = "acctestSNET2-%d" + name = "acctestSNET2-%[1]d" resource_group_name = azurerm_resource_group.test2.name virtual_network_name = azurerm_virtual_network.test2.name address_prefix = "10.1.1.0/24" } +resource "azurerm_network_security_group" "test2" { + name = "acctest-NSG2-%[1]d" + location = azurerm_resource_group.test2.location + resource_group_name = azurerm_resource_group.test2.name +} + +resource "azurerm_subnet_network_security_group_association" "test2" { + subnet_id = azurerm_subnet.test2.id + network_security_group_id = azurerm_network_security_group.test2.id +} + +resource "azurerm_network_security_rule" "port_3443_2" { + name = "Port_3443" + priority = 100 + direction = "Inbound" + access = "Allow" + protocol = "Tcp" + source_port_range = "*" + destination_port_range = "3443" + source_address_prefix = "ApiManagement" + destination_address_prefix = "VirtualNetwork" + resource_group_name = azurerm_resource_group.test2.name + network_security_group_name = azurerm_network_security_group.test2.name +} + +resource "azurerm_network_security_rule" "port_443_1433_2" { + name = "Port_443_1433" + priority = 100 + direction = "Outbound" + access = "Allow" + protocol = "Tcp" + source_port_range = "*" + destination_port_ranges = ["443", "1433"] + source_address_prefix = "VirtualNetwork" + destination_address_prefix = "VirtualNetwork" + resource_group_name = azurerm_resource_group.test2.name + network_security_group_name = azurerm_network_security_group.test2.name +} + + resource "azurerm_api_management" "test" { - name = "acctestAM-%d" + name = "acctestAM-%[1]d" location = azurerm_resource_group.test1.location resource_group_name = azurerm_resource_group.test1.name publisher_name = "pub1" @@ -1041,7 +1102,7 @@ resource "azurerm_api_management" "test" { subnet_id = azurerm_subnet.test1.id } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.Locations.Secondary, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger) +`, data.RandomInteger, data.Locations.Primary, data.Locations.Secondary) } func testAccAzureRMApiManagement_identityUserAssigned(data acceptance.TestData) string { @@ -1078,7 +1139,7 @@ resource "azurerm_api_management" "test" { ] } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomIntOfLength(12)) +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger) } func testAccAzureRMApiManagement_identitySystemAssigned(data acceptance.TestData) string { @@ -1087,11 +1148,11 @@ provider "azurerm" { features {} } resource "azurerm_resource_group" "test" { - name = "acctestRG-%[1]d" - location = "%[2]s" + name = "acctestRG-%d" + location = "%s" } resource "azurerm_api_management" "test" { - name = "acctestAM-%[3]d" + name = "acctestAM-%d" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name publisher_name = "pub1" @@ -1101,7 +1162,7 @@ resource "azurerm_api_management" "test" { type = "SystemAssigned" } } -`, data.RandomInteger, data.Locations.Primary, data.RandomIntOfLength(12)) +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) } func testAccAzureRMApiManagement_identitySystemAssignedUserAssigned(data acceptance.TestData) string { @@ -1137,7 +1198,30 @@ resource "azurerm_api_management" "test" { ] } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomIntOfLength(12)) +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger) +} + +func testAccAzureRMApiManagement_identityNone(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_api_management" "test" { + name = "acctestAM-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + publisher_name = "pub1" + publisher_email = "pub1@email.com" + + sku_name = "Developer_1" +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) } func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId(data acceptance.TestData) string { @@ -1151,7 +1235,7 @@ resource "azurerm_resource_group" "test" { } data "azurerm_client_config" "current" {} resource "azurerm_key_vault" "test" { - name = "acctestKV-%[3]d" + name = "acctestKV-%[4]s" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name tenant_id = data.azurerm_client_config.current.tenant_id @@ -1234,16 +1318,12 @@ resource "azurerm_api_management" "test" { resource_group_name = azurerm_resource_group.test.name publisher_name = "pub1" publisher_email = "pub1@email.com" - -sku_name = "Developer_1" - + sku_name = "Developer_1" identity { type = "SystemAssigned" } - - } -`, data.RandomInteger, data.Locations.Primary, data.RandomIntOfLength(12)) +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomString) } func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultIdUpdateCD(data acceptance.TestData) string { @@ -1287,7 +1367,6 @@ resource "azurerm_key_vault_access_policy" "test" { "List", "Purge", ] - depends_on = [azurerm_key_vault.test] } resource "azurerm_key_vault_access_policy" "test2" { key_vault_id = azurerm_key_vault.test.id @@ -1297,7 +1376,6 @@ resource "azurerm_key_vault_access_policy" "test2" { "Get", "List", ] - depends_on = [azurerm_key_vault.test] } resource "azurerm_key_vault_certificate" "test" { depends_on = [azurerm_key_vault_access_policy.test] @@ -1399,20 +1477,18 @@ resource "azurerm_key_vault_access_policy" "test" { "List", "Purge", ] - depends_on = [azurerm_key_vault.test] } resource "azurerm_key_vault_access_policy" "test2" { key_vault_id = azurerm_key_vault.test.id - tenant_id = azurerm_api_management.test.identity[0].tenant_id - object_id = azurerm_api_management.test.identity[0].principal_id + tenant_id = azurerm_api_management.test.identity.0.tenant_id + object_id = azurerm_api_management.test.identity.0.principal_id secret_permissions = [ "Get", "List", ] - depends_on = [azurerm_key_vault.test] } resource "azurerm_key_vault_certificate" "test" { - depends_on = [azurerm_key_vault_access_policy.test, azurerm_key_vault.test] + depends_on = [azurerm_key_vault_access_policy.test] name = "acctestKVCert-%[3]d" key_vault_id = azurerm_key_vault.test.id certificate_policy { @@ -1459,7 +1535,7 @@ resource "azurerm_api_management" "test" { type = "SystemAssigned" } } -`, data.RandomInteger, data.Locations.Primary, data.RandomIntOfLength(12)) +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomString) } func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultIdUpdateCD(data acceptance.TestData) string { @@ -1473,7 +1549,7 @@ resource "azurerm_resource_group" "test" { } data "azurerm_client_config" "current" {} resource "azurerm_key_vault" "test" { - name = "acctestKV-%[3]d" + name = "acctestKV-%[4]s" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name tenant_id = data.azurerm_client_config.current.tenant_id diff --git a/azurerm/internal/services/apimanagement/tests/testdata/api_management_developer_portal_test.pfx b/azurerm/internal/services/apimanagement/tests/testdata/api_management_developer_portal_test.pfx index cede6279330b25ddbc204d1aeee450eec66856b1..c86d2089c0fd2dce173228ca86ad2ec362f74d81 100644 GIT binary patch literal 2660 zcmZXVcU05a632gmBnVPMCsKldfCw586S~r*g(8R`(pzYuNfT)Xf)s;v2{o{Eq)Cyo zz#>G7ARvMuRirG^q$7&FxbMB)^Ugc>%$b?b%zV$ef80ArJcBI_h!%-w;DgdjL~BPM zGJ)toWIO{C7|%dY#SkPO%KSei$SD{P`HsXx4ym>aJ^P<2Mg|ZK84o@{;=wyeJ}AR~ zF;$!yd`4-3^+mpMZafVQ-7pvr9yS)KSuEXnci|(BHJ-0<*7%y`)8-9~Ow8d;G@1ZJ z)xR+_aLPcpKV(m^>hA|@bflUd81u>qinviq>>l9M76r%iEP5_zJm;6W%g_*wgR9-J zPZvio+M?)}rx>E&(sqjn+N)bC-Qpx{BfCUqn<)K z`debFflV>C6-2kaV!vvDvWi(Q_MsD;E$g|GTM(6nH6Y^Sn^H+e{u$kVFt=;oSay!bzfsK4dnZvj0pH4>MCe=HHF$Jp>3^t=T z5Z#qU^KhH8pC`i_eSi$UEXpmMW$*UxUSn-T+ht6fPvo6ICvrqAP1$)6eeR^GmX+?D z{u)ZM`Cw}-EBS`}w*@5`yI?zw$CODVrN}Uk6jXybkl2j#nv5j))n|-7#m}{uhbK7X z%@zgYmBiao+Ge+tZ8$ANpswQndt))c2weG1ek$ zx`oQuqoJ@TgUn{?9;(uXUIwA}Ptz@iTM}0fRo}1BFX~pdF$#`*OJ~Q!c8S$Nt+k z$b|&$K;MIz?ObdHmLr2<{y+vTXNI{AEAy+)@A|O=e|L*e{b2y%XdZVHDEE#M64(#3 zrDKgDnCDkQ<`h>>tMlR_my0Wt2S30V&nzC-1+SReodkq@o%D>+Eud8NT5F8AtUCLx z9ZOy0;|V|G&OzvYY;K6lOw%h)Un)I95LgVV5o%Dbb*q&8wdtCp5|#o|%eNiC7|^H80P_T&NA4?{n+?*G;ptIjk2HM!3`J4&2adWeR* zWJ8N4Mj?FQlSCbCeAKV5OLabHszErHY)l-^b9(tbQSVu zWW^+98QL?#q{63k7VQ&OH8EkOJf)>DbaAZxO(OBh;+*E5sc5nY_<13-;DeMRzGKcF zF-~E}SiPq0iaFEnBag43DZ?7Ky>5Hu`O|FE>ozGQc21L|^hDAoy<%_; zd01AA+rh9YCy{8Z} zOr@Fhqg5pP-`^xXr!*7{1_1zoH2Vi~Nb3q{1F`@bKmvAvJ>URHQ;!tY>;We#lcLt_ z0LQQPF1P0;JtRQI*0XIM&aHTf+0$#v< zsyPFGfFPg=cmgg|QBP{ci7G7!7y~AND%JmDzj9S-j|tVQQzst)-qa~C085SSsIkjm zih_XAU#bDr%#o_%i^S7z{e3GsC>c*XPes!-Ab{$|{;7cee+@{41Q>Ju8Bq3X4MgB+ zOTD1YUct3k*K2zLm62VXmUBE+_|Y<^Cg zk-23UQ#P;qyq>(wX9I~$3DNqvzB|I(+d&Nt=Yn9K@k&c;bGcb{WlX+TC@%13p3q1{ z61XssMUX>HmTrgUWKk;iQg;f2Zhn)^ov024*$?&JJj5?6-rEBJ*%XYYy38Wqz(bM;};+ z=6Y4T@4#bJ!d3B}14nLW=)2OHU9Zy~gidZoVoqMN(dA@3h60ANMZz+yXuyeYUzaR1+PG4W~7HgM>YeHLYj}k~N zVRFIlE}pM)BnY07xOjy%*M)BQw!d`5-mby#n`JNFo?SgC*8vHPgRvcPlzo|-X@&zq zwP{_?%1A4?$2ZbdcAMO%D>QkB-uT>>e4`tseNj^gDPfY772iIXx1v=GZAHUy!lD7+ zO7?o-U#&KOvBSow%k_J!ADvnjERVZwg ze2+^rz&)V3!IVA|HLDmTl2L40ou0>5nA?iNl}bS-qMZzNgljxrM1?hx6qvw;qm`9BF8aE#-~zc28xaR9s!SajU>7YEbL zB6fBd=sF23R#!gsZEaUzy9E}nvGd`DmQ5KabP(P4i~HOMttg_w@|++AzhF0PDCpX^ zt#F}Fu;ZY(VXJ^TpOm8~+;;^^#vHWs%!KuJkS&yEXY(utgF51Dyy(_%&wz(eoIN& zlyu-yMA=`AvC@)uwNlli8$BDacD^4o+W_65W!72nojOmhaHJ1^Y_7CD!ETwpzw^^v zF+ zc4YQamH+gqCkr##wb*ZRcT3fos^^M@5rZsB+-Xo;nP`WL-MX5H1&=xIKKQv5g=*!* znR44cN$sFi3Ac^`;qj$ZI$uoX{R8Xt@>6+v2_MzT`<)S2?BRFd{dP5sw{-lFF+a}3 zPk(W@@=+SV9}o-Gs(Mudt8mfO-xf;?W`tz~iVf3zMjJVan#Fc2BzWNzJ=rb~Q*jLQ za=yMv&Pql_X^g{X=w1JoLM>2qWHR%iujX|xeuVs}i+&g#pC7 zIZ`JhWa<8+87*(@g&Ox|&oRHBR~7|}Ky^cHEesM>>3K^HE*bOslEE>@AGxVo?C2Lr z7BF998+_Z;Lp1XQ9|SHmfj`vWD+`%+nNQ0?MFB-E_(ADGbSV?j@okXL+w4ot8Tga| zU*kprb7}=8@rpr+z;jmAlSpSg6?7Q(t!@PAuWS6*M6%S!`Q+8*w}-Cf%vkzY>OMr= zuOg41%tG3=!QG)1ahr_kyYuvCw{O|FJAr(pHcBRaZWir6fYT42Zdc>XZ#U_&xTy^U z^cCboNldc?g9*fh6Z`qX0TR^Ye?%%HL61X7P|ZWEdN_(4-2c()2pfAD39=55AfNxw zHt=uTq>Xu8unJ4W-?l+Wkmm?R*%$TUkCGI9yOe`5zLbNO%$g-!VDZvdjx?>Tv^T9- zZNcx}kr8{y5tNU$Yh~e>qts$H%Ti%2AiDoFHeO($#O)b1HKAE((%6w`^yxKsTUo3X zNIGcM=vyLm#N<{e`PwO`PO-7PiH(7LeSzALNk#DLw}5u56ya=xTT#@hRx-%BZuTa> zyYL%pE4b3kvKAw_6bm*{7HqkI8PR`KGZ|N@sg4E{JV;PP{zkpMLKPfv0nU$lSm!#!SG;8xp@f}; zTuK~&z4Y@g`lq2@V*1`_mTX^31Hhx41CEKDUaMlj3pbs<>@@MNom#3*)5)iFR3pb% zrP~R_;etlT^c)gJu410mO&#P?SAdvy1jlWPc2cLh>@B zv9c;gm3x=1c^WE{y?PhjA+MWN(&3YQ&ybBb8P2yLYo&0ZX7NuBhu<%jLH1ns!QG2YQJ%VFJdvb6k{X`uVD$*)Q_Z7r$d%v>#a8 zBLeUAX;vpdy3NkMMiK~Q2B3n`HEjKLY)HB8hdnet*j6#+tW83hmc-sqNy!i?tPs=K zonr8JnV{0%cd4gx@VqWTgE^c;%?+W^eW>BtpuJ>_hnU=(H!*U@_nwpu#y)MU4JsMw zuamHLqi75~*)!9!fK)HZ0-$)#lhb*8#A$CLXU6776)(+C1@YDNGu9dmk0~+HT~`n9 zj3ADEG*SPL)O%U{9zx)_+ezXTUU#1t7Td^3GH&X;s)Rh5kNBmseU_*&HyK4U zuEvc8^>{!w5DSG|Y2_u^`Br!Kv{(0Z Date: Sat, 10 Oct 2020 10:09:53 +0800 Subject: [PATCH 04/14] update --- .../apimanagement/api_management_resource.go | 12 ++---------- website/allowed-subcategories | 2 +- website/docs/r/api_management.html.markdown | 3 ++- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/azurerm/internal/services/apimanagement/api_management_resource.go b/azurerm/internal/services/apimanagement/api_management_resource.go index 9d7462ff50c4..f6dfaf8851bd 100644 --- a/azurerm/internal/services/apimanagement/api_management_resource.go +++ b/azurerm/internal/services/apimanagement/api_management_resource.go @@ -665,7 +665,7 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{ d.Set("location", azure.NormalizeLocation(*location)) } - identity := flattenAzureRmApiManagementMachineIdentity(resp.Identity, d) + identity := flattenAzureRmApiManagementMachineIdentity(resp.Identity) if err := d.Set("identity", identity); err != nil { return fmt.Errorf("setting `identity`: %+v", err) } @@ -1062,15 +1062,7 @@ func expandAzureRmApiManagementIdentity(vs []interface{}) (*apimanagement.Servic return &managedServiceIdentity, nil } -func flattenAzureRmApiManagementMachineIdentity(identity *apimanagement.ServiceIdentity, d *schema.ResourceData) []interface{} { - // service api will not return identity when type = `None` - //if v, ok := d.GetOk("identity.0.type"); ok && v.(string) == string(apimanagement.None) && identity == nil { - // return []interface{}{ - // map[string]interface{}{ - // "type": v.(string), - // }, - // } - //} +func flattenAzureRmApiManagementMachineIdentity(identity *apimanagement.ServiceIdentity) []interface{} { if identity == nil || identity.Type == apimanagement.None { return make([]interface{}, 0) } diff --git a/website/allowed-subcategories b/website/allowed-subcategories index 8b9011f49b6f..3bd6f9ea178a 100644 --- a/website/allowed-subcategories +++ b/website/allowed-subcategories @@ -26,7 +26,7 @@ Data Share Database Database Migration Databricks -DesktopVirtualization +Desktop Virtualization Dev Test DevSpace HDInsight diff --git a/website/docs/r/api_management.html.markdown b/website/docs/r/api_management.html.markdown index 4c20071cf791..c7d8bcfd6a05 100644 --- a/website/docs/r/api_management.html.markdown +++ b/website/docs/r/api_management.html.markdown @@ -79,7 +79,8 @@ The following arguments are supported: * `sign_up` - (Optional) A `sign_up` block as defined below. -* `virtual_network_type` - (Optional) The type of virtual network you want to use, valid values include: `None`, `External`, `Internal`. +* `virtual_network_type` - (Optional) The type of virtual network you want to use, valid values include: `None`, `External`, `Internal`. +> **NOTE:** Please ensure that in the subnet, inbound port 3443 is open when `virtual_network_type` is `Internal` or `External`. And please ensure other necessary ports are open according to [api management network configuration](https://docs.microsoft.com/en-us/azure/api-management/api-management-using-with-vnet#-common-network-configuration-issues). * `virtual_network_configuration` - (Optional) A `virtual_network_configuration` block as defined below. Required when `virtual_network_type` is `External` or `Internal`. From 7366921561f442395c0b97d0408ca4b88927b512 Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Sat, 10 Oct 2020 13:38:49 +0800 Subject: [PATCH 05/14] update --- .../internal/services/apimanagement/api_management_resource.go | 1 - 1 file changed, 1 deletion(-) diff --git a/azurerm/internal/services/apimanagement/api_management_resource.go b/azurerm/internal/services/apimanagement/api_management_resource.go index f6dfaf8851bd..d71656b2f11b 100644 --- a/azurerm/internal/services/apimanagement/api_management_resource.go +++ b/azurerm/internal/services/apimanagement/api_management_resource.go @@ -930,7 +930,6 @@ func mapAzureRmApiManagementHostNameType(t apimanagement.HostnameType) string { default: return strings.ToLower(string(t)) } - } func expandAzureRmApiManagementCertificates(d *schema.ResourceData) *[]apimanagement.CertificateConfiguration { From e53509418e29922063c25c4c62b3ca9de88661fd Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Sat, 10 Oct 2020 17:07:07 +0800 Subject: [PATCH 06/14] update --- .../internal/services/apimanagement/api_management_resource.go | 1 - 1 file changed, 1 deletion(-) diff --git a/azurerm/internal/services/apimanagement/api_management_resource.go b/azurerm/internal/services/apimanagement/api_management_resource.go index d71656b2f11b..fdb202e3e591 100644 --- a/azurerm/internal/services/apimanagement/api_management_resource.go +++ b/azurerm/internal/services/apimanagement/api_management_resource.go @@ -300,7 +300,6 @@ func resourceArmApiManagementService() *schema.Resource { "hostname_configuration": { Type: schema.TypeList, Optional: true, - Computed: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ From da36e672c2a3e6ad2e1ddd67f86e2287bb4f1d62 Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Fri, 13 Nov 2020 11:04:32 +0800 Subject: [PATCH 07/14] update --- website/docs/r/api_management.html.markdown | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/website/docs/r/api_management.html.markdown b/website/docs/r/api_management.html.markdown index 32e2cf4f4bed..8cb2f5318790 100644 --- a/website/docs/r/api_management.html.markdown +++ b/website/docs/r/api_management.html.markdown @@ -326,10 +326,10 @@ An `identity` block exports the following: The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/docs/configuration/resources.html#timeouts) for certain actions: -* `create` - (Defaults to 60 minutes) Used when creating the API Management Service. -* `update` - (Defaults to 60 minutes) Used when updating the API Management Service. +* `create` - (Defaults to 3 hours) Used when creating the API Management Service. +* `update` - (Defaults to 3 hours) Used when updating the API Management Service. * `read` - (Defaults to 5 minutes) Used when retrieving the API Management Service. -* `delete` - (Defaults to 60 minutes) Used when deleting the API Management Service. +* `delete` - (Defaults to 3 hours) Used when deleting the API Management Service. ## Import From 3522bfafe8e235265159409270dc4739678db21b Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Mon, 16 Nov 2020 17:00:14 +0800 Subject: [PATCH 08/14] update --- .../apimanagement/api_management_resource.go | 11 +- .../tests/api_management_resource_test.go | 503 ++++-------------- 2 files changed, 117 insertions(+), 397 deletions(-) diff --git a/azurerm/internal/services/apimanagement/api_management_resource.go b/azurerm/internal/services/apimanagement/api_management_resource.go index c3e7942356c0..5d1713de9538 100644 --- a/azurerm/internal/services/apimanagement/api_management_resource.go +++ b/azurerm/internal/services/apimanagement/api_management_resource.go @@ -10,10 +10,10 @@ import ( "github.com/Azure/azure-sdk-for-go/services/apimanagement/mgmt/2019-12-01/apimanagement" "github.com/hashicorp/go-azure-helpers/response" + "github.com/hashicorp/terraform-plugin-sdk/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" @@ -145,7 +145,6 @@ func resourceArmApiManagementService() *schema.Resource { "subnet_id": { Type: schema.TypeString, Required: true, - ForceNew: true, ValidateFunc: azure.ValidateResourceID, }, }, @@ -471,6 +470,14 @@ func resourceArmApiManagementService() *schema.Resource { "tags": tags.Schema(), }, + + 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) + }), + ), } } diff --git a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go index 0ca8f72f0b85..7d72585adcc0 100644 --- a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go +++ b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go @@ -183,6 +183,39 @@ func TestAccAzureRMApiManagement_virtualNetworkInternal(t *testing.T) { }) } +func TestAccAzureRMApiManagement_virtualNetworkInternalUpdate(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_api_management", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMApiManagementDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMApiManagement_basic(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMApiManagementExists(data.ResourceName), + ), + }, + data.ImportStep(), + { + Config: testAccAzureRMApiManagement_virtualNetworkInternal(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMApiManagementExists(data.ResourceName), + ), + }, + data.ImportStep(), + { + Config: testAccAzureRMApiManagement_virtualNetworkInternalUpdate(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMApiManagementExists(data.ResourceName), + ), + }, + data.ImportStep(), + }, + }) +} + func TestAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_api_management", "test") @@ -218,7 +251,7 @@ func TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurati CheckDestroy: testCheckAzureRMApiManagementDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId(data), + Config: testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsKeyVaultId(data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMApiManagementExists(data.ResourceName), ), @@ -244,7 +277,7 @@ func TestAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurati CheckDestroy: testCheckAzureRMApiManagementDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId(data), + Config: testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsKeyVaultId(data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMApiManagementExists(data.ResourceName), ), @@ -875,88 +908,7 @@ resource "azurerm_api_management" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.Locations.Secondary, data.RandomInteger, data.Locations.Ternary, data.RandomInteger) } -func testAccAzureRMApiManagement_virtualNetworkInternal(data acceptance.TestData) string { - return fmt.Sprintf(` -provider "azurerm" { - features {} -} - -resource "azurerm_resource_group" "test" { - name = "acctestRG-%[1]d" - location = "%[2]s" -} - -resource "azurerm_virtual_network" "test" { - name = "acctestVNET-%[1]d" - location = azurerm_resource_group.test.location - resource_group_name = azurerm_resource_group.test.name - address_space = ["10.0.0.0/16"] -} - -resource "azurerm_subnet" "test" { - name = "acctestSNET-%[1]d" - resource_group_name = azurerm_resource_group.test.name - virtual_network_name = azurerm_virtual_network.test.name - address_prefixes = ["10.0.1.0/24"] -} - -resource "azurerm_network_security_group" "test" { - name = "acctest-NSG-%[1]d" - location = azurerm_resource_group.test.location - resource_group_name = azurerm_resource_group.test.name -} - -resource "azurerm_subnet_network_security_group_association" "test" { - subnet_id = azurerm_subnet.test.id - network_security_group_id = azurerm_network_security_group.test.id -} - -resource "azurerm_network_security_rule" "port_3443" { - name = "Port_3443" - priority = 100 - direction = "Inbound" - access = "Allow" - protocol = "Tcp" - source_port_range = "*" - destination_port_range = "3443" - source_address_prefix = "ApiManagement" - destination_address_prefix = "VirtualNetwork" - resource_group_name = azurerm_resource_group.test.name - network_security_group_name = azurerm_network_security_group.test.name -} - -resource "azurerm_network_security_rule" "port_443_1433" { - name = "Port_443_1433" - priority = 100 - direction = "Outbound" - access = "Allow" - protocol = "Tcp" - source_port_range = "*" - destination_port_ranges = ["443", "1433"] - source_address_prefix = "VirtualNetwork" - destination_address_prefix = "VirtualNetwork" - resource_group_name = azurerm_resource_group.test.name - network_security_group_name = azurerm_network_security_group.test.name -} - -resource "azurerm_api_management" "test" { - name = "acctestAM-%[1]d" - location = azurerm_resource_group.test.location - resource_group_name = azurerm_resource_group.test.name - publisher_name = "pub1" - publisher_email = "pub1@email.com" - - sku_name = "Developer_1" - - virtual_network_type = "Internal" - virtual_network_configuration { - subnet_id = azurerm_subnet.test.id - } -} -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger) -} - -func testAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation(data acceptance.TestData) string { +func testAccAzureRMApiManagement_virtualNetworkTemplate(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { features {} @@ -984,7 +936,7 @@ resource "azurerm_subnet" "test1" { name = "acctestSNET1-%[1]d" resource_group_name = azurerm_resource_group.test1.name virtual_network_name = azurerm_virtual_network.test1.name - address_prefix = "10.0.1.0/24" + address_prefixes = ["10.0.1.0/24"] } resource "azurerm_network_security_group" "test1" { @@ -1012,20 +964,6 @@ resource "azurerm_network_security_rule" "port_3443" { network_security_group_name = azurerm_network_security_group.test1.name } -resource "azurerm_network_security_rule" "port_443_1433" { - name = "Port_443_1433" - priority = 100 - direction = "Outbound" - access = "Allow" - protocol = "Tcp" - source_port_range = "*" - destination_port_ranges = ["443", "1433"] - source_address_prefix = "VirtualNetwork" - destination_address_prefix = "VirtualNetwork" - resource_group_name = azurerm_resource_group.test1.name - network_security_group_name = azurerm_network_security_group.test1.name -} - // subnet2 from the second location resource "azurerm_virtual_network" "test2" { name = "acctestVNET2-%[1]d" @@ -1038,7 +976,7 @@ resource "azurerm_subnet" "test2" { name = "acctestSNET2-%[1]d" resource_group_name = azurerm_resource_group.test2.name virtual_network_name = azurerm_virtual_network.test2.name - address_prefix = "10.1.1.0/24" + address_prefixes = ["10.1.1.0/24"] } resource "azurerm_network_security_group" "test2" { @@ -1065,24 +1003,60 @@ resource "azurerm_network_security_rule" "port_3443_2" { resource_group_name = azurerm_resource_group.test2.name network_security_group_name = azurerm_network_security_group.test2.name } +`, data.RandomInteger, data.Locations.Primary, data.Locations.Secondary) +} -resource "azurerm_network_security_rule" "port_443_1433_2" { - name = "Port_443_1433" - priority = 100 - direction = "Outbound" - access = "Allow" - protocol = "Tcp" - source_port_range = "*" - destination_port_ranges = ["443", "1433"] - source_address_prefix = "VirtualNetwork" - destination_address_prefix = "VirtualNetwork" - resource_group_name = azurerm_resource_group.test2.name - network_security_group_name = azurerm_network_security_group.test2.name +func testAccAzureRMApiManagement_virtualNetworkInternal(data acceptance.TestData) string { + template := testAccAzureRMApiManagement_virtualNetworkTemplate(data) + return fmt.Sprintf(` +%s + +resource "azurerm_api_management" "test" { + name = "acctestAM-%d" + location = azurerm_resource_group.test1.location + resource_group_name = azurerm_resource_group.test1.name + publisher_name = "pub1" + publisher_email = "pub1@email.com" + + sku_name = "Developer_1" + + virtual_network_type = "Internal" + virtual_network_configuration { + subnet_id = azurerm_subnet.test1.id + } +} +`, template, data.RandomInteger) +} + +func testAccAzureRMApiManagement_virtualNetworkInternalUpdate(data acceptance.TestData) string { + template := testAccAzureRMApiManagement_virtualNetworkTemplate(data) + return fmt.Sprintf(` +%s + +resource "azurerm_api_management" "test" { + name = "acctestAM-%d" + location = azurerm_resource_group.test1.location + resource_group_name = azurerm_resource_group.test1.name + publisher_name = "pub1" + publisher_email = "pub1@email.com" + + sku_name = "Developer_1" + + virtual_network_type = "Internal" + virtual_network_configuration { + subnet_id = azurerm_subnet.test2.id + } +} +`, template, data.RandomInteger) } +func testAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation(data acceptance.TestData) string { + template := testAccAzureRMApiManagement_virtualNetworkTemplate(data) + return fmt.Sprintf(` +%s resource "azurerm_api_management" "test" { - name = "acctestAM-%[1]d" + name = "acctestAM-%d" location = azurerm_resource_group.test1.location resource_group_name = azurerm_resource_group.test1.name publisher_name = "pub1" @@ -1102,7 +1076,7 @@ resource "azurerm_api_management" "test" { subnet_id = azurerm_subnet.test1.id } } -`, data.RandomInteger, data.Locations.Primary, data.Locations.Secondary) +`, template, data.RandomInteger) } func testAccAzureRMApiManagement_identityUserAssigned(data acceptance.TestData) string { @@ -1224,7 +1198,7 @@ resource "azurerm_api_management" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) } -func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultId(data acceptance.TestData) string { +func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsTemplate(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { features {} @@ -1312,8 +1286,15 @@ resource "azurerm_key_vault_certificate" "test" { } } } +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomString) +} +func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsKeyVaultId(data acceptance.TestData) string { + template := testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsTemplate(data) + return fmt.Sprintf(` +%s + resource "azurerm_api_management" "test" { - name = "acctestAM-%[3]d" + name = "acctestAM-%d" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name publisher_name = "pub1" @@ -1323,99 +1304,16 @@ resource "azurerm_api_management" "test" { type = "SystemAssigned" } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomString) +`, template, data.RandomInteger) } func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionlessKeyVaultIdUpdateCD(data acceptance.TestData) string { + template := testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsTemplate(data) return fmt.Sprintf(` -provider "azurerm" { - features {} -} -resource "azurerm_resource_group" "test" { - name = "acctestRG-%[1]d" - location = "%[2]s" -} -data "azurerm_client_config" "current" {} -resource "azurerm_key_vault" "test" { - name = "acctestKV-%[4]s" - location = azurerm_resource_group.test.location - resource_group_name = azurerm_resource_group.test.name - tenant_id = data.azurerm_client_config.current.tenant_id - sku_name = "standard" -} -resource "azurerm_key_vault_access_policy" "test" { - key_vault_id = azurerm_key_vault.test.id - tenant_id = data.azurerm_client_config.current.tenant_id - object_id = data.azurerm_client_config.current.object_id - certificate_permissions = [ - "Create", - "Delete", - "Deleteissuers", - "Get", - "Getissuers", - "Import", - "List", - "Listissuers", - "Managecontacts", - "Manageissuers", - "Setissuers", - "Update", - ] - secret_permissions = [ - "Delete", - "Get", - "List", - "Purge", - ] -} -resource "azurerm_key_vault_access_policy" "test2" { - key_vault_id = azurerm_key_vault.test.id - tenant_id = azurerm_api_management.test.identity[0].tenant_id - object_id = azurerm_api_management.test.identity[0].principal_id - secret_permissions = [ - "Get", - "List", - ] -} -resource "azurerm_key_vault_certificate" "test" { - depends_on = [azurerm_key_vault_access_policy.test] - name = "acctestKVCert-%[3]d" - key_vault_id = azurerm_key_vault.test.id - certificate_policy { - issuer_parameters { - name = "Self" - } - key_properties { - exportable = true - key_size = 2048 - key_type = "RSA" - reuse_key = true - } - secret_properties { - content_type = "application/x-pkcs12" - } - x509_certificate_properties { - # Server Authentication = 1.3.6.1.5.5.7.3.1 - # Client Authentication = 1.3.6.1.5.5.7.3.2 - extended_key_usage = ["1.3.6.1.5.5.7.3.1"] - key_usage = [ - "cRLSign", - "dataEncipherment", - "digitalSignature", - "keyAgreement", - "keyCertSign", - "keyEncipherment", - ] - subject_alternative_names { - dns_names = ["api.terraform.io"] - } - subject = "CN=api.terraform.io" - validity_in_months = 1 - } - } -} +%s + resource "azurerm_api_management" "test" { - name = "acctestAM-%[3]d" + name = "acctestAM-%d" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name publisher_name = "pub1" @@ -1433,201 +1331,16 @@ resource "azurerm_api_management" "test" { } } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomString) -} - -func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultId(data acceptance.TestData) string { - return fmt.Sprintf(` -provider "azurerm" { - features {} -} -resource "azurerm_resource_group" "test" { - name = "acctestRG-%[1]d" - location = "%[2]s" -} -data "azurerm_client_config" "current" {} -resource "azurerm_key_vault" "test" { - name = "acctestKV-%[4]s" - location = azurerm_resource_group.test.location - resource_group_name = azurerm_resource_group.test.name - tenant_id = data.azurerm_client_config.current.tenant_id - sku_name = "standard" -} -resource "azurerm_key_vault_access_policy" "test" { - key_vault_id = azurerm_key_vault.test.id - tenant_id = data.azurerm_client_config.current.tenant_id - object_id = data.azurerm_client_config.current.object_id - certificate_permissions = [ - "Create", - "Delete", - "Deleteissuers", - "Get", - "Getissuers", - "Import", - "List", - "Listissuers", - "Managecontacts", - "Manageissuers", - "Setissuers", - "Update", - ] - secret_permissions = [ - "Delete", - "Get", - "List", - "Purge", - ] -} -resource "azurerm_key_vault_access_policy" "test2" { - key_vault_id = azurerm_key_vault.test.id - tenant_id = azurerm_api_management.test.identity.0.tenant_id - object_id = azurerm_api_management.test.identity.0.principal_id - secret_permissions = [ - "Get", - "List", - ] -} -resource "azurerm_key_vault_certificate" "test" { - depends_on = [azurerm_key_vault_access_policy.test] - name = "acctestKVCert-%[3]d" - key_vault_id = azurerm_key_vault.test.id - certificate_policy { - issuer_parameters { - name = "Self" - } - key_properties { - exportable = true - key_size = 2048 - key_type = "RSA" - reuse_key = true - } - secret_properties { - content_type = "application/x-pkcs12" - } - x509_certificate_properties { - # Server Authentication = 1.3.6.1.5.5.7.3.1 - # Client Authentication = 1.3.6.1.5.5.7.3.2 - extended_key_usage = ["1.3.6.1.5.5.7.3.1"] - key_usage = [ - "cRLSign", - "dataEncipherment", - "digitalSignature", - "keyAgreement", - "keyCertSign", - "keyEncipherment", - ] - subject_alternative_names { - dns_names = ["api.terraform.io"] - } - subject = "CN=api.terraform.io" - validity_in_months = 1 - } - } -} -resource "azurerm_api_management" "test" { - name = "acctestAM-%[3]d" - location = azurerm_resource_group.test.location - resource_group_name = azurerm_resource_group.test.name - publisher_name = "pub1" - publisher_email = "pub1@email.com" - sku_name = "Developer_1" - identity { - type = "SystemAssigned" - } -} -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomString) +`, template, data.RandomInteger) } func testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsVersionedKeyVaultIdUpdateCD(data acceptance.TestData) string { + template := testAccAzureRMApiManagement_identitySystemAssignedUpdateHostnameConfigurationsTemplate(data) return fmt.Sprintf(` -provider "azurerm" { - features {} -} -resource "azurerm_resource_group" "test" { - name = "acctestRG-%[1]d" - location = "%[2]s" -} -data "azurerm_client_config" "current" {} -resource "azurerm_key_vault" "test" { - name = "acctestKV-%[4]s" - location = azurerm_resource_group.test.location - resource_group_name = azurerm_resource_group.test.name - tenant_id = data.azurerm_client_config.current.tenant_id - sku_name = "standard" -} -resource "azurerm_key_vault_access_policy" "test" { - key_vault_id = azurerm_key_vault.test.id - tenant_id = data.azurerm_client_config.current.tenant_id - object_id = data.azurerm_client_config.current.object_id - certificate_permissions = [ - "Create", - "Delete", - "Deleteissuers", - "Get", - "Getissuers", - "Import", - "List", - "Listissuers", - "Managecontacts", - "Manageissuers", - "Setissuers", - "Update", - ] - secret_permissions = [ - "Delete", - "Get", - "List", - "Purge", - ] -} -resource "azurerm_key_vault_access_policy" "test2" { - key_vault_id = azurerm_key_vault.test.id - tenant_id = azurerm_api_management.test.identity[0].tenant_id - object_id = azurerm_api_management.test.identity[0].principal_id - secret_permissions = [ - "Get", - "List", - ] -} -resource "azurerm_key_vault_certificate" "test" { - depends_on = [azurerm_key_vault_access_policy.test] - name = "acctestKVCert-%[3]d" - key_vault_id = azurerm_key_vault.test.id - certificate_policy { - issuer_parameters { - name = "Self" - } - key_properties { - exportable = true - key_size = 2048 - key_type = "RSA" - reuse_key = true - } - secret_properties { - content_type = "application/x-pkcs12" - } - x509_certificate_properties { - # Server Authentication = 1.3.6.1.5.5.7.3.1 - # Client Authentication = 1.3.6.1.5.5.7.3.2 - extended_key_usage = ["1.3.6.1.5.5.7.3.1"] - key_usage = [ - "cRLSign", - "dataEncipherment", - "digitalSignature", - "keyAgreement", - "keyCertSign", - "keyEncipherment", - ] - subject_alternative_names { - dns_names = ["api.terraform.io"] - } - subject = "CN=api.terraform.io" - validity_in_months = 1 - } - } -} +%s + resource "azurerm_api_management" "test" { - name = "acctestAM-%[3]d" + name = "acctestAM-%d" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name publisher_name = "pub1" @@ -1645,5 +1358,5 @@ resource "azurerm_api_management" "test" { } } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomString) +`, template, data.RandomInteger) } From 031d55707dbc73eca450819a3c170bf899b4a7b3 Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Tue, 17 Nov 2020 10:11:07 +0800 Subject: [PATCH 09/14] update --- .../tests/api_management_resource_test.go | 167 ++++++++++++------ 1 file changed, 112 insertions(+), 55 deletions(-) diff --git a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go index 7d72585adcc0..9a10875b5a05 100644 --- a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go +++ b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go @@ -914,6 +914,114 @@ provider "azurerm" { features {} } +resource "azurerm_resource_group" "test" { + name = "acctestRG-%[1]d" + location = "%[2]s" +} + +resource "azurerm_virtual_network" "test" { + name = "acctestVNET-%[1]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + address_space = ["10.0.0.0/16"] +} + +resource "azurerm_subnet" "test" { + name = "acctestSNET-%[1]d" + resource_group_name = azurerm_resource_group.test.name + virtual_network_name = azurerm_virtual_network.test.name + address_prefixes = ["10.0.1.0/24"] +} + +resource "azurerm_subnet" "test2" { + name = "acctestSNET2-%[1]d" + resource_group_name = azurerm_resource_group.test.name + virtual_network_name = azurerm_virtual_network.test.name + address_prefixes = ["10.0.2.0/24"] +} + +resource "azurerm_network_security_group" "test" { + name = "acctest-NSG-%[1]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name +} + +resource "azurerm_subnet_network_security_group_association" "test" { + subnet_id = azurerm_subnet.test.id + network_security_group_id = azurerm_network_security_group.test.id +} + +resource "azurerm_subnet_network_security_group_association" "test2" { + subnet_id = azurerm_subnet.test2.id + network_security_group_id = azurerm_network_security_group.test.id +} + +resource "azurerm_network_security_rule" "port_3443" { + name = "Port_3443" + priority = 100 + direction = "Inbound" + access = "Allow" + protocol = "Tcp" + source_port_range = "*" + destination_port_range = "3443" + source_address_prefix = "ApiManagement" + destination_address_prefix = "VirtualNetwork" + resource_group_name = azurerm_resource_group.test.name + network_security_group_name = azurerm_network_security_group.test.name +} +`, data.RandomInteger, data.Locations.Primary) +} + +func testAccAzureRMApiManagement_virtualNetworkInternal(data acceptance.TestData) string { + template := testAccAzureRMApiManagement_virtualNetworkTemplate(data) + return fmt.Sprintf(` +%s + +resource "azurerm_api_management" "test" { + name = "acctestAM-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + publisher_name = "pub1" + publisher_email = "pub1@email.com" + + sku_name = "Developer_1" + + virtual_network_type = "Internal" + virtual_network_configuration { + subnet_id = azurerm_subnet.test.id + } +} +`, template, data.RandomInteger) +} + +func testAccAzureRMApiManagement_virtualNetworkInternalUpdate(data acceptance.TestData) string { + template := testAccAzureRMApiManagement_virtualNetworkTemplate(data) + return fmt.Sprintf(` +%s + +resource "azurerm_api_management" "test" { + name = "acctestAM-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + publisher_name = "pub1" + publisher_email = "pub1@email.com" + + sku_name = "Developer_1" + + virtual_network_type = "Internal" + virtual_network_configuration { + subnet_id = azurerm_subnet.test2.id + } +} +`, template, data.RandomInteger) +} + +func testAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + resource "azurerm_resource_group" "test1" { name = "acctestRG1-%[1]d" location = "%[2]s" @@ -936,7 +1044,7 @@ resource "azurerm_subnet" "test1" { name = "acctestSNET1-%[1]d" resource_group_name = azurerm_resource_group.test1.name virtual_network_name = azurerm_virtual_network.test1.name - address_prefixes = ["10.0.1.0/24"] + address_prefix = "10.0.1.0/24" } resource "azurerm_network_security_group" "test1" { @@ -976,7 +1084,7 @@ resource "azurerm_subnet" "test2" { name = "acctestSNET2-%[1]d" resource_group_name = azurerm_resource_group.test2.name virtual_network_name = azurerm_virtual_network.test2.name - address_prefixes = ["10.1.1.0/24"] + address_prefix = "10.1.1.0/24" } resource "azurerm_network_security_group" "test2" { @@ -1003,60 +1111,9 @@ resource "azurerm_network_security_rule" "port_3443_2" { resource_group_name = azurerm_resource_group.test2.name network_security_group_name = azurerm_network_security_group.test2.name } -`, data.RandomInteger, data.Locations.Primary, data.Locations.Secondary) -} - -func testAccAzureRMApiManagement_virtualNetworkInternal(data acceptance.TestData) string { - template := testAccAzureRMApiManagement_virtualNetworkTemplate(data) - return fmt.Sprintf(` -%s - -resource "azurerm_api_management" "test" { - name = "acctestAM-%d" - location = azurerm_resource_group.test1.location - resource_group_name = azurerm_resource_group.test1.name - publisher_name = "pub1" - publisher_email = "pub1@email.com" - - sku_name = "Developer_1" - - virtual_network_type = "Internal" - virtual_network_configuration { - subnet_id = azurerm_subnet.test1.id - } -} -`, template, data.RandomInteger) -} - -func testAccAzureRMApiManagement_virtualNetworkInternalUpdate(data acceptance.TestData) string { - template := testAccAzureRMApiManagement_virtualNetworkTemplate(data) - return fmt.Sprintf(` -%s resource "azurerm_api_management" "test" { - name = "acctestAM-%d" - location = azurerm_resource_group.test1.location - resource_group_name = azurerm_resource_group.test1.name - publisher_name = "pub1" - publisher_email = "pub1@email.com" - - sku_name = "Developer_1" - - virtual_network_type = "Internal" - virtual_network_configuration { - subnet_id = azurerm_subnet.test2.id - } -} -`, template, data.RandomInteger) -} - -func testAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation(data acceptance.TestData) string { - template := testAccAzureRMApiManagement_virtualNetworkTemplate(data) - return fmt.Sprintf(` -%s - -resource "azurerm_api_management" "test" { - name = "acctestAM-%d" + name = "acctestAM-%[1]d" location = azurerm_resource_group.test1.location resource_group_name = azurerm_resource_group.test1.name publisher_name = "pub1" @@ -1076,7 +1133,7 @@ resource "azurerm_api_management" "test" { subnet_id = azurerm_subnet.test1.id } } -`, template, data.RandomInteger) +`, data.RandomInteger,data.Locations.Primary,data.Locations.Secondary) } func testAccAzureRMApiManagement_identityUserAssigned(data acceptance.TestData) string { From cdf3927358295aa328b06d110a0fb0647e3d9023 Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Tue, 17 Nov 2020 13:00:05 +0800 Subject: [PATCH 10/14] update --- .../apimanagement/api_management_resource.go | 3 ++ .../tests/api_management_resource_test.go | 43 +------------------ 2 files changed, 4 insertions(+), 42 deletions(-) diff --git a/azurerm/internal/services/apimanagement/api_management_resource.go b/azurerm/internal/services/apimanagement/api_management_resource.go index 5d1713de9538..293a238af3e5 100644 --- a/azurerm/internal/services/apimanagement/api_management_resource.go +++ b/azurerm/internal/services/apimanagement/api_management_resource.go @@ -477,6 +477,9 @@ func resourceArmApiManagementService() *schema.Resource { old.(string) == string(apimanagement.VirtualNetworkTypeInternal)) && new.(string) == string(apimanagement.VirtualNetworkTypeNone) }), + customdiff.ForceNewIfChange("virtual_network_configuration", func(old, new, meta interface{}) bool { + return !(len(old.([]interface{})) == 0 && len(new.([]interface{})) > 0) + }), ), } } diff --git a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go index 9a10875b5a05..ef8fb57b2df5 100644 --- a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go +++ b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go @@ -205,13 +205,6 @@ func TestAccAzureRMApiManagement_virtualNetworkInternalUpdate(t *testing.T) { ), }, data.ImportStep(), - { - Config: testAccAzureRMApiManagement_virtualNetworkInternalUpdate(data), - Check: resource.ComposeTestCheckFunc( - testCheckAzureRMApiManagementExists(data.ResourceName), - ), - }, - data.ImportStep(), }, }) } @@ -933,13 +926,6 @@ resource "azurerm_subnet" "test" { address_prefixes = ["10.0.1.0/24"] } -resource "azurerm_subnet" "test2" { - name = "acctestSNET2-%[1]d" - resource_group_name = azurerm_resource_group.test.name - virtual_network_name = azurerm_virtual_network.test.name - address_prefixes = ["10.0.2.0/24"] -} - resource "azurerm_network_security_group" "test" { name = "acctest-NSG-%[1]d" location = azurerm_resource_group.test.location @@ -951,11 +937,6 @@ resource "azurerm_subnet_network_security_group_association" "test" { network_security_group_id = azurerm_network_security_group.test.id } -resource "azurerm_subnet_network_security_group_association" "test2" { - subnet_id = azurerm_subnet.test2.id - network_security_group_id = azurerm_network_security_group.test.id -} - resource "azurerm_network_security_rule" "port_3443" { name = "Port_3443" priority = 100 @@ -994,28 +975,6 @@ resource "azurerm_api_management" "test" { `, template, data.RandomInteger) } -func testAccAzureRMApiManagement_virtualNetworkInternalUpdate(data acceptance.TestData) string { - template := testAccAzureRMApiManagement_virtualNetworkTemplate(data) - return fmt.Sprintf(` -%s - -resource "azurerm_api_management" "test" { - name = "acctestAM-%d" - location = azurerm_resource_group.test.location - resource_group_name = azurerm_resource_group.test.name - publisher_name = "pub1" - publisher_email = "pub1@email.com" - - sku_name = "Developer_1" - - virtual_network_type = "Internal" - virtual_network_configuration { - subnet_id = azurerm_subnet.test2.id - } -} -`, template, data.RandomInteger) -} - func testAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { @@ -1133,7 +1092,7 @@ resource "azurerm_api_management" "test" { subnet_id = azurerm_subnet.test1.id } } -`, data.RandomInteger,data.Locations.Primary,data.Locations.Secondary) +`, data.RandomInteger, data.Locations.Primary, data.Locations.Secondary) } func testAccAzureRMApiManagement_identityUserAssigned(data acceptance.TestData) string { From 211da893875f2e05d0838944249b6c89a8cb5bd9 Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Thu, 19 Nov 2020 09:49:48 +0800 Subject: [PATCH 11/14] update --- .../apimanagement/api_management_resource.go | 16 +---------- .../tests/api_management_resource_test.go | 27 ------------------- 2 files changed, 1 insertion(+), 42 deletions(-) diff --git a/azurerm/internal/services/apimanagement/api_management_resource.go b/azurerm/internal/services/apimanagement/api_management_resource.go index 293a238af3e5..0d9fd0dca0a6 100644 --- a/azurerm/internal/services/apimanagement/api_management_resource.go +++ b/azurerm/internal/services/apimanagement/api_management_resource.go @@ -10,7 +10,6 @@ import ( "github.com/Azure/azure-sdk-for-go/services/apimanagement/mgmt/2019-12-01/apimanagement" "github.com/hashicorp/go-azure-helpers/response" - "github.com/hashicorp/terraform-plugin-sdk/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" @@ -121,9 +120,6 @@ 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 "virtual_network_type": { Type: schema.TypeString, Optional: true, @@ -145,6 +141,7 @@ func resourceArmApiManagementService() *schema.Resource { "subnet_id": { Type: schema.TypeString, Required: true, + ForceNew: true, ValidateFunc: azure.ValidateResourceID, }, }, @@ -470,17 +467,6 @@ func resourceArmApiManagementService() *schema.Resource { "tags": tags.Schema(), }, - - 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) - }), - customdiff.ForceNewIfChange("virtual_network_configuration", func(old, new, meta interface{}) bool { - return !(len(old.([]interface{})) == 0 && len(new.([]interface{})) > 0) - }), - ), } } diff --git a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go index ef8fb57b2df5..e9af0828808d 100644 --- a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go +++ b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go @@ -183,32 +183,6 @@ func TestAccAzureRMApiManagement_virtualNetworkInternal(t *testing.T) { }) } -func TestAccAzureRMApiManagement_virtualNetworkInternalUpdate(t *testing.T) { - data := acceptance.BuildTestData(t, "azurerm_api_management", "test") - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acceptance.PreCheck(t) }, - Providers: acceptance.SupportedProviders, - CheckDestroy: testCheckAzureRMApiManagementDestroy, - Steps: []resource.TestStep{ - { - Config: testAccAzureRMApiManagement_basic(data), - Check: resource.ComposeTestCheckFunc( - testCheckAzureRMApiManagementExists(data.ResourceName), - ), - }, - data.ImportStep(), - { - Config: testAccAzureRMApiManagement_virtualNetworkInternal(data), - Check: resource.ComposeTestCheckFunc( - testCheckAzureRMApiManagementExists(data.ResourceName), - ), - }, - data.ImportStep(), - }, - }) -} - func TestAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_api_management", "test") @@ -299,7 +273,6 @@ func testCheckAzureRMApiManagementDestroy(s *terraform.State) error { name := rs.Primary.Attributes["name"] resourceGroup := rs.Primary.Attributes["resource_group_name"] resp, err := conn.Get(ctx, resourceGroup, name) - if err != nil { if utils.ResponseWasNotFound(resp.Response) { return nil From b16eea3c2e0bc1a381ada8572fd1ef063c76c481 Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Mon, 23 Nov 2020 17:11:44 +0800 Subject: [PATCH 12/14] update --- .../apimanagement/api_management_resource.go | 18 +++++++++++-- .../tests/api_management_resource_test.go | 26 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/azurerm/internal/services/apimanagement/api_management_resource.go b/azurerm/internal/services/apimanagement/api_management_resource.go index 0d9fd0dca0a6..e3305e7e53f1 100644 --- a/azurerm/internal/services/apimanagement/api_management_resource.go +++ b/azurerm/internal/services/apimanagement/api_management_resource.go @@ -10,6 +10,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/apimanagement/mgmt/2019-12-01/apimanagement" "github.com/hashicorp/go-azure-helpers/response" + "github.com/hashicorp/terraform-plugin-sdk/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" @@ -124,7 +125,6 @@ func resourceArmApiManagementService() *schema.Resource { Type: schema.TypeString, Optional: true, Default: string(apimanagement.VirtualNetworkTypeNone), - ForceNew: true, ValidateFunc: validation.StringInSlice([]string{ string(apimanagement.VirtualNetworkTypeNone), string(apimanagement.VirtualNetworkTypeExternal), @@ -141,7 +141,6 @@ func resourceArmApiManagementService() *schema.Resource { "subnet_id": { Type: schema.TypeString, Required: true, - ForceNew: true, ValidateFunc: azure.ValidateResourceID, }, }, @@ -467,6 +466,21 @@ func resourceArmApiManagementService() *schema.Resource { "tags": tags.Schema(), }, + + // we can only change `virtual_network_type` from None to Internal Or External, Else the subnet can not be destroyed cause “InUseSubnetCannotBeDeleted” for 3 hours + // we can not change the subnet from subnet1 to subnet2 either, Else the subnet1 can not be destroyed cause “InUseSubnetCannotBeDeleted” for 3 hours + // Issue: https://github.com/Azure/azure-rest-api-specs/issues/10395 + CustomizeDiff: customdiff.All( + customdiff.ForceNewIfChange("virtual_network_type", func(old, new, meta interface{}) bool { + return !(old.(string) == string(apimanagement.VirtualNetworkTypeNone) && + (new.(string) == string(apimanagement.VirtualNetworkTypeInternal) || + new.(string) == string(apimanagement.VirtualNetworkTypeExternal))) + }), + + customdiff.ForceNewIfChange("virtual_network_configuration", func(old, new, meta interface{}) bool { + return !(len(old.([]interface{})) == 0 && len(new.([]interface{})) > 0) + }), + ), } } diff --git a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go index e9af0828808d..2b2676992ccd 100644 --- a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go +++ b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go @@ -183,6 +183,32 @@ func TestAccAzureRMApiManagement_virtualNetworkInternal(t *testing.T) { }) } +func TestAccAzureRMApiManagement_virtualNetworkInternalUpdate(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_api_management", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMApiManagementDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMApiManagement_basic(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMApiManagementExists(data.ResourceName), + ), + }, + data.ImportStep(), + { + Config: testAccAzureRMApiManagement_virtualNetworkInternal(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMApiManagementExists(data.ResourceName), + ), + }, + data.ImportStep(), + }, + }) +} + func TestAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_api_management", "test") From af4522e467c337bf3bc04fa456c28a93e195b17d Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Mon, 23 Nov 2020 17:17:14 +0800 Subject: [PATCH 13/14] update --- .../tests/api_management_resource_test.go | 68 +++---------------- 1 file changed, 11 insertions(+), 57 deletions(-) diff --git a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go index 2b2676992ccd..19f92c61072e 100644 --- a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go +++ b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go @@ -975,78 +975,32 @@ resource "azurerm_api_management" "test" { } func testAccAzureRMApiManagement_virtualNetworkInternalAdditionalLocation(data acceptance.TestData) string { + template := testAccAzureRMApiManagement_virtualNetworkTemplate(data) return fmt.Sprintf(` -provider "azurerm" { - features {} -} - -resource "azurerm_resource_group" "test1" { - name = "acctestRG1-%[1]d" - location = "%[2]s" -} +%[1]s resource "azurerm_resource_group" "test2" { - name = "acctestRG2-%[1]d" + name = "acctestRG2-%[2]d" location = "%[3]s" } -// subnet1 from the first location -resource "azurerm_virtual_network" "test1" { - name = "acctestVNET1-%[1]d" - location = azurerm_resource_group.test1.location - resource_group_name = azurerm_resource_group.test1.name - address_space = ["10.0.0.0/16"] -} - -resource "azurerm_subnet" "test1" { - name = "acctestSNET1-%[1]d" - resource_group_name = azurerm_resource_group.test1.name - virtual_network_name = azurerm_virtual_network.test1.name - address_prefix = "10.0.1.0/24" -} - -resource "azurerm_network_security_group" "test1" { - name = "acctest-NSG1-%[1]d" - location = azurerm_resource_group.test1.location - resource_group_name = azurerm_resource_group.test1.name -} - -resource "azurerm_subnet_network_security_group_association" "test1" { - subnet_id = azurerm_subnet.test1.id - network_security_group_id = azurerm_network_security_group.test1.id -} - -resource "azurerm_network_security_rule" "port_3443" { - name = "Port_3443" - priority = 100 - direction = "Inbound" - access = "Allow" - protocol = "Tcp" - source_port_range = "*" - destination_port_range = "3443" - source_address_prefix = "ApiManagement" - destination_address_prefix = "VirtualNetwork" - resource_group_name = azurerm_resource_group.test1.name - network_security_group_name = azurerm_network_security_group.test1.name -} - // subnet2 from the second location resource "azurerm_virtual_network" "test2" { - name = "acctestVNET2-%[1]d" + name = "acctestVNET2-%[2]d" location = azurerm_resource_group.test2.location resource_group_name = azurerm_resource_group.test2.name address_space = ["10.1.0.0/16"] } resource "azurerm_subnet" "test2" { - name = "acctestSNET2-%[1]d" + name = "acctestSNET2-%[2]d" resource_group_name = azurerm_resource_group.test2.name virtual_network_name = azurerm_virtual_network.test2.name address_prefix = "10.1.1.0/24" } resource "azurerm_network_security_group" "test2" { - name = "acctest-NSG2-%[1]d" + name = "acctest-NSG2-%[2]d" location = azurerm_resource_group.test2.location resource_group_name = azurerm_resource_group.test2.name } @@ -1071,9 +1025,9 @@ resource "azurerm_network_security_rule" "port_3443_2" { } resource "azurerm_api_management" "test" { - name = "acctestAM-%[1]d" - location = azurerm_resource_group.test1.location - resource_group_name = azurerm_resource_group.test1.name + name = "acctestAM-%[2]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name publisher_name = "pub1" publisher_email = "pub1@email.com" @@ -1088,10 +1042,10 @@ resource "azurerm_api_management" "test" { virtual_network_type = "Internal" virtual_network_configuration { - subnet_id = azurerm_subnet.test1.id + subnet_id = azurerm_subnet.test.id } } -`, data.RandomInteger, data.Locations.Primary, data.Locations.Secondary) +`, template,data.RandomInteger, data.Locations.Secondary) } func testAccAzureRMApiManagement_identityUserAssigned(data acceptance.TestData) string { From 9aaf7c8acb3b2506eca399abd019834474a11ff9 Mon Sep 17 00:00:00 2001 From: yupwei68 Date: Mon, 23 Nov 2020 17:22:28 +0800 Subject: [PATCH 14/14] fmt --- .../apimanagement/tests/api_management_resource_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go index 19f92c61072e..a40a8a81a618 100644 --- a/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go +++ b/azurerm/internal/services/apimanagement/tests/api_management_resource_test.go @@ -1045,7 +1045,7 @@ resource "azurerm_api_management" "test" { subnet_id = azurerm_subnet.test.id } } -`, template,data.RandomInteger, data.Locations.Secondary) +`, template, data.RandomInteger, data.Locations.Secondary) } func testAccAzureRMApiManagement_identityUserAssigned(data acceptance.TestData) string {