From b1d820bd721823d99bda9c9ecd55b9fb186bcdfe Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Tue, 24 May 2022 12:52:24 +0100
Subject: [PATCH 01/34] Deprecating number in favour of numeric

---
 internal/provider/resource_password.go | 37 ++++++++++++++++++++++++++
 internal/provider/resource_string.go   |  7 +++++
 internal/provider/string.go            | 30 ++++++++++++++++-----
 3 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go
index 057ce298..c861e766 100644
--- a/internal/provider/resource_password.go
+++ b/internal/provider/resource_password.go
@@ -18,6 +18,7 @@ func resourcePassword() *schema.Resource {
 			"This resource *does* use a cryptographic random number generator.",
 		CreateContext: createPassword,
 		ReadContext:   readNil,
+		UpdateContext: updatePassword,
 		DeleteContext: RemoveResourceFromState,
 		Schema:        passwordSchemaV1(),
 		Importer: &schema.ResourceImporter{
@@ -34,6 +35,42 @@ func resourcePassword() *schema.Resource {
 	}
 }
 
+// updatePassword is only implemented to handle the deprecation of "number" in favour of "numeric".
+// ConflictsWith defined on the schema for both attributes ensures that only one of these attributes
+// can be supplied in the configuration.
+// What are the possible permutations?
+//  - "number" should always be set, as it has a Default set on the schema, so at the time of update
+//		create must have been run previously, so the default value of true should be present.
+func updatePassword(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+	var number, numeric bool
+	var diags diag.Diagnostics
+
+	if v, ok := d.GetOk("number"); ok {
+		number = v.(bool)
+
+		if _, ok := d.GetOk("numeric"); !ok {
+			numeric = number
+
+			err := d.Set("numeric", numeric)
+			if err != nil {
+				diags = append(diags, diag.Errorf("Could not set numeric to value of number: %s", err)...)
+			}
+
+			return diags
+		}
+	}
+
+	if d.HasChange("number") {
+		return createPassword(ctx, d, meta)
+	}
+
+	if d.HasChange("numeric") {
+		return createPassword(ctx, d, meta)
+	}
+
+	return nil
+}
+
 func createPassword(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	diags := createStringFunc(true)(ctx, d, meta)
 	if diags.HasError() {
diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go
index 1a184e31..109cc5a5 100644
--- a/internal/provider/resource_string.go
+++ b/internal/provider/resource_string.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
@@ -18,6 +19,7 @@ func resourceString() *schema.Resource {
 			"it in a password. For backwards compatibility it will continue to exist. For unique ids please " +
 			"use [random_id](id.html), for sensitive random values please use [random_password](password.html).",
 		CreateContext: createStringFunc(false),
+		UpdateContext: updateString,
 		ReadContext:   readNil,
 		DeleteContext: RemoveResourceFromState,
 		// MigrateState is deprecated but the implementation is being left in place as per the
@@ -31,6 +33,11 @@ func resourceString() *schema.Resource {
 	}
 }
 
+func updateString(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+
+	return nil
+}
+
 func importStringFunc(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
 	val := d.Id()
 
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 810ec5c8..5ec32a51 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -94,11 +94,21 @@ func passwordStringSchema() map[string]*schema.Schema {
 		},
 
 		"number": {
-			Description: "Include numeric characters in the result. Default value is `true`.",
-			Type:        schema.TypeBool,
-			Optional:    true,
-			Default:     true,
-			ForceNew:    true,
+			Description: "Include numeric characters in the result. Default value is `true`. " +
+				"**NOTE**: This is deprecated, use `numeric` instead.",
+			Type:          schema.TypeBool,
+			Optional:      true,
+			Default:       true,
+			Deprecated:    "Use numeric instead.",
+			ConflictsWith: []string{"numeric"},
+		},
+
+		"numeric": {
+			Description:   "Include numeric characters in the result. Default value is `true`.",
+			Type:          schema.TypeBool,
+			Optional:      true,
+			Default:       true,
+			ConflictsWith: []string{"number"},
 		},
 
 		"min_numeric": {
@@ -170,12 +180,18 @@ func createStringFunc(sensitive bool) func(_ context.Context, d *schema.Resource
 		minUpper := d.Get("min_upper").(int)
 		lower := d.Get("lower").(bool)
 		minLower := d.Get("min_lower").(int)
-		number := d.Get("number").(bool)
 		minNumeric := d.Get("min_numeric").(int)
 		special := d.Get("special").(bool)
 		minSpecial := d.Get("min_special").(int)
 		overrideSpecial := d.Get("override_special").(string)
 
+		var numeric bool
+		if v, ok := d.GetOk("number"); ok {
+			numeric = v.(bool)
+		} else if v, ok := d.GetOk("numeric"); ok {
+			numeric = v.(bool)
+		}
+
 		if length < minUpper+minLower+minNumeric+minSpecial {
 			return append(diags, diag.Diagnostic{
 				Severity: diag.Error,
@@ -194,7 +210,7 @@ func createStringFunc(sensitive bool) func(_ context.Context, d *schema.Resource
 		if lower {
 			chars += lowerChars
 		}
-		if number {
+		if numeric {
 			chars += numChars
 		}
 		if special {

From 15c6e3b8f0407c13c24633711e654fe7900046ce Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Tue, 24 May 2022 16:21:40 +0100
Subject: [PATCH 02/34] Switching to using a state upgrader for deprecation of
 number attribute

---
 internal/provider/resource_password.go | 67 ++++++++----------
 internal/provider/resource_string.go   |  7 --
 internal/provider/string.go            | 97 ++++++++++++++++++++------
 3 files changed, 104 insertions(+), 67 deletions(-)

diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go
index c861e766..43b34ff3 100644
--- a/internal/provider/resource_password.go
+++ b/internal/provider/resource_password.go
@@ -18,59 +18,27 @@ func resourcePassword() *schema.Resource {
 			"This resource *does* use a cryptographic random number generator.",
 		CreateContext: createPassword,
 		ReadContext:   readNil,
-		UpdateContext: updatePassword,
 		DeleteContext: RemoveResourceFromState,
-		Schema:        passwordSchemaV1(),
+		Schema:        passwordSchemaV2(),
 		Importer: &schema.ResourceImporter{
 			StateContext: importPasswordFunc,
 		},
-		SchemaVersion: 1,
+		SchemaVersion: 2,
 		StateUpgraders: []schema.StateUpgrader{
 			{
 				Version: 0,
 				Type:    resourcePasswordV0().CoreConfigSchema().ImpliedType(),
 				Upgrade: resourcePasswordStateUpgradeV0,
 			},
+			{
+				Version: 1,
+				Type:    resourcePasswordV1().CoreConfigSchema().ImpliedType(),
+				Upgrade: resourcePasswordStateUpgradeV1,
+			},
 		},
 	}
 }
 
-// updatePassword is only implemented to handle the deprecation of "number" in favour of "numeric".
-// ConflictsWith defined on the schema for both attributes ensures that only one of these attributes
-// can be supplied in the configuration.
-// What are the possible permutations?
-//  - "number" should always be set, as it has a Default set on the schema, so at the time of update
-//		create must have been run previously, so the default value of true should be present.
-func updatePassword(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
-	var number, numeric bool
-	var diags diag.Diagnostics
-
-	if v, ok := d.GetOk("number"); ok {
-		number = v.(bool)
-
-		if _, ok := d.GetOk("numeric"); !ok {
-			numeric = number
-
-			err := d.Set("numeric", numeric)
-			if err != nil {
-				diags = append(diags, diag.Errorf("Could not set numeric to value of number: %s", err)...)
-			}
-
-			return diags
-		}
-	}
-
-	if d.HasChange("number") {
-		return createPassword(ctx, d, meta)
-	}
-
-	if d.HasChange("numeric") {
-		return createPassword(ctx, d, meta)
-	}
-
-	return nil
-}
-
 func createPassword(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	diags := createStringFunc(true)(ctx, d, meta)
 	if diags.HasError() {
@@ -111,6 +79,27 @@ func importPasswordFunc(ctx context.Context, d *schema.ResourceData, meta interf
 	return []*schema.ResourceData{d}, nil
 }
 
+func resourcePasswordV1() *schema.Resource {
+	return &schema.Resource{
+		Schema: passwordSchemaV1(),
+	}
+}
+
+func resourcePasswordStateUpgradeV1(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
+	if rawState == nil {
+		return nil, fmt.Errorf("resource password state upgrade failed, state is nil")
+	}
+
+	number, ok := rawState["number"].(bool)
+	if !ok {
+		return nil, fmt.Errorf("resource password state upgrade failed, number could not be asserted as bool: %T", rawState["number"])
+	}
+
+	rawState["numeric"] = number
+
+	return rawState, nil
+}
+
 func resourcePasswordV0() *schema.Resource {
 	return &schema.Resource{
 		Schema: passwordSchemaV0(),
diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go
index 109cc5a5..1a184e31 100644
--- a/internal/provider/resource_string.go
+++ b/internal/provider/resource_string.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
@@ -19,7 +18,6 @@ func resourceString() *schema.Resource {
 			"it in a password. For backwards compatibility it will continue to exist. For unique ids please " +
 			"use [random_id](id.html), for sensitive random values please use [random_password](password.html).",
 		CreateContext: createStringFunc(false),
-		UpdateContext: updateString,
 		ReadContext:   readNil,
 		DeleteContext: RemoveResourceFromState,
 		// MigrateState is deprecated but the implementation is being left in place as per the
@@ -33,11 +31,6 @@ func resourceString() *schema.Resource {
 	}
 }
 
-func updateString(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
-
-	return nil
-}
-
 func importStringFunc(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
 	val := d.Id()
 
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 5ec32a51..1b7ee1c8 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -15,6 +15,34 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 )
 
+// passwordSchemaV2 uses passwordSchemaV1 to obtain the V1 version of the Schema key-value entries but requires that
+// the numeric entry be configured and that the number entry be altered to include ConflictsWith.
+func passwordSchemaV2() map[string]*schema.Schema {
+	passwordSchema := passwordSchemaV1()
+
+	passwordSchema["number"] = &schema.Schema{
+		Description: "Include numeric characters in the result. Default value is `true`. " +
+			"**NOTE**: This is deprecated, use `numeric` instead.",
+		Type:          schema.TypeBool,
+		Optional:      true,
+		Computed:      true,
+		ForceNew:      true,
+		ConflictsWith: []string{"numeric"},
+		Deprecated:    "Use numeric instead.",
+	}
+
+	passwordSchema["numeric"] = &schema.Schema{
+		Description:   "Include numeric characters in the result. Default value is `true`.",
+		Type:          schema.TypeBool,
+		Optional:      true,
+		Computed:      true,
+		ForceNew:      true,
+		ConflictsWith: []string{"number"},
+	}
+
+	return passwordSchema
+}
+
 // passwordSchemaV1 uses passwordSchemaV0 to obtain the V0 version of the Schema key-value entries but requires that
 // the bcrypt_hash entry be configured.
 func passwordSchemaV1() map[string]*schema.Schema {
@@ -39,6 +67,34 @@ func passwordSchemaV0() map[string]*schema.Schema {
 	return passwordSchema
 }
 
+// stringSchemaV2 uses stringSchemaV1 to obtain the V1 version of the Schema key-value entries but requires that
+// the numeric entry be configured and that the number entry be altered to include ConflictsWith.
+func stringSchemaV2() map[string]*schema.Schema {
+	stringSchema := stringSchemaV1()
+
+	stringSchema["number"] = &schema.Schema{
+		Description: "Include numeric characters in the result. Default value is `true`. " +
+			"**NOTE**: This is deprecated, use `numeric` instead.",
+		Type:          schema.TypeBool,
+		Optional:      true,
+		Computed:      true,
+		ForceNew:      true,
+		ConflictsWith: []string{"numeric"},
+		Deprecated:    "Use numeric instead.",
+	}
+
+	stringSchema["numeric"] = &schema.Schema{
+		Description:   "Include numeric characters in the result. Default value is `true`.",
+		Type:          schema.TypeBool,
+		Optional:      true,
+		Computed:      true,
+		ForceNew:      true,
+		ConflictsWith: []string{"number"},
+	}
+
+	return stringSchema
+}
+
 // stringSchemaV1 uses passwordStringSchema to obtain the default Schema key-value entries but requires that the id
 // description be configured.
 func stringSchemaV1() map[string]*schema.Schema {
@@ -94,21 +150,11 @@ func passwordStringSchema() map[string]*schema.Schema {
 		},
 
 		"number": {
-			Description: "Include numeric characters in the result. Default value is `true`. " +
-				"**NOTE**: This is deprecated, use `numeric` instead.",
-			Type:          schema.TypeBool,
-			Optional:      true,
-			Default:       true,
-			Deprecated:    "Use numeric instead.",
-			ConflictsWith: []string{"numeric"},
-		},
-
-		"numeric": {
-			Description:   "Include numeric characters in the result. Default value is `true`.",
-			Type:          schema.TypeBool,
-			Optional:      true,
-			Default:       true,
-			ConflictsWith: []string{"number"},
+			Description: "Include numeric characters in the result. Default value is `true`.",
+			Type:        schema.TypeBool,
+			Optional:    true,
+			Default:     true,
+			ForceNew:    true,
 		},
 
 		"min_numeric": {
@@ -165,8 +211,8 @@ func passwordStringSchema() map[string]*schema.Schema {
 	}
 }
 
-func createStringFunc(sensitive bool) func(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
-	return func(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+func createStringFunc(sensitive bool) func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+	return func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 		const numChars = "0123456789"
 		const lowerChars = "abcdefghijklmnopqrstuvwxyz"
 		const upperChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
@@ -186,10 +232,11 @@ func createStringFunc(sensitive bool) func(_ context.Context, d *schema.Resource
 		overrideSpecial := d.Get("override_special").(string)
 
 		var numeric bool
-		if v, ok := d.GetOk("number"); ok {
-			numeric = v.(bool)
-		} else if v, ok := d.GetOk("numeric"); ok {
-			numeric = v.(bool)
+
+		if d.HasChange("number") {
+			numeric = d.Get("number").(bool)
+		} else if d.HasChange("numeric") {
+			numeric = d.Get("numeric").(bool)
 		}
 
 		if length < minUpper+minLower+minNumeric+minSpecial {
@@ -248,6 +295,14 @@ func createStringFunc(sensitive bool) func(_ context.Context, d *schema.Resource
 			return append(diags, diag.Errorf("error setting result: %s", err)...)
 		}
 
+		// This ensures that both number and numeric are kept in-sync whilst both attributes exist.
+		if err := d.Set("number", numeric); err != nil {
+			return append(diags, diag.Errorf("error setting number: %s", err)...)
+		}
+		if err := d.Set("numeric", numeric); err != nil {
+			return append(diags, diag.Errorf("error setting numeric: %s", err)...)
+		}
+
 		if sensitive {
 			d.SetId("none")
 		} else {

From 71b3e9db63e89b9374b4f30344a0cb622db1378a Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Tue, 24 May 2022 18:55:52 +0100
Subject: [PATCH 03/34] Using HasChange to detect whether attribute value has
 been altered

---
 internal/provider/resource_string.go | 32 ++++++++++++++++++++++++++--
 internal/provider/string.go          |  7 +++---
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go
index 1a184e31..0269a5c6 100644
--- a/internal/provider/resource_string.go
+++ b/internal/provider/resource_string.go
@@ -23,11 +23,18 @@ func resourceString() *schema.Resource {
 		// MigrateState is deprecated but the implementation is being left in place as per the
 		// [SDK documentation](https://github.com/hashicorp/terraform-plugin-sdk/blob/main/helper/schema/resource.go#L91).
 		MigrateState:  resourceRandomStringMigrateState,
-		SchemaVersion: 1,
-		Schema:        stringSchemaV1(),
+		SchemaVersion: 2,
+		Schema:        stringSchemaV2(),
 		Importer: &schema.ResourceImporter{
 			StateContext: importStringFunc,
 		},
+		StateUpgraders: []schema.StateUpgrader{
+			{
+				Version: 1,
+				Type:    resourceStringV1().CoreConfigSchema().ImpliedType(),
+				Upgrade: resourceStringStateUpgradeV1,
+			},
+		},
 	}
 }
 
@@ -40,3 +47,24 @@ func importStringFunc(ctx context.Context, d *schema.ResourceData, meta interfac
 
 	return []*schema.ResourceData{d}, nil
 }
+
+func resourceStringV1() *schema.Resource {
+	return &schema.Resource{
+		Schema: stringSchemaV1(),
+	}
+}
+
+func resourceStringStateUpgradeV1(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
+	if rawState == nil {
+		return nil, fmt.Errorf("resource password state upgrade failed, state is nil")
+	}
+
+	number, ok := rawState["number"].(bool)
+	if !ok {
+		return nil, fmt.Errorf("resource password state upgrade failed, number could not be asserted as bool: %T", rawState["number"])
+	}
+
+	rawState["numeric"] = number
+
+	return rawState, nil
+}
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 1b7ee1c8..0660ddca 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -211,8 +211,8 @@ func passwordStringSchema() map[string]*schema.Schema {
 	}
 }
 
-func createStringFunc(sensitive bool) func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
-	return func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+func createStringFunc(sensitive bool) func(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+	return func(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 		const numChars = "0123456789"
 		const lowerChars = "abcdefghijklmnopqrstuvwxyz"
 		const upperChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
@@ -231,7 +231,8 @@ func createStringFunc(sensitive bool) func(ctx context.Context, d *schema.Resour
 		minSpecial := d.Get("min_special").(int)
 		overrideSpecial := d.Get("override_special").(string)
 
-		var numeric bool
+		// Default to true unless number or numeric has been changed.
+		numeric := true
 
 		if d.HasChange("number") {
 			numeric = d.Get("number").(bool)

From bccd2c758170cb61d1913dd31146e9b63ca951e7 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Wed, 25 May 2022 09:43:46 +0100
Subject: [PATCH 04/34] Using CustomizeDiff to keep number and numeric in-sync

---
 internal/provider/resource_password.go | 19 +++++++++++++++++++
 internal/provider/string.go            | 18 +++++-------------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go
index 43b34ff3..3faba6f0 100644
--- a/internal/provider/resource_password.go
+++ b/internal/provider/resource_password.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"golang.org/x/crypto/bcrypt"
 )
@@ -36,6 +37,24 @@ func resourcePassword() *schema.Resource {
 				Upgrade: resourcePasswordStateUpgradeV1,
 			},
 		},
+		CustomizeDiff: customdiff.All(
+			customdiff.IfValueChange("number",
+				func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
+					return oldValue != newValue
+				},
+				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+					return d.SetNew("numeric", d.Get("number"))
+				},
+			),
+			customdiff.IfValueChange("numeric",
+				func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
+					return oldValue != newValue
+				},
+				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+					return d.SetNew("number", d.Get("numeric"))
+				},
+			),
+		),
 	}
 }
 
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 0660ddca..a282c2ce 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -211,8 +211,8 @@ func passwordStringSchema() map[string]*schema.Schema {
 	}
 }
 
-func createStringFunc(sensitive bool) func(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
-	return func(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+func createStringFunc(sensitive bool) func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+	return func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 		const numChars = "0123456789"
 		const lowerChars = "abcdefghijklmnopqrstuvwxyz"
 		const upperChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
@@ -226,20 +226,13 @@ func createStringFunc(sensitive bool) func(_ context.Context, d *schema.Resource
 		minUpper := d.Get("min_upper").(int)
 		lower := d.Get("lower").(bool)
 		minLower := d.Get("min_lower").(int)
+		number := d.Get("number").(bool)
+		numeric := d.Get("numeric").(bool)
 		minNumeric := d.Get("min_numeric").(int)
 		special := d.Get("special").(bool)
 		minSpecial := d.Get("min_special").(int)
 		overrideSpecial := d.Get("override_special").(string)
 
-		// Default to true unless number or numeric has been changed.
-		numeric := true
-
-		if d.HasChange("number") {
-			numeric = d.Get("number").(bool)
-		} else if d.HasChange("numeric") {
-			numeric = d.Get("numeric").(bool)
-		}
-
 		if length < minUpper+minLower+minNumeric+minSpecial {
 			return append(diags, diag.Diagnostic{
 				Severity: diag.Error,
@@ -296,8 +289,7 @@ func createStringFunc(sensitive bool) func(_ context.Context, d *schema.Resource
 			return append(diags, diag.Errorf("error setting result: %s", err)...)
 		}
 
-		// This ensures that both number and numeric are kept in-sync whilst both attributes exist.
-		if err := d.Set("number", numeric); err != nil {
+		if err := d.Set("number", number); err != nil {
 			return append(diags, diag.Errorf("error setting number: %s", err)...)
 		}
 		if err := d.Set("numeric", numeric); err != nil {

From b35d514a5100f6635dcbc1a98c6303fea193dbf0 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Wed, 25 May 2022 12:58:45 +0100
Subject: [PATCH 05/34] Examine raw config to determine whether number and
 numeric have been set

---
 internal/provider/string.go | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/internal/provider/string.go b/internal/provider/string.go
index a282c2ce..3d5b8fd0 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -233,6 +233,11 @@ func createStringFunc(sensitive bool) func(ctx context.Context, d *schema.Resour
 		minSpecial := d.Get("min_special").(int)
 		overrideSpecial := d.Get("override_special").(string)
 
+		vm := d.GetRawConfig().AsValueMap()
+		if vm["number"].IsNull() && vm["numeric"].IsNull() {
+			number, numeric = true, true
+		}
+
 		if length < minUpper+minLower+minNumeric+minSpecial {
 			return append(diags, diag.Diagnostic{
 				Severity: diag.Error,

From 194e8bdb2456221fc1cf610e7dd73da71e82ea7a Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Wed, 25 May 2022 13:44:09 +0100
Subject: [PATCH 06/34] Extend CustomizeDiff to set number and numeric to true
 when both are null in config

---
 internal/provider/resource_password.go | 38 ++++++++++++++++++++++++++
 internal/provider/string.go            |  9 ++----
 2 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go
index 3faba6f0..1f905670 100644
--- a/internal/provider/resource_password.go
+++ b/internal/provider/resource_password.go
@@ -38,6 +38,44 @@ func resourcePassword() *schema.Resource {
 			},
 		},
 		CustomizeDiff: customdiff.All(
+			customdiff.IfValue("number",
+				func(ctx context.Context, value, meta interface{}) bool {
+					return !value.(bool)
+				},
+				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+					vm := d.GetRawConfig().AsValueMap()
+					if vm["number"].IsNull() && vm["numeric"].IsNull() {
+						err := d.SetNew("number", true)
+						if err != nil {
+							return err
+						}
+						err = d.SetNew("numeric", true)
+						if err != nil {
+							return err
+						}
+					}
+					return nil
+				},
+			),
+			customdiff.IfValue("numeric",
+				func(ctx context.Context, value, meta interface{}) bool {
+					return !value.(bool)
+				},
+				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+					vm := d.GetRawConfig().AsValueMap()
+					if vm["number"].IsNull() && vm["numeric"].IsNull() {
+						err := d.SetNew("number", true)
+						if err != nil {
+							return err
+						}
+						err = d.SetNew("numeric", true)
+						if err != nil {
+							return err
+						}
+					}
+					return nil
+				},
+			),
 			customdiff.IfValueChange("number",
 				func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
 					return oldValue != newValue
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 3d5b8fd0..53885c14 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -211,8 +211,8 @@ func passwordStringSchema() map[string]*schema.Schema {
 	}
 }
 
-func createStringFunc(sensitive bool) func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
-	return func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+func createStringFunc(sensitive bool) func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics {
+	return func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics {
 		const numChars = "0123456789"
 		const lowerChars = "abcdefghijklmnopqrstuvwxyz"
 		const upperChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
@@ -233,11 +233,6 @@ func createStringFunc(sensitive bool) func(ctx context.Context, d *schema.Resour
 		minSpecial := d.Get("min_special").(int)
 		overrideSpecial := d.Get("override_special").(string)
 
-		vm := d.GetRawConfig().AsValueMap()
-		if vm["number"].IsNull() && vm["numeric"].IsNull() {
-			number, numeric = true, true
-		}
-
 		if length < minUpper+minLower+minNumeric+minSpecial {
 			return append(diags, diag.Diagnostic{
 				Severity: diag.Error,

From 33f8878288aa66fb35b758566ff10638e74e38b3 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Wed, 25 May 2022 13:56:03 +0100
Subject: [PATCH 07/34] Renaming misspelled test file

---
 .../{resource_pasword_test.go => resource_password_test.go}       | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename internal/provider/{resource_pasword_test.go => resource_password_test.go} (100%)

diff --git a/internal/provider/resource_pasword_test.go b/internal/provider/resource_password_test.go
similarity index 100%
rename from internal/provider/resource_pasword_test.go
rename to internal/provider/resource_password_test.go

From dc2b121f47594773e46c0e2610afdca772d9c0a4 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Wed, 25 May 2022 14:04:02 +0100
Subject: [PATCH 08/34] Adding test for state upgrade v1

---
 internal/provider/resource_password_test.go | 49 +++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index 0e90eb06..6f1bd4b9 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -151,3 +151,52 @@ func TestResourcePasswordStateUpgradeV0(t *testing.T) {
 		})
 	}
 }
+
+func TestResourcePasswordStateUpgradeV1(t *testing.T) {
+	cases := []struct {
+		name            string
+		stateV1         map[string]interface{}
+		shouldError     bool
+		errMsg          string
+		expectedStateV2 map[string]interface{}
+	}{
+		{
+			name:        "number is not bool",
+			stateV1:     map[string]interface{}{"number": 0},
+			shouldError: true,
+			errMsg:      "resource password state upgrade failed, number could not be asserted as bool: int",
+		},
+		{
+			name:            "success",
+			stateV1:         map[string]interface{}{"number": true},
+			shouldError:     false,
+			expectedStateV2: map[string]interface{}{"number": true, "numeric": true},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			actualStateV2, err := resourcePasswordStateUpgradeV1(context.Background(), c.stateV1, nil)
+
+			if c.shouldError {
+				if !cmp.Equal(c.errMsg, err.Error()) {
+					t.Errorf("expected: %q, got: %q", c.errMsg, err)
+				}
+				if !cmp.Equal(c.expectedStateV2, actualStateV2) {
+					t.Errorf("expected: %+v, got: %+v", c.expectedStateV2, err)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("err should be nil, actual: %v", err)
+				}
+
+				for k := range c.expectedStateV2 {
+					_, ok := actualStateV2[k]
+					if !ok {
+						t.Errorf("expected key: %s is missing from state", k)
+					}
+				}
+			}
+		})
+	}
+}

From af7fa6422822ee630c7cfe0af87c35508044a5a5 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Wed, 25 May 2022 14:35:52 +0100
Subject: [PATCH 09/34] Adding test for updating number and numeric

---
 internal/provider/resource_password_test.go | 58 +++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index 6f1bd4b9..52f5a22f 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -103,6 +103,64 @@ func TestAccResourcePasswordMin(t *testing.T) {
 	})
 }
 
+func TestAccResourcePassword_UpdateNumberAndNumeric(t *testing.T) {
+	t.Parallel()
+	resource.UnitTest(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: `resource "random_password" "default" {
+							length = 12
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+					resource.TestCheckResourceAttr("random_password.default", "numeric", "true"),
+				),
+			},
+			{
+				Config: `resource "random_password" "default" {
+							length = 12
+							number = false
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("random_password.default", "number", "false"),
+					resource.TestCheckResourceAttr("random_password.default", "numeric", "false"),
+				),
+			},
+			{
+				Config: `resource "random_password" "default" {
+							length = 12
+							numeric = true
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+					resource.TestCheckResourceAttr("random_password.default", "numeric", "true"),
+				),
+			},
+			{
+				Config: `resource "random_password" "default" {
+							length = 12
+							numeric = false
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("random_password.default", "number", "false"),
+					resource.TestCheckResourceAttr("random_password.default", "numeric", "false"),
+				),
+			},
+			{
+				Config: `resource "random_password" "default" {
+							length = 12
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+					resource.TestCheckResourceAttr("random_password.default", "numeric", "true"),
+				),
+			},
+		},
+	})
+}
+
 func TestResourcePasswordStateUpgradeV0(t *testing.T) {
 	cases := []struct {
 		name            string

From 28ed88c78ab8e7c2611077760417c6c6c67f9b7c Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Wed, 25 May 2022 15:05:38 +0100
Subject: [PATCH 10/34] Adding CustomizeDiff and tests for state upgrade and
 customize diff

---
 internal/provider/resource_string.go      |  59 ++++++++++-
 internal/provider/resource_string_test.go | 114 +++++++++++++++++++++-
 2 files changed, 170 insertions(+), 3 deletions(-)

diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go
index 0269a5c6..d85f3716 100644
--- a/internal/provider/resource_string.go
+++ b/internal/provider/resource_string.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
@@ -35,6 +36,62 @@ func resourceString() *schema.Resource {
 				Upgrade: resourceStringStateUpgradeV1,
 			},
 		},
+		CustomizeDiff: customdiff.All(
+			customdiff.IfValue("number",
+				func(ctx context.Context, value, meta interface{}) bool {
+					return !value.(bool)
+				},
+				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+					vm := d.GetRawConfig().AsValueMap()
+					if vm["number"].IsNull() && vm["numeric"].IsNull() {
+						err := d.SetNew("number", true)
+						if err != nil {
+							return err
+						}
+						err = d.SetNew("numeric", true)
+						if err != nil {
+							return err
+						}
+					}
+					return nil
+				},
+			),
+			customdiff.IfValue("numeric",
+				func(ctx context.Context, value, meta interface{}) bool {
+					return !value.(bool)
+				},
+				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+					vm := d.GetRawConfig().AsValueMap()
+					if vm["number"].IsNull() && vm["numeric"].IsNull() {
+						err := d.SetNew("number", true)
+						if err != nil {
+							return err
+						}
+						err = d.SetNew("numeric", true)
+						if err != nil {
+							return err
+						}
+					}
+					return nil
+				},
+			),
+			customdiff.IfValueChange("number",
+				func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
+					return oldValue != newValue
+				},
+				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+					return d.SetNew("numeric", d.Get("number"))
+				},
+			),
+			customdiff.IfValueChange("numeric",
+				func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
+					return oldValue != newValue
+				},
+				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+					return d.SetNew("number", d.Get("numeric"))
+				},
+			),
+		),
 	}
 }
 
@@ -61,7 +118,7 @@ func resourceStringStateUpgradeV1(_ context.Context, rawState map[string]interfa
 
 	number, ok := rawState["number"].(bool)
 	if !ok {
-		return nil, fmt.Errorf("resource password state upgrade failed, number could not be asserted as bool: %T", rawState["number"])
+		return nil, fmt.Errorf("resource string state upgrade failed, number could not be asserted as bool: %T", rawState["number"])
 	}
 
 	rawState["numeric"] = number
diff --git a/internal/provider/resource_string_test.go b/internal/provider/resource_string_test.go
index 54f2a391..eeae9d68 100644
--- a/internal/provider/resource_string_test.go
+++ b/internal/provider/resource_string_test.go
@@ -1,11 +1,14 @@
 package provider
 
 import (
+	"context"
 	"fmt"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 	"regexp"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 )
 
 type customLens struct {
@@ -74,6 +77,113 @@ func TestAccResourceStringMin(t *testing.T) {
 	})
 }
 
+func TestAccResourceString_UpdateNumberAndNumeric(t *testing.T) {
+	t.Parallel()
+	resource.UnitTest(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: `resource "random_string" "default" {
+							length = 12
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+					resource.TestCheckResourceAttr("random_string.default", "numeric", "true"),
+				),
+			},
+			{
+				Config: `resource "random_string" "default" {
+							length = 12
+							number = false
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("random_string.default", "number", "false"),
+					resource.TestCheckResourceAttr("random_string.default", "numeric", "false"),
+				),
+			},
+			{
+				Config: `resource "random_string" "default" {
+							length = 12
+							numeric = true
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+					resource.TestCheckResourceAttr("random_string.default", "numeric", "true"),
+				),
+			},
+			{
+				Config: `resource "random_string" "default" {
+							length = 12
+							numeric = false
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("random_string.default", "number", "false"),
+					resource.TestCheckResourceAttr("random_string.default", "numeric", "false"),
+				),
+			},
+			{
+				Config: `resource "random_string" "default" {
+							length = 12
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+					resource.TestCheckResourceAttr("random_string.default", "numeric", "true"),
+				),
+			},
+		},
+	})
+}
+
+func TestResourceStringStateUpgradeV1(t *testing.T) {
+	cases := []struct {
+		name            string
+		stateV1         map[string]interface{}
+		shouldError     bool
+		errMsg          string
+		expectedStateV2 map[string]interface{}
+	}{
+		{
+			name:        "number is not bool",
+			stateV1:     map[string]interface{}{"number": 0},
+			shouldError: true,
+			errMsg:      "resource string state upgrade failed, number could not be asserted as bool: int",
+		},
+		{
+			name:            "success",
+			stateV1:         map[string]interface{}{"number": true},
+			shouldError:     false,
+			expectedStateV2: map[string]interface{}{"number": true, "numeric": true},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			actualStateV2, err := resourceStringStateUpgradeV1(context.Background(), c.stateV1, nil)
+
+			if c.shouldError {
+				if !cmp.Equal(c.errMsg, err.Error()) {
+					t.Errorf("expected: %q, got: %q", c.errMsg, err)
+				}
+				if !cmp.Equal(c.expectedStateV2, actualStateV2) {
+					t.Errorf("expected: %+v, got: %+v", c.expectedStateV2, err)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("err should be nil, actual: %v", err)
+				}
+
+				for k := range c.expectedStateV2 {
+					_, ok := actualStateV2[k]
+					if !ok {
+						t.Errorf("expected key: %s is missing from state", k)
+					}
+				}
+			}
+		})
+	}
+}
+
 func TestAccResourceStringErrors(t *testing.T) {
 	resource.UnitTest(t, resource.TestCase{
 		PreCheck:          func() { testAccPreCheck(t) },

From 318fc7c6b2934deea1203273ceb20c813a84b520 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Wed, 25 May 2022 15:07:12 +0100
Subject: [PATCH 11/34] Fix import state func tests

---
 internal/provider/resource_password_test.go | 2 +-
 internal/provider/resource_string_test.go   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index 52f5a22f..73d2e9c4 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -45,7 +45,7 @@ func TestAccResourcePasswordBasic(t *testing.T) {
 				},
 				ImportState:             true,
 				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"bcrypt_hash", "length", "lower", "number", "special", "upper", "min_lower", "min_numeric", "min_special", "min_upper", "override_special"},
+				ImportStateVerifyIgnore: []string{"bcrypt_hash", "length", "lower", "number", "numeric", "special", "upper", "min_lower", "min_numeric", "min_special", "min_upper", "override_special"},
 			},
 		},
 	})
diff --git a/internal/provider/resource_string_test.go b/internal/provider/resource_string_test.go
index eeae9d68..ca443331 100644
--- a/internal/provider/resource_string_test.go
+++ b/internal/provider/resource_string_test.go
@@ -32,7 +32,7 @@ func TestAccResourceString(t *testing.T) {
 				ResourceName:            "random_string.basic",
 				ImportState:             true,
 				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"length", "lower", "number", "special", "upper", "min_lower", "min_numeric", "min_special", "min_upper", "override_special"},
+				ImportStateVerifyIgnore: []string{"length", "lower", "number", "numeric", "special", "upper", "min_lower", "min_numeric", "min_special", "min_upper", "override_special"},
 			},
 		},
 	})

From 3cc6aaa823350d1c98adf77dac5ccf6e32a14b26 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Wed, 25 May 2022 17:34:01 +0100
Subject: [PATCH 12/34] DRY-up func for adding number to state during resource
 state upgrade for password and string resources

---
 internal/provider/resource_password.go | 15 ++-------------
 internal/provider/resource_string.go   | 15 ++-------------
 internal/provider/string.go            | 17 +++++++++++++++++
 3 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go
index 1f905670..f02a68b5 100644
--- a/internal/provider/resource_password.go
+++ b/internal/provider/resource_password.go
@@ -142,19 +142,8 @@ func resourcePasswordV1() *schema.Resource {
 	}
 }
 
-func resourcePasswordStateUpgradeV1(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
-	if rawState == nil {
-		return nil, fmt.Errorf("resource password state upgrade failed, state is nil")
-	}
-
-	number, ok := rawState["number"].(bool)
-	if !ok {
-		return nil, fmt.Errorf("resource password state upgrade failed, number could not be asserted as bool: %T", rawState["number"])
-	}
-
-	rawState["numeric"] = number
-
-	return rawState, nil
+func resourcePasswordStateUpgradeV1(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
+	return resourceStateUpgradeAddNumeric(ctx, rawState, meta, "password")(ctx, rawState, meta)
 }
 
 func resourcePasswordV0() *schema.Resource {
diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go
index d85f3716..342696b9 100644
--- a/internal/provider/resource_string.go
+++ b/internal/provider/resource_string.go
@@ -111,17 +111,6 @@ func resourceStringV1() *schema.Resource {
 	}
 }
 
-func resourceStringStateUpgradeV1(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
-	if rawState == nil {
-		return nil, fmt.Errorf("resource password state upgrade failed, state is nil")
-	}
-
-	number, ok := rawState["number"].(bool)
-	if !ok {
-		return nil, fmt.Errorf("resource string state upgrade failed, number could not be asserted as bool: %T", rawState["number"])
-	}
-
-	rawState["numeric"] = number
-
-	return rawState, nil
+func resourceStringStateUpgradeV1(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
+	return resourceStateUpgradeAddNumeric(ctx, rawState, meta, "string")(ctx, rawState, meta)
 }
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 53885c14..9e5956e5 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -321,3 +321,20 @@ func generateRandomBytes(charSet *string, length int) ([]byte, error) {
 func readNil(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	return nil
 }
+
+func resourceStateUpgradeAddNumeric(_ context.Context, rawState map[string]interface{}, _ interface{}, resourceName string) func(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
+	return func(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
+		if rawState == nil {
+			return nil, fmt.Errorf("resource %s state upgrade failed, state is nil", resourceName)
+		}
+
+		number, ok := rawState["number"].(bool)
+		if !ok {
+			return nil, fmt.Errorf("resource %s state upgrade failed, number could not be asserted as bool: %T", resourceName, rawState["number"])
+		}
+
+		rawState["numeric"] = number
+
+		return rawState, nil
+	}
+}

From 8530696e600cdd9ad9f77f094ceffdcb1cffe12c Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Wed, 25 May 2022 17:56:50 +0100
Subject: [PATCH 13/34] DRY-up funcs for customize diff

---
 internal/provider/resource_password.go | 58 ++------------------------
 internal/provider/resource_string.go   | 58 ++------------------------
 internal/provider/string.go            | 48 +++++++++++++++++++++
 3 files changed, 56 insertions(+), 108 deletions(-)

diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go
index f02a68b5..d7a9dc72 100644
--- a/internal/provider/resource_password.go
+++ b/internal/provider/resource_password.go
@@ -38,60 +38,10 @@ func resourcePassword() *schema.Resource {
 			},
 		},
 		CustomizeDiff: customdiff.All(
-			customdiff.IfValue("number",
-				func(ctx context.Context, value, meta interface{}) bool {
-					return !value.(bool)
-				},
-				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
-					vm := d.GetRawConfig().AsValueMap()
-					if vm["number"].IsNull() && vm["numeric"].IsNull() {
-						err := d.SetNew("number", true)
-						if err != nil {
-							return err
-						}
-						err = d.SetNew("numeric", true)
-						if err != nil {
-							return err
-						}
-					}
-					return nil
-				},
-			),
-			customdiff.IfValue("numeric",
-				func(ctx context.Context, value, meta interface{}) bool {
-					return !value.(bool)
-				},
-				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
-					vm := d.GetRawConfig().AsValueMap()
-					if vm["number"].IsNull() && vm["numeric"].IsNull() {
-						err := d.SetNew("number", true)
-						if err != nil {
-							return err
-						}
-						err = d.SetNew("numeric", true)
-						if err != nil {
-							return err
-						}
-					}
-					return nil
-				},
-			),
-			customdiff.IfValueChange("number",
-				func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
-					return oldValue != newValue
-				},
-				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
-					return d.SetNew("numeric", d.Get("number"))
-				},
-			),
-			customdiff.IfValueChange("numeric",
-				func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
-					return oldValue != newValue
-				},
-				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
-					return d.SetNew("number", d.Get("numeric"))
-				},
-			),
+			customDiffIfValue("number"),
+			customDiffIfValue("numeric"),
+			customDiffIfValueChangeNumber(),
+			customDiffIfValueChangeNumeric(),
 		),
 	}
 }
diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go
index 342696b9..882992f1 100644
--- a/internal/provider/resource_string.go
+++ b/internal/provider/resource_string.go
@@ -37,60 +37,10 @@ func resourceString() *schema.Resource {
 			},
 		},
 		CustomizeDiff: customdiff.All(
-			customdiff.IfValue("number",
-				func(ctx context.Context, value, meta interface{}) bool {
-					return !value.(bool)
-				},
-				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
-					vm := d.GetRawConfig().AsValueMap()
-					if vm["number"].IsNull() && vm["numeric"].IsNull() {
-						err := d.SetNew("number", true)
-						if err != nil {
-							return err
-						}
-						err = d.SetNew("numeric", true)
-						if err != nil {
-							return err
-						}
-					}
-					return nil
-				},
-			),
-			customdiff.IfValue("numeric",
-				func(ctx context.Context, value, meta interface{}) bool {
-					return !value.(bool)
-				},
-				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
-					vm := d.GetRawConfig().AsValueMap()
-					if vm["number"].IsNull() && vm["numeric"].IsNull() {
-						err := d.SetNew("number", true)
-						if err != nil {
-							return err
-						}
-						err = d.SetNew("numeric", true)
-						if err != nil {
-							return err
-						}
-					}
-					return nil
-				},
-			),
-			customdiff.IfValueChange("number",
-				func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
-					return oldValue != newValue
-				},
-				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
-					return d.SetNew("numeric", d.Get("number"))
-				},
-			),
-			customdiff.IfValueChange("numeric",
-				func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
-					return oldValue != newValue
-				},
-				func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
-					return d.SetNew("number", d.Get("numeric"))
-				},
-			),
+			customDiffIfValue("number"),
+			customDiffIfValue("numeric"),
+			customDiffIfValueChangeNumber(),
+			customDiffIfValueChangeNumeric(),
 		),
 	}
 }
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 9e5956e5..8d4b36db 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -11,6 +11,7 @@ import (
 	"sort"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 )
