From bc31169503834e6334c6fea9afa69f93ca2b74bf Mon Sep 17 00:00:00 2001 From: kt Date: Thu, 13 Sep 2018 15:52:37 -0700 Subject: [PATCH 1/3] fixs public IP import bug wrt idle timeout & some minor refactoring --- azurestack/resource_arm_public_ip.go | 84 ++++++++----------- azurestack/resource_arm_public_ip_test.go | 36 -------- azurestack/resource_arm_route_table.go | 4 +- .../resource_arm_virtual_network_gateway.go | 4 +- ..._arm_virtual_network_gateway_connection.go | 4 +- 5 files changed, 41 insertions(+), 91 deletions(-) diff --git a/azurestack/resource_arm_public_ip.go b/azurestack/resource_arm_public_ip.go index 6db3b6003..884a010c1 100644 --- a/azurestack/resource_arm_public_ip.go +++ b/azurestack/resource_arm_public_ip.go @@ -8,6 +8,8 @@ import ( "github.com/Azure/azure-sdk-for-go/profiles/2017-03-09/network/mgmt/network" "github.com/hashicorp/terraform/helper/schema" + "github.com/hashicorp/terraform/helper/validation" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" "github.com/terraform-providers/terraform-provider-azurestack/azurestack/utils" ) @@ -17,6 +19,7 @@ func resourceArmPublicIp() *schema.Resource { Read: resourceArmPublicIpRead, Update: resourceArmPublicIpCreate, Delete: resourceArmPublicIpDelete, + Importer: &schema.ResourceImporter{ State: func(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { id, err := parseAzureResourceID(d.Id()) @@ -33,9 +36,10 @@ func resourceArmPublicIp() *schema.Resource { Schema: map[string]*schema.Schema{ "name": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.NoZeroValues, }, "location": locationSchema(), @@ -48,22 +52,18 @@ func resourceArmPublicIp() *schema.Resource { "public_ip_address_allocation": { Type: schema.TypeString, Required: true, - ValidateFunc: validatePublicIpAllocation, StateFunc: ignoreCaseStateFunc, - DiffSuppressFunc: ignoreCaseDiffSuppressFunc, + DiffSuppressFunc: suppress.CaseDifference, + ValidateFunc: validation.StringInSlice([]string{ + string(network.Dynamic), + string(network.Static), + }, true), }, "idle_timeout_in_minutes": { - Type: schema.TypeInt, - Optional: true, - ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) { - value := v.(int) - if value < 4 || value > 30 { - errors = append(errors, fmt.Errorf( - "The idle timeout must be between 4 and 30 minutes")) - } - return - }, + Type: schema.TypeInt, + Optional: true, + ValidateFunc: validation.IntBetween(4, 30), }, "domain_name_label": { @@ -123,8 +123,6 @@ func resourceArmPublicIpCreate(d *schema.ResourceData, meta interface{}) error { // Not supported for 2017-03-09 profile // zones := expandZones(d.Get("zones").([]interface{})) - ipAllocationMethod := network.IPAllocationMethod(d.Get("public_ip_address_allocation").(string)) - // Not supported for 2017-03-09 profile // if strings.ToLower(string(sku.Name)) == "standard" { // if strings.ToLower(string(ipAllocationMethod)) != "static" { @@ -132,8 +130,18 @@ func resourceArmPublicIpCreate(d *schema.ResourceData, meta interface{}) error { // } // } - properties := network.PublicIPAddressPropertiesFormat{ - PublicIPAllocationMethod: ipAllocationMethod, + ipAllocationMethod := d.Get("public_ip_address_allocation").(string) + + publicIp := network.PublicIPAddress{ + Name: &name, + Location: &location, + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAllocationMethod: network.IPAllocationMethod(ipAllocationMethod), + }, + Tags: *expandTags(tags), + // Not supported for 2017-03-09 profile + // Sku: &sku, + // Zones: zones, } dnl, hasDnl := d.GetOk("domain_name_label") @@ -152,22 +160,11 @@ func resourceArmPublicIpCreate(d *schema.ResourceData, meta interface{}) error { dnsSettings.DomainNameLabel = &domainNameLabel } - properties.DNSSettings = &dnsSettings + publicIp.PublicIPAddressPropertiesFormat.DNSSettings = &dnsSettings } if v, ok := d.GetOk("idle_timeout_in_minutes"); ok { - idleTimeout := int32(v.(int)) - properties.IdleTimeoutInMinutes = &idleTimeout - } - - publicIp := network.PublicIPAddress{ - Name: &name, - Location: &location, - PublicIPAddressPropertiesFormat: &properties, - Tags: *expandTags(tags), - // Not supported for 2017-03-09 profile - // Sku: &sku, - // Zones: zones, + publicIp.PublicIPAddressPropertiesFormat.IdleTimeoutInMinutes = utils.Int32(int32(v.(int))) } future, err := client.CreateOrUpdate(ctx, resGroup, name, publicIp) @@ -175,8 +172,7 @@ func resourceArmPublicIpCreate(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("Error Creating/Updating Public IP %q (Resource Group %q): %+v", name, resGroup, err) } - err = future.WaitForCompletionRef(ctx, client.Client) - if err != nil { + if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { return fmt.Errorf("Error waiting for completion of Public IP %q (Resource Group %q): %+v", name, resGroup, err) } @@ -239,6 +235,10 @@ func resourceArmPublicIpRead(d *schema.ResourceData, meta interface{}) error { if ip := props.IPAddress; ip != nil { d.Set("ip_address", ip) } + + if idleTimeout := props.IdleTimeoutInMinutes; idleTimeout != nil { + d.Set("idle_timeout_in_minutes", idleTimeout) + } } flattenAndSetTags(d, &resp.Tags) @@ -262,27 +262,13 @@ func resourceArmPublicIpDelete(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("Error deleting Public IP %q (Resource Group %q): %+v", name, resGroup, err) } - err = future.WaitForCompletionRef(ctx, client.Client) - if err != nil { + if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { return fmt.Errorf("Error waiting for deletion of Public IP %q (Resource Group %q): %+v", name, resGroup, err) } return nil } -func validatePublicIpAllocation(v interface{}, k string) (ws []string, errors []error) { - value := strings.ToLower(v.(string)) - allocations := map[string]bool{ - "static": true, - "dynamic": true, - } - - if !allocations[value] { - errors = append(errors, fmt.Errorf("Public IP Allocation must be an accepted value: Static, Dynamic")) - } - return -} - func validatePublicIpDomainNameLabel(v interface{}, k string) (ws []string, errors []error) { value := v.(string) if !regexp.MustCompile(`^[a-z0-9-]+$`).MatchString(value) { diff --git a/azurestack/resource_arm_public_ip_test.go b/azurestack/resource_arm_public_ip_test.go index 8025daf7a..ddc2abd92 100644 --- a/azurestack/resource_arm_public_ip_test.go +++ b/azurestack/resource_arm_public_ip_test.go @@ -12,42 +12,6 @@ import ( "regexp" ) -func TestResourceAzureStackPublicIpAllocation_validation(t *testing.T) { - cases := []struct { - Value string - ErrCount int - }{ - { - Value: "Random", - ErrCount: 1, - }, - { - Value: "Static", - ErrCount: 0, - }, - { - Value: "Dynamic", - ErrCount: 0, - }, - { - Value: "STATIC", - ErrCount: 0, - }, - { - Value: "static", - ErrCount: 0, - }, - } - - for _, tc := range cases { - _, errors := validatePublicIpAllocation(tc.Value, "azurestack_public_ip") - - if len(errors) != tc.ErrCount { - t.Fatalf("Expected the Azure RM Public IP allocation to trigger a validation error") - } - } -} - func TestResourceAzureStackPublicIpDomainNameLabel_validation(t *testing.T) { cases := []struct { Value string diff --git a/azurestack/resource_arm_route_table.go b/azurestack/resource_arm_route_table.go index 7d553841a..cbccab1a1 100644 --- a/azurestack/resource_arm_route_table.go +++ b/azurestack/resource_arm_route_table.go @@ -116,7 +116,7 @@ func resourceArmRouteTableCreateUpdate(d *schema.ResourceData, meta interface{}) return fmt.Errorf("Error Creating/Updating Route Table %q (Resource Group %q): %+v", name, resGroup, err) } - if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { + if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { return fmt.Errorf("Error waiting for completion of Route Table %q (Resource Group %q): %+v", name, resGroup, err) } @@ -192,7 +192,7 @@ func resourceArmRouteTableDelete(d *schema.ResourceData, meta interface{}) error } } - if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { + if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { return fmt.Errorf("Error waiting for deletion of Route Table %q (Resource Group %q): %+v", name, resGroup, err) } diff --git a/azurestack/resource_arm_virtual_network_gateway.go b/azurestack/resource_arm_virtual_network_gateway.go index 9eb092193..2b949ed45 100644 --- a/azurestack/resource_arm_virtual_network_gateway.go +++ b/azurestack/resource_arm_virtual_network_gateway.go @@ -298,7 +298,7 @@ func resourceArmVirtualNetworkGatewayCreateUpdate(d *schema.ResourceData, meta i return fmt.Errorf("Error Creating/Updating AzureRM Virtual Network Gateway %q (Resource Group %q): %+v", name, resGroup, err) } - if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { + if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { return fmt.Errorf("Error waiting for completion of AzureRM Virtual Network Gateway %q (Resource Group %q): %+v", name, resGroup, err) } @@ -393,7 +393,7 @@ func resourceArmVirtualNetworkGatewayDelete(d *schema.ResourceData, meta interfa return fmt.Errorf("Error deleting Virtual Network Gateway %q (Resource Group %q): %+v", name, resGroup, err) } - if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { + if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { return fmt.Errorf("Error waiting for deletion of Virtual Network Gateway %q (Resource Group %q): %+v", name, resGroup, err) } diff --git a/azurestack/resource_arm_virtual_network_gateway_connection.go b/azurestack/resource_arm_virtual_network_gateway_connection.go index 117d31a23..710bea372 100644 --- a/azurestack/resource_arm_virtual_network_gateway_connection.go +++ b/azurestack/resource_arm_virtual_network_gateway_connection.go @@ -132,7 +132,7 @@ func resourceArmVirtualNetworkGatewayConnectionCreateUpdate(d *schema.ResourceDa return fmt.Errorf("Error Creating/Updating AzureRM Virtual Network Gateway Connection %q (Resource Group %q): %+v", name, resGroup, err) } - if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { + if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { return fmt.Errorf("Error waiting for completion of Virtual Network Gateway Connection %q (Resource Group %q): %+v", name, resGroup, err) } @@ -240,7 +240,7 @@ func resourceArmVirtualNetworkGatewayConnectionDelete(d *schema.ResourceData, me return fmt.Errorf("Error Deleting Virtual Network Gateway Connection %q (Resource Group %q): %+v", name, resGroup, err) } - if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { + if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { return fmt.Errorf("Error waiting for deletion of Virtual Network Gateway Connection %q (Resource Group %q): %+v", name, resGroup, err) } From 41f0e1c4681a36e88408dadd76551cd28a8d52e2 Mon Sep 17 00:00:00 2001 From: kt Date: Thu, 13 Sep 2018 16:29:18 -0700 Subject: [PATCH 2/3] updates public ip idle timeout default value to 4 --- azurestack/resource_arm_public_ip.go | 7 +++---- azurestack/resource_arm_public_ip_test.go | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/azurestack/resource_arm_public_ip.go b/azurestack/resource_arm_public_ip.go index 884a010c1..bf505517e 100644 --- a/azurestack/resource_arm_public_ip.go +++ b/azurestack/resource_arm_public_ip.go @@ -63,6 +63,7 @@ func resourceArmPublicIp() *schema.Resource { "idle_timeout_in_minutes": { Type: schema.TypeInt, Optional: true, + Default: 4, ValidateFunc: validation.IntBetween(4, 30), }, @@ -131,12 +132,14 @@ func resourceArmPublicIpCreate(d *schema.ResourceData, meta interface{}) error { // } ipAllocationMethod := d.Get("public_ip_address_allocation").(string) + idleTimeout := d.Get("idle_timeout_in_minutes").(int) publicIp := network.PublicIPAddress{ Name: &name, Location: &location, PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ PublicIPAllocationMethod: network.IPAllocationMethod(ipAllocationMethod), + IdleTimeoutInMinutes: utils.Int32(int32(idleTimeout)), }, Tags: *expandTags(tags), // Not supported for 2017-03-09 profile @@ -163,10 +166,6 @@ func resourceArmPublicIpCreate(d *schema.ResourceData, meta interface{}) error { publicIp.PublicIPAddressPropertiesFormat.DNSSettings = &dnsSettings } - if v, ok := d.GetOk("idle_timeout_in_minutes"); ok { - publicIp.PublicIPAddressPropertiesFormat.IdleTimeoutInMinutes = utils.Int32(int32(v.(int))) - } - future, err := client.CreateOrUpdate(ctx, resGroup, name, publicIp) if err != nil { return fmt.Errorf("Error Creating/Updating Public IP %q (Resource Group %q): %+v", name, resGroup, err) diff --git a/azurestack/resource_arm_public_ip_test.go b/azurestack/resource_arm_public_ip_test.go index ddc2abd92..6288bbd00 100644 --- a/azurestack/resource_arm_public_ip_test.go +++ b/azurestack/resource_arm_public_ip_test.go @@ -3,13 +3,13 @@ package azurestack import ( "fmt" "net/http" + "os" + "regexp" "testing" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" - "os" - "regexp" ) func TestResourceAzureStackPublicIpDomainNameLabel_validation(t *testing.T) { From ac030b24809d0a7d56eabb393938e0ddb66afa87 Mon Sep 17 00:00:00 2001 From: kt Date: Thu, 13 Sep 2018 22:51:06 -0700 Subject: [PATCH 3/3] simplify d.Set()s --- azurestack/resource_arm_public_ip.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/azurestack/resource_arm_public_ip.go b/azurestack/resource_arm_public_ip.go index bf505517e..52b0fde14 100644 --- a/azurestack/resource_arm_public_ip.go +++ b/azurestack/resource_arm_public_ip.go @@ -226,18 +226,11 @@ func resourceArmPublicIpRead(d *schema.ResourceData, meta interface{}) error { d.Set("public_ip_address_allocation", strings.ToLower(string(props.PublicIPAllocationMethod))) if settings := props.DNSSettings; settings != nil { - if fqdn := settings.Fqdn; fqdn != nil { - d.Set("fqdn", fqdn) - } + d.Set("fqdn", settings.Fqdn) } - if ip := props.IPAddress; ip != nil { - d.Set("ip_address", ip) - } - - if idleTimeout := props.IdleTimeoutInMinutes; idleTimeout != nil { - d.Set("idle_timeout_in_minutes", idleTimeout) - } + d.Set("ip_address", props.IPAddress) + d.Set("idle_timeout_in_minutes", props.IdleTimeoutInMinutes) } flattenAndSetTags(d, &resp.Tags)