Add SecurityHub central organization configuration support

internal/service/securityhub/organization_configuration.go

@@ -5,7 +5,9 @@ package securityhub
 
 import (
 	"context"
+	"fmt"
 	"log"
+	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/securityhub"
@@ -43,6 +45,24 @@ func ResourceOrganizationConfiguration() *schema.Resource {
 				Computed:         true,
 				ValidateDiagFunc: enum.Validate[types.AutoEnableStandards](),
 			},
+			"organization_configuration": {
+				Type:     schema.TypeList,
+				Optional: true,
+				MaxItems: 1,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"configuration_type": {
+							Type:             schema.TypeString,
+							Required:         true,
+							ValidateDiagFunc: enum.Validate[types.OrganizationConfigurationConfigurationType](),
+						},
+						"status": {
+							Type:     schema.TypeString,
+							Computed: true,
+						},
+					},
+				},
+			},
 		},
 	}
 }
@@ -59,6 +79,10 @@ func resourceOrganizationConfigurationUpdate(ctx context.Context, d *schema.Reso
 		input.AutoEnableStandards = types.AutoEnableStandards(v.(string))
 	}
 
+	if v, ok := d.GetOk("organization_configuration"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil {
+		input.OrganizationConfiguration = expandOrganizationConfiguration(v.([]interface{})[0].(map[string]interface{}))
+	}
+
 	_, err := conn.UpdateOrganizationConfiguration(ctx, input)
 
 	if err != nil {
@@ -76,7 +100,7 @@ func resourceOrganizationConfigurationRead(ctx context.Context, d *schema.Resour
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).SecurityHubClient(ctx)
 
-	output, err := FindOrganizationConfiguration(ctx, conn)
+	output, err := WaitOrganizationConfigurationEnabled(ctx, conn, d.Timeout(schema.TimeoutDefault))
 
 	if !d.IsNewResource() && tfresource.NotFound(err) {
 		log.Printf("[WARN] Security Hub Organization Configuration %s not found, removing from state", d.Id())
@@ -91,28 +115,96 @@ func resourceOrganizationConfigurationRead(ctx context.Context, d *schema.Resour
 	d.Set("auto_enable", output.AutoEnable)
 	d.Set("auto_enable_standards", output.AutoEnableStandards)
 
+	if err := d.Set("organization_configuration", []interface{}{flattenOrganizationConfiguration(output.OrganizationConfiguration)}); err != nil {
+		return sdkdiag.AppendErrorf(diags, "setting organization_configuration: %s", err)
+	}
+
 	return diags
 }
 
-func FindOrganizationConfiguration(ctx context.Context, conn *securityhub.Client) (*securityhub.DescribeOrganizationConfigurationOutput, error) {
-	input := &securityhub.DescribeOrganizationConfigurationInput{}
+func findOrganizationConfiguration(ctx context.Context, conn *securityhub.Client) retry.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		input := &securityhub.DescribeOrganizationConfigurationInput{}
+		output, err := conn.DescribeOrganizationConfiguration(ctx, input)
 
-	output, err := conn.DescribeOrganizationConfiguration(ctx, input)
+		if tfawserr.ErrCodeEquals(err, errCodeResourceNotFoundException) || tfawserr.ErrMessageContains(err, errCodeInvalidAccessException, "not subscribed to AWS Security Hub") {
+			return nil, "", &retry.NotFoundError{
+				LastError:   err,
+				LastRequest: input,
+			}
+		}
 
-	if tfawserr.ErrCodeEquals(err, errCodeResourceNotFoundException) || tfawserr.ErrMessageContains(err, errCodeInvalidAccessException, "not subscribed to AWS Security Hub") {
-		return nil, &retry.NotFoundError{
-			LastError:   err,
-			LastRequest: input,
+		if err != nil {
+			return nil, "", err
+		}
+
+		if output == nil || output.OrganizationConfiguration == nil {
+			return nil, "", tfresource.NewEmptyResultError(input)
+		}
+
+		switch output.OrganizationConfiguration.Status {
+		case types.OrganizationConfigurationStatusPending:
+			return nil, "", nil
+		case types.OrganizationConfigurationStatusEnabled, "":
+			return output, string(output.OrganizationConfiguration.Status), nil
+		default:
+			var statusErr error
+			if msg := output.OrganizationConfiguration.StatusMessage; msg != nil && len(*msg) > 0 {
+				statusErr = fmt.Errorf("StatusMessage: %s", *msg)
+			}
+			return nil, "", &retry.UnexpectedStateError{
+				LastError: statusErr,
+				State:     string(output.OrganizationConfiguration.Status),
+				ExpectedState: []string{
+					string(types.OrganizationConfigurationStatusEnabled),
+					string(types.OrganizationConfigurationStatusPending),
+				},
+			}
 		}
 	}
+}
 
-	if err != nil {
-		return nil, err
+func WaitOrganizationConfigurationEnabled(ctx context.Context, conn *securityhub.Client, timeout time.Duration) (*securityhub.DescribeOrganizationConfigurationOutput, error) {
+	stateConf := &retry.StateChangeConf{
+		Pending:                   enum.Slice(types.OrganizationConfigurationStatusPending),
+		Target:                    append(enum.Slice(types.OrganizationConfigurationStatusEnabled), ""),
+		Refresh:                   findOrganizationConfiguration(ctx, conn),
+		Timeout:                   timeout,
+		NotFoundChecks:            20,
+		ContinuousTargetOccurence: 2,
+	}
+
+	outputRaw, err := stateConf.WaitForStateContext(ctx)
+	if out, ok := outputRaw.(*securityhub.DescribeOrganizationConfigurationOutput); ok {
+		return out, err
+	}
+
+	return nil, err
+}
+
+func expandOrganizationConfiguration(tfMap map[string]interface{}) *types.OrganizationConfiguration {
+	if tfMap == nil {
+		return nil
+	}
+
+	apiObject := &types.OrganizationConfiguration{}
+
+	if v, ok := tfMap["configuration_type"].(string); ok && len(v) > 0 {
+		apiObject.ConfigurationType = types.OrganizationConfigurationConfigurationType(v)
+	}
+
+	return apiObject
+}
+
+func flattenOrganizationConfiguration(apiObject *types.OrganizationConfiguration) map[string]interface{} {
+	if apiObject == nil {
+		return nil
 	}
 
-	if output == nil || output.OrganizationConfiguration == nil {
-		return nil, tfresource.NewEmptyResultError(input)
+	tfMap := map[string]interface{}{
+		"configuration_type": apiObject.ConfigurationType,
+		"status":             apiObject.Status,
 	}
 
-	return output, nil
+	return tfMap
 }

internal/service/securityhub/organization_configuration_test.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
@@ -32,6 +33,7 @@ func testAccOrganizationConfiguration_basic(t *testing.T) {
 					testAccOrganizationConfigurationExists(ctx, resourceName),
 					resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"),
 					resource.TestCheckResourceAttr(resourceName, "auto_enable_standards", "DEFAULT"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.#", "0"),
 				),
 			},
 			{
@@ -45,6 +47,7 @@ func testAccOrganizationConfiguration_basic(t *testing.T) {
 					testAccOrganizationConfigurationExists(ctx, resourceName),
 					resource.TestCheckResourceAttr(resourceName, "auto_enable", "false"),
 					resource.TestCheckResourceAttr(resourceName, "auto_enable_standards", "DEFAULT"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.#", "0"),
 				),
 			},
 		},
@@ -67,6 +70,7 @@ func testAccOrganizationConfiguration_autoEnableStandards(t *testing.T) {
 					testAccOrganizationConfigurationExists(ctx, resourceName),
 					resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"),
 					resource.TestCheckResourceAttr(resourceName, "auto_enable_standards", "DEFAULT"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.#", "0"),
 				),
 			},
 			{
@@ -80,6 +84,87 @@ func testAccOrganizationConfiguration_autoEnableStandards(t *testing.T) {
 					testAccOrganizationConfigurationExists(ctx, resourceName),
 					resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"),
 					resource.TestCheckResourceAttr(resourceName, "auto_enable_standards", "NONE"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.#", "0"),
+				),
+			},
+		},
+	})
+}
+
+// CENTRAL configuration has unique set of constraints vs other SecurityHub organization_config:
+//
+// 1. Must be done from a *member* delegated admin account:
+// "Central configuration couldn't be enabled because the organization management account is designated as the delegated Security Hub administrator account."
+// "Designate a different account as the delegated administrator, and retry."
+//
+// To allow for this the following is a multi-account test:
+// The primary provider is expected to be a member account and alternate provider a management account.
+//
+// 2. Dependencies on DelegatedAdmin and FindingAggregators invert (!) after central config is created.
+// "Finding Aggregator must be created to enable Central Configuration"
+// "You must [...] disable central configuration in order to remove or change your aggregation Region."
+// "You must [...] disable central configuration in order to remove or change the delegated Security Hub administrator"
+//
+// Due to this API behaviour, this resource isn't very terraform friendly.
+func TestAccOrganizationConfiguration_centralConfiguration(t *testing.T) {
+	ctx := acctest.Context(t)
+	resourceName := "aws_securityhub_organization_configuration.test"
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() {
+			acctest.PreCheck(ctx, t)
+			acctest.PreCheckAlternateAccount(t)
+			acctest.PreCheckAlternateRegionIs(t, acctest.Region())
+			acctest.PreCheckOrganizationMemberAccount(ctx, t)
+		},
+		ErrorCheck:               acctest.ErrorCheck(t, names.SecurityHubEndpointID),
+		ProtoV5ProviderFactories: acctest.ProtoV5FactoriesAlternate(ctx, t),
+		CheckDestroy:             acctest.CheckDestroyNoop,
+		Steps: []resource.TestStep{
+			{
+				// Start with LOCAL e.g default behaviour
+				// This allows us to create an finding_aggregator in test without breaking dependency flow on destroy
+				Config: testAccOrganizationConfigurationConfig_centralConfiguration(true, "DEFAULT", "LOCAL"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccOrganizationConfigurationExists(ctx, resourceName),
+					resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"),
+					resource.TestCheckResourceAttr(resourceName, "auto_enable_standards", "DEFAULT"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.0.configuration_type", "LOCAL"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.0.status", "ENABLED"),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			// Enable CENTRAL configuration
+			{
+				Config: testAccOrganizationConfigurationConfig_centralConfiguration(false, "NONE", "CENTRAL"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccOrganizationConfigurationExists(ctx, resourceName),
+					resource.TestCheckResourceAttr(resourceName, "auto_enable", "false"),
+					resource.TestCheckResourceAttr(resourceName, "auto_enable_standards", "NONE"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.0.configuration_type", "CENTRAL"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.0.status", "ENABLED"),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			// Go back to LOCAL; this allows us to destroy the delegated admin with the given dependency flow that is necessary for creates.
+			{
+				Config: testAccOrganizationConfigurationConfig_centralConfiguration(true, "DEFAULT", "LOCAL"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccOrganizationConfigurationExists(ctx, resourceName),
+					resource.TestCheckResourceAttr(resourceName, "auto_enable", "true"),
+					resource.TestCheckResourceAttr(resourceName, "auto_enable_standards", "DEFAULT"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.0.configuration_type", "LOCAL"),
+					resource.TestCheckResourceAttr(resourceName, "organization_configuration.0.status", "ENABLED"),
 				),
 			},
 		},
@@ -95,7 +180,7 @@ func testAccOrganizationConfigurationExists(ctx context.Context, n string) resou
 
 		conn := acctest.Provider.Meta().(*conns.AWSClient).SecurityHubClient(ctx)
 
-		_, err := tfsecurityhub.FindOrganizationConfiguration(ctx, conn)
+		_, err := tfsecurityhub.WaitOrganizationConfigurationEnabled(ctx, conn, 2*time.Minute)
 
 		return err
 	}
@@ -133,3 +218,33 @@ resource "aws_securityhub_organization_configuration" "test" {
 }
 `, autoEnableStandards))
 }
+
+func testAccOrganizationConfigurationConfig_centralConfiguration(autoEnable bool, autoEnableStandards, configType string) string {
+	return acctest.ConfigCompose(
+		acctest.ConfigAlternateAccountProvider(),
+		fmt.Sprintf(`
+data "aws_caller_identity" "member" {}
+
+resource "aws_securityhub_organization_admin_account" "test" {
+  admin_account_id = data.aws_caller_identity.member.account_id
+
+  provider = awsalternate
+}
+
+resource "aws_securityhub_finding_aggregator" "test" {
+  linking_mode = "ALL_REGIONS"
+
+  depends_on = [aws_securityhub_organization_admin_account.test]
+}
+
+resource "aws_securityhub_organization_configuration" "test" {
+  auto_enable           = %[1]t
+  auto_enable_standards = %[2]q
+  organization_configuration {
+    configuration_type = %[3]q
+  }
+
+  depends_on = [aws_securityhub_organization_admin_account.test]
+}
+`, autoEnable, autoEnableStandards, configType))
+}

internal/service/securityhub/securityhub_test.go

@@ -53,8 +53,9 @@ func TestAccSecurityHub_serial(t *testing.T) {
 			"MultiRegion": testAccOrganizationAdminAccount_MultiRegion,
 		},
 		"OrganizationConfiguration": {
-			"basic":               testAccOrganizationConfiguration_basic,
-			"AutoEnableStandards": testAccOrganizationConfiguration_autoEnableStandards,
+			"basic":                testAccOrganizationConfiguration_basic,
+			"AutoEnableStandards":  testAccOrganizationConfiguration_autoEnableStandards,
+			"CentralConfiguration": TestAccOrganizationConfiguration_centralConfiguration,
 		},
 		"ProductSubscription": {
 			"basic": testAccProductSubscription_basic,