From 5496db8fcb4f78aa52b84273bf9d9407080f1ac5 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 19 Feb 2019 09:28:25 -0800 Subject: [PATCH] resource/aws_route53_record: Add validation for alias name and zone_id arguments References: * https://github.com/terraform-providers/terraform-provider-aws/issues/7557 * https://docs.aws.amazon.com/Route53/latest/APIReference/API_AliasTarget.html The Route53 API returns a confusing API error message when implementing an ALIAS record for Custom VPC Endpoints if the name and zone_id values are swapped: ``` --- FAIL: TestAccAWSRoute53Record_alias_vpcendpoint (363.09s) testing.go:531: Step 0, expected error: Error applying: 1 error occurred: * aws_route53_record.test: 1 error occurred: * aws_route53_record.test: [ERR]: Error building changeset: InvalidInput: Invalid XML ; cvc-maxLength-valid: Value 'vpce-0bd0cf7081a1a0f93-jtogpkxk.vpce-svc-0f1eac6867ad44720.us-west-2.vpce.amazonaws.com' with length = '87' is not facet-valid with respect to maxLength '32' for type 'ResourceId'. ``` This updates the resource to now report a validation error: ``` Error applying: 1 error occurred: * aws_route53_record.test: expected length of alias.0.zone_id to be in the range (1 - 32), got vpce-0ed0dbc5b2e181289-kr42z80r.vpce-svc-04a7ad6c6da00d525.us-west-2.vpce.amazonaws.com ``` Output from acceptance testing: ``` --- PASS: TestAccAWSRoute53Record_Alias_VpcEndpoint (423.77s) ``` --- aws/resource_aws_route53_record.go | 6 +- aws/resource_aws_route53_record_test.go | 140 +++++++++++++++++++++--- 2 files changed, 129 insertions(+), 17 deletions(-) diff --git a/aws/resource_aws_route53_record.go b/aws/resource_aws_route53_record.go index 671cc7af881..44eb98cd3c3 100644 --- a/aws/resource_aws_route53_record.go +++ b/aws/resource_aws_route53_record.go @@ -101,8 +101,9 @@ func resourceAwsRoute53Record() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "zone_id": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringLenBetween(1, 32), }, "name": { @@ -112,6 +113,7 @@ func resourceAwsRoute53Record() *schema.Resource { DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { return strings.EqualFold(old, new) }, + ValidateFunc: validation.StringLenBetween(1, 1024), }, "evaluate_target_health": { diff --git a/aws/resource_aws_route53_record_test.go b/aws/resource_aws_route53_record_test.go index 292b508c95f..2e330af2501 100644 --- a/aws/resource_aws_route53_record_test.go +++ b/aws/resource_aws_route53_record_test.go @@ -395,11 +395,11 @@ func TestAccAWSRoute53Record_weighted_basic(t *testing.T) { }) } -func TestAccAWSRoute53Record_alias(t *testing.T) { +func TestAccAWSRoute53Record_Alias_Elb(t *testing.T) { var record1 route53.ResourceRecordSet rs := acctest.RandString(10) - config := fmt.Sprintf(testAccRoute53ElbAliasRecord, rs) + config := fmt.Sprintf(testAccRoute53RecordConfigAliasElb, rs) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, IDRefreshName: "aws_route53_record.alias", @@ -416,19 +416,17 @@ func TestAccAWSRoute53Record_alias(t *testing.T) { }) } -func TestAccAWSRoute53Record_aliasUppercase(t *testing.T) { +func TestAccAWSRoute53Record_Alias_S3(t *testing.T) { var record1 route53.ResourceRecordSet + rName := acctest.RandomWithPrefix("tf-acc-test") - rs := acctest.RandString(10) - config := fmt.Sprintf(testAccRoute53ElbAliasRecordUppercase, rs) resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - IDRefreshName: "aws_route53_record.alias", - Providers: testAccProviders, - CheckDestroy: testAccCheckRoute53RecordDestroy, + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckRoute53RecordDestroy, Steps: []resource.TestStep{ { - Config: config, + Config: testAccRoute53RecordConfigAliasS3(rName), Check: resource.ComposeTestCheckFunc( testAccCheckRoute53RecordExists("aws_route53_record.alias", &record1), ), @@ -437,9 +435,10 @@ func TestAccAWSRoute53Record_aliasUppercase(t *testing.T) { }) } -func TestAccAWSRoute53Record_s3_alias(t *testing.T) { +func TestAccAWSRoute53Record_Alias_VpcEndpoint(t *testing.T) { var record1 route53.ResourceRecordSet rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_route53_record.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -447,7 +446,32 @@ func TestAccAWSRoute53Record_s3_alias(t *testing.T) { CheckDestroy: testAccCheckRoute53RecordDestroy, Steps: []resource.TestStep{ { - Config: testAccRoute53S3AliasRecord(rName), + Config: testAccRoute53RecordConfigAliasCustomVpcEndpointSwappedAliasAttributes(rName), + ExpectError: regexp.MustCompile(`expected length of`), + }, + { + Config: testAccRoute53RecordConfigCustomVpcEndpoint(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckRoute53RecordExists(resourceName, &record1), + ), + }, + }, + }) +} + +func TestAccAWSRoute53Record_Alias_Uppercase(t *testing.T) { + var record1 route53.ResourceRecordSet + + rs := acctest.RandString(10) + config := fmt.Sprintf(testAccRoute53RecordConfigAliasElbUppercase, rs) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + IDRefreshName: "aws_route53_record.alias", + Providers: testAccProviders, + CheckDestroy: testAccCheckRoute53RecordDestroy, + Steps: []resource.TestStep{ + { + Config: config, Check: resource.ComposeTestCheckFunc( testAccCheckRoute53RecordExists("aws_route53_record.alias", &record1), ), @@ -1182,7 +1206,7 @@ resource "aws_route53_record" "ap-northeast-1" { } ` -const testAccRoute53ElbAliasRecord = ` +const testAccRoute53RecordConfigAliasElb = ` resource "aws_route53_zone" "main" { name = "notexample.com" } @@ -1212,7 +1236,7 @@ resource "aws_elb" "main" { } ` -const testAccRoute53ElbAliasRecordUppercase = ` +const testAccRoute53RecordConfigAliasElbUppercase = ` resource "aws_route53_zone" "main" { name = "notexample.com" } @@ -1242,7 +1266,7 @@ resource "aws_elb" "main" { } ` -func testAccRoute53S3AliasRecord(rName string) string { +func testAccRoute53RecordConfigAliasS3(rName string) string { return fmt.Sprintf(` resource "aws_route53_zone" "main" { name = "notexample.com" @@ -1270,6 +1294,92 @@ resource "aws_route53_record" "alias" { `, rName) } +func testAccRoute53CustomVpcEndpointBase(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" + + tags = { + Name = %[1]q + } +} + +resource "aws_subnet" "test" { + cidr_block = "10.0.0.0/24" + vpc_id = "${aws_vpc.test.id}" + + tags = { + Name = %[1]q + } +} + +resource "aws_lb" "test" { + internal = true + load_balancer_type = "network" + name = %[1]q + subnets = ["${aws_subnet.test.id}"] +} + +resource "aws_default_security_group" "test" { + vpc_id = "${aws_vpc.test.id}" +} + +resource "aws_vpc_endpoint_service" "test" { + acceptance_required = false + network_load_balancer_arns = ["${aws_lb.test.id}"] +} + +resource "aws_vpc_endpoint" "test" { + private_dns_enabled = false + security_group_ids = ["${aws_default_security_group.test.id}"] + service_name = "${aws_vpc_endpoint_service.test.service_name}" + subnet_ids = ["${aws_subnet.test.id}"] + vpc_endpoint_type = "Interface" + vpc_id = "${aws_vpc.test.id}" +} + +resource "aws_route53_zone" "test" { + name = "notexample.com" + + vpc { + vpc_id = "${aws_vpc.test.id}" + } +} +`, rName) +} + +func testAccRoute53RecordConfigAliasCustomVpcEndpointSwappedAliasAttributes(rName string) string { + return testAccRoute53CustomVpcEndpointBase(rName) + fmt.Sprintf(` +resource "aws_route53_record" "test" { + name = "test" + type = "A" + zone_id = "${aws_route53_zone.test.zone_id}" + + alias { + evaluate_target_health = false + name = "${lookup(aws_vpc_endpoint.test.dns_entry[0], "hosted_zone_id")}" + zone_id = "${lookup(aws_vpc_endpoint.test.dns_entry[0], "dns_name")}" + } +} +`) +} + +func testAccRoute53RecordConfigCustomVpcEndpoint(rName string) string { + return testAccRoute53CustomVpcEndpointBase(rName) + fmt.Sprintf(` +resource "aws_route53_record" "test" { + name = "test" + type = "A" + zone_id = "${aws_route53_zone.test.zone_id}" + + alias { + evaluate_target_health = false + name = "${lookup(aws_vpc_endpoint.test.dns_entry[0], "dns_name")}" + zone_id = "${lookup(aws_vpc_endpoint.test.dns_entry[0], "hosted_zone_id")}" + } +} +`) +} + const testAccRoute53WeightedElbAliasRecord = ` resource "aws_route53_zone" "main" { name = "notexample.com"