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

provider: Introduce tag resource generator, new aws_dynamodb_tag and aws_ecs_tag resources #13783

Merged
merged 41 commits into from
Aug 20, 2021

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Jun 17, 2020

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" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #6859
Closes #9061
Closes #11951
Closes #13725

Release note for CHANGELOG:

* **New Resource:** `aws_dynamodb_tag`
* **New Resource:** `aws_ecs_tag`

Output from acceptance testing:

--- PASS: TestAccAWSDynamodbTag_disappears (27.18s)
--- PASS: TestAccAWSDynamodbTag_basic (31.00s)
--- PASS: TestAccAWSDynamodbTag_Value (42.71s)
--- PASS: TestAccAWSDynamodbTag_ResourceArn_TableReplica (159.24s)

--- PASS: TestAccAWSEc2Tag_disappears (429.66s)
--- PASS: TestAccAWSEc2Tag_Value (530.85s)
--- PASS: TestAccAWSEc2Tag_basic (537.38s)

--- PASS: TestAccAWSEcsTag_disappears (29.26s)
--- PASS: TestAccAWSEcsTag_basic (31.50s)
--- PASS: TestAccAWSEcsTag_Value (42.11s)
--- PASS: TestAccAWSEcsTag_ResourceArn_BatchComputeEnvironment (46.40s)

@bflad bflad added the new-resource Introduces a new resource. label Jun 17, 2020
@bflad bflad requested a review from a team June 17, 2020 04:47
@ghost ghost added size/XXL Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/dynamodb Issues and PRs that pertain to the dynamodb service. service/ec2 Issues and PRs that pertain to the ec2 service. service/ecs Issues and PRs that pertain to the ecs service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Jun 17, 2020
@DrFaust92
Copy link
Collaborator

Hey @bflad, is it possible to add asg_tag as well to this PR?
like proposed in #13643? or does it differ to much from this logic

@bflad
Copy link
Contributor Author

bflad commented Jun 17, 2020

@DrFaust92 unfortunately Auto Scaling service tags differ from other services since they contain an extra PropagateAtLaunch field, which cannot be handled in the keyvaluetags package at the moment and subsequently this generator. I'll write up a separate issue since we would like to tackle Auto Scaling tags more holistically.

Base automatically changed from master to main January 23, 2021 00:58
@breathingdust breathingdust requested a review from a team as a code owner January 23, 2021 00:58
@gdavison gdavison self-assigned this Feb 16, 2021
@gdavison
Copy link
Contributor

@bflad, can you please rebase this?

Copy link
Contributor

@gdavison gdavison left a comment

Choose a reason for hiding this comment

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

I have a few comments, otherwise looks good. I'll complete the review once this PR has been rebased

aws/resource_aws_dynamodb_tag_gen.go Outdated Show resolved Hide resolved
aws/resource_aws_dynamodb_tag_gen_test.go Outdated Show resolved Hide resolved
aws/resource_aws_dynamodb_tag_gen_test.go Outdated Show resolved Hide resolved
aws/resource_aws_ec2_tag_gen_test.go Show resolved Hide resolved
aws/resource_aws_ec2_tag_gen_test.go Show resolved Hide resolved
docs/contributing/contribution-checklists.md Outdated Show resolved Hide resolved
bflad added a commit that referenced this pull request Mar 24, 2021
@bflad
Copy link
Contributor Author

bflad commented Mar 25, 2021

Alrighty -- everything has been rebased and modernized.

Latest output from acceptance testing:

--- PASS: TestAccAWSDynamodbTag_basic (37.69s)
--- PASS: TestAccAWSDynamodbTag_disappears (41.74s)
--- PASS: TestAccAWSDynamodbTag_ResourceArn_TableReplica (259.64s)
--- PASS: TestAccAWSDynamodbTag_Value (53.88s)

--- PASS: TestAccAWSEc2Tag_basic (592.92s)
--- PASS: TestAccAWSEc2Tag_disappears (576.53s)
--- PASS: TestAccAWSEc2Tag_Value (714.84s)

--- PASS: TestAccAWSEcsTag_basic (41.02s)
--- PASS: TestAccAWSEcsTag_disappears (40.40s)
--- PASS: TestAccAWSEcsTag_ResourceArn_BatchComputeEnvironment (54.92s)
--- PASS: TestAccAWSEcsTag_Value (50.42s)

@bflad
Copy link
Contributor Author

bflad commented Apr 21, 2021

@gdavison is there anything else that needs to be done?

@ewbankkit
Copy link
Contributor

