Refactor WorkSpaces resources to use keyvaluetags package #11645

ewbankkit · 2020-01-17T19:37:35Z

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

Relates #10688.

Release note for CHANGELOG:

NONE

Output from acceptance testing:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsWorkspacesIpGroup_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesIpGroup_ -timeout 120m
=== RUN   TestAccAwsWorkspacesIpGroup_basic
=== PAUSE TestAccAwsWorkspacesIpGroup_basic
=== CONT  TestAccAwsWorkspacesIpGroup_basic
--- PASS: TestAccAwsWorkspacesIpGroup_basic (44.39s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	44.423s
$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsWorkspacesDirectory'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesDirectory -timeout 120m
=== RUN   TestAccAwsWorkspacesDirectory
=== RUN   TestAccAwsWorkspacesDirectory/basic
=== RUN   TestAccAwsWorkspacesDirectory/subnetIds
--- PASS: TestAccAwsWorkspacesDirectory (1464.50s)
    --- PASS: TestAccAwsWorkspacesDirectory/basic (852.48s)
    --- PASS: TestAccAwsWorkspacesDirectory/subnetIds (612.02s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1464.541s

Remove the generated version of WorkspacesUpdateTags as it leads to the removal of existing tags that are unchanged
* Move the custom version of WorkspacesUpdateTags to the keyvaluetags/custom directory
* Includes commit from Move custom service-specific tagging functions to own namespace #11638

ewbankkit · 2020-02-09T23:32:49Z

Tested with ignore_tags functionality to verify that tag updating works when there are ignored tags.

Acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsWorkspacesIpGroup_ignoreTags'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesIpGroup_ignoreTags -timeout 120m
=== RUN   TestAccAwsWorkspacesIpGroup_ignoreTags
=== PAUSE TestAccAwsWorkspacesIpGroup_ignoreTags
=== CONT  TestAccAwsWorkspacesIpGroup_ignoreTags
--- PASS: TestAccAwsWorkspacesIpGroup_ignoreTags (37.95s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	38.002s
$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsWorkspacesDirectory/ignoreTags'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesDirectory/ignoreTags -timeout 120m
=== RUN   TestAccAwsWorkspacesDirectory
=== RUN   TestAccAwsWorkspacesDirectory/ignoreTags
--- PASS: TestAccAwsWorkspacesDirectory (947.18s)
    --- PASS: TestAccAwsWorkspacesDirectory/ignoreTags (947.18s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	947.249s

bflad · 2020-02-10T16:22:57Z

aws/resource_aws_workspaces_directory.go

@@ -140,6 +140,8 @@ func resourceAwsWorkspacesDirectoryCreate(d *schema.ResourceData, meta interface

 func resourceAwsWorkspacesDirectoryRead(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).workspacesconn
+	ignoreTags := meta.(*AWSClient).ignoreTags


Hi @ewbankkit 👋 We would prefer to not add resources/logic to the ignore_tags preview at this time. We are finishing up a round of feedback on the functionality, so the implementation may change.

@bflad I was using the ignore_tags functionality to test the claim of the CreateTags API:

If you want to add new tags to a set of existing tags, you must submit all of the existing tags along with the new ones.

by having tags that are not managed by Terraform and seeing if they get removed when updating tags via Terraform (it turns out they don't, so the API in fact doesn't overwrite any existing tags).
I'll change this PR to WIP and remove the ignore_tags stuff.
I'll use the same approach to test the S3 object tagging functionality in #11964 but again not leave the changes in the final PR.

ewbankkit · 2020-02-12T14:51:56Z

OK, removed the ignore_tags changes as mentioned in the review comments above.
Re-ran acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsWorkspacesIpGroup_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesIpGroup_ -timeout 120m
=== RUN   TestAccAwsWorkspacesIpGroup_basic
=== PAUSE TestAccAwsWorkspacesIpGroup_basic
=== CONT  TestAccAwsWorkspacesIpGroup_basic
--- PASS: TestAccAwsWorkspacesIpGroup_basic (44.64s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	44.684s
$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsWorkspacesDirectory'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesDirectory -timeout 120m
=== RUN   TestAccAwsWorkspacesDirectory
=== RUN   TestAccAwsWorkspacesDirectory/basic
=== RUN   TestAccAwsWorkspacesDirectory/subnetIds
--- PASS: TestAccAwsWorkspacesDirectory (1398.14s)
    --- PASS: TestAccAwsWorkspacesDirectory/basic (788.30s)
    --- PASS: TestAccAwsWorkspacesDirectory/subnetIds (609.84s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1398.189s

Removing WIP.

ewbankkit · 2020-02-24T01:01:51Z

Rebased to remove merge conflicts.
Re-ran acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsWorkspacesIpGroup_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesIpGroup_ -timeout 120m
=== RUN   TestAccAwsWorkspacesIpGroup_basic
=== PAUSE TestAccAwsWorkspacesIpGroup_basic
=== CONT  TestAccAwsWorkspacesIpGroup_basic
--- PASS: TestAccAwsWorkspacesIpGroup_basic (45.06s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	45.101s
$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsWorkspacesDirectory'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesDirectory -timeout 120m
=== RUN   TestAccAwsWorkspacesDirectory
=== RUN   TestAccAwsWorkspacesDirectory/basic
=== RUN   TestAccAwsWorkspacesDirectory/disappears
=== RUN   TestAccAwsWorkspacesDirectory/subnetIds
--- PASS: TestAccAwsWorkspacesDirectory (1847.66s)
    --- PASS: TestAccAwsWorkspacesDirectory/basic (651.98s)
    --- PASS: TestAccAwsWorkspacesDirectory/disappears (596.50s)
    --- PASS: TestAccAwsWorkspacesDirectory/subnetIds (599.17s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1847.706s

… that doesn't change across update. $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsWorkspacesIpGroup_basic' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesIpGroup_basic -timeout 120m === RUN TestAccAwsWorkspacesIpGroup_basic === PAUSE TestAccAwsWorkspacesIpGroup_basic === CONT TestAccAwsWorkspacesIpGroup_basic --- PASS: TestAccAwsWorkspacesIpGroup_basic (45.42s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 45.454s

…sion. Acceptance test output: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsWorkspacesDirectory/basic' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesDirectory/basic -timeout 120m === RUN TestAccAwsWorkspacesDirectory === RUN TestAccAwsWorkspacesDirectory/basic --- PASS: TestAccAwsWorkspacesDirectory (661.39s) --- PASS: TestAccAwsWorkspacesDirectory/basic (661.39s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 661.429s

ewbankkit · 2020-02-25T18:13:24Z

Rebased to include ef098db and pass linting.

bflad

Looks great, thanks @ewbankkit 🚀

Output from acceptance testing:

--- PASS: TestAccAwsWorkspacesIpGroup_basic (14.01s)
--- PASS: TestAccAwsWorkspacesDirectory (1809.78s)
    --- PASS: TestAccAwsWorkspacesDirectory/disappears (662.61s)
    --- PASS: TestAccAwsWorkspacesDirectory/subnetIds (561.97s)
    --- PASS: TestAccAwsWorkspacesDirectory/basic (585.20s)

ghost · 2020-03-12T21:33:12Z

This has been released in version 2.53.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 · 2020-04-06T17:10:22Z

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!

ewbankkit requested a review from a team January 17, 2020 19:37

This comment has been minimized.

Sign in to view

ewbankkit changed the title ~~Refactor WorkSpaces resources to use keyvaluetags package~~ [WIP] Refactor WorkSpaces resources to use keyvaluetags package Jan 22, 2020

ewbankkit force-pushed the workspaces-keyvaluetags branch from 557f22a to 917715d Compare February 8, 2020 22:31

ghost added size/L Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Feb 8, 2020

ewbankkit force-pushed the workspaces-keyvaluetags branch from c309487 to 17df7dd Compare February 9, 2020 23:28

ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Feb 9, 2020

ewbankkit changed the title ~~[WIP] Refactor WorkSpaces resources to use keyvaluetags package~~ Refactor WorkSpaces resources to use keyvaluetags package Feb 9, 2020

bflad reviewed Feb 10, 2020

View reviewed changes

ewbankkit changed the title ~~Refactor WorkSpaces resources to use keyvaluetags package~~ [WIP] Refactor WorkSpaces resources to use keyvaluetags package Feb 11, 2020

ghost added size/L Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Feb 12, 2020

ewbankkit force-pushed the workspaces-keyvaluetags branch from 53546eb to 628e34b Compare February 12, 2020 14:48

ewbankkit changed the title ~~[WIP] Refactor WorkSpaces resources to use keyvaluetags package~~ Refactor WorkSpaces resources to use keyvaluetags package Feb 12, 2020

ewbankkit force-pushed the workspaces-keyvaluetags branch from 628e34b to c3db6f9 Compare February 22, 2020 22:33

ewbankkit mentioned this pull request Feb 22, 2020

Fix and enable tfproviderlint R007 and R008 checks: Deprecated d.Partial() and d.SetPartial() #12083

Closed

68 tasks

ewbankkit force-pushed the workspaces-keyvaluetags branch from c3db6f9 to 08b7d14 Compare February 24, 2020 01:00

ewbankkit added 2 commits February 25, 2020 13:04

ewbankkit force-pushed the workspaces-keyvaluetags branch from 08b7d14 to ebfceef Compare February 25, 2020 18:12

bflad removed the needs-triage Waiting for first response or review from a maintainer. label Mar 6, 2020

bflad self-assigned this Mar 6, 2020

bflad added this to the v2.53.0 milestone Mar 6, 2020

bflad approved these changes Mar 6, 2020

View reviewed changes

bflad merged commit 02cdaf9 into hashicorp:master Mar 6, 2020

ewbankkit deleted the workspaces-keyvaluetags branch March 6, 2020 20:22

ghost locked and limited conversation to collaborators Apr 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor WorkSpaces resources to use keyvaluetags package #11645

Refactor WorkSpaces resources to use keyvaluetags package #11645

ewbankkit commented Jan 17, 2020 •

edited

Loading

This comment has been minimized.

ewbankkit commented Feb 9, 2020 •

edited

Loading

bflad Feb 10, 2020

ewbankkit Feb 11, 2020 •

edited

Loading

ewbankkit commented Feb 12, 2020

ewbankkit commented Feb 24, 2020

ewbankkit commented Feb 25, 2020

bflad left a comment

ghost commented Mar 12, 2020

ghost commented Apr 6, 2020

Refactor WorkSpaces resources to use keyvaluetags package #11645

Refactor WorkSpaces resources to use keyvaluetags package #11645

Conversation

ewbankkit commented Jan 17, 2020 • edited Loading

Community Note

This comment has been minimized.

ewbankkit commented Feb 9, 2020 • edited Loading

bflad Feb 10, 2020

Choose a reason for hiding this comment

ewbankkit Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

ewbankkit commented Feb 12, 2020

ewbankkit commented Feb 24, 2020

ewbankkit commented Feb 25, 2020

bflad left a comment

Choose a reason for hiding this comment

ghost commented Mar 12, 2020

ghost commented Apr 6, 2020

ewbankkit commented Jan 17, 2020 •

edited

Loading

ewbankkit commented Feb 9, 2020 •

edited

Loading

ewbankkit Feb 11, 2020 •

edited

Loading