From 8f3194020c5658dd9ad3e562361bc64fc408df8d Mon Sep 17 00:00:00 2001 From: John Hixson Date: Wed, 4 Mar 2020 14:18:56 -0800 Subject: [PATCH 1/2] virtualnetwork: allow address_prefix to be null The azure subnet resource allows for multiple address prefixes using the address_prefixes argument. The virtual network resource allows inline subnets, but does not currently support this argument. When the address_prefixes argument is used, address_prefix will be null when terraform refreshes its state causing terraform to panic. This code updates the virtual network resource to allow address_prefixes to be used and for address_prefix to be null. Tests have been added to test the multiple prefixes. The magic number in the new tests is the same magic number used in the other VNet tests. I'm not 100% clear on this number, but it appears to be a known ID. It's origin is documented (not very well) here: https://github.com/terraform-providers/terraform-provider-azurerm/pull/1913 This fix addresses the following bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1805251 https://bugzilla.redhat.com/show_bug.cgi?id=1808973 https://bugzilla.redhat.com/show_bug.cgi?id=1805936 https://bugzilla.redhat.com/show_bug.cgi?id=1808969 --- .../network/resource_arm_virtual_network.go | 64 +++++++++++++++++-- .../tests/data_source_virtual_network_test.go | 50 +++++++++++++++ .../resource_arm_virtual_network_test.go | 42 ++++++++++++ 3 files changed, 149 insertions(+), 7 deletions(-) diff --git a/azurerm/internal/services/network/resource_arm_virtual_network.go b/azurerm/internal/services/network/resource_arm_virtual_network.go index f90e79f1b1a8..64697aa3c58c 100644 --- a/azurerm/internal/services/network/resource_arm_virtual_network.go +++ b/azurerm/internal/services/network/resource_arm_virtual_network.go @@ -106,9 +106,15 @@ func resourceArmVirtualNetwork() *schema.Resource { }, "address_prefix": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validate.NoEmptyStrings, + Type: schema.TypeString, + Optional: true, + Deprecated: "Use the `address_prefixes` property instead.", + }, + + "address_prefixes": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, }, "security_group": { @@ -307,7 +313,36 @@ func expandVirtualNetworkProperties(ctx context.Context, d *schema.ResourceData, } log.Printf("[INFO] Completed GET of Subnet props ") - prefix := subnet["address_prefix"].(string) + var prefixSet int = 0 + properties := network.SubnetPropertiesFormat{} + if value, ok := subnet["address_prefixes"]; ok { + var addressPrefixes []string + for _, item := range value.([]interface{}) { + addressPrefixes = append(addressPrefixes, item.(string)) + } + properties.AddressPrefixes = &addressPrefixes + if len(addressPrefixes) > 0 { + prefixSet += 1 + } + } + if value, ok := subnet["address_prefix"]; ok { + addressPrefix := value.(string) + properties.AddressPrefix = &addressPrefix + if len(addressPrefix) > 0 { + prefixSet += 1 + } + } + if properties.AddressPrefixes != nil && len(*properties.AddressPrefixes) == 1 { + properties.AddressPrefix = &(*properties.AddressPrefixes)[0] + properties.AddressPrefixes = nil + } + if prefixSet == 0 { + return nil, fmt.Errorf("[ERROR] either address_prefix or address_prefixes is required") + } + if prefixSet == 2 { + return nil, fmt.Errorf("[ERROR] either address_prefix or address_prefixes must be set, not both") + } + secGroup := subnet["security_group"].(string) //set the props from config and leave the rest intact @@ -316,7 +351,8 @@ func expandVirtualNetworkProperties(ctx context.Context, d *schema.ResourceData, subnetObj.SubnetPropertiesFormat = &network.SubnetPropertiesFormat{} } - subnetObj.SubnetPropertiesFormat.AddressPrefix = &prefix + subnetObj.SubnetPropertiesFormat.AddressPrefixes = properties.AddressPrefixes + subnetObj.SubnetPropertiesFormat.AddressPrefix = properties.AddressPrefix if secGroup != "" { subnetObj.SubnetPropertiesFormat.NetworkSecurityGroup = &network.SecurityGroup{ @@ -399,6 +435,14 @@ func flattenVirtualNetworkSubnets(input *[]network.Subnet) *schema.Set { } if props := subnet.SubnetPropertiesFormat; props != nil { + if prefixes := props.AddressPrefixes; prefixes != nil { + var addressPrefixes []interface{} + for _, prefix := range *prefixes { + addressPrefixes = append(addressPrefixes, prefix) + } + output["address_prefixes"] = addressPrefixes + } + if prefix := props.AddressPrefix; prefix != nil { output["address_prefix"] = *prefix } @@ -434,8 +478,14 @@ func resourceAzureSubnetHash(v interface{}) int { if m, ok := v.(map[string]interface{}); ok { buf.WriteString(m["name"].(string)) - buf.WriteString(m["address_prefix"].(string)) - + if v, ok := m["address_prefixes"]; ok { + for _, a := range v.([]interface{}) { + buf.WriteString(a.(string)) + } + } + if v, ok := m["address_prefix"]; ok { + buf.WriteString(v.(string)) + } if v, ok := m["security_group"]; ok { buf.WriteString(v.(string)) } diff --git a/azurerm/internal/services/network/tests/data_source_virtual_network_test.go b/azurerm/internal/services/network/tests/data_source_virtual_network_test.go index a4f9ee316784..04c22a9b7f01 100644 --- a/azurerm/internal/services/network/tests/data_source_virtual_network_test.go +++ b/azurerm/internal/services/network/tests/data_source_virtual_network_test.go @@ -32,6 +32,29 @@ func TestAccDataSourceArmVirtualNetwork_basic(t *testing.T) { }) } +func TestAccDataSourceArmVirtualNetwork_basic_addressPrefixes(t *testing.T) { + data := acceptance.BuildTestData(t, "data.azurerm_virtual_network", "test") + + name := fmt.Sprintf("acctestvnet-%d", data.RandomInteger) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + Steps: []resource.TestStep{ + { + Config: testAccDataSourceArmVirtualNetwork_basic_addressPrefixes(data), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(data.ResourceName, "name", name), + resource.TestCheckResourceAttr(data.ResourceName, "location", azure.NormalizeLocation(data.Locations.Primary)), + resource.TestCheckResourceAttr(data.ResourceName, "dns_servers.0", "10.0.0.4"), + resource.TestCheckResourceAttr(data.ResourceName, "address_spaces.0", "10.0.0.0/16"), + resource.TestCheckResourceAttr(data.ResourceName, "subnets.0", "subnet1"), + ), + }, + }, + }) +} + func TestAccDataSourceArmVirtualNetwork_peering(t *testing.T) { data := acceptance.BuildTestData(t, "data.azurerm_virtual_network", "test") @@ -83,6 +106,33 @@ data "azurerm_virtual_network" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) } +func testAccDataSourceArmVirtualNetwork_basic_addressPrefixes(data acceptance.TestData) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctest%d-rg" + location = "%s" +} + +resource "azurerm_virtual_network" "test" { + name = "acctestvnet-%d" + address_space = ["10.0.0.0/16"] + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + dns_servers = ["10.0.0.4"] + + subnet { + name = "subnet1" + address_prefixes = ["10.0.1.0/24", "10.0.2.0/24"] + } +} + +data "azurerm_virtual_network" "test" { + resource_group_name = "${azurerm_resource_group.test.name}" + name = "${azurerm_virtual_network.test.name}" +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +} + func testAccDataSourceArmVirtualNetwork_peering(data acceptance.TestData) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { diff --git a/azurerm/internal/services/network/tests/resource_arm_virtual_network_test.go b/azurerm/internal/services/network/tests/resource_arm_virtual_network_test.go index 353cda16a364..e8b055431a58 100644 --- a/azurerm/internal/services/network/tests/resource_arm_virtual_network_test.go +++ b/azurerm/internal/services/network/tests/resource_arm_virtual_network_test.go @@ -33,6 +33,27 @@ func TestAccAzureRMVirtualNetwork_basic(t *testing.T) { }) } +func TestAccAzureRMVirtualNetwork_basic_addressPrefixes(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_virtual_network", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMVirtualNetworkDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMVirtualNetwork_basic_addressPrefixes(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMVirtualNetworkExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "subnet.#", "1"), + resource.TestCheckResourceAttrSet(data.ResourceName, "subnet.1472110187.id"), + ), + }, + data.ImportStep(), + }, + }) +} + func TestAccAzureRMVirtualNetwork_basicUpdated(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_virtual_network", "test") @@ -289,6 +310,27 @@ resource "azurerm_virtual_network" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) } +func testAccAzureRMVirtualNetwork_basic_addressPrefixes(data acceptance.TestData) 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}" + + subnet { + name = "subnet1" + address_prefixes = ["10.0.1.0/24", "10.0.2.0/24"] + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +} + func testAccAzureRMVirtualNetwork_basicUpdated(data acceptance.TestData) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { From aacd63a314578fd662ba6feff296a1c1cec18d56 Mon Sep 17 00:00:00 2001 From: John Hixson Date: Wed, 4 Mar 2020 14:20:47 -0800 Subject: [PATCH 2/2] loadbalancer: don't assume IPv4 IP address on load balancer The azure load balancer resource's frontend_ip_configuration argument assumes the IP address is only going to be IPv4. This removes that restriction to be in line with the rest of the terraform code that does not have that validation. https://bugzilla.redhat.com/show_bug.cgi?id=1808973 --- .../internal/services/network/resource_arm_loadbalancer.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/azurerm/internal/services/network/resource_arm_loadbalancer.go b/azurerm/internal/services/network/resource_arm_loadbalancer.go index f6ddeb6fd41f..a144565418a0 100644 --- a/azurerm/internal/services/network/resource_arm_loadbalancer.go +++ b/azurerm/internal/services/network/resource_arm_loadbalancer.go @@ -81,10 +81,9 @@ func resourceArmLoadBalancer() *schema.Resource { }, "private_ip_address": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validate.IPv4AddressOrEmpty, + Type: schema.TypeString, + Optional: true, + Computed: true, }, "private_ip_address_version": {