lakeformation: Add new aws_lakeformation_resource_lf_tags resource #25565

YakDriver · 2022-06-24T20:27:13Z

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 #19640
Closes #20996
Relates #19523
Relates #19648

Output from acceptance testing:

% make testacc TESTS="TestAccLakeFormation_serial/ResourceLFTags" PKG=lakeformation
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lakeformation/... -v -count 1 -parallel 20 -run='TestAccLakeFormation_serial/ResourceLFTags'  -timeout 180m
--- PASS: TestAccLakeFormation_serial (283.81s)
    --- PASS: TestAccLakeFormation_serial/ResourceLFTags (283.81s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/tableWithColumns (66.94s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/basic (33.07s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/database (59.78s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/databaseMultiple (61.75s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/table (62.27s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/lakeformation	285.088s

YakDriver · 2022-06-24T22:01:49Z

@sbrandtb @danielcmessias @stevenayers @maiarareinaldo @simonB2020 Do you have any feedback on this PR?

danielcmessias

Looks great, few small comments/questions

danielcmessias · 2022-06-27T08:53:56Z

website/docs/r/lakeformation_resource_lf_tags.html.markdown

+}
+
+resource "aws_lakeformation_resource_lf_tags" "example" {
+  catalog_id = data.aws_caller_identity.current.account_id


minor but this is inferred by default, in which case probably not needed in these examples as you'd likely only be setting for multi-catalog / cross-account access setups?

danielcmessias · 2022-06-27T09:03:26Z

website/docs/r/lakeformation_resource_lf_tags.html.markdown

+  lf_tag {
+    key   = "left"
+    value = "aintree"
+  }


what happens if you assign the same key twice but with diferent values, e.g.

lf_tag { key = "left" value = "luffield" } lf_tag { key = "left" value = "aintree" }

Does the LF api catch this and error, or will it silently pick one?

It catches the error:

Error: creating AWS Lake Formation Resource LF Tags ({ LFTags: [{ TagKey: "tf-acc-test-1974583174579168682-2", TagValues: ["vale"] },{ TagKey: "tf-acc-test-1974583174579168682", TagValues: ["luffield"] },{ TagKey: "tf-acc-test-1974583174579168682", TagValues: ["woodcote"] }], Resource: { TableWithColumns: { ColumnNames: ["event","timestamp"], DatabaseName: "tf-acc-test-1974583174579168682", Name: "tf-acc-test-1974583174579168682" } } }): InvalidInputException: Provided list of tags contains duplicate tag key

danielcmessias · 2022-06-27T09:11:29Z

internal/service/lakeformation/resource_lf_tags.go

+		}
+
+		if failures.Len() > 0 {
+			return names.DiagError(names.LakeFormation, names.ErrActionCreating, resourceLFTags, "", failures.ErrorOrNil())


should we be looking to update the terraform state for all/any of the successful lf_tag applies before exiting with error if there are any failures?

We could return a diag.Warning for each LFTag that fails and only an overall diag.Error if failures.Len() == len(input. LFTags)?

The trouble is that we don't update state in Create (except a tiny bit). I'm not sure how helpful it would be in that these are very fast operations and on the next successful Read, the state will be fixed. I think we need to stick with the framework and let Read handle state updates unless there's some really compelling reason. This is all ForceNew, so it's going to Delete and Create again in order to do any creating. That may cause problems but updating state in Create won't fix that.

I don't know where else we're doing something like this so it might be a first. Stack of failure warning, as requested. In my testing, I haven't seen exactly what a failure is. For example, if you send duplicates of a key, you get an error back on the whole AddLFTagsToResource operation so this logic is never reached. Hopefully it won't confuse people.

.github/workflows/terraform_provider.yml

ewbankkit · 2022-06-27T19:57:52Z

% make testacc TESTS="TestAccLakeFormation_serial/ResourceLFTags" PKG=lakeformation
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lakeformation/... -v -count 1 -parallel 20 -run='TestAccLakeFormation_serial/ResourceLFTags'  -timeout 180m
=== RUN   TestAccLakeFormation_serial
=== RUN   TestAccLakeFormation_serial/ResourceLFTags
=== RUN   TestAccLakeFormation_serial/ResourceLFTags/basic
=== RUN   TestAccLakeFormation_serial/ResourceLFTags/database
=== RUN   TestAccLakeFormation_serial/ResourceLFTags/databaseMultiple
=== RUN   TestAccLakeFormation_serial/ResourceLFTags/table
=== RUN   TestAccLakeFormation_serial/ResourceLFTags/tableWithColumns
--- PASS: TestAccLakeFormation_serial (155.17s)
    --- PASS: TestAccLakeFormation_serial/ResourceLFTags (155.17s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/basic (20.17s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/database (30.77s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/databaseMultiple (33.93s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/table (34.12s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/tableWithColumns (36.18s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/lakeformation	159.543s

YakDriver · 2022-06-27T20:05:50Z

Looks great, few small comments/questions

Thank you for taking a look!

stevenayers · 2022-06-28T06:57:15Z

@YakDriver this looks fantastic, thanks for closing the loop here.

ewbankkit

LGTM 🚀.

% make testacc TESTS="TestAccLakeFormation_serial/ResourceLFTags" PKG=lakeformation
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lakeformation/... -v -count 1 -parallel 20 -run='TestAccLakeFormation_serial/ResourceLFTags'  -timeout 180m
=== RUN   TestAccLakeFormation_serial
=== RUN   TestAccLakeFormation_serial/ResourceLFTags
=== RUN   TestAccLakeFormation_serial/ResourceLFTags/tableWithColumns
=== RUN   TestAccLakeFormation_serial/ResourceLFTags/basic
=== RUN   TestAccLakeFormation_serial/ResourceLFTags/database
=== RUN   TestAccLakeFormation_serial/ResourceLFTags/databaseMultiple
=== RUN   TestAccLakeFormation_serial/ResourceLFTags/table
--- PASS: TestAccLakeFormation_serial (161.23s)
    --- PASS: TestAccLakeFormation_serial/ResourceLFTags (161.23s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/tableWithColumns (37.66s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/basic (17.18s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/database (35.28s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/databaseMultiple (33.85s)
        --- PASS: TestAccLakeFormation_serial/ResourceLFTags/table (37.26s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/lakeformation	166.215s

github-actions · 2022-07-01T01:11:10Z

This functionality has been released in v4.21.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 · 2022-07-31T02:42:49Z

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.

YakDriver added 7 commits June 24, 2022 16:23

provider: Add resource_lf_tags resource

c84a86f

names: Add setting error action

5d6ed3a

lf/lf_tag/test: Minor cleanup

ccbd2fa

test/lakeformation: Add new resource tests

ff92519

docs/lakeformation: Add new resource docs

fdddae6

lakeformation: Add new resource_lf_tags resource

3fd6b84

tests/lakeformation: Add tests for new resource_lf_tags resource

b562d1f

Add changelog

68add08

YakDriver mentioned this pull request Jun 24, 2022

Lake Formation Tag-Based Access Control related resources and data sources #19640

Closed

YakDriver added 3 commits June 24, 2022 16:33

lakeformation: Linted windows

c4e2026

lakeformation/res_lf_tags: Switch to one value

ebdd2a9

ci/lint: Fixes

cab3233

github-actions bot added github_actions Pull requests that update Github_actions code linter Pertains to changes to or issues with the various linters. repository Repository modifications; GitHub Actions, developer docs, issue templates, codeowners, changelog. labels Jun 24, 2022

Linteresting

06bb566

danielcmessias reviewed Jun 27, 2022

View reviewed changes

ewbankkit reviewed Jun 27, 2022

View reviewed changes

.github/workflows/terraform_provider.yml Show resolved Hide resolved

YakDriver added 2 commits June 27, 2022 16:07

lakeformation: Review fixes

9a6c5fd

lakeformation/resource_lf_tags: Rework failures as warning

ec66549

lakeformation/resource_lf_tags: Simplify

8ba4b3d

ewbankkit approved these changes Jun 28, 2022

View reviewed changes

YakDriver merged commit f39c0f7 into main Jun 28, 2022

YakDriver deleted the f-lakeformation_lf_tag_resource branch June 28, 2022 16:20

github-actions bot added this to the v4.21.0 milestone Jun 28, 2022

github-actions bot pushed a commit that referenced this pull request Jun 28, 2022

Update CHANGELOG.md for #25565

cceaf8e

github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lakeformation: Add new aws_lakeformation_resource_lf_tags resource #25565

lakeformation: Add new aws_lakeformation_resource_lf_tags resource #25565

YakDriver commented Jun 24, 2022 •

edited

Loading

YakDriver commented Jun 24, 2022 •

edited

Loading

danielcmessias left a comment

danielcmessias Jun 27, 2022

YakDriver Jun 27, 2022

danielcmessias Jun 27, 2022

YakDriver Jun 27, 2022

danielcmessias Jun 27, 2022

ewbankkit Jun 27, 2022

YakDriver Jun 27, 2022 •

edited

Loading

YakDriver Jun 27, 2022

ewbankkit commented Jun 27, 2022

YakDriver commented Jun 27, 2022

stevenayers commented Jun 28, 2022

ewbankkit left a comment

github-actions bot commented Jul 1, 2022

github-actions bot commented Jul 31, 2022

lakeformation: Add new aws_lakeformation_resource_lf_tags resource #25565

lakeformation: Add new aws_lakeformation_resource_lf_tags resource #25565

Conversation

YakDriver commented Jun 24, 2022 • edited Loading

Community Note

YakDriver commented Jun 24, 2022 • edited Loading

danielcmessias left a comment

Choose a reason for hiding this comment

danielcmessias Jun 27, 2022

Choose a reason for hiding this comment

YakDriver Jun 27, 2022

Choose a reason for hiding this comment

danielcmessias Jun 27, 2022

Choose a reason for hiding this comment

YakDriver Jun 27, 2022

Choose a reason for hiding this comment

danielcmessias Jun 27, 2022

Choose a reason for hiding this comment

ewbankkit Jun 27, 2022

Choose a reason for hiding this comment

YakDriver Jun 27, 2022 • edited Loading

Choose a reason for hiding this comment

YakDriver Jun 27, 2022

Choose a reason for hiding this comment

ewbankkit commented Jun 27, 2022

YakDriver commented Jun 27, 2022

stevenayers commented Jun 28, 2022

ewbankkit left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 1, 2022

github-actions bot commented Jul 31, 2022

YakDriver commented Jun 24, 2022 •

edited

Loading

YakDriver commented Jun 24, 2022 •

edited

Loading

YakDriver Jun 27, 2022 •

edited

Loading