Not reviewing any specific details of this PR, more a general comment:
Would collapsing the err and exists return values from the keyvaluetags.<Service>GetTag function so that a resourceNotFoundError is equivalent to !exists be more consistent with where we are thinking of going with resource-not-found error handling (#15945)?

bflad and others added 19 commits August 19, 2021 12:10
Reference: #6859
Reference: #13725

Output from acceptance testing:

```
--- PASS: TestAccAWSDynamodbTag_disappears (27.18s)
--- PASS: TestAccAWSDynamodbTag_basic (31.00s)
--- PASS: TestAccAWSDynamodbTag_Value (42.71s)
--- PASS: TestAccAWSDynamodbTag_ResourceArn_TableReplica (159.24s)
```
…ation for Terraform Plugin SDK v2

Output from acceptance testing:

```
--- PASS: TestAccAWSDynamodbTag_basic (37.69s)
--- PASS: TestAccAWSDynamodbTag_disappears (41.74s)
--- PASS: TestAccAWSDynamodbTag_ResourceArn_TableReplica (259.64s)
--- PASS: TestAccAWSDynamodbTag_Value (53.88s)

--- PASS: TestAccAWSEc2Tag_basic (592.92s)
--- PASS: TestAccAWSEc2Tag_disappears (576.53s)
--- PASS: TestAccAWSEc2Tag_Value (714.84s)

--- PASS: TestAccAWSEcsTag_basic (41.02s)
--- PASS: TestAccAWSEcsTag_disappears (40.40s)
--- PASS: TestAccAWSEcsTag_ResourceArn_BatchComputeEnvironment (54.92s)
--- PASS: TestAccAWSEcsTag_Value (50.42s)
```
% make testacc TESTARGS='-run=TestAccAWSDynamodbTag_\|TestAccAWSEc2Tag_\|TestAccAWSEcsTag_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSDynamodbTag_\|TestAccAWSEc2Tag_\|TestAccAWSEcsTag_ -timeout 180m
=== RUN   TestAccAWSDynamodbTag_basic
=== PAUSE TestAccAWSDynamodbTag_basic
=== RUN   TestAccAWSDynamodbTag_disappears
=== PAUSE TestAccAWSDynamodbTag_disappears
=== RUN   TestAccAWSDynamodbTag_ResourceArn_TableReplica
=== PAUSE TestAccAWSDynamodbTag_ResourceArn_TableReplica
=== RUN   TestAccAWSDynamodbTag_Value
=== PAUSE TestAccAWSDynamodbTag_Value
=== RUN   TestAccAWSEc2Tag_basic
=== PAUSE TestAccAWSEc2Tag_basic
=== RUN   TestAccAWSEc2Tag_disappears
=== PAUSE TestAccAWSEc2Tag_disappears
=== RUN   TestAccAWSEc2Tag_Value
=== PAUSE TestAccAWSEc2Tag_Value
=== RUN   TestAccAWSEcsTag_basic
=== PAUSE TestAccAWSEcsTag_basic
=== RUN   TestAccAWSEcsTag_disappears
=== PAUSE TestAccAWSEcsTag_disappears
=== RUN   TestAccAWSEcsTag_ResourceArn_BatchComputeEnvironment
=== PAUSE TestAccAWSEcsTag_ResourceArn_BatchComputeEnvironment
=== RUN   TestAccAWSEcsTag_Value
=== PAUSE TestAccAWSEcsTag_Value
=== CONT  TestAccAWSDynamodbTag_basic
=== CONT  TestAccAWSEc2Tag_Value
=== CONT  TestAccAWSDynamodbTag_disappears
=== CONT  TestAccAWSEcsTag_basic
=== CONT  TestAccAWSDynamodbTag_Value
=== CONT  TestAccAWSEc2Tag_disappears
=== CONT  TestAccAWSEc2Tag_basic
=== CONT  TestAccAWSEcsTag_ResourceArn_BatchComputeEnvironment
=== CONT  TestAccAWSDynamodbTag_ResourceArn_TableReplica
=== CONT  TestAccAWSEcsTag_Value
=== CONT  TestAccAWSEcsTag_disappears
--- PASS: TestAccAWSDynamodbTag_disappears (27.72s)
--- PASS: TestAccAWSDynamodbTag_basic (30.17s)
--- PASS: TestAccAWSEcsTag_disappears (30.37s)
--- PASS: TestAccAWSEcsTag_basic (33.00s)
--- PASS: TestAccAWSEcsTag_ResourceArn_BatchComputeEnvironment (40.65s)
--- PASS: TestAccAWSEcsTag_Value (43.02s)
--- PASS: TestAccAWSDynamodbTag_Value (47.20s)
--- PASS: TestAccAWSDynamodbTag_ResourceArn_TableReplica (235.23s)
--- PASS: TestAccAWSEc2Tag_disappears (587.96s)
--- PASS: TestAccAWSEc2Tag_basic (642.95s)
--- PASS: TestAccAWSEc2Tag_Value (723.77s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       727.053s
Acceptance test output:

% make testacc TESTARGS='-run=TestAccAWSEc2Tag_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSEc2Tag_basic -timeout 180m
=== RUN   TestAccAWSEc2Tag_basic
=== PAUSE TestAccAWSEc2Tag_basic
=== CONT  TestAccAWSEc2Tag_basic
--- PASS: TestAccAWSEc2Tag_basic (643.11s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       646.359s
@github-actions github-actions bot added the size/XL Managed by automation to categorize the size of a PR. label Aug 20, 2021
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

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

LGTM 🚀.

@ewbankkit ewbankkit merged commit 24b31c5 into main Aug 20, 2021
@ewbankkit ewbankkit deleted the f-tagresource-generator branch August 20, 2021 19:25
@github-actions github-actions bot added this to the v3.56.0 milestone Aug 20, 2021
github-actions bot pushed a commit that referenced this pull request Aug 20, 2021
@github-actions
Copy link

This functionality has been released in v3.56.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. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. new-resource Introduces a new resource. provider Pertains to the provider itself, rather than any interaction with AWS. service/dynamodb Issues and PRs that pertain to the dynamodb service. service/ec2 Issues and PRs that pertain to the ec2 service. service/ecs Issues and PRs that pertain to the ecs service. size/XL Managed by automation to categorize the size of a PR. size/XXL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
4 participants