diff --git a/.changelog/38604.txt b/.changelog/38604.txt new file mode 100644 index 00000000000..d01b0a85f80 --- /dev/null +++ b/.changelog/38604.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_opensearchserverless_security_policy: Normalize `policy` content to prevent persistent differences +``` diff --git a/internal/service/opensearchserverless/security_policy.go b/internal/service/opensearchserverless/security_policy.go index 8b144cc0b11..8eba293d7f0 100644 --- a/internal/service/opensearchserverless/security_policy.go +++ b/internal/service/opensearchserverless/security_policy.go @@ -12,8 +12,8 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/opensearchserverless" awstypes "github.com/aws/aws-sdk-go-v2/service/opensearchserverless/types" + "github.com/hashicorp/terraform-plugin-framework-jsontypes/jsontypes" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" - "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" @@ -36,12 +36,12 @@ func newResourceSecurityPolicy(_ context.Context) (resource.ResourceWithConfigur } type resourceSecurityPolicyData struct { - Description types.String `tfsdk:"description"` - ID types.String `tfsdk:"id"` - Name types.String `tfsdk:"name"` - Policy types.String `tfsdk:"policy"` - PolicyVersion types.String `tfsdk:"policy_version"` - Type types.String `tfsdk:"type"` + Description types.String `tfsdk:"description"` + ID types.String `tfsdk:"id"` + Name types.String `tfsdk:"name"` + Policy jsontypes.Normalized `tfsdk:"policy"` + PolicyVersion types.String `tfsdk:"policy_version"` + Type types.String `tfsdk:"type"` } const ( @@ -76,13 +76,17 @@ func (r *resourceSecurityPolicy) Schema(ctx context.Context, req resource.Schema }, }, names.AttrPolicy: schema.StringAttribute{ - Required: true, + CustomType: jsontypes.NormalizedType{}, + Required: true, Validators: []validator.String{ stringvalidator.LengthBetween(1, 20480), }, }, "policy_version": schema.StringAttribute{ Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, names.AttrType: schema.StringAttribute{ Required: true, @@ -108,17 +112,15 @@ func (r *resourceSecurityPolicy) Create(ctx context.Context, req resource.Create conn := r.Meta().OpenSearchServerlessClient(ctx) - in := &opensearchserverless.CreateSecurityPolicyInput{ - ClientToken: aws.String(id.UniqueId()), - Name: aws.String(plan.Name.ValueString()), - Policy: aws.String(plan.Policy.ValueString()), - Type: awstypes.SecurityPolicyType(plan.Type.ValueString()), - } + in := &opensearchserverless.CreateSecurityPolicyInput{} - if !plan.Description.IsNull() { - in.Description = aws.String(plan.Description.ValueString()) + resp.Diagnostics.Append(flex.Expand(ctx, plan, in)...) + if resp.Diagnostics.HasError() { + return } + in.ClientToken = aws.String(id.UniqueId()) + out, err := conn.CreateSecurityPolicy(ctx, in) if err != nil { resp.Diagnostics.AddError( @@ -129,7 +131,12 @@ func (r *resourceSecurityPolicy) Create(ctx context.Context, req resource.Create } state := plan - resp.Diagnostics.Append(state.refreshFromOutput(ctx, out.SecurityPolicyDetail)...) + resp.Diagnostics.Append(flex.Flatten(ctx, out.SecurityPolicyDetail, &state)...) + if resp.Diagnostics.HasError() { + return + } + + state.ID = flex.StringToFramework(ctx, out.SecurityPolicyDetail.Name) resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) } @@ -157,7 +164,11 @@ func (r *resourceSecurityPolicy) Read(ctx context.Context, req resource.ReadRequ return } - resp.Diagnostics.Append(state.refreshFromOutput(ctx, out)...) + resp.Diagnostics.Append(flex.Flatten(ctx, out, &state)...) + if resp.Diagnostics.HasError() { + return + } + resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) } @@ -173,20 +184,14 @@ func (r *resourceSecurityPolicy) Update(ctx context.Context, req resource.Update if !plan.Description.Equal(state.Description) || !plan.Policy.Equal(state.Policy) { - input := &opensearchserverless.UpdateSecurityPolicyInput{ - ClientToken: aws.String(id.UniqueId()), - Name: flex.StringFromFramework(ctx, plan.Name), - PolicyVersion: flex.StringFromFramework(ctx, state.PolicyVersion), - Type: awstypes.SecurityPolicyType(plan.Type.ValueString()), - } + input := &opensearchserverless.UpdateSecurityPolicyInput{} - if !plan.Policy.Equal(state.Policy) { - input.Policy = aws.String(plan.Policy.ValueString()) + resp.Diagnostics.Append(flex.Expand(ctx, plan, input)...) + if resp.Diagnostics.HasError() { + return } - if !plan.Description.Equal(state.Description) { - input.Description = aws.String(plan.Description.ValueString()) - } + input.ClientToken = aws.String(id.UniqueId()) out, err := conn.UpdateSecurityPolicy(ctx, input) @@ -194,7 +199,10 @@ func (r *resourceSecurityPolicy) Update(ctx context.Context, req resource.Update resp.Diagnostics.AddError(fmt.Sprintf("updating Security Policy (%s)", plan.Name.ValueString()), err.Error()) return } - resp.Diagnostics.Append(state.refreshFromOutput(ctx, out.SecurityPolicyDetail)...) + resp.Diagnostics.Append(flex.Flatten(ctx, out.SecurityPolicyDetail, &state)...) + if resp.Diagnostics.HasError() { + return + } } resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) @@ -246,30 +254,3 @@ func (r *resourceSecurityPolicy) ImportState(ctx context.Context, req resource.I return } } - -// refreshFromOutput writes state data from an AWS response object -func (rd *resourceSecurityPolicyData) refreshFromOutput(ctx context.Context, out *awstypes.SecurityPolicyDetail) diag.Diagnostics { - var diags diag.Diagnostics - - if out == nil { - return diags - } - - rd.ID = flex.StringToFramework(ctx, out.Name) - rd.Description = flex.StringToFramework(ctx, out.Description) - rd.Name = flex.StringToFramework(ctx, out.Name) - rd.Type = flex.StringValueToFramework(ctx, out.Type) - rd.PolicyVersion = flex.StringToFramework(ctx, out.PolicyVersion) - - policyBytes, err := out.Policy.MarshalSmithyDocument() - if err != nil { - diags.AddError(fmt.Sprintf("refreshing state for Security Policy (%s)", rd.Name), err.Error()) - return diags - } - - p := string(policyBytes) - - rd.Policy = flex.StringToFramework(ctx, &p) - - return diags -} diff --git a/internal/service/opensearchserverless/security_policy_test.go b/internal/service/opensearchserverless/security_policy_test.go index 8eaf68c35e5..89cba6f818e 100644 --- a/internal/service/opensearchserverless/security_policy_test.go +++ b/internal/service/opensearchserverless/security_policy_test.go @@ -121,6 +121,79 @@ func TestAccOpenSearchServerlessSecurityPolicy_disappears(t *testing.T) { }) } +func TestAccOpenSearchServerlessSecurityPolicy_string(t *testing.T) { + ctx := acctest.Context(t) + var securitypolicy types.SecurityPolicyDetail + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_opensearchserverless_security_policy.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckPartitionHasService(t, names.OpenSearchServerlessEndpointID) + testAccPreCheck(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, names.OpenSearchServerlessServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckSecurityPolicyDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccSecurityPolicyConfig_string(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckSecurityPolicyExists(ctx, resourceName, &securitypolicy), + acctest.CheckResourceAttrEquivalentJSON(resourceName, names.AttrPolicy, testAccSecurityPolicyConfig_String_ExpectedJSON(rName)), + ), + }, + { + // verify no planned changes + Config: testAccSecurityPolicyConfig_string(rName), + PlanOnly: true, + }, + { + ResourceName: resourceName, + ImportStateIdFunc: testAccSecurityPolicyImportStateIdFunc(resourceName), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{names.AttrPolicy}, // JSON is semantically correct but can be set in state in a different order + }, + }, + }) +} + +func TestAccOpenSearchServerlessSecurityPolicy_stringUpdate(t *testing.T) { + ctx := acctest.Context(t) + var securitypolicy types.SecurityPolicyDetail + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_opensearchserverless_security_policy.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckPartitionHasService(t, names.OpenSearchServerlessEndpointID) + testAccPreCheck(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, names.OpenSearchServerlessServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckSecurityPolicyDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccSecurityPolicyConfig_string(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckSecurityPolicyExists(ctx, resourceName, &securitypolicy), + acctest.CheckResourceAttrEquivalentJSON(resourceName, names.AttrPolicy, testAccSecurityPolicyConfig_String_ExpectedJSON(rName)), + ), + }, + { + Config: testAccSecurityPolicyConfig_stringUpdate(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckSecurityPolicyExists(ctx, resourceName, &securitypolicy), + acctest.CheckResourceAttrEquivalentJSON(resourceName, names.AttrPolicy, testAccSecurityPolicyConfig_String_ExpectedJSON(rName)), + ), + }, + }, + }) +} + func testAccCheckSecurityPolicyDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).OpenSearchServerlessClient(ctx) @@ -242,3 +315,34 @@ resource "aws_opensearchserverless_security_policy" "test" { } `, rName, collection, description) } + +func testAccSecurityPolicyConfig_String_ExpectedJSON(rName string) string { + collection := fmt.Sprintf("collection/%s", rName) + return fmt.Sprintf(`{"Rules":[{"Resource":["%s"],"ResourceType":"collection"}],"AWSOwnedKey":true}`, collection) +} + +func testAccSecurityPolicyConfig_string(rName string) string { + collection := fmt.Sprintf("collection/%s", rName) + return fmt.Sprintf(` +resource "aws_opensearchserverless_security_policy" "test" { + name = %[1]q + type = "encryption" + description = %[1]q + policy = "{\"Rules\":[{\"Resource\":[\"%[2]s\"],\"ResourceType\":\"collection\"}],\"AWSOwnedKey\":true}" +} +`, rName, collection) +} + +// testAccSecurityPolicyConfig_stringUpdate uses the same policy with additional whitespace +// to verify normalization prevents persistent differences +func testAccSecurityPolicyConfig_stringUpdate(rName string) string { + collection := fmt.Sprintf("collection/%s", rName) + return fmt.Sprintf(` +resource "aws_opensearchserverless_security_policy" "test" { + name = %[1]q + type = "encryption" + description = "%[1]s-updated" + policy = "{\"Rules\":[{\"Resource\":[\"%[2]s\"],\"ResourceType\":\"collection\"}],\"AWSOwnedKey\": true }" +} +`, rName, collection) +}