From 61cdc4e0880354441e8e3ec438df56965a7338e6 Mon Sep 17 00:00:00 2001 From: Vladimir Kuznichenkov Date: Sat, 31 Aug 2024 13:53:19 +0300 Subject: [PATCH] aws-cognito-user-pool: fix empty string_attribute_constraints on user_pool Fixing bug when empty string_attribute_constraints on user_pool attribute creates no-op update that results in a failure when applied Rebase for [PR][1] Closes #20276 Closes #37687 [1]: https://github.com/hashicorp/terraform-provider-aws/pull/20386 --- internal/acctest/consts_gen.go | 2 + internal/service/cognitoidp/user_pool.go | 54 +++++++++++++++- internal/service/cognitoidp/user_pool_test.go | 62 +++++++++++++++++-- 3 files changed, 113 insertions(+), 5 deletions(-) diff --git a/internal/acctest/consts_gen.go b/internal/acctest/consts_gen.go index e83704fb2498..13b1b2cae1b9 100644 --- a/internal/acctest/consts_gen.go +++ b/internal/acctest/consts_gen.go @@ -11,6 +11,8 @@ const ( Ct2 = "2" Ct3 = "3" Ct4 = "4" + Ct5 = "5" + Ct6 = "6" CtBasic = "basic" CtCertificatePEM = "certificate_pem" CtDisappears = "disappears" diff --git a/internal/service/cognitoidp/user_pool.go b/internal/service/cognitoidp/user_pool.go index d5e146dfbf28..cfc032e6df2c 100644 --- a/internal/service/cognitoidp/user_pool.go +++ b/internal/service/cognitoidp/user_pool.go @@ -4,6 +4,7 @@ package cognitoidp import ( + "bytes" "context" "fmt" "log" @@ -20,6 +21,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/create" "github.com/hashicorp/terraform-provider-aws/internal/enum" "github.com/hashicorp/terraform-provider-aws/internal/errs" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" @@ -421,6 +423,7 @@ func resourceUserPool() *schema.Resource { Optional: true, MinItems: 1, MaxItems: 50, + Set: resourceUserPoolSchemaHash, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "attribute_data_type": { @@ -645,6 +648,51 @@ func resourceUserPool() *schema.Resource { } } +func resourceUserPoolSchemaHash(v interface{}) int { + var buf bytes.Buffer + m := v.(map[string]interface{}) + buf.WriteString(fmt.Sprintf("%s-", m["name"].(string))) + buf.WriteString(fmt.Sprintf("%s-", m["attribute_data_type"].(string))) + buf.WriteString(fmt.Sprintf("%t-", m["developer_only_attribute"].(bool))) + buf.WriteString(fmt.Sprintf("%t-", m["mutable"].(bool))) + buf.WriteString(fmt.Sprintf("%t-", m["required"].(bool))) + + if v, ok := m["string_attribute_constraints"]; ok { + data := v.([]interface{}) + + if len(data) > 0 { + buf.WriteString("string_attribute_constraints-") + m, _ := data[0].(map[string]interface{}) + if l, ok := m["min_length"]; ok && l.(string) != "" { + buf.WriteString(fmt.Sprintf("%s-", l.(string))) + } + + if l, ok := m["max_length"]; ok && l.(string) != "" { + buf.WriteString(fmt.Sprintf("%s-", l.(string))) + } + } + } + + if v, ok := m["number_attribute_constraints"]; ok { + data := v.([]interface{}) + + if len(data) > 0 { + buf.WriteString("number_attribute_constraints-") + m, _ := data[0].(map[string]interface{}) + + if l, ok := m["min_value"]; ok && l.(string) != "" { + buf.WriteString(fmt.Sprintf("%s-", l.(string))) + } + + if l, ok := m["max_value"]; ok && l.(string) != "" { + buf.WriteString(fmt.Sprintf("%s-", l.(string))) + } + } + } + + return create.StringHashcode(buf.String()) +} + func resourceUserPoolCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).CognitoIDPClient(ctx) @@ -1692,7 +1740,11 @@ func expandSchemaAttributeTypes(tfList []interface{}) []awstypes.SchemaAttribute sact.MinLength = aws.String(v.(string)) } - apiObject.StringAttributeConstraints = sact + if sact.MinLength == nil && sact.MaxLength == nil { + apiObject.StringAttributeConstraints = nil + } else { + apiObject.StringAttributeConstraints = sact + } } } } diff --git a/internal/service/cognitoidp/user_pool_test.go b/internal/service/cognitoidp/user_pool_test.go index 51665d730f8d..2047a12fe9bf 100644 --- a/internal/service/cognitoidp/user_pool_test.go +++ b/internal/service/cognitoidp/user_pool_test.go @@ -1434,7 +1434,7 @@ func TestAccCognitoIDPUserPool_schemaAttributes(t *testing.T) { Config: testAccUserPoolConfig_schemaAttributes(rName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckUserPoolExists(ctx, resourceName, &pool1), - resource.TestCheckResourceAttr(resourceName, "schema.#", acctest.Ct2), + resource.TestCheckResourceAttr(resourceName, "schema.#", acctest.Ct4), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ "attribute_data_type": "String", "developer_only_attribute": acctest.CtFalse, @@ -1443,9 +1443,27 @@ func TestAccCognitoIDPUserPool_schemaAttributes(t *testing.T) { "number_attribute_constraints.#": acctest.Ct0, "required": acctest.CtTrue, "string_attribute_constraints.#": acctest.Ct1, - "string_attribute_constraints.0.min_length": "5", + "string_attribute_constraints.0.min_length": acctest.Ct5, "string_attribute_constraints.0.max_length": acctest.Ct10, }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ + "attribute_data_type": "String", + "developer_only_attribute": acctest.CtFalse, + "mutable": acctest.CtTrue, + "name": "strattr", + "required": acctest.CtFalse, + "number_attribute_constraints.#": acctest.Ct0, + "string_attribute_constraints.#": acctest.Ct1, + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ + "attribute_data_type": "Number", + "developer_only_attribute": acctest.CtFalse, + "mutable": acctest.CtTrue, + "name": "numattr", + "required": acctest.CtFalse, + "number_attribute_constraints.#": acctest.Ct1, + "string_attribute_constraints.#": acctest.Ct0, + }), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ "attribute_data_type": "Boolean", "developer_only_attribute": acctest.CtTrue, @@ -1471,7 +1489,7 @@ func TestAccCognitoIDPUserPool_schemaAttributes(t *testing.T) { "number_attribute_constraints.#": acctest.Ct0, "required": acctest.CtTrue, "string_attribute_constraints.#": acctest.Ct1, - "string_attribute_constraints.0.min_length": "5", + "string_attribute_constraints.0.min_length": acctest.Ct5, "string_attribute_constraints.0.max_length": acctest.Ct10, }), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ @@ -1490,7 +1508,7 @@ func TestAccCognitoIDPUserPool_schemaAttributes(t *testing.T) { names.AttrName: "mynondevnumber", "number_attribute_constraints.#": acctest.Ct1, "number_attribute_constraints.0.min_value": acctest.Ct2, - "number_attribute_constraints.0.max_value": "6", + "number_attribute_constraints.0.max_value": acctest.Ct6, "required": acctest.CtFalse, "string_attribute_constraints.#": acctest.Ct0, }), @@ -2732,6 +2750,24 @@ resource "aws_cognito_user_pool" "test" { } } + schema { + attribute_data_type = "String" + developer_only_attribute = false + mutable = true + name = "strattr" + required = false + string_attribute_constraints {} + } + + schema { + attribute_data_type = "Number" + developer_only_attribute = false + mutable = true + name = "numattr" + required = false + number_attribute_constraints {} + } + schema { attribute_data_type = "Boolean" developer_only_attribute = true @@ -2761,6 +2797,24 @@ resource "aws_cognito_user_pool" "test" { } } + schema { + attribute_data_type = "String" + developer_only_attribute = false + mutable = true + name = "strattr" + required = false + string_attribute_constraints {} + } + + schema { + attribute_data_type = "Number" + developer_only_attribute = false + mutable = true + name = "numattr" + required = false + number_attribute_constraints {} + } + schema { attribute_data_type = "Boolean" developer_only_attribute = true