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

r/aws_securityhub_account: Remove default value for control_finding_generator #33095

Merged
merged 7 commits into from
Aug 21, 2023
3 changes: 3 additions & 0 deletions .changelog/33095.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_securityhub_account: Remove default value (`SECURITY_CONTROL`) for `control_finding_generator` argument and mark as Computed
```
24 changes: 19 additions & 5 deletions internal/service/securityhub/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func ResourceAccount() *schema.Resource {
"control_finding_generator": {
Type: schema.TypeString,
Optional: true,
Default: securityhub.ControlFindingGeneratorSecurityControl,
Computed: true,
ValidateFunc: validation.StringInSlice(securityhub.ControlFindingGenerator_Values(), false),
},
"enable_default_standards": {
Expand Down Expand Up @@ -94,8 +94,19 @@ func resourceAccountCreate(ctx context.Context, d *schema.ResourceData, meta int

d.SetId(meta.(*conns.AWSClient).AccountID)

// auto_enable_controls has to be done from the update API
return append(diags, resourceAccountUpdate(ctx, d, meta)...)
if autoEnableControls := d.Get("auto_enable_controls").(bool); !autoEnableControls {
input := &securityhub.UpdateSecurityHubConfigurationInput{
AutoEnableControls: aws.Bool(autoEnableControls),
}

_, err := conn.UpdateSecurityHubConfigurationWithContext(ctx, input)

if err != nil {
return sdkdiag.AppendErrorf(diags, "updating Security Hub Account (%s): %s", d.Id(), err)
}
}

return append(diags, resourceAccountRead(ctx, d, meta)...)
}

func resourceAccountRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
Expand Down Expand Up @@ -136,8 +147,11 @@ func resourceAccountUpdate(ctx context.Context, d *schema.ResourceData, meta int
conn := meta.(*conns.AWSClient).SecurityHubConn(ctx)

input := &securityhub.UpdateSecurityHubConfigurationInput{
ControlFindingGenerator: aws.String(d.Get("control_finding_generator").(string)),
AutoEnableControls: aws.Bool(d.Get("auto_enable_controls").(bool)),
AutoEnableControls: aws.Bool(d.Get("auto_enable_controls").(bool)),
}

if d.HasChange("control_finding_generator") {
input.ControlFindingGenerator = aws.String(d.Get("control_finding_generator").(string))
}

_, err := conn.UpdateSecurityHubConfigurationWithContext(ctx, input)
Expand Down
75 changes: 60 additions & 15 deletions internal/service/securityhub/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"testing"

"github.com/aws/aws-sdk-go/aws/endpoints"
"github.com/aws/aws-sdk-go/service/securityhub"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/terraform"
Expand All @@ -20,6 +21,10 @@ import (
func testAccAccount_basic(t *testing.T) {
ctx := acctest.Context(t)
resourceName := "aws_securityhub_account.test"
controlFindingGeneratorDefaultValueFromAWS := "SECURITY_CONTROL"
if acctest.Partition() == endpoints.AwsUsGovPartitionID {
controlFindingGeneratorDefaultValueFromAWS = ""
}

resource.Test(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
Expand All @@ -32,7 +37,7 @@ func testAccAccount_basic(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckAccountExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, "enable_default_standards", "true"),
resource.TestCheckResourceAttr(resourceName, "control_finding_generator", "SECURITY_CONTROL"),
resource.TestCheckResourceAttr(resourceName, "control_finding_generator", controlFindingGeneratorDefaultValueFromAWS),
resource.TestCheckResourceAttr(resourceName, "auto_enable_controls", "true"),
),
},
Expand Down Expand Up @@ -94,27 +99,26 @@ func testAccAccount_full(t *testing.T) {
resourceName := "aws_securityhub_account.test"

resource.Test(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
// control_finding_generator not supported in AWS GovCloud.
PreCheck: func() { acctest.PreCheck(ctx, t); acctest.PreCheckPartitionNot(t, endpoints.AwsUsGovPartitionID) },
ErrorCheck: acctest.ErrorCheck(t, securityhub.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckAccountDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccAccountConfig_basic,
Config: testAccAccountConfig_full(false, "STANDARD_CONTROL"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAccountExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, "enable_default_standards", "true"),
resource.TestCheckResourceAttr(resourceName, "control_finding_generator", "SECURITY_CONTROL"),
resource.TestCheckResourceAttr(resourceName, "auto_enable_controls", "true"),
resource.TestCheckResourceAttr(resourceName, "auto_enable_controls", "false"),
resource.TestCheckResourceAttr(resourceName, "control_finding_generator", "STANDARD_CONTROL"),
),
},
{
Config: testAccAccountConfig_full,
Config: testAccAccountConfig_full(true, "SECURITY_CONTROL"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAccountExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, "enable_default_standards", "false"),
resource.TestCheckResourceAttr(resourceName, "control_finding_generator", "STANDARD_CONTROL"),
resource.TestCheckResourceAttr(resourceName, "auto_enable_controls", "false"),
resource.TestCheckResourceAttr(resourceName, "auto_enable_controls", "true"),
resource.TestCheckResourceAttr(resourceName, "control_finding_generator", "SECURITY_CONTROL"),
),
},
},
Expand Down Expand Up @@ -152,6 +156,46 @@ func testAccAccount_migrateV0(t *testing.T) {
})
}

// https://github.com/hashicorp/terraform-provider-aws/issues/33039 et al.
func testAccAccount_removeControlFindingGeneratorDefaultValue(t *testing.T) {
ctx := acctest.Context(t)
resourceName := "aws_securityhub_account.test"
controlFindingGeneratorExpectedValue := "SECURITY_CONTROL"
if acctest.Partition() == endpoints.AwsUsGovPartitionID {
controlFindingGeneratorExpectedValue = ""
}
expectNonEmptyPlan := acctest.Partition() == endpoints.AwsUsGovPartitionID

resource.Test(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, securityhub.EndpointsID),
CheckDestroy: testAccCheckAccountDestroy(ctx),
Steps: []resource.TestStep{
{
ExternalProviders: map[string]resource.ExternalProvider{
"aws": {
Source: "hashicorp/aws",
VersionConstraint: "5.13.0",
},
},
Config: testAccAccountConfig_basic,
Check: resource.ComposeTestCheckFunc(
testAccCheckAccountExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, "enable_default_standards", "true"),
resource.TestCheckResourceAttr(resourceName, "control_finding_generator", controlFindingGeneratorExpectedValue),
resource.TestCheckResourceAttr(resourceName, "auto_enable_controls", "true"),
),
ExpectNonEmptyPlan: expectNonEmptyPlan,
},
{
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Config: testAccAccountConfig_basic,
PlanOnly: true,
},
},
})
}

func testAccCheckAccountExists(ctx context.Context, n string) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
Expand Down Expand Up @@ -207,10 +251,11 @@ resource "aws_securityhub_account" "test" {
}
`

const testAccAccountConfig_full = `
func testAccAccountConfig_full(autoEnableControls bool, controlFindingGenerator string) string {
return fmt.Sprintf(`
resource "aws_securityhub_account" "test" {
enable_default_standards = false
control_finding_generator = "STANDARD_CONTROL"
auto_enable_controls = false
control_finding_generator = %[2]q
auto_enable_controls = %[1]t
}
`, autoEnableControls, controlFindingGenerator)
}
`
3 changes: 2 additions & 1 deletion internal/service/securityhub/securityhub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ func TestAccSecurityHub_serial(t *testing.T) {
"disappears": testAccAccount_disappears,
"EnableDefaultStandardsFalse": testAccAccount_enableDefaultStandardsFalse,
"MigrateV0": testAccAccount_migrateV0,
"full": testAccAccount_full,
"Full": testAccAccount_full,
"RemoveControlFindingGeneratorDefaultValue": testAccAccount_removeControlFindingGeneratorDefaultValue,
},
"Member": {
"basic": testAccMember_basic,
Expand Down