add aws_dataexchange_event_action #40552

TSKnuhtsen · 2024-12-13T03:36:14Z

Description

This PR adds a resource for aws_dataexchange_event_action

Relations

Output from Acceptance Testing

thyge@u5f9f1f071ee45a:~/development/clone/terraform-provider-aws$ make testacc TESTS=TestAccDataExchangeEventAction_* PKG=dataexchange
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/dataexchange/... -v -count 1 -parallel 20 -run='TestAccDataExchangeEventAction_*'  -timeout 360m
2024/12/12 20:29:23 Initializing Terraform AWS Provider...
=== RUN   TestAccDataExchangeEventAction_basic
--- PASS: TestAccDataExchangeEventAction_basic (0.67s)
=== RUN   TestAccDataExchangeEventAction_update
--- PASS: TestAccDataExchangeEventAction_update (0.61s)
=== RUN   TestAccDataExchangeEventAction_disappears
--- PASS: TestAccDataExchangeEventAction_disappears (0.66s)
=== RUN   TestAccDataExchangeEventAction_keyPattern
--- PASS: TestAccDataExchangeEventAction_keyPattern (0.62s)
=== RUN   TestAccDataExchangeEventAction_encryption
--- PASS: TestAccDataExchangeEventAction_encryption (0.60s)
=== RUN   TestAccDataExchangeEventAction_kmsKeyEncryption
--- PASS: TestAccDataExchangeEventAction_kmsKeyEncryption (0.64s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/dataexchange	4.067s
...

github-actions · 2024-12-13T03:36:25Z

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.

jar-b

Thanks for your submission!

I added some comments with suggestions on how to adopt some standard provider conventions. A couple other points not specific to the existing code:

You'll need to run make gen to get the service package generator to pick up the presence of this new resource. I suspect the existing acceptance tests are not actually running, because this resource is not currently registered correctly and that should be failing any test configuration which references it. https://hashicorp.github.io/terraform-provider-aws/add-a-new-resource/#register-resource-to-the-provider
The registry documentation for this resource is missing and will need to be included as part of this PR (if you used skaff to generate this resource it should have also generated a corresponding file in the website subfolder). https://hashicorp.github.io/terraform-provider-aws/add-a-new-resource/#create-documentation-for-the-resource
A new-resource changelog entry should also be included. https://hashicorp.github.io/terraform-provider-aws/changelog-process/#new-resource
The source code for the aws_dataexchange_job resource is also included in this PR. That can be removed in favor of add aws_dataexchange_job #40551 .

jar-b · 2024-12-13T20:04:11Z

internal/service/dataexchange/event_action_test.go

+	bucketName := strconv.Itoa(int(time.Now().UnixNano()))
+
+	if _, okAcc := os.LookupEnv("TF_ACC"); !okAcc {
+		t.Skipf("TF_ACC must be set")
+	}
+
+	cfg, err := config.LoadDefaultConfig(ctx)
+	if err != nil {
+		t.Error(err)
+	}
+	stsConn := sts.NewFromConfig(cfg)
+	identity, err := stsConn.GetCallerIdentity(ctx, &sts.GetCallerIdentityInput{})
+
+	dataSetId, err := helperAccEventActionGetReceivedDataSet(ctx, awstypes.AssetTypeS3Snapshot)
+	if err != nil {
+		t.Error(err)
+	}
+
+	if dataSetId == "" {
+		return
+	}


I'm having trouble identifying the purpose of this setup - is there a way this can be incorporated into the test configuration itself, rather than manual steps outside the scope of the test run?

jar-b · 2024-12-13T20:05:51Z

internal/service/dataexchange/event_action_test.go

+resource "aws_s3_bucket" "test" {
+  bucket = "%s"
+  force_destroy = true
+}
+
+resource "aws_s3_bucket_policy" "test" {
+  bucket = aws_s3_bucket.test.id
+  policy = data.aws_iam_policy_document.test.json
+}
+
+data "aws_iam_policy_document" "test" {
+  statement {
+    principals {
+      type        = "Service"
+      identifiers = ["dataexchange.amazonaws.com"]
+    }
+
+    actions = [
+      "s3:PutObject",
+      "s3:PutObjectAcl",
+    ]
+
+    resources = [
+      "${aws_s3_bucket.test.arn}/*",
+    ]
+
+    condition {
+      test     = "StringEquals"
+      variable = "aws:SourceAccount"
+
+      values = [
+        "%s"
+      ]
+    }
+  }
+}


Configuration shared across tests can be moved into its own helper function (e.g. testAccEventActionConfigBase) and then merged with test-specific config via the acctest.ConfigCompose helper function.

jar-b · 2024-12-13T20:06:48Z

internal/service/dataexchange/event_action_test.go

+	if _, okAcc := os.LookupEnv("TF_ACC"); !okAcc {
+		t.Skipf("TF_ACC must be set")
+	}


This is already implicitly handled and does not need to be included in each test body.

jar-b · 2024-12-13T20:37:18Z

internal/service/dataexchange/event_action.go

+	input, diags := r.buildCreateInput(ctx, data)
+	if diags.HasError() {
+		resp.Diagnostics.Append(diags...)
+		return
+	}


Rather than custom methods to construct the input, AutoFlex should be used. At a high level, this process uses reflection to convert between the Terraform and AWS data structures. More information available here:

https://hashicorp.github.io/terraform-provider-aws/data-handling-and-conversion/#autoflex-for-terraform-plugin-framework-preferred

jar-b · 2024-12-13T20:38:55Z

internal/service/dataexchange/event_action.go

+		return
+	}
+
+	resp.Diagnostics.Append(r.buildModelFromOutput(ctx, out, &state)...)