@@ -338,3 +339,50 @@ func resourceStateUpgradeAddNumeric(_ context.Context, rawState map[string]inter
 		return rawState, nil
 	}
 }
+
+func customDiffIfValue(key string) func(context.Context, *schema.ResourceDiff, interface{}) error {
+	return customdiff.IfValue(
+		key,
+		func(ctx context.Context, value, meta interface{}) bool {
+			return !value.(bool)
+		},
+		func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+			vm := d.GetRawConfig().AsValueMap()
+			if vm["number"].IsNull() && vm["numeric"].IsNull() {
+				err := d.SetNew("number", true)
+				if err != nil {
+					return err
+				}
+				err = d.SetNew("numeric", true)
+				if err != nil {
+					return err
+				}
+			}
+			return nil
+		},
+	)
+}
+
+func customDiffIfValueChangeNumber() func(context.Context, *schema.ResourceDiff, interface{}) error {
+	return customdiff.IfValueChange(
+		"number",
+		func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
+			return oldValue != newValue
+		},
+		func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+			return d.SetNew("numeric", d.Get("number"))
+		},
+	)
+}
+
+func customDiffIfValueChangeNumeric() func(context.Context, *schema.ResourceDiff, interface{}) error {
+	return customdiff.IfValueChange(
+		"numeric",
+		func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
+			return oldValue != newValue
+		},
+		func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+			return d.SetNew("number", d.Get("numeric"))
+		},
+	)
+}

From 333d4d42d0f3d397c417fcdd3a0a9820abc8afca Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Thu, 26 May 2022 07:27:08 +0100
Subject: [PATCH 14/34] Removing unnecessary func args for state upgrader

---
 internal/provider/resource_password.go | 2 +-
 internal/provider/resource_string.go   | 2 +-
 internal/provider/string.go            | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go
index d7a9dc72..bde94cc7 100644
--- a/internal/provider/resource_password.go
+++ b/internal/provider/resource_password.go
@@ -93,7 +93,7 @@ func resourcePasswordV1() *schema.Resource {
 }
 
 func resourcePasswordStateUpgradeV1(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
-	return resourceStateUpgradeAddNumeric(ctx, rawState, meta, "password")(ctx, rawState, meta)
+	return resourceStateUpgradeAddNumeric("password")(ctx, rawState, meta)
 }
 
 func resourcePasswordV0() *schema.Resource {
diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go
index 882992f1..9df3bfb3 100644
--- a/internal/provider/resource_string.go
+++ b/internal/provider/resource_string.go
@@ -62,5 +62,5 @@ func resourceStringV1() *schema.Resource {
 }
 
 func resourceStringStateUpgradeV1(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
-	return resourceStateUpgradeAddNumeric(ctx, rawState, meta, "string")(ctx, rawState, meta)
+	return resourceStateUpgradeAddNumeric("string")(ctx, rawState, meta)
 }
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 8d4b36db..04d26db0 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -323,7 +323,7 @@ func readNil(_ context.Context, d *schema.ResourceData, meta interface{}) diag.D
 	return nil
 }
 
-func resourceStateUpgradeAddNumeric(_ context.Context, rawState map[string]interface{}, _ interface{}, resourceName string) func(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
+func resourceStateUpgradeAddNumeric(resourceName string) func(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
 	return func(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
 		if rawState == nil {
 			return nil, fmt.Errorf("resource %s state upgrade failed, state is nil", resourceName)

From bc5003536ac17a103681e86fad120ac1d46bb243 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Thu, 26 May 2022 08:59:57 +0100
Subject: [PATCH 15/34] Updating docs to deprecate number and add numeric

---
 docs/resources/password.md | 3 ++-
 docs/resources/string.md   | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/resources/password.md b/docs/resources/password.md
index 2b3a7f78..fec48e08 100644
--- a/docs/resources/password.md
+++ b/docs/resources/password.md
@@ -46,7 +46,8 @@ resource "aws_db_instance" "example" {
 - `min_numeric` (Number) Minimum number of numeric characters in the result. Default value is `0`.
 - `min_special` (Number) Minimum number of special characters in the result. Default value is `0`.
 - `min_upper` (Number) Minimum number of uppercase alphabet characters in the result. Default value is `0`.
-- `number` (Boolean) Include numeric characters in the result. Default value is `true`.
+- `number` (Boolean, Deprecated) Include numeric characters in the result. Default value is `true`. **NOTE**: This is deprecated, use `numeric` instead.
+- `numeric` (Boolean) Include numeric characters in the result. Default value is `true`.
 - `override_special` (String) Supply your own list of special characters to use for string generation.  This overrides the default character list in the special argument.  The `special` argument must still be set to true for any overwritten characters to be used in generation.
 - `special` (Boolean) Include special characters in the result. These are `!@#$%&*()-_=+[]{}<>:?`. Default value is `true`.
 - `upper` (Boolean) Include uppercase alphabet characters in the result. Default value is `true`.
diff --git a/docs/resources/string.md b/docs/resources/string.md
index 31fd7d88..faa1aea5 100644
--- a/docs/resources/string.md
+++ b/docs/resources/string.md
@@ -41,7 +41,8 @@ resource "random_string" "random" {
 - `min_numeric` (Number) Minimum number of numeric characters in the result. Default value is `0`.
 - `min_special` (Number) Minimum number of special characters in the result. Default value is `0`.
 - `min_upper` (Number) Minimum number of uppercase alphabet characters in the result. Default value is `0`.
-- `number` (Boolean) Include numeric characters in the result. Default value is `true`.
+- `number` (Boolean, Deprecated) Include numeric characters in the result. Default value is `true`. **NOTE**: This is deprecated, use `numeric` instead.
+- `numeric` (Boolean) Include numeric characters in the result. Default value is `true`.
 - `override_special` (String) Supply your own list of special characters to use for string generation.  This overrides the default character list in the special argument.  The `special` argument must still be set to true for any overwritten characters to be used in generation.
 - `special` (Boolean) Include special characters in the result. These are `!@#$%&*()-_=+[]{}<>:?`. Default value is `true`.
 - `upper` (Boolean) Include uppercase alphabet characters in the result. Default value is `true`.

From dd28e5b174bc7f01c13d105c04cc76e5b94c75de Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Thu, 26 May 2022 09:07:39 +0100
Subject: [PATCH 16/34] Rewording error messages

---
 internal/provider/resource_password.go      | 2 +-
 internal/provider/resource_password_test.go | 4 ++--
 internal/provider/resource_string_test.go   | 2 +-
 internal/provider/string.go                 | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go
index bde94cc7..498efb92 100644
--- a/internal/provider/resource_password.go
+++ b/internal/provider/resource_password.go
@@ -109,7 +109,7 @@ func resourcePasswordStateUpgradeV0(_ context.Context, rawState map[string]inter
 
 	result, ok := rawState["result"].(string)
 	if !ok {
-		return nil, fmt.Errorf("resource password state upgrade failed, result could not be asserted as string: %T", rawState["result"])
+		return nil, fmt.Errorf("resource password state upgrade failed, result is not a string: %T", rawState["result"])
 	}
 
 	hash, err := generateHash(result)
diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index 73d2e9c4..52f6aa96 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -173,7 +173,7 @@ func TestResourcePasswordStateUpgradeV0(t *testing.T) {
 			name:        "result is not string",
 			stateV0:     map[string]interface{}{"result": 0},
 			shouldError: true,
-			errMsg:      "resource password state upgrade failed, result could not be asserted as string: int",
+			errMsg:      "resource password state upgrade failed, result is not a string: int",
 		},
 		{
 			name:            "success",
@@ -222,7 +222,7 @@ func TestResourcePasswordStateUpgradeV1(t *testing.T) {
 			name:        "number is not bool",
 			stateV1:     map[string]interface{}{"number": 0},
 			shouldError: true,
-			errMsg:      "resource password state upgrade failed, number could not be asserted as bool: int",
+			errMsg:      "resource password state upgrade failed, number is not a boolean: int",
 		},
 		{
 			name:            "success",
diff --git a/internal/provider/resource_string_test.go b/internal/provider/resource_string_test.go
index ca443331..dd778077 100644
--- a/internal/provider/resource_string_test.go
+++ b/internal/provider/resource_string_test.go
@@ -147,7 +147,7 @@ func TestResourceStringStateUpgradeV1(t *testing.T) {
 			name:        "number is not bool",
 			stateV1:     map[string]interface{}{"number": 0},
 			shouldError: true,
-			errMsg:      "resource string state upgrade failed, number could not be asserted as bool: int",
+			errMsg:      "resource string state upgrade failed, number is not a boolean: int",
 		},
 		{
 			name:            "success",
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 04d26db0..63fa913b 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -331,7 +331,7 @@ func resourceStateUpgradeAddNumeric(resourceName string) func(_ context.Context,
 
 		number, ok := rawState["number"].(bool)
 		if !ok {
-			return nil, fmt.Errorf("resource %s state upgrade failed, number could not be asserted as bool: %T", resourceName, rawState["number"])
+			return nil, fmt.Errorf("resource %s state upgrade failed, number is not a boolean: %T", resourceName, rawState["number"])
 		}
 
 		rawState["numeric"] = number

From 5edfc96b48e8fac849b789cf09ce193ecf0b4fa7 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Thu, 26 May 2022 09:44:57 +0100
Subject: [PATCH 17/34] Adding docs to provide context to the usage of
 customize diff funcs

---
 internal/provider/resource_password.go |  8 ++++++--
 internal/provider/resource_string.go   |  8 ++++++--
 internal/provider/string.go            | 24 +++++++++---------------
 3 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go
index 498efb92..c14a8f71 100644
--- a/internal/provider/resource_password.go
+++ b/internal/provider/resource_password.go
@@ -10,6 +10,10 @@ import (
 	"golang.org/x/crypto/bcrypt"
 )
 
+// resourcePassword and resourceString both use the same set of CustomizeDiffFunc(s) in order to handle the deprecation
+// of the `number` attribute and the simultaneous addition of the `numeric` attribute. customDiffIfValue handles
+// ensuring that both `number` and `numeric` default to `true` when they are both absent from config.
+// customDiffIfValueChange handles keeping number and numeric in-sync when either one has been changed.
 func resourcePassword() *schema.Resource {
 	return &schema.Resource{
 		Description: "Identical to [random_string](string.html) with the exception that the result is " +
@@ -40,8 +44,8 @@ func resourcePassword() *schema.Resource {
 		CustomizeDiff: customdiff.All(
 			customDiffIfValue("number"),
 			customDiffIfValue("numeric"),
-			customDiffIfValueChangeNumber(),
-			customDiffIfValueChangeNumeric(),
+			customDiffIfValueChange("number", "numeric"),
+			customDiffIfValueChange("numeric", "number"),
 		),
 	}
 }
diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go
index 9df3bfb3..b0962133 100644
--- a/internal/provider/resource_string.go
+++ b/internal/provider/resource_string.go
@@ -8,6 +8,10 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
+// resourceString and resourcePassword both use the same set of CustomizeDiffFunc(s) in order to handle the deprecation
+// of the `number` attribute and the simultaneous addition of the `numeric` attribute. customDiffIfValue handles
+// ensuring that both `number` and `numeric` default to `true` when they are both absent from config.
+// customDiffIfValueChange handles keeping number and numeric in-sync when either one has been changed.
 func resourceString() *schema.Resource {
 	return &schema.Resource{
 		Description: "The resource `random_string` generates a random permutation of alphanumeric " +
@@ -39,8 +43,8 @@ func resourceString() *schema.Resource {
 		CustomizeDiff: customdiff.All(
 			customDiffIfValue("number"),
 			customDiffIfValue("numeric"),
-			customDiffIfValueChangeNumber(),
-			customDiffIfValueChangeNumeric(),
+			customDiffIfValueChange("number", "numeric"),
+			customDiffIfValueChange("numeric", "number"),
 		),
 	}
 }
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 63fa913b..072b25a7 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -340,6 +340,10 @@ func resourceStateUpgradeAddNumeric(resourceName string) func(_ context.Context,
 	}
 }
 
+// customDiffIfValue handles ensuring that both `number` and `numeric` attributes default to `true` when neither are set
+// in the config and, they had been previously set to `false`. This behaviour mimics setting `Default: true` on the
+// attributes. Usage of `Default` is avoided as `Default` cannot be used with CustomizeDiffFunc(s) which are required in
+// order to keep `number` and `numeric` in-sync (see customDiffIfValueChange).
 func customDiffIfValue(key string) func(context.Context, *schema.ResourceDiff, interface{}) error {
 	return customdiff.IfValue(
 		key,
@@ -363,26 +367,16 @@ func customDiffIfValue(key string) func(context.Context, *schema.ResourceDiff, i
 	)
 }
 
-func customDiffIfValueChangeNumber() func(context.Context, *schema.ResourceDiff, interface{}) error {
+// customDiffIfValueChange handles keeping `number` and `numeric` in-sync. If either is changed the value of both is
+// set to the new value of the attribute that has changed.
+func customDiffIfValueChange(key, keyToSync string) func(context.Context, *schema.ResourceDiff, interface{}) error {
 	return customdiff.IfValueChange(
-		"number",
-		func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
-			return oldValue != newValue
-		},
-		func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
-			return d.SetNew("numeric", d.Get("number"))
-		},
-	)
-}
-
-func customDiffIfValueChangeNumeric() func(context.Context, *schema.ResourceDiff, interface{}) error {
-	return customdiff.IfValueChange(
-		"numeric",
+		key,
 		func(ctx context.Context, oldValue, newValue, meta interface{}) bool {
 			return oldValue != newValue
 		},
 		func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
-			return d.SetNew("number", d.Get("numeric"))
+			return d.SetNew(keyToSync, d.Get(key))
 		},
 	)
 }

From d65b4d3effe80d9af98660cfb28ef4dcf3602c45 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Thu, 26 May 2022 10:10:37 +0100
Subject: [PATCH 18/34] Updating CHANGELOG.md

---
 CHANGELOG.md | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2543b009..03c700fd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,10 @@
+## 3.3.0 (May 26, 2022)
+
+ENHANCEMENTS:
+
+* resource/random_password: `number` is now deprecated and `numeric` has been added to align attribute naming. `number` will be removed in the next major release ([#258](https://github.com/hashicorp/terraform-provider-random/pull/258)).
+* resource/random_string: `number` is now deprecated and `numeric` has been added to align attribute naming. `number` will be removed in the next major release ([#258](https://github.com/hashicorp/terraform-provider-random/pull/258)).
+
 ## 3.2.0 (May 18, 2022)
 
 NEW FEATURES:

From 8d662103e9e5a44a2546a056e27f930a05a6bd06 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Thu, 26 May 2022 12:40:40 +0100
Subject: [PATCH 19/34] Setting the CHANGELOG as unreleased

---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 03c700fd..8324648a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,4 @@
-## 3.3.0 (May 26, 2022)
+## 3.3.0 (Unreleased)
 
 ENHANCEMENTS:
 

From f45ca8533955c099c25fa82f2647ef48c2f50447 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Fri, 27 May 2022 09:39:39 +0100
Subject: [PATCH 20/34] Temporarily using sdk/v2 sha to use ExternalProviders
 in TestStep

---
 go.mod | 16 ++++++++++++++++
 go.sum | 10 ++++++++++
 2 files changed, 26 insertions(+)

diff --git a/go.mod b/go.mod
index f6617478..f721857f 100644
--- a/go.mod
+++ b/go.mod
@@ -6,9 +6,15 @@ require (
 	github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0
 	github.com/google/go-cmp v0.5.8
 	github.com/hashicorp/go-uuid v1.0.3
+<<<<<<< HEAD
 	github.com/hashicorp/terraform-plugin-docs v0.9.0
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0
 	golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e
+=======
+	github.com/hashicorp/terraform-plugin-docs v0.8.1
+	github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220526194717-a8e255aecae2
+	golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e
+>>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
 )
 
 require (
@@ -22,7 +28,11 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/fatih/color v1.13.0 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
+<<<<<<< HEAD
 	github.com/google/uuid v1.3.0 // indirect
+=======
+	github.com/google/uuid v1.1.2 // indirect
+>>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-checkpoint v0.5.0 // indirect
 	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
@@ -60,9 +70,15 @@ require (
 	github.com/vmihailenco/msgpack/v4 v4.3.12 // indirect
 	github.com/vmihailenco/tagparser v0.1.1 // indirect
 	github.com/zclconf/go-cty v1.10.0 // indirect
+<<<<<<< HEAD
 	golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2 // indirect
 	golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a // indirect
 	golang.org/x/text v0.3.7 // indirect
+=======
+	golang.org/x/net v0.0.0-20210326060303-6b1517762897 // indirect
+	golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6 // indirect
+	golang.org/x/text v0.3.5 // indirect
+>>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
 	google.golang.org/appengine v1.6.6 // indirect
 	google.golang.org/genproto v0.0.0-20200711021454-869866162049 // indirect
 	google.golang.org/grpc v1.46.0 // indirect
diff --git a/go.sum b/go.sum
index 8f688f80..3173c689 100644
--- a/go.sum
+++ b/go.sum
@@ -150,14 +150,24 @@ github.com/hashicorp/terraform-exec v0.16.1/go.mod h1:aj0lVshy8l+MHhFNoijNHtqTJQ
 github.com/hashicorp/terraform-json v0.13.0/go.mod h1:y5OdLBCT+rxbwnpxZs9kGL7R9ExU76+cpdY8zHwoazk=
 github.com/hashicorp/terraform-json v0.14.0 h1:sh9iZ1Y8IFJLx+xQiKHGud6/TSUCM0N8e17dKDpqV7s=
 github.com/hashicorp/terraform-json v0.14.0/go.mod h1:5A9HIWPkk4e5aeeXIBbkcOvaZbIYnAIkEyqP2pNSckM=
+<<<<<<< HEAD
 github.com/hashicorp/terraform-plugin-docs v0.9.0 h1:CEu7NToNWRR2os6DfT/Du2s+8qzXHyIcZQ10oiMdbJs=
 github.com/hashicorp/terraform-plugin-docs v0.9.0/go.mod h1:47ZcsxMUJxAjGzHf+dZ9q78oYf4PeJxO1N+i5XDtXBc=
+=======
+github.com/hashicorp/terraform-plugin-docs v0.8.1 h1:XJC/cDvmE7zJfDFCtOI1bURaencBQC0xYx3DZ5cWbhE=
+github.com/hashicorp/terraform-plugin-docs v0.8.1/go.mod h1:p40z/69HYNUN/G2RDYp8XUCA5B1VzGTZl7/N9V+BWXU=
+>>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
 github.com/hashicorp/terraform-plugin-go v0.9.1 h1:vXdHaQ6aqL+OF076nMSBV+JKPdmXlzG5mzVDD04WyPs=
 github.com/hashicorp/terraform-plugin-go v0.9.1/go.mod h1:ItjVSlQs70otlzcCwlPcU8FRXLdO973oYFRZwAOxy8M=
 github.com/hashicorp/terraform-plugin-log v0.4.0 h1:F3eVnm8r2EfQCe2k9blPIiF/r2TT01SHijXnS7bujvc=
 github.com/hashicorp/terraform-plugin-log v0.4.0/go.mod h1:9KclxdunFownr4pIm1jdmwKRmE4d6HVG2c9XDq47rpg=
+<<<<<<< HEAD
 github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0 h1:Qr5fWNg1SPSfCRMtou67Y6Kcy9UnMYRNlIJTKRuUvXU=
 github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0/go.mod h1:b+LFg8WpYgFgvEBP/6Htk5H9/pJp1V1E8NJAekfH2Ws=
+=======
+github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220526194717-a8e255aecae2 h1:yuFvcxHuK6DI6nCzPN1Z+a5jedtIZA+14BFAEC5nY1w=
+github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220526194717-a8e255aecae2/go.mod h1:b+LFg8WpYgFgvEBP/6Htk5H9/pJp1V1E8NJAekfH2Ws=
+>>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
 github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896 h1:1FGtlkJw87UsTMg5s8jrekrHmUPUJaMcu6ELiVhQrNw=
 github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896/go.mod h1:bzBPnUIkI0RxauU8Dqo+2KrZZ28Cf48s8V6IHt3p4co=
 github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 h1:HKLsbzeOsfXmKNpr3GiT18XAblV0BjCbzL8KQAMZGa0=

From 2f596d54df9fd0e2574700a2a63533210559a20e Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Fri, 27 May 2022 10:47:54 +0100
Subject: [PATCH 21/34] Adding test to verify state upgrades by using
 ExternalProviders within TestStep.

---
 internal/provider/resource_password_test.go | 31 +++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index 52f6aa96..3d6fd3c9 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -161,6 +161,37 @@ func TestAccResourcePassword_UpdateNumberAndNumeric(t *testing.T) {
 	})
 }
 
+func TestAccResourcePassword_V0ToV2(t *testing.T) {
+	t.Parallel()
+
+	resource.UnitTest(t, resource.TestCase{
+		Steps: []resource.TestStep{
+			{
+				ExternalProviders: map[string]resource.ExternalProvider{"random": {
+					VersionConstraint: "3.1.3",
+					Source:            "hashicorp/random",
+				}},
+				Config: `terraform {
+							required_providers {
+								random = {
+									source = "hashicorp/random"
+									version = "3.1.3"
+								}
+							}
+						}
+						provider "random" {}
+						resource "random_password" "default" {
+							length = 12
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrSet("random_password.default", "bcrypt_hash"),
+					resource.TestCheckResourceAttrSet("random_password.default", "numeric"),
+				),
+			},
+		},
+	})
+}
+
 func TestResourcePasswordStateUpgradeV0(t *testing.T) {
 	cases := []struct {
 		name            string

From 102133a5cfc3de63ce52483381dc63c5626e2947 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Fri, 27 May 2022 17:21:50 +0100
Subject: [PATCH 22/34] Bumping SDK version to use ExternalProviders in
 TestStep fix and adding test to verify behaviour

---
 go.mod                                      |  2 +-
 go.sum                                      |  5 +++++
 internal/provider/resource_password_test.go | 23 +++++++++++----------
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/go.mod b/go.mod
index f721857f..111c4961 100644
--- a/go.mod
+++ b/go.mod
@@ -12,7 +12,7 @@ require (
 	golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e
 =======
 	github.com/hashicorp/terraform-plugin-docs v0.8.1
-	github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220526194717-a8e255aecae2
+	github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220527155737-8a0031d0a59f
 	golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e
 >>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
 )
diff --git a/go.sum b/go.sum
index 3173c689..a5df7855 100644
--- a/go.sum
+++ b/go.sum
@@ -167,7 +167,12 @@ github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0/go.mod h1:b+LFg8WpYgFgvEBP/
 =======
 github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220526194717-a8e255aecae2 h1:yuFvcxHuK6DI6nCzPN1Z+a5jedtIZA+14BFAEC5nY1w=
 github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220526194717-a8e255aecae2/go.mod h1:b+LFg8WpYgFgvEBP/6Htk5H9/pJp1V1E8NJAekfH2Ws=
+<<<<<<< HEAD
 >>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
+=======
+github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220527155737-8a0031d0a59f h1:VXOO0M6G+Up71Meb8QaIiPYcO2nLU9OQwU+amxynwMQ=
+github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220527155737-8a0031d0a59f/go.mod h1:b+LFg8WpYgFgvEBP/6Htk5H9/pJp1V1E8NJAekfH2Ws=
+>>>>>>> 1ba96af (Bumping SDK version to use ExternalProviders in TestStep fix and adding test to verify behaviour)
 github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896 h1:1FGtlkJw87UsTMg5s8jrekrHmUPUJaMcu6ELiVhQrNw=
 github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896/go.mod h1:bzBPnUIkI0RxauU8Dqo+2KrZZ28Cf48s8V6IHt3p4co=
 github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 h1:HKLsbzeOsfXmKNpr3GiT18XAblV0BjCbzL8KQAMZGa0=
diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index 3d6fd3c9..cd509f44 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -171,21 +171,22 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 					VersionConstraint: "3.1.3",
 					Source:            "hashicorp/random",
 				}},
-				Config: `terraform {
-							required_providers {
-								random = {
-									source = "hashicorp/random"
-									version = "3.1.3"
-								}
-							}
-						}
-						provider "random" {}
-						resource "random_password" "default" {
+				Config: `resource "random_password" "default" {
+							length = 12
+						}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckNoResourceAttr("random_password.default", "bcrypt_hash"),
+					resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
+				),
+			},
+			{
+				ProviderFactories: testAccProviders,
+				Config: `resource "random_password" "default" {
 							length = 12
 						}`,
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttrSet("random_password.default", "bcrypt_hash"),
-					resource.TestCheckResourceAttrSet("random_password.default", "numeric"),
+					resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
 				),
 			},
 		},

From 80f58438ee52f8629dc06ed0090dfb4f70e60526 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Mon, 30 May 2022 10:43:01 +0100
Subject: [PATCH 23/34] Adding test cases to verify random password state
 upgrade behaviour for number and numeric

---
 internal/provider/resource_password_test.go | 211 ++++++++++++++++++++
 1 file changed, 211 insertions(+)

diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index cd509f44..fa950184 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -193,6 +193,217 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 	})
 }
 
+func TestAccResourcePassword_V0ToV2_NumberAndNumeric(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name                string
+		configBeforeUpgrade string
+		configDuringUpgrade string
+		beforeStateUpgrade  []resource.TestCheckFunc
+		afterStateUpgrade   []resource.TestCheckFunc
+	}{
+		{
+			name: "number is absent",
+			configBeforeUpgrade: `resource "random_password" "default" {
+						length = 12
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
+			},
+		},
+		{
+			name: "number is absent then true",
+			configBeforeUpgrade: `resource "random_password" "default" {
+						length = 12
+					}`,
+			configDuringUpgrade: `resource "random_password" "default" {
+						length = 12
+						number = true
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
+			},
+		},
+		{
+			name: "number is absent then false",
+			configBeforeUpgrade: `resource "random_password" "default" {
+						length = 12
+					}`,
+			configDuringUpgrade: `resource "random_password" "default" {
+						length = 12
+						number = false
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
+				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
+			},
+		},
+		{
+			name: "number is true",
+			configBeforeUpgrade: `resource "random_password" "default" {
+						length = 12
+						number = true
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
+			},
+		},
+		{
+			name: "number is true then absent",
+			configBeforeUpgrade: `resource "random_password" "default" {
+						length = 12
+						number = true
+					}`,
+			configDuringUpgrade: `resource "random_password" "default" {
+						length = 12
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
+			},
+		},
+		{
+			name: "number is true then false",
+			configBeforeUpgrade: `resource "random_password" "default" {
+						length = 12
+						number = true
+					}`,
+			configDuringUpgrade: `resource "random_password" "default" {
+						length = 12
+						number = false
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
+				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
+			},
+		},
+		{
+			name: "number is false",
+			configBeforeUpgrade: `resource "random_password" "default" {
+						length = 12
+						number = false
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
+				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
+				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
+			},
+		},
+		{
+			name: "number is false then absent",
+			configBeforeUpgrade: `resource "random_password" "default" {
+						length = 12
+						number = false
+					}`,
+			configDuringUpgrade: `resource "random_password" "default" {
+						length = 12
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
+				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
+			},
+		},
+		{
+			name: "number is false then true",
+			configBeforeUpgrade: `resource "random_password" "default" {
+						length = 12
+						number = false
+					}`,
+			configDuringUpgrade: `resource "random_password" "default" {
+						length = 12
+						number = true
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
+				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "number"),
+				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
+			},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if c.configDuringUpgrade == "" {
+				c.configDuringUpgrade = c.configBeforeUpgrade
+			}
+
+			resource.UnitTest(t, resource.TestCase{
+				Steps: []resource.TestStep{
+					{
+						ExternalProviders: map[string]resource.ExternalProvider{"random": {
+							VersionConstraint: "3.1.3",
+							Source:            "hashicorp/random",
+						}},
+						Config: c.configBeforeUpgrade,
+						Check:  resource.ComposeTestCheckFunc(c.beforeStateUpgrade...),
+					},
+					{
+						ProviderFactories: testAccProviders,
+						Config:            c.configDuringUpgrade,
+						Check:             resource.ComposeTestCheckFunc(c.afterStateUpgrade...),
+					},
+				},
+			})
+		})
+	}
+}
+
 func TestResourcePasswordStateUpgradeV0(t *testing.T) {
 	cases := []struct {
 		name            string

From 121e1d21af8e95ec2037f631800704573fec2541 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Mon, 30 May 2022 10:50:31 +0100
Subject: [PATCH 24/34] Consolidating V0 to V2 state upgrade tests

---
 internal/provider/resource_password_test.go | 45 ++++++---------------
 1 file changed, 13 insertions(+), 32 deletions(-)

diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index fa950184..f1bb0b95 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -161,41 +161,10 @@ func TestAccResourcePassword_UpdateNumberAndNumeric(t *testing.T) {
 	})
 }
 
+// TestAccResourcePassword_V0ToV2 covers the addition of bcrypt_hash and numeric attributes
 func TestAccResourcePassword_V0ToV2(t *testing.T) {
 	t.Parallel()
 
-	resource.UnitTest(t, resource.TestCase{
-		Steps: []resource.TestStep{
-			{
-				ExternalProviders: map[string]resource.ExternalProvider{"random": {
-					VersionConstraint: "3.1.3",
-					Source:            "hashicorp/random",
-				}},
-				Config: `resource "random_password" "default" {
-							length = 12
-						}`,
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckNoResourceAttr("random_password.default", "bcrypt_hash"),
-					resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
-				),
-			},
-			{
-				ProviderFactories: testAccProviders,
-				Config: `resource "random_password" "default" {
-							length = 12
-						}`,
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttrSet("random_password.default", "bcrypt_hash"),
-					resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
-				),
-			},
-		},
-	})
-}
-
-func TestAccResourcePassword_V0ToV2_NumberAndNumeric(t *testing.T) {
-	t.Parallel()
-
 	cases := []struct {
 		name                string
 		configBeforeUpgrade string
@@ -203,6 +172,18 @@ func TestAccResourcePassword_V0ToV2_NumberAndNumeric(t *testing.T) {
 		beforeStateUpgrade  []resource.TestCheckFunc
 		afterStateUpgrade   []resource.TestCheckFunc
 	}{
+		{
+			name: "bcrypt_hash",
+			configBeforeUpgrade: `resource "random_password" "default" {
+						length = 12
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckNoResourceAttr("random_password.default", "bcrypt_hash"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_password.default", "bcrypt_hash"),
+			},
+		},
 		{
 			name: "number is absent",
 			configBeforeUpgrade: `resource "random_password" "default" {

From 10efe170a49392994daf6323f8e50afe62adf70b Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Mon, 30 May 2022 11:11:23 +0100
Subject: [PATCH 25/34] Update unit tests for state upgraders

---
 internal/provider/resource_password_test.go | 27 +++++++++++++--------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index f1bb0b95..b6f7cf7f 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -9,6 +9,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
+	"golang.org/x/crypto/bcrypt"
 )
 
 func TestAccResourcePasswordBasic(t *testing.T) {
@@ -412,9 +413,11 @@ func TestResourcePasswordStateUpgradeV0(t *testing.T) {
 			actualStateV1, err := resourcePasswordStateUpgradeV0(context.Background(), c.stateV0, nil)
 
 			if c.shouldError {
+				// Check error msg
 				if !cmp.Equal(c.errMsg, err.Error()) {
 					t.Errorf("expected: %q, got: %q", c.errMsg, err)
 				}
+				// Check actualStateV1 is nil
 				if !cmp.Equal(c.expectedStateV1, actualStateV1) {
 					t.Errorf("expected: %+v, got: %+v", c.expectedStateV1, err)
 				}
@@ -423,11 +426,18 @@ func TestResourcePasswordStateUpgradeV0(t *testing.T) {
 					t.Errorf("err should be nil, actual: %v", err)
 				}
 
-				for k := range c.expectedStateV1 {
-					_, ok := actualStateV1[k]
-					if !ok {
-						t.Errorf("expected key: %s is missing from state", k)
-					}
+				// Compare bcrypt_hash with plaintext equivalent to verify match
+				bcryptHash := actualStateV1["bcrypt_hash"]
+				err := bcrypt.CompareHashAndPassword([]byte(bcryptHash.(string)), []byte(c.stateV0["result"].(string)))
+				if err != nil {
+					t.Error("hash and password do not match")
+				}
+
+				// Delete bcrypt_hash from actualStateV1 and expectedStateV1 so can compare
+				delete(actualStateV1, "bcrypt_hash")
+				delete(c.expectedStateV1, "bcrypt_hash")
+				if !cmp.Equal(actualStateV1, c.expectedStateV1) {
+					t.Errorf("expected: %v, got: %v", c.expectedStateV1, actualStateV1)
 				}
 			}
 		})
@@ -472,11 +482,8 @@ func TestResourcePasswordStateUpgradeV1(t *testing.T) {
 					t.Errorf("err should be nil, actual: %v", err)
 				}
 
-				for k := range c.expectedStateV2 {
-					_, ok := actualStateV2[k]
-					if !ok {
-						t.Errorf("expected key: %s is missing from state", k)
-					}
+				if !cmp.Equal(actualStateV2, c.expectedStateV2) {
+					t.Errorf("expected: %v, got: %v", c.expectedStateV2, actualStateV2)
 				}
 			}
 		})

From 6095a6242ec8ea7f968cdd4cd5c0ba4803464745 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Mon, 30 May 2022 11:47:48 +0100
Subject: [PATCH 26/34] Consolidate testing of state upgraders

---
 internal/provider/resource_password_test.go | 113 ++++++++++++--------
 1 file changed, 68 insertions(+), 45 deletions(-)

diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index b6f7cf7f..c8a56577 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -162,11 +162,12 @@ func TestAccResourcePassword_UpdateNumberAndNumeric(t *testing.T) {
 	})
 }
 
-// TestAccResourcePassword_V0ToV2 covers the addition of bcrypt_hash and numeric attributes
-func TestAccResourcePassword_V0ToV2(t *testing.T) {
+// TestAccResourcePassword_StateUpgraders covers the state upgrades from v0 to V2 and V1 to V2.
+// This includes the addition of bcrypt_hash and numeric attributes.
+func TestAccResourcePassword_StateUpgraders(t *testing.T) {
 	t.Parallel()
 
-	cases := []struct {
+	v1Cases := []struct {
 		name                string
 		configBeforeUpgrade string
 		configDuringUpgrade string
@@ -174,19 +175,7 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 		afterStateUpgrade   []resource.TestCheckFunc
 	}{
 		{
-			name: "bcrypt_hash",
-			configBeforeUpgrade: `resource "random_password" "default" {
-						length = 12
-					}`,
-			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckNoResourceAttr("random_password.default", "bcrypt_hash"),
-			},
-			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "bcrypt_hash"),
-			},
-		},
-		{
-			name: "number is absent",
+			name: "%s number is absent",
 			configBeforeUpgrade: `resource "random_password" "default" {
 						length = 12
 					}`,
@@ -202,7 +191,7 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 			},
 		},
 		{
-			name: "number is absent then true",
+			name: "%s number is absent then true",
 			configBeforeUpgrade: `resource "random_password" "default" {
 						length = 12
 					}`,
@@ -222,7 +211,7 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 			},
 		},
 		{
-			name: "number is absent then false",
+			name: "%s number is absent then false",
 			configBeforeUpgrade: `resource "random_password" "default" {
 						length = 12
 					}`,
@@ -242,7 +231,7 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 			},
 		},
 		{
-			name: "number is true",
+			name: "%s number is true",
 			configBeforeUpgrade: `resource "random_password" "default" {
 						length = 12
 						number = true
@@ -259,7 +248,7 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 			},
 		},
 		{
-			name: "number is true then absent",
+			name: "%s number is true then absent",
 			configBeforeUpgrade: `resource "random_password" "default" {
 						length = 12
 						number = true
@@ -279,7 +268,7 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 			},
 		},
 		{
-			name: "number is true then false",
+			name: "%s number is true then false",
 			configBeforeUpgrade: `resource "random_password" "default" {
 						length = 12
 						number = true
@@ -300,7 +289,7 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 			},
 		},
 		{
-			name: "number is false",
+			name: "%s number is false",
 			configBeforeUpgrade: `resource "random_password" "default" {
 						length = 12
 						number = false
@@ -317,7 +306,7 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 			},
 		},
 		{
-			name: "number is false then absent",
+			name: "%s number is false then absent",
 			configBeforeUpgrade: `resource "random_password" "default" {
 						length = 12
 						number = false
@@ -337,7 +326,7 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 			},
 		},
 		{
-			name: "number is false then true",
+			name: "%s number is false then true",
 			configBeforeUpgrade: `resource "random_password" "default" {
 						length = 12
 						number = false
@@ -359,30 +348,64 @@ func TestAccResourcePassword_V0ToV2(t *testing.T) {
 		},
 	}
 
-	for _, c := range cases {
-		t.Run(c.name, func(t *testing.T) {
-			if c.configDuringUpgrade == "" {
-				c.configDuringUpgrade = c.configBeforeUpgrade
-			}
+	v0Cases := v1Cases
+	v0Cases = append(v0Cases, struct {
+		name                string
+		configBeforeUpgrade string
+		configDuringUpgrade string
+		beforeStateUpgrade  []resource.TestCheckFunc
+		afterStateUpgrade   []resource.TestCheckFunc
+	}{
+		name: "%s bcrypt_hash",
+		configBeforeUpgrade: `resource "random_password" "default" {
+						length = 12
+					}`,
+		beforeStateUpgrade: []resource.TestCheckFunc{
+			resource.TestCheckNoResourceAttr("random_password.default", "bcrypt_hash"),
+		},
+		afterStateUpgrade: []resource.TestCheckFunc{
+			resource.TestCheckResourceAttrSet("random_password.default", "bcrypt_hash"),
+		},
+	})
 
-			resource.UnitTest(t, resource.TestCase{
-				Steps: []resource.TestStep{
-					{
-						ExternalProviders: map[string]resource.ExternalProvider{"random": {
-							VersionConstraint: "3.1.3",
-							Source:            "hashicorp/random",
-						}},
-						Config: c.configBeforeUpgrade,
-						Check:  resource.ComposeTestCheckFunc(c.beforeStateUpgrade...),
-					},
-					{
-						ProviderFactories: testAccProviders,
-						Config:            c.configDuringUpgrade,
-						Check:             resource.ComposeTestCheckFunc(c.afterStateUpgrade...),
+	cases := map[string][]struct {
+		name                string
+		configBeforeUpgrade string
+		configDuringUpgrade string
+		beforeStateUpgrade  []resource.TestCheckFunc
+		afterStateUpgrade   []resource.TestCheckFunc
+	}{
+		"3.1.3": v0Cases,
+		"3.2.0": v1Cases,
+	}
+
+	for providerVersion, v := range cases {
+		for _, c := range v {
+			name := fmt.Sprintf(c.name, providerVersion)
+			t.Run(name, func(t *testing.T) {
+				if c.configDuringUpgrade == "" {
+					c.configDuringUpgrade = c.configBeforeUpgrade
+				}
+
+				resource.UnitTest(t, resource.TestCase{
+					Steps: []resource.TestStep{
+						{
+							ExternalProviders: map[string]resource.ExternalProvider{"random": {
+								VersionConstraint: providerVersion,
+								Source:            "hashicorp/random",
+							}},
+							Config: c.configBeforeUpgrade,
+							Check:  resource.ComposeTestCheckFunc(c.beforeStateUpgrade...),
+						},
+						{
+							ProviderFactories: testAccProviders,
+							Config:            c.configDuringUpgrade,
+							Check:             resource.ComposeTestCheckFunc(c.afterStateUpgrade...),
+						},
 					},
-				},
+				})
 			})
-		})
+		}
 	}
 }
 

From fd6f3decee4a45ffbc1b7ca996c9821fa6a4d124 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Mon, 30 May 2022 11:57:35 +0100
Subject: [PATCH 27/34] Adding testing of state upgrade from v1 to v2 for
 random string

---
 internal/provider/resource_string_test.go | 233 +++++++++++++++++++++-
 1 file changed, 228 insertions(+), 5 deletions(-)

diff --git a/internal/provider/resource_string_test.go b/internal/provider/resource_string_test.go
index dd778077..52a53ed2 100644
--- a/internal/provider/resource_string_test.go
+++ b/internal/provider/resource_string_test.go
@@ -135,6 +135,232 @@ func TestAccResourceString_UpdateNumberAndNumeric(t *testing.T) {
 	})
 }
 
+// TestAccResourceString_StateUpgraders covers the state upgrade from V1 to V2.
+// This includes the addition of numeric attribute.
+func TestAccResourceString_StateUpgraders(t *testing.T) {
+	t.Parallel()
+
+	v1Cases := []struct {
+		name                string
+		configBeforeUpgrade string
+		configDuringUpgrade string
+		beforeStateUpgrade  []resource.TestCheckFunc
+		afterStateUpgrade   []resource.TestCheckFunc
+	}{
+		{
+			name: "%s number is absent",
+			configBeforeUpgrade: `resource "random_string" "default" {
+						length = 12
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
+			},
+		},
+		{
+			name: "%s number is absent then true",
+			configBeforeUpgrade: `resource "random_string" "default" {
+						length = 12
+					}`,
+			configDuringUpgrade: `resource "random_string" "default" {
+						length = 12
+						number = true
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
+			},
+		},
+		{
+			name: "%s number is absent then false",
+			configBeforeUpgrade: `resource "random_string" "default" {
+						length = 12
+					}`,
+			configDuringUpgrade: `resource "random_string" "default" {
+						length = 12
+						number = false
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
+				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
+			},
+		},
+		{
+			name: "%s number is true",
+			configBeforeUpgrade: `resource "random_string" "default" {
+						length = 12
+						number = true
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
+			},
+		},
+		{
+			name: "%s number is true then absent",
+			configBeforeUpgrade: `resource "random_string" "default" {
+						length = 12
+						number = true
+					}`,
+			configDuringUpgrade: `resource "random_string" "default" {
+						length = 12
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
+			},
+		},
+		{
+			name: "%s number is true then false",
+			configBeforeUpgrade: `resource "random_string" "default" {
+						length = 12
+						number = true
+					}`,
+			configDuringUpgrade: `resource "random_string" "default" {
+						length = 12
+						number = false
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
+				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
+			},
+		},
+		{
+			name: "%s number is false",
+			configBeforeUpgrade: `resource "random_string" "default" {
+						length = 12
+						number = false
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
+				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
+				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
+			},
+		},
+		{
+			name: "%s number is false then absent",
+			configBeforeUpgrade: `resource "random_string" "default" {
+						length = 12
+						number = false
+					}`,
+			configDuringUpgrade: `resource "random_string" "default" {
+						length = 12
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
+				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
+			},
+		},
+		{
+			name: "%s number is false then true",
+			configBeforeUpgrade: `resource "random_string" "default" {
+						length = 12
+						number = false
+					}`,
+			configDuringUpgrade: `resource "random_string" "default" {
+						length = 12
+						number = true
+					}`,
+			beforeStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
+				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
+			},
+			afterStateUpgrade: []resource.TestCheckFunc{
+				resource.TestCheckResourceAttrSet("random_string.default", "number"),
+				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
+				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
+			},
+		},
+	}
+
+	cases := map[string][]struct {
+		name                string
+		configBeforeUpgrade string
+		configDuringUpgrade string
+		beforeStateUpgrade  []resource.TestCheckFunc
+		afterStateUpgrade   []resource.TestCheckFunc
+	}{
+		"3.2.0": v1Cases,
+	}
+
+	for providerVersion, v := range cases {
+		for _, c := range v {
+			name := fmt.Sprintf(c.name, providerVersion)
+			t.Run(name, func(t *testing.T) {
+				if c.configDuringUpgrade == "" {
+					c.configDuringUpgrade = c.configBeforeUpgrade
+				}
+
+				resource.UnitTest(t, resource.TestCase{
+					Steps: []resource.TestStep{
+						{
+							ExternalProviders: map[string]resource.ExternalProvider{"random": {
+								VersionConstraint: providerVersion,
+								Source:            "hashicorp/random",
+							}},
+							Config: c.configBeforeUpgrade,
+							Check:  resource.ComposeTestCheckFunc(c.beforeStateUpgrade...),
+						},
+						{
+							ProviderFactories: testAccProviders,
+							Config:            c.configDuringUpgrade,
+							Check:             resource.ComposeTestCheckFunc(c.afterStateUpgrade...),
+						},
+					},
+				})
+			})
+		}
+	}
+}
+
 func TestResourceStringStateUpgradeV1(t *testing.T) {
 	cases := []struct {
 		name            string
@@ -173,11 +399,8 @@ func TestResourceStringStateUpgradeV1(t *testing.T) {
 					t.Errorf("err should be nil, actual: %v", err)
 				}
 
-				for k := range c.expectedStateV2 {
-					_, ok := actualStateV2[k]
-					if !ok {
-						t.Errorf("expected key: %s is missing from state", k)
-					}
+				if !cmp.Equal(actualStateV2, c.expectedStateV2) {
+					t.Errorf("expected: %v, got: %v", c.expectedStateV2, actualStateV2)
 				}
 			}
 		})

From fccb472d005b86fcae7a50cb767f0520c113f078 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Tue, 31 May 2022 15:55:55 +0100
Subject: [PATCH 28/34] Using v2.17.0 of terraform-plugin-sdk/v2 for
 ExternalProviders in TestStep

---
 go.mod | 2 +-
 go.sum | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/go.mod b/go.mod
index 111c4961..95bc2cbb 100644
--- a/go.mod
+++ b/go.mod
@@ -12,7 +12,7 @@ require (
 	golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e
 =======
 	github.com/hashicorp/terraform-plugin-docs v0.8.1
-	github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220527155737-8a0031d0a59f
+	github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0
 	golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e
 >>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
 )
diff --git a/go.sum b/go.sum
index a5df7855..f7d12069 100644
--- a/go.sum
+++ b/go.sum
@@ -162,6 +162,7 @@ github.com/hashicorp/terraform-plugin-go v0.9.1/go.mod h1:ItjVSlQs70otlzcCwlPcU8
 github.com/hashicorp/terraform-plugin-log v0.4.0 h1:F3eVnm8r2EfQCe2k9blPIiF/r2TT01SHijXnS7bujvc=
 github.com/hashicorp/terraform-plugin-log v0.4.0/go.mod h1:9KclxdunFownr4pIm1jdmwKRmE4d6HVG2c9XDq47rpg=
 <<<<<<< HEAD
+<<<<<<< HEAD
 github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0 h1:Qr5fWNg1SPSfCRMtou67Y6Kcy9UnMYRNlIJTKRuUvXU=
 github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0/go.mod h1:b+LFg8WpYgFgvEBP/6Htk5H9/pJp1V1E8NJAekfH2Ws=
 =======
@@ -173,6 +174,10 @@ github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220526194717-a8e255aeca
 github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220527155737-8a0031d0a59f h1:VXOO0M6G+Up71Meb8QaIiPYcO2nLU9OQwU+amxynwMQ=
 github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220527155737-8a0031d0a59f/go.mod h1:b+LFg8WpYgFgvEBP/6Htk5H9/pJp1V1E8NJAekfH2Ws=
 >>>>>>> 1ba96af (Bumping SDK version to use ExternalProviders in TestStep fix and adding test to verify behaviour)
+=======
+github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0 h1:Qr5fWNg1SPSfCRMtou67Y6Kcy9UnMYRNlIJTKRuUvXU=
+github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0/go.mod h1:b+LFg8WpYgFgvEBP/6Htk5H9/pJp1V1E8NJAekfH2Ws=
+>>>>>>> 92a1322 (Using v2.17.0 of terraform-plugin-sdk/v2 for ExternalProviders in TestStep)
 github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896 h1:1FGtlkJw87UsTMg5s8jrekrHmUPUJaMcu6ELiVhQrNw=
 github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896/go.mod h1:bzBPnUIkI0RxauU8Dqo+2KrZZ28Cf48s8V6IHt3p4co=
 github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 h1:HKLsbzeOsfXmKNpr3GiT18XAblV0BjCbzL8KQAMZGa0=

From 59eb92011b0440983b5de9ccecd3562ca705e305 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Fri, 3 Jun 2022 11:03:17 +0100
Subject: [PATCH 29/34] Remove redundant checks for TestCheckResourceAttrSet

---
 internal/provider/resource_password_test.go | 18 ------------------
 internal/provider/resource_string_test.go   | 18 ------------------
 2 files changed, 36 deletions(-)

diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index c8a56577..b621c292 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -180,12 +180,10 @@ func TestAccResourcePassword_StateUpgraders(t *testing.T) {
 						length = 12
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
 			},
@@ -200,12 +198,10 @@ func TestAccResourcePassword_StateUpgraders(t *testing.T) {
 						number = true
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
 			},
@@ -220,12 +216,10 @@ func TestAccResourcePassword_StateUpgraders(t *testing.T) {
 						number = false
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
 				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
 			},
@@ -237,12 +231,10 @@ func TestAccResourcePassword_StateUpgraders(t *testing.T) {
 						number = true
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
 			},
@@ -257,12 +249,10 @@ func TestAccResourcePassword_StateUpgraders(t *testing.T) {
 						length = 12
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
 			},
@@ -278,12 +268,10 @@ func TestAccResourcePassword_StateUpgraders(t *testing.T) {
 						number = false
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
 				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
 			},
@@ -295,12 +283,10 @@ func TestAccResourcePassword_StateUpgraders(t *testing.T) {
 						number = false
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
 				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
 				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
 			},
@@ -315,12 +301,10 @@ func TestAccResourcePassword_StateUpgraders(t *testing.T) {
 						length = 12
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
 				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
 			},
@@ -336,12 +320,10 @@ func TestAccResourcePassword_StateUpgraders(t *testing.T) {
 						number = true
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "false"),
 				resource.TestCheckNoResourceAttr("random_password.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_password.default", "number"),
 				resource.TestCheckResourceAttr("random_password.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_password.default", "number", "random_password.default", "numeric"),
 			},
diff --git a/internal/provider/resource_string_test.go b/internal/provider/resource_string_test.go
index 52a53ed2..6fc6306b 100644
--- a/internal/provider/resource_string_test.go
+++ b/internal/provider/resource_string_test.go
@@ -153,12 +153,10 @@ func TestAccResourceString_StateUpgraders(t *testing.T) {
 						length = 12
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
 			},
@@ -173,12 +171,10 @@ func TestAccResourceString_StateUpgraders(t *testing.T) {
 						number = true
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
 			},
@@ -193,12 +189,10 @@ func TestAccResourceString_StateUpgraders(t *testing.T) {
 						number = false
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
 				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
 			},
@@ -210,12 +204,10 @@ func TestAccResourceString_StateUpgraders(t *testing.T) {
 						number = true
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
 			},
@@ -230,12 +222,10 @@ func TestAccResourceString_StateUpgraders(t *testing.T) {
 						length = 12
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
 			},
@@ -251,12 +241,10 @@ func TestAccResourceString_StateUpgraders(t *testing.T) {
 						number = false
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
 				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
 			},
@@ -268,12 +256,10 @@ func TestAccResourceString_StateUpgraders(t *testing.T) {
 						number = false
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
 				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
 				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
 			},
@@ -288,12 +274,10 @@ func TestAccResourceString_StateUpgraders(t *testing.T) {
 						length = 12
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
 				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
 			},
@@ -309,12 +293,10 @@ func TestAccResourceString_StateUpgraders(t *testing.T) {
 						number = true
 					}`,
 			beforeStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "false"),
 				resource.TestCheckNoResourceAttr("random_string.default", "numeric"),
 			},
 			afterStateUpgrade: []resource.TestCheckFunc{
-				resource.TestCheckResourceAttrSet("random_string.default", "number"),
 				resource.TestCheckResourceAttr("random_string.default", "number", "true"),
 				resource.TestCheckResourceAttrPair("random_string.default", "number", "random_string.default", "numeric"),
 			},

From 0e0d7c5c0944dc02262f0df900b5e55c9380711f Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Fri, 3 Jun 2022 11:10:51 +0100
Subject: [PATCH 30/34] Remove shouldError bool from tests

---
 internal/provider/resource_password_test.go | 35 +++++++++------------
 internal/provider/resource_string_test.go   | 18 +++++------
 2 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index b621c292..b3a5d63b 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -2,6 +2,7 @@ package provider
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"regexp"
 	"testing"
@@ -395,20 +396,17 @@ func TestResourcePasswordStateUpgradeV0(t *testing.T) {
 	cases := []struct {
 		name            string
 		stateV0         map[string]interface{}
-		shouldError     bool
-		errMsg          string
+		err             error
 		expectedStateV1 map[string]interface{}
 	}{
 		{
-			name:        "result is not string",
-			stateV0:     map[string]interface{}{"result": 0},
-			shouldError: true,
-			errMsg:      "resource password state upgrade failed, result is not a string: int",
+			name:    "result is not string",
+			stateV0: map[string]interface{}{"result": 0},
+			err:     errors.New("resource password state upgrade failed, result is not a string: int"),
 		},
 		{
 			name:            "success",
 			stateV0:         map[string]interface{}{"result": "abc123"},
-			shouldError:     false,
 			expectedStateV1: map[string]interface{}{"result": "abc123", "bcrypt_hash": "123"},
 		},
 	}
@@ -417,10 +415,10 @@ func TestResourcePasswordStateUpgradeV0(t *testing.T) {
 		t.Run(c.name, func(t *testing.T) {
 			actualStateV1, err := resourcePasswordStateUpgradeV0(context.Background(), c.stateV0, nil)
 
-			if c.shouldError {
+			if c.err != nil {
 				// Check error msg
-				if !cmp.Equal(c.errMsg, err.Error()) {
-					t.Errorf("expected: %q, got: %q", c.errMsg, err)
+				if !cmp.Equal(c.err.Error(), err.Error()) {
+					t.Errorf("expected: %q, got: %q", c.err.Error(), err)
 				}
 				// Check actualStateV1 is nil
 				if !cmp.Equal(c.expectedStateV1, actualStateV1) {
@@ -453,20 +451,17 @@ func TestResourcePasswordStateUpgradeV1(t *testing.T) {
 	cases := []struct {
 		name            string
 		stateV1         map[string]interface{}
-		shouldError     bool
-		errMsg          string
+		err             error
 		expectedStateV2 map[string]interface{}
 	}{
 		{
-			name:        "number is not bool",
-			stateV1:     map[string]interface{}{"number": 0},
-			shouldError: true,
-			errMsg:      "resource password state upgrade failed, number is not a boolean: int",
+			name:    "number is not bool",
+			stateV1: map[string]interface{}{"number": 0},
+			err:     errors.New("resource password state upgrade failed, number is not a boolean: int"),
 		},
 		{
 			name:            "success",
 			stateV1:         map[string]interface{}{"number": true},
-			shouldError:     false,
 			expectedStateV2: map[string]interface{}{"number": true, "numeric": true},
 		},
 	}
@@ -475,9 +470,9 @@ func TestResourcePasswordStateUpgradeV1(t *testing.T) {
 		t.Run(c.name, func(t *testing.T) {
 			actualStateV2, err := resourcePasswordStateUpgradeV1(context.Background(), c.stateV1, nil)
 
-			if c.shouldError {
-				if !cmp.Equal(c.errMsg, err.Error()) {
-					t.Errorf("expected: %q, got: %q", c.errMsg, err)
+			if c.err != nil {
+				if !cmp.Equal(c.err.Error(), err.Error()) {
+					t.Errorf("expected: %q, got: %q", c.err.Error(), err)
 				}
 				if !cmp.Equal(c.expectedStateV2, actualStateV2) {
 					t.Errorf("expected: %+v, got: %+v", c.expectedStateV2, err)
diff --git a/internal/provider/resource_string_test.go b/internal/provider/resource_string_test.go
index 6fc6306b..4f1897ac 100644
--- a/internal/provider/resource_string_test.go
+++ b/internal/provider/resource_string_test.go
@@ -2,6 +2,7 @@ package provider
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"regexp"
 	"testing"
@@ -347,20 +348,17 @@ func TestResourceStringStateUpgradeV1(t *testing.T) {
 	cases := []struct {
 		name            string
 		stateV1         map[string]interface{}
-		shouldError     bool
-		errMsg          string
+		err             error
 		expectedStateV2 map[string]interface{}
 	}{
 		{
-			name:        "number is not bool",
-			stateV1:     map[string]interface{}{"number": 0},
-			shouldError: true,
-			errMsg:      "resource string state upgrade failed, number is not a boolean: int",
+			name:    "number is not bool",
+			stateV1: map[string]interface{}{"number": 0},
+			err:     errors.New("resource string state upgrade failed, number is not a boolean: int"),
 		},
 		{
 			name:            "success",
 			stateV1:         map[string]interface{}{"number": true},
-			shouldError:     false,
 			expectedStateV2: map[string]interface{}{"number": true, "numeric": true},
 		},
 	}
@@ -369,9 +367,9 @@ func TestResourceStringStateUpgradeV1(t *testing.T) {
 		t.Run(c.name, func(t *testing.T) {
 			actualStateV2, err := resourceStringStateUpgradeV1(context.Background(), c.stateV1, nil)
 
-			if c.shouldError {
-				if !cmp.Equal(c.errMsg, err.Error()) {
-					t.Errorf("expected: %q, got: %q", c.errMsg, err)
+			if c.err != nil {
+				if !cmp.Equal(c.err.Error(), err.Error()) {
+					t.Errorf("expected: %q, got: %q", c.err.Error(), err)
 				}
 				if !cmp.Equal(c.expectedStateV2, actualStateV2) {
 					t.Errorf("expected: %+v, got: %+v", c.expectedStateV2, err)

From 7a1c9be19dc1b3a9c5a8fa8a2724dbfe509b7eb6 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Fri, 3 Jun 2022 12:26:17 +0100
Subject: [PATCH 31/34] Refactoring customize diff funcs

---
 internal/provider/resource_password.go | 13 +++---
 internal/provider/resource_string.go   | 13 +++---
 internal/provider/string.go            | 64 +++++++++++++++++---------
 3 files changed, 55 insertions(+), 35 deletions(-)

diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go
index c14a8f71..aa82c7de 100644
--- a/internal/provider/resource_password.go
+++ b/internal/provider/resource_password.go
@@ -11,10 +11,14 @@ import (
 )
 
 // resourcePassword and resourceString both use the same set of CustomizeDiffFunc(s) in order to handle the deprecation
-// of the `number` attribute and the simultaneous addition of the `numeric` attribute. customDiffIfValue handles
+// of the `number` attribute and the simultaneous addition of the `numeric` attribute. planDefaultIfAllNull handles
 // ensuring that both `number` and `numeric` default to `true` when they are both absent from config.
-// customDiffIfValueChange handles keeping number and numeric in-sync when either one has been changed.
+// planSyncIfChange handles keeping number and numeric in-sync when either one has been changed.
 func resourcePassword() *schema.Resource {
+	customizeDiffFuncs := planDefaultIfAllNull(true, "number", "numeric")
+	customizeDiffFuncs = append(customizeDiffFuncs, planSyncIfChange("number", "numeric"))
+	customizeDiffFuncs = append(customizeDiffFuncs, planSyncIfChange("numeric", "number"))
+
 	return &schema.Resource{
 		Description: "Identical to [random_string](string.html) with the exception that the result is " +
 			"treated as sensitive and, thus, _not_ displayed in console output. Read more about sensitive " +
@@ -42,10 +46,7 @@ func resourcePassword() *schema.Resource {
 			},
 		},
 		CustomizeDiff: customdiff.All(
-			customDiffIfValue("number"),
-			customDiffIfValue("numeric"),
-			customDiffIfValueChange("number", "numeric"),
-			customDiffIfValueChange("numeric", "number"),
+			customizeDiffFuncs...,
 		),
 	}
 }
diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go
index b0962133..5749aeb0 100644
--- a/internal/provider/resource_string.go
+++ b/internal/provider/resource_string.go
@@ -9,10 +9,14 @@ import (
 )
 
 // resourceString and resourcePassword both use the same set of CustomizeDiffFunc(s) in order to handle the deprecation
-// of the `number` attribute and the simultaneous addition of the `numeric` attribute. customDiffIfValue handles
+// of the `number` attribute and the simultaneous addition of the `numeric` attribute. planDefaultIfAllNull handles
 // ensuring that both `number` and `numeric` default to `true` when they are both absent from config.
-// customDiffIfValueChange handles keeping number and numeric in-sync when either one has been changed.
+// planSyncIfChange handles keeping number and numeric in-sync when either one has been changed.
 func resourceString() *schema.Resource {
+	customizeDiffFuncs := planDefaultIfAllNull(true, "number", "numeric")
+	customizeDiffFuncs = append(customizeDiffFuncs, planSyncIfChange("number", "numeric"))
+	customizeDiffFuncs = append(customizeDiffFuncs, planSyncIfChange("numeric", "number"))
+
 	return &schema.Resource{
 		Description: "The resource `random_string` generates a random permutation of alphanumeric " +
 			"characters and optionally special characters.\n" +
@@ -41,10 +45,7 @@ func resourceString() *schema.Resource {
 			},
 		},
 		CustomizeDiff: customdiff.All(
-			customDiffIfValue("number"),
-			customDiffIfValue("numeric"),
-			customDiffIfValueChange("number", "numeric"),
-			customDiffIfValueChange("numeric", "number"),
+			customizeDiffFuncs...,
 		),
 	}
 }
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 072b25a7..3e2eb863 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -6,6 +6,7 @@ package provider
 import (
 	"context"
 	"crypto/rand"
+	"errors"
 	"fmt"
 	"math/big"
 	"sort"
@@ -340,36 +341,53 @@ func resourceStateUpgradeAddNumeric(resourceName string) func(_ context.Context,
 	}
 }
 
-// customDiffIfValue handles ensuring that both `number` and `numeric` attributes default to `true` when neither are set
+// planDefaultIfAllNull handles ensuring that both `number` and `numeric` attributes default to `true` when neither are set
 // in the config and, they had been previously set to `false`. This behaviour mimics setting `Default: true` on the
 // attributes. Usage of `Default` is avoided as `Default` cannot be used with CustomizeDiffFunc(s) which are required in
-// order to keep `number` and `numeric` in-sync (see customDiffIfValueChange).
-func customDiffIfValue(key string) func(context.Context, *schema.ResourceDiff, interface{}) error {
-	return customdiff.IfValue(
-		key,
-		func(ctx context.Context, value, meta interface{}) bool {
-			return !value.(bool)
-		},
-		func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
-			vm := d.GetRawConfig().AsValueMap()
-			if vm["number"].IsNull() && vm["numeric"].IsNull() {
-				err := d.SetNew("number", true)
-				if err != nil {
-					return err
+// order to keep `number` and `numeric` in-sync (see planSyncIfChange).
+func planDefaultIfAllNull(defaultVal interface{}, keys ...string) []schema.CustomizeDiffFunc {
+	var result []schema.CustomizeDiffFunc
+
+	for _, key := range keys {
+		result = append(result, customdiff.IfValue(
+			key,
+			func(ctx context.Context, value, meta interface{}) bool {
+				return !value.(bool)
+			},
+			func(_ context.Context, d *schema.ResourceDiff, _ interface{}) error {
+				vm := d.GetRawConfig().AsValueMap()
+
+				number, ok := vm["number"]
+				if !ok {
+					return errors.New("number is absent from raw config")
 				}
-				err = d.SetNew("numeric", true)
-				if err != nil {
-					return err
+
+				numeric, ok := vm["numeric"]
+				if !ok {
+					return errors.New("numeric is absent from raw config")
 				}
-			}
-			return nil
-		},
-	)
+
+				if number.IsNull() && numeric.IsNull() {
+					err := d.SetNew("number", defaultVal)
+					if err != nil {
+						return err
+					}
+					err = d.SetNew("numeric", defaultVal)
+					if err != nil {
+						return err
+					}
+				}
+				return nil
+			},
+		))
+	}
+
+	return result
 }
 
-// customDiffIfValueChange handles keeping `number` and `numeric` in-sync. If either is changed the value of both is
+// planSyncIfChange handles keeping `number` and `numeric` in-sync. If either is changed the value of both is
 // set to the new value of the attribute that has changed.
-func customDiffIfValueChange(key, keyToSync string) func(context.Context, *schema.ResourceDiff, interface{}) error {
+func planSyncIfChange(key, keyToSync string) func(context.Context, *schema.ResourceDiff, interface{}) error {
 	return customdiff.IfValueChange(
 		key,
 		func(ctx context.Context, oldValue, newValue, meta interface{}) bool {

From 40442ed197d7339af0f39d6c8565be41b98af511 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Fri, 3 Jun 2022 12:43:53 +0100
Subject: [PATCH 32/34] Updating dependencies

---
 go.mod | 16 ----------------
 go.sum | 20 --------------------
 2 files changed, 36 deletions(-)

diff --git a/go.mod b/go.mod
index 95bc2cbb..f6617478 100644
--- a/go.mod
+++ b/go.mod
@@ -6,15 +6,9 @@ require (
 	github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0
 	github.com/google/go-cmp v0.5.8
 	github.com/hashicorp/go-uuid v1.0.3
-<<<<<<< HEAD
 	github.com/hashicorp/terraform-plugin-docs v0.9.0
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0
 	golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e
-=======
-	github.com/hashicorp/terraform-plugin-docs v0.8.1
-	github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0
-	golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e
->>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
 )
 
 require (
@@ -28,11 +22,7 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/fatih/color v1.13.0 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
-<<<<<<< HEAD
 	github.com/google/uuid v1.3.0 // indirect
-=======
-	github.com/google/uuid v1.1.2 // indirect
->>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-checkpoint v0.5.0 // indirect
 	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
@@ -70,15 +60,9 @@ require (
 	github.com/vmihailenco/msgpack/v4 v4.3.12 // indirect
 	github.com/vmihailenco/tagparser v0.1.1 // indirect
 	github.com/zclconf/go-cty v1.10.0 // indirect
-<<<<<<< HEAD
 	golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2 // indirect
 	golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a // indirect
 	golang.org/x/text v0.3.7 // indirect
-=======
-	golang.org/x/net v0.0.0-20210326060303-6b1517762897 // indirect
-	golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6 // indirect
-	golang.org/x/text v0.3.5 // indirect
->>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
 	google.golang.org/appengine v1.6.6 // indirect
 	google.golang.org/genproto v0.0.0-20200711021454-869866162049 // indirect
 	google.golang.org/grpc v1.46.0 // indirect
diff --git a/go.sum b/go.sum
index f7d12069..8f688f80 100644
--- a/go.sum
+++ b/go.sum
@@ -150,34 +150,14 @@ github.com/hashicorp/terraform-exec v0.16.1/go.mod h1:aj0lVshy8l+MHhFNoijNHtqTJQ
 github.com/hashicorp/terraform-json v0.13.0/go.mod h1:y5OdLBCT+rxbwnpxZs9kGL7R9ExU76+cpdY8zHwoazk=
 github.com/hashicorp/terraform-json v0.14.0 h1:sh9iZ1Y8IFJLx+xQiKHGud6/TSUCM0N8e17dKDpqV7s=
 github.com/hashicorp/terraform-json v0.14.0/go.mod h1:5A9HIWPkk4e5aeeXIBbkcOvaZbIYnAIkEyqP2pNSckM=
-<<<<<<< HEAD
 github.com/hashicorp/terraform-plugin-docs v0.9.0 h1:CEu7NToNWRR2os6DfT/Du2s+8qzXHyIcZQ10oiMdbJs=
 github.com/hashicorp/terraform-plugin-docs v0.9.0/go.mod h1:47ZcsxMUJxAjGzHf+dZ9q78oYf4PeJxO1N+i5XDtXBc=
-=======
-github.com/hashicorp/terraform-plugin-docs v0.8.1 h1:XJC/cDvmE7zJfDFCtOI1bURaencBQC0xYx3DZ5cWbhE=
-github.com/hashicorp/terraform-plugin-docs v0.8.1/go.mod h1:p40z/69HYNUN/G2RDYp8XUCA5B1VzGTZl7/N9V+BWXU=
->>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
 github.com/hashicorp/terraform-plugin-go v0.9.1 h1:vXdHaQ6aqL+OF076nMSBV+JKPdmXlzG5mzVDD04WyPs=
 github.com/hashicorp/terraform-plugin-go v0.9.1/go.mod h1:ItjVSlQs70otlzcCwlPcU8FRXLdO973oYFRZwAOxy8M=
 github.com/hashicorp/terraform-plugin-log v0.4.0 h1:F3eVnm8r2EfQCe2k9blPIiF/r2TT01SHijXnS7bujvc=
 github.com/hashicorp/terraform-plugin-log v0.4.0/go.mod h1:9KclxdunFownr4pIm1jdmwKRmE4d6HVG2c9XDq47rpg=
-<<<<<<< HEAD
-<<<<<<< HEAD
 github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0 h1:Qr5fWNg1SPSfCRMtou67Y6Kcy9UnMYRNlIJTKRuUvXU=
 github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0/go.mod h1:b+LFg8WpYgFgvEBP/6Htk5H9/pJp1V1E8NJAekfH2Ws=
-=======
-github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220526194717-a8e255aecae2 h1:yuFvcxHuK6DI6nCzPN1Z+a5jedtIZA+14BFAEC5nY1w=
-github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220526194717-a8e255aecae2/go.mod h1:b+LFg8WpYgFgvEBP/6Htk5H9/pJp1V1E8NJAekfH2Ws=
-<<<<<<< HEAD
->>>>>>> 8bf6993 (Temporarily using sdk/v2 sha to use ExternalProviders in TestStep)
-=======
-github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220527155737-8a0031d0a59f h1:VXOO0M6G+Up71Meb8QaIiPYcO2nLU9OQwU+amxynwMQ=
-github.com/hashicorp/terraform-plugin-sdk/v2 v2.16.1-0.20220527155737-8a0031d0a59f/go.mod h1:b+LFg8WpYgFgvEBP/6Htk5H9/pJp1V1E8NJAekfH2Ws=
->>>>>>> 1ba96af (Bumping SDK version to use ExternalProviders in TestStep fix and adding test to verify behaviour)
-=======
-github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0 h1:Qr5fWNg1SPSfCRMtou67Y6Kcy9UnMYRNlIJTKRuUvXU=
-github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0/go.mod h1:b+LFg8WpYgFgvEBP/6Htk5H9/pJp1V1E8NJAekfH2Ws=
->>>>>>> 92a1322 (Using v2.17.0 of terraform-plugin-sdk/v2 for ExternalProviders in TestStep)
 github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896 h1:1FGtlkJw87UsTMg5s8jrekrHmUPUJaMcu6ELiVhQrNw=
 github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896/go.mod h1:bzBPnUIkI0RxauU8Dqo+2KrZZ28Cf48s8V6IHt3p4co=
 github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 h1:HKLsbzeOsfXmKNpr3GiT18XAblV0BjCbzL8KQAMZGa0=

From f4bb2c3b87f5d232fb1af95c32a7a005fc33ae67 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Fri, 3 Jun 2022 12:57:46 +0100
Subject: [PATCH 33/34] Adding terraform 1.2.* to testing matrix

---
 .github/workflows/test.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 37cd4688..3d3aa6aa 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -65,6 +65,7 @@ jobs:
           - '0.15.*'
           - '1.0.*'
           - '1.1.*'
+          - '1.2.*'
     steps:
 
     - name: Setup Go

From 3e61cf7b92100bafc3a70d6b9fd7541207061247 Mon Sep 17 00:00:00 2001
From: Benjamin Bennett <ben.bennett@hashicorp.com>
Date: Mon, 6 Jun 2022 09:12:01 +0100
Subject: [PATCH 34/34] Refactoring to remove usage of resource name in errors

---
 internal/provider/resource_password.go      |  6 +--
 internal/provider/resource_password_test.go | 43 ----------------
 internal/provider/resource_string.go        |  6 +--
 internal/provider/resource_string_test.go   | 46 -----------------
 internal/provider/string.go                 | 22 ++++----
 internal/provider/string_test.go            | 57 +++++++++++++++++++++
 6 files changed, 69 insertions(+), 111 deletions(-)
 create mode 100644 internal/provider/string_test.go

diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go
index aa82c7de..e6452c1a 100644
--- a/internal/provider/resource_password.go
+++ b/internal/provider/resource_password.go
@@ -42,7 +42,7 @@ func resourcePassword() *schema.Resource {
 			{
 				Version: 1,
 				Type:    resourcePasswordV1().CoreConfigSchema().ImpliedType(),
-				Upgrade: resourcePasswordStateUpgradeV1,
+				Upgrade: resourcePasswordStringStateUpgradeV1,
 			},
 		},
 		CustomizeDiff: customdiff.All(
@@ -97,10 +97,6 @@ func resourcePasswordV1() *schema.Resource {
 	}
 }
 
-func resourcePasswordStateUpgradeV1(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
-	return resourceStateUpgradeAddNumeric("password")(ctx, rawState, meta)
-}
-
 func resourcePasswordV0() *schema.Resource {
 	return &schema.Resource{
 		Schema: passwordSchemaV0(),
diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go
index b3a5d63b..7e011a79 100644
--- a/internal/provider/resource_password_test.go
+++ b/internal/provider/resource_password_test.go
@@ -446,46 +446,3 @@ func TestResourcePasswordStateUpgradeV0(t *testing.T) {
 		})
 	}
 }
-
-func TestResourcePasswordStateUpgradeV1(t *testing.T) {
-	cases := []struct {
-		name            string
-		stateV1         map[string]interface{}
-		err             error
-		expectedStateV2 map[string]interface{}
-	}{
-		{
-			name:    "number is not bool",
-			stateV1: map[string]interface{}{"number": 0},
-			err:     errors.New("resource password state upgrade failed, number is not a boolean: int"),
-		},
-		{
-			name:            "success",
-			stateV1:         map[string]interface{}{"number": true},
-			expectedStateV2: map[string]interface{}{"number": true, "numeric": true},
-		},
-	}
-
-	for _, c := range cases {
-		t.Run(c.name, func(t *testing.T) {
-			actualStateV2, err := resourcePasswordStateUpgradeV1(context.Background(), c.stateV1, nil)
-
-			if c.err != nil {
-				if !cmp.Equal(c.err.Error(), err.Error()) {
-					t.Errorf("expected: %q, got: %q", c.err.Error(), err)
-				}
-				if !cmp.Equal(c.expectedStateV2, actualStateV2) {
-					t.Errorf("expected: %+v, got: %+v", c.expectedStateV2, err)
-				}
-			} else {
-				if err != nil {
-					t.Errorf("err should be nil, actual: %v", err)
-				}
-
-				if !cmp.Equal(actualStateV2, c.expectedStateV2) {
-					t.Errorf("expected: %v, got: %v", c.expectedStateV2, actualStateV2)
-				}
-			}
-		})
-	}
-}
diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go
index 5749aeb0..9b5db706 100644
--- a/internal/provider/resource_string.go
+++ b/internal/provider/resource_string.go
@@ -41,7 +41,7 @@ func resourceString() *schema.Resource {
 			{
 				Version: 1,
 				Type:    resourceStringV1().CoreConfigSchema().ImpliedType(),
-				Upgrade: resourceStringStateUpgradeV1,
+				Upgrade: resourcePasswordStringStateUpgradeV1,
 			},
 		},
 		CustomizeDiff: customdiff.All(
@@ -65,7 +65,3 @@ func resourceStringV1() *schema.Resource {
 		Schema: stringSchemaV1(),
 	}
 }
-
-func resourceStringStateUpgradeV1(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
-	return resourceStateUpgradeAddNumeric("string")(ctx, rawState, meta)
-}
diff --git a/internal/provider/resource_string_test.go b/internal/provider/resource_string_test.go
index 4f1897ac..5168525f 100644
--- a/internal/provider/resource_string_test.go
+++ b/internal/provider/resource_string_test.go
@@ -1,13 +1,10 @@
 package provider
 
 import (
-	"context"
-	"errors"
 	"fmt"
 	"regexp"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 )
@@ -344,49 +341,6 @@ func TestAccResourceString_StateUpgraders(t *testing.T) {
 	}
 }
 
-func TestResourceStringStateUpgradeV1(t *testing.T) {
-	cases := []struct {
-		name            string
-		stateV1         map[string]interface{}
-		err             error
-		expectedStateV2 map[string]interface{}
-	}{
-		{
-			name:    "number is not bool",
-			stateV1: map[string]interface{}{"number": 0},
-			err:     errors.New("resource string state upgrade failed, number is not a boolean: int"),
-		},
-		{
-			name:            "success",
-			stateV1:         map[string]interface{}{"number": true},
-			expectedStateV2: map[string]interface{}{"number": true, "numeric": true},
-		},
-	}
-
-	for _, c := range cases {
-		t.Run(c.name, func(t *testing.T) {
-			actualStateV2, err := resourceStringStateUpgradeV1(context.Background(), c.stateV1, nil)
-
-			if c.err != nil {
-				if !cmp.Equal(c.err.Error(), err.Error()) {
-					t.Errorf("expected: %q, got: %q", c.err.Error(), err)
-				}
-				if !cmp.Equal(c.expectedStateV2, actualStateV2) {
-					t.Errorf("expected: %+v, got: %+v", c.expectedStateV2, err)
-				}
-			} else {
-				if err != nil {
-					t.Errorf("err should be nil, actual: %v", err)
-				}
-
-				if !cmp.Equal(actualStateV2, c.expectedStateV2) {
-					t.Errorf("expected: %v, got: %v", c.expectedStateV2, actualStateV2)
-				}
-			}
-		})
-	}
-}
-
 func TestAccResourceStringErrors(t *testing.T) {
 	resource.UnitTest(t, resource.TestCase{
 		PreCheck:          func() { testAccPreCheck(t) },
diff --git a/internal/provider/string.go b/internal/provider/string.go
index 3e2eb863..33330873 100644
--- a/internal/provider/string.go
+++ b/internal/provider/string.go
@@ -324,21 +324,19 @@ func readNil(_ context.Context, d *schema.ResourceData, meta interface{}) diag.D
 	return nil
 }
 
-func resourceStateUpgradeAddNumeric(resourceName string) func(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
-	return func(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
-		if rawState == nil {
-			return nil, fmt.Errorf("resource %s state upgrade failed, state is nil", resourceName)
-		}
+func resourcePasswordStringStateUpgradeV1(_ context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
+	if rawState == nil {
+		return nil, errors.New("state upgrade failed, state is nil")
+	}
 
-		number, ok := rawState["number"].(bool)
-		if !ok {
-			return nil, fmt.Errorf("resource %s state upgrade failed, number is not a boolean: %T", resourceName, rawState["number"])
-		}
+	number, ok := rawState["number"].(bool)
+	if !ok {
+		return nil, fmt.Errorf("state upgrade failed, number is not a boolean: %T", rawState["number"])
+	}
 
-		rawState["numeric"] = number
+	rawState["numeric"] = number
 
-		return rawState, nil
-	}
+	return rawState, nil
 }
 
 // planDefaultIfAllNull handles ensuring that both `number` and `numeric` attributes default to `true` when neither are set
diff --git a/internal/provider/string_test.go b/internal/provider/string_test.go
new file mode 100644
index 00000000..e4ecf3b3
--- /dev/null
+++ b/internal/provider/string_test.go
@@ -0,0 +1,57 @@
+package provider
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestResourcePasswordStringStateUpgradeV1(t *testing.T) {
+	cases := []struct {
+		name            string
+		stateV1         map[string]interface{}
+		err             error
+		expectedStateV2 map[string]interface{}
+	}{
+		{
+			name:    "raw state is nil",
+			stateV1: nil,
+			err:     errors.New("state upgrade failed, state is nil"),
+		},
+		{
+			name:    "number is not bool",
+			stateV1: map[string]interface{}{"number": 0},
+			err:     errors.New("state upgrade failed, number is not a boolean: int"),
+		},
+		{
+			name:            "success",
+			stateV1:         map[string]interface{}{"number": true},
+			expectedStateV2: map[string]interface{}{"number": true, "numeric": true},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			actualStateV2, err := resourcePasswordStringStateUpgradeV1(context.Background(), c.stateV1, nil)
+
+			if c.err != nil {
+				if !cmp.Equal(c.err.Error(), err.Error()) {
+					t.Errorf("expected: %q, got: %q", c.err.Error(), err)
+				}
+				if !cmp.Equal(c.expectedStateV2, actualStateV2) {
+					t.Errorf("expected: %+v, got: %+v", c.expectedStateV2, err)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("err should be nil, actual: %v", err)
+				}
+
+				if !cmp.Equal(actualStateV2, c.expectedStateV2) {
+					t.Errorf("expected: %v, got: %v", c.expectedStateV2, actualStateV2)
+				}
+			}
+		})
+	}
+}