New Resource: aws_guardduty_ipset #3161

atsushi-ishibashi · 2018-01-27T13:59:41Z

TF_ACC=1 go test ./aws -v -run=TestAccAwsGuardDutyIpset_basic -timeout 120m
=== RUN   TestAccAwsGuardDutyIpset_basic
--- PASS: TestAccAwsGuardDutyIpset_basic (164.15s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	164.185s

bflad

Overall this is looking pretty good! Let me know if you can or when you address these comments and I'll take another look. 👍

bflad · 2018-01-29T07:11:56Z

aws/resource_aws_guardduty_ipset.go

+ return fmt.Errorf("[WARN] Error waiting for GuardDuty IpSet status to be \"%s\": %s", guardduty.IpSetStatusActive, err)
+ }
+
+ d.SetId(*resp.IpSetId)


Can you make the resource ID fmt.Sprintf("%s:%s", detectorID, *resp.IpSetId) so we can also add the passthrough import please?

bflad · 2018-01-29T07:14:16Z

aws/resource_aws_guardduty_ipset_test.go

+`, testAccGuardDutyDetectorConfig_basic1, rName, rName, rName)
+}
+
+func testAccGuardDutyIpsetConfig_update(rName, modName string) string {


Nitpick: Can you make the Config_basic able to take in all the required inputs so we don't need two configuration functions please?

bflad · 2018-01-29T07:15:01Z

aws/validators.go

+
+func validateGuardDutyIpsetFormat(v interface{}, k string) (ws []string, errors []error) {
+ value := v.(string)
+ validType := []string{"TXT", "STIX", "OTX_CSV", "ALIEN_VAULT", "PROOF_POINT", "FIRE_EYE"}


Nitpick: Can you use the SDK provided constants please? e.g. guardduty.IpSetFormatAlienVault

bflad · 2018-01-29T07:15:45Z

website/docs/r/guardduty_ipset.html.markdown

+
+# aws_guardduty_ipset
+
+Provides a resource to manage a GuardDuty detector.


Copypasta: Should be GuardDuty IPSet

We might want to include the note from the GuardDuty API documentation here.

~> **Note:** Currently in GuardDuty, users from member accounts cannot upload and further manage IPSets. IPSets that are uploaded by the master account are imposed on GuardDuty functionality in its member accounts.

bflad · 2018-01-29T07:18:02Z

aws/resource_aws_guardduty_ipset_test.go

+ if !ok {
+ return fmt.Errorf("Not found: %s", name)
+ }
+


Generally we try to verify the resources actually exist in AWS in these. Once the ID is updated, you can do that here. 👍

bflad · 2018-01-29T07:26:52Z

aws/resource_aws_guardduty_ipset.go

+ }
+ return err
+ }
+


We need to d.Set("activate", *resp.Status == guardduty.IpSetStatusActive) here. We may want to consider renaming the attribute active.

atsushi-ishibashi · 2018-01-29T14:56:46Z

TF_ACC=1 go test ./aws -v -run=TestAccAwsGuardDutyIpset_* -timeout 120m
=== RUN   TestAccAwsGuardDutyIpset_basic
--- PASS: TestAccAwsGuardDutyIpset_basic (212.81s)
=== RUN   TestAccAwsGuardDutyIpset_import
--- FAIL: TestAccAwsGuardDutyIpset_import (114.30s)
	testing.go:513: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
		
		(map[string]string) {
		}
		
		
		(map[string]string) (len=1) {
		 (string) (len=11) "detector_id": (string) (len=32) "c8b0a11cff4d9f541ec460da3967be55"
		}
		
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	327.160s
make: *** [testacc] Error 1

I didn't understand how Importer works so could you help me? @bflad

bflad · 2018-01-29T16:07:19Z

@atsushi-ishibashi d.Set("detector_id", detectorId) in the read function from the value you get from parsing the ID 😄

atsushi-ishibashi · 2018-01-29T23:38:55Z

@bflad Thank you!

TF_ACC=1 go test ./aws -v -run=TestAccAwsGuardDutyIpset_import -timeout 120m
=== RUN   TestAccAwsGuardDutyIpset_import
--- PASS: TestAccAwsGuardDutyIpset_import (79.31s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	79.360s

bflad · 2018-01-30T13:28:35Z

Thanks so much @atsushi-ishibashi! This looks great and I'll be merging it momentarily.

make testacc TEST=./aws TESTARGS='-run=TestAccAwsGuardDutyIpset'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAwsGuardDutyIpset -timeout 120m
=== RUN   TestAccAwsGuardDutyIpset_basic
--- PASS: TestAccAwsGuardDutyIpset_basic (56.88s)
=== RUN   TestAccAwsGuardDutyIpset_import
--- PASS: TestAccAwsGuardDutyIpset_import (35.16s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	92.090s

FYI I will be adding one very minor commit to this, its needed to ensure the acceptance testing works when run in parallel testing environments (like our daily repository acceptance testing) since GuardDuty only allows one detector in each account at a time (check out: https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_guardduty_test.go). We don't expect most folks to know or worry about this most of the time since we're very appreciative of make testacc runners already! 🥇

atsushi-ishibashi · 2018-01-30T13:34:41Z

@bflad I got it. Thank you!

bflad · 2018-02-09T22:29:11Z

This has been released in terraform-provider-aws version 1.9.0. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-04-08T17:20:31Z

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!

New Reaource: guardduty_ipset

d332664

ghost added the size/L Managed by automation to categorize the size of a PR. label Jan 27, 2018

atsushi-ishibashi changed the title ~~New Resource: guardduty_ipset~~ New Resource: aws_guardduty_ipset Jan 27, 2018

bflad added new-resource Introduces a new resource. service/guardduty Issues and PRs that pertain to the guardduty service. labels Jan 28, 2018

bflad reviewed Jan 29, 2018

View reviewed changes

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Jan 29, 2018

reflect reviews

803e0d3

ghost added the size/L Managed by automation to categorize the size of a PR. label Jan 29, 2018

fix importer

c77a96c

ghost added the size/L Managed by automation to categorize the size of a PR. label Jan 29, 2018

radeksimko removed the waiting-response Maintainers are waiting on response from community or contributor. label Jan 30, 2018

bflad merged commit c77a96c into hashicorp:master Jan 30, 2018

bflad added a commit that referenced this pull request Jan 30, 2018

Update CHANGELOG for #3161

980e6bb

bflad added this to the v1.9.0 milestone Jan 30, 2018

ghost locked and limited conversation to collaborators Apr 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: aws_guardduty_ipset #3161

New Resource: aws_guardduty_ipset #3161

atsushi-ishibashi commented Jan 27, 2018 •

edited

Loading

bflad left a comment

bflad Jan 29, 2018

bflad Jan 29, 2018

bflad Jan 29, 2018

bflad Jan 29, 2018

bflad Jan 29, 2018

bflad Jan 29, 2018

bflad Jan 29, 2018

atsushi-ishibashi commented Jan 29, 2018

bflad commented Jan 29, 2018

atsushi-ishibashi commented Jan 29, 2018

bflad commented Jan 30, 2018

atsushi-ishibashi commented Jan 30, 2018

bflad commented Feb 9, 2018

ghost commented Apr 8, 2020


		# aws_guardduty_ipset

		Provides a resource to manage a GuardDuty detector.

New Resource: aws_guardduty_ipset #3161

New Resource: aws_guardduty_ipset #3161

Conversation

atsushi-ishibashi commented Jan 27, 2018 • edited Loading

bflad left a comment

Choose a reason for hiding this comment

bflad Jan 29, 2018

Choose a reason for hiding this comment

bflad Jan 29, 2018

Choose a reason for hiding this comment

bflad Jan 29, 2018

Choose a reason for hiding this comment

bflad Jan 29, 2018

Choose a reason for hiding this comment

bflad Jan 29, 2018

Choose a reason for hiding this comment

bflad Jan 29, 2018

Choose a reason for hiding this comment

bflad Jan 29, 2018

Choose a reason for hiding this comment

atsushi-ishibashi commented Jan 29, 2018

bflad commented Jan 29, 2018

atsushi-ishibashi commented Jan 29, 2018

bflad commented Jan 30, 2018

atsushi-ishibashi commented Jan 30, 2018

bflad commented Feb 9, 2018

ghost commented Apr 8, 2020

atsushi-ishibashi commented Jan 27, 2018 •

edited

Loading