diff --git a/.changelog/32860.txt b/.changelog/32860.txt new file mode 100644 index 00000000000..30aedd6841f --- /dev/null +++ b/.changelog/32860.txt @@ -0,0 +1,7 @@ +```release-note:enhancement +resource/aws_fms_admin_account: Add configurable timeouts +``` + +```release-note:bug +resource/aws_fms_policy: Prevent erroneous diffs on `security_service_policy_data.managed_service_data` +``` \ No newline at end of file diff --git a/internal/service/fms/admin_account.go b/internal/service/fms/admin_account.go index 26fb7501230..2da51479c4f 100644 --- a/internal/service/fms/admin_account.go +++ b/internal/service/fms/admin_account.go @@ -16,11 +16,12 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "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" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) // @SDKResource("aws_fms_admin_account") -func ResourceAdminAccount() *schema.Resource { +func resourceAdminAccount() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceAdminAccountCreate, ReadWithoutTimeout: resourceAdminAccountRead, @@ -30,6 +31,11 @@ func ResourceAdminAccount() *schema.Resource { StateContext: schema.ImportStatePassthroughContext, }, + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(30 * time.Minute), + Delete: schema.DefaultTimeout(10 * time.Minute), + }, + Schema: map[string]*schema.Schema{ "account_id": { Type: schema.TypeString, @@ -46,36 +52,20 @@ func resourceAdminAccountCreate(ctx context.Context, d *schema.ResourceData, met var diags diag.Diagnostics conn := meta.(*conns.AWSClient).FMSConn(ctx) - // Ensure there is not an existing FMS Admin Account - output, err := conn.GetAdminAccountWithContext(ctx, &fms.GetAdminAccountInput{}) - - if err != nil && !tfawserr.ErrCodeEquals(err, fms.ErrCodeResourceNotFoundException) { - return sdkdiag.AppendErrorf(diags, "getting FMS Admin Account: %s", err) - } + // Ensure there is not an existing FMS Admin Account. + output, err := findAdminAccount(ctx, conn) - if output != nil && output.AdminAccount != nil && aws.StringValue(output.RoleStatus) == fms.AccountRoleStatusReady { + if !tfresource.NotFound(err) { return sdkdiag.AppendErrorf(diags, "FMS Admin Account (%s) already associated: import this Terraform resource to manage", aws.StringValue(output.AdminAccount)) } accountID := meta.(*conns.AWSClient).AccountID - if v, ok := d.GetOk("account_id"); ok { accountID = v.(string) } - stateConf := &retry.StateChangeConf{ - Pending: []string{ - fms.AccountRoleStatusDeleted, // Recreating association can return this status - fms.AccountRoleStatusCreating, - }, - Target: []string{fms.AccountRoleStatusReady}, - Refresh: associateAdminAccountRefreshFunc(ctx, conn, accountID), - Timeout: 30 * time.Minute, - Delay: 10 * time.Second, - } - - if _, err := stateConf.WaitForStateContext(ctx); err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for FMS Admin Account (%s) association: %s", accountID, err) + if _, err := waitAdminAccountCreated(ctx, conn, accountID, d.Timeout(schema.TimeoutCreate)); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for FMS Admin Account (%s) create: %s", d.Id(), err) } d.SetId(accountID) @@ -83,64 +73,20 @@ func resourceAdminAccountCreate(ctx context.Context, d *schema.ResourceData, met return append(diags, resourceAdminAccountRead(ctx, d, meta)...) } -func associateAdminAccountRefreshFunc(ctx context.Context, conn *fms.FMS, accountId string) retry.StateRefreshFunc { - // This is all wrapped in a refresh func since AssociateAdminAccount returns - // success even though it failed if called too quickly after creating an organization - return func() (interface{}, string, error) { - req := &fms.AssociateAdminAccountInput{ - AdminAccount: aws.String(accountId), - } - - _, aserr := conn.AssociateAdminAccountWithContext(ctx, req) - if aserr != nil { - return nil, "", aserr - } - - res, err := conn.GetAdminAccountWithContext(ctx, &fms.GetAdminAccountInput{}) - if err != nil { - // FMS returns an AccessDeniedException if no account is associated, - // but does not define this in its error codes - if tfawserr.ErrMessageContains(err, "AccessDeniedException", "is not currently delegated by AWS FM") { - return nil, "", nil - } - if tfawserr.ErrCodeEquals(err, fms.ErrCodeResourceNotFoundException) { - return nil, "", nil - } - return nil, "", err - } - - if aws.StringValue(res.AdminAccount) != accountId { - return nil, "", nil - } - - return res, aws.StringValue(res.RoleStatus), err - } -} - func resourceAdminAccountRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).FMSConn(ctx) - output, err := conn.GetAdminAccountWithContext(ctx, &fms.GetAdminAccountInput{}) + output, err := findAdminAccount(ctx, conn) - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, fms.ErrCodeResourceNotFoundException) { + if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] FMS Admin Account (%s) not found, removing from state", d.Id()) d.SetId("") return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "getting FMS Admin Account (%s): %s", d.Id(), err) - } - - if aws.StringValue(output.RoleStatus) == fms.AccountRoleStatusDeleted { - if d.IsNewResource() { - return sdkdiag.AppendErrorf(diags, "getting FMS Admin Account (%s): %s after creation", d.Id(), aws.StringValue(output.RoleStatus)) - } - - log.Printf("[WARN] FMS Admin Account (%s) not found, removing from state", d.Id()) - d.SetId("") - return diags + return sdkdiag.AppendErrorf(diags, "reading FMS Admin Account (%s): %s", d.Id(), err) } d.Set("account_id", output.AdminAccount) @@ -158,39 +104,136 @@ func resourceAdminAccountDelete(ctx context.Context, d *schema.ResourceData, met return sdkdiag.AppendErrorf(diags, "disassociating FMS Admin Account (%s): %s", d.Id(), err) } - if err := waitForAdminAccountDeletion(ctx, conn); err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for FMS Admin Account (%s) disassociation: %s", d.Id(), err) + if _, err := waitAdminAccountDeleted(ctx, conn, d.Timeout(schema.TimeoutDelete)); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for FMS Admin Account (%s) delete: %s", d.Id(), err) } return diags } -func waitForAdminAccountDeletion(ctx context.Context, conn *fms.FMS) error { +func findAdminAccount(ctx context.Context, conn *fms.FMS) (*fms.GetAdminAccountOutput, error) { + input := &fms.GetAdminAccountInput{} + + output, err := conn.GetAdminAccountWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, fms.ErrCodeResourceNotFoundException) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + if status := aws.StringValue(output.RoleStatus); status == fms.AccountRoleStatusDeleted { + return nil, &retry.NotFoundError{ + Message: status, + LastRequest: input, + } + } + + return output, nil +} + +func statusAssociateAdminAccount(ctx context.Context, conn *fms.FMS, accountID string) retry.StateRefreshFunc { + // This is all wrapped in a StateRefreshFunc since AssociateAdminAccount returns + // success even though it failed if called too quickly after creating an Organization. + return func() (interface{}, string, error) { + input := &fms.AssociateAdminAccountInput{ + AdminAccount: aws.String(accountID), + } + + _, err := conn.AssociateAdminAccountWithContext(ctx, input) + + if err != nil { + return nil, "", err + } + + output, err := conn.GetAdminAccountWithContext(ctx, &fms.GetAdminAccountInput{}) + + // FMS returns an AccessDeniedException if no account is associated, + // but does not define this in its error codes. + if tfawserr.ErrMessageContains(err, "AccessDeniedException", "is not currently delegated by AWS FM") { + return nil, "", nil + } + + if tfawserr.ErrCodeEquals(err, fms.ErrCodeResourceNotFoundException) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + if aws.StringValue(output.AdminAccount) != accountID { + return nil, "", nil + } + + return output, aws.StringValue(output.RoleStatus), err + } +} + +func statusAdminAccount(ctx context.Context, conn *fms.FMS) retry.StateRefreshFunc { + return func() (interface{}, string, error) { + output, err := findAdminAccount(ctx, conn) + + if tfresource.NotFound(err) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + return output, aws.StringValue(output.RoleStatus), nil + } +} + +func waitAdminAccountCreated(ctx context.Context, conn *fms.FMS, accountID string, timeout time.Duration) (*fms.GetAdminAccountOutput, error) { stateConf := &retry.StateChangeConf{ Pending: []string{ - fms.AccountRoleStatusDeleting, - fms.AccountRoleStatusPendingDeletion, - fms.AccountRoleStatusReady, + fms.AccountRoleStatusDeleted, // Recreating association can return this status. + fms.AccountRoleStatusCreating, }, - Target: []string{fms.AccountRoleStatusDeleted}, - Refresh: func() (interface{}, string, error) { - output, err := conn.GetAdminAccountWithContext(ctx, &fms.GetAdminAccountInput{}) + Target: []string{fms.AccountRoleStatusReady}, + Refresh: statusAssociateAdminAccount(ctx, conn, accountID), + Timeout: timeout, + Delay: 10 * time.Second, + } - if tfawserr.ErrCodeEquals(err, fms.ErrCodeResourceNotFoundException) { - return output, fms.AccountRoleStatusDeleted, nil - } + outputRaw, err := stateConf.WaitForStateContext(ctx) - if err != nil { - return nil, "", err - } + if output, ok := outputRaw.(*fms.GetAdminAccountOutput); ok { + return output, err + } - return output, aws.StringValue(output.RoleStatus), nil + return nil, err +} + +func waitAdminAccountDeleted(ctx context.Context, conn *fms.FMS, timeout time.Duration) (*fms.GetAdminAccountOutput, error) { + stateConf := &retry.StateChangeConf{ + Pending: []string{ + fms.AccountRoleStatusDeleting, + fms.AccountRoleStatusPendingDeletion, + fms.AccountRoleStatusReady, }, - Timeout: 10 * time.Minute, + Target: []string{}, + Refresh: statusAdminAccount(ctx, conn), + Timeout: timeout, Delay: 10 * time.Second, } - _, err := stateConf.WaitForStateContext(ctx) + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if output, ok := outputRaw.(*fms.GetAdminAccountOutput); ok { + return output, err + } - return err + return nil, err } diff --git a/internal/service/fms/admin_account_test.go b/internal/service/fms/admin_account_test.go index de1ffb92819..fddee37d089 100644 --- a/internal/service/fms/admin_account_test.go +++ b/internal/service/fms/admin_account_test.go @@ -8,14 +8,14 @@ import ( "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/service/fms" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "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" + tffms "github.com/hashicorp/terraform-provider-aws/internal/service/fms" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func testAccAdminAccount_basic(t *testing.T) { @@ -26,7 +26,8 @@ func testAccAdminAccount_basic(t *testing.T) { PreCheck: func() { acctest.PreCheck(ctx, t) acctest.PreCheckRegion(t, endpoints.UsEast1RegionID) - acctest.PreCheckOrganizationsAccount(ctx, t) + acctest.PreCheckOrganizationsEnabled(ctx, t) + acctest.PreCheckOrganizationManagementAccount(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, fms.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, @@ -42,6 +43,33 @@ func testAccAdminAccount_basic(t *testing.T) { }) } +func testAccAdminAccount_disappears(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_fms_admin_account.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckRegion(t, endpoints.UsEast1RegionID) + acctest.PreCheckOrganizationsEnabled(ctx, t) + acctest.PreCheckOrganizationManagementAccount(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, fms.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckAdminAccountDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccAdminAccountConfig_basic, + Check: resource.ComposeTestCheckFunc( + acctest.CheckResourceAttrAccountID(resourceName, "account_id"), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tffms.ResourceAdminAccount(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func testAccCheckAdminAccountDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).FMSConn(ctx) @@ -51,9 +79,9 @@ func testAccCheckAdminAccountDestroy(ctx context.Context) resource.TestCheckFunc continue } - output, err := conn.GetAdminAccountWithContext(ctx, &fms.GetAdminAccountInput{}) + _, err := tffms.FindAdminAccount(ctx, conn) - if tfawserr.ErrCodeEquals(err, fms.ErrCodeResourceNotFoundException) { + if tfresource.NotFound(err) { continue } @@ -61,11 +89,7 @@ func testAccCheckAdminAccountDestroy(ctx context.Context) resource.TestCheckFunc return err } - if aws.StringValue(output.RoleStatus) == fms.AccountRoleStatusDeleted { - continue - } - - return fmt.Errorf("FMS Admin Account (%s) still exists with status: %s", aws.StringValue(output.AdminAccount), aws.StringValue(output.RoleStatus)) + return fmt.Errorf("FMS Admin Account %s still exists", rs.Primary.ID) } return nil @@ -73,14 +97,9 @@ func testAccCheckAdminAccountDestroy(ctx context.Context) resource.TestCheckFunc } const testAccAdminAccountConfig_basic = ` -data "aws_partition" "current" {} - -resource "aws_organizations_organization" "test" { - aws_service_access_principals = ["fms.${data.aws_partition.current.dns_suffix}"] - feature_set = "ALL" -} +data "aws_caller_identity" "current" {} resource "aws_fms_admin_account" "test" { - account_id = aws_organizations_organization.test.master_account_id + account_id = data.aws_caller_identity.current.account_id } ` diff --git a/internal/service/fms/exports_test.go b/internal/service/fms/exports_test.go new file mode 100644 index 00000000000..42bd8e6b323 --- /dev/null +++ b/internal/service/fms/exports_test.go @@ -0,0 +1,14 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package fms + +// Exports for use in tests only. +var ( + ResourceAdminAccount = resourceAdminAccount + ResourcePolicy = resourcePolicy + + FindAdminAccount = findAdminAccount + FindPolicyByID = findPolicyByID + RemoveEmptyFieldsFromJSON = removeEmptyFieldsFromJSON +) diff --git a/internal/service/fms/fms_test.go b/internal/service/fms/fms_test.go index 64f2f215b71..c80a0d02bc3 100644 --- a/internal/service/fms/fms_test.go +++ b/internal/service/fms/fms_test.go @@ -14,16 +14,20 @@ func TestAccFMS_serial(t *testing.T) { testCases := map[string]map[string]func(t *testing.T){ "AdminAccount": { - "basic": testAccAdminAccount_basic, + "basic": testAccAdminAccount_basic, + "disappears": testAccAdminAccount_disappears, }, "Policy": { + "alb": testAccPolicy_alb, "basic": testAccPolicy_basic, "cloudfrontDistribution": testAccPolicy_cloudFrontDistribution, + "disappears": testAccPolicy_disappears, "includeMap": testAccPolicy_includeMap, - "update": testAccPolicy_update, "policyOption": testAccPolicy_policyOption, "resourceTags": testAccPolicy_resourceTags, + "securityGroup": testAccPolicy_securityGroup, "tags": testAccPolicy_tags, + "update": testAccPolicy_update, }, } diff --git a/internal/service/fms/managed_service_data.go b/internal/service/fms/managed_service_data.go new file mode 100644 index 00000000000..46591259947 --- /dev/null +++ b/internal/service/fms/managed_service_data.go @@ -0,0 +1,71 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package fms + +import ( + "encoding/json" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-provider-aws/internal/service/fms/ujson" + "github.com/hashicorp/terraform-provider-aws/internal/verify" +) + +// suppressEquivalentManagedServiceDataJSON provides custom difference suppression +// for strings that are equivalent once empty fields have been removed. +func suppressEquivalentManagedServiceDataJSON(k, old, new string, d *schema.ResourceData) bool { + if !json.Valid([]byte(old)) || !json.Valid([]byte(new)) { + return old == new + } + + old, new = removeEmptyFieldsFromJSON(old), removeEmptyFieldsFromJSON(new) + + return verify.JSONStringsEqual(old, new) +} + +// removeEmptyFieldsFromJSON removes `null` and empty array (`[]`) fields from a valid JSON string. +func removeEmptyFieldsFromJSON(in string) string { + out := make([]byte, 0, len(in)) + lenBeforeArray := 0 + + err := ujson.Walk([]byte(in), func(_ int, key, value []byte) bool { + n := len(out) + + // For valid JSON, value will never be empty. + skip := false + switch value[0] { + case 'n': // Null (null) + skip = true + case '[': // Start of array + lenBeforeArray = n + case ']': // End of array + if out[n-1] == '[' { + // Truncate output. + out = out[:lenBeforeArray] + lenBeforeArray = 0 + skip = true + } + } + + if skip { + return false + } + + if n != 0 && ujson.ShouldAddComma(value, out[n-1]) { + out = append(out, ',') + } + if len(key) > 0 { + out = append(out, key...) + out = append(out, ':') + } + out = append(out, value...) + + return true + }) + + if err != nil { + return "" + } + + return string(out) +} diff --git a/internal/service/fms/managed_service_data_test.go b/internal/service/fms/managed_service_data_test.go new file mode 100644 index 00000000000..035550c29b3 --- /dev/null +++ b/internal/service/fms/managed_service_data_test.go @@ -0,0 +1,72 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package fms_test + +import ( + "testing" + + tffms "github.com/hashicorp/terraform-provider-aws/internal/service/fms" +) + +func TestRemoveEmptyFieldsFromJSON(t *testing.T) { + t.Parallel() + + testCases := []struct { + testName string + input string + want string + }{ + { + testName: "empty JSON", + input: "{}", + want: "{}", + }, + { + testName: "single field", + input: `{ "key": 42 }`, + want: `{"key":42}`, + }, + { + testName: "single null field", + input: `{"key": null}`, + want: `{}`, + }, + { + testName: "single empty array field", + input: `{"key": []}`, + want: `{}`, + }, + { + testName: "null field and empty array field", + input: `{"key1": [], "key2": null}`, + want: `{}`, + }, + { + testName: "AWS DNS_FIREWALL example", + input: "{\"type\":\"DNS_FIREWALL\",\"preProcessRuleGroups\":[{\"ruleGroupId\":\"rslvr-frg-1\",\"priority\":10}],\"postProcessRuleGroups\":[{\"ruleGroupId\":\"rslvr-frg-2\",\"priority\":9911}]}", + want: "{\"type\":\"DNS_FIREWALL\",\"preProcessRuleGroups\":[{\"ruleGroupId\":\"rslvr-frg-1\",\"priority\":10}],\"postProcessRuleGroups\":[{\"ruleGroupId\":\"rslvr-frg-2\",\"priority\":9911}]}", + }, + { + testName: "AWS IMPORT_NETWORK_FIREWALL example", + input: "{\"type\":\"IMPORT_NETWORK_FIREWALL\",\"awsNetworkFirewallConfig\":{\"networkFirewallStatelessRuleGroupReferences\":[{\"resourceARN\":\"arn:aws:network-firewall:us-west-2:000000000000:stateless-rulegroup/rg1\",\"priority\":1}],\"networkFirewallStatelessDefaultActions\":[\"aws:drop\"],\"networkFirewallStatelessFragmentDefaultActions\":[\"aws:pass\"],\"networkFirewallStatelessCustomActions\":[],\"networkFirewallStatefulRuleGroupReferences\":[{\"resourceARN\":\"arn:aws:network-firewall:us-west-2:aws-managed:stateful-rulegroup/ThreatSignaturesEmergingEventsStrictOrder\",\"priority\":8}],\"networkFirewallStatefulEngineOptions\":{\"ruleOrder\":\"STRICT_ORDER\"},\"networkFirewallStatefulDefaultActions\":[\"aws:drop_strict\"]}}", //lintignore:AWSAT003,AWSAT005 + want: "{\"type\":\"IMPORT_NETWORK_FIREWALL\",\"awsNetworkFirewallConfig\":{\"networkFirewallStatelessRuleGroupReferences\":[{\"resourceARN\":\"arn:aws:network-firewall:us-west-2:000000000000:stateless-rulegroup/rg1\",\"priority\":1}],\"networkFirewallStatelessDefaultActions\":[\"aws:drop\"],\"networkFirewallStatelessFragmentDefaultActions\":[\"aws:pass\"],\"networkFirewallStatefulRuleGroupReferences\":[{\"resourceARN\":\"arn:aws:network-firewall:us-west-2:aws-managed:stateful-rulegroup/ThreatSignaturesEmergingEventsStrictOrder\",\"priority\":8}],\"networkFirewallStatefulEngineOptions\":{\"ruleOrder\":\"STRICT_ORDER\"},\"networkFirewallStatefulDefaultActions\":[\"aws:drop_strict\"]}}", //lintignore:AWSAT003,AWSAT005 + }, + { + testName: "AWS NETWORK_FIREWALL example", + input: "{\"type\":\"NETWORK_FIREWALL\",\"awsNetworkFirewallConfig\":{\"networkFirewallStatelessRuleGroupReferences\":[{\"resourceARN\":\"arn:aws:network-firewall:us-east-1:123456789011:stateless-rulegroup/test\",\"priority\":1}],\"networkFirewallStatelessDefaultActions\":[\"aws:forward_to_sfe\",\"customActionName\"],\"networkFirewallStatelessFragmentDefaultActions\":[\"aws:forward_to_sfe\",\"customActionName\"],\"networkFirewallStatelessCustomActions\":[{\"actionName\":\"customActionName\",\"actionDefinition\":{\"publishMetricAction\":{\"dimensions\":[{\"value\":\"metricdimensionvalue\"}]}}}],\"networkFirewallStatefulRuleGroupReferences\":[{\"resourceARN\":\"arn:aws:network-firewall:us-east-1:123456789011:stateful-rulegroup/test\"}],\"networkFirewallLoggingConfiguration\":{\"logDestinationConfigs\":[{\"logDestinationType\":\"S3\",\"logType\":\"ALERT\",\"logDestination\":{\"bucketName\":\"s3-bucket-name\"}},{\"logDestinationType\":\"S3\",\"logType\":\"FLOW\",\"logDestination\":{\"bucketName\":\"s3-bucket-name\"}}],\"overrideExistingConfig\":true}},\"firewallDeploymentModel\":{\"centralizedFirewallDeploymentModel\":{\"centralizedFirewallOrchestrationConfig\":{\"inspectionVpcIds\":[{\"resourceId\":\"vpc-1234\",\"accountId\":\"123456789011\"}],\"firewallCreationConfig\":{\"endpointLocation\":{\"availabilityZoneConfigList\":[{\"availabilityZoneId\":null,\"availabilityZoneName\":\"us-east-1a\",\"allowedIPV4CidrList\":[\"10.0.0.0/28\"]}]}},\"allowedIPV4CidrList\":[]}}}}", //lintignore:AWSAT003,AWSAT005 + want: "{\"type\":\"NETWORK_FIREWALL\",\"awsNetworkFirewallConfig\":{\"networkFirewallStatelessRuleGroupReferences\":[{\"resourceARN\":\"arn:aws:network-firewall:us-east-1:123456789011:stateless-rulegroup/test\",\"priority\":1}],\"networkFirewallStatelessDefaultActions\":[\"aws:forward_to_sfe\",\"customActionName\"],\"networkFirewallStatelessFragmentDefaultActions\":[\"aws:forward_to_sfe\",\"customActionName\"],\"networkFirewallStatelessCustomActions\":[{\"actionName\":\"customActionName\",\"actionDefinition\":{\"publishMetricAction\":{\"dimensions\":[{\"value\":\"metricdimensionvalue\"}]}}}],\"networkFirewallStatefulRuleGroupReferences\":[{\"resourceARN\":\"arn:aws:network-firewall:us-east-1:123456789011:stateful-rulegroup/test\"}],\"networkFirewallLoggingConfiguration\":{\"logDestinationConfigs\":[{\"logDestinationType\":\"S3\",\"logType\":\"ALERT\",\"logDestination\":{\"bucketName\":\"s3-bucket-name\"}},{\"logDestinationType\":\"S3\",\"logType\":\"FLOW\",\"logDestination\":{\"bucketName\":\"s3-bucket-name\"}}],\"overrideExistingConfig\":true}},\"firewallDeploymentModel\":{\"centralizedFirewallDeploymentModel\":{\"centralizedFirewallOrchestrationConfig\":{\"inspectionVpcIds\":[{\"resourceId\":\"vpc-1234\",\"accountId\":\"123456789011\"}],\"firewallCreationConfig\":{\"endpointLocation\":{\"availabilityZoneConfigList\":[{\"availabilityZoneName\":\"us-east-1a\",\"allowedIPV4CidrList\":[\"10.0.0.0/28\"]}]}}}}}}", //lintignore:AWSAT003,AWSAT005 + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.testName, func(t *testing.T) { + t.Parallel() + + if got, want := tffms.RemoveEmptyFieldsFromJSON(testCase.input), testCase.want; got != want { + t.Errorf("RemoveEmptyFieldsFromJSON(%q) = %q, want %q", testCase.input, got, want) + } + }) + } +} diff --git a/internal/service/fms/policy.go b/internal/service/fms/policy.go index 084e5304ce7..90e8899960c 100644 --- a/internal/service/fms/policy.go +++ b/internal/service/fms/policy.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "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/structure" "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" @@ -24,13 +25,9 @@ import ( "github.com/hashicorp/terraform-provider-aws/names" ) -const ( - ResNamePolicy = "Policy" -) - // @SDKResource("aws_fms_policy", name="Policy") // @Tags(identifierAttribute="arn") -func ResourcePolicy() *schema.Resource { +func resourcePolicy() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourcePolicyCreate, ReadWithoutTimeout: resourcePolicyRead, @@ -151,9 +148,15 @@ func ResourcePolicy() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "managed_service_data": { - Type: schema.TypeString, - Optional: true, - DiffSuppressFunc: verify.SuppressEquivalentJSONDiffs, + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringIsJSON, + DiffSuppressFunc: suppressEquivalentManagedServiceDataJSON, + DiffSuppressOnRefresh: true, + StateFunc: func(v interface{}) string { + json, _ := structure.NormalizeJsonString(v) + return json + }, }, "policy_option": { Type: schema.TypeList, @@ -229,7 +232,7 @@ func resourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta interf var diags diag.Diagnostics conn := meta.(*conns.AWSClient).FMSConn(ctx) - output, err := FindPolicyByID(ctx, conn, d.Id()) + output, err := findPolicyByID(ctx, conn, d.Id()) if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] FMS Policy %s not found, removing from state", d.Id()) @@ -315,7 +318,7 @@ func resourcePolicyDelete(ctx context.Context, d *schema.ResourceData, meta inte return diags } -func FindPolicyByID(ctx context.Context, conn *fms.FMS, id string) (*fms.GetPolicyOutput, error) { +func findPolicyByID(ctx context.Context, conn *fms.FMS, id string) (*fms.GetPolicyOutput, error) { input := &fms.GetPolicyInput{ PolicyId: aws.String(id), } diff --git a/internal/service/fms/policy_test.go b/internal/service/fms/policy_test.go index e2ad4e17be5..686319c73ee 100644 --- a/internal/service/fms/policy_test.go +++ b/internal/service/fms/policy_test.go @@ -57,6 +57,34 @@ func testAccPolicy_basic(t *testing.T) { }) } +func testAccPolicy_disappears(t *testing.T) { + ctx := acctest.Context(t) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_fms_policy.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckRegion(t, endpoints.UsEast1RegionID) + acctest.PreCheckOrganizationsEnabled(ctx, t) + acctest.PreCheckOrganizationManagementAccount(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, fms.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckPolicyDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccPolicyConfig_basic(rName, rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckPolicyExists(ctx, resourceName), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tffms.ResourcePolicy(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func testAccPolicy_cloudFrontDistribution(t *testing.T) { ctx := acctest.Context(t) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -158,6 +186,8 @@ func testAccPolicy_update(t *testing.T) { } func testAccPolicy_policyOption(t *testing.T) { + acctest.Skip(t, "PolicyOption not returned from AWS API") + ctx := acctest.Context(t) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_fms_policy.test" @@ -180,11 +210,11 @@ func testAccPolicy_policyOption(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "delete_unused_fm_managed_resources", "false"), resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.#", "1"), - resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.policy_option.#", "1"), - resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.policy_option.0.network_firewall_policy.#", "1"), - resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.policy_option.0.network_firewall_policy.0.firewall_deployment_model", "CENTRALIZED"), - resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.policy_option.0.third_party_firewall_policy.#", "1"), - resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.policy_option.0.third_party_firewall_policy.0.firewall_deployment_model", "DISTRIBUTED"), + resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.0.policy_option.#", "1"), + resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.0.policy_option.0.network_firewall_policy.#", "1"), + resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.0.policy_option.0.network_firewall_policy.0.firewall_deployment_model", "CENTRALIZED"), + resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.0.policy_option.0.third_party_firewall_policy.#", "1"), + resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.0.policy_option.0.third_party_firewall_policy.0.firewall_deployment_model", "DISTRIBUTED"), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ), }, @@ -270,6 +300,72 @@ func testAccPolicy_tags(t *testing.T) { }) } +func testAccPolicy_alb(t *testing.T) { + ctx := acctest.Context(t) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_fms_policy.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckRegion(t, endpoints.UsEast1RegionID) + acctest.PreCheckOrganizationsEnabled(ctx, t) + acctest.PreCheckOrganizationManagementAccount(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, fms.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckPolicyDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccPolicyConfig_alb(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckPolicyExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "resource_type", "ResourceTypeList"), + resource.TestCheckResourceAttr(resourceName, "resource_type_list.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "resource_type_list.*", "AWS::ElasticLoadBalancingV2::LoadBalancer"), + resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.#", "1"), + resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.0.type", "WAFV2"), + acctest.CheckResourceAttrJMES(resourceName, "security_service_policy_data.0.managed_service_data", "type", "WAFV2"), + acctest.CheckResourceAttrJMES(resourceName, "security_service_policy_data.0.managed_service_data", "defaultAction.type", "ALLOW"), + ), + }, + }, + }) +} + +func testAccPolicy_securityGroup(t *testing.T) { + ctx := acctest.Context(t) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_fms_policy.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckRegion(t, endpoints.UsEast1RegionID) + acctest.PreCheckOrganizationsEnabled(ctx, t) + acctest.PreCheckOrganizationManagementAccount(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, fms.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckPolicyDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccPolicyConfig_securityGroup(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckPolicyExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "resource_type", "AWS::EC2::SecurityGroup"), + resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.#", "1"), + resource.TestCheckResourceAttr(resourceName, "security_service_policy_data.0.type", "SECURITY_GROUPS_CONTENT_AUDIT"), + acctest.CheckResourceAttrJMES(resourceName, "security_service_policy_data.0.managed_service_data", "type", "SECURITY_GROUPS_CONTENT_AUDIT"), + acctest.CheckResourceAttrJMES(resourceName, "security_service_policy_data.0.managed_service_data", "securityGroupAction.type", "ALLOW"), + ), + }, + }, + }) +} + func testAccCheckPolicyDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).FMSConn(ctx) @@ -315,16 +411,8 @@ func testAccCheckPolicyExists(ctx context.Context, n string) resource.TestCheckF } } -const testAccPolicyConfig_baseOrgMgmtAccount = ` -data "aws_caller_identity" "current" {} - -resource "aws_fms_admin_account" "test" { - account_id = data.aws_caller_identity.current.account_id -} -` - func testAccPolicyConfig_basic(policyName, ruleGroupName string) string { - return acctest.ConfigCompose(testAccPolicyConfig_baseOrgMgmtAccount, fmt.Sprintf(` + return acctest.ConfigCompose(testAccAdminAccountConfig_basic, fmt.Sprintf(` resource "aws_fms_policy" "test" { exclude_resource_tags = false name = %[1]q @@ -352,7 +440,7 @@ resource "aws_wafregional_rule_group" "test" { } func testAccPolicyConfig_policyOption(policyName, ruleGroupName string) string { - return acctest.ConfigCompose(testAccPolicyConfig_baseOrgMgmtAccount, fmt.Sprintf(` + return acctest.ConfigCompose(testAccAdminAccountConfig_basic, fmt.Sprintf(` resource "aws_fms_policy" "test" { exclude_resource_tags = false name = %[1]q @@ -389,7 +477,9 @@ resource "aws_wafregional_rule_group" "test" { } func testAccPolicyConfig_cloudFrontDistribution(rName string) string { - return acctest.ConfigCompose(testAccPolicyConfig_baseOrgMgmtAccount, fmt.Sprintf(` + return acctest.ConfigCompose(testAccAdminAccountConfig_basic, fmt.Sprintf(` +data "aws_partition" "current" {} + resource "aws_fms_policy" "test" { exclude_resource_tags = false name = %[1]q @@ -449,14 +539,13 @@ resource "aws_iam_role" "test" { resource "aws_s3_bucket" "test" { bucket = %[1]q - acl = "private" } resource "aws_kinesis_firehose_delivery_stream" "test" { - name = %[1]q - destination = "s3" + name = "aws-waf-logs-%[1]s" + destination = "extended_s3" - s3_configuration { + extended_s3_configuration { role_arn = aws_iam_role.test.arn bucket_arn = aws_s3_bucket.test.arn } @@ -465,7 +554,7 @@ resource "aws_kinesis_firehose_delivery_stream" "test" { } func testAccPolicyConfig_updated(policyName, ruleGroupName string) string { - return acctest.ConfigCompose(testAccPolicyConfig_baseOrgMgmtAccount, fmt.Sprintf(` + return acctest.ConfigCompose(testAccAdminAccountConfig_basic, fmt.Sprintf(` resource "aws_fms_policy" "test" { exclude_resource_tags = false name = %[1]q @@ -496,7 +585,7 @@ resource "aws_wafregional_rule_group" "test" { } func testAccPolicyConfig_include(rName string) string { - return acctest.ConfigCompose(testAccPolicyConfig_baseOrgMgmtAccount, fmt.Sprintf(` + return acctest.ConfigCompose(testAccAdminAccountConfig_basic, fmt.Sprintf(` resource "aws_fms_policy" "test" { exclude_resource_tags = false name = %[1]q @@ -523,7 +612,7 @@ resource "aws_wafregional_rule_group" "test" { } func testAccPolicyConfig_resourceTags1(rName, tagKey1, tagValue1 string) string { - return acctest.ConfigCompose(testAccPolicyConfig_baseOrgMgmtAccount, fmt.Sprintf(` + return acctest.ConfigCompose(testAccAdminAccountConfig_basic, fmt.Sprintf(` resource "aws_fms_policy" "test" { exclude_resource_tags = false name = %[1]q @@ -550,7 +639,7 @@ resource "aws_wafregional_rule_group" "test" { } func testAccPolicyConfig_resourceTags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { - return acctest.ConfigCompose(testAccPolicyConfig_baseOrgMgmtAccount, fmt.Sprintf(` + return acctest.ConfigCompose(testAccAdminAccountConfig_basic, fmt.Sprintf(` resource "aws_fms_policy" "test" { exclude_resource_tags = false name = %[1]q @@ -578,7 +667,7 @@ resource "aws_wafregional_rule_group" "test" { } func testAccPolicyConfig_tags1(rName, tagKey1, tagValue1 string) string { - return acctest.ConfigCompose(testAccPolicyConfig_baseOrgMgmtAccount, fmt.Sprintf(` + return acctest.ConfigCompose(testAccAdminAccountConfig_basic, fmt.Sprintf(` resource "aws_fms_policy" "test" { exclude_resource_tags = false name = %[1]q @@ -605,7 +694,7 @@ resource "aws_wafregional_rule_group" "test" { } func testAccPolicyConfig_tags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { - return acctest.ConfigCompose(testAccPolicyConfig_baseOrgMgmtAccount, fmt.Sprintf(` + return acctest.ConfigCompose(testAccAdminAccountConfig_basic, fmt.Sprintf(` resource "aws_fms_policy" "test" { exclude_resource_tags = false name = %[1]q @@ -631,3 +720,133 @@ resource "aws_wafregional_rule_group" "test" { } `, rName, tagKey1, tagValue1, tagKey2, tagValue2)) } + +func testAccPolicyConfig_alb(rName string) string { + return acctest.ConfigCompose(testAccAdminAccountConfig_basic, fmt.Sprintf(` +locals { + msd = { + type = "WAFV2" + preProcessRuleGroups = [ + { + overrideAction = { + type = "NONE" + } + managedRuleGroupIdentifier = { + vendorName = "AWS" + managedRuleGroupName = "AWSManagedRulesAmazonIpReputationList" + version = null + } + ruleGroupArn = null + ruleGroupType = "ManagedRuleGroup" + excludeRules = null + sampledRequestsEnabled = true + }, + { + overrideAction = { + type = "NONE" + }, + managedRuleGroupIdentifier = { + vendorName = "AWS" + managedRuleGroupName = "AWSManagedRulesKnownBadInputsRuleSet" + version = "Version_1.1" + }, + ruleGroupArn = null + ruleGroupType = "ManagedRuleGroup" + excludeRules = null + sampledRequestsEnabled = true + } + ] + postProcessRuleGroups = [] + loggingConfiguration = null + sampledRequestsEnabledForDefaultActions = true + defaultAction = { + type = "ALLOW" + } + overrideCustomerWebACLAssociation = true + } +} + +resource "aws_fms_policy" "test" { + name = %[1]q + resource_tags = { + "disable" = "" + } + exclude_resource_tags = true + remediation_enabled = true + resource_type_list = ["AWS::ElasticLoadBalancingV2::LoadBalancer"] + delete_all_policy_resources = false + + security_service_policy_data { + type = "WAFV2" + managed_service_data = jsonencode(local.msd) + } + + depends_on = [aws_fms_admin_account.test] +} +`, rName)) +} + +func testAccPolicyConfig_securityGroup(rName string) string { + return acctest.ConfigCompose(testAccAdminAccountConfig_basic, fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" + + tags = { + Name = %[1]q + } +} + +resource "aws_security_group" "test" { + name = %[1]q + vpc_id = aws_vpc.test.id + + tags = { + Name = %[1]q + } + + ingress { + protocol = "6" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + description = "" + } + + egress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + description = "" + } +} + +resource "aws_fms_policy" "test" { + name = %[1]q + delete_all_policy_resources = false + exclude_resource_tags = false + remediation_enabled = false + resource_type = "AWS::EC2::SecurityGroup" + + security_service_policy_data { + type = "SECURITY_GROUPS_CONTENT_AUDIT" + + managed_service_data = jsonencode({ + type = "SECURITY_GROUPS_CONTENT_AUDIT", + + securityGroupAction = { + type = "ALLOW" + }, + + securityGroups = [ + { + id = aws_security_group.test.id + } + ], + }) + } + + depends_on = [aws_fms_admin_account.test] +} +`, rName)) +} diff --git a/internal/service/fms/service_package_gen.go b/internal/service/fms/service_package_gen.go index 089b55274f9..4445e5de36f 100644 --- a/internal/service/fms/service_package_gen.go +++ b/internal/service/fms/service_package_gen.go @@ -30,11 +30,11 @@ func (p *servicePackage) SDKDataSources(ctx context.Context) []*types.ServicePac func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePackageSDKResource { return []*types.ServicePackageSDKResource{ { - Factory: ResourceAdminAccount, + Factory: resourceAdminAccount, TypeName: "aws_fms_admin_account", }, { - Factory: ResourcePolicy, + Factory: resourcePolicy, TypeName: "aws_fms_policy", Name: "Policy", Tags: &types.ServicePackageResourceTags{ diff --git a/internal/service/fms/ujson/LICENSE b/internal/service/fms/ujson/LICENSE new file mode 100644 index 00000000000..f57b29a5092 --- /dev/null +++ b/internal/service/fms/ujson/LICENSE @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2020 Oliver N + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/internal/service/fms/ujson/quote.go b/internal/service/fms/ujson/quote.go new file mode 100644 index 00000000000..8a25bbb324d --- /dev/null +++ b/internal/service/fms/ujson/quote.go @@ -0,0 +1,90 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 +package ujson + +import ( + "bytes" + "strconv" + "unicode/utf8" + "unsafe" +) + +// ErrSyntax indicates that the value has invalid syntax. +var ErrSyntax = strconv.ErrSyntax + +// AppendQuote appends a double-quoted string valid for json key and value, to +// dst and returns the extended buffer. +func AppendQuote(dst []byte, s []byte) []byte { + return strconv.AppendQuote(dst, unsafeBytesToString(s)) +} + +// AppendQuoteToASCII appends a double-quoted string valid for json key and +// value, to dst and returns the extended buffer. +func AppendQuoteToASCII(dst []byte, s []byte) []byte { + return strconv.AppendQuoteToASCII(dst, unsafeBytesToString(s)) +} + +// AppendQuoteToGraphic appends a double-quoted string valid for json key and +// value, to dst and returns the extended buffer. +func AppendQuoteToGraphic(dst []byte, s []byte) []byte { + return strconv.AppendQuoteToGraphic(dst, unsafeBytesToString(s)) +} + +// AppendQuoteString returns a double-quoted string valid for json key or value. +func AppendQuoteString(dst []byte, s string) []byte { + return strconv.AppendQuote(dst, s) +} + +// Unquote decodes a double-quoted string key or value to retrieve the +// original string value. It will avoid allocation whenever possible. +// +// The code is inspired by strconv.Unquote, but only accepts valid json string. +func Unquote(s []byte) ([]byte, error) { + n := len(s) + if n < 2 { + return nil, ErrSyntax + } + if s[0] != '"' || s[n-1] != '"' { + return nil, ErrSyntax + } + s = s[1 : n-1] + if bytes.IndexByte(s, '\n') != -1 { + return nil, ErrSyntax + } + + // avoid allocation if the string is trivial + if bytes.IndexByte(s, '\\') == -1 { + if utf8.Valid(s) { + return s, nil + } + } + + // the following code is taken from strconv.Unquote (with modification) + var runeTmp [utf8.UTFMax]byte + buf := make([]byte, 0, 3*len(s)/2) // Try to avoid more allocations. + for len(s) > 0 { + // Convert []byte to string for satisfying UnquoteChar. We won't keep + // the retured string, so it's safe to use unsafe here. + c, multibyte, tail, err := strconv.UnquoteChar(unsafeBytesToString(s), '"') + if err != nil { + return nil, err + } + + // UnquoteChar returns tail as the remaining unprocess string. Because + // we are processing []byte, we use len(tail) to get the remaining bytes + // instead. + s = s[len(s)-len(tail):] + if c < utf8.RuneSelf || !multibyte { + buf = append(buf, byte(c)) + } else { + n = utf8.EncodeRune(runeTmp[:], c) + buf = append(buf, runeTmp[:n]...) + } + } + return buf, nil +} + +//go:nosplit +func unsafeBytesToString(b []byte) string { + return *(*string)(unsafe.Pointer(&b)) +} diff --git a/internal/service/fms/ujson/quote_test.go b/internal/service/fms/ujson/quote_test.go new file mode 100644 index 00000000000..914a6cf50d1 --- /dev/null +++ b/internal/service/fms/ujson/quote_test.go @@ -0,0 +1,162 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package ujson_test + +import ( + "errors" + "testing" + + . "github.com/hashicorp/terraform-provider-aws/internal/service/fms/ujson" +) + +type quoteTest struct { + in string + out string + ascii string + graphic string +} + +var quotetests = []quoteTest{ + {"\a\b\f\r\n\t\v", `"\a\b\f\r\n\t\v"`, `"\a\b\f\r\n\t\v"`, `"\a\b\f\r\n\t\v"`}, + {"\\", `"\\"`, `"\\"`, `"\\"`}, + {"abc\xffdef", `"abc\xffdef"`, `"abc\xffdef"`, `"abc\xffdef"`}, + {"\u263a", `"☺"`, `"\u263a"`, `"☺"`}, + {"\U0010ffff", `"\U0010ffff"`, `"\U0010ffff"`, `"\U0010ffff"`}, + {"\x04", `"\x04"`, `"\x04"`, `"\x04"`}, + // Some non-printable but graphic runes. Final column is double-quoted. + {"!\u00a0!\u2000!\u3000!", `"!\u00a0!\u2000!\u3000!"`, `"!\u00a0!\u2000!\u3000!"`, "\"!\u00a0!\u2000!\u3000!\""}, +} + +func TestQuote(t *testing.T) { + t.Parallel() + + for _, tt := range quotetests { + if out := AppendQuote([]byte("abc"), []byte(tt.in)); string(out) != "abc"+tt.out { + t.Errorf("AppendQuote(%q, %s) = %s, want %s", "abc", tt.in, out, "abc"+tt.out) + } + } +} + +func TestQuoteToASCII(t *testing.T) { + t.Parallel() + + for _, tt := range quotetests { + if out := AppendQuoteToASCII([]byte("abc"), []byte(tt.in)); string(out) != "abc"+tt.ascii { + t.Errorf("AppendQuoteToASCII(%q, %s) = %s, want %s", "abc", tt.in, out, "abc"+tt.ascii) + } + } +} + +func TestQuoteToGraphic(t *testing.T) { + t.Parallel() + + for _, tt := range quotetests { + if out := AppendQuoteToGraphic([]byte("abc"), []byte(tt.in)); string(out) != "abc"+tt.graphic { + t.Errorf("AppendQuoteToGraphic(%q, %s) = %s, want %s", "abc", tt.in, out, "abc"+tt.graphic) + } + } +} + +type unQuoteTest struct { + in string + out string +} + +var unquotetests = []unQuoteTest{ + {`""`, ""}, + {`"a"`, "a"}, + {`"abc"`, "abc"}, + {`"☺"`, "☺"}, + {`"hello world"`, "hello world"}, + {`"\xFF"`, "\xFF"}, + {`"\377"`, "\377"}, + {`"\u1234"`, "\u1234"}, + {`"\U00010111"`, "\U00010111"}, + {`"\U0001011111"`, "\U0001011111"}, + {`"\a\b\f\n\r\t\v\\\""`, "\a\b\f\n\r\t\v\\\""}, + {`"'"`, "'"}, +} + +var misquoted = []string{ + ``, + `"`, + `"a`, + `"'`, + `b"`, + `"\"`, + `"\9"`, + `"\19"`, + `"\129"`, + `'\'`, + `'\9'`, + `'\19'`, + `'\129'`, + `'ab'`, + `"\x1!"`, + `"\U12345678"`, + `"\z"`, + "`", + "`xxx", + "`\"", + `"\'"`, + `'\"'`, + "\"\n\"", + "\"\\n\n\"", + "'\n'", +} + +func TestUnquote(t *testing.T) { + t.Parallel() + + for _, tt := range unquotetests { + if out, err := Unquote([]byte(tt.in)); err != nil || string(out) != tt.out { + t.Errorf("Unquote(%#q) = %q, %v want %q, nil", tt.in, out, err, tt.out) + } + } + + // run the quote tests too, backward + for _, tt := range quotetests { + if in, err := Unquote([]byte(tt.out)); string(in) != tt.in { + t.Errorf("Unquote(%#q) = %q, %v, want %q, nil", tt.out, in, err, tt.in) + } + } + + for _, s := range misquoted { + if out, err := Unquote([]byte(s)); out != nil || !errors.Is(err, ErrSyntax) { + t.Errorf("Unquote(%#q) = %q, %v want %q, %v", s, out, err, "", ErrSyntax) + } + } +} + +// Issue 23685: invalid UTF-8 should not go through the fast path. +func TestUnquoteInvalidUTF8(t *testing.T) { + t.Parallel() + + tests := []struct { + in string + + // one of: + want string + wantErr string + }{ + {in: `"foo"`, want: "foo"}, + {in: `"foo`, wantErr: "invalid syntax"}, + {in: `"` + "\xc0" + `"`, want: "\xef\xbf\xbd"}, + {in: `"a` + "\xc0" + `"`, want: "a\xef\xbf\xbd"}, + {in: `"\t` + "\xc0" + `"`, want: "\t\xef\xbf\xbd"}, + } + for i, tt := range tests { + got, err := Unquote([]byte(tt.in)) + var gotErr string + if err != nil { + gotErr = err.Error() + } + if gotErr != tt.wantErr { + t.Errorf("%d. Unquote(%q) = err %v; want %q", i, tt.in, err, tt.wantErr) + } + if tt.wantErr == "" && err == nil && string(got) != tt.want { + t.Errorf("%d. Unquote(%q) = %02x; want %02x", i, tt.in, got, []byte(tt.want)) + } + } +} diff --git "a/internal/service/fms/ujson/\302\265json.go" "b/internal/service/fms/ujson/\302\265json.go" new file mode 100644 index 00000000000..45ca81e1945 --- /dev/null +++ "b/internal/service/fms/ujson/\302\265json.go" @@ -0,0 +1,227 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +// Package ujson implements a fast and minimal JSON parser and transformer that +// works on unstructured json. Example use cases: +// +// 1. Walk through unstructured json: +// - Print all keys and values +// - Extract some values +// 2. Transform unstructured json: +// - Remove all spaces +// - Reformat +// - Remove blacklist fields +// - Wrap int64 in string for processing by JavaScript +// +// without fully unmarshalling it into a map[string]interface{} +// +// CAUTION: Behaviour is undefined on invalid json. Use on trusted input only. +// +// The single most important function is "Walk()", which parses the given json +// and call callback function for each key/value pair processed. +// +// { +// "id": 12345, +// "name": "foo", +// "numbers": ["one", "two"], +// "tags": {"color": "red", "priority": "high"}, +// "active": true +// } +// +// Calling "Walk()" with the above input will produce: +// +// | level | key | value | +// |-------|------------|---------| +// | 0 | | { | +// | 1 | "id" | 12345 | +// | 1 | "name" | "foo" | +// | 1 | "numbers" | [ | +// | 2 | | "one" | +// | 2 | | "two" | +// | 1 | | ] | +// | 1 | "tags" | { | +// | 2 | "color" | "red" | +// | 2 | "priority" | "high" | +// | 1 | | } | +// | 1 | "active" | true | +// | 0 | | } | +// +// "level" indicates the indentation of the key/value pair as if the json is +// formatted properly. Keys and values are provided as raw literal. Strings are +// always double-quoted. To get the original string, use "Unquote". +// +// "value" will never be empty (for valid json). You can test the first byte +// ("value[0]") to get its type: +// +// - 'n' : Null ("null") +// - 'f', 't': Boolean ("false", "true") +// - '0'-'9' : Number +// - '"' : String, see "Unquote" +// - '[', ']': Array +// - '{', '}': Object +// +// When processing arrays and objects, first the open bracket ("[", "{") will be +// provided as "value", followed by its children, and finally the close bracket +// ("]", "}"). When encountering open brackets, you can make the callback +// function return "false" to skip the array/object entirely. +package ujson + +import "fmt" + +// Walk parses the given json and call "callback" for each key/value pair. See +// examples for sample callback params. +// +// The function "callback": +// +// - may convert key and value to string for processing +// - may return false to skip processing the current object or array +// - must not modify any slice it receives. +func Walk(input []byte, callback func(level int, key, value []byte) bool) error { + var key []byte + i, si, ei, st, sst := 0, 0, 0, 0, 1024 + + // trim the last newline + if len(input) > 0 && input[len(input)-1] == '\n' { + input = input[:len(input)-1] + } + +value: + si = i + switch input[i] { + case 'n', 't': // null, true + i += 4 + ei = i + if st <= sst { + callback(st, key, input[si:i]) + } + key = nil + goto closing + case 'f': // false + i += 5 + ei = i + if st <= sst { + callback(st, key, input[si:i]) + } + key = nil + goto closing + case '{', '[': + if st <= sst && !callback(st, key, input[i:i+1]) { + sst = st + } + key = nil + st++ + i++ + if input[i] == '}' || input[i] == ']' { + goto closing + } + goto value + case '"': // scan string + for { + i++ + switch input[i] { + case '\\': // \. - skip 2 + i++ + case '"': // end of string + i++ + ei = i // space, ignore + for input[i] == ' ' || + input[i] == '\t' || + input[i] == '\n' || + input[i] == '\r' { + i++ + } + if input[i] != ':' { + if st <= sst { + callback(st, key, input[si:ei]) + } + key = nil + } + goto closing + } + } + case ' ', '\t', '\n', '\r': // space, ignore + i++ + goto value + default: // scan number + for i < len(input) { + switch input[i] { + case ',', '}', ']', ' ', '\t', '\n', '\r': + ei = i + for input[i] == ' ' || + input[i] == '\t' || + input[i] == '\n' || + input[i] == '\r' { + i++ + } + if st <= sst { + callback(st, key, input[si:ei]) + } + key = nil + goto closing + } + i++ + } + } + +closing: + if i >= len(input) { + return nil + } + switch input[i] { + case ':': + key = input[si:ei] + i++ + goto value + case ',': + i++ + goto value + case ']', '}': + st-- + if st == sst { + sst = 1024 + } else if st < sst { + callback(st, nil, input[i:i+1]) + } + if st <= 0 { + return nil + } + i++ + goto closing + case ' ', '\t', '\n', '\r': + i++ // space, ignore + goto closing + default: + return parseError(i, input[i], `expect ']', '}' or ','`) + } +} + +func parseError(i int, c byte, msg string) error { + return fmt.Errorf("µjson: error at %v '%c' 0x%2x: %v", i, c, c, msg) +} + +// ShouldAddComma decides if a comma should be appended while constructing +// output json. See Reconstruct for an example of rebuilding the json. +func ShouldAddComma(value []byte, lastChar byte) bool { + // for valid json, the value will never be empty, so we can safely test only + // the first byte + return value[0] != '}' && value[0] != ']' && + lastChar != ',' && lastChar != '{' && lastChar != '[' +} + +// Reconstruct walks through the input json and rebuild it. It's put here as an +// example of using Walk. +func Reconstruct(input []byte) ([]byte, error) { + b := make([]byte, 0, len(input)) + err := Walk(input, func(st int, key, value []byte) bool { + if len(b) != 0 && ShouldAddComma(value, b[len(b)-1]) { + b = append(b, ',') + } + if len(key) > 0 { + b = append(b, key...) + b = append(b, ':') + } + b = append(b, value...) + return true + }) + return b, err +} diff --git "a/internal/service/fms/ujson/\302\265json_test.go" "b/internal/service/fms/ujson/\302\265json_test.go" new file mode 100644 index 00000000000..8f84077c0d5 --- /dev/null +++ "b/internal/service/fms/ujson/\302\265json_test.go" @@ -0,0 +1,360 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package ujson + +import ( + "fmt" + "strings" + "testing" +) + +func TestWalk(t *testing.T) { + t.Parallel() + + tests := []struct { + input string + expected string + }{ + { + `null`, + ` +0 null`, + }, + { + "null\n", // end with newline + ` +0 null`, + }, + { + `{}`, + ` +0 { +0 }`, + }, + { + `{"foo":""}`, + ` +0 { +1 "foo" "" +0 }`, + }, + { + `{"foo": ""}`, // Space + ` +0 { +1 "foo" "" +0 }`, + }, + { + `{"foo":"bar"}`, + ` +0 { +1 "foo" "bar" +0 }`, + }, + { + `{"foo":"bar","baz":""}`, + ` +0 { +1 "foo" "bar" +1 "baz" "" +0 }`, + }, + { + `{ "foo" : "bar" , "baz" : 2 }`, // Space + ` +0 { +1 "foo" "bar" +1 "baz" 2 +0 }`, + }, + { + `{"foo":null}`, + ` +0 { +1 "foo" null +0 }`, + }, + { + `{"foo":123}`, + ` +0 { +1 "foo" 123 +0 }`, + }, + { + `{"foo":-123}`, + ` +0 { +1 "foo" -123 +0 }`, + }, + { + `{"foo":42.1}`, + ` +0 { +1 "foo" 42.1 +0 }`, + }, + { + `{"foo":+0}`, + ` +0 { +1 "foo" +0 +0 }`, + }, + { + `{"foo":"b\"ar"}`, + ` +0 { +1 "foo" "b\"ar" +0 }`, + }, + { + `{"😀":"🎶\""}`, + ` +0 { +1 "😀" "🎶\"" +0 }`, + }, + { + `{"foo":{}}`, + ` +0 { +1 "foo" { +1 } +0 }`, + }, + { + `{"foo":{"bar":false,"baz":true,"quix":null}}`, + ` +0 { +1 "foo" { +2 "bar" false +2 "baz" true +2 "quix" null +1 } +0 }`, + }, + { + `{"1":{"1.1":{"1.1.1":"foo","1.1.2":"bar"},"1.2":{"1.2.1":"baz"}}}`, + ` +0 { +1 "1" { +2 "1.1" { +3 "1.1.1" "foo" +3 "1.1.2" "bar" +2 } +2 "1.2" { +3 "1.2.1" "baz" +2 } +1 } +0 }`, + }, + { + `[]`, + ` +0 [ +0 ]`, + }, + { + `[null]`, + ` +0 [ +1 null +0 ]`, + }, + { + `[0]`, + ` +0 [ +1 0 +0 ]`, + }, + { + `["foo"]`, + ` +0 [ +1 "foo" +0 ]`, + }, + { + `["",""]`, + ` +0 [ +1 "" +1 "" +0 ]`, + }, + { + `["foo","bar"]`, + ` +0 [ +1 "foo" +1 "bar" +0 ]`, + }, + { + `[[]]`, + ` +0 [ +1 [ +1 ] +0 ]`, + }, + { + `[{},[]]`, + ` +0 [ +1 { +1 } +1 [ +1 ] +0 ]`, + }, + { + `{"foo":[]}`, + ` +0 { +1 "foo" [ +1 ] +0 }`, + }, + { + `{"foo":[{"k":"v"}]}`, + ` +0 { +1 "foo" [ +2 { +3 "k" "v" +2 } +1 ] +0 }`, + }, + { + `{"foo":[{"k1":"v1","k2":"v2"}]}`, + ` +0 { +1 "foo" [ +2 { +3 "k1" "v1" +3 "k2" "v2" +2 } +1 ] +0 }`, + }, + { + `{"foo":[{"k1.1":"v1.1","k1.2":"v1.2"},{"k2.1":"v2.1"}],"bar":{}}`, + ` +0 { +1 "foo" [ +2 { +3 "k1.1" "v1.1" +3 "k1.2" "v1.2" +2 } +2 { +3 "k2.1" "v2.1" +2 } +1 ] +1 "bar" { +1 } +0 }`, + }, + { + `{"1":[{"2":{"k1":"v1","k2":"v2"}}]}`, + ` +0 { +1 "1" [ +2 { +3 "2" { +4 "k1" "v1" +4 "k2" "v2" +3 } +2 } +1 ] +0 }`, + }, + { + `{"1":[{"2":[{"k1":"v1","k2":"v2"},{"k3":"v3"}]}]}`, + ` +0 { +1 "1" [ +2 { +3 "2" [ +4 { +5 "k1" "v1" +5 "k2" "v2" +4 } +4 { +5 "k3" "v3" +4 } +3 ] +2 } +1 ] +0 }`, + }, + { + `{ "1" : [ { "2": [ { "k1" : "v1" , "k2" : "v2" } ,{"k3":"v3" } ] } ] }`, + ` +0 { +1 "1" [ +2 { +3 "2" [ +4 { +5 "k1" "v1" +5 "k2" "v2" +4 } +4 { +5 "k3" "v3" +4 } +3 ] +2 } +1 ] +0 }`, + }, + } + + for _, tt := range tests { + tt := tt + t.Run("Walk/"+tt.input, func(t *testing.T) { + t.Parallel() + + var b strings.Builder + err := Walk([]byte(tt.input), + func(st int, key, value []byte) bool { + fmt.Fprintf(&b, "\n%v %s %s", st, key, value) + return true + }) + if err != nil { + t.Error(err) + } else if b.String() != tt.expected { + t.Errorf("\nExpect: `%v`\nOutput: `%v`\n", tt.expected, b.String()) + } + }) + } + + for _, tt := range tests { + tt := tt + t.Run("Reconstruct/"+tt.input, func(t *testing.T) { + t.Parallel() + + // Handle the special testcase ending with newline. This test + // reconstructs output json and compare with the input. Because + // reconstruct will not append the last newline, so we must trim it + // before comparing. + expected := tt.input + if expected[len(expected)-1] == '\n' { + expected = expected[:len(expected)-1] + } + expected = strings.Replace(expected, " ", "", -1) + + data, err := Reconstruct([]byte(tt.input)) + if err != nil { + t.Error(err) + } else if s := string(data); s != expected { + t.Errorf("\nExpect: %v\nOutput: %v\n", expected, s) + } + }) + } +} diff --git a/website/docs/r/fms_admin_account.html.markdown b/website/docs/r/fms_admin_account.html.markdown index 809b740c9db..4d9aec89c90 100644 --- a/website/docs/r/fms_admin_account.html.markdown +++ b/website/docs/r/fms_admin_account.html.markdown @@ -28,6 +28,13 @@ This resource exports the following attributes in addition to the arguments abov * `id` - The AWS account ID of the AWS Firewall Manager administrator account. +## Timeouts + +[Configuration options](https://developer.hashicorp.com/terraform/language/resources/syntax#operation-timeouts): + +- `create` - (Default `30m`) +- `delete` - (Default `10m`) + ## Import In Terraform v1.5.0 and later, use an [`import` block](https://developer.hashicorp.com/terraform/language/import) to import Firewall Manager administrator account association using the account ID. For example: