From fb0220dbddf7c2f278754da225dcb0ce6a7e6c61 Mon Sep 17 00:00:00 2001 From: Junyi Yi Date: Tue, 22 Jan 2019 15:36:36 -0800 Subject: [PATCH 1/2] Add test cases to repro bug #1619 --- azurerm/resource_arm_subnet_test.go | 99 +++++++++++++++++++++++------ 1 file changed, 79 insertions(+), 20 deletions(-) diff --git a/azurerm/resource_arm_subnet_test.go b/azurerm/resource_arm_subnet_test.go index 6848dff78978..1ab37638f750 100644 --- a/azurerm/resource_arm_subnet_test.go +++ b/azurerm/resource_arm_subnet_test.go @@ -221,6 +221,57 @@ func TestAccAzureRMSubnet_disappears(t *testing.T) { }) } +func TestAccAzureRMSubnet_serviceEndpoints(t *testing.T) { + resourceName := "azurerm_subnet.test" + ri := tf.AccRandTimeInt() + config := testAccAzureRMSubnet_serviceEndpoints(ri, testLocation()) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMSubnetDestroy, + Steps: []resource.TestStep{ + { + Config: config, + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMSubnetExists("azurerm_subnet.test"), + resource.TestCheckResourceAttr(resourceName, "service_endpoints.#", "2"), + ), + }, + }, + }) +} + +func TestAccAzureRMSubnet_serviceEndpointsVNetUpdate(t *testing.T) { + resourceName := "azurerm_subnet.test" + ri := tf.AccRandTimeInt() + location := testLocation() + config := testAccAzureRMSubnet_serviceEndpoints(ri, location) + updatedConfig := testAccAzureRMSubnet_serviceEndpointsVNetUpdate(ri, location) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMSubnetDestroy, + Steps: []resource.TestStep{ + { + Config: config, + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMSubnetExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "service_endpoints.#", "2"), + ), + }, + { + Config: updatedConfig, + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMSubnetExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "service_endpoints.#", "2"), + ), + }, + }, + }) +} + func testCheckAzureRMSubnetExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { // Ensure we have enough information in state to look up in API @@ -362,26 +413,6 @@ func testCheckAzureRMSubnetDestroy(s *terraform.State) error { return nil } -func TestAccAzureRMSubnet_serviceEndpoints(t *testing.T) { - - ri := tf.AccRandTimeInt() - config := testAccAzureRMSubnet_serviceEndpoints(ri, testLocation()) - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testCheckAzureRMSubnetDestroy, - Steps: []resource.TestStep{ - { - Config: config, - Check: resource.ComposeTestCheckFunc( - testCheckAzureRMSubnetExists("azurerm_subnet.test"), - ), - }, - }, - }) -} - func testAccAzureRMSubnet_basic(rInt int, location string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { @@ -779,3 +810,31 @@ resource "azurerm_subnet" "test" { } `, rInt, location, rInt, rInt) } + +func testAccAzureRMSubnet_serviceEndpointsVNetUpdate(rInt int, location string) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_virtual_network" "test" { + name = "acctestvirtnet%d" + address_space = ["10.0.0.0/16"] + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + + tags { + Environment = "Staging" + } +} + +resource "azurerm_subnet" "test" { + name = "acctestsubnet%d" + resource_group_name = "${azurerm_resource_group.test.name}" + virtual_network_name = "${azurerm_virtual_network.test.name}" + address_prefix = "10.0.2.0/24" + service_endpoints = ["Microsoft.Sql", "Microsoft.Storage"] +} +`, rInt, location, rInt, rInt) +} From dadad41b556eace1c6e8141a2e0ce845a04523a2 Mon Sep 17 00:00:00 2001 From: Junyi Yi Date: Tue, 22 Jan 2019 15:42:38 -0800 Subject: [PATCH 2/2] Make sure all properties of subnet are covered during VNet update --- azurerm/resource_arm_virtual_network.go | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/azurerm/resource_arm_virtual_network.go b/azurerm/resource_arm_virtual_network.go index 1dde17e85b65..c926127da147 100644 --- a/azurerm/resource_arm_virtual_network.go +++ b/azurerm/resource_arm_virtual_network.go @@ -412,36 +412,19 @@ func resourceAzureSubnetHash(v interface{}) int { } func getExistingSubnet(ctx context.Context, resGroup string, vnetName string, subnetName string, meta interface{}) (*network.Subnet, error) { - //attempt to retrieve existing subnet from the server - existingSubnet := network.Subnet{} subnetClient := meta.(*ArmClient).subnetClient resp, err := subnetClient.Get(ctx, resGroup, vnetName, subnetName, "") if err != nil { if resp.StatusCode == http.StatusNotFound { - return &existingSubnet, nil + return &network.Subnet{}, nil } //raise an error if there was an issue other than 404 in getting subnet properties return nil, err } - existingSubnet.SubnetPropertiesFormat = &network.SubnetPropertiesFormat{ - AddressPrefix: resp.SubnetPropertiesFormat.AddressPrefix, - } - - if resp.SubnetPropertiesFormat.NetworkSecurityGroup != nil { - existingSubnet.SubnetPropertiesFormat.NetworkSecurityGroup = resp.SubnetPropertiesFormat.NetworkSecurityGroup - } - - if resp.SubnetPropertiesFormat.RouteTable != nil { - existingSubnet.SubnetPropertiesFormat.RouteTable = resp.SubnetPropertiesFormat.RouteTable - } - - if resp.SubnetPropertiesFormat.IPConfigurations != nil { - existingSubnet.SubnetPropertiesFormat.IPConfigurations = resp.SubnetPropertiesFormat.IPConfigurations - } - - return &existingSubnet, nil + // Return it directly rather than copy the fields to prevent potential uncovered properties (for example, `ServiceEndpoints` mentioned in #1619) + return &resp, nil } func expandAzureRmVirtualNetworkVirtualNetworkSecurityGroupNames(d *schema.ResourceData) ([]string, error) {