LF-Tag support for aws_lakeformation_permissions #19648

danielcmessias · 2021-06-03T16:28:18Z

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 #19640

Updates

21/04/2022: PR has been refactored into Service Packages (#21306). Massive thanks to @stevenayers for his work on this!

04/08/2021: I have added the next step for TBAC which is to support LFTagPolicy resource, allowing principals to be assigned permissions based on tag policy expression (as described here). Since this PR is still awaiting triage, I've comitted the new functionality here instead of opening a new PR because, personally, I think it's easier to review together. Happy to split this new functionality into a subsequent branch if you'd prefer! The addition is made in this commit.

03/08/2021: I have renamed 'policy tag' to 'LF Tag' everywhere, to ensure consistency with the AWS API and Console. The change is made in this commit.

To be reviewed AFTER #19523
This work depends on the aws_lakeformation_lf_tag resource in the linked PR, so this branch is a continuation of that one. Files Changed is going to be wrong, relevant commits are downstream of 5bf3443 (inclusive). Let me know if there's a different preferred practice for dependant PRs.

Adds the two new resources to aws_lakeformation_permissions that were made available with the TBAC workflow, LFTag and LFTagPolicy. This is essentially steps 4 and 5 in AWS TBAC documentation.

lf_tag

Allows granting ASSOCIATE and DESCRIBE permissions to Lake Formation Policy Tags, and reading those permissions from a data source.

Example:

resource "aws_lakeformation_permissions" "environment" {
  principal = aws_iam_role.some_role.arn

  permissions = ["ASSOCIATE", "DESCRIBE"]

  lf_tag {
    key = "Team"
    values = ["Sales", "Marketing"]
  }
}

lf_tag_resource

Allows granting LF-Tag policies that give permissions to principals based on a set of tag expressions.

Example:

resource "aws_lakeformation_permissions" "sales" {
  principal = aws_iam_role.sales_role.arn

  permissions = ["CREATE_TABLE", "DROP"]

  lf_tag_policy {
    resource_type = "DATABASE"

    expression {
      key = "Team"
      values = ["Sales"]
    }

    expression {
      key = "Environment"
      values = ["Dev", "Production"]
    }

  }
}

Output from acceptance testing:

$ make testacc TESTS=TestAccLakeFormation_serial 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'  -timeout 180m
=== RUN   TestAccLakeFormation_serial
=== RUN   TestAccLakeFormation_serial/PermissionsBasic
=== RUN   TestAccLakeFormation_serial/PermissionsBasic/disappears
=== RUN   TestAccLakeFormation_serial/PermissionsBasic/lfTag
=== RUN   TestAccLakeFormation_serial/PermissionsBasic/lfTagPolicy
=== RUN   TestAccLakeFormation_serial/PermissionsBasic/basic
=== RUN   TestAccLakeFormation_serial/PermissionsBasic/database
=== RUN   TestAccLakeFormation_serial/PermissionsBasic/databaseIAMAllowed
=== RUN   TestAccLakeFormation_serial/PermissionsBasic/databaseMultiple
=== RUN   TestAccLakeFormation_serial/PermissionsBasic/dataLocation
=== RUN   TestAccLakeFormation_serial/PermissionsDataSource
=== RUN   TestAccLakeFormation_serial/PermissionsDataSource/basic
=== RUN   TestAccLakeFormation_serial/PermissionsDataSource/database
=== RUN   TestAccLakeFormation_serial/PermissionsDataSource/dataLocation
=== RUN   TestAccLakeFormation_serial/PermissionsDataSource/lfTag
=== RUN   TestAccLakeFormation_serial/PermissionsDataSource/lfTagPolicy
=== RUN   TestAccLakeFormation_serial/PermissionsDataSource/table
=== RUN   TestAccLakeFormation_serial/PermissionsDataSource/tableWithColumns
=== RUN   TestAccLakeFormation_serial/PermissionsTable
=== RUN   TestAccLakeFormation_serial/PermissionsTable/multipleRoles
=== RUN   TestAccLakeFormation_serial/PermissionsTable/wildcardNoSelect
=== RUN   TestAccLakeFormation_serial/PermissionsTable/wildcardSelectOnly
=== RUN   TestAccLakeFormation_serial/PermissionsTable/wildcardSelectPlus
=== RUN   TestAccLakeFormation_serial/PermissionsTable/basic
=== RUN   TestAccLakeFormation_serial/PermissionsTable/iamAllowed
=== RUN   TestAccLakeFormation_serial/PermissionsTable/implicit
=== RUN   TestAccLakeFormation_serial/PermissionsTable/selectOnly
=== RUN   TestAccLakeFormation_serial/PermissionsTable/selectPlus
=== RUN   TestAccLakeFormation_serial/PermissionsTableWithColumns
=== RUN   TestAccLakeFormation_serial/PermissionsTableWithColumns/wildcardExcludedColumns
=== RUN   TestAccLakeFormation_serial/PermissionsTableWithColumns/wildcardSelectOnly
=== RUN   TestAccLakeFormation_serial/PermissionsTableWithColumns/wildcardSelectPlus
=== RUN   TestAccLakeFormation_serial/PermissionsTableWithColumns/basic
=== RUN   TestAccLakeFormation_serial/PermissionsTableWithColumns/implicit
=== RUN   TestAccLakeFormation_serial/LFTags
=== RUN   TestAccLakeFormation_serial/LFTags/basic
=== RUN   TestAccLakeFormation_serial/LFTags/disappears
=== RUN   TestAccLakeFormation_serial/LFTags/values
=== RUN   TestAccLakeFormation_serial/DataLakeSettings
=== RUN   TestAccLakeFormation_serial/DataLakeSettings/basic
=== RUN   TestAccLakeFormation_serial/DataLakeSettings/dataSource
=== RUN   TestAccLakeFormation_serial/DataLakeSettings/disappears
=== RUN   TestAccLakeFormation_serial/DataLakeSettings/withoutCatalogId
--- PASS: TestAccLakeFormation_serial (1156.95s)
    --- PASS: TestAccLakeFormation_serial/PermissionsBasic (306.83s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/disappears (90.26s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/lfTag (27.24s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/lfTagPolicy (27.40s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/basic (27.02s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/database (27.54s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/databaseIAMAllowed (48.14s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/databaseMultiple (27.91s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/dataLocation (31.32s)
    --- PASS: TestAccLakeFormation_serial/PermissionsDataSource (223.20s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/basic (36.85s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/database (30.48s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/dataLocation (33.84s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/lfTag (29.10s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/lfTagPolicy (28.95s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/table (32.47s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/tableWithColumns (31.50s)
    --- PASS: TestAccLakeFormation_serial/PermissionsTable (287.18s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTable/multipleRoles (29.34s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTable/wildcardNoSelect (27.58s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTable/wildcardSelectOnly (28.95s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTable/wildcardSelectPlus (29.35s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTable/basic (29.57s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTable/iamAllowed (50.94s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTable/implicit (29.93s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTable/selectOnly (31.91s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTable/selectPlus (29.60s)
    --- PASS: TestAccLakeFormation_serial/PermissionsTableWithColumns (197.43s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTableWithColumns/wildcardExcludedColumns (29.93s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTableWithColumns/wildcardSelectOnly (29.65s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTableWithColumns/wildcardSelectPlus (30.03s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTableWithColumns/basic (77.82s)
        --- PASS: TestAccLakeFormation_serial/PermissionsTableWithColumns/implicit (30.00s)
    --- PASS: TestAccLakeFormation_serial/LFTags (73.22s)
        --- PASS: TestAccLakeFormation_serial/LFTags/basic (20.11s)
        --- PASS: TestAccLakeFormation_serial/LFTags/disappears (17.62s)
        --- PASS: TestAccLakeFormation_serial/LFTags/values (35.49s)
    --- PASS: TestAccLakeFormation_serial/DataLakeSettings (69.10s)
        --- PASS: TestAccLakeFormation_serial/DataLakeSettings/basic (16.77s)
        --- PASS: TestAccLakeFormation_serial/DataLakeSettings/dataSource (18.53s)
        --- PASS: TestAccLakeFormation_serial/DataLakeSettings/disappears (17.01s)
        --- PASS: TestAccLakeFormation_serial/DataLakeSettings/withoutCatalogId (16.78s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/lakeformation	1158.457s

zhelding · 2021-10-19T16:37:12Z

Pull request #21306 has significantly refactored the AWS Provider codebase. As a result, most PRs opened prior to the refactor now have merge conflicts that must be resolved before proceeding.

Specifically, PR #21306 relocated the code for all AWS resources and data sources from a single aws directory to a large number of separate directories in internal/service, each corresponding to a particular AWS service. This separation of code has also allowed for us to simplify the names of underlying functions -- while still avoiding namespace collisions.

We recognize that many pull requests have been open for some time without yet being addressed by our maintainers. Therefore, we want to make it clear that resolving these conflicts in no way affects the prioritization of a particular pull request. Once a pull request has been prioritized for review, the necessary changes will be made by a maintainer -- either directly or in collaboration with the pull request author.

For a more complete description of this refactor, including examples of how old filepaths and function names correspond to their new counterparts: please refer to issue #20000.

For a quick guide on how to amend your pull request to resolve the merge conflicts resulting from this refactor and bring it in line with our new code patterns: please refer to our Service Package Refactor Pull Request Guide.

danielcmessias · 2022-02-22T11:02:28Z

Hi @zhelding @breathingdust! Any way I can find out where this sits with you guys in terms of prioritisation? Cheers :)

WarFox · 2022-03-04T16:56:30Z

Super critical feature from AWS to have in terraform. We've tested a build from this branch and it works great!! It would be awesome to get this merged into an official release 🚀 🚀 🚀

gh-jvirdee · 2022-03-21T04:34:46Z

@danielcmessias - Thank you for working on this feature. Could you please rebase the code so that it can be merged?

Also related:
Lake Formation Tag-Based Access Control related resources and data sources

danielcmessias · 2022-04-21T10:02:32Z

PR has been refactored into Service Packages (#21306). Massive thanks to @stevenayers for his work on this!

stevenayers · 2022-04-21T10:07:00Z

@zhelding & @ewbankkit conflicts sorted when you get a chance, thank you! ❤️

stevenayers · 2022-05-04T11:45:49Z

@justinretzolk did a review of this PR make it into Q2?

danielcmessias · 2022-06-21T20:26:52Z

@danielcmessias @stevenayers Do you mind rebasing this now that #19523 is merged?

@YakDriver Rebased!

YakDriver · 2022-06-21T20:45:48Z

@danielcmessias @stevenayers It looks like only a little cleanup is needed. Basically, this looks really good after a brief look. If it's good with you, I'll clean it up and we'll see if we can get it merged. If you are planning to make any changes, please coordinate with me. Thanks for all the great work!

stevenayers · 2022-06-21T20:48:10Z

@danielcmessias @stevenayers It looks like only a little cleanup is needed. Basically, this looks really good after a brief look. If it's good with you, I'll clean it up and we'll see if we can get it merged. If you are planning to make any changes, please coordinate with me. Thanks for all the great work!

No objections from me, go for it!

YakDriver · 2022-06-21T20:53:28Z

My impression is that this closes #19640 and I changed it to close it. If anyone objects or I'm missing something, let me know.

danielcmessias · 2022-06-21T20:59:11Z

My impression is that this closes #19640 and I changed it to close it. If anyone objects or I'm missing something, let me know.

@YakDriver
The only bit of that issue that isn't covered by the two PRs @stevenayers and I worked on is the bit that applies LF tags to Glue resources, e.g.

resource "aws_lakeformation_tag_resource" "" {
  catalog_id = "123456789"

  # A block to identify the Data Catalog Resource that the tag is being assigned to. Similar to aws_lakeformation_permissions
  database {
    #similar to aws_lakeformation_permissions
  }
  table {
    #similar to aws_lakeformation_permissions
  }
  table_with_columns {
    #similar to aws_lakeformation_permissions
  }

  # The tags that are being assigned
  lf_tags = [
    {
        catalog_id = "123456789"
        tag_key = "module"
        tag_values = ["A"]
      },
      {
        catalog_id = "123456789"
        tag_key = "region"
        tag_values = ["west"]
      }
  ]
}

To my knowledge, there is no open PR that adds that functionality.

But regarding this PR, no incoming changes from me, looking forward to getting it merged. Thanks for reviewing! :)

YakDriver

Looks good! 🎉

% make testacc TESTS="TestAccLakeFormation_serial/LFTags|TestAccLakeFormation_serial/PermissionsDataSource|TestAccLakeFormation_serial/PermissionsBasic" 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/LFTags|TestAccLakeFormation_serial/PermissionsDataSource|TestAccLakeFormation_serial/PermissionsBasic'  -timeout 180m
--- PASS: TestAccLakeFormation_serial (546.83s)
    --- PASS: TestAccLakeFormation_serial/PermissionsBasic (288.59s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/disappears (87.15s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/lfTag (23.81s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/lfTagPolicy (25.10s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/basic (24.03s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/database (23.92s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/databaseIAMAllowed (45.17s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/databaseMultiple (33.22s)
        --- PASS: TestAccLakeFormation_serial/PermissionsBasic/dataLocation (26.18s)
    --- PASS: TestAccLakeFormation_serial/PermissionsDataSource (198.13s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/basic (24.15s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/database (34.09s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/dataLocation (27.75s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/lfTag (24.42s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/lfTagPolicy (24.42s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/table (28.31s)
        --- PASS: TestAccLakeFormation_serial/PermissionsDataSource/tableWithColumns (34.99s)
    --- PASS: TestAccLakeFormation_serial/LFTags (60.11s)
        --- PASS: TestAccLakeFormation_serial/LFTags/basic (17.10s)
        --- PASS: TestAccLakeFormation_serial/LFTags/disappears (14.17s)
        --- PASS: TestAccLakeFormation_serial/LFTags/values (28.84s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/lakeformation	548.288s

github-actions · 2022-06-23T23:10:30Z

This functionality has been released in v4.20.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!

zerfahp · 2022-06-24T08:11:31Z

Excuse my ignorance but I can't see this brilliant enhancement on the changelog. I can see docs have been updated and I can't wait to get stuck into using this capability. Thanks for all the work to enable this. Ahead of cloudformation again.

github-actions · 2022-07-25T02:38:44Z

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.

danielcmessias requested a review from a team as a code owner June 3, 2021 16:28

ghost added size/XL Managed by automation to categorize the size of a PR. labels Jun 3, 2021

danielcmessias marked this pull request as draft June 3, 2021 16:28

danielcmessias force-pushed the f-aws_lakeformation_permissions-tag-support branch from e796126 to 5bf3443 Compare June 3, 2021 16:38

danielcmessias changed the title ~~Policy Tag support for aws_lakeformation_permissions support~~ Policy Tag support for aws_lakeformation_permissions Jun 3, 2021

danielcmessias marked this pull request as ready for review June 3, 2021 16:45

danielcmessias changed the title ~~Policy Tag support for aws_lakeformation_permissions~~ LF-Tag support for aws_lakeformation_permissions Aug 3, 2021

danielcmessias force-pushed the f-aws_lakeformation_permissions-tag-support branch 3 times, most recently from 255934f to 924eb29 Compare August 3, 2021 15:37

danielcmessias mentioned this pull request Aug 5, 2021

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

Closed

breathingdust added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Aug 31, 2021

stevenayers mentioned this pull request Apr 20, 2022

LF-Tag support for aws_lakeformation_permissions #24315

Closed

danielcmessias force-pushed the f-aws_lakeformation_permissions-tag-support branch from 75d7bc5 to 19a63f1 Compare April 21, 2022 10:02

Adding initial files

25fe426

danielcmessias force-pushed the f-aws_lakeformation_permissions-tag-support branch from 19a63f1 to 25fe426 Compare June 21, 2022 20:26

github-actions bot removed provider Pertains to the provider itself, rather than any interaction with AWS. waiting-response Maintainers are waiting on response from community or contributor. labels Jun 21, 2022

YakDriver added this to the Roadmap milestone Jun 21, 2022

lf: Clean up

310bcb1

lf: Lint fixes

5462217

github-actions bot added github_actions Pull requests that update Github_actions code repository Repository modifications; GitHub Actions, developer docs, issue templates, codeowners, changelog. labels Jun 21, 2022

YakDriver added 3 commits June 21, 2022 18:11

ci: Fix false positive issue

0375c0c

Add changelog

fa758dc

changelog: Undo change

e024c7e

YakDriver approved these changes Jun 22, 2022

View reviewed changes

YakDriver merged commit c297592 into hashicorp:main Jun 22, 2022

github-actions bot modified the milestones: Roadmap, v4.20.0 Jun 22, 2022

YakDriver mentioned this pull request Jun 24, 2022

lakeformation: Add new aws_lakeformation_resource_lf_tags resource #25565

Merged

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LF-Tag support for aws_lakeformation_permissions #19648

LF-Tag support for aws_lakeformation_permissions #19648

danielcmessias commented Jun 3, 2021 •

edited by YakDriver

Loading

zhelding commented Oct 19, 2021

danielcmessias commented Feb 22, 2022

WarFox commented Mar 4, 2022

gh-jvirdee commented Mar 21, 2022 •

edited

Loading

danielcmessias commented Apr 21, 2022 •

edited

Loading

stevenayers commented Apr 21, 2022 •

edited

Loading

stevenayers commented May 4, 2022

danielcmessias commented Jun 21, 2022

YakDriver commented Jun 21, 2022 •

edited

Loading

stevenayers commented Jun 21, 2022

YakDriver commented Jun 21, 2022

danielcmessias commented Jun 21, 2022 •

edited

Loading

YakDriver left a comment •

edited

Loading

github-actions bot commented Jun 23, 2022

zerfahp commented Jun 24, 2022

github-actions bot commented Jul 25, 2022

LF-Tag support for aws_lakeformation_permissions #19648

LF-Tag support for aws_lakeformation_permissions #19648

Conversation

danielcmessias commented Jun 3, 2021 • edited by YakDriver Loading

Community Note

Updates

lf_tag

lf_tag_resource

zhelding commented Oct 19, 2021

danielcmessias commented Feb 22, 2022

WarFox commented Mar 4, 2022

gh-jvirdee commented Mar 21, 2022 • edited Loading

danielcmessias commented Apr 21, 2022 • edited Loading

stevenayers commented Apr 21, 2022 • edited Loading

stevenayers commented May 4, 2022

danielcmessias commented Jun 21, 2022

YakDriver commented Jun 21, 2022 • edited Loading

stevenayers commented Jun 21, 2022

YakDriver commented Jun 21, 2022

danielcmessias commented Jun 21, 2022 • edited Loading

YakDriver left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jun 23, 2022

zerfahp commented Jun 24, 2022

github-actions bot commented Jul 25, 2022

danielcmessias commented Jun 3, 2021 •

edited by YakDriver

Loading

gh-jvirdee commented Mar 21, 2022 •

edited

Loading

danielcmessias commented Apr 21, 2022 •

edited

Loading

stevenayers commented Apr 21, 2022 •

edited

Loading

YakDriver commented Jun 21, 2022 •

edited

Loading

danielcmessias commented Jun 21, 2022 •

edited

Loading

YakDriver left a comment •

edited

Loading