From 538083973ede38cbf1d643f54d121c973b897af5 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Aug 2023 11:37:17 -0400 Subject: [PATCH 1/6] r/aws_securityhub_account: Remove default value for 'control_finding_generator' and mark as Computed. --- internal/service/securityhub/account.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/securityhub/account.go b/internal/service/securityhub/account.go index 93e59f2f6bc..f4215e4f59e 100644 --- a/internal/service/securityhub/account.go +++ b/internal/service/securityhub/account.go @@ -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": { From 4c4486b649b3ef9ff89295dc78b21e90fd4a01cd Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Aug 2023 11:37:32 -0400 Subject: [PATCH 2/6] Add CHANGELOG entry. --- .changelog/#####.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/#####.txt diff --git a/.changelog/#####.txt b/.changelog/#####.txt new file mode 100644 index 00000000000..36992055470 --- /dev/null +++ b/.changelog/#####.txt @@ -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 +``` \ No newline at end of file From 733ca6838585aa8d4295eda27dddef1879694c70 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Aug 2023 13:01:21 -0400 Subject: [PATCH 3/6] r/aws_securityhub_account: Call Read, not Update, from Create. --- internal/service/securityhub/account.go | 27 ++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/internal/service/securityhub/account.go b/internal/service/securityhub/account.go index f4215e4f59e..9dbf945bf45 100644 --- a/internal/service/securityhub/account.go +++ b/internal/service/securityhub/account.go @@ -94,8 +94,20 @@ 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 { @@ -135,9 +147,14 @@ func resourceAccountUpdate(ctx context.Context, d *schema.ResourceData, meta int var diags diag.Diagnostics 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)), + input := &securityhub.UpdateSecurityHubConfigurationInput{} + + if d.HasChange("auto_enable_controls") { + input.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) From 30b4c510f37ebf66624c305d81fd3be2948e4ec8 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Aug 2023 13:29:00 -0400 Subject: [PATCH 4/6] r/aws_securityhub_account: Add 'testAccAccount_removeControlFindingGeneratorDefaultValue'. --- internal/service/securityhub/account_test.go | 54 +++++++++++++++---- .../service/securityhub/securityhub_test.go | 3 +- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/internal/service/securityhub/account_test.go b/internal/service/securityhub/account_test.go index 49f61769a5b..73921693536 100644 --- a/internal/service/securityhub/account_test.go +++ b/internal/service/securityhub/account_test.go @@ -100,21 +100,19 @@ func testAccAccount_full(t *testing.T) { CheckDestroy: testAccCheckAccountDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccAccountConfig_basic, + Config: testAccAccountConfig_full(true, "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, "control_finding_generator", "STANDARD_CONTROL"), ), }, { - Config: testAccAccountConfig_full, + Config: testAccAccountConfig_full(false, "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, "control_finding_generator", "SECURITY_CONTROL"), ), }, }, @@ -152,6 +150,40 @@ 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" + + 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", "SECURITY_CONTROL"), + resource.TestCheckResourceAttr(resourceName, "auto_enable_controls", "true"), + ), + }, + { + 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] @@ -207,10 +239,12 @@ 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) } -` diff --git a/internal/service/securityhub/securityhub_test.go b/internal/service/securityhub/securityhub_test.go index 42cf0495a83..189af9b1c0c 100644 --- a/internal/service/securityhub/securityhub_test.go +++ b/internal/service/securityhub/securityhub_test.go @@ -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, From 5e17ad4e9769efe25f8bd1d5389f84974c021c61 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Aug 2023 13:52:45 -0400 Subject: [PATCH 5/6] r/aws_securityhub_account: Tests passing in GovCloud. --- internal/service/securityhub/account.go | 7 ++--- internal/service/securityhub/account_test.go | 27 ++++++++++++++------ 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/internal/service/securityhub/account.go b/internal/service/securityhub/account.go index 9dbf945bf45..aa44cefc065 100644 --- a/internal/service/securityhub/account.go +++ b/internal/service/securityhub/account.go @@ -104,7 +104,6 @@ func resourceAccountCreate(ctx context.Context, d *schema.ResourceData, meta int if err != nil { return sdkdiag.AppendErrorf(diags, "updating Security Hub Account (%s): %s", d.Id(), err) } - } return append(diags, resourceAccountRead(ctx, d, meta)...) @@ -147,10 +146,8 @@ func resourceAccountUpdate(ctx context.Context, d *schema.ResourceData, meta int var diags diag.Diagnostics conn := meta.(*conns.AWSClient).SecurityHubConn(ctx) - input := &securityhub.UpdateSecurityHubConfigurationInput{} - - if d.HasChange("auto_enable_controls") { - input.AutoEnableControls = aws.Bool(d.Get("auto_enable_controls").(bool)) + input := &securityhub.UpdateSecurityHubConfigurationInput{ + AutoEnableControls: aws.Bool(d.Get("auto_enable_controls").(bool)), } if d.HasChange("control_finding_generator") { diff --git a/internal/service/securityhub/account_test.go b/internal/service/securityhub/account_test.go index 73921693536..8da51974e1a 100644 --- a/internal/service/securityhub/account_test.go +++ b/internal/service/securityhub/account_test.go @@ -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" @@ -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) }, @@ -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"), ), }, @@ -94,24 +99,25 @@ 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_full(true, "STANDARD_CONTROL"), + Config: testAccAccountConfig_full(false, "STANDARD_CONTROL"), Check: resource.ComposeTestCheckFunc( testAccCheckAccountExists(ctx, resourceName), - 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(false, "SECURITY_CONTROL"), + Config: testAccAccountConfig_full(true, "SECURITY_CONTROL"), Check: resource.ComposeTestCheckFunc( testAccCheckAccountExists(ctx, resourceName), - resource.TestCheckResourceAttr(resourceName, "auto_enable_controls", "false"), + resource.TestCheckResourceAttr(resourceName, "auto_enable_controls", "true"), resource.TestCheckResourceAttr(resourceName, "control_finding_generator", "SECURITY_CONTROL"), ), }, @@ -154,6 +160,11 @@ func testAccAccount_migrateV0(t *testing.T) { 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) }, @@ -171,9 +182,10 @@ func testAccAccount_removeControlFindingGeneratorDefaultValue(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", controlFindingGeneratorExpectedValue), resource.TestCheckResourceAttr(resourceName, "auto_enable_controls", "true"), ), + ExpectNonEmptyPlan: expectNonEmptyPlan, }, { ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, @@ -242,7 +254,6 @@ resource "aws_securityhub_account" "test" { func testAccAccountConfig_full(autoEnableControls bool, controlFindingGenerator string) string { return fmt.Sprintf(` resource "aws_securityhub_account" "test" { - enable_default_standards = false control_finding_generator = %[2]q auto_enable_controls = %[1]t } From 3be7d9a22757ae98804283787f16441e42058395 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Aug 2023 14:29:33 -0400 Subject: [PATCH 6/6] Correct CHANGELOG entry file name. --- .changelog/{#####.txt => 33095.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{#####.txt => 33095.txt} (100%) diff --git a/.changelog/#####.txt b/.changelog/33095.txt similarity index 100% rename from .changelog/#####.txt rename to .changelog/33095.txt