Amazon GuardDuty supports exporting findings to an Amazon S3 bucket #10920 #12398

shaepe-nc · 2020-03-15T15:35:00Z

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

Release note for CHANGELOG:

**New Resource**: `aws_guardduty_publishing_destination` (#10920)

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAwsGuardDutyPublishingDestination'

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -count 1 -parallel 20 -run=TestAccAwsGuardDutyPublishingDestination -timeout 120m
?       github.com/terraform-providers/terraform-provider-aws   [no test files]
=== RUN   TestAccAwsGuardDutyPublishingDestination_basic
--- PASS: TestAccAwsGuardDutyPublishingDestination_basic (105.17s)
=== RUN   TestAccAwsGuardDutyPublishingDestination_import
--- PASS: TestAccAwsGuardDutyPublishingDestination_import (106.17s)
PASS
...

…ashicorp#10920)

ewbankkit · 2020-03-16T11:47:20Z

aws/resource_aws_guardduty_publishing_destination.go

+			},
+			"destination_arn": {
+				Type:     schema.TypeString,
+				Required: true,


Add ValidateFunc: validateArn,?

ewbankkit · 2020-03-16T11:47:53Z

aws/resource_aws_guardduty_publishing_destination.go

+			},
+			"kms_key_arn": {
+				Type:     schema.TypeString,
+				Required: true,


Add ValidateFunc: validateArn,?

ewbankkit · 2020-03-16T11:48:28Z

aws/resource_aws_guardduty_publishing_destination.go

+			"destination_type": {
+				Type:     schema.TypeString,
+				Optional: true,
+				Default:  "S3",


Can use AWS SDK-provided constant guardduty.DestinationTypeS3.

ewbankkit · 2020-03-16T11:52:00Z

aws/resource_aws_guardduty_publishing_destination.go

+		}
+		resp, err := conn.DescribePublishingDestination(input)
+		if err != nil {
+			return nil, "failed", err


Maybe add as

// Constants not currently provided by the AWS Go SDK const ( guardDutyPublishingStatusFailed = "FAILED" )

at top of file?

ewbankkit · 2020-03-16T11:53:26Z

aws/resource_aws_guardduty_publishing_destination.go

+func resourceAwsGuardDutyPublishingDestinationRead(d *schema.ResourceData, meta interface{}) error {
+	conn := meta.(*AWSClient).guarddutyconn
+
+	destination_id, detector_id, err_state_read := decodeGuardDutyPublishDestinationID(d.Id())


Prefer camelCase over snake_case here (and others).

ewbankkit · 2020-03-16T11:56:12Z

aws/resource_aws_guardduty_publishing_destination.go

+	}
+
+	log.Printf("[DEBUG] Delete GuardDuty publishing destination: %s", input)
+	_, err := conn.DeletePublishingDestination(&input)


What happens if the PublishingDestination has been deleted outside Terraform?
Do we need the if isAWSErr(err, guardduty.ErrCodeBadRequestException,... check here?

miguelaferreira · 2020-06-11T07:09:13Z

Hi @shaepe. Thanks for this PR, my team is looking to use this resource and I'm wondering if you will have the time to address the review from @ewbankkit, otherwise we could do that to make progress on your PR.

* Merged latest master changes and resolved conflicts

ewbankkit · 2020-06-14T19:52:40Z

aws/resource_aws_guardduty_publishing_destination.go

+				Type:     schema.TypeString,
+				Optional: true,
+				Default:  guardduty.DestinationTypeS3,
+			},


Add

ValidateFunc: validation.StringInSlice([]string{ guardduty.DestinationTypeS3, }, false),

ewbankkit · 2020-06-14T19:54:03Z

aws/resource_aws_guardduty_publishing_destination.go

+			guardduty.PublishingStatusPublishing, err)
+	}
+
+	d.SetId(fmt.Sprintf("%s:%s", d.Get("detector_id"), *output.DestinationId))


d.SetId(fmt.Sprintf("%s:%s", d.Get("detector_id"), aws.StringValue(output.DestinationId)))

ewbankkit · 2020-06-14T19:55:59Z

