r/aws_sqs_queue: Make name generation and validation consistent with …

…aws_sns_topic (#15828). Acceptance test output: % make testacc TEST=./aws TESTARGS='-run=TestAccAWSSQSQueue_' ACCTEST_PARALLELISM=4 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 4 -run=TestAccAWSSQSQueue_ -timeout 180m === RUN TestAccAWSSQSQueue_basic === PAUSE TestAccAWSSQSQueue_basic === RUN TestAccAWSSQSQueue_tags === PAUSE TestAccAWSSQSQueue_tags === RUN TestAccAWSSQSQueue_namePrefix === PAUSE TestAccAWSSQSQueue_namePrefix === RUN TestAccAWSSQSQueue_namePrefix_fifo === PAUSE TestAccAWSSQSQueue_namePrefix_fifo === RUN TestAccAWSSQSQueue_policy === PAUSE TestAccAWSSQSQueue_policy === RUN TestAccAWSSQSQueue_queueDeletedRecently === PAUSE TestAccAWSSQSQueue_queueDeletedRecently === RUN TestAccAWSSQSQueue_redrivePolicy === PAUSE TestAccAWSSQSQueue_redrivePolicy === RUN TestAccAWSSQSQueue_Policybasic === PAUSE TestAccAWSSQSQueue_Policybasic === RUN TestAccAWSSQSQueue_FIFO === PAUSE TestAccAWSSQSQueue_FIFO === RUN TestAccAWSSQSQueue_FIFOExpectNameError === PAUSE TestAccAWSSQSQueue_FIFOExpectNameError === RUN TestAccAWSSQSQueue_FIFOWithContentBasedDeduplication === PAUSE TestAccAWSSQSQueue_FIFOWithContentBasedDeduplication === RUN TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError === PAUSE TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError === RUN TestAccAWSSQSQueue_Encryption === PAUSE TestAccAWSSQSQueue_Encryption === RUN TestAccAWSSQSQueue_FIFO_MinusName === PAUSE TestAccAWSSQSQueue_FIFO_MinusName === CONT TestAccAWSSQSQueue_basic === CONT TestAccAWSSQSQueue_FIFO === CONT TestAccAWSSQSQueue_Encryption === CONT TestAccAWSSQSQueue_policy --- PASS: TestAccAWSSQSQueue_FIFO (14.10s) === CONT TestAccAWSSQSQueue_Policybasic --- PASS: TestAccAWSSQSQueue_Encryption (14.34s) === CONT TestAccAWSSQSQueue_redrivePolicy --- PASS: TestAccAWSSQSQueue_redrivePolicy (14.96s) === CONT TestAccAWSSQSQueue_queueDeletedRecently --- PASS: TestAccAWSSQSQueue_basic (33.02s) === CONT TestAccAWSSQSQueue_FIFOWithContentBasedDeduplication --- PASS: TestAccAWSSQSQueue_FIFOWithContentBasedDeduplication (11.85s) === CONT TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError --- PASS: TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError (1.64s) === CONT TestAccAWSSQSQueue_namePrefix_fifo --- PASS: TestAccAWSSQSQueue_queueDeletedRecently (17.96s) === CONT TestAccAWSSQSQueue_FIFOExpectNameError --- PASS: TestAccAWSSQSQueue_FIFOExpectNameError (1.59s) === CONT TestAccAWSSQSQueue_FIFO_MinusName --- PASS: TestAccAWSSQSQueue_namePrefix_fifo (13.39s) === CONT TestAccAWSSQSQueue_tags --- PASS: TestAccAWSSQSQueue_FIFO_MinusName (13.12s) === CONT TestAccAWSSQSQueue_namePrefix --- PASS: TestAccAWSSQSQueue_policy (66.36s) --- PASS: TestAccAWSSQSQueue_namePrefix (11.90s) --- PASS: TestAccAWSSQSQueue_Policybasic (64.42s) --- PASS: TestAccAWSSQSQueue_tags (30.27s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 94.367s
hashicorp · Apr 15, 2021 · b6f347e · b6f347e
1 parent a1e946e
commit b6f347e
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 174 deletions.
diff --git a/aws/internal/service/sqs/consts.go b/aws/internal/service/sqs/consts.go
@@ -0,0 +1,5 @@
+package sqs
+
+const (
+	FifoQueueNameSuffix = ".fifo"
+)
diff --git a/aws/resource_aws_sqs_queue.go b/aws/resource_aws_sqs_queue.go
@@ -1,9 +1,11 @@
 package aws
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"net/url"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -17,6 +19,8 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 	"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/naming"
+	tfsqs "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/sqs"
 )
 
 var sqsQueueAttributeMap = map[string]string{
@@ -46,19 +50,20 @@ func resourceAwsSqsQueue() *schema.Resource {
 		Importer: &schema.ResourceImporter{
 			State: schema.ImportStatePassthrough,
 		},
+		CustomizeDiff: resourceAwsSqsQueueCustomizeDiff,
 
 		Schema: map[string]*schema.Schema{
 			"name": {
 				Type:          schema.TypeString,
 				Optional:      true,
-				ForceNew:      true,
 				Computed:      true,
+				ForceNew:      true,
 				ConflictsWith: []string{"name_prefix"},
-				ValidateFunc:  validateSQSQueueName,
 			},
 			"name_prefix": {
 				Type:          schema.TypeString,
 				Optional:      true,
+				Computed:      true,
 				ForceNew:      true,
 				ConflictsWith: []string{"name"},
 			},
@@ -136,38 +141,12 @@ func resourceAwsSqsQueueCreate(d *schema.ResourceData, meta interface{}) error {
 	sqsconn := meta.(*AWSClient).sqsconn
 
 	var name string
-	appendFifoSuffix := false
+	fifoQueue := d.Get("fifo_queue").(bool)
 
-	fq := d.Get("fifo_queue").(bool)
-
-	if v, ok := d.GetOk("name"); ok {
-		name = v.(string)
-	} else if v, ok := d.GetOk("name_prefix"); ok {
-		name = resource.PrefixedUniqueId(v.(string))
-		appendFifoSuffix = true
+	if fifoQueue {
+		name = naming.GenerateWithSuffix(d.Get("name").(string), d.Get("name_prefix").(string), tfsqs.FifoQueueNameSuffix)
 	} else {
-		name = resource.UniqueId()
-		appendFifoSuffix = true
-	}
-
-	if fq && appendFifoSuffix {
-		name += ".fifo"
-	}
-
-	cbd := d.Get("content_based_deduplication").(bool)
-
-	if fq {
-		if errors := validateSQSFifoQueueName(name); len(errors) > 0 {
-			return fmt.Errorf("Error validating the FIFO queue name: %v", errors)
-		}
-	} else {
-		if errors := validateSQSNonFifoQueueName(name); len(errors) > 0 {
-			return fmt.Errorf("Error validating SQS queue name: %v", errors)
-		}
-	}
-
-	if !fq && cbd {
-		return fmt.Errorf("Content based deduplication can only be set with FIFO queues")
+		name = naming.Generate(d.Get("name").(string), d.Get("name_prefix").(string))
 	}
 
 	log.Printf("[DEBUG] SQS queue create: %s", name)
@@ -305,16 +284,16 @@ func resourceAwsSqsQueueRead(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}
 
+	fifoQueue := false
+
 	// Always set attribute defaults
 	d.Set("arn", "")
 	d.Set("content_based_deduplication", false)
 	d.Set("delay_seconds", 0)
-	d.Set("fifo_queue", false)
 	d.Set("kms_data_key_reuse_period_seconds", 300)
 	d.Set("kms_master_key_id", "")
 	d.Set("max_message_size", 262144)
 	d.Set("message_retention_seconds", 345600)
-	d.Set("name", name)
 	d.Set("policy", "")
 	d.Set("receive_wait_time_seconds", 0)
 	d.Set("redrive_policy", "")
@@ -354,7 +333,7 @@ func resourceAwsSqsQueueRead(d *schema.ResourceData, meta interface{}) error {
 				return fmt.Errorf("error parsing fifo_queue value (%s) into boolean: %s", v, err)
 			}
 
-			d.Set("fifo_queue", vBool)
+			fifoQueue = vBool
 		}
 
 		if v, ok := queueAttributes[sqs.QueueAttributeNameKmsDataKeyReusePeriodSeconds]; ok && v != "" {
@@ -420,6 +399,14 @@ func resourceAwsSqsQueueRead(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
+	d.Set("fifo_queue", fifoQueue)
+	d.Set("name", name)
+	if fifoQueue {
+		d.Set("name_prefix", naming.NamePrefixFromNameWithSuffix(name, tfsqs.FifoQueueNameSuffix))
+	} else {
+		d.Set("name_prefix", naming.NamePrefixFromName(name))
+	}
+
 	tags, err := keyvaluetags.SqsListTags(sqsconn, d.Id())
 
 	if err != nil {
@@ -448,6 +435,42 @@ func resourceAwsSqsQueueDelete(d *schema.ResourceData, meta interface{}) error {
 	return err
 }
 
+func resourceAwsSqsQueueCustomizeDiff(_ context.Context, diff *schema.ResourceDiff, meta interface{}) error {
+	fifoQueue := diff.Get("fifo_queue").(bool)
+	contentBasedDeduplication := diff.Get("content_based_deduplication").(bool)
+
+	if diff.Id() == "" {
+		// Create.
+
+		var name string
+
+		if fifoQueue {
+			name = naming.GenerateWithSuffix(diff.Get("name").(string), diff.Get("name_prefix").(string), tfsqs.FifoQueueNameSuffix)
+		} else {
+			name = naming.Generate(diff.Get("name").(string), diff.Get("name_prefix").(string))
+		}
+
+		var re *regexp.Regexp
+
+		if fifoQueue {
+			re = regexp.MustCompile(`^[a-zA-Z0-9_-]{1,75}\.fifo$`)
+		} else {
+			re = regexp.MustCompile(`^[a-zA-Z0-9_-]{1,80}$`)
+		}
+
+		if !re.MatchString(name) {
+			return fmt.Errorf("invalid queue name: %s", name)
+		}
+
+	}
+
+	if !fifoQueue && contentBasedDeduplication {
+		return fmt.Errorf("content-based deduplication can only be set for FIFO queue")
+	}
+
+	return nil
+}
+
 func extractNameFromSqsQueueUrl(queue string) (string, error) {
 	//http://sqs.us-west-2.amazonaws.com/123456789012/queueName
 	u, err := url.Parse(queue)

diff --git a/aws/resource_aws_sqs_queue_test.go b/aws/resource_aws_sqs_queue_test.go
@@ -385,7 +385,7 @@ func TestAccAWSSQSQueue_FIFOExpectNameError(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config:      testAccAWSSQSConfigWithFIFOExpectError(acctest.RandString(10)),
-				ExpectError: regexp.MustCompile(`Error validating the FIFO queue name`),
+				ExpectError: regexp.MustCompile(`invalid queue name:`),
 			},
 		},
 	})
@@ -430,7 +430,7 @@ func TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config:      testAccExpectContentBasedDeduplicationError(acctest.RandString(10)),
-				ExpectError: regexp.MustCompile(`Content based deduplication can only be set with FIFO queues`),
+				ExpectError: regexp.MustCompile(`content-based deduplication can only be set for FIFO queue`),
 			},
 		},
 	})

diff --git a/aws/validators.go b/aws/validators.go
@@ -1033,54 +1033,6 @@ func validateApiGatewayIntegrationContentHandling() schema.SchemaValidateFunc {
 	}, false)
 }
 
