From 5188242a0263153ddeca71bb678a0c36112076e7 Mon Sep 17 00:00:00 2001 From: kt Date: Fri, 19 Oct 2018 17:59:21 -0700 Subject: [PATCH 1/3] azurerm_public_ip: fixed domain name label validation --- azurerm/helpers/validate/public_ip.go | 16 +++++ azurerm/helpers/validate/public_ip_test.go | 35 ++++++++++ azurerm/resource_arm_public_ip.go | 29 +-------- azurerm/resource_arm_public_ip_test.go | 76 +++++++++++++--------- 4 files changed, 97 insertions(+), 59 deletions(-) create mode 100644 azurerm/helpers/validate/public_ip.go create mode 100644 azurerm/helpers/validate/public_ip_test.go diff --git a/azurerm/helpers/validate/public_ip.go b/azurerm/helpers/validate/public_ip.go new file mode 100644 index 000000000000..14fa42242677 --- /dev/null +++ b/azurerm/helpers/validate/public_ip.go @@ -0,0 +1,16 @@ +package validate + +import ( + "fmt" + "regexp" +) + +func PublicIpDomainNameLabel(v interface{}, k string) (ws []string, errors []error) { + value := v.(string) + if !regexp.MustCompile(`^[a-z][a-z0-9-]{1,61}[a-z0-9]$`).MatchString(value) { + errors = append(errors, fmt.Errorf( + "Domain must contain only lowercase alphanumeric characters, numbers and hyphens. It must start with a letter and end only with a number or letter", + k, value)) + } + return +} diff --git a/azurerm/helpers/validate/public_ip_test.go b/azurerm/helpers/validate/public_ip_test.go new file mode 100644 index 000000000000..1259ee7520a0 --- /dev/null +++ b/azurerm/helpers/validate/public_ip_test.go @@ -0,0 +1,35 @@ +package validate + +import "testing" + +func TestPublicIpDomainNameLabel(t *testing.T) { + cases := []struct { + Value string + ErrCount int + }{ + { + Value: "tEsting123", + ErrCount: 1, + }, + { + Value: "testing123!", + ErrCount: 1, + }, + { + Value: "testing123-", + ErrCount: 1, + }, + { + Value: acctest.RandString(80), + ErrCount: 1, + }, + } + + for _, tc := range cases { + _, errors := PublicIpDomainNameLabel(tc.Value, "azurerm_public_ip") + + if len(errors) != tc.ErrCount { + t.Fatalf("Expected the Azure RM Public IP Domain Name Label to trigger a validation error") + } + } +} diff --git a/azurerm/resource_arm_public_ip.go b/azurerm/resource_arm_public_ip.go index 84716ac742b3..570a5f08369c 100644 --- a/azurerm/resource_arm_public_ip.go +++ b/azurerm/resource_arm_public_ip.go @@ -2,8 +2,8 @@ package azurerm import ( "fmt" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" "log" - "regexp" "strings" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" @@ -95,7 +95,7 @@ func resourceArmPublicIp() *schema.Resource { "domain_name_label": { Type: schema.TypeString, Optional: true, - ValidateFunc: validatePublicIpDomainNameLabel, + ValidateFunc: validate.PublicIpDomainNameLabel, }, "reverse_fqdn": { @@ -277,28 +277,3 @@ func resourceArmPublicIpDelete(d *schema.ResourceData, meta interface{}) error { return nil } - -func validatePublicIpDomainNameLabel(v interface{}, k string) (ws []string, errors []error) { - value := v.(string) - if !regexp.MustCompile(`^[a-z0-9-]+$`).MatchString(value) { - errors = append(errors, fmt.Errorf( - "only lowercase alphanumeric characters and hyphens allowed in %q: %q", - k, value)) - } - - if len(value) > 61 { - errors = append(errors, fmt.Errorf( - "%q cannot be longer than 61 characters: %q", k, value)) - } - - if len(value) == 0 { - errors = append(errors, fmt.Errorf( - "%q cannot be an empty string: %q", k, value)) - } - if regexp.MustCompile(`-$`).MatchString(value) { - errors = append(errors, fmt.Errorf( - "%q cannot end with a hyphen: %q", k, value)) - } - - return -} diff --git a/azurerm/resource_arm_public_ip_test.go b/azurerm/resource_arm_public_ip_test.go index d2a6cbd1d520..e95ec828849c 100644 --- a/azurerm/resource_arm_public_ip_test.go +++ b/azurerm/resource_arm_public_ip_test.go @@ -12,38 +12,6 @@ import ( "github.com/hashicorp/terraform/terraform" ) -func TestResourceAzureRMPublicIpDomainNameLabel_validation(t *testing.T) { - cases := []struct { - Value string - ErrCount int - }{ - { - Value: "tEsting123", - ErrCount: 1, - }, - { - Value: "testing123!", - ErrCount: 1, - }, - { - Value: "testing123-", - ErrCount: 1, - }, - { - Value: acctest.RandString(80), - ErrCount: 1, - }, - } - - for _, tc := range cases { - _, errors := validatePublicIpDomainNameLabel(tc.Value, "azurerm_public_ip") - - if len(errors) != tc.ErrCount { - t.Fatalf("Expected the Azure RM Public IP Domain Name Label to trigger a validation error") - } - } -} - func TestAccAzureRMPublicIpStatic_basic(t *testing.T) { resourceName := "azurerm_public_ip.test" ri := acctest.RandInt() @@ -414,6 +382,32 @@ func TestAccAzureRMPublicIpStatic_importIdError(t *testing.T) { }) } +func TestAccAzureRMPublicIpStatic_canLabelBe63(t *testing.T) { + resourceName := "azurerm_public_ip.test" + ri := acctest.RandInt() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMPublicIpDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMPublicIPStatic_canLabelBe63(ri, testLocation()), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMPublicIpExists(resourceName), + resource.TestCheckResourceAttrSet(resourceName, "ip_address"), + resource.TestCheckResourceAttr(resourceName, "public_ip_address_allocation", "static"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func testCheckAzureRMPublicIpExists(name string) resource.TestCheckFunc { return func(s *terraform.State) error { // Ensure we have enough information in state to look up in API @@ -714,3 +708,21 @@ resource "azurerm_public_ip" "test" { } `, rInt, location, rInt) } + +func testAccAzureRMPublicIPStatic_canLabelBe63(rInt int, location string) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_public_ip" "test" { + name = "acctestpublicip-%d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + + public_ip_address_allocation = "static" + domain_name_label = "k2345678-1-2345678-2-2345678-3-2345678-4-2345678-5-2345678-6-23" +} +`, rInt, location, rInt) +} From b6569deb4d4ea91f20e59ab8c34a733eb0b2df8b Mon Sep 17 00:00:00 2001 From: kt Date: Fri, 19 Oct 2018 18:10:14 -0700 Subject: [PATCH 2/3] fixed test --- azurerm/helpers/validate/public_ip.go | 4 +--- azurerm/helpers/validate/public_ip_test.go | 12 +++++++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/azurerm/helpers/validate/public_ip.go b/azurerm/helpers/validate/public_ip.go index 14fa42242677..2c5bb2bd798d 100644 --- a/azurerm/helpers/validate/public_ip.go +++ b/azurerm/helpers/validate/public_ip.go @@ -8,9 +8,7 @@ import ( func PublicIpDomainNameLabel(v interface{}, k string) (ws []string, errors []error) { value := v.(string) if !regexp.MustCompile(`^[a-z][a-z0-9-]{1,61}[a-z0-9]$`).MatchString(value) { - errors = append(errors, fmt.Errorf( - "Domain must contain only lowercase alphanumeric characters, numbers and hyphens. It must start with a letter and end only with a number or letter", - k, value)) + errors = append(errors, fmt.Errorf("Domain must contain only lowercase alphanumeric characters, numbers and hyphens. It must start with a letter and end only with a number or letter")) } return } diff --git a/azurerm/helpers/validate/public_ip_test.go b/azurerm/helpers/validate/public_ip_test.go index 1259ee7520a0..4d6d9cb9ec02 100644 --- a/azurerm/helpers/validate/public_ip_test.go +++ b/azurerm/helpers/validate/public_ip_test.go @@ -1,6 +1,8 @@ package validate -import "testing" +import ( + "testing" +) func TestPublicIpDomainNameLabel(t *testing.T) { cases := []struct { @@ -20,7 +22,11 @@ func TestPublicIpDomainNameLabel(t *testing.T) { ErrCount: 1, }, { - Value: acctest.RandString(80), + Value: "k2345678-1-2345678-2-2345678-3-2345678-4-2345678-5-2345678-6-23", + ErrCount: 0, + }, + { + Value: "k2345678-1-2345678-2-2345678-3-2345678-4-2345678-5-2345678-6-234", ErrCount: 1, }, } @@ -29,7 +35,7 @@ func TestPublicIpDomainNameLabel(t *testing.T) { _, errors := PublicIpDomainNameLabel(tc.Value, "azurerm_public_ip") if len(errors) != tc.ErrCount { - t.Fatalf("Expected the Azure RM Public IP Domain Name Label to trigger a validation error") + t.Fatalf("Expected the Azure RM Public IP Domain Name Label %s to trigger a validation error", tc.Value) } } } From ba2b70ade9a90e986c5a537b4c97d04b24927704 Mon Sep 17 00:00:00 2001 From: "Tom Harvey (on holiday)" Date: Sat, 20 Oct 2018 19:18:33 -0700 Subject: [PATCH 3/3] Update azurerm/helpers/validate/public_ip.go Co-Authored-By: katbyte --- azurerm/helpers/validate/public_ip.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azurerm/helpers/validate/public_ip.go b/azurerm/helpers/validate/public_ip.go index 2c5bb2bd798d..3c50e6b84e05 100644 --- a/azurerm/helpers/validate/public_ip.go +++ b/azurerm/helpers/validate/public_ip.go @@ -8,7 +8,7 @@ import ( func PublicIpDomainNameLabel(v interface{}, k string) (ws []string, errors []error) { value := v.(string) if !regexp.MustCompile(`^[a-z][a-z0-9-]{1,61}[a-z0-9]$`).MatchString(value) { - errors = append(errors, fmt.Errorf("Domain must contain only lowercase alphanumeric characters, numbers and hyphens. It must start with a letter and end only with a number or letter")) + errors = append(errors, fmt.Errorf("%s must contain only lowercase alphanumeric characters, numbers and hyphens. It must start with a letter and end only with a number or letter", k)) } return }