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.

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

add tag on create for aws_sqs_queue resource #10156

Conversation

teraken0509
Copy link
Contributor

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Closes #9850

Release note for CHANGELOG:

NONE

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSQSQueue_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSSQSQueue_ -timeout 120m
=== 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
=== CONT  TestAccAWSSQSQueue_basic
=== CONT  TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError
=== CONT  TestAccAWSSQSQueue_Policybasic
=== CONT  TestAccAWSSQSQueue_FIFOWithContentBasedDeduplication
=== CONT  TestAccAWSSQSQueue_FIFOExpectNameError
=== CONT  TestAccAWSSQSQueue_FIFO
=== CONT  TestAccAWSSQSQueue_policy
=== CONT  TestAccAWSSQSQueue_redrivePolicy
=== CONT  TestAccAWSSQSQueue_queueDeletedRecently
=== CONT  TestAccAWSSQSQueue_namePrefix
=== CONT  TestAccAWSSQSQueue_namePrefix_fifo
=== CONT  TestAccAWSSQSQueue_Encryption
=== CONT  TestAccAWSSQSQueue_tags
--- PASS: TestAccAWSSQSQueue_FIFOExpectNameError (19.38s)
--- PASS: TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError (19.59s)
--- PASS: TestAccAWSSQSQueue_Encryption (39.92s)
--- PASS: TestAccAWSSQSQueue_FIFO (40.89s)
--- PASS: TestAccAWSSQSQueue_FIFOWithContentBasedDeduplication (41.26s)
--- PASS: TestAccAWSSQSQueue_namePrefix (41.36s)
--- PASS: TestAccAWSSQSQueue_namePrefix_fifo (41.39s)
--- PASS: TestAccAWSSQSQueue_redrivePolicy (45.74s)
--- PASS: TestAccAWSSQSQueue_policy (48.26s)
--- PASS: TestAccAWSSQSQueue_Policybasic (48.49s)
--- PASS: TestAccAWSSQSQueue_basic (82.93s)
--- PASS: TestAccAWSSQSQueue_tags (83.79s)
--- PASS: TestAccAWSSQSQueue_queueDeletedRecently (119.09s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	119.220s

@teraken0509 teraken0509 requested a review from a team September 19, 2019 09:38
@ghost ghost added size/XS Managed by automation to categorize the size of a PR. service/sqs Issues and PRs that pertain to the sqs service. labels Sep 19, 2019
@bflad bflad added the enhancement Requests to existing resources that expand the functionality or scope. label Sep 24, 2019
@bflad bflad self-assigned this Sep 24, 2019
@bflad bflad added this to the v2.30.0 milestone Sep 24, 2019
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Thanks for this, @kterada0509 🚀 While the changes look okay, it looks like this functionality is not working properly in AWS GovCloud (US) yet and unfortunately breaking in a way we don't usually want to workaround with 500 Internal Server Errors:

2019/09/24 10:56:19 [DEBUG] [aws-sdk-go] DEBUG: Retrying Request sqs/CreateQueue, attempt 12
2019/09/24 10:56:19 [DEBUG] [aws-sdk-go] DEBUG: Request sqs/CreateQueue Details:
---[ REQUEST POST-SIGN ]-----------------------------
POST / HTTP/1.1
Host: sqs.us-gov-west-1.amazonaws.com
...

Action=CreateQueue&Attribute.1.Name=MaximumMessageSize&Attribute.1.Value=262144&Attribute.2.Name=MessageRetentionPeriod&Attribute.2.Value=345600&Attribute.3.Name=VisibilityTimeout&Attribute.3.Value=30&QueueName=sqs-queue-q6fiozksv7&Tag.1.Key=Environment&Tag.1.Value=production&Tag.2.Key=Usage&Tag.2.Value=original&Version=2012-11-05
-----------------------------------------------------
2019/09/24 10:56:19 [DEBUG] [aws-sdk-go] DEBUG: Response sqs/CreateQueue Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 500 Internal Server Error
...
2019/09/24 10:56:19 [DEBUG] [aws-sdk-go] <?xml version="1.0"?><ErrorResponse xmlns="http://queue.amazonaws.com/doc/2012-11-05/"><Error><Type>Receiver</Type><Code>InternalError</Code><Message>We encountered an internal error. Please try again.</Message><Detail/></Error><RequestId>5c1fe0da-0bdf-53a8-9432-b6de3b0cbcd3</RequestId></ErrorResponse>
2019/09/24 10:56:19 [DEBUG] [aws-sdk-go] DEBUG: Validate Response sqs/CreateQueue failed, attempt 12/25, error InternalError: We encountered an internal error. Please try again.
	status code: 500, request id: 5c1fe0da-0bdf-53a8-9432-b6de3b0cbcd3

The announcement does specifically call out:

now available in all commercial AWS Regions.

To maintain compatibility with other AWS partitions, this unfortunately looks we will need to ensure we're in a commercial region or otherwise fallback to the old logic. I left suggestions below. Please reach out if you have any questions.

Output from acceptance testing in AWS Commercial:

--- PASS: TestAccAWSSQSQueue_FIFOExpectNameError (5.08s)
--- PASS: TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError (5.23s)
--- PASS: TestAccAWSSQSQueue_FIFO (14.00s)
--- PASS: TestAccAWSSQSQueue_FIFOWithContentBasedDeduplication (14.36s)
--- PASS: TestAccAWSSQSQueue_namePrefix (14.37s)
--- PASS: TestAccAWSSQSQueue_namePrefix_fifo (14.37s)
--- PASS: TestAccAWSSQSQueue_Encryption (14.42s)
--- PASS: TestAccAWSSQSQueue_redrivePolicy (17.24s)
--- PASS: TestAccAWSSQSQueue_policy (19.11s)
--- PASS: TestAccAWSSQSQueue_Policybasic (19.21s)
--- PASS: TestAccAWSSQSQueue_basic (31.64s)
--- PASS: TestAccAWSSQSQueue_tags (31.89s)
--- PASS: TestAccAWSSQSQueue_queueDeletedRecently (92.05s)

@@ -170,6 +170,10 @@ func resourceAwsSqsQueueCreate(d *schema.ResourceData, meta interface{}) error {
QueueName: aws.String(name),
}

if v, ok := d.GetOk("tags"); ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if v, ok := d.GetOk("tags"); ok {
// Tag-on-create is currently only supported in AWS Commercial
if v, ok := d.GetOk("tags"); ok && meta.(*AWSClient).partition == endpoints.AwsPartitionID {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed.

@@ -215,7 +219,7 @@ func resourceAwsSqsQueueCreate(d *schema.ResourceData, meta interface{}) error {

d.SetId(aws.StringValue(output.QueueUrl))

return resourceAwsSqsQueueUpdate(d, meta)
return resourceAwsSqsQueueRead(d, meta)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return resourceAwsSqsQueueRead(d, meta)
// Tag-on-create is currently only supported in AWS Commercial
if meta.(*AWSClient).partition == endpoints.AwsPartitionID {
return resourceAwsSqsQueueRead(d, meta)
} else {
return resourceAwsSqsQueueUpdate(d, meta)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed.

@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Sep 24, 2019
@bflad bflad removed this from the v2.30.0 milestone Sep 24, 2019
@teraken0509 teraken0509 force-pushed the feature/add-support-tag-on-create-for-aws_sqs_queue-resource branch from 7077980 to 606f0ad Compare September 24, 2019 15:35
@teraken0509
Copy link
Contributor Author

teraken0509 commented Sep 24, 2019

Re-run acctest.

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSQSQueue_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSSQSQueue_ -timeout 120m
=== 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
=== CONT  TestAccAWSSQSQueue_basic
=== CONT  TestAccAWSSQSQueue_Policybasic
=== CONT  TestAccAWSSQSQueue_redrivePolicy
=== CONT  TestAccAWSSQSQueue_Encryption
=== CONT  TestAccAWSSQSQueue_queueDeletedRecently
=== CONT  TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError
=== CONT  TestAccAWSSQSQueue_namePrefix_fifo
=== CONT  TestAccAWSSQSQueue_namePrefix
=== CONT  TestAccAWSSQSQueue_tags
=== CONT  TestAccAWSSQSQueue_FIFOWithContentBasedDeduplication
=== CONT  TestAccAWSSQSQueue_FIFOExpectNameError
=== CONT  TestAccAWSSQSQueue_FIFO
=== CONT  TestAccAWSSQSQueue_policy
--- PASS: TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError (12.21s)
--- PASS: TestAccAWSSQSQueue_FIFOExpectNameError (12.24s)
--- PASS: TestAccAWSSQSQueue_namePrefix_fifo (30.59s)
--- PASS: TestAccAWSSQSQueue_namePrefix (30.99s)
--- PASS: TestAccAWSSQSQueue_FIFO (31.06s)
--- PASS: TestAccAWSSQSQueue_FIFOWithContentBasedDeduplication (31.23s)
--- PASS: TestAccAWSSQSQueue_Encryption (31.77s)
--- PASS: TestAccAWSSQSQueue_redrivePolicy (36.06s)
--- PASS: TestAccAWSSQSQueue_Policybasic (37.79s)
--- PASS: TestAccAWSSQSQueue_policy (37.86s)
--- PASS: TestAccAWSSQSQueue_tags (67.83s)
--- PASS: TestAccAWSSQSQueue_basic (68.36s)
--- PASS: TestAccAWSSQSQueue_queueDeletedRecently (108.93s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	109.028s

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 24, 2019
@bflad bflad added this to the v2.30.0 milestone Sep 24, 2019
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, @kterada0509 🚀

Output from acceptance testing in AWS Commercial:

--- PASS: TestAccAWSSQSQueue_FIFOExpectNameError (6.08s)
--- PASS: TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError (6.23s)
--- PASS: TestAccAWSSQSQueue_FIFO (14.78s)
--- PASS: TestAccAWSSQSQueue_namePrefix_fifo (15.02s)
--- PASS: TestAccAWSSQSQueue_FIFOWithContentBasedDeduplication (15.19s)
--- PASS: TestAccAWSSQSQueue_Encryption (15.42s)
--- PASS: TestAccAWSSQSQueue_namePrefix (15.63s)
--- PASS: TestAccAWSSQSQueue_redrivePolicy (18.64s)
--- PASS: TestAccAWSSQSQueue_Policybasic (20.19s)
--- PASS: TestAccAWSSQSQueue_policy (20.24s)
--- PASS: TestAccAWSSQSQueue_basic (32.75s)
--- PASS: TestAccAWSSQSQueue_tags (32.95s)
--- PASS: TestAccAWSSQSQueue_queueDeletedRecently (92.70s)

Output from acceptance testing in AWS GovCloud (US) (turns out SQS resource tagging isn't supported at all in GovCloud, but at least it errors immediately now):

--- PASS: TestAccAWSSQSQueue_ExpectContentBasedDeduplicationError (7.35s)
--- PASS: TestAccAWSSQSQueue_FIFOExpectNameError (7.54s)
--- FAIL: TestAccAWSSQSQueue_tags (10.83s)
    testing.go:569: Step 0 error: errors during apply:

        Error: AWS.SimpleQueueService.UnsupportedOperation: Tag queue in TagQueue operation are not supported.
        	status code: 400, request id: 7ec0febb-c893-526c-8a48-a8bff7a8d585

          on /var/folders/v0/_d108fkx1pbbg4_sh864_7740000gn/T/tf-test491518323/main.tf line 2:
          (source code not available)


--- PASS: TestAccAWSSQSQueue_namePrefix (18.94s)
--- PASS: TestAccAWSSQSQueue_namePrefix_fifo (19.07s)
--- PASS: TestAccAWSSQSQueue_Encryption (19.23s)
--- PASS: TestAccAWSSQSQueue_FIFO (20.09s)
--- PASS: TestAccAWSSQSQueue_FIFOWithContentBasedDeduplication (21.17s)
--- PASS: TestAccAWSSQSQueue_Policybasic (24.20s)
--- PASS: TestAccAWSSQSQueue_redrivePolicy (25.04s)
--- PASS: TestAccAWSSQSQueue_policy (25.15s)
--- PASS: TestAccAWSSQSQueue_queueDeletedRecently (98.77s)

@bflad bflad merged commit 02edb1b into hashicorp:master Sep 24, 2019
bflad added a commit that referenced this pull request Sep 24, 2019
@ghost
Copy link

ghost commented Sep 26, 2019

This has been released in version 2.30.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Nov 1, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 1, 2019
@teraken0509 teraken0509 deleted the feature/add-support-tag-on-create-for-aws_sqs_queue-resource branch March 5, 2020 14:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/sqs Issues and PRs that pertain to the sqs service. size/XS Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

r/aws_sqs_queue: Tag on create
2 participants