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.

Sign up for GitHub

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

Jump to bottom

r/lakeformation_permissions: Fix update bug #19817

Merged
merged 23 commits into from Jun 17, 2021
Merged

r/lakeformation_permissions: Fix update bug #19817

merged 23 commits into from Jun 17, 2021

Conversation

YakDriver
Copy link
Member

@YakDriver YakDriver commented Jun 15, 2021 •
edited

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 #19643
Closes #19465
Closes #19397
Closes #19409
Closes #19860

This PR address 3 problems:

  1. The order of column_names or exclude_column_names could cause diffs when reapplying (see Lakeformation table permissions need to be alphabetically ordered - otherwise aws_lakeformation_permissions stay out of sync #19465)
  2. Updating twc config caused an inconsistent result error (see aws_lakeformation_permissions inconsistent result after apply #19643)
  3. The resource previously may not have removed deleted resources from state (see Replace testAccCheck...Disappears Implementations with testAccCheckResourceDisappears #13527)

Output from acceptance testing (GovCloud):

--- PASS: TestAccAWSLakeFormation_serial (621.11s)
    --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings (62.28s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/dataSource (17.66s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/basic (14.84s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/disappears (15.05s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/withoutCatalogId (14.73s)
    --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions (160.16s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/database (23.30s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/disappears (83.41s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/basic (27.11s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/dataLocation (26.34s)
    --- PASS: TestAccAWSLakeFormation_serial/TablePermissions (118.90s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/tableName (23.88s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/tableWildcard (23.10s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/tableWildcardPermissions (23.63s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/implicitTablePermissions (24.34s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/selectPermissions (23.95s)
    --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions (144.08s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/tableWithColumns (66.52s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/columnWildcardExcludedColumnsPermissions (25.02s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/columnWildcardPermissions (28.10s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/implicitTableWithColumnsPermissions (24.43s)
    --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions (135.70s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/databaseDataSource (26.72s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/tableDataSource (27.41s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/tableWithColumnsDataSource (24.83s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/basicDataSource (27.93s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/dataLocationDataSource (28.80s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	622.403s

@github-actions github-actions bot added size/L Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/lakeformation Issues and PRs that pertain to the lakeformation service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Jun 15, 2021
@github-actions github-actions bot 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 Jun 16, 2021
This was referenced Jun 16, 2021
Closed
Closed
Closed
ewbankkit
ewbankkit approved these changes Jun 17, 2021
View reviewed changes
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 🚀.

Commercial
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSLakeFormation_serial'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLakeFormation_serial -timeout 180m
=== RUN   TestAccAWSLakeFormation_serial
=== RUN   TestAccAWSLakeFormation_serial/DataLakeSettings
=== RUN   TestAccAWSLakeFormation_serial/DataLakeSettings/basic
=== RUN   TestAccAWSLakeFormation_serial/DataLakeSettings/disappears
=== RUN   TestAccAWSLakeFormation_serial/DataLakeSettings/withoutCatalogId
=== RUN   TestAccAWSLakeFormation_serial/DataLakeSettings/dataSource
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions/basic
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions/dataLocation        
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions/database
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions/disappears
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions/implicitTablePermissions
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions/selectPermissions
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions/tableName
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions/tableWildcard
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions/tableWildcardPermissions
=== RUN   TestAccAWSLakeFormation_serial/TableWithColumnsPermissions
=== RUN   TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/columnWildcardExcludedColumnsPermissions
=== RUN   TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/columnWildcardPermissions
=== RUN   TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/implicitTableWithColumnsPermissions
=== RUN   TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/tableWithColumns
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions/tableDataSource
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions/tableWithColumnsDataSource
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions/basicDataSource
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions/dataLocationDataSource
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions/databaseDataSource
--- PASS: TestAccAWSLakeFormation_serial (605.13s)
    --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings (44.50s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/basic (12.02s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/disappears (10.82s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/withoutCatalogId (10.51s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/dataSource (11.15s)
    --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions (159.32s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/basic (21.95s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/dataLocation (27.88s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/database (23.02s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/disappears (86.48s)
    --- PASS: TestAccAWSLakeFormation_serial/TablePermissions (123.23s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/implicitTablePermissions (24.93s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/selectPermissions (23.23s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/tableName (26.52s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/tableWildcard (22.68s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/tableWildcardPermissions (25.87s)
    --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions (139.62s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/columnWildcardExcludedColumnsPermissions (24.28s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/columnWildcardPermissions (26.70s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/implicitTableWithColumnsPermissions (25.95s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/tableWithColumns (62.68s)
    --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions (138.46s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/tableDataSource (30.85s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/tableWithColumnsDataSource (28.31s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/basicDataSource (22.06s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/dataLocationDataSource (29.11s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/databaseDataSource (28.13s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	609.295s
GovCloud
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSLakeFormation_serial'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLakeFormation_serial -timeout 180m
=== RUN   TestAccAWSLakeFormation_serial
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions/basic
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions/dataLocation
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions/database
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions/disappears
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions/tableWildcardPermissions
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions/implicitTablePermissions
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions/selectPermissions
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions/tableName
=== RUN   TestAccAWSLakeFormation_serial/TablePermissions/tableWildcard
=== RUN   TestAccAWSLakeFormation_serial/TableWithColumnsPermissions
=== RUN   TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/columnWildcardExcludedColumnsPermissions
=== RUN   TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/columnWildcardPermissions
=== RUN   TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/implicitTableWithColumnsPermissions
=== RUN   TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/tableWithColumns
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions/basicDataSource
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions/dataLocationDataSource
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions/databaseDataSource
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions/tableDataSource
=== RUN   TestAccAWSLakeFormation_serial/DataSourcePermissions/tableWithColumnsDataSource
=== RUN   TestAccAWSLakeFormation_serial/DataLakeSettings
=== RUN   TestAccAWSLakeFormation_serial/DataLakeSettings/basic
=== RUN   TestAccAWSLakeFormation_serial/DataLakeSettings/disappears
=== RUN   TestAccAWSLakeFormation_serial/DataLakeSettings/withoutCatalogId
=== RUN   TestAccAWSLakeFormation_serial/DataLakeSettings/dataSource
--- PASS: TestAccAWSLakeFormation_serial (651.02s)
    --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions (166.47s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/basic (29.41s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/dataLocation (29.60s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/database (23.57s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/disappears (83.89s)
    --- PASS: TestAccAWSLakeFormation_serial/TablePermissions (135.03s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/tableWildcardPermissions (26.15s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/implicitTablePermissions (30.01s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/selectPermissions (25.24s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/tableName (24.54s)
        --- PASS: TestAccAWSLakeFormation_serial/TablePermissions/tableWildcard (29.08s)
    --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions (150.83s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/columnWildcardExcludedColumnsPermissions (25.07s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/columnWildcardPermissions (25.41s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/implicitTableWithColumnsPermissions (30.24s)
        --- PASS: TestAccAWSLakeFormation_serial/TableWithColumnsPermissions/tableWithColumns (70.11s)
    --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions (137.93s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/basicDataSource (24.52s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/dataLocationDataSource (30.67s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/databaseDataSource (28.41s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/tableDataSource (29.13s)
        --- PASS: TestAccAWSLakeFormation_serial/DataSourcePermissions/tableWithColumnsDataSource (25.20s)
    --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings (60.77s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/basic (15.54s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/disappears (14.82s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/withoutCatalogId (14.78s)
        --- PASS: TestAccAWSLakeFormation_serial/DataLakeSettings/dataSource (15.62s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	654.295s

@YakDriver YakDriver mentioned this pull request Jun 17, 2021
Closed
@YakDriver YakDriver added this to the v3.46.0 milestone Jun 17, 2021
ewbankkit and others added 7 commits June 17, 2021 11:53
@ewbankkit
r/aws_lakeformation_permissions: Fix 'testAccAWSLakeFormationPermissi…
314e02c 
…ons_dataLocation'.

Previous acceptance test output:

=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions/dataLocation
    testing_new.go:63: Error running post-test destroy, there may be dangling resources: exit status 1
        2021/06/17 09:38:54 [DEBUG] Using modified User-Agent: Terraform/0.12.31 HashiCorp-terraform-exec/0.13.3

        Error: error deregistering Lake Formation Resource (arn:aws:s3:::tf-acc-test-7389732714506454004): InvalidInputException: Must manually delete service-linked role to deregister last S3 location.

Acceptance test output:

% make testacc TEST=./aws TESTARGS='-run=TestAccAWSLakeFormation_serial/BasicPermissions/dataLocation'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLakeFormation_serial/BasicPermissions/dataLocation -timeout 180m
=== RUN   TestAccAWSLakeFormation_serial
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions
=== RUN   TestAccAWSLakeFormation_serial/BasicPermissions/dataLocation
--- PASS: TestAccAWSLakeFormation_serial (29.94s)
    --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions (29.94s)
        --- PASS: TestAccAWSLakeFormation_serial/BasicPermissions/dataLocation (29.94s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	33.278s
@YakDriver
i/lakeformation: Add filter unit tests
3d1a96c
@YakDriver
Merge branch 'b-lakeformation-19643' of https://github.com/hashicorp/…
8e5d8e8 
…terraform-provider-aws into b-lakeformation-19643
@YakDriver
i/lakeformation: Add explanation, extra safety check
261a493
@YakDriver
i/lakeformation: Add filter unit tests
2f46e1f
@YakDriver
i/lakeformation: Fix comments
8fa959a
@YakDriver
i/lakeformation: Fix comments
5bf5b84
@YakDriver YakDriver merged commit e22269b into main Jun 17, 2021
@YakDriver YakDriver deleted the b-lakeformation-19643 branch June 17, 2021 17:41
github-actions bot pushed a commit that referenced this pull request Jun 17, 2021
Update CHANGELOG.md for #19817
c149356
@github-actions
Copy link

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

@YakDriver YakDriver mentioned this pull request Jul 8, 2021
Merged
@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 Jul 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ewbankkit ewbankkit ewbankkit approved these changes

Assignees
No one assigned
Labels
documentation Introduces or discusses updates to documentation. service/lakeformation Issues and PRs that pertain to the lakeformation service. size/XL 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
Milestone
v3.46.0
2 participants
@YakDriver @ewbankkit