From f40997988e52aaaca52fc899e0472d5c8153e782 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Tue, 21 Mar 2017 00:06:45 +1100 Subject: [PATCH] Add `name_prefix` to `aws_autoscaling_group` and `aws_elb` resources (#12629) Adds support for `name_prefix` to the `aws_autoscaling_group` and `aws_elb` resources. Unfortunately when using `name_prefix` with `aws_elb`, this means that the specified prefix can only be a maximum of 6 characters in length. This is because the maximum length for an ELB name is 32 characters, and `resource.PrefixedUniqueId` generates a 26-character unique identifier. I was considering truncating the unique identifier to allow for a longer `name_prefix`, but I worried that doing so would increase the risk of collisions. --- .../aws/resource_aws_autoscaling_group.go | 28 ++++++++-- .../resource_aws_autoscaling_group_test.go | 37 +++++++++++++ builtin/providers/aws/resource_aws_elb.go | 17 ++++-- .../providers/aws/resource_aws_elb_test.go | 36 +++++++++++++ builtin/providers/aws/validators.go | 17 ++++++ builtin/providers/aws/validators_test.go | 53 +++++++++++++++++++ .../aws/r/autoscaling_group.html.markdown | 4 +- .../docs/providers/aws/r/elb.html.markdown | 4 +- 8 files changed, 186 insertions(+), 10 deletions(-) diff --git a/builtin/providers/aws/resource_aws_autoscaling_group.go b/builtin/providers/aws/resource_aws_autoscaling_group.go index 6a35164b0..e2de87d75 100644 --- a/builtin/providers/aws/resource_aws_autoscaling_group.go +++ b/builtin/providers/aws/resource_aws_autoscaling_group.go @@ -29,10 +29,11 @@ func resourceAwsAutoscalingGroup() *schema.Resource { Schema: map[string]*schema.Schema{ "name": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ConflictsWith: []string{"name_prefix"}, ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) { // https://github.com/boto/botocore/blob/9f322b1/botocore/data/autoscaling/2011-01-01/service-2.json#L1862-L1873 value := v.(string) @@ -43,6 +44,19 @@ func resourceAwsAutoscalingGroup() *schema.Resource { return }, }, + "name_prefix": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) { + value := v.(string) + if len(value) > 229 { + errors = append(errors, fmt.Errorf( + "%q cannot be longer than 229 characters, name is limited to 255", k)) + } + return + }, + }, "launch_configuration": &schema.Schema{ Type: schema.TypeString, @@ -282,7 +296,11 @@ func resourceAwsAutoscalingGroupCreate(d *schema.ResourceData, meta interface{}) if v, ok := d.GetOk("name"); ok { asgName = v.(string) } else { - asgName = resource.PrefixedUniqueId("tf-asg-") + if v, ok := d.GetOk("name_prefix"); ok { + asgName = resource.PrefixedUniqueId(v.(string)) + } else { + asgName = resource.PrefixedUniqueId("tf-asg-") + } d.Set("name", asgName) } diff --git a/builtin/providers/aws/resource_aws_autoscaling_group_test.go b/builtin/providers/aws/resource_aws_autoscaling_group_test.go index bb5453ce0..1c310433f 100644 --- a/builtin/providers/aws/resource_aws_autoscaling_group_test.go +++ b/builtin/providers/aws/resource_aws_autoscaling_group_test.go @@ -84,6 +84,27 @@ func TestAccAWSAutoScalingGroup_basic(t *testing.T) { }) } +func TestAccAWSAutoScalingGroup_namePrefix(t *testing.T) { + nameRegexp := regexp.MustCompile("^test-") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAutoScalingGroupDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSAutoScalingGroupConfig_namePrefix, + Check: resource.ComposeTestCheckFunc( + resource.TestMatchResourceAttr( + "aws_autoscaling_group.test", "name", nameRegexp), + resource.TestCheckResourceAttrSet( + "aws_autoscaling_group.test", "arn"), + ), + }, + }, + }) +} + func TestAccAWSAutoScalingGroup_autoGeneratedName(t *testing.T) { asgNameRegexp := regexp.MustCompile("^tf-asg-") @@ -748,6 +769,22 @@ resource "aws_autoscaling_group" "bar" { } ` +const testAccAWSAutoScalingGroupConfig_namePrefix = ` +resource "aws_launch_configuration" "test" { + image_id = "ami-21f78e11" + instance_type = "t1.micro" +} + +resource "aws_autoscaling_group" "test" { + availability_zones = ["us-west-2a"] + desired_capacity = 0 + max_size = 0 + min_size = 0 + name_prefix = "test-" + launch_configuration = "${aws_launch_configuration.test.name}" +} +` + const testAccAWSAutoScalingGroupConfig_terminationPoliciesEmpty = ` resource "aws_launch_configuration" "foobar" { image_id = "ami-21f78e11" diff --git a/builtin/providers/aws/resource_aws_elb.go b/builtin/providers/aws/resource_aws_elb.go index ce69ff6ef..2379ea49a 100644 --- a/builtin/providers/aws/resource_aws_elb.go +++ b/builtin/providers/aws/resource_aws_elb.go @@ -30,11 +30,18 @@ func resourceAwsElb() *schema.Resource { Schema: map[string]*schema.Schema{ "name": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ConflictsWith: []string{"name_prefix"}, + ValidateFunc: validateElbName, + }, + "name_prefix": &schema.Schema{ Type: schema.TypeString, Optional: true, - Computed: true, ForceNew: true, - ValidateFunc: validateElbName, + ValidateFunc: validateElbNamePrefix, }, "internal": &schema.Schema{ @@ -247,7 +254,11 @@ func resourceAwsElbCreate(d *schema.ResourceData, meta interface{}) error { if v, ok := d.GetOk("name"); ok { elbName = v.(string) } else { - elbName = resource.PrefixedUniqueId("tf-lb-") + if v, ok := d.GetOk("name_prefix"); ok { + elbName = resource.PrefixedUniqueId(v.(string)) + } else { + elbName = resource.PrefixedUniqueId("tf-lb-") + } d.Set("name", elbName) } diff --git a/builtin/providers/aws/resource_aws_elb_test.go b/builtin/providers/aws/resource_aws_elb_test.go index d39f76bff..bc6856f35 100644 --- a/builtin/providers/aws/resource_aws_elb_test.go +++ b/builtin/providers/aws/resource_aws_elb_test.go @@ -172,6 +172,28 @@ func TestAccAWSELB_AccessLogs_disabled(t *testing.T) { }) } +func TestAccAWSELB_namePrefix(t *testing.T) { + var conf elb.LoadBalancerDescription + nameRegex := regexp.MustCompile("^test-") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + IDRefreshName: "aws_elb.test", + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSELBDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSELB_namePrefix, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSELBExists("aws_elb.test", &conf), + resource.TestMatchResourceAttr( + "aws_elb.test", "name", nameRegex), + ), + }, + }, + }) +} + func TestAccAWSELB_generatedName(t *testing.T) { var conf elb.LoadBalancerDescription generatedNameRegexp := regexp.MustCompile("^tf-lb-") @@ -1138,6 +1160,20 @@ resource "aws_elb" "foo" { `, r, r) } +const testAccAWSELB_namePrefix = ` +resource "aws_elb" "test" { + name_prefix = "test-" + availability_zones = ["us-west-2a", "us-west-2b", "us-west-2c"] + + listener { + instance_port = 8000 + instance_protocol = "http" + lb_port = 80 + lb_protocol = "http" + } +} +` + const testAccAWSELBGeneratedName = ` resource "aws_elb" "foo" { availability_zones = ["us-west-2a", "us-west-2b", "us-west-2c"] diff --git a/builtin/providers/aws/validators.go b/builtin/providers/aws/validators.go index 4990e9396..03d2b3943 100644 --- a/builtin/providers/aws/validators.go +++ b/builtin/providers/aws/validators.go @@ -141,7 +141,24 @@ func validateElbName(v interface{}, k string) (ws []string, errors []error) { "%q cannot end with a hyphen: %q", k, value)) } return +} +func validateElbNamePrefix(v interface{}, k string) (ws []string, errors []error) { + value := v.(string) + if !regexp.MustCompile(`^[0-9A-Za-z-]+$`).MatchString(value) { + errors = append(errors, fmt.Errorf( + "only alphanumeric characters and hyphens allowed in %q: %q", + k, value)) + } + if len(value) > 6 { + errors = append(errors, fmt.Errorf( + "%q cannot be longer than 6 characters: %q", k, value)) + } + if regexp.MustCompile(`^-`).MatchString(value) { + errors = append(errors, fmt.Errorf( + "%q cannot begin with a hyphen: %q", k, value)) + } + return } func validateEcrRepositoryName(v interface{}, k string) (ws []string, errors []error) { diff --git a/builtin/providers/aws/validators_test.go b/builtin/providers/aws/validators_test.go index f717b5060..0c37308fe 100644 --- a/builtin/providers/aws/validators_test.go +++ b/builtin/providers/aws/validators_test.go @@ -1732,3 +1732,56 @@ func TestValidateApiGatewayUsagePlanQuotaSettings(t *testing.T) { } } } + +func TestValidateElbName(t *testing.T) { + validNames := []string{ + "tf-test-elb", + } + + for _, s := range validNames { + _, errors := validateElbName(s, "name") + if len(errors) > 0 { + t.Fatalf("%q should be a valid ELB name: %v", s, errors) + } + } + + invalidNames := []string{ + "tf.test.elb.1", + "tf-test-elb-tf-test-elb-tf-test-elb", + "-tf-test-elb", + "tf-test-elb-", + } + + for _, s := range invalidNames { + _, errors := validateElbName(s, "name") + if len(errors) == 0 { + t.Fatalf("%q should not be a valid ELB name: %v", s, errors) + } + } +} + +func TestValidateElbNamePrefix(t *testing.T) { + validNamePrefixes := []string{ + "test-", + } + + for _, s := range validNamePrefixes { + _, errors := validateElbNamePrefix(s, "name_prefix") + if len(errors) > 0 { + t.Fatalf("%q should be a valid ELB name prefix: %v", s, errors) + } + } + + invalidNamePrefixes := []string{ + "tf.test.elb.", + "tf-test", + "-test", + } + + for _, s := range invalidNamePrefixes { + _, errors := validateElbNamePrefix(s, "name_prefix") + if len(errors) == 0 { + t.Fatalf("%q should not be a valid ELB name prefix: %v", s, errors) + } + } +} diff --git a/website/source/docs/providers/aws/r/autoscaling_group.html.markdown b/website/source/docs/providers/aws/r/autoscaling_group.html.markdown index 5aad4c8ab..0ceacc2ba 100644 --- a/website/source/docs/providers/aws/r/autoscaling_group.html.markdown +++ b/website/source/docs/providers/aws/r/autoscaling_group.html.markdown @@ -64,7 +64,9 @@ EOF The following arguments are supported: -* `name` - (Optional) The name of the auto scale group. By default generated by terraform. +* `name` - (Optional) The name of the auto scaling group. By default generated by Terraform. +* `name_prefix` - (Optional) Creates a unique name beginning with the specified + prefix. Conflicts with `name`. * `max_size` - (Required) The maximum size of the auto scale group. * `min_size` - (Required) The minimum size of the auto scale group. (See also [Waiting for Capacity](#waiting-for-capacity) below.) diff --git a/website/source/docs/providers/aws/r/elb.html.markdown b/website/source/docs/providers/aws/r/elb.html.markdown index ee0bbdd4b..f682817d0 100644 --- a/website/source/docs/providers/aws/r/elb.html.markdown +++ b/website/source/docs/providers/aws/r/elb.html.markdown @@ -72,7 +72,9 @@ resource "aws_elb" "bar" { The following arguments are supported: -* `name` - (Optional) The name of the ELB. By default generated by terraform. +* `name` - (Optional) The name of the ELB. By default generated by Terraform. +* `name_prefix` - (Optional, Forces new resource) Creates a unique name beginning with the specified + prefix. Conflicts with `name`. * `access_logs` - (Optional) An Access Logs block. Access Logs documented below. * `availability_zones` - (Required for an EC2-classic ELB) The AZ's to serve traffic in. * `security_groups` - (Optional) A list of security group IDs to assign to the ELB.