r/aws_securityhub: Add aws_securityhub_product_subscription resource #6921

gazoakley · 2018-12-19T07:52:28Z

Partly addresses #6674

Changes proposed in this pull request:

New Resource: aws_securityhub_product_subscription

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSecurityHub/ProductSubscription'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSSecurityHub/ProductSubscription -timeout 120m
=== RUN   TestAccAWSSecurityHub
=== RUN   TestAccAWSSecurityHub/ProductSubscription
=== RUN   TestAccAWSSecurityHub/ProductSubscription/basic
--- PASS: TestAccAWSSecurityHub (46.97s)
    --- PASS: TestAccAWSSecurityHub/ProductSubscription (46.97s)
        --- PASS: TestAccAWSSecurityHub/ProductSubscription/basic (46.97s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	47.021s

bflad

Couple little things, then good to go, thanks @gazoakley!

bflad · 2018-12-19T20:15:13Z

aws/resource_aws_securityhub_product_subscription.go

+		Create: resourceAwsSecurityHubProductSubscriptionCreate,
+		Read:   resourceAwsSecurityHubProductSubscriptionRead,
+		Delete: resourceAwsSecurityHubProductSubscriptionDelete,
+		Importer: &schema.ResourceImporter{


This is currently missing acceptance testing and documentation. 😅 I think the current comment in the acceptance testing might be outdated since it looks like the Read function is implemented just fine below.

I left this in accidentally - but I might have a misunderstanding about the import process. Do resources need to be able to read/populate all attributes solely from the resource ID? The API doesn't have a way to read back the product_arn given a product subscription ARN (you can only check a subscription exists). It looks like I might be able to make an import test that works if I specify ImportStateVerifyIgnore: []string{"product_arn"}

Ah yikes, I was conflating product ARNs with product subscription ARNs. Yeah, for imports we need to set all attributes in the Read function or it'll show them as a difference after import (e.g. attribute: "" => "configured-value").

We have a few options since the API doesn't have a way to read it back:

Use ImportStateVerifyIgnore as you mention for now, but it'll have the difference problem noted above

Make the resource ID two parts, containing both ARNs, then Read has all the information it needs to search for the subscription and properly set product ARN as well

Or as a potentially crazy idea, if we can assume the production subscription ARN is always derivable from the product ARN (maybe looks possible from at least AWS subscriptions?), switch the resource identifier to the product ARN and calculate the subscription ARN

arn:aws:securityhub:us-west-2::product/aws/guardduty # Set AccountID # Replace Resource product with product-subscription arn:aws:securityhub:us-west-2:ACCOUNTID:product-subscription/aws/guardduty

I'd lean towards including both ARNs in the resource ID for ease, operator friendliness after import, and safer than trying to derive it.

I thought about option 3 previously and then ran away scared 🤣 (I think it's doable, but the resource would need to know the current account ID and region to build an ARN and it feels brittle). Option 2 sounds good to me - I'll look at it later today.

aws/resource_aws_securityhub_product_subscription.go

bflad · 2018-12-19T20:36:07Z

aws/resource_aws_securityhub_product_subscription_test.go

+					testAccCheckAWSSecurityHubProductSubscriptionExists("aws_securityhub_product_subscription.example"),
+				),
+			},
+			// Import is not supported, since the API to lookup product_arn from a product


{ ResourceName: "aws_securityhub_product_subscription.example", ImportState: true, ImportStateVerify: true, }

I've added an import test step, but I've need to use ImportStateVerifyIgnore:

{ ResourceName: "aws_securityhub_product_subscription.example", ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{"product_arn"}, },

bflad · 2018-12-19T20:38:35Z

website/docs/r/securityhub_product_subscription.markdown

+
+The following attributes are exported in addition to the arguments listed above:
+
+* `id` - The ARN of a resource that represents your subscription to the product that generates the findings that you want to import into Security Hub.


* `id` - The ARN of a resource that represents your subscription to the product that generates the findings that you want to import into Security Hub. ## Import Security Hub Product Subscriptions can be imported via the product ARN, e.g. ```sh $ terraform import aws_securityhub_product_subscription.example arn:aws:securityhub:us-east-1::product/aws/guardduty ```

The ID is the subscription ARN since that's the only thing I can test still exists - I've added an Import section describing it.

…t_subscription

gazoakley · 2018-12-21T07:41:55Z

Hi @bflad - updated to address review comments 👍

bflad

Thanks, @gazoakley! Looks great. 🚀 I'm going to merge this as-is and only do the minor testing thing below if its easy and works as expected.

bflad · 2018-12-21T13:43:10Z

aws/resource_aws_securityhub_product_subscription_test.go

+
+resource "aws_securityhub_product_subscription" "example" {
+  depends_on  = ["aws_securityhub_account.example"]
+  product_arn = "arn:aws:securityhub:${data.aws_region.current.name}:733251395267:product/alertlogic/althreatmanagement"


Just noting here for future posterity and not that we don't like our friends over at AlertLogic, but its a little bit of a bummer that the AWS services are automatically subscribed for acceptance testing purposes since they are effectively free, assumed stable for a long while, and doesn't involve an extra EULA. (I'm a little surprised the API doesn't require accepting the EULA first.)

I'll try to switch to using a PreConfig to disable something like GuardDuty and re-enable it here in the configuration, but if that doesn't go as well as I'd expect, the pricing for this service is fine for the (presumably single hour) time it would be billed.

🎉 7cdbfcf

bflad · 2018-12-21T23:10:37Z

This has been released in version 1.54.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-04-01T17:40:33Z

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!

r/aws_securityhub: Add aws_securityhub_product_subscription resource

20ec213

bflad added new-resource Introduces a new resource. service/securityhub Issues and PRs that pertain to the securityhub service. labels Dec 19, 2018

bflad reviewed Dec 19, 2018

View reviewed changes

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

gazoakley added 2 commits December 20, 2018 08:13

r/aws_securityhub: Address review comments

e0ee6c7

r/aws_securityhub: Address review comments for aws_securityhub_produc…

73f02d0

…t_subscription

ghost 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 Dec 21, 2018

ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Dec 21, 2018

bflad approved these changes Dec 21, 2018

View reviewed changes

bflad added this to the v1.54.0 milestone Dec 21, 2018

bflad merged commit 8a46bfb into hashicorp:master Dec 21, 2018

bflad added a commit that referenced this pull request Dec 21, 2018

Update CHANGELOG for #6921

78462cf

ghost locked and limited conversation to collaborators Apr 1, 2020

gazoakley deleted the f-security-hub-product branch April 7, 2020 16:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

r/aws_securityhub: Add aws_securityhub_product_subscription resource #6921

r/aws_securityhub: Add aws_securityhub_product_subscription resource #6921

gazoakley commented Dec 19, 2018 •

edited

Loading

bflad left a comment

bflad Dec 19, 2018

gazoakley Dec 20, 2018 •

edited

Loading

bflad Dec 20, 2018 •

edited

Loading

gazoakley Dec 20, 2018

bflad Dec 19, 2018

gazoakley Dec 20, 2018

bflad Dec 19, 2018

gazoakley Dec 20, 2018

gazoakley commented Dec 21, 2018

bflad left a comment

bflad Dec 21, 2018

bflad Dec 21, 2018

bflad commented Dec 21, 2018

ghost commented Apr 1, 2020


		The following attributes are exported in addition to the arguments listed above:

		* `id` - The ARN of a resource that represents your subscription to the product that generates the findings that you want to import into Security Hub.

r/aws_securityhub: Add aws_securityhub_product_subscription resource #6921

r/aws_securityhub: Add aws_securityhub_product_subscription resource #6921

Conversation

gazoakley commented Dec 19, 2018 • edited Loading

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gazoakley Dec 20, 2018 • edited Loading

Choose a reason for hiding this comment

bflad Dec 20, 2018 • edited Loading

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

gazoakley commented Dec 21, 2018

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bflad commented Dec 21, 2018

ghost commented Apr 1, 2020

gazoakley commented Dec 19, 2018 •

edited

Loading

gazoakley Dec 20, 2018 •

edited

Loading

bflad Dec 20, 2018 •

edited

Loading