Similar to the comment about, this should use the AutoFlex Flatten function rather than custom logic to translate between AWS and Terraform data structures.

jar-b · 2024-12-13T20:41:15Z

internal/service/dataexchange/event_action.go

+		_, err := conn.DeleteEventAction(ctx, &dataexchange.DeleteEventActionInput{
+			EventActionId: state.ID.ValueStringPointer(),
+		})


In most cases, the Update method should not directly call a Delete API. Instead, the attribute that triggers a replacement should be marked with a RequiresReplace() plan modifier.
https://developer.hashicorp.com/terraform/plugin/framework/resources/plan-modification#requiresreplace

internal/service/dataexchange/event_action_test.go

jar-b · 2024-12-13T20:44:55Z

internal/service/dataexchange/event_action_test.go

+			_, err := conn.GetEventAction(ctx, &dataexchange.GetEventActionInput{
+				EventActionId: aws.String(rs.Primary.ID),
+			})


The finder function findEventActionByID can be used here in place of the call directly to the Get API. The same is true of the check for existence below.

TSKnuhtsen added 2 commits December 12, 2024 21:21

add aws_dataexchange_job

3f11626

aws_dataexchange_event_action

82d8236

TSKnuhtsen requested a review from a team as a code owner December 13, 2024 03:36

github-actions bot added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/dataexchange Issues and PRs that pertain to the dataexchange service. needs-triage Waiting for first response or review from a maintainer. labels Dec 13, 2024

justinretzolk added new-resource Introduces a new resource. and removed needs-triage Waiting for first response or review from a maintainer. labels Dec 13, 2024

jar-b reviewed Dec 13, 2024

View reviewed changes

Addresses review comments for aws_dataexchange_event_action

5f8b5d1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add aws_dataexchange_event_action #40552

add aws_dataexchange_event_action #40552

TSKnuhtsen commented Dec 13, 2024

github-actions bot commented Dec 13, 2024

jar-b left a comment

jar-b Dec 13, 2024

jar-b Dec 13, 2024

jar-b Dec 13, 2024

jar-b Dec 13, 2024

jar-b Dec 13, 2024

jar-b Dec 13, 2024

jar-b Dec 13, 2024

add aws_dataexchange_event_action #40552

Are you sure you want to change the base?

add aws_dataexchange_event_action #40552

Conversation

TSKnuhtsen commented Dec 13, 2024

Description

Relations

Output from Acceptance Testing

github-actions bot commented Dec 13, 2024

Community Note

jar-b left a comment

Choose a reason for hiding this comment

jar-b Dec 13, 2024

Choose a reason for hiding this comment

jar-b Dec 13, 2024

Choose a reason for hiding this comment

jar-b Dec 13, 2024

Choose a reason for hiding this comment

jar-b Dec 13, 2024

Choose a reason for hiding this comment

jar-b Dec 13, 2024

Choose a reason for hiding this comment

jar-b Dec 13, 2024

Choose a reason for hiding this comment

jar-b Dec 13, 2024

Choose a reason for hiding this comment