-func validateSQSQueueName(v interface{}, k string) (ws []string, errors []error) {
-	value := v.(string)
-	if len(value) > 80 {
-		errors = append(errors, fmt.Errorf("%q cannot be longer than 80 characters", k))
-	}
-
-	if !regexp.MustCompile(`^[0-9A-Za-z-_]+(\.fifo)?$`).MatchString(value) {
-		errors = append(errors, fmt.Errorf("only alphanumeric characters and hyphens allowed in %q", k))
-	}
-	return
-}
-
-func validateSQSNonFifoQueueName(v interface{}) (errors []error) {
-	k := "name"
-	value := v.(string)
-	if len(value) > 80 {
-		errors = append(errors, fmt.Errorf("%q cannot be longer than 80 characters", k))
-	}
-
-	if !regexp.MustCompile(`^[0-9A-Za-z-_]+$`).MatchString(value) {
-		errors = append(errors, fmt.Errorf("only alphanumeric characters and hyphens allowed in %q", k))
-	}
-	return
-}
-
-func validateSQSFifoQueueName(v interface{}) (errors []error) {
-	k := "name"
-	value := v.(string)
-
-	if len(value) > 80 {
-		errors = append(errors, fmt.Errorf("%q cannot be longer than 80 characters", k))
-	}
-
-	if !regexp.MustCompile(`^[0-9A-Za-z-_.]+$`).MatchString(value) {
-		errors = append(errors, fmt.Errorf("only alphanumeric characters and hyphens allowed in %q", k))
-	}
-
-	if regexp.MustCompile(`^[^a-zA-Z0-9-_]`).MatchString(value) {
-		errors = append(errors, fmt.Errorf("FIFO queue name must start with one of these characters [a-zA-Z0-9-_]: %v", value))
-	}
-
-	if !regexp.MustCompile(`\.fifo$`).MatchString(value) {
-		errors = append(errors, fmt.Errorf("FIFO queue name should end with \".fifo\": %v", value))
-	}
-
-	return
-}
-
 func validateOnceAWeekWindowFormat(v interface{}, k string) (ws []string, errors []error) {
 	// valid time format is "ddd:hh24:mi"
 	validTimeFormat := "(sun|mon|tue|wed|thu|fri|sat):([0-1][0-9]|2[0-3]):([0-5][0-9])"

diff --git a/aws/validators_test.go b/aws/validators_test.go
@@ -1,7 +1,6 @@
 package aws
 
 import (
-	"fmt"
 	"regexp"
 	"strings"
 	"testing"
@@ -974,92 +973,6 @@ func TestValidateStringIsJsonOrYaml(t *testing.T) {
 	}
 }
 
-func TestValidateSQSQueueName(t *testing.T) {
-	validNames := []string{
-		"valid-name",
-		"valid02-name",
-		"Valid-Name1",
-		"_",
-		"-",
-		strings.Repeat("W", 80),
-	}
-	for _, v := range validNames {
-		if _, errors := validateSQSQueueName(v, "test_attribute"); len(errors) > 0 {
-			t.Fatalf("%q should be a valid SQS queue Name", v)
-		}
-
-		if errors := validateSQSNonFifoQueueName(v); len(errors) > 0 {
-			t.Fatalf("%q should be a valid SQS non-fifo queue Name", v)
-		}
-	}
-
-	invalidNames := []string{
-		"Here is a name with: colon",
-		"another * invalid name",
-		"also $ invalid",
-		"This . is also %% invalid@!)+(",
-		"*",
-		"",
-		" ",
-		".",
-		strings.Repeat("W", 81), // length > 80
-	}
-	for _, v := range invalidNames {
-		if _, errors := validateSQSQueueName(v, "test_attribute"); len(errors) == 0 {
-			t.Fatalf("%q should be an invalid SQS queue Name", v)
-		}
-
-		if errors := validateSQSNonFifoQueueName(v); len(errors) == 0 {
-			t.Fatalf("%q should be an invalid SQS non-fifo queue Name", v)
-		}
-	}
-}
-
-func TestValidateSQSFifoQueueName(t *testing.T) {
-	validNames := []string{
-		"valid-name.fifo",
-		"valid02-name.fifo",
-		"Valid-Name1.fifo",
-		"_.fifo",
-		"a.fifo",
-		"A.fifo",
-		"9.fifo",
-		"-.fifo",
-		fmt.Sprintf("%s.fifo", strings.Repeat("W", 75)),
-	}
-	for _, v := range validNames {
-		if _, errors := validateSQSQueueName(v, "test_attribute"); len(errors) > 0 {
-			t.Fatalf("%q should be a valid SQS queue Name", v)
-		}
-
-		if errors := validateSQSFifoQueueName(v); len(errors) > 0 {
-			t.Fatalf("%q should be a valid SQS FIFO queue Name: %v", v, errors)
-		}
-	}
-
-	invalidNames := []string{
-		"Here is a name with: colon",
-		"another * invalid name",
-		"also $ invalid",
-		"This . is also %% invalid@!)+(",
-		".fifo",
-		"*",
-		"",
-		" ",
-		".",
-		strings.Repeat("W", 81), // length > 80
-	}
-	for _, v := range invalidNames {
-		if _, errors := validateSQSQueueName(v, "test_attribute"); len(errors) == 0 {
-			t.Fatalf("%q should be an invalid SQS queue Name", v)
-		}
-
-		if errors := validateSQSFifoQueueName(v); len(errors) == 0 {
-			t.Fatalf("%q should be an invalid SQS FIFO queue Name: %v", v, errors)
-		}
-	}
-}
-
 func TestValidateOnceAWeekWindowFormat(t *testing.T) {
 	cases := []struct {
 		Value    string

diff --git a/website/docs/r/sqs_queue.html.markdown b/website/docs/r/sqs_queue.html.markdown
@@ -52,8 +52,8 @@ resource "aws_sqs_queue" "terraform_queue" {
 
 The following arguments are supported:
 
-* `name` - (Optional) This is the human-readable name of the queue. If omitted, Terraform will assign a random name.
-* `name_prefix` - (Optional) Creates a unique name beginning with the specified prefix. Conflicts with `name`.
+* `name` - (Optional) The name of the queue. Queue names must be made up of only uppercase and lowercase ASCII letters, numbers, underscores, and hyphens, and must be between 1 and 80 characters long. For a FIFO (first-in-first-out) queue, the name must end with the `.fifo` suffix. If omitted, Terraform will assign a random, unique name. Conflicts with `name_prefix`
+* `name_prefix` - (Optional) Creates a unique name beginning with the specified prefix. Conflicts with `name`
 * `visibility_timeout_seconds` - (Optional) The visibility timeout for the queue. An integer from 0 to 43200 (12 hours). The default for this attribute is 30. For more information about visibility timeout, see [AWS docs](https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/AboutVT.html).
 * `message_retention_seconds` - (Optional) The number of seconds Amazon SQS retains a message. Integer representing seconds, from 60 (1 minute) to 1209600 (14 days). The default for this attribute is 345600 (4 days).
 * `max_message_size` - (Optional) The limit of how many bytes a message can contain before Amazon SQS rejects it. An integer from 1024 bytes (1 KiB) up to 262144 bytes (256 KiB). The default for this attribute is 262144 (256 KiB).