F/ New Resource Lakeformation OptIn #41611

nam054 · 2025-02-28T18:53:11Z

Description

Relations

References

Output from Acceptance Testing

> make testacc PKG=lakeformation TESTS=TestAccLakeFormationOptIn_
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.5 test ./internal/service/lakeformation/... -v -count 1 -parallel 20 -run='TestAccLakeFormationOptIn_'  -timeout 360m -vet=off
2025/03/03 02:43:10 Initializing Terraform AWS Provider...
=== RUN   TestAccLakeFormationOptIn_basic
=== PAUSE TestAccLakeFormationOptIn_basic
=== RUN   TestAccLakeFormationOptIn_disappears
=== PAUSE TestAccLakeFormationOptIn_disappears
=== CONT  TestAccLakeFormationOptIn_basic
=== CONT  TestAccLakeFormationOptIn_disappears
--- PASS: TestAccLakeFormationOptIn_basic (24.46s)
--- PASS: TestAccLakeFormationOptIn_disappears (24.47s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/lakeformation      31.403s

...

github-actions · 2025-02-28T18:53:24Z

Community Note

Voting for Prioritization

Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
Please see our prioritization guide for information on how we prioritize.
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

website/docs/r/lakeformation_opt_in.html.markdown

internal/service/lakeformation/opt_in_test.go

YakDriver · 2025-03-03T22:05:36Z

internal/service/lakeformation/opt_in.go

+		return
+	}
+
+	in := lakeformation.CreateLakeFormationOptInInput{}


Calling it an "opt in" blurs the line with a verb but the API is using it as a noun so it seems fine to me.

Could I get clarification? Rename the in to opt_in?
The in is part of the generated naming convention of the input.

Oh, sorry for the confusion! The name aws_lakeformation_opt_in is fine. I just meant that some might find "opt in" tricky since it can be both a verb and a noun. However, since AWS uses it as a noun (e.g., "create an opt-in"), it seems perfectly fine to me.

YakDriver · 2025-03-03T22:14:15Z

internal/service/lakeformation/opt_in_test.go

+				Resource: &awstypes.Resource{},
+			}
+
+			if v, ok := rs.Primary.Attributes["resource_data.0.catalog.0.id"]; ok && v != "" {


The way this works is a bit of a mess and if-else is one way to handle it. Idiomatic Go tries to avoid else entirely, however. I propose either switching to a switch or a separate return-early function that takes the attributes and returns the resource.

internal/service/lakeformation/opt_in_test.go

YakDriver · 2025-03-04T19:42:25Z

internal/service/lakeformation/opt_in_test.go

+			},
+		}
+
+		for path, constructor := range resourceConstructors {


That's a cool approach!

YakDriver · 2025-03-04T20:05:43Z

internal/service/lakeformation/opt_in_test.go

+
+		type resourceConstructor func(*terraform.ResourceState) *awstypes.Resource
+
+		resourceConstructors := map[string]resourceConstructor{


Could both of these be in a separate function so they can be shared by exists and destroy?

YakDriver

LGTM 🎉

% make testacc PKG=lakeformation TESTS=TestAccLakeFormationOptIn_
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.5 test ./internal/service/lakeformation/... -v -count 1 -parallel 20 -run='TestAccLakeFormationOptIn_'  -timeout 360m -vet=off
go: downloading github.com/aws/aws-sdk-go-v2/service/databasemigrationservice v1.50.0
go: downloading github.com/aws/aws-sdk-go-v2/service/eks v1.59.0
go: downloading github.com/aws/aws-sdk-go-v2/service/mediaconvert v1.68.0
go: downloading github.com/aws/aws-sdk-go-v2/service/pricing v1.33.0
go: downloading github.com/aws/aws-sdk-go-v2/service/ssm v1.57.0
2025/03/04 16:46:35 Initializing Terraform AWS Provider...
=== RUN   TestAccLakeFormationOptIn_basic
=== PAUSE TestAccLakeFormationOptIn_basic
=== RUN   TestAccLakeFormationOptIn_disappears
=== PAUSE TestAccLakeFormationOptIn_disappears
=== CONT  TestAccLakeFormationOptIn_basic
=== CONT  TestAccLakeFormationOptIn_disappears
--- PASS: TestAccLakeFormationOptIn_disappears (33.96s)
--- PASS: TestAccLakeFormationOptIn_basic (34.01s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/lakeformation	38.437s

github-actions · 2025-03-06T21:56:25Z

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

nam054 added 7 commits February 27, 2025 18:57

Get to no redline state

4faf641

No red lines in test file

3413e3d

Add markdown

0c5e36d

make gen

70f641b

Get to working state

ecc53fd

Merge branch 'main' into f-lakeformation-opt-in

7349c9a

Comment cleanup

20d67dd

nam054 added 2 commits February 28, 2025 12:58

Various Linter fixes; Change import func

bfa0525

Linter fixes

220d994

nam054 marked this pull request as ready for review March 3, 2025 06:30

nam054 requested a review from a team as a code owner March 3, 2025 06:30

nam054 added 6 commits March 2, 2025 22:33

Cleanup; more permissions

1570f2e

CleanUp

282d7c8

More cleanup

3b5ba5f

Another linter fix

d74ac03

Yet more fixes

3d2dabd

Add changelog

8750075

ewbankkit added the new-resource Introduces a new resource. label Mar 3, 2025

YakDriver reviewed Mar 3, 2025

View reviewed changes

website/docs/r/lakeformation_opt_in.html.markdown Outdated Show resolved Hide resolved

Typo

49c67bb

YakDriver reviewed Mar 3, 2025

View reviewed changes

Review Changes

ea8dcf1

YakDriver previously approved these changes Mar 4, 2025

View reviewed changes

Create helper function for cleaner code

ecd95f9

nam054 dismissed YakDriver’s stale review via ecd95f9 March 4, 2025 20:50

YakDriver approved these changes Mar 4, 2025

View reviewed changes

nam054 merged commit 1316652 into main Mar 4, 2025
44 checks passed

nam054 deleted the f-lakeformation-opt-in branch March 4, 2025 21:59

github-actions bot added this to the v5.90.0 milestone Mar 4, 2025

terraform-aws-provider bot pushed a commit that referenced this pull request Mar 4, 2025

Update CHANGELOG.md for #41611

bfd8bbd

github-actions bot removed the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Mar 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

F/ New Resource Lakeformation OptIn #41611

F/ New Resource Lakeformation OptIn #41611

nam054 commented Feb 28, 2025 •

edited

Loading

github-actions bot commented Feb 28, 2025

YakDriver Mar 3, 2025

nam054 Mar 3, 2025

YakDriver Mar 4, 2025 •

edited

Loading

YakDriver Mar 3, 2025

YakDriver Mar 4, 2025

YakDriver Mar 4, 2025

YakDriver left a comment

github-actions bot commented Mar 6, 2025


		type resourceConstructor func(terraform.ResourceState) awstypes.Resource

		resourceConstructors := map[string]resourceConstructor{

F/ New Resource Lakeformation OptIn #41611

F/ New Resource Lakeformation OptIn #41611

Conversation

nam054 commented Feb 28, 2025 • edited Loading

Description

Relations

References

Output from Acceptance Testing

github-actions bot commented Feb 28, 2025

Community Note

YakDriver Mar 3, 2025

Choose a reason for hiding this comment

nam054 Mar 3, 2025

Choose a reason for hiding this comment

YakDriver Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

YakDriver Mar 3, 2025

Choose a reason for hiding this comment

YakDriver Mar 4, 2025

Choose a reason for hiding this comment

YakDriver Mar 4, 2025

Choose a reason for hiding this comment

YakDriver left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 6, 2025

nam054 commented Feb 28, 2025 •

edited

Loading

YakDriver Mar 4, 2025 •

edited

Loading