aws/resource_aws_guardduty_publishing_destination.go

+		return fmt.Errorf("Creating GuardDuty publishing destination failed: %s", err.Error())
+	}
+
+	stateConf := &resource.StateChangeConf{


Could you please move this code to aws/internal/service/guardduty/waiter and follow the pattern there?
See #12840.

bendehaan · 2020-08-03T09:36:34Z

@ewbankkit moved to new PR since original author is unresponsive: #13894

ewbankkit · 2020-08-04T20:15:03Z

Closing in favour of #13894.
@shaepe Thanks for your work on this.

ghost · 2020-09-04T17:10:19Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

shaepe-nc added 3 commits March 15, 2020 15:17

Implementation of resource new resource type for GuardDuty S3 Export (h…

789114a

…ashicorp#10920)

Added tests and documentation

9014a40

Fixed test namings

c88e753

shaepe-nc requested a review from a team March 15, 2020 15:35

shaepe-nc added 3 commits March 15, 2020 17:13

Fixed linter issues and removed explicit import test case

a3d8238

Fixed HCL formatting in documentation

a8bb772

Fixed some namings and sidebar link

3826b50

ewbankkit reviewed Mar 16, 2020

View reviewed changes

bendehaan mentioned this pull request Jun 11, 2020

Update/refactor publishing destination shaepe-nc/terraform-provider-aws#1

Merged

Update/refactor publishing destination (#1)

35f730c

* Merged latest master changes and resolved conflicts

ghost added size/XXL Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Jun 14, 2020

shaepe-nc added 2 commits June 14, 2020 12:17

Merged from Upstream master and squashed commits

dd8331b

Merged SDK files again and cleaned aws.erb from merge conflict strings

8aa3366

ghost added the dependencies Used to indicate dependency changes. label Jun 14, 2020

shaepe-nc and others added 2 commits June 14, 2020 12:31

Delete defaults.go

5e5fc08

Removed changes from vendor subdirectory and synched with master.

2f72640

ghost removed the size/XXL Managed by automation to categorize the size of a PR. label Jun 14, 2020

ghost added the size/XL Managed by automation to categorize the size of a PR. label Jun 14, 2020

ewbankkit reviewed Jun 14, 2020

View reviewed changes

bendehaan mentioned this pull request Jun 15, 2020

Refactor GuardDuty to waiter pattern shaepe-nc/terraform-provider-aws#2

Open

bflad added new-resource Introduces a new resource. and removed dependencies Used to indicate dependency changes. needs-triage Waiting for first response or review from a maintainer. labels Jun 15, 2020

bendehaan mentioned this pull request Jun 23, 2020

Amazon GuardDuty supports exporting findings to an Amazon S3 bucket #10920 #13894

Merged

anGie44 force-pushed the master branch from 54db70d to 48ba2f7 Compare July 21, 2020 18:27

ewbankkit closed this Aug 4, 2020

ghost locked and limited conversation to collaborators Sep 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Amazon GuardDuty supports exporting findings to an Amazon S3 bucket #10920 #12398

Amazon GuardDuty supports exporting findings to an Amazon S3 bucket #10920 #12398

shaepe-nc commented Mar 15, 2020

ewbankkit Mar 16, 2020

ewbankkit Mar 16, 2020

ewbankkit Mar 16, 2020

ewbankkit Mar 16, 2020

ewbankkit Mar 16, 2020

ewbankkit Mar 16, 2020

miguelaferreira commented Jun 11, 2020

ewbankkit Jun 14, 2020

ewbankkit Jun 14, 2020

ewbankkit Jun 14, 2020

bendehaan commented Aug 3, 2020

ewbankkit commented Aug 4, 2020

ghost commented Sep 4, 2020

Amazon GuardDuty supports exporting findings to an Amazon S3 bucket #10920 #12398

Amazon GuardDuty supports exporting findings to an Amazon S3 bucket #10920 #12398

Conversation

shaepe-nc commented Mar 15, 2020

Community Note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miguelaferreira commented Jun 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bendehaan commented Aug 3, 2020

ewbankkit commented Aug 4, 2020

ghost commented Sep 4, 2020