Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/aws_inspector_assessment_target: subscribe-to-event #3957

Closed
wants to merge 1 commit into from

Conversation

s-maj
Copy link
Contributor

@s-maj s-maj commented Mar 28, 2018

Based on PR: hashicorp/terraform#13534
Fixes: #843
Fixed tests and added retries to avoid SNS Topic policy eventual consistency issues.

make testacc TEST=./aws TESTARGS='-run=TestAccAWSInspectorTemplate'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInspectorTemplate -timeout 120m
=== RUN   TestAccAWSInspectorTemplateEventSubscriptions_basic
--- PASS: TestAccAWSInspectorTemplateEventSubscriptions_basic (47.16s)
=== RUN   TestAccAWSInspectorTemplateEventSubscriptions_update
--- PASS: TestAccAWSInspectorTemplateEventSubscriptions_update (128.64s)
=== RUN   TestAccAWSInspectorTemplate_basic
--- PASS: TestAccAWSInspectorTemplate_basic (57.17s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	233.018s

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Mar 28, 2018
@radeksimko radeksimko added service/inspector Issues and PRs that pertain to the inspector service. enhancement Requests to existing resources that expand the functionality or scope. labels Mar 29, 2018
@vpadronblanco
Copy link
Contributor

vpadronblanco commented Apr 9, 2018

Can we get this merged? I have use of this as well

@s-maj s-maj changed the title AWS Inspector subscribe-to-event resource/aws_inspector_assessment_target: subscribe-to-event May 18, 2018
@joaquinrinaudo-olx
Copy link

+1: We also need SNS subscription to Inspector.

@humayunjamal
Copy link

+1: Please merge

@gileshinchcliff
Copy link

Any update on this?

@s-maj
Copy link
Contributor Author

s-maj commented Aug 27, 2018

Hi @bflad!
Would be possible to merge this or something is still missing here?

@bribot0225
Copy link

Would love to see this merged as well.

return fmt.Sprintf(testAccAWSInspectorTemplateAssessmentConfig, "", "")
}

func testAccAWSInspectorTemplateAssessmentConfigTwoEventSubscritpions() string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function name is misspelled (Subscritpions), should probably be testAccAWSInspectorTemplateAssessmentConfigTwoEventSubscriptions

%s
}`

var AWSInspectorTwoEventSubscriptions = `
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these don't need to be exported

"event": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validateSubscribeToEvent,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use validation.StringInSlice instead

@@ -119,3 +228,68 @@ func resourceAwsInspectorAssessmentTemplateDelete(d *schema.ResourceData, meta i

return nil
}

// validateSubscribeToEvent validates the string is a known keyword
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove this once you swap validation to validation.StringInSlice


Provides a Inspector assessment target

## Example Usage

```hcl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should stay, the language of the example is hcl

},
},
},
Set: func(v interface{}) int {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary, the hash by default will contain all the keys.

},
"topic_arn": {
Type: schema.TypeString,
Required: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

// substractEventSubscriptions return elements of 'a' which are not contained in 'b'
func substractEventSubscriptions(a []map[string]interface{}, b []map[string]interface{}) (result []map[string]interface{}) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

schema.Set has a Difference function that would make this cleaner, can probably eliminate it

return
}

func containsEventSubscription(s []map[string]interface{}, e map[string]interface{}) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

schema.Set has a Contains function that would make this cleaner, can probably eliminate it

@CaseyLabs
Copy link

Hi there, what's the status of this merge request? I've just ran into this issue as well, where I'm unable to associate an AWS Inspector template with an SNS topic via Terraform.

@someeodtech
Copy link

Any status on resolving conflicts & finishing this merge?

@frederiksf
Copy link

Any updates here?

@chilliblast
Copy link

Hi all

Is there any update on this as I have a pending need for this functionality also.

Thanks

@rafaelmagu
Copy link

@s-maj will you continue working on this branch or has this PR been abandoned?

@dharada1
Copy link
Contributor

we need this

@joshpavel
Copy link

joshpavel commented Jun 14, 2019

FYI, my workaround:

resource "null_resource" "inspector_sns" {

  provisioner "local-exec" {
    command = "aws --region ${data.aws_region.current.name} inspector subscribe-to-event --resource-arn ${aws_inspector_assessment_template.inspector_assessment_template.arn} --event FINDING_REPORTED --topic-arn ${aws_sns_topic.inspector.arn}"
  }

  depends_on = [
    "aws_lambda_function.inspector_reports",
    "aws_sns_topic.inspector"
  ]
}

@bflad
Copy link
Contributor

bflad commented Dec 20, 2019

Hi folks 👋 Since there has been no activity with this pull request, especially in response to the requested changes from a year ago and the current merge conflicts, we are going to close this. If the original author or a community member has the ambition to continue working on this, please submit a new pull request and the maintainers will take a fresh look. Thanks.

@fedecz
Copy link

fedecz commented Mar 5, 2020

I submitted PR #12261 to address this issue. I'd appreciate if you thumb'd it up for visibility. Thanks 👍

@ghost
Copy link

ghost commented Mar 6, 2020

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!

@ghost ghost locked and limited conversation to collaborators Mar 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/inspector Issues and PRs that pertain to the inspector service. size/L Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Aws_inspector_assessment_template ability to send findings to SNS topic.