From 52676cb0111f2954103b6361d41c304261584a7e Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 11 Oct 2023 15:49:25 -0400 Subject: [PATCH 01/12] r/aws_guardduty_organization_configuration: Alphabetize attributes. --- .../guardduty/organization_configuration.go | 70 +++++++++---------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/internal/service/guardduty/organization_configuration.go b/internal/service/guardduty/organization_configuration.go index ae09eea6d54..67c2d53f469 100644 --- a/internal/service/guardduty/organization_configuration.go +++ b/internal/service/guardduty/organization_configuration.go @@ -21,9 +21,9 @@ import ( // @SDKResource("aws_guardduty_organization_configuration") func ResourceOrganizationConfiguration() *schema.Resource { return &schema.Resource{ - CreateWithoutTimeout: resourceOrganizationConfigurationUpdate, + CreateWithoutTimeout: resourceOrganizationConfigurationPut, ReadWithoutTimeout: resourceOrganizationConfigurationRead, - UpdateWithoutTimeout: resourceOrganizationConfigurationUpdate, + UpdateWithoutTimeout: resourceOrganizationConfigurationPut, DeleteWithoutTimeout: schema.NoopContext, Importer: &schema.ResourceImporter{ @@ -38,7 +38,6 @@ func ResourceOrganizationConfiguration() *schema.Resource { ExactlyOneOf: []string{"auto_enable", "auto_enable_organization_members"}, Deprecated: "Use auto_enable_organization_members instead", }, - "auto_enable_organization_members": { Type: schema.TypeString, Optional: true, @@ -46,7 +45,6 @@ func ResourceOrganizationConfiguration() *schema.Resource { ExactlyOneOf: []string{"auto_enable", "auto_enable_organization_members"}, ValidateFunc: validation.StringInSlice(guardduty.AutoEnableMembers_Values(), false), }, - "datasources": { Type: schema.TypeList, Optional: true, @@ -54,20 +52,6 @@ func ResourceOrganizationConfiguration() *schema.Resource { MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "s3_logs": { - Type: schema.TypeList, - Optional: true, - Computed: true, - MaxItems: 1, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "auto_enable": { - Type: schema.TypeBool, - Required: true, - }, - }, - }, - }, "kubernetes": { Type: schema.TypeList, Optional: true, @@ -123,10 +107,23 @@ func ResourceOrganizationConfiguration() *schema.Resource { }, }, }, + "s3_logs": { + Type: schema.TypeList, + Optional: true, + Computed: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "auto_enable": { + Type: schema.TypeBool, + Required: true, + }, + }, + }, + }, }, }, }, - "detector_id": { Type: schema.TypeString, Required: true, @@ -166,12 +163,11 @@ func ResourceOrganizationConfiguration() *schema.Resource { } } -func resourceOrganizationConfigurationUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func resourceOrganizationConfigurationPut(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).GuardDutyConn(ctx) detectorID := d.Get("detector_id").(string) - input := &guardduty.UpdateOrganizationConfigurationInput{ AutoEnableOrganizationMembers: aws.String(d.Get("auto_enable_organization_members").(string)), DetectorId: aws.String(detectorID), @@ -187,7 +183,9 @@ func resourceOrganizationConfigurationUpdate(ctx context.Context, d *schema.Reso return sdkdiag.AppendErrorf(diags, "updating GuardDuty Organization Configuration (%s): %s", detectorID, err) } - d.SetId(detectorID) + if d.IsNewResource() { + d.SetId(detectorID) + } return append(diags, resourceOrganizationConfigurationRead(ctx, d, meta)...) } @@ -239,10 +237,6 @@ func expandOrganizationDataSourceConfigurations(tfMap map[string]interface{}) *g apiObject := &guardduty.OrganizationDataSourceConfigurations{} - if v, ok := tfMap["s3_logs"].([]interface{}); ok && len(v) > 0 { - apiObject.S3Logs = expandOrganizationS3LogsConfiguration(v[0].(map[string]interface{})) - } - if v, ok := tfMap["kubernetes"].([]interface{}); ok && len(v) > 0 { apiObject.Kubernetes = expandOrganizationKubernetesConfiguration(v[0].(map[string]interface{})) } @@ -251,6 +245,10 @@ func expandOrganizationDataSourceConfigurations(tfMap map[string]interface{}) *g apiObject.MalwareProtection = expandOrganizationMalwareProtectionConfiguration(v[0].(map[string]interface{})) } + if v, ok := tfMap["s3_logs"].([]interface{}); ok && len(v) > 0 { + apiObject.S3Logs = expandOrganizationS3LogsConfiguration(v[0].(map[string]interface{})) + } + return apiObject } @@ -304,11 +302,11 @@ func expandOrganizationMalwareProtectionConfiguration(tfMap map[string]interface } return &guardduty.OrganizationMalwareProtectionConfiguration{ - ScanEc2InstanceWithFindings: expandOrganizationScanEC2InstanceWithFindingsConfiguration(m), + ScanEc2InstanceWithFindings: expandOrganizationScanEc2InstanceWithFindings(m), } } -func expandOrganizationScanEC2InstanceWithFindingsConfiguration(tfMap map[string]interface{}) *guardduty.OrganizationScanEc2InstanceWithFindings { +func expandOrganizationScanEc2InstanceWithFindings(tfMap map[string]interface{}) *guardduty.OrganizationScanEc2InstanceWithFindings { if tfMap == nil { return nil } @@ -324,11 +322,11 @@ func expandOrganizationScanEC2InstanceWithFindingsConfiguration(tfMap map[string } return &guardduty.OrganizationScanEc2InstanceWithFindings{ - EbsVolumes: expandOrganizationEBSVolumesConfiguration(m), + EbsVolumes: expandOrganizationEbsVolumes(m), } } -func expandOrganizationEBSVolumesConfiguration(tfMap map[string]interface{}) *guardduty.OrganizationEbsVolumes { +func expandOrganizationEbsVolumes(tfMap map[string]interface{}) *guardduty.OrganizationEbsVolumes { if tfMap == nil { return nil } @@ -397,13 +395,13 @@ func flattenOrganizationKubernetesConfigurationResult(apiObject *guardduty.Organ tfMap := map[string]interface{}{} if v := apiObject.AuditLogs; v != nil { - tfMap["audit_logs"] = []interface{}{flattenOrganizationKubernetesAuditLogsConfiguration(v)} + tfMap["audit_logs"] = []interface{}{flattenOrganizationKubernetesAuditLogsConfigurationResult(v)} } return tfMap } -func flattenOrganizationKubernetesAuditLogsConfiguration(apiObject *guardduty.OrganizationKubernetesAuditLogsConfigurationResult) map[string]interface{} { +func flattenOrganizationKubernetesAuditLogsConfigurationResult(apiObject *guardduty.OrganizationKubernetesAuditLogsConfigurationResult) map[string]interface{} { if apiObject == nil { return nil } @@ -425,13 +423,13 @@ func flattenOrganizationMalwareProtectionConfigurationResult(apiObject *guarddut tfMap := map[string]interface{}{} if v := apiObject.ScanEc2InstanceWithFindings; v != nil { - tfMap["scan_ec2_instance_with_findings"] = []interface{}{flattenOrganizationMalwareProtectionScanEC2InstanceWithFindingsResult(v)} + tfMap["scan_ec2_instance_with_findings"] = []interface{}{flattenOrganizationScanEc2InstanceWithFindingsResult(v)} } return tfMap } -func flattenOrganizationMalwareProtectionScanEC2InstanceWithFindingsResult(apiObject *guardduty.OrganizationScanEc2InstanceWithFindingsResult) map[string]interface{} { +func flattenOrganizationScanEc2InstanceWithFindingsResult(apiObject *guardduty.OrganizationScanEc2InstanceWithFindingsResult) map[string]interface{} { if apiObject == nil { return nil } @@ -439,13 +437,13 @@ func flattenOrganizationMalwareProtectionScanEC2InstanceWithFindingsResult(apiOb tfMap := map[string]interface{}{} if v := apiObject.EbsVolumes; v != nil { - tfMap["ebs_volumes"] = []interface{}{flattenOrganizationMalwareProtectionEBSVolumesResult(v)} + tfMap["ebs_volumes"] = []interface{}{flattenOrganizationEbsVolumesResult(v)} } return tfMap } -func flattenOrganizationMalwareProtectionEBSVolumesResult(apiObject *guardduty.OrganizationEbsVolumesResult) map[string]interface{} { +func flattenOrganizationEbsVolumesResult(apiObject *guardduty.OrganizationEbsVolumesResult) map[string]interface{} { if apiObject == nil { return nil } From 92d9db703ff3b9f09b3b115a7b60d46d982e1b89 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 11 Oct 2023 15:53:54 -0400 Subject: [PATCH 02/12] r/aws_guardduty_organization_configuration: Tidy up acceptance tests. --- .../organization_configuration_test.go | 39 +++---------------- 1 file changed, 5 insertions(+), 34 deletions(-) diff --git a/internal/service/guardduty/organization_configuration_test.go b/internal/service/guardduty/organization_configuration_test.go index 5743d933b15..8b756549060 100644 --- a/internal/service/guardduty/organization_configuration_test.go +++ b/internal/service/guardduty/organization_configuration_test.go @@ -25,9 +25,7 @@ func testAccOrganizationConfiguration_basic(t *testing.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), + CheckDestroy: acctest.CheckDestroyNoop, Steps: []resource.TestStep{ { Config: testAccOrganizationConfigurationConfig_autoEnable(true), @@ -67,9 +65,7 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.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), + CheckDestroy: acctest.CheckDestroyNoop, Steps: []resource.TestStep{ { Config: testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers("ALL"), @@ -92,11 +88,6 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), ), }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - }, { Config: testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers("ALL"), Check: resource.ComposeTestCheckFunc( @@ -105,11 +96,6 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), ), }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - }, { Config: testAccOrganizationConfigurationConfig_autoEnable(true), Check: resource.ComposeTestCheckFunc( @@ -118,11 +104,6 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), ), }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - }, { Config: testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers("NONE"), Check: resource.ComposeTestCheckFunc( @@ -131,11 +112,6 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), ), }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - }, { Config: testAccOrganizationConfigurationConfig_autoEnable(false), Check: resource.ComposeTestCheckFunc( @@ -144,11 +120,6 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), ), }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - }, }, }) } @@ -166,7 +137,7 @@ func testAccOrganizationConfiguration_s3logs(t *testing.T) { }, ErrorCheck: acctest.ErrorCheck(t, guardduty.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckDetectorDestroy(ctx), + CheckDestroy: acctest.CheckDestroyNoop, Steps: []resource.TestStep{ { Config: testAccOrganizationConfigurationConfig_s3Logs(true), @@ -210,7 +181,7 @@ func testAccOrganizationConfiguration_kubernetes(t *testing.T) { }, ErrorCheck: acctest.ErrorCheck(t, guardduty.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckDetectorDestroy(ctx), + CheckDestroy: acctest.CheckDestroyNoop, Steps: []resource.TestStep{ { Config: testAccOrganizationConfigurationConfig_kubernetes(true), @@ -256,7 +227,7 @@ func testAccOrganizationConfiguration_malwareprotection(t *testing.T) { }, ErrorCheck: acctest.ErrorCheck(t, guardduty.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckDetectorDestroy(ctx), + CheckDestroy: acctest.CheckDestroyNoop, Steps: []resource.TestStep{ { Config: testAccOrganizationConfigurationConfig_malwareprotection(true), From 8c83a48cac0929e6b87e2142816e0636315cafb4 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 11 Oct 2023 15:57:32 -0400 Subject: [PATCH 03/12] Fix semgrep 'ci.caps3-in-func-name'. --- internal/service/guardduty/organization_configuration.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/service/guardduty/organization_configuration.go b/internal/service/guardduty/organization_configuration.go index 67c2d53f469..256ab0870b8 100644 --- a/internal/service/guardduty/organization_configuration.go +++ b/internal/service/guardduty/organization_configuration.go @@ -306,7 +306,7 @@ func expandOrganizationMalwareProtectionConfiguration(tfMap map[string]interface } } -func expandOrganizationScanEc2InstanceWithFindings(tfMap map[string]interface{}) *guardduty.OrganizationScanEc2InstanceWithFindings { +func expandOrganizationScanEc2InstanceWithFindings(tfMap map[string]interface{}) *guardduty.OrganizationScanEc2InstanceWithFindings { // nosemgrep:ci.caps3-in-func-name if tfMap == nil { return nil } @@ -326,7 +326,7 @@ func expandOrganizationScanEc2InstanceWithFindings(tfMap map[string]interface{}) } } -func expandOrganizationEbsVolumes(tfMap map[string]interface{}) *guardduty.OrganizationEbsVolumes { +func expandOrganizationEbsVolumes(tfMap map[string]interface{}) *guardduty.OrganizationEbsVolumes { // nosemgrep:ci.caps3-in-func-name if tfMap == nil { return nil } @@ -429,7 +429,7 @@ func flattenOrganizationMalwareProtectionConfigurationResult(apiObject *guarddut return tfMap } -func flattenOrganizationScanEc2InstanceWithFindingsResult(apiObject *guardduty.OrganizationScanEc2InstanceWithFindingsResult) map[string]interface{} { +func flattenOrganizationScanEc2InstanceWithFindingsResult(apiObject *guardduty.OrganizationScanEc2InstanceWithFindingsResult) map[string]interface{} { // nosemgrep:ci.caps3-in-func-name if apiObject == nil { return nil } @@ -443,7 +443,7 @@ func flattenOrganizationScanEc2InstanceWithFindingsResult(apiObject *guardduty.O return tfMap } -func flattenOrganizationEbsVolumesResult(apiObject *guardduty.OrganizationEbsVolumesResult) map[string]interface{} { +func flattenOrganizationEbsVolumesResult(apiObject *guardduty.OrganizationEbsVolumesResult) map[string]interface{} { // nosemgrep:ci.caps3-in-func-name if apiObject == nil { return nil } From 454e9ce3b38a6eb33fabdfc2a684451a2eb0e63c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 11 Oct 2023 16:07:09 -0400 Subject: [PATCH 04/12] Acceptance test output: % make testacc TESTARGS='-run=TestAccGuardDuty_serial/^OrganizationConfiguration$$' PKG=guardduty ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/guardduty/... -v -count 1 -parallel 20 -run=TestAccGuardDuty_serial/^OrganizationConfiguration$ -timeout 360m === RUN TestAccGuardDuty_serial === PAUSE TestAccGuardDuty_serial === CONT TestAccGuardDuty_serial === RUN TestAccGuardDuty_serial/OrganizationConfiguration === RUN TestAccGuardDuty_serial/OrganizationConfiguration/s3Logs === RUN TestAccGuardDuty_serial/OrganizationConfiguration/kubernetes === RUN TestAccGuardDuty_serial/OrganizationConfiguration/malwareProtection === RUN TestAccGuardDuty_serial/OrganizationConfiguration/basic === RUN TestAccGuardDuty_serial/OrganizationConfiguration/autoEnableOrganizationMembers --- PASS: TestAccGuardDuty_serial (338.25s) --- PASS: TestAccGuardDuty_serial/OrganizationConfiguration (338.25s) --- PASS: TestAccGuardDuty_serial/OrganizationConfiguration/s3Logs (55.80s) --- PASS: TestAccGuardDuty_serial/OrganizationConfiguration/kubernetes (51.32s) --- PASS: TestAccGuardDuty_serial/OrganizationConfiguration/malwareProtection (47.81s) --- PASS: TestAccGuardDuty_serial/OrganizationConfiguration/basic (53.18s) --- PASS: TestAccGuardDuty_serial/OrganizationConfiguration/autoEnableOrganizationMembers (130.14s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/guardduty 343.041s From 39f82d388a0f4c431f135994ec8780dff4fecbff Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 11 Oct 2023 16:26:53 -0400 Subject: [PATCH 05/12] guardduty: Add 'FindOrganizationConfigurationByID'. --- internal/service/guardduty/detector.go | 4 ++ .../guardduty/organization_configuration.go | 37 +++++++++++++++---- .../organization_configuration_test.go | 33 +++++++++++++++++ 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/internal/service/guardduty/detector.go b/internal/service/guardduty/detector.go index 239a2294c44..1e15ae1bd6e 100644 --- a/internal/service/guardduty/detector.go +++ b/internal/service/guardduty/detector.go @@ -510,6 +510,10 @@ func FindDetectorByID(ctx context.Context, conn *guardduty.GuardDuty, id string) return nil, err } + if output == nil { + return nil, tfresource.NewEmptyResultError(input) + } + return output, nil } diff --git a/internal/service/guardduty/organization_configuration.go b/internal/service/guardduty/organization_configuration.go index 256ab0870b8..0ef943d3ec7 100644 --- a/internal/service/guardduty/organization_configuration.go +++ b/internal/service/guardduty/organization_configuration.go @@ -12,10 +12,12 @@ import ( "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/retry" "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" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) // @SDKResource("aws_guardduty_organization_configuration") @@ -194,13 +196,9 @@ func resourceOrganizationConfigurationRead(ctx context.Context, d *schema.Resour var diags diag.Diagnostics conn := meta.(*conns.AWSClient).GuardDutyConn(ctx) - input := &guardduty.DescribeOrganizationConfigurationInput{ - DetectorId: aws.String(d.Id()), - } - - output, err := conn.DescribeOrganizationConfigurationWithContext(ctx, input) + output, err := FindOrganizationConfigurationByID(ctx, conn, d.Id()) - if tfawserr.ErrMessageContains(err, guardduty.ErrCodeBadRequestException, "The request is rejected because the input detectorId is not owned by the current account.") { + if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] GuardDuty Organization Configuration (%s) not found, removing from state", d.Id()) d.SetId("") return diags @@ -216,7 +214,6 @@ 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 { return sdkdiag.AppendErrorf(diags, "setting datasources: %s", err) @@ -224,7 +221,6 @@ func resourceOrganizationConfigurationRead(ctx context.Context, d *schema.Resour } else { d.Set("datasources", nil) } - d.Set("detector_id", d.Id()) return diags @@ -456,3 +452,28 @@ func flattenOrganizationEbsVolumesResult(apiObject *guardduty.OrganizationEbsVol return tfMap } + +func FindOrganizationConfigurationByID(ctx context.Context, conn *guardduty.GuardDuty, id string) (*guardduty.DescribeOrganizationConfigurationOutput, error) { + input := &guardduty.DescribeOrganizationConfigurationInput{ + DetectorId: aws.String(id), + } + + output, err := conn.DescribeOrganizationConfigurationWithContext(ctx, input) + + if tfawserr.ErrMessageContains(err, guardduty.ErrCodeBadRequestException, "The request is rejected because the input detectorId is not owned by the current account.") { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output, nil +} diff --git a/internal/service/guardduty/organization_configuration_test.go b/internal/service/guardduty/organization_configuration_test.go index 8b756549060..94e218f45a7 100644 --- a/internal/service/guardduty/organization_configuration_test.go +++ b/internal/service/guardduty/organization_configuration_test.go @@ -4,12 +4,16 @@ package guardduty_test import ( + "context" "fmt" "testing" "github.com/aws/aws-sdk-go/service/guardduty" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" + "github.com/hashicorp/terraform-provider-aws/internal/conns" + tfguardduty "github.com/hashicorp/terraform-provider-aws/internal/service/guardduty" ) func testAccOrganizationConfiguration_basic(t *testing.T) { @@ -30,6 +34,7 @@ func testAccOrganizationConfiguration_basic(t *testing.T) { { Config: testAccOrganizationConfigurationConfig_autoEnable(true), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"), resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NEW"), resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), @@ -43,6 +48,7 @@ func testAccOrganizationConfiguration_basic(t *testing.T) { { Config: testAccOrganizationConfigurationConfig_autoEnable(false), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable", "false"), resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NONE"), resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), @@ -70,6 +76,7 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T { Config: testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers("ALL"), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "ALL"), resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"), resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), @@ -83,6 +90,7 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T { Config: testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers("NONE"), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NONE"), resource.TestCheckResourceAttr(resourceName, "auto_enable", "false"), resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), @@ -91,6 +99,7 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T { Config: testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers("ALL"), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "ALL"), resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"), resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), @@ -99,6 +108,7 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T { Config: testAccOrganizationConfigurationConfig_autoEnable(true), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NEW"), resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"), resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), @@ -107,6 +117,7 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T { Config: testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers("NONE"), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NONE"), resource.TestCheckResourceAttr(resourceName, "auto_enable", "false"), resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), @@ -115,6 +126,7 @@ func testAccOrganizationConfiguration_autoEnableOrganizationMembers(t *testing.T { Config: testAccOrganizationConfigurationConfig_autoEnable(false), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable_organization_members", "NONE"), resource.TestCheckResourceAttr(resourceName, "auto_enable", "false"), resource.TestCheckResourceAttrPair(resourceName, "detector_id", detectorResourceName, "id"), @@ -142,6 +154,7 @@ func testAccOrganizationConfiguration_s3logs(t *testing.T) { { Config: testAccOrganizationConfigurationConfig_s3Logs(true), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"), resource.TestCheckResourceAttr(resourceName, "datasources.#", "1"), resource.TestCheckResourceAttr(resourceName, "datasources.0.s3_logs.#", "1"), @@ -157,6 +170,7 @@ func testAccOrganizationConfiguration_s3logs(t *testing.T) { { Config: testAccOrganizationConfigurationConfig_s3Logs(false), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"), resource.TestCheckResourceAttr(resourceName, "datasources.#", "1"), resource.TestCheckResourceAttr(resourceName, "datasources.0.s3_logs.#", "1"), @@ -186,6 +200,7 @@ func testAccOrganizationConfiguration_kubernetes(t *testing.T) { { Config: testAccOrganizationConfigurationConfig_kubernetes(true), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"), resource.TestCheckResourceAttr(resourceName, "datasources.#", "1"), resource.TestCheckResourceAttr(resourceName, "datasources.0.kubernetes.#", "1"), @@ -202,6 +217,7 @@ func testAccOrganizationConfiguration_kubernetes(t *testing.T) { { Config: testAccOrganizationConfigurationConfig_kubernetes(false), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"), resource.TestCheckResourceAttr(resourceName, "datasources.#", "1"), resource.TestCheckResourceAttr(resourceName, "datasources.0.kubernetes.#", "1"), @@ -232,6 +248,7 @@ func testAccOrganizationConfiguration_malwareprotection(t *testing.T) { { Config: testAccOrganizationConfigurationConfig_malwareprotection(true), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"), resource.TestCheckResourceAttr(resourceName, "datasources.#", "1"), resource.TestCheckResourceAttr(resourceName, "datasources.0.malware_protection.#", "1"), @@ -249,6 +266,7 @@ func testAccOrganizationConfiguration_malwareprotection(t *testing.T) { { Config: testAccOrganizationConfigurationConfig_malwareprotection(false), Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationConfigurationExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"), resource.TestCheckResourceAttr(resourceName, "datasources.#", "1"), resource.TestCheckResourceAttr(resourceName, "datasources.0.malware_protection.#", "1"), @@ -262,6 +280,21 @@ func testAccOrganizationConfiguration_malwareprotection(t *testing.T) { }) } +func testAccCheckOrganizationConfigurationExists(ctx context.Context, n string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + conn := acctest.Provider.Meta().(*conns.AWSClient).GuardDutyConn(ctx) + + _, err := tfguardduty.FindOrganizationConfigurationByID(ctx, conn, rs.Primary.ID) + + return err + } +} + const testAccOrganizationConfigurationConfigBase = ` data "aws_caller_identity" "current" {} From 111da26cd015bf8df02a778eb07418d19ed01042 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 11 Oct 2023 17:09:43 -0400 Subject: [PATCH 06/12] r/aws_guardduty_organization_configuration_feature: New resource. --- .../guardduty/organization_configuration.go | 2 +- .../organization_configuration_feature.go | 244 ++++++++++++++++++ .../service/guardduty/service_package_gen.go | 6 + 3 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 internal/service/guardduty/organization_configuration_feature.go diff --git a/internal/service/guardduty/organization_configuration.go b/internal/service/guardduty/organization_configuration.go index 0ef943d3ec7..74460889b05 100644 --- a/internal/service/guardduty/organization_configuration.go +++ b/internal/service/guardduty/organization_configuration.go @@ -20,7 +20,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -// @SDKResource("aws_guardduty_organization_configuration") +// @SDKResource("aws_guardduty_organization_configuration", name="Organization Configuration") func ResourceOrganizationConfiguration() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceOrganizationConfigurationPut, diff --git a/internal/service/guardduty/organization_configuration_feature.go b/internal/service/guardduty/organization_configuration_feature.go new file mode 100644 index 00000000000..51b8b536ddb --- /dev/null +++ b/internal/service/guardduty/organization_configuration_feature.go @@ -0,0 +1,244 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package guardduty + +import ( + "context" + "fmt" + "log" + "strings" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/guardduty" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "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" + "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" +) + +// @SDKResource("aws_guardduty_organization_configuration_feature", name="Organization Configuration Feature") +func ResourceOrganizationConfigurationFeature() *schema.Resource { + return &schema.Resource{ + CreateWithoutTimeout: resourceOrganizationConfigurationFeaturePut, + ReadWithoutTimeout: resourceOrganizationConfigurationFeatureRead, + UpdateWithoutTimeout: resourceOrganizationConfigurationFeaturePut, + DeleteWithoutTimeout: schema.NoopContext, + + Schema: map[string]*schema.Schema{ + "additional_configuration": { + Optional: true, + ForceNew: true, + Type: schema.TypeList, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "auto_enable": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice(guardduty.OrgFeatureStatus_Values(), false), + }, + "name": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.StringInSlice(guardduty.OrgFeatureAdditionalConfiguration_Values(), false), + }, + }, + }, + }, + "auto_enable": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice(guardduty.OrgFeatureStatus_Values(), false), + }, + "detector_id": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + "name": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.StringInSlice(guardduty.OrgFeature_Values(), false), + }, + }, + } +} + +func resourceOrganizationConfigurationFeaturePut(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + var diags diag.Diagnostics + conn := meta.(*conns.AWSClient).GuardDutyConn(ctx) + + detectorID, name := d.Get("detector_id").(string), d.Get("name").(string) + feature := &guardduty.OrganizationFeatureConfiguration{ + AutoEnable: aws.String(d.Get("auto_enable").(string)), + Name: aws.String(name), + } + + if v, ok := d.GetOk("additional_configuration"); ok && len(v.([]interface{})) > 0 { + feature.AdditionalConfiguration = expandOrganizationAdditionalConfigurations(v.([]interface{})) + } + + input := &guardduty.UpdateOrganizationConfigurationInput{ + DetectorId: aws.String(detectorID), + Features: []*guardduty.OrganizationFeatureConfiguration{feature}, + } + + _, err := conn.UpdateOrganizationConfigurationWithContext(ctx, input) + + if err != nil { + return sdkdiag.AppendErrorf(diags, "updating GuardDuty Organization Configuration (%s) Feature (%s): %s", detectorID, name, err) + } + + if d.IsNewResource() { + d.SetId(organizationConfigurationFeatureCreateResourceID(detectorID, name)) + } + + return append(diags, resourceOrganizationConfigurationFeatureRead(ctx, d, meta)...) +} + +func resourceOrganizationConfigurationFeatureRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + var diags diag.Diagnostics + conn := meta.(*conns.AWSClient).GuardDutyConn(ctx) + + detectorID, name, err := organizationConfigurationFeatureParseResourceID(d.Id()) + if err != nil { + return sdkdiag.AppendFromErr(diags, err) + } + + feature, err := FindOrganizationConfigurationFeatureByTwoPartKey(ctx, conn, detectorID, name) + + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] GuardDuty Organization Configuration Feature (%s) not found, removing from state", d.Id()) + d.SetId("") + return diags + } + + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading GuardDuty Organization Configuration Feature (%s): %s", d.Id(), err) + } + + if err := d.Set("additional_configuration", flattenOrganizationAdditionalConfigurationResults(feature.AdditionalConfiguration)); err != nil { + return sdkdiag.AppendErrorf(diags, "setting additional_configuration: %s", err) + } + d.Set("auto_enable", feature.AutoEnable) + d.Set("detector_id", detectorID) + d.Set("name", feature.Name) + + return diags +} + +const organizationConfigurationFeatureResourceIDSeparator = "/" + +func organizationConfigurationFeatureCreateResourceID(detectorID, name string) string { + parts := []string{detectorID, name} + id := strings.Join(parts, organizationConfigurationFeatureResourceIDSeparator) + + return id +} + +func organizationConfigurationFeatureParseResourceID(id string) (string, string, error) { + parts := strings.Split(id, organizationConfigurationFeatureResourceIDSeparator) + + if len(parts) == 2 && parts[0] != "" && parts[1] != "" { + return parts[0], parts[1], nil + } + + return "", "", fmt.Errorf("unexpected format for ID (%[1]s), expected DETECTORID%[2]sFEATURENAME", id, organizationConfigurationFeatureResourceIDSeparator) +} + +func FindOrganizationConfigurationFeatureByTwoPartKey(ctx context.Context, conn *guardduty.GuardDuty, detectorID, name string) (*guardduty.OrganizationFeatureConfigurationResult, error) { + output, err := FindOrganizationConfigurationByID(ctx, conn, detectorID) + + if err != nil { + return nil, err + } + + return tfresource.AssertSinglePtrResult(tfslices.Filter(output.Features, func(v *guardduty.OrganizationFeatureConfigurationResult) bool { + return aws.StringValue(v.Name) == name + })) +} + +func expandOrganizationAdditionalConfiguration(tfMap map[string]interface{}) *guardduty.OrganizationAdditionalConfiguration { + if tfMap == nil { + return nil + } + + apiObject := &guardduty.OrganizationAdditionalConfiguration{} + + if v, ok := tfMap["auto_enable"].(string); ok && v != "" { + apiObject.AutoEnable = aws.String(v) + } + + if v, ok := tfMap["name"].(string); ok && v != "" { + apiObject.Name = aws.String(v) + } + + return apiObject +} + +func expandOrganizationAdditionalConfigurations(tfList []interface{}) []*guardduty.OrganizationAdditionalConfiguration { + if len(tfList) == 0 { + return nil + } + + var apiObjects []*guardduty.OrganizationAdditionalConfiguration + + for _, tfMapRaw := range tfList { + tfMap, ok := tfMapRaw.(map[string]interface{}) + + if !ok { + continue + } + + apiObject := expandOrganizationAdditionalConfiguration(tfMap) + + if apiObject == nil { + continue + } + + apiObjects = append(apiObjects, apiObject) + } + + return apiObjects +} + +func flattenOrganizationAdditionalConfigurationResult(apiObject *guardduty.OrganizationAdditionalConfigurationResult) map[string]interface{} { + if apiObject == nil { + return nil + } + + tfMap := map[string]interface{}{} + + if v := apiObject.AutoEnable; v != nil { + tfMap["auto_enable"] = aws.StringValue(v) + } + + if v := apiObject.Name; v != nil { + tfMap["name"] = aws.StringValue(v) + } + + return tfMap +} + +func flattenOrganizationAdditionalConfigurationResults(apiObjects []*guardduty.OrganizationAdditionalConfigurationResult) []interface{} { + if len(apiObjects) == 0 { + return nil + } + + var tfList []interface{} + + for _, apiObject := range apiObjects { + if apiObject == nil { + continue + } + + tfList = append(tfList, flattenOrganizationAdditionalConfigurationResult(apiObject)) + } + + return tfList +} diff --git a/internal/service/guardduty/service_package_gen.go b/internal/service/guardduty/service_package_gen.go index ed2dbc93187..034bfff562b 100644 --- a/internal/service/guardduty/service_package_gen.go +++ b/internal/service/guardduty/service_package_gen.go @@ -83,6 +83,12 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka { Factory: ResourceOrganizationConfiguration, TypeName: "aws_guardduty_organization_configuration", + Name: "Organization Configuration", + }, + { + Factory: ResourceOrganizationConfigurationFeature, + TypeName: "aws_guardduty_organization_configuration_feature", + Name: "Organization Configuration Feature", }, { Factory: ResourcePublishingDestination, From 14ea38012fded3f835fd34f265f6d5ff6dcd59ea Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 11 Oct 2023 17:23:50 -0400 Subject: [PATCH 07/12] r/aws_guardduty_organization_configuration_feature: Add documentation. --- ...zation_configuration_feature.html.markdown | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 website/docs/r/guardduty_organization_configuration_feature.html.markdown diff --git a/website/docs/r/guardduty_organization_configuration_feature.html.markdown b/website/docs/r/guardduty_organization_configuration_feature.html.markdown new file mode 100644 index 00000000000..c57da5cc329 --- /dev/null +++ b/website/docs/r/guardduty_organization_configuration_feature.html.markdown @@ -0,0 +1,52 @@ +--- +subcategory: "GuardDuty" +layout: "aws" +page_title: "AWS: aws_guardduty_organization_configuration_feature" +description: |- + Provides a resource to manage an Amazon GuardDuty organization configuration feature +--- + +# Resource: aws_guardduty_organization_configuration_feature + +Provides a resource to manage a single Amazon GuardDuty [organization configuration feature](https://docs.aws.amazon.com/guardduty/latest/ug/guardduty-features-activation-model.html#guardduty-features). + +~> **NOTE:** Deleting this resource does not disable the organization configuration feature, the resource in simply removed from state instead. + +## Example Usage + +```terraform +resource "aws_guardduty_detector" "example" { + enable = true +} + +resource "aws_guardduty_organization_configuration_feature" "eks_runtime_monitoring" { + detector_id = aws_guardduty_detector.example.id + name = "EKS_RUNTIME_MONITORING" + auto_enable = "ALL" + + additional_configuration { + name = "EKS_ADDON_MANAGEMENT" + auto_enable = "NEW" + } +} +``` + +## Argument Reference + +This resource supports the following arguments: + +* `auto_enable` - (Required) The status of the feature that is configured for the member accounts within the organization. Valid values: `NEW`, `ALL`, `NONE`. +* `detector_id` - (Required) The ID of the detector that configures the delegated administrator. +* `name` - (Required) The name of the feature that will be configured for the organization. Valid values: `S3_DATA_EVENTS`, `EKS_AUDIT_LOGS`, `EBS_MALWARE_PROTECTION`, `RDS_LOGIN_EVENTS`, `EKS_RUNTIME_MONITORING`, `LAMBDA_NETWORK_LOGS`. +* `additional_configuration` - (Optional) The additional information that will be configured for the organization See [below](#additional-configuration). + +### Additional Configuration + +The `additional_configuration` block supports the following: + +* `auto_enable` - (Required) The status of the additional configuration that will be configured for the organization. Valid values: `NEW`, `ALL`, `NONE`. +* `name` - (Required) The name of the additional configuration that will be configured for the organization. Valid values: `EKS_ADDON_MANAGEMENT`. + +## Attribute Reference + +This resource exports no additional attributes. From a94246ef4bd30a4873bf3951d004a5a1bcac4940 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 12 Oct 2023 10:59:31 -0400 Subject: [PATCH 08/12] r/aws_guardduty_organization_configuration_feature: Add acceptance tests. --- internal/service/guardduty/guardduty_test.go | 5 + .../organization_configuration_feature.go | 16 +- ...organization_configuration_feature_test.go | 237 ++++++++++++++++++ .../organization_configuration_test.go | 22 +- 4 files changed, 260 insertions(+), 20 deletions(-) create mode 100644 internal/service/guardduty/organization_configuration_feature_test.go diff --git a/internal/service/guardduty/guardduty_test.go b/internal/service/guardduty/guardduty_test.go index 2eed345735d..fa0ccf8b32b 100644 --- a/internal/service/guardduty/guardduty_test.go +++ b/internal/service/guardduty/guardduty_test.go @@ -59,6 +59,11 @@ func TestAccGuardDuty_serial(t *testing.T) { "kubernetes": testAccOrganizationConfiguration_kubernetes, "malwareProtection": testAccOrganizationConfiguration_malwareprotection, }, + "OrganizationConfigurationFeature": { + "basic": testAccOrganizationConfigurationFeature_basic, + "additional_configuration": testAccOrganizationConfigurationFeature_additionalConfiguration, + "multiple": testAccOrganizationConfigurationFeature_multiple, + }, "ThreatIntelSet": { "basic": testAccThreatIntelSet_basic, "tags": testAccThreatIntelSet_tags, diff --git a/internal/service/guardduty/organization_configuration_feature.go b/internal/service/guardduty/organization_configuration_feature.go index 51b8b536ddb..82e63a4f59d 100644 --- a/internal/service/guardduty/organization_configuration_feature.go +++ b/internal/service/guardduty/organization_configuration_feature.go @@ -73,7 +73,14 @@ func resourceOrganizationConfigurationFeaturePut(ctx context.Context, d *schema. var diags diag.Diagnostics conn := meta.(*conns.AWSClient).GuardDutyConn(ctx) - detectorID, name := d.Get("detector_id").(string), d.Get("name").(string) + detectorID := d.Get("detector_id").(string) + output, err := FindOrganizationConfigurationByID(ctx, conn, detectorID) + + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading GuardDuty Organization Configuration (%s): %s", detectorID, err) + } + + name := d.Get("name").(string) feature := &guardduty.OrganizationFeatureConfiguration{ AutoEnable: aws.String(d.Get("auto_enable").(string)), Name: aws.String(name), @@ -84,11 +91,12 @@ func resourceOrganizationConfigurationFeaturePut(ctx context.Context, d *schema. } input := &guardduty.UpdateOrganizationConfigurationInput{ - DetectorId: aws.String(detectorID), - Features: []*guardduty.OrganizationFeatureConfiguration{feature}, + AutoEnableOrganizationMembers: output.AutoEnableOrganizationMembers, + DetectorId: aws.String(detectorID), + Features: []*guardduty.OrganizationFeatureConfiguration{feature}, } - _, err := conn.UpdateOrganizationConfigurationWithContext(ctx, input) + _, err = conn.UpdateOrganizationConfigurationWithContext(ctx, input) if err != nil { return sdkdiag.AppendErrorf(diags, "updating GuardDuty Organization Configuration (%s) Feature (%s): %s", detectorID, name, err) diff --git a/internal/service/guardduty/organization_configuration_feature_test.go b/internal/service/guardduty/organization_configuration_feature_test.go new file mode 100644 index 00000000000..10616b4a337 --- /dev/null +++ b/internal/service/guardduty/organization_configuration_feature_test.go @@ -0,0 +1,237 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package guardduty_test + +import ( + "context" + "fmt" + "testing" + + "github.com/aws/aws-sdk-go/service/guardduty" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/hashicorp/terraform-provider-aws/internal/acctest" + "github.com/hashicorp/terraform-provider-aws/internal/conns" + tfguardduty "github.com/hashicorp/terraform-provider-aws/internal/service/guardduty" +) + +func testAccOrganizationConfigurationFeature_basic(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_guardduty_organization_configuration_feature.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckOrganizationsAccount(ctx, t) + testAccPreCheckDetectorNotExists(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, guardduty.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: acctest.CheckDestroyNoop, + Steps: []resource.TestStep{ + { + Config: testAccOrganizationConfigurationFeatureConfig_basic("RDS_LOGIN_EVENTS", "ALL"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccOrganizationConfigurationFeatureExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "additional_configuration.#", "0"), + resource.TestCheckResourceAttr(resourceName, "auto_enable", "ALL"), + resource.TestCheckResourceAttrSet(resourceName, "detector_id"), + resource.TestCheckResourceAttr(resourceName, "name", "RDS_LOGIN_EVENTS"), + ), + }, + }, + }) +} + +func testAccOrganizationConfigurationFeature_additionalConfiguration(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_guardduty_organization_configuration_feature.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckOrganizationsAccount(ctx, t) + testAccPreCheckDetectorNotExists(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, guardduty.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: acctest.CheckDestroyNoop, + Steps: []resource.TestStep{ + { + Config: testAccOrganizationConfigurationFeatureConfig_additionalConfiguration("NEW", "NONE"), + Check: resource.ComposeTestCheckFunc( + testAccOrganizationConfigurationFeatureExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "auto_enable", "NEW"), + resource.TestCheckResourceAttr(resourceName, "additional_configuration.#", "1"), + resource.TestCheckResourceAttr(resourceName, "additional_configuration.0.auto_enable", "NONE"), + resource.TestCheckResourceAttr(resourceName, "additional_configuration.0.name", "EKS_ADDON_MANAGEMENT"), + resource.TestCheckResourceAttr(resourceName, "name", "EKS_RUNTIME_MONITORING"), + ), + }, + { + Config: testAccOrganizationConfigurationFeatureConfig_additionalConfiguration("ALL", "ALL"), + Check: resource.ComposeTestCheckFunc( + testAccOrganizationConfigurationFeatureExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "auto_enable", "ALL"), + resource.TestCheckResourceAttr(resourceName, "additional_configuration.#", "1"), + resource.TestCheckResourceAttr(resourceName, "additional_configuration.0.auto_enable", "ALL"), + resource.TestCheckResourceAttr(resourceName, "additional_configuration.0.name", "EKS_ADDON_MANAGEMENT"), + resource.TestCheckResourceAttr(resourceName, "name", "EKS_RUNTIME_MONITORING"), + ), + }, + }, + }) +} + +func testAccOrganizationConfigurationFeature_multiple(t *testing.T) { + ctx := acctest.Context(t) + resource1Name := "aws_guardduty_organization_configuration_feature.test1" + resource2Name := "aws_guardduty_organization_configuration_feature.test2" + resource3Name := "aws_guardduty_organization_configuration_feature.test3" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckOrganizationsAccount(ctx, t) + testAccPreCheckDetectorNotExists(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, guardduty.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: acctest.CheckDestroyNoop, + Steps: []resource.TestStep{ + { + Config: testAccOrganizationConfigurationFeatureConfig_multiple("ALL", "NEW", "NONE"), + Check: resource.ComposeTestCheckFunc( + testAccOrganizationConfigurationFeatureExists(ctx, resource1Name), + testAccOrganizationConfigurationFeatureExists(ctx, resource2Name), + testAccOrganizationConfigurationFeatureExists(ctx, resource3Name), + resource.TestCheckResourceAttr(resource1Name, "additional_configuration.#", "0"), + resource.TestCheckResourceAttr(resource1Name, "auto_enable", "ALL"), + resource.TestCheckResourceAttr(resource1Name, "name", "EBS_MALWARE_PROTECTION"), + resource.TestCheckResourceAttr(resource2Name, "additional_configuration.#", "0"), + resource.TestCheckResourceAttr(resource2Name, "auto_enable", "NEW"), + resource.TestCheckResourceAttr(resource2Name, "name", "LAMBDA_NETWORK_LOGS"), + resource.TestCheckResourceAttr(resource3Name, "additional_configuration.#", "0"), + resource.TestCheckResourceAttr(resource3Name, "auto_enable", "NONE"), + resource.TestCheckResourceAttr(resource3Name, "name", "S3_DATA_EVENTS"), + ), + }, + { + Config: testAccOrganizationConfigurationFeatureConfig_multiple("NEW", "ALL", "ALL"), + Check: resource.ComposeTestCheckFunc( + testAccOrganizationConfigurationFeatureExists(ctx, resource1Name), + testAccOrganizationConfigurationFeatureExists(ctx, resource2Name), + testAccOrganizationConfigurationFeatureExists(ctx, resource3Name), + resource.TestCheckResourceAttr(resource1Name, "additional_configuration.#", "0"), + resource.TestCheckResourceAttr(resource1Name, "auto_enable", "NEW"), + resource.TestCheckResourceAttr(resource1Name, "name", "EBS_MALWARE_PROTECTION"), + resource.TestCheckResourceAttr(resource2Name, "additional_configuration.#", "0"), + resource.TestCheckResourceAttr(resource2Name, "auto_enable", "ALL"), + resource.TestCheckResourceAttr(resource2Name, "name", "LAMBDA_NETWORK_LOGS"), + resource.TestCheckResourceAttr(resource3Name, "additional_configuration.#", "0"), + resource.TestCheckResourceAttr(resource3Name, "auto_enable", "ALL"), + resource.TestCheckResourceAttr(resource3Name, "name", "S3_DATA_EVENTS"), + ), + }, + { + Config: testAccOrganizationConfigurationFeatureConfig_multiple("NONE", "NONE", "NONE"), + Check: resource.ComposeTestCheckFunc( + testAccOrganizationConfigurationFeatureExists(ctx, resource1Name), + testAccOrganizationConfigurationFeatureExists(ctx, resource2Name), + testAccOrganizationConfigurationFeatureExists(ctx, resource3Name), + resource.TestCheckResourceAttr(resource1Name, "additional_configuration.#", "0"), + resource.TestCheckResourceAttr(resource1Name, "auto_enable", "NONE"), + resource.TestCheckResourceAttr(resource1Name, "name", "EBS_MALWARE_PROTECTION"), + resource.TestCheckResourceAttr(resource2Name, "additional_configuration.#", "0"), + resource.TestCheckResourceAttr(resource2Name, "auto_enable", "NONE"), + resource.TestCheckResourceAttr(resource2Name, "name", "LAMBDA_NETWORK_LOGS"), + resource.TestCheckResourceAttr(resource3Name, "additional_configuration.#", "0"), + resource.TestCheckResourceAttr(resource3Name, "auto_enable", "NONE"), + resource.TestCheckResourceAttr(resource3Name, "name", "S3_DATA_EVENTS"), + ), + }, + }, + }) +} + +func testAccOrganizationConfigurationFeatureExists(ctx context.Context, n string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + conn := acctest.Provider.Meta().(*conns.AWSClient).GuardDutyConn(ctx) + + _, err := tfguardduty.FindOrganizationConfigurationFeatureByTwoPartKey(ctx, conn, rs.Primary.Attributes["detector_id"], rs.Primary.Attributes["name"]) + + return err + } +} + +var testAccOrganizationConfigurationFeatureConfig_base = acctest.ConfigCompose(testAccOrganizationConfigurationConfig_base, ` +resource "aws_guardduty_organization_configuration" "test" { + depends_on = [aws_guardduty_organization_admin_account.test] + + auto_enable_organization_members = "ALL" + detector_id = aws_guardduty_detector.test.id +} +`) + +func testAccOrganizationConfigurationFeatureConfig_basic(name, autoEnable string) string { + return acctest.ConfigCompose(testAccOrganizationConfigurationFeatureConfig_base, fmt.Sprintf(` +resource "aws_guardduty_organization_configuration_feature" "test" { + depends_on = [aws_guardduty_organization_configuration.test] + + detector_id = aws_guardduty_detector.test.id + name = %[1]q + auto_enable = %[2]q +} +`, name, autoEnable)) +} + +func testAccOrganizationConfigurationFeatureConfig_additionalConfiguration(featureAutoEnable, additionalConfigurationAutoEnable string) string { + return acctest.ConfigCompose(testAccOrganizationConfigurationFeatureConfig_base, fmt.Sprintf(` +resource "aws_guardduty_organization_configuration_feature" "test" { + depends_on = [aws_guardduty_organization_configuration.test] + + detector_id = aws_guardduty_detector.test.id + name = "EKS_RUNTIME_MONITORING" + auto_enable = %[1]q + + additional_configuration { + name = "EKS_ADDON_MANAGEMENT" + auto_enable = %[2]q + } +} +`, featureAutoEnable, additionalConfigurationAutoEnable)) +} + +func testAccOrganizationConfigurationFeatureConfig_multiple(autoEnable1, autoEnable2, autoEnable3 string) string { + return acctest.ConfigCompose(testAccOrganizationConfigurationFeatureConfig_base, fmt.Sprintf(` +resource "aws_guardduty_organization_configuration_feature" "test1" { + depends_on = [aws_guardduty_organization_configuration.test] + + detector_id = aws_guardduty_detector.test.id + name = "EBS_MALWARE_PROTECTION" + auto_enable = %[1]q +} + +resource "aws_guardduty_organization_configuration_feature" "test2" { + depends_on = [aws_guardduty_organization_configuration.test] + + detector_id = aws_guardduty_detector.test.id + name = "LAMBDA_NETWORK_LOGS" + auto_enable = %[2]q +} + +resource "aws_guardduty_organization_configuration_feature" "test3" { + depends_on = [aws_guardduty_organization_configuration.test] + + detector_id = aws_guardduty_detector.test.id + name = "S3_DATA_EVENTS" + auto_enable = %[3]q +} +`, autoEnable1, autoEnable2, autoEnable3)) +} diff --git a/internal/service/guardduty/organization_configuration_test.go b/internal/service/guardduty/organization_configuration_test.go index 94e218f45a7..83e6c21f905 100644 --- a/internal/service/guardduty/organization_configuration_test.go +++ b/internal/service/guardduty/organization_configuration_test.go @@ -295,7 +295,7 @@ func testAccCheckOrganizationConfigurationExists(ctx context.Context, n string) } } -const testAccOrganizationConfigurationConfigBase = ` +const testAccOrganizationConfigurationConfig_base = ` data "aws_caller_identity" "current" {} data "aws_partition" "current" {} @@ -319,9 +319,7 @@ resource "aws_guardduty_organization_admin_account" "test" { ` func testAccOrganizationConfigurationConfig_autoEnable(autoEnable bool) string { - return acctest.ConfigCompose( - testAccOrganizationConfigurationConfigBase, - fmt.Sprintf(` + return acctest.ConfigCompose(testAccOrganizationConfigurationConfig_base, fmt.Sprintf(` resource "aws_guardduty_organization_configuration" "test" { depends_on = [aws_guardduty_organization_admin_account.test] @@ -332,9 +330,7 @@ resource "aws_guardduty_organization_configuration" "test" { } func testAccOrganizationConfigurationConfig_autoEnableOrganizationMembers(value string) string { - return acctest.ConfigCompose( - testAccOrganizationConfigurationConfigBase, - fmt.Sprintf(` + return acctest.ConfigCompose(testAccOrganizationConfigurationConfig_base, fmt.Sprintf(` resource "aws_guardduty_organization_configuration" "test" { depends_on = [aws_guardduty_organization_admin_account.test] @@ -345,9 +341,7 @@ resource "aws_guardduty_organization_configuration" "test" { } func testAccOrganizationConfigurationConfig_s3Logs(autoEnable bool) string { - return acctest.ConfigCompose( - testAccOrganizationConfigurationConfigBase, - fmt.Sprintf(` + return acctest.ConfigCompose(testAccOrganizationConfigurationConfig_base, fmt.Sprintf(` resource "aws_guardduty_organization_configuration" "test" { depends_on = [aws_guardduty_organization_admin_account.test] @@ -364,9 +358,7 @@ resource "aws_guardduty_organization_configuration" "test" { } func testAccOrganizationConfigurationConfig_kubernetes(autoEnable bool) string { - return acctest.ConfigCompose( - testAccOrganizationConfigurationConfigBase, - fmt.Sprintf(` + return acctest.ConfigCompose(testAccOrganizationConfigurationConfig_base, fmt.Sprintf(` resource "aws_guardduty_organization_configuration" "test" { depends_on = [aws_guardduty_organization_admin_account.test] @@ -385,9 +377,7 @@ resource "aws_guardduty_organization_configuration" "test" { } func testAccOrganizationConfigurationConfig_malwareprotection(autoEnable bool) string { - return acctest.ConfigCompose( - testAccOrganizationConfigurationConfigBase, - fmt.Sprintf(` + return acctest.ConfigCompose(testAccOrganizationConfigurationConfig_base, fmt.Sprintf(` resource "aws_guardduty_organization_configuration" "test" { depends_on = [aws_guardduty_organization_admin_account.test] From 109adf133bdd5a93bc39c129f22a39b1c86041b1 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 12 Oct 2023 11:03:10 -0400 Subject: [PATCH 09/12] 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..b81487af89c --- /dev/null +++ b/.changelog/#####.txt @@ -0,0 +1,3 @@ +```release-note:new-resource +aws_guardduty_organization_configuration_feature +``` \ No newline at end of file From 135c427f0c04c5c7a90146c228b075693cce529f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 12 Oct 2023 11:06:59 -0400 Subject: [PATCH 10/12] Correct CHANGELOG entry file name. --- .changelog/{#####.txt => 33913.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{#####.txt => 33913.txt} (100%) diff --git a/.changelog/#####.txt b/.changelog/33913.txt similarity index 100% rename from .changelog/#####.txt rename to .changelog/33913.txt From 347b3939ce7df7940ca8a6c406600b8f62ea7ae5 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 12 Oct 2023 11:10:48 -0400 Subject: [PATCH 11/12] Fix terrafmt errors. --- .../guardduty/organization_configuration_feature_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/guardduty/organization_configuration_feature_test.go b/internal/service/guardduty/organization_configuration_feature_test.go index 10616b4a337..282b57c7853 100644 --- a/internal/service/guardduty/organization_configuration_feature_test.go +++ b/internal/service/guardduty/organization_configuration_feature_test.go @@ -198,10 +198,10 @@ resource "aws_guardduty_organization_configuration_feature" "test" { detector_id = aws_guardduty_detector.test.id name = "EKS_RUNTIME_MONITORING" - auto_enable = %[1]q + auto_enable = %[1]q additional_configuration { - name = "EKS_ADDON_MANAGEMENT" + name = "EKS_ADDON_MANAGEMENT" auto_enable = %[2]q } } From 6956b3b4abb70c60560e0703c8ff6d4f03c33243 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 12 Oct 2023 14:17:23 -0400 Subject: [PATCH 12/12] r/aws_guardduty_organization_configuration(_feature): Add a mutex to ensure that multiple features being updated concurrently don't trample on each other. --- internal/service/guardduty/organization_configuration.go | 5 +++++ .../service/guardduty/organization_configuration_feature.go | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/internal/service/guardduty/organization_configuration.go b/internal/service/guardduty/organization_configuration.go index 74460889b05..676d497b1ab 100644 --- a/internal/service/guardduty/organization_configuration.go +++ b/internal/service/guardduty/organization_configuration.go @@ -179,6 +179,11 @@ func resourceOrganizationConfigurationPut(ctx context.Context, d *schema.Resourc input.DataSources = expandOrganizationDataSourceConfigurations(v.([]interface{})[0].(map[string]interface{})) } + // We have seen occasional acceptance test failures when updating multiple features on the same detector concurrently, + // so use a mutex to ensure that multiple features being updated concurrently don't trample on each other. + conns.GlobalMutexKV.Lock(detectorID) + defer conns.GlobalMutexKV.Unlock(detectorID) + _, err := conn.UpdateOrganizationConfigurationWithContext(ctx, input) if err != nil { diff --git a/internal/service/guardduty/organization_configuration_feature.go b/internal/service/guardduty/organization_configuration_feature.go index 82e63a4f59d..c3ec6809440 100644 --- a/internal/service/guardduty/organization_configuration_feature.go +++ b/internal/service/guardduty/organization_configuration_feature.go @@ -74,6 +74,12 @@ func resourceOrganizationConfigurationFeaturePut(ctx context.Context, d *schema. conn := meta.(*conns.AWSClient).GuardDutyConn(ctx) detectorID := d.Get("detector_id").(string) + + // We have seen occasional acceptance test failures when updating multiple features on the same detector concurrently, + // so use a mutex to ensure that multiple features being updated concurrently don't trample on each other. + conns.GlobalMutexKV.Lock(detectorID) + defer conns.GlobalMutexKV.Unlock(detectorID) + output, err := FindOrganizationConfigurationByID(ctx, conn, detectorID) if err != nil {