Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurestack_public_ip: correct import & read idle_timeout_in_minutes #1925

Merged
merged 2 commits into from
Sep 14, 2018
Merged

azurestack_public_ip: correct import & read idle_timeout_in_minutes #1925

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
98699ef
fixs public IP import bug wrt idle timeout & some minor refactoring
katbyte Sep 14, 2018
6d60bcd
updates requested by PR
katbyte Sep 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 0 additions & 105 deletions azurerm/import_arm_public_ip_test.go
View file
Open in desktop

This file was deleted.

67 changes: 34 additions & 33 deletions azurerm/resource_arm_public_ip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package azurerm

import (
"fmt"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress"
"log"
"regexp"
"strings"
Expand Down Expand Up @@ -35,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(),
Expand All @@ -46,33 +48,34 @@ func resourceArmPublicIp() *schema.Resource {

"zones": singleZonesSchema(),

//should this perhaps be allocation_method?
//should this perhaps be allocation_method? (yes i think so)
"public_ip_address_allocation": {
Type: schema.TypeString,
Required: true,
Type: schema.TypeString,
Required: true,
DiffSuppressFunc: suppress.CaseDifference,
StateFunc: ignoreCaseStateFunc,
ValidateFunc: validation.StringInSlice([]string{
string(network.Static),
string(network.Dynamic),
}, true),
DiffSuppressFunc: ignoreCaseDiffSuppressFunc,
StateFunc: ignoreCaseStateFunc,
},

"sku": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Default: string(network.PublicIPAddressSkuNameBasic),
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Default: string(network.PublicIPAddressSkuNameBasic),
DiffSuppressFunc: suppress.CaseDifference,
ValidateFunc: validation.StringInSlice([]string{
string(network.PublicIPAddressSkuNameBasic),
string(network.PublicIPAddressSkuNameStandard),
}, true),
DiffSuppressFunc: ignoreCaseDiffSuppressFunc,
},

"idle_timeout_in_minutes": {
Type: schema.TypeInt,
Optional: true,
Default: 4,
ValidateFunc: validation.IntBetween(4, 30),
},

Expand Down Expand Up @@ -117,6 +120,7 @@ func resourceArmPublicIpCreate(d *schema.ResourceData, meta interface{}) error {
tags := d.Get("tags").(map[string]interface{})
zones := expandZones(d.Get("zones").([]interface{}))

idleTimeout := d.Get("idle_timeout_in_minutes").(int)
ipAllocationMethod := network.IPAllocationMethod(d.Get("public_ip_address_allocation").(string))

if strings.ToLower(string(sku.Name)) == "standard" {
Expand All @@ -125,8 +129,16 @@ func resourceArmPublicIpCreate(d *schema.ResourceData, meta interface{}) error {
}
}

properties := network.PublicIPAddressPropertiesFormat{
PublicIPAllocationMethod: ipAllocationMethod,
publicIp := network.PublicIPAddress{
Name: &name,
Location: &location,
Sku: &sku,
PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{
PublicIPAllocationMethod: ipAllocationMethod,
IdleTimeoutInMinutes: utils.Int32(int32(idleTimeout)),
},
Tags: expandTags(tags),
Zones: zones,
}

dnl, dnlOk := d.GetOk("domain_name_label")
Expand All @@ -145,29 +157,15 @@ func resourceArmPublicIpCreate(d *schema.ResourceData, meta interface{}) error {
dnsSettings.DomainNameLabel = &domainNameLabel
}

properties.DNSSettings = &dnsSettings
}

if v, ok := d.GetOk("idle_timeout_in_minutes"); ok {
properties.IdleTimeoutInMinutes = utils.Int32(int32(v.(int)))
}

publicIp := network.PublicIPAddress{
Name: &name,
Location: &location,
Sku: &sku,
PublicIPAddressPropertiesFormat: &properties,
Tags: expandTags(tags),
Zones: zones,
publicIp.PublicIPAddressPropertiesFormat.DNSSettings = &dnsSettings
}

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)
}

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)
}

Expand Down Expand Up @@ -236,6 +234,10 @@ func resourceArmPublicIpRead(d *schema.ResourceData, meta interface{}) error {
} else {
d.Set("ip_address", "")
}

if idleTimeout := props.IdleTimeoutInMinutes; idleTimeout != nil {
d.Set("idle_timeout_in_minutes", idleTimeout)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d.Set will handle this value being nil - so we should be able to omit the if statement here?

}
}

flattenAndSetTags(d, resp.Tags)
Expand All @@ -259,8 +261,7 @@ 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)
}

Expand Down
62 changes: 60 additions & 2 deletions azurerm/resource_arm_public_ip_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package azurerm
import (
"fmt"
"net/http"
"os"
"regexp"
"testing"

"github.com/hashicorp/terraform/helper/acctest"
Expand Down Expand Up @@ -60,6 +62,11 @@ func TestAccAzureRMPublicIpStatic_basic(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "public_ip_address_allocation", "static"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
Expand All @@ -84,11 +91,17 @@ func TestAccAzureRMPublicIpStatic_basic_withDNSLabel(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "domain_name_label", dnl),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccAzureRMPublicIpStatic_standard(t *testing.T) {
resourceName := "azurerm_public_ip.test"
ri := acctest.RandInt()
config := testAccAzureRMPublicIPStatic_standard(ri, testLocation())

Expand All @@ -100,9 +113,14 @@ func TestAccAzureRMPublicIpStatic_standard(t *testing.T) {
{
Config: config,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMPublicIpExists("azurerm_public_ip.test"),
testCheckAzureRMPublicIpExists(resourceName),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
Expand Down Expand Up @@ -146,6 +164,11 @@ func TestAccAzureRMPublicIpStatic_idleTimeout(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "idle_timeout_in_minutes", "30"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
Expand Down Expand Up @@ -208,11 +231,17 @@ func TestAccAzureRMPublicIpStatic_update(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "domain_name_label", fmt.Sprintf("acctest-%d", ri)),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccAzureRMPublicIpDynamic_basic(t *testing.T) {
resourceName := "azurerm_public_ip.test"
ri := acctest.RandInt()
config := testAccAzureRMPublicIPDynamic_basic(ri, testLocation())

Expand All @@ -224,9 +253,38 @@ func TestAccAzureRMPublicIpDynamic_basic(t *testing.T) {
{
Config: config,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMPublicIpExists("azurerm_public_ip.test"),
testCheckAzureRMPublicIpExists(resourceName),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccAzureRMPublicIpStatic_importIdError(t *testing.T) {
resourceName := "azurerm_public_ip.test"

ri := acctest.RandInt()
config := testAccAzureRMPublicIPStatic_basic(ri, testLocation())
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMPublicIpDestroy,
Steps: []resource.TestStep{
{
Config: config,
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateId: fmt.Sprintf("/subscriptions/%s/resourceGroups/acctestRG-%d/providers/Microsoft.Network/publicIPAdresses/acctestpublicip-%d", os.Getenv("ARM_SUBSCRIPTION_ID"), ri, ri),
ExpectError: regexp.MustCompile("Error parsing supplied resource id."),
},
},
})
}
Expand Down