Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(opensearchserverless): security_policy policy attribute doesn't normalize json strings #38604

Merged
merged 4 commits into from
Jul 31, 2024
Merged

fix(opensearchserverless): security_policy policy attribute doesn't normalize json strings #38604

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b11625a
fix(opensearchserverless): security_policy policy attribute doesn't n…
corymhall Jul 30, 2024
108205a
r/aws_opensearchserverless_security_policy(test): tweak string policy…
jar-b Jul 31, 2024
a3a5643
chore: changelog
jar-b Jul 31, 2024
5172767
chore: make fix-constants
jar-b Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/38604.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_opensearchserverless_security_policy: Normalize `policy` content to prevent persistent differences
```
95 changes: 38 additions & 57 deletions internal/service/opensearchserverless/security_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 (
Expand Down Expand Up @@ -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(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was useful or needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was needed. Without it the update test failed with missing required field, UpdateSecurityPolicyInput.PolicyVersion.

},
},
names.AttrType: schema.StringAttribute{
Required: true,
Expand All @@ -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(
Expand All @@ -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)...)
}

Expand Down Expand Up @@ -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)...)
}

Expand All @@ -173,28 +184,25 @@ 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)

if err != nil {
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)...)
Expand Down Expand Up @@ -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
}
104 changes: 104 additions & 0 deletions internal/service/opensearchserverless/security_policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{"policy"}, // 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)
Expand Down Expand Up @@ -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)
}
Loading