r/aws_securityhub: Add aws_securityhub_accept_invitation resource

[kamsz]

That's a first take on adding new resources for security hub. Please let me know if that's the proper approach I've been thinking of. If that's alright, I'll add tests.

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" comments, they generate extra noise for pull request followers and do not help prioritize the request

Relates #6674

Release note for CHANGELOG:

New Security Hub resources - aws_securityhub_member, aws_securityhub_invite, aws_securityhub_accept_invitation

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

[kamsz]

@gazoakley

[iandone]

Any updates on when we can expect this to be reviewed/merged?

[gdavison]

Thanks for this PR, @kamsz, it's a great start. Before we review it, however, we will need tests for the functionality and documentation. We look forward to your update!

[gazoakley]

@gdavison @kamsz I wrote an acceptor a while back too, although I wasn't sure at the time how to deal with tests spanning multiple account (I think @bflad might have come up with a pattern for that since). There's an example doc already here:

https://github.com/gazoakley/terraform-provider-aws/blob/f-security-hub/website/docs/r/securityhub_invite_accepter.markdown

[bflad]

Hi @kamsz and @gazoakley 👋 There's a Contributing Guide section on cross-account acceptance testing. 👍 One other quick note here is that we would likely want to name this aws_securityhub_invite_accepter, to match GuardDuty and other similar resource naming.

[bflad]

Oh and the aws_securityhub_member resource has been merged into master, so that can be used as well in testing. 😄

[Zordrak]

Not clear who's running the show on securityhub cross-account membership. We've got this and #12684 . All focussing on invite_accepter, though not seeing an invite resource anywhere (securityhub needs member, invite & invite accepter).

Either way would really love to see movement; handling these things through a shell script provider is shonky at best.

[teamterraform]

Notification of Recent and Upcoming Changes to Contributions

Thank you for this contribution! There have been a few recent development changes that affect this pull request. We apologize for the inconvenience, especially if there have been long review delays up until now. Please note that this is automated message from an unmonitored account. See the FAQ for additional information on the maintainer team and review prioritization.

If you are unable to complete these updates, please leave a comment for the community and maintainers so someone can potentially continue the work. The maintainers will encourage other contributors to use the existing contribution as the base for additional changes as appropriate. Otherwise, contributions that do not receive updated code or comments from the original contributor may be closed in the future so the maintainers can focus on active items.

For the most up to date information about Terraform AWS Provider development, see the Contributing Guide. Additional technical debt changes can be tracked with the technical-debt label on issues.

As part of updating a pull request with these changes, the most current unit testing and linting will run. These may report issues that were not previously reported.

Terraform 0.12 Syntax

Reference: #8950
Reference: #14417

Version 3 and later of the Terraform AWS Provider, which all existing contributions would potentially be added, only supports Terraform 0.12 and later. Certain syntax elements of Terraform 0.11 and earlier show deprecation warnings during runs with Terraform 0.12. Documentation and test configurations, such as those including deprecated string interpolations (some_attribute = "${aws_service_thing.example.id}") should be updated to the newer syntax (some_attribute = aws_service_thing.example.id). Contribution testing will automatically fail on older syntax in the near future. Please see the referenced issues for additional information.

Action Required: Terraform Plugin SDK Version 2

Reference: #14551

The Terraform AWS Provider has been upgraded to the latest version of the Terraform Plugin SDK. Generally, most changes to contributions should only involve updating Go import paths in source code files. Please see the referenced issue for additional information.

Removal of website/aws.erb File

Reference: #14712

Any changes to the website/aws.erb file are no longer necessary and should be removed from this contribution to prevent merge issues in the near future when the file is removed from the repository. Please see the referenced issue for additional information.

Upcoming Change of Git Branch Naming

Reference: #14292

Development environments will need their upstream Git branch updated from master to main in the near future. Please see the referenced issue for additional information and scheduling.

Upcoming Change of GitHub Organization

Reference: #14715

This repository will be migrating from https://github.com/terraform-providers/terraform-provider-aws to https://github.com/hashicorp/terraform-provider-aws. No practitioner or developer action is anticipated and most GitHub functionality will automatically redirect to the new location. Go import paths including terraform-providers can remain for now. Please see the referenced issue for additional information and scheduling.

[lanejlanej]

FYI If a securityhub invitation is accepted by an out-of-band mechanism (eg. local-exec script), it causes terraform destroy to fail with following sort of error message:

• Error: Error deleting Security Hub member <account_id>: UnprocessedAccounts is not empty

[lorengordon]

@lanejlanej you can work around that by issuing the "disassociate from master" command as the destroy action for the local-exec provisioner.

• https://docs.aws.amazon.com/cli/latest/reference/securityhub/disassociate-from-master-account.html
• https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/securityhub.html#SecurityHub.Client.disassociate_from_master_account

here's our setup for the accepter:

resource null_resource accepter {
  provisioner local-exec {
    command = join(" ", local.create)
  }

  provisioner local-exec {
    when    = destroy
    command = self.triggers.destroy_command
  }

  provisioner local-exec {
    when    = destroy
    command = "python -c 'import time; time.sleep(5)'"
  }

  lifecycle {
    ignore_changes = [triggers["destroy_command"]]
  }

  triggers = {
    destroy_command = join(" ", local.destroy)
  }
}

[lanejlanej]

Hi Loren, Thanks. That works for me.

…

On Wed, Oct 7, 2020 at 3:03 PM Loren Gordon ***@***.***> wrote: @lanejlanej <https://github.com/lanejlanej> you can work around that by issuing the "disassociate from master" command as the destroy action for the local-exec provisioner. - https://docs.aws.amazon.com/cli/latest/reference/securityhub/disassociate-from-master-account.html - https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/securityhub.html#SecurityHub.Client.disassociate_from_master_account — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10003 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA2ZQMASVRPZJAACWQ7YVKDSJRYM7ANCNFSM4IT3QLZA> .

[ghost]

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

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

[lorengordon]

Fyi, the changelog and the docs say the resource name is aws_securityhub_invite_accepter but the code makes it look like it is really aws_securityhub_accept_invitation? Which is correct?

Edit: Looks like an artifact of the merge process. This work was updated in #12684, where it is indeed aws_securityhub_invite_accepter

[ghost]

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!