From 673ecad5b30144bc8a4ddf180b290fc50a9770a3 Mon Sep 17 00:00:00 2001 From: "Gavin.Guohao.Wu" Date: Sat, 31 Jul 2021 16:25:00 +0800 Subject: [PATCH 1/4] r/aws_cognito_user_pool: update test case to add empty schema --- internal/service/cognitoidp/user_pool_test.go | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/internal/service/cognitoidp/user_pool_test.go b/internal/service/cognitoidp/user_pool_test.go index fce568c4946..24ad6fdd3da 100644 --- a/internal/service/cognitoidp/user_pool_test.go +++ b/internal/service/cognitoidp/user_pool_test.go @@ -1389,7 +1389,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.Ct3), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ "attribute_data_type": "String", "developer_only_attribute": acctest.CtFalse, @@ -1410,6 +1410,15 @@ func TestAccCognitoIDPUserPool_schemaAttributes(t *testing.T) { "required": acctest.CtFalse, "string_attribute_constraints.#": acctest.Ct0, }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ + "attribute_data_type": "String", + "developer_only_attribute": acctest.CtFalse, + "mutable": acctest.CtTrue, + "name": "alias", + "number_attribute_constraints.#": acctest.Ct0, + "required": acctest.CtFalse, + "string_attribute_constraints.#": acctest.Ct1, + }), ), }, { @@ -1417,7 +1426,7 @@ func TestAccCognitoIDPUserPool_schemaAttributes(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckUserPoolExists(ctx, resourceName, &pool2), testAccCheckUserPoolNotRecreated(&pool1, &pool2), - resource.TestCheckResourceAttr(resourceName, "schema.#", acctest.Ct3), + resource.TestCheckResourceAttr(resourceName, "schema.#", acctest.Ct4), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ "attribute_data_type": "String", "developer_only_attribute": acctest.CtFalse, @@ -1449,6 +1458,15 @@ func TestAccCognitoIDPUserPool_schemaAttributes(t *testing.T) { "required": acctest.CtFalse, "string_attribute_constraints.#": acctest.Ct0, }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ + "attribute_data_type": "String", + "developer_only_attribute": acctest.CtFalse, + "mutable": acctest.CtTrue, + "name": "alias", + "number_attribute_constraints.#": acctest.Ct0, + "required": acctest.CtFalse, + "string_attribute_constraints.#": acctest.Ct1, + }), ), }, { @@ -2662,6 +2680,15 @@ resource "aws_cognito_user_pool" "test" { } } + schema { + attribute_data_type = "String" + developer_only_attribute = false + mutable = true + name = "alias" + required = false + string_attribute_constraints {} + } + schema { attribute_data_type = "Boolean" developer_only_attribute = true @@ -2691,6 +2718,15 @@ resource "aws_cognito_user_pool" "test" { } } + schema { + attribute_data_type = "String" + developer_only_attribute = false + mutable = true + name = "alias" + required = false + string_attribute_constraints {} + } + schema { attribute_data_type = "Boolean" developer_only_attribute = true From 76167b091abcca11ca2be7eab2aefd9a45e2c55d Mon Sep 17 00:00:00 2001 From: "Gavin.Guohao.Wu" Date: Sat, 31 Jul 2021 20:46:55 +0800 Subject: [PATCH 2/4] r/aws_cognito_user_pool: implements custom set function for schema data --- internal/service/cognitoidp/user_pool.go | 54 +++++++++++++++++- internal/service/cognitoidp/user_pool_test.go | 55 ++++++++++++++++--- 2 files changed, 101 insertions(+), 8 deletions(-) diff --git a/internal/service/cognitoidp/user_pool.go b/internal/service/cognitoidp/user_pool.go index 3ee923d33ae..9bc8949cc6a 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" @@ -422,6 +424,52 @@ func resourceUserPool() *schema.Resource { Optional: true, MinItems: 1, MaxItems: 50, + Set: func(v any) int { + var buf bytes.Buffer + m := v.(map[string]any) + buf.WriteString(fmt.Sprintf("%s-", m[names.AttrName].(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.([]any) + + if len(data) > 0 { + buf.WriteString("string_attribute_constraints-") + m, _ := data[0].(map[string]any) + if ok { + 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.([]any) + + if len(data) > 0 { + buf.WriteString("number_attribute_constraints-") + m, _ := data[0].(map[string]any) + if ok { + 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()) + }, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "attribute_data_type": { @@ -1693,7 +1741,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 24ad6fdd3da..be19a8a58a6 100644 --- a/internal/service/cognitoidp/user_pool_test.go +++ b/internal/service/cognitoidp/user_pool_test.go @@ -1389,7 +1389,7 @@ func TestAccCognitoIDPUserPool_schemaAttributes(t *testing.T) { Config: testAccUserPoolConfig_schemaAttributes(rName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckUserPoolExists(ctx, resourceName, &pool1), - resource.TestCheckResourceAttr(resourceName, "schema.#", acctest.Ct3), + resource.TestCheckResourceAttr(resourceName, "schema.#", acctest.Ct4), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ "attribute_data_type": "String", "developer_only_attribute": acctest.CtFalse, @@ -1414,19 +1414,27 @@ func TestAccCognitoIDPUserPool_schemaAttributes(t *testing.T) { "attribute_data_type": "String", "developer_only_attribute": acctest.CtFalse, "mutable": acctest.CtTrue, - "name": "alias", + names.AttrName: "strattr", "number_attribute_constraints.#": acctest.Ct0, "required": acctest.CtFalse, "string_attribute_constraints.#": acctest.Ct1, }), - ), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ + "attribute_data_type": "Number", + "developer_only_attribute": acctest.CtFalse, + "mutable": acctest.CtTrue, + names.AttrName: "numattr", + "required": acctest.CtFalse, + "number_attribute_constraints.#": acctest.Ct1, + "string_attribute_constraints.#": acctest.Ct0, + })), }, { Config: testAccUserPoolConfig_schemaAttributesUpdated(rName, "mybool"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckUserPoolExists(ctx, resourceName, &pool2), testAccCheckUserPoolNotRecreated(&pool1, &pool2), - resource.TestCheckResourceAttr(resourceName, "schema.#", acctest.Ct4), + resource.TestCheckResourceAttr(resourceName, "schema.#", "5"), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ "attribute_data_type": "String", "developer_only_attribute": acctest.CtFalse, @@ -1462,17 +1470,32 @@ func TestAccCognitoIDPUserPool_schemaAttributes(t *testing.T) { "attribute_data_type": "String", "developer_only_attribute": acctest.CtFalse, "mutable": acctest.CtTrue, - "name": "alias", + names.AttrName: "strattr", "number_attribute_constraints.#": acctest.Ct0, "required": acctest.CtFalse, "string_attribute_constraints.#": acctest.Ct1, }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "schema.*", map[string]string{ + "attribute_data_type": "Number", + "developer_only_attribute": acctest.CtFalse, + "mutable": acctest.CtTrue, + names.AttrName: "numattr", + "required": acctest.CtFalse, + "number_attribute_constraints.#": acctest.Ct1, + "string_attribute_constraints.#": acctest.Ct0, + }), ), }, { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "schema.2.number_attribute_constraints.0.max_value", + "schema.2.number_attribute_constraints.0.min_value", + "schema.4.string_attribute_constraints.0.max_length", + "schema.4.string_attribute_constraints.0.min_length", + }, }, }, }) @@ -2684,11 +2707,20 @@ resource "aws_cognito_user_pool" "test" { attribute_data_type = "String" developer_only_attribute = false mutable = true - name = "alias" + 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 @@ -2722,11 +2754,20 @@ resource "aws_cognito_user_pool" "test" { attribute_data_type = "String" developer_only_attribute = false mutable = true - name = "alias" + 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 From ec94aeb3b8e236fb9e228a215fc8a124ad8a9cee Mon Sep 17 00:00:00 2001 From: "Gavin.Guohao.Wu" Date: Sat, 31 Jul 2021 20:54:29 +0800 Subject: [PATCH 3/4] r/aws_cognito_user_pool: add change log doc --- .changelog/20386.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/20386.txt diff --git a/.changelog/20386.txt b/.changelog/20386.txt new file mode 100644 index 00000000000..617280584a8 --- /dev/null +++ b/.changelog/20386.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_cognito_user_pool: Fixes error when `schema` has empty `string_attribute_constraints` or `number_attribute_constraints` +``` From 20828b00174fa0b2aebbb18d1b6e82ed9f2057f6 Mon Sep 17 00:00:00 2001 From: "Gavin.Guohao.Wu" Date: Sat, 31 Jul 2021 21:08:17 +0800 Subject: [PATCH 4/4] r/aws_cognito_user_pool: extract method for calculate hash --- internal/service/cognitoidp/user_pool.go | 94 ++++++++++++------------ 1 file changed, 48 insertions(+), 46 deletions(-) diff --git a/internal/service/cognitoidp/user_pool.go b/internal/service/cognitoidp/user_pool.go index 9bc8949cc6a..a80454d1272 100644 --- a/internal/service/cognitoidp/user_pool.go +++ b/internal/service/cognitoidp/user_pool.go @@ -424,52 +424,7 @@ func resourceUserPool() *schema.Resource { Optional: true, MinItems: 1, MaxItems: 50, - Set: func(v any) int { - var buf bytes.Buffer - m := v.(map[string]any) - buf.WriteString(fmt.Sprintf("%s-", m[names.AttrName].(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.([]any) - - if len(data) > 0 { - buf.WriteString("string_attribute_constraints-") - m, _ := data[0].(map[string]any) - if ok { - 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.([]any) - - if len(data) > 0 { - buf.WriteString("number_attribute_constraints-") - m, _ := data[0].(map[string]any) - if ok { - 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()) - }, + Set: resourceUserPoolSchemaHash, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "attribute_data_type": { @@ -2331,3 +2286,50 @@ func userPoolSchemaAttributeMatchesStandardAttribute(apiObject *awstypes.SchemaA return false } + +func resourceUserPoolSchemaHash(v any) int { + var buf bytes.Buffer + m := v.(map[string]any) + buf.WriteString(fmt.Sprintf("%s-", m[names.AttrName].(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.([]any) + + if len(data) > 0 { + buf.WriteString("string_attribute_constraints-") + m, _ := data[0].(map[string]any) + if ok { + 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.([]any) + + if len(data) > 0 { + buf.WriteString("number_attribute_constraints-") + m, _ := data[0].(map[string]any) + if ok { + 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()) +}