Skip to content

Commit

Permalink
Merge pull request #30736 from roberth-k/f-aws_guardduty_organization…
Browse files Browse the repository at this point in the history
…_configuration-auto_enable_organization_members

r/aws_guardduty_organization_configuration: add auto_enable_organization_members attribute
  • Loading branch information
ewbankkit authored May 4, 2023
2 parents 1509ac5 + 9202df8 commit 5478cf2
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 71 deletions.
7 changes: 7 additions & 0 deletions .changelog/30736.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:enhancement
resource/aws_guardduty_organization_configuration: Add `auto_enable_organization_members` attribute
```

```release-note:note
resource/aws_guardduty_organization_configuration: The `auto_enable` argument has been deprecated. Use the `auto_enable_organization_members` argument instead.
```
9 changes: 5 additions & 4 deletions internal/service/guardduty/guardduty_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,11 @@ func TestAccGuardDuty_serial(t *testing.T) {
"basic": testAccOrganizationAdminAccount_basic,
},
"OrganizationConfiguration": {
"basic": testAccOrganizationConfiguration_basic,
"s3Logs": testAccOrganizationConfiguration_s3logs,
"kubernetes": testAccOrganizationConfiguration_kubernetes,
"malwareProtection": testAccOrganizationConfiguration_malwareprotection,
"basic": testAccOrganizationConfiguration_basic,
"autoEnableOrganizationMembers": testAccOrganizationConfiguration_autoEnableOrganizationMembers,
"s3Logs": testAccOrganizationConfiguration_s3logs,
"kubernetes": testAccOrganizationConfiguration_kubernetes,
"malwareProtection": testAccOrganizationConfiguration_malwareprotection,
},
"ThreatIntelSet": {
"basic": testAccThreatIntelSet_basic,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ resource "aws_organizations_organization" "test" {
resource "aws_guardduty_detector" "test" {}
resource "aws_guardduty_organization_admin_account" "test" {
depends_on = [aws_organizations_organization.test]
depends_on = [aws_organizations_organization.test, aws_guardduty_detector.test]
admin_account_id = data.aws_caller_identity.current.account_id
}
Expand Down
50 changes: 46 additions & 4 deletions internal/service/guardduty/organization_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/aws/aws-sdk-go/service/guardduty"
"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
Expand All @@ -28,8 +29,19 @@ func ResourceOrganizationConfiguration() *schema.Resource {

Schema: map[string]*schema.Schema{
"auto_enable": {
Type: schema.TypeBool,
Required: true,
Type: schema.TypeBool,
Optional: true,
Computed: true,
ExactlyOneOf: []string{"auto_enable", "auto_enable_organization_members"},
Deprecated: "Use auto_enable_organization_members instead",
},

"auto_enable_organization_members": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ExactlyOneOf: []string{"auto_enable", "auto_enable_organization_members"},
ValidateFunc: validation.StringInSlice(guardduty.AutoEnableMembers_Values(), false),
},

"datasources": {
Expand Down Expand Up @@ -119,6 +131,35 @@ func ResourceOrganizationConfiguration() *schema.Resource {
ValidateFunc: validation.NoZeroValues,
},
},

CustomizeDiff: customdiff.Sequence(
func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
// When creating an organization configuration with AutoEnable=true,
// AWS will automatically set AutoEnableOrganizationMembers=NEW.
//
// When configuring AutoEnableOrganizationMembers=ALL or NEW,
// AWS will automatically set AutoEnable=true.
//
// This diff customization keeps things consistent when configuring
// the resource against deprecation advice from AutoEnableOrganizationMembers=ALL
// to AutoEnable=true, and it also removes the need to use
// AutoEnable in the resource update function.

if attr := d.GetRawConfig().GetAttr("auto_enable_organization_members"); attr.IsKnown() && !attr.IsNull() {
return d.SetNew("auto_enable", attr.AsString() != guardduty.AutoEnableMembersNone)
}

if attr := d.GetRawConfig().GetAttr("auto_enable"); attr.IsKnown() && !attr.IsNull() {
if attr.True() {
return d.SetNew("auto_enable_organization_members", guardduty.AutoEnableMembersNew)
} else {
return d.SetNew("auto_enable_organization_members", guardduty.AutoEnableMembersNone)
}
}

return nil
},
),
}
}

Expand All @@ -129,8 +170,8 @@ func resourceOrganizationConfigurationUpdate(ctx context.Context, d *schema.Reso
detectorID := d.Get("detector_id").(string)

input := &guardduty.UpdateOrganizationConfigurationInput{
AutoEnable: aws.Bool(d.Get("auto_enable").(bool)),
DetectorId: aws.String(detectorID),
AutoEnableOrganizationMembers: aws.String(d.Get("auto_enable_organization_members").(string)),
DetectorId: aws.String(detectorID),
}

if v, ok := d.GetOk("datasources"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil {
Expand Down Expand Up @@ -173,6 +214,7 @@ func resourceOrganizationConfigurationRead(ctx context.Context, d *schema.Resour
}

d.Set("auto_enable", output.AutoEnable)
d.Set("auto_enable_organization_members", output.AutoEnableOrganizationMembers)

if output.DataSources != nil {
if err := d.Set("datasources", []interface{}{flattenOrganizationDataSourceConfigurationsResult(output.DataSources)}); err != nil {
Expand Down
196 changes: 136 additions & 60 deletions internal/service/guardduty/organization_configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func testAccOrganizationConfiguration_basic(t *testing.T) {
Config: testAccOrganizationConfigurationConfig_autoEnable(true),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"),
resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NEW"),
resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"),
),
},
Expand All @@ -41,13 +42,112 @@ func testAccOrganizationConfiguration_basic(t *testing.T) {
Config: testAccOrganizationConfigurationConfig_autoEnable(false),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "auto_enable", "false"),
resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NONE"),
resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"),
),
},
},
})
}

func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T) {
ctx := acctest.Context(t)
detectorResourceName := "aws_guardduty_detector.test"
resourceName := "aws_guardduty_organization_configuration.test"

resource.Test(t, resource.TestCase{
PreCheck: func() {
acctest.PreCheck(ctx, t)
acctest.PreCheckOrganizationsAccount(ctx, t)
},
ErrorCheck: acctest.ErrorCheck(t, guardduty.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
// GuardDuty Organization Configuration cannot be deleted separately.
// Ensure parent resource is destroyed instead.
CheckDestroy: testAccCheckDetectorDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers("ALL"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "ALL"),
resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"),
resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers("NONE"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NONE"),
resource.TestCheckResourceAttr(resourceName, "auto_enable", "false"),
resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers("ALL"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "ALL"),
resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"),
resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccOrganizationConfigurationConfig_autoEnable(true),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NEW"),
resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"),
resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers("NONE"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NONE"),
resource.TestCheckResourceAttr(resourceName, "auto_enable", "false"),
resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccOrganizationConfigurationConfig_autoEnable(false),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NONE"),
resource.TestCheckResourceAttr(resourceName, "auto_enable", "false"),
resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func testAccOrganizationConfiguration_s3logs(t *testing.T) {
ctx := acctest.Context(t)
detectorResourceName := "aws_guardduty_detector.test"
Expand Down Expand Up @@ -183,53 +283,59 @@ func testAccOrganizationConfiguration_malwareprotection(t *testing.T) {
})
}

func testAccOrganizationConfigurationConfig_autoEnable(autoEnable bool) string {
return fmt.Sprintf(`
const testAccOrganizationConfigurationConfigBase = `
data "aws_caller_identity" "current" {}
data "aws_partition" "current" {}
resource "aws_organizations_organization" "test" {
aws_service_access_principals = ["guardduty.${data.aws_partition.current.dns_suffix}"]
feature_set = "ALL"
aws_service_access_principals = [
"guardduty.${data.aws_partition.current.dns_suffix}",
"malware-protection.guardduty.${data.aws_partition.current.dns_suffix}",
]
feature_set = "ALL"
}
resource "aws_guardduty_detector" "test" {}
resource "aws_guardduty_organization_admin_account" "test" {
depends_on = [aws_organizations_organization.test]
depends_on = [aws_organizations_organization.test, aws_guardduty_detector.test]
admin_account_id = data.aws_caller_identity.current.account_id
}
`

func testAccOrganizationConfigurationConfig_autoEnable(autoEnable bool) string {
return acctest.ConfigCompose(
testAccOrganizationConfigurationConfigBase,
fmt.Sprintf(`
resource "aws_guardduty_organization_configuration" "test" {
depends_on = [aws_guardduty_organization_admin_account.test]
auto_enable = %[1]t
detector_id = aws_guardduty_detector.test.id
}
`, autoEnable)
`, autoEnable))
}

func testAccOrganizationConfigurationConfig_s3Logs(autoEnable bool) string {
return fmt.Sprintf(`
data "aws_caller_identity" "current" {}
data "aws_partition" "current" {}
func testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers(value string) string {
return acctest.ConfigCompose(
testAccOrganizationConfigurationConfigBase,
fmt.Sprintf(`
resource "aws_guardduty_organization_configuration" "test" {
depends_on = [aws_guardduty_organization_admin_account.test]
resource "aws_organizations_organization" "test" {
aws_service_access_principals = ["guardduty.${data.aws_partition.current.dns_suffix}"]
feature_set = "ALL"
auto_enable_organization_members = %[1]q
detector_id = aws_guardduty_detector.test.id
}
resource "aws_guardduty_detector" "test" {}
resource "aws_guardduty_organization_admin_account" "test" {
depends_on = [aws_organizations_organization.test]
admin_account_id = data.aws_caller_identity.current.account_id
`, value))
}

func testAccOrganizationConfigurationConfig_s3Logs(autoEnable bool) string {
return acctest.ConfigCompose(
testAccOrganizationConfigurationConfigBase,
fmt.Sprintf(`
resource "aws_guardduty_organization_configuration" "test" {
depends_on = [aws_guardduty_organization_admin_account.test]
Expand All @@ -242,28 +348,13 @@ resource "aws_guardduty_organization_configuration" "test" {
}
}
}
`, autoEnable)
`, autoEnable))
}

func testAccOrganizationConfigurationConfig_kubernetes(autoEnable bool) string {
return fmt.Sprintf(`
data "aws_caller_identity" "current" {}
data "aws_partition" "current" {}
resource "aws_organizations_organization" "test" {
aws_service_access_principals = ["guardduty.${data.aws_partition.current.dns_suffix}"]
feature_set = "ALL"
}
resource "aws_guardduty_detector" "test" {}
resource "aws_guardduty_organization_admin_account" "test" {
depends_on = [aws_organizations_organization.test]
admin_account_id = data.aws_caller_identity.current.account_id
}
return acctest.ConfigCompose(
testAccOrganizationConfigurationConfigBase,
fmt.Sprintf(`
resource "aws_guardduty_organization_configuration" "test" {
depends_on = [aws_guardduty_organization_admin_account.test]
Expand All @@ -278,28 +369,13 @@ resource "aws_guardduty_organization_configuration" "test" {
}
}
}
`, autoEnable)
`, autoEnable))
}

func testAccOrganizationConfigurationConfig_malwareprotection(autoEnable bool) string {
return fmt.Sprintf(`
data "aws_caller_identity" "current" {}
data "aws_partition" "current" {}
resource "aws_organizations_organization" "test" {
aws_service_access_principals = ["guardduty.${data.aws_partition.current.dns_suffix}", "malware-protection.guardduty.${data.aws_partition.current.dns_suffix}"]
feature_set = "ALL"
}
resource "aws_guardduty_detector" "test" {}
resource "aws_guardduty_organization_admin_account" "test" {
depends_on = [aws_organizations_organization.test]
admin_account_id = data.aws_caller_identity.current.account_id
}
return acctest.ConfigCompose(
testAccOrganizationConfigurationConfigBase,
fmt.Sprintf(`
resource "aws_guardduty_organization_configuration" "test" {
depends_on = [aws_guardduty_organization_admin_account.test]
Expand All @@ -316,5 +392,5 @@ resource "aws_guardduty_organization_configuration" "test" {
}
}
}
`, autoEnable)
`, autoEnable))
}
Loading

0 comments on commit 5478cf2

Please sign in to comment.