From 3c57c406d67ef9746a2f9b29878a690857374c3e Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 2 Sep 2022 17:01:52 -0400 Subject: [PATCH 1/9] resource/random_password: Implement state upgrade for invalid bcrypt_hash --- internal/provider/resource_password.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go index cbfcba19..899462a6 100644 --- a/internal/provider/resource_password.go +++ b/internal/provider/resource_password.go @@ -269,7 +269,6 @@ func upgradePasswordStateV2toV3(ctx context.Context, req resource.UpgradeStateRe // Schema version 2 to schema version 3 is a duplicate of the data, // however the BcryptHash value may have been incorrectly generated. - //nolint:gosimple // V3 model will expand over time so all fields are written out to help future code changes. passwordDataV3 := passwordModelV3{ BcryptHash: passwordDataV2.BcryptHash, From 4ec5c10f59df3d7dfd8dfc2e8142998edf440956 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Tue, 6 Sep 2022 15:42:22 +0100 Subject: [PATCH 2/9] Add state upgrader to handle null values that could be present in state following import with earlier versions of the provider. For instance, using v3.3.1 to import a random password resource will result in the state file containing null values for length, lower, number, special, upper, min_lower, min_numeric, min_special, min_upper attributes. --- internal/provider/provider_test.go | 9 + internal/provider/resource_password.go | 653 +++++++++++++++++--- internal/provider/resource_password_test.go | 578 ++++++++++++++++- 3 files changed, 1154 insertions(+), 86 deletions(-) diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index aa22c51d..c5cfbb7f 100644 --- a/internal/provider/provider_test.go +++ b/internal/provider/provider_test.go @@ -22,6 +22,15 @@ func providerVersion221() map[string]resource.ExternalProvider { } } +func providerVersion313() map[string]resource.ExternalProvider { + return map[string]resource.ExternalProvider{ + "random": { + VersionConstraint: "3.1.3", + Source: "hashicorp/random", + }, + } +} + func providerVersion320() map[string]resource.ExternalProvider { return map[string]resource.ExternalProvider{ "random": { diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go index 899462a6..89902b92 100644 --- a/internal/provider/resource_password.go +++ b/internal/provider/resource_password.go @@ -23,7 +23,7 @@ var _ provider.ResourceType = (*passwordResourceType)(nil) type passwordResourceType struct{} func (r *passwordResourceType) GetSchema(context.Context) (tfsdk.Schema, diag.Diagnostics) { - return passwordSchemaV3(), nil + return passwordSchemaV4(), nil } func (r *passwordResourceType) NewResource(_ context.Context, _ provider.Provider) (resource.Resource, diag.Diagnostics) { @@ -39,7 +39,7 @@ var ( type passwordResource struct{} func (r *passwordResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { - var plan passwordModelV3 + var plan passwordModelV4 diags := req.Plan.Get(ctx, &plan) resp.Diagnostics.Append(diags...) @@ -84,7 +84,7 @@ func (r *passwordResource) Read(ctx context.Context, req resource.ReadRequest, r // Update ensures the plan value is copied to the state to complete the update. func (r *passwordResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { - var model passwordModelV3 + var model passwordModelV4 resp.Diagnostics.Append(req.Plan.Get(ctx, &model)...) @@ -103,7 +103,7 @@ func (r *passwordResource) Delete(ctx context.Context, req resource.DeleteReques func (r *passwordResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { id := req.ID - state := passwordModelV3{ + state := passwordModelV4{ ID: types.String{Value: "none"}, Result: types.String{Value: id}, Length: types.Int64{Value: int64(len(id))}, @@ -141,24 +141,29 @@ func (r *passwordResource) UpgradeState(context.Context) map[int64]resource.Stat schemaV0 := passwordSchemaV0() schemaV1 := passwordSchemaV1() schemaV2 := passwordSchemaV2() + schemaV3 := passwordSchemaV3() return map[int64]resource.StateUpgrader{ 0: { PriorSchema: &schemaV0, - StateUpgrader: upgradePasswordStateV0toV3, + StateUpgrader: upgradePasswordStateV0toV4, }, 1: { PriorSchema: &schemaV1, - StateUpgrader: upgradePasswordStateV1toV3, + StateUpgrader: upgradePasswordStateV1toV4, }, 2: { PriorSchema: &schemaV2, - StateUpgrader: upgradePasswordStateV2toV3, + StateUpgrader: upgradePasswordStateV2toV4, + }, + 3: { + PriorSchema: &schemaV3, + StateUpgrader: upgradePasswordStateV3toV4, }, } } -func upgradePasswordStateV0toV3(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { +func upgradePasswordStateV0toV4(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { type modelV0 struct { ID types.String `tfsdk:"id"` Keepers types.Map `tfsdk:"keepers"` @@ -182,36 +187,99 @@ func upgradePasswordStateV0toV3(ctx context.Context, req resource.UpgradeStateRe return } - passwordDataV3 := passwordModelV3{ + // Setting fields that can contain null to non-null to prevent forced replacement. + // This can occur in cases where import has been used in provider versions v3.3.1 and earlier. + // If import has been used with v3.3.1, for instance then length, lower, number, special, upper, + // min_lower, min_numeric, min_special and min_upper attributes will all be null in state. + length := passwordDataV0.Length + + if length.IsNull() { + length.Null = false + length.Value = int64(len(passwordDataV0.Result.Value)) + } + + minNumeric := passwordDataV0.MinNumeric + + if minNumeric.IsNull() { + minNumeric.Null = false + } + + minUpper := passwordDataV0.MinUpper + + if minUpper.IsNull() { + minUpper.Null = false + } + + minLower := passwordDataV0.MinLower + + if minLower.IsNull() { + minLower.Null = false + } + + minSpecial := passwordDataV0.MinSpecial + + if minSpecial.IsNull() { + minSpecial.Null = false + } + + special := passwordDataV0.Special + + if special.IsNull() { + special.Null = false + special.Value = true + } + + upper := passwordDataV0.Upper + + if upper.IsNull() { + upper.Null = false + upper.Value = true + } + + lower := passwordDataV0.Lower + + if lower.IsNull() { + lower.Null = false + lower.Value = true + } + + number := passwordDataV0.Number + + if number.IsNull() { + number.Null = false + number.Value = true + } + + passwordDataV4 := passwordModelV4{ Keepers: passwordDataV0.Keepers, - Length: passwordDataV0.Length, - Special: passwordDataV0.Special, - Upper: passwordDataV0.Upper, - Lower: passwordDataV0.Lower, - Number: passwordDataV0.Number, - Numeric: passwordDataV0.Number, - MinNumeric: passwordDataV0.MinNumeric, - MinUpper: passwordDataV0.MinUpper, - MinLower: passwordDataV0.MinLower, - MinSpecial: passwordDataV0.MinSpecial, + Length: length, + Special: special, + Upper: upper, + Lower: lower, + Number: number, + Numeric: number, + MinNumeric: minNumeric, + MinUpper: minUpper, + MinLower: minLower, + MinSpecial: minSpecial, OverrideSpecial: passwordDataV0.OverrideSpecial, Result: passwordDataV0.Result, ID: passwordDataV0.ID, } - hash, err := generateHash(passwordDataV3.Result.Value) + hash, err := generateHash(passwordDataV4.Result.Value) if err != nil { resp.Diagnostics.Append(diagnostics.HashGenerationError(err.Error())...) return } - passwordDataV3.BcryptHash.Value = hash + passwordDataV4.BcryptHash.Value = hash - diags := resp.State.Set(ctx, passwordDataV3) + diags := resp.State.Set(ctx, passwordDataV4) resp.Diagnostics.Append(diags...) } -func upgradePasswordStateV1toV3(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { +func upgradePasswordStateV1toV4(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { type modelV1 struct { ID types.String `tfsdk:"id"` Keepers types.Map `tfsdk:"keepers"` @@ -236,29 +304,110 @@ func upgradePasswordStateV1toV3(ctx context.Context, req resource.UpgradeStateRe return } - passwordDataV3 := passwordModelV3{ + // Setting fields that can contain null to non-null to prevent forced replacement. + // This can occur in cases where import has been used in provider versions v3.3.1 and earlier. + // If import has been used with v3.3.1, for instance then length, lower, number, special, upper, + // min_lower, min_numeric, min_special and min_upper attributes will all be null in state. + length := passwordDataV1.Length + + if length.IsNull() { + length.Null = false + length.Value = int64(len(passwordDataV1.Result.Value)) + } + + minNumeric := passwordDataV1.MinNumeric + + if minNumeric.IsNull() { + minNumeric.Null = false + } + + minUpper := passwordDataV1.MinUpper + + if minUpper.IsNull() { + minUpper.Null = false + } + + minLower := passwordDataV1.MinLower + + if minLower.IsNull() { + minLower.Null = false + } + + minSpecial := passwordDataV1.MinSpecial + + if minSpecial.IsNull() { + minSpecial.Null = false + } + + special := passwordDataV1.Special + + if special.IsNull() { + special.Null = false + special.Value = true + } + + upper := passwordDataV1.Upper + + if upper.IsNull() { + upper.Null = false + upper.Value = true + } + + lower := passwordDataV1.Lower + + if lower.IsNull() { + lower.Null = false + lower.Value = true + } + + number := passwordDataV1.Number + + if number.IsNull() { + number.Null = false + number.Value = true + } + + passwordDataV4 := passwordModelV4{ Keepers: passwordDataV1.Keepers, - Length: passwordDataV1.Length, - Special: passwordDataV1.Special, - Upper: passwordDataV1.Upper, - Lower: passwordDataV1.Lower, - Number: passwordDataV1.Number, - Numeric: passwordDataV1.Number, - MinNumeric: passwordDataV1.MinNumeric, - MinUpper: passwordDataV1.MinUpper, - MinLower: passwordDataV1.MinLower, - MinSpecial: passwordDataV1.MinSpecial, + Length: length, + Special: special, + Upper: upper, + Lower: lower, + Number: number, + Numeric: number, + MinNumeric: minNumeric, + MinUpper: minUpper, + MinLower: minLower, + MinSpecial: minSpecial, OverrideSpecial: passwordDataV1.OverrideSpecial, BcryptHash: passwordDataV1.BcryptHash, Result: passwordDataV1.Result, ID: passwordDataV1.ID, } - diags := resp.State.Set(ctx, passwordDataV3) + diags := resp.State.Set(ctx, passwordDataV4) resp.Diagnostics.Append(diags...) } -func upgradePasswordStateV2toV3(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { +func upgradePasswordStateV2toV4(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { + type passwordModelV2 struct { + ID types.String `tfsdk:"id"` + Keepers types.Map `tfsdk:"keepers"` + Length types.Int64 `tfsdk:"length"` + Special types.Bool `tfsdk:"special"` + Upper types.Bool `tfsdk:"upper"` + Lower types.Bool `tfsdk:"lower"` + Number types.Bool `tfsdk:"number"` + Numeric types.Bool `tfsdk:"numeric"` + MinNumeric types.Int64 `tfsdk:"min_numeric"` + MinUpper types.Int64 `tfsdk:"min_upper"` + MinLower types.Int64 `tfsdk:"min_lower"` + MinSpecial types.Int64 `tfsdk:"min_special"` + OverrideSpecial types.String `tfsdk:"override_special"` + Result types.String `tfsdk:"result"` + BcryptHash types.String `tfsdk:"bcrypt_hash"` + } + var passwordDataV2 passwordModelV2 resp.Diagnostics.Append(req.State.Get(ctx, &passwordDataV2)...) @@ -267,30 +416,100 @@ func upgradePasswordStateV2toV3(ctx context.Context, req resource.UpgradeStateRe return } - // Schema version 2 to schema version 3 is a duplicate of the data, + // Setting fields that can contain null to non-null to prevent forced replacement. + // This can occur in cases where import has been used in provider versions v3.3.1 and earlier. + // If import has been used with v3.3.1, for instance then length, lower, number, special, upper, + // min_lower, min_numeric, min_special and min_upper attributes will all be null in state. + length := passwordDataV2.Length + + if length.IsNull() { + length.Null = false + length.Value = int64(len(passwordDataV2.Result.Value)) + } + + minNumeric := passwordDataV2.MinNumeric + + if minNumeric.IsNull() { + minNumeric.Null = false + } + + minUpper := passwordDataV2.MinUpper + + if minUpper.IsNull() { + minUpper.Null = false + } + + minLower := passwordDataV2.MinLower + + if minLower.IsNull() { + minLower.Null = false + } + + minSpecial := passwordDataV2.MinSpecial + + if minSpecial.IsNull() { + minSpecial.Null = false + } + + special := passwordDataV2.Special + + if special.IsNull() { + special.Null = false + special.Value = true + } + + upper := passwordDataV2.Upper + + if upper.IsNull() { + upper.Null = false + upper.Value = true + } + + lower := passwordDataV2.Lower + + if lower.IsNull() { + lower.Null = false + lower.Value = true + } + + number := passwordDataV2.Number + + if number.IsNull() { + number.Null = false + number.Value = true + } + + numeric := passwordDataV2.Number + + if numeric.IsNull() { + numeric.Null = false + numeric.Value = true + } + + // Schema version 2 to schema version 4 is a duplicate of the data, // however the BcryptHash value may have been incorrectly generated. - //nolint:gosimple // V3 model will expand over time so all fields are written out to help future code changes. - passwordDataV3 := passwordModelV3{ + //nolint:gosimple // V4 model will expand over time so all fields are written out to help future code changes. + passwordDataV4 := passwordModelV4{ BcryptHash: passwordDataV2.BcryptHash, ID: passwordDataV2.ID, Keepers: passwordDataV2.Keepers, - Length: passwordDataV2.Length, - Lower: passwordDataV2.Lower, - MinLower: passwordDataV2.MinLower, - MinNumeric: passwordDataV2.MinNumeric, - MinSpecial: passwordDataV2.MinSpecial, - MinUpper: passwordDataV2.MinUpper, - Number: passwordDataV2.Number, - Numeric: passwordDataV2.Numeric, + Length: length, + Lower: lower, + MinLower: minLower, + MinNumeric: minNumeric, + MinSpecial: minSpecial, + MinUpper: minUpper, + Number: number, + Numeric: numeric, OverrideSpecial: passwordDataV2.OverrideSpecial, Result: passwordDataV2.Result, - Special: passwordDataV2.Special, - Upper: passwordDataV2.Upper, + Special: special, + Upper: upper, } // Set the duplicated data now so we can easily return early below. // The BcryptHash value will be adjusted later if it is incorrect. - resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV3)...) + resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV4)...) if resp.Diagnostics.HasError() { return @@ -328,9 +547,131 @@ func upgradePasswordStateV2toV3(ctx context.Context, req resource.UpgradeStateRe return } - passwordDataV3.BcryptHash.Value = string(newBcryptHash) + passwordDataV4.BcryptHash.Value = string(newBcryptHash) + + resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV4)...) +} + +func upgradePasswordStateV3toV4(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { + type passwordModelV3 struct { + ID types.String `tfsdk:"id"` + Keepers types.Map `tfsdk:"keepers"` + Length types.Int64 `tfsdk:"length"` + Special types.Bool `tfsdk:"special"` + Upper types.Bool `tfsdk:"upper"` + Lower types.Bool `tfsdk:"lower"` + Number types.Bool `tfsdk:"number"` + Numeric types.Bool `tfsdk:"numeric"` + MinNumeric types.Int64 `tfsdk:"min_numeric"` + MinUpper types.Int64 `tfsdk:"min_upper"` + MinLower types.Int64 `tfsdk:"min_lower"` + MinSpecial types.Int64 `tfsdk:"min_special"` + OverrideSpecial types.String `tfsdk:"override_special"` + Result types.String `tfsdk:"result"` + BcryptHash types.String `tfsdk:"bcrypt_hash"` + } + + var passwordDataV3 passwordModelV3 + + resp.Diagnostics.Append(req.State.Get(ctx, &passwordDataV3)...) + + if resp.Diagnostics.HasError() { + return + } + + // Setting fields that can contain null to non-null to prevent forced replacement. + // This can occur in cases where import has been used in provider versions v3.3.1 and earlier. + // If import has been used with v3.3.1, for instance then length, lower, number, special, upper, + // min_lower, min_numeric, min_special and min_upper attributes will all be null in state. + length := passwordDataV3.Length + + if length.IsNull() { + length.Null = false + length.Value = int64(len(passwordDataV3.Result.Value)) + } + + minNumeric := passwordDataV3.MinNumeric + + if minNumeric.IsNull() { + minNumeric.Null = false + } + + minUpper := passwordDataV3.MinUpper + + if minUpper.IsNull() { + minUpper.Null = false + } + + minLower := passwordDataV3.MinLower - resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV3)...) + if minLower.IsNull() { + minLower.Null = false + } + + minSpecial := passwordDataV3.MinSpecial + + if minSpecial.IsNull() { + minSpecial.Null = false + } + + special := passwordDataV3.Special + + if special.IsNull() { + special.Null = false + special.Value = true + } + + upper := passwordDataV3.Upper + + if upper.IsNull() { + upper.Null = false + upper.Value = true + } + + lower := passwordDataV3.Lower + + if lower.IsNull() { + lower.Null = false + lower.Value = true + } + + number := passwordDataV3.Number + + if number.IsNull() { + number.Null = false + number.Value = true + } + + numeric := passwordDataV3.Number + + if numeric.IsNull() { + numeric.Null = false + numeric.Value = true + } + + // Schema version 3 to schema version 4 is a duplicate of the data, + // however the . + + //nolint:gosimple // V4 model will expand over time so all fields are written out to help future code changes. + passwordDataV4 := passwordModelV4{ + BcryptHash: passwordDataV3.BcryptHash, + ID: passwordDataV3.ID, + Keepers: passwordDataV3.Keepers, + Length: length, + Lower: lower, + MinLower: minLower, + MinNumeric: minNumeric, + MinSpecial: minSpecial, + MinUpper: minUpper, + Number: number, + Numeric: numeric, + OverrideSpecial: passwordDataV3.OverrideSpecial, + Result: passwordDataV3.Result, + Special: special, + Upper: upper, + } + + resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV4)...) } func generateHash(toHash string) (string, error) { @@ -339,6 +680,194 @@ func generateHash(toHash string) (string, error) { return string(hash), err } +func passwordSchemaV4() tfsdk.Schema { + return tfsdk.Schema{ + Version: 4, + 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 " + + "data handling in the " + + "[Terraform documentation](https://www.terraform.io/docs/language/state/sensitive-data.html).\n\n" + + "This resource *does* use a cryptographic random number generator.", + Attributes: map[string]tfsdk.Attribute{ + "keepers": { + Description: "Arbitrary map of values that, when changed, will trigger recreation of " + + "resource. See [the main provider documentation](../index.html) for more information.", + Type: types.MapType{ + ElemType: types.StringType, + }, + Optional: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.RequiresReplaceIfValuesNotNull(), + }, + }, + + "length": { + Description: "The length of the string desired. The minimum value for length is 1 and, length " + + "must also be >= (`min_upper` + `min_lower` + `min_numeric` + `min_special`).", + Type: types.Int64Type, + Required: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.RequiresReplace(), + }, + Validators: []tfsdk.AttributeValidator{ + int64validator.AtLeast(1), + int64validator.AtLeastSumOf( + path.MatchRoot("min_upper"), + path.MatchRoot("min_lower"), + path.MatchRoot("min_numeric"), + path.MatchRoot("min_special"), + ), + }, + }, + + "special": { + Description: "Include special characters in the result. These are `!@#$%&*()-_=+[]{}<>:?`. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Bool{Value: true}), + planmodifiers.RequiresReplace(), + }, + }, + + "upper": { + Description: "Include uppercase alphabet characters in the result. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Bool{Value: true}), + planmodifiers.RequiresReplace(), + }, + }, + + "lower": { + Description: "Include lowercase alphabet characters in the result. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Bool{Value: true}), + planmodifiers.RequiresReplace(), + }, + }, + + "number": { + Description: "Include numeric characters in the result. Default value is `true`. " + + "**NOTE**: This is deprecated, use `numeric` instead.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.NumberNumericAttributePlanModifier(), + planmodifiers.RequiresReplace(), + }, + DeprecationMessage: "**NOTE**: This is deprecated, use `numeric` instead.", + }, + + "numeric": { + Description: "Include numeric characters in the result. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.NumberNumericAttributePlanModifier(), + planmodifiers.RequiresReplace(), + }, + }, + + "min_numeric": { + Description: "Minimum number of numeric characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, + + "min_upper": { + Description: "Minimum number of uppercase alphabet characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, + + "min_lower": { + Description: "Minimum number of lowercase alphabet characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, + + "min_special": { + Description: "Minimum number of special characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, + + "override_special": { + Description: "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.", + Type: types.StringType, + Optional: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.RequiresReplaceIf( + planmodifiers.RequiresReplaceUnlessEmptyStringToNull(), + "Replace on modification unless updating from empty string (\"\") to null.", + "Replace on modification unless updating from empty string (`\"\"`) to `null`.", + ), + }, + }, + + "result": { + Description: "The generated random string.", + Type: types.StringType, + Computed: true, + Sensitive: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.UseStateForUnknown(), + }, + }, + + "bcrypt_hash": { + Description: "A bcrypt hash of the generated random string.", + Type: types.StringType, + Computed: true, + Sensitive: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.UseStateForUnknown(), + }, + }, + + "id": { + Description: "A static value used internally by Terraform, this should not be referenced in configurations.", + Computed: true, + Type: types.StringType, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.UseStateForUnknown(), + }, + }, + }, + } +} + func passwordSchemaV3() tfsdk.Schema { return tfsdk.Schema{ Version: 3, @@ -1025,25 +1554,7 @@ func passwordSchemaV0() tfsdk.Schema { } } -type passwordModelV3 struct { - ID types.String `tfsdk:"id"` - Keepers types.Map `tfsdk:"keepers"` - Length types.Int64 `tfsdk:"length"` - Special types.Bool `tfsdk:"special"` - Upper types.Bool `tfsdk:"upper"` - Lower types.Bool `tfsdk:"lower"` - Number types.Bool `tfsdk:"number"` - Numeric types.Bool `tfsdk:"numeric"` - MinNumeric types.Int64 `tfsdk:"min_numeric"` - MinUpper types.Int64 `tfsdk:"min_upper"` - MinLower types.Int64 `tfsdk:"min_lower"` - MinSpecial types.Int64 `tfsdk:"min_special"` - OverrideSpecial types.String `tfsdk:"override_special"` - Result types.String `tfsdk:"result"` - BcryptHash types.String `tfsdk:"bcrypt_hash"` -} - -type passwordModelV2 struct { +type passwordModelV4 struct { ID types.String `tfsdk:"id"` Keepers types.Map `tfsdk:"keepers"` Length types.Int64 `tfsdk:"length"` diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go index 90803530..aaf8f258 100644 --- a/internal/provider/resource_password_test.go +++ b/internal/provider/resource_password_test.go @@ -15,8 +15,9 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" - "github.com/terraform-providers/terraform-provider-random/internal/random" "golang.org/x/crypto/bcrypt" + + "github.com/terraform-providers/terraform-provider-random/internal/random" ) func TestGenerateHash(t *testing.T) { @@ -279,6 +280,201 @@ func TestAccResourcePassword_OverrideSpecial_FromVersion3_4_2(t *testing.T) { }) } +// TestAccResourcePassword_Import_FromVersion3_1_3 verifies behaviour when resource has been imported and stores +// null for length, lower, number, special, upper, min_lower, min_numeric, min_special, min_upper attributes in state. +// v3.1.3 was selected as this is the last provider version using schema version 0. +func TestAccResourcePassword_Import_FromVersion3_1_3(t *testing.T) { + var result1, result2 string + + resource.ParallelTest(t, resource.TestCase{ + Steps: []resource.TestStep{ + { + ExternalProviders: providerVersion313(), + Config: `resource "random_password" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + testExtractResourceAttr("random_password.test", "result", &result1), + ), + }, + { + ExternalProviders: providerVersion313(), + ResourceName: "random_password.test", + // Usage of ImportStateIdFunc is required as the value passed to the `terraform import` command needs + // to be the password itself, as the password resource sets ID to "none" and "result" to the password + // supplied during import. + ImportStateIdFunc: func(s *terraform.State) (string, error) { + id := "random_password.test" + rs, ok := s.RootModule().Resources[id] + if !ok { + return "", fmt.Errorf("not found: %s", id) + } + if rs.Primary.ID == "" { + return "", fmt.Errorf("no ID is set") + } + + return rs.Primary.Attributes["result"], nil + }, + ImportState: true, + // These checks should fail as running terraform import with v3.1.3 stores null for result and number + // attributes in state. + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + resource.TestCheckResourceAttr("random_password.test", "number", "true"), + ), + }, + { + // This test is not really verifying desired behaviour as the import is populating length, number etc + ProtoV5ProviderFactories: protoV5ProviderFactories(), + Config: `resource "random_password" "test" { + length = 12 + }`, + PlanOnly: true, + }, + { + // This test is not really verifying desired behaviour as the import is populating length, number etc + ProtoV5ProviderFactories: protoV5ProviderFactories(), + Config: `resource "random_password" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + testExtractResourceAttr("random_password.test", "result", &result2), + testCheckAttributeValuesEqual(&result1, &result2), + ), + }, + }, + }) +} + +// TestAccResourcePassword_Import_FromVersion3_2_0 verifies behaviour when resource has been imported and stores +// null for length, lower, number, special, upper, min_lower, min_numeric, min_special, min_upper attributes in state. +// v3.2.0 was selected as this is the last provider version using schema version 1. +func TestAccResourcePassword_Import_FromVersion3_2_0(t *testing.T) { + var result1, result2 string + + resource.ParallelTest(t, resource.TestCase{ + Steps: []resource.TestStep{ + { + ExternalProviders: providerVersion320(), + Config: `resource "random_password" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + testExtractResourceAttr("random_password.test", "result", &result1), + ), + }, + { + ExternalProviders: providerVersion320(), + ResourceName: "random_password.test", + // Usage of ImportStateIdFunc is required as the value passed to the `terraform import` command needs + // to be the password itself, as the password resource sets ID to "none" and "result" to the password + // supplied during import. + ImportStateIdFunc: func(s *terraform.State) (string, error) { + id := "random_password.test" + rs, ok := s.RootModule().Resources[id] + if !ok { + return "", fmt.Errorf("not found: %s", id) + } + if rs.Primary.ID == "" { + return "", fmt.Errorf("no ID is set") + } + + return rs.Primary.Attributes["result"], nil + }, + ImportState: true, + // These checks should fail as running terraform import with v3.2.0 stores null for result and number + // attributes in state. + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + resource.TestCheckResourceAttr("random_password.test", "number", "true"), + ), + }, + { + // This test is not really verifying desired behaviour as the import is populating length, number etc + ProtoV5ProviderFactories: protoV5ProviderFactories(), + Config: `resource "random_password" "test" { + length = 12 + }`, + PlanOnly: true, + }, + { + // This test is not really verifying desired behaviour as the import is populating length, number etc + ProtoV5ProviderFactories: protoV5ProviderFactories(), + Config: `resource "random_password" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + testExtractResourceAttr("random_password.test", "result", &result2), + testCheckAttributeValuesEqual(&result1, &result2), + ), + }, + }, + }) +} + +// TestAccResourcePassword_Import_FromVersion3_2_0 verifies behaviour when resource has been imported and stores +// empty map {} for keepers and empty string for override_special in state. +// v3.4.2 was selected as this is the last provider version using schema version 2. +func TestAccResourcePassword_Import_FromVersion3_4_2(t *testing.T) { + var result1, result2 string + + resource.ParallelTest(t, resource.TestCase{ + Steps: []resource.TestStep{ + { + ExternalProviders: providerVersion342(), + Config: `resource "random_password" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + testExtractResourceAttr("random_password.test", "result", &result1), + ), + }, + { + ExternalProviders: providerVersion342(), + ResourceName: "random_password.test", + // Usage of ImportStateIdFunc is required as the value passed to the `terraform import` command needs + // to be the password itself, as the password resource sets ID to "none" and "result" to the password + // supplied during import. + ImportStateIdFunc: func(s *terraform.State) (string, error) { + id := "random_password.test" + rs, ok := s.RootModule().Resources[id] + if !ok { + return "", fmt.Errorf("not found: %s", id) + } + if rs.Primary.ID == "" { + return "", fmt.Errorf("no ID is set") + } + + return rs.Primary.Attributes["result"], nil + }, + ImportState: true, + // These checks should fail as running terraform import with v3.2.0 stores null for result and number + // attributes in state. + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + resource.TestCheckResourceAttr("random_password.test", "override_special", ""), + ), + }, + { + ProtoV5ProviderFactories: protoV5ProviderFactories(), + Config: `resource "random_password" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + testExtractResourceAttr("random_password.test", "result", &result2), + testCheckAttributeValuesEqual(&result1, &result2), + ), + }, + }, + }) +} + // TestAccResourcePassword_StateUpgradeV0toV2 covers the state upgrades from V0 to V2. // This includes the the addition of `numeric` and `bcrypt_hash` attributes. // v3.1.3 is used as this is last version before `bcrypt_hash` attributed was added. @@ -941,7 +1137,9 @@ func TestAccResourcePassword_UpgradeFromVersion3_3_2(t *testing.T) { }) } -func TestUpgradePasswordStateV0toV3(t *testing.T) { +func TestUpgradePasswordStateV0toV4(t *testing.T) { + t.Parallel() + raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ "id": tftypes.NewValue(tftypes.String, "none"), "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), @@ -967,13 +1165,13 @@ func TestUpgradePasswordStateV0toV3(t *testing.T) { resp := &res.UpgradeStateResponse{ State: tfsdk.State{ - Schema: passwordSchemaV3(), + Schema: passwordSchemaV4(), }, } - upgradePasswordStateV0toV3(context.Background(), req, resp) + upgradePasswordStateV0toV4(context.Background(), req, resp) - expected := passwordModelV3{ + expected := passwordModelV4{ ID: types.String{Value: "none"}, Keepers: types.Map{Null: true, ElemType: types.StringType}, Length: types.Int64{Value: 16}, @@ -990,7 +1188,7 @@ func TestUpgradePasswordStateV0toV3(t *testing.T) { Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, } - actual := passwordModelV3{} + actual := passwordModelV4{} diags := resp.State.Get(context.Background(), &actual) if diags.HasError() { t.Errorf("error getting state: %v", diags) @@ -1009,7 +1207,79 @@ func TestUpgradePasswordStateV0toV3(t *testing.T) { } } -func TestUpgradePasswordStateV1toV3(t *testing.T) { +func TestUpgradePasswordStateV0toV4_NullValues(t *testing.T) { + t.Parallel() + + raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, nil), + "lower": tftypes.NewValue(tftypes.Bool, nil), + "min_lower": tftypes.NewValue(tftypes.Number, nil), + "min_numeric": tftypes.NewValue(tftypes.Number, nil), + "min_special": tftypes.NewValue(tftypes.Number, nil), + "min_upper": tftypes.NewValue(tftypes.Number, nil), + "number": tftypes.NewValue(tftypes.Bool, nil), + "override_special": tftypes.NewValue(tftypes.String, nil), + "result": tftypes.NewValue(tftypes.String, "DZy_3*tnonj%Q%Yx"), + "special": tftypes.NewValue(tftypes.Bool, nil), + "upper": tftypes.NewValue(tftypes.Bool, nil), + }) + + req := res.UpgradeStateRequest{ + State: &tfsdk.State{ + Raw: raw, + Schema: passwordSchemaV0(), + }, + } + + resp := &res.UpgradeStateResponse{ + State: tfsdk.State{ + Schema: passwordSchemaV4(), + }, + } + + upgradePasswordStateV0toV4(context.Background(), req, resp) + + expected := passwordModelV4{ + ID: types.String{Value: "none"}, + Keepers: types.Map{Null: true, ElemType: types.StringType}, + Length: types.Int64{Value: 16}, + Special: types.Bool{Value: true}, + Upper: types.Bool{Value: true}, + Lower: types.Bool{Value: true}, + Number: types.Bool{Value: true}, + Numeric: types.Bool{Value: true}, + MinNumeric: types.Int64{Value: 0}, + MinUpper: types.Int64{Value: 0}, + MinLower: types.Int64{Value: 0}, + MinSpecial: types.Int64{Value: 0}, + OverrideSpecial: types.String{Null: true}, + Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, + } + + actual := passwordModelV4{} + diags := resp.State.Get(context.Background(), &actual) + if diags.HasError() { + t.Errorf("error getting state: %v", diags) + } + + err := bcrypt.CompareHashAndPassword([]byte(actual.BcryptHash.Value), []byte(actual.Result.Value)) + if err != nil { + t.Errorf("unexpected bcrypt comparison error: %s", err) + } + + // Setting actual.BcryptHash to zero value to allow direct comparison of expected and actual. + actual.BcryptHash = types.String{} + + if !cmp.Equal(expected, actual) { + t.Errorf("expected: %+v, got: %+v", expected, actual) + } +} + +func TestUpgradePasswordStateV1toV4(t *testing.T) { + t.Parallel() + raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ "id": tftypes.NewValue(tftypes.String, "none"), "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), @@ -1036,13 +1306,13 @@ func TestUpgradePasswordStateV1toV3(t *testing.T) { resp := &res.UpgradeStateResponse{ State: tfsdk.State{ - Schema: passwordSchemaV3(), + Schema: passwordSchemaV4(), }, } - upgradePasswordStateV1toV3(context.Background(), req, resp) + upgradePasswordStateV1toV4(context.Background(), req, resp) - expected := passwordModelV3{ + expected := passwordModelV4{ ID: types.String{Value: "none"}, Keepers: types.Map{Null: true, ElemType: types.StringType}, Length: types.Int64{Value: 16}, @@ -1060,7 +1330,71 @@ func TestUpgradePasswordStateV1toV3(t *testing.T) { Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, } - actual := passwordModelV3{} + actual := passwordModelV4{} + diags := resp.State.Get(context.Background(), &actual) + if diags.HasError() { + t.Errorf("error getting state: %v", diags) + } + + if !cmp.Equal(expected, actual) { + t.Errorf("expected: %+v, got: %+v", expected, actual) + } +} + +func TestUpgradePasswordStateV1toV4_NullValues(t *testing.T) { + t.Parallel() + + raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, nil), + "lower": tftypes.NewValue(tftypes.Bool, nil), + "min_lower": tftypes.NewValue(tftypes.Number, nil), + "min_numeric": tftypes.NewValue(tftypes.Number, nil), + "min_special": tftypes.NewValue(tftypes.Number, nil), + "min_upper": tftypes.NewValue(tftypes.Number, nil), + "number": tftypes.NewValue(tftypes.Bool, nil), + "override_special": tftypes.NewValue(tftypes.String, nil), + "result": tftypes.NewValue(tftypes.String, "DZy_3*tnonj%Q%Yx"), + "special": tftypes.NewValue(tftypes.Bool, nil), + "upper": tftypes.NewValue(tftypes.Bool, nil), + "bcrypt_hash": tftypes.NewValue(tftypes.String, "bcrypt_hash"), + }) + + req := res.UpgradeStateRequest{ + State: &tfsdk.State{ + Raw: raw, + Schema: passwordSchemaV1(), + }, + } + + resp := &res.UpgradeStateResponse{ + State: tfsdk.State{ + Schema: passwordSchemaV4(), + }, + } + + upgradePasswordStateV1toV4(context.Background(), req, resp) + + expected := passwordModelV4{ + ID: types.String{Value: "none"}, + Keepers: types.Map{Null: true, ElemType: types.StringType}, + Length: types.Int64{Value: 16}, + Special: types.Bool{Value: true}, + Upper: types.Bool{Value: true}, + Lower: types.Bool{Value: true}, + Number: types.Bool{Value: true}, + Numeric: types.Bool{Value: true}, + MinNumeric: types.Int64{Value: 0}, + MinUpper: types.Int64{Value: 0}, + MinLower: types.Int64{Value: 0}, + MinSpecial: types.Int64{Value: 0}, + OverrideSpecial: types.String{Null: true}, + BcryptHash: types.String{Value: "bcrypt_hash"}, + Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, + } + + actual := passwordModelV4{} diags := resp.State.Get(context.Background(), &actual) if diags.HasError() { t.Errorf("error getting state: %v", diags) @@ -1071,7 +1405,7 @@ func TestUpgradePasswordStateV1toV3(t *testing.T) { } } -func TestUpgradePasswordStateV2toV3(t *testing.T) { +func TestUpgradePasswordStateV2toV4(t *testing.T) { t.Parallel() testCases := map[string]struct { @@ -1158,7 +1492,7 @@ func TestUpgradePasswordStateV2toV3(t *testing.T) { "special": tftypes.NewValue(tftypes.Bool, true), "upper": tftypes.NewValue(tftypes.Bool, true), }), - Schema: passwordSchemaV3(), + Schema: passwordSchemaV4(), }, }, }, @@ -1242,7 +1576,91 @@ func TestUpgradePasswordStateV2toV3(t *testing.T) { "special": tftypes.NewValue(tftypes.Bool, true), "upper": tftypes.NewValue(tftypes.Bool, true), }), - Schema: passwordSchemaV3(), + Schema: passwordSchemaV4(), + }, + }, + }, + "valid-hash-null-values": { + request: res.UpgradeStateRequest{ + State: &tfsdk.State{ + Raw: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "bcrypt_hash": tftypes.String, + "id": tftypes.String, + "keepers": tftypes.Map{ElementType: tftypes.String}, + "length": tftypes.Number, + "lower": tftypes.Bool, + "min_lower": tftypes.Number, + "min_numeric": tftypes.Number, + "min_special": tftypes.Number, + "min_upper": tftypes.Number, + "number": tftypes.Bool, + "numeric": tftypes.Bool, + "override_special": tftypes.String, + "result": tftypes.String, + "special": tftypes.Bool, + "upper": tftypes.Bool, + }, + }, map[string]tftypes.Value{ + "bcrypt_hash": tftypes.NewValue(tftypes.String, "$2a$10$d9zhEkVg.O1jZ6fEIMRlRuu/vMa0/4UIzeK5joaTBhZJlYiIPhWWa"), + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, nil), + "lower": tftypes.NewValue(tftypes.Bool, nil), + "min_lower": tftypes.NewValue(tftypes.Number, nil), + "min_numeric": tftypes.NewValue(tftypes.Number, nil), + "min_special": tftypes.NewValue(tftypes.Number, nil), + "min_upper": tftypes.NewValue(tftypes.Number, nil), + "number": tftypes.NewValue(tftypes.Bool, nil), + "numeric": tftypes.NewValue(tftypes.Bool, nil), + "override_special": tftypes.NewValue(tftypes.String, ""), + "result": tftypes.NewValue(tftypes.String, "n:um[a9kO&x!L=9og[EM"), + "special": tftypes.NewValue(tftypes.Bool, nil), + "upper": tftypes.NewValue(tftypes.Bool, nil), + }), + Schema: passwordSchemaV2(), + }, + }, + expected: &res.UpgradeStateResponse{ + State: tfsdk.State{ + Raw: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "bcrypt_hash": tftypes.String, + "id": tftypes.String, + "keepers": tftypes.Map{ElementType: tftypes.String}, + "length": tftypes.Number, + "lower": tftypes.Bool, + "min_lower": tftypes.Number, + "min_numeric": tftypes.Number, + "min_special": tftypes.Number, + "min_upper": tftypes.Number, + "number": tftypes.Bool, + "numeric": tftypes.Bool, + "override_special": tftypes.String, + "result": tftypes.String, + "special": tftypes.Bool, + "upper": tftypes.Bool, + }, + }, map[string]tftypes.Value{ + // The difference checking should compare this actual + // value since it should not be updated. + "bcrypt_hash": tftypes.NewValue(tftypes.String, "$2a$10$d9zhEkVg.O1jZ6fEIMRlRuu/vMa0/4UIzeK5joaTBhZJlYiIPhWWa"), + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, 20), + "lower": tftypes.NewValue(tftypes.Bool, true), + "min_lower": tftypes.NewValue(tftypes.Number, 0), + "min_numeric": tftypes.NewValue(tftypes.Number, 0), + "min_special": tftypes.NewValue(tftypes.Number, 0), + "min_upper": tftypes.NewValue(tftypes.Number, 0), + "number": tftypes.NewValue(tftypes.Bool, true), + "numeric": tftypes.NewValue(tftypes.Bool, true), + "override_special": tftypes.NewValue(tftypes.String, ""), + "result": tftypes.NewValue(tftypes.String, "n:um[a9kO&x!L=9og[EM"), + "special": tftypes.NewValue(tftypes.Bool, true), + "upper": tftypes.NewValue(tftypes.Bool, true), + }), + Schema: passwordSchemaV4(), }, }, }, @@ -1260,7 +1678,7 @@ func TestUpgradePasswordStateV2toV3(t *testing.T) { }, } - upgradePasswordStateV2toV3(context.Background(), testCase.request, &got) + upgradePasswordStateV2toV4(context.Background(), testCase.request, &got) // Since bcrypt_hash is generated, this test is very involved to // ensure the test case is set up properly and the generated @@ -1381,6 +1799,136 @@ func TestUpgradePasswordStateV2toV3(t *testing.T) { } } +func TestUpgradePasswordStateV3toV4(t *testing.T) { + t.Parallel() + + raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, 16), + "lower": tftypes.NewValue(tftypes.Bool, true), + "min_lower": tftypes.NewValue(tftypes.Number, 0), + "min_numeric": tftypes.NewValue(tftypes.Number, 0), + "min_special": tftypes.NewValue(tftypes.Number, 0), + "min_upper": tftypes.NewValue(tftypes.Number, 0), + "number": tftypes.NewValue(tftypes.Bool, true), + "numeric": tftypes.NewValue(tftypes.Bool, true), + "override_special": tftypes.NewValue(tftypes.String, "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"), + "result": tftypes.NewValue(tftypes.String, "DZy_3*tnonj%Q%Yx"), + "special": tftypes.NewValue(tftypes.Bool, true), + "upper": tftypes.NewValue(tftypes.Bool, true), + "bcrypt_hash": tftypes.NewValue(tftypes.String, "bcrypt_hash"), + }) + + req := res.UpgradeStateRequest{ + State: &tfsdk.State{ + Raw: raw, + Schema: passwordSchemaV3(), + }, + } + + resp := &res.UpgradeStateResponse{ + State: tfsdk.State{ + Schema: passwordSchemaV4(), + }, + } + + upgradePasswordStateV3toV4(context.Background(), req, resp) + + expected := passwordModelV4{ + ID: types.String{Value: "none"}, + Keepers: types.Map{Null: true, ElemType: types.StringType}, + Length: types.Int64{Value: 16}, + Special: types.Bool{Value: true}, + Upper: types.Bool{Value: true}, + Lower: types.Bool{Value: true}, + Number: types.Bool{Value: true}, + Numeric: types.Bool{Value: true}, + MinNumeric: types.Int64{Value: 0}, + MinUpper: types.Int64{Value: 0}, + MinLower: types.Int64{Value: 0}, + MinSpecial: types.Int64{Value: 0}, + OverrideSpecial: types.String{Value: "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"}, + BcryptHash: types.String{Value: "bcrypt_hash"}, + Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, + } + + actual := passwordModelV4{} + diags := resp.State.Get(context.Background(), &actual) + if diags.HasError() { + t.Errorf("error getting state: %v", diags) + } + + if !cmp.Equal(expected, actual) { + t.Errorf("expected: %+v, got: %+v", expected, actual) + } +} + +func TestUpgradePasswordStateV3toV4_NullValues(t *testing.T) { + t.Parallel() + + raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, nil), + "lower": tftypes.NewValue(tftypes.Bool, true), + "min_lower": tftypes.NewValue(tftypes.Number, nil), + "min_numeric": tftypes.NewValue(tftypes.Number, nil), + "min_special": tftypes.NewValue(tftypes.Number, nil), + "min_upper": tftypes.NewValue(tftypes.Number, nil), + "number": tftypes.NewValue(tftypes.Bool, nil), + "numeric": tftypes.NewValue(tftypes.Bool, nil), + "override_special": tftypes.NewValue(tftypes.String, "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"), + "result": tftypes.NewValue(tftypes.String, "DZy_3*tnonj%Q%Yx"), + "special": tftypes.NewValue(tftypes.Bool, nil), + "upper": tftypes.NewValue(tftypes.Bool, nil), + "bcrypt_hash": tftypes.NewValue(tftypes.String, "bcrypt_hash"), + }) + + req := res.UpgradeStateRequest{ + State: &tfsdk.State{ + Raw: raw, + Schema: passwordSchemaV3(), + }, + } + + resp := &res.UpgradeStateResponse{ + State: tfsdk.State{ + Schema: passwordSchemaV4(), + }, + } + + upgradePasswordStateV3toV4(context.Background(), req, resp) + + expected := passwordModelV4{ + ID: types.String{Value: "none"}, + Keepers: types.Map{Null: true, ElemType: types.StringType}, + Length: types.Int64{Value: 16}, + Special: types.Bool{Value: true}, + Upper: types.Bool{Value: true}, + Lower: types.Bool{Value: true}, + Number: types.Bool{Value: true}, + Numeric: types.Bool{Value: true}, + MinNumeric: types.Int64{Value: 0}, + MinUpper: types.Int64{Value: 0}, + MinLower: types.Int64{Value: 0}, + MinSpecial: types.Int64{Value: 0}, + OverrideSpecial: types.String{Value: "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"}, + BcryptHash: types.String{Value: "bcrypt_hash"}, + Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, + } + + actual := passwordModelV4{} + diags := resp.State.Get(context.Background(), &actual) + if diags.HasError() { + t.Errorf("error getting state: %v", diags) + } + + if !cmp.Equal(expected, actual) { + t.Errorf("expected: %+v, got: %+v", expected, actual) + } +} + func TestAccResourcePassword_NumberNumericErrors(t *testing.T) { resource.UnitTest(t, resource.TestCase{ ProtoV5ProviderFactories: protoV5ProviderFactories(), From b189ab8c6113ef499b5eb29528c7731917bb0d4c Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Tue, 6 Sep 2022 16:58:30 +0100 Subject: [PATCH 3/9] Add state upgrader for string resource to handle null values that could be present in state following import with earlier versions of the provider. For instance, using v3.3.1 to import a random password resource will result in the state file containing null values for length, lower, number, special, upper, min_lower, min_numeric, min_special, min_upper attributes. --- internal/provider/resource_string.go | 634 +++++++++++++++++----- internal/provider/resource_string_test.go | 384 +++++++++++++ 2 files changed, 881 insertions(+), 137 deletions(-) diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go index 319730f3..19a7d2cf 100644 --- a/internal/provider/resource_string.go +++ b/internal/provider/resource_string.go @@ -21,6 +21,338 @@ var _ provider.ResourceType = (*stringResourceType)(nil) type stringResourceType struct{} func (r stringResourceType) GetSchema(context.Context) (tfsdk.Schema, diag.Diagnostics) { + return stringSchemaV3(), nil +} + +func (r stringResourceType) NewResource(_ context.Context, p provider.Provider) (resource.Resource, diag.Diagnostics) { + return &stringResource{}, nil +} + +var ( + _ resource.Resource = (*stringResource)(nil) + _ resource.ResourceWithImportState = (*stringResource)(nil) + _ resource.ResourceWithUpgradeState = (*stringResource)(nil) +) + +type stringResource struct{} + +func (r *stringResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + var plan stringModelV3 + + diags := req.Plan.Get(ctx, &plan) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + + params := random.StringParams{ + Length: plan.Length.Value, + Upper: plan.Upper.Value, + MinUpper: plan.MinUpper.Value, + Lower: plan.Lower.Value, + MinLower: plan.MinLower.Value, + Numeric: plan.Numeric.Value, + MinNumeric: plan.MinNumeric.Value, + Special: plan.Special.Value, + MinSpecial: plan.MinSpecial.Value, + OverrideSpecial: plan.OverrideSpecial.Value, + } + + result, err := random.CreateString(params) + if err != nil { + resp.Diagnostics.Append(diagnostics.RandomReadError(err.Error())...) + return + } + + plan.ID = types.String{Value: string(result)} + plan.Result = types.String{Value: string(result)} + + resp.Diagnostics.Append(resp.State.Set(ctx, plan)...) +} + +// Read does not need to perform any operations as the state in ReadResourceResponse is already populated. +func (r *stringResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { +} + +// Update ensures the plan value is copied to the state to complete the update. +func (r *stringResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + var model stringModelV3 + + resp.Diagnostics.Append(req.Plan.Get(ctx, &model)...) + + if resp.Diagnostics.HasError() { + return + } + + resp.Diagnostics.Append(resp.State.Set(ctx, &model)...) +} + +// Delete does not need to explicitly call resp.State.RemoveResource() as this is automatically handled by the +// [framework](https://github.com/hashicorp/terraform-plugin-framework/pull/301). +func (r *stringResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { +} + +func (r *stringResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { + id := req.ID + + state := stringModelV3{ + ID: types.String{Value: id}, + Result: types.String{Value: id}, + Length: types.Int64{Value: int64(len(id))}, + Special: types.Bool{Value: true}, + Upper: types.Bool{Value: true}, + Lower: types.Bool{Value: true}, + Number: types.Bool{Value: true}, + Numeric: types.Bool{Value: true}, + MinSpecial: types.Int64{Value: 0}, + MinUpper: types.Int64{Value: 0}, + MinLower: types.Int64{Value: 0}, + MinNumeric: types.Int64{Value: 0}, + } + + state.Keepers.ElemType = types.StringType + + diags := resp.State.Set(ctx, &state) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } +} + +func (r *stringResource) UpgradeState(context.Context) map[int64]resource.StateUpgrader { + schemaV1 := stringSchemaV1() + schemaV2 := stringSchemaV2() + + return map[int64]resource.StateUpgrader{ + 1: { + PriorSchema: &schemaV1, + StateUpgrader: upgradeStringStateV1toV3, + }, + 2: { + PriorSchema: &schemaV2, + StateUpgrader: upgradeStringStateV2toV3, + }, + } +} + +func upgradeStringStateV1toV3(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { + type modelV1 struct { + ID types.String `tfsdk:"id"` + Keepers types.Map `tfsdk:"keepers"` + Length types.Int64 `tfsdk:"length"` + Special types.Bool `tfsdk:"special"` + Upper types.Bool `tfsdk:"upper"` + Lower types.Bool `tfsdk:"lower"` + Number types.Bool `tfsdk:"number"` + MinNumeric types.Int64 `tfsdk:"min_numeric"` + MinUpper types.Int64 `tfsdk:"min_upper"` + MinLower types.Int64 `tfsdk:"min_lower"` + MinSpecial types.Int64 `tfsdk:"min_special"` + OverrideSpecial types.String `tfsdk:"override_special"` + Result types.String `tfsdk:"result"` + } + + var stringDataV1 modelV1 + + resp.Diagnostics.Append(req.State.Get(ctx, &stringDataV1)...) + if resp.Diagnostics.HasError() { + return + } + + // Setting fields that can contain null to non-null to prevent forced replacement. + // This can occur in cases where import has been used in provider versions v3.3.1 and earlier. + // If import has been used with v3.3.1, for instance then length, lower, number, special, upper, + // min_lower, min_numeric, min_special and min_upper attributes will all be null in state. + length := stringDataV1.Length + + if length.IsNull() { + length.Null = false + length.Value = int64(len(stringDataV1.Result.Value)) + } + + minNumeric := stringDataV1.MinNumeric + + if minNumeric.IsNull() { + minNumeric.Null = false + } + + minUpper := stringDataV1.MinUpper + + if minUpper.IsNull() { + minUpper.Null = false + } + + minLower := stringDataV1.MinLower + + if minLower.IsNull() { + minLower.Null = false + } + + minSpecial := stringDataV1.MinSpecial + + if minSpecial.IsNull() { + minSpecial.Null = false + } + + special := stringDataV1.Special + + if special.IsNull() { + special.Null = false + special.Value = true + } + + upper := stringDataV1.Upper + + if upper.IsNull() { + upper.Null = false + upper.Value = true + } + + lower := stringDataV1.Lower + + if lower.IsNull() { + lower.Null = false + lower.Value = true + } + + number := stringDataV1.Number + + if number.IsNull() { + number.Null = false + number.Value = true + } + + stringDataV3 := stringModelV3{ + Keepers: stringDataV1.Keepers, + Length: length, + Special: special, + Upper: upper, + Lower: lower, + Number: number, + Numeric: number, + MinNumeric: minNumeric, + MinUpper: minUpper, + MinLower: minLower, + MinSpecial: minSpecial, + OverrideSpecial: stringDataV1.OverrideSpecial, + Result: stringDataV1.Result, + ID: stringDataV1.ID, + } + + diags := resp.State.Set(ctx, stringDataV3) + resp.Diagnostics.Append(diags...) +} + +func upgradeStringStateV2toV3(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { + type modelV2 struct { + ID types.String `tfsdk:"id"` + Keepers types.Map `tfsdk:"keepers"` + Length types.Int64 `tfsdk:"length"` + Special types.Bool `tfsdk:"special"` + Upper types.Bool `tfsdk:"upper"` + Lower types.Bool `tfsdk:"lower"` + Number types.Bool `tfsdk:"number"` + Numeric types.Bool `tfsdk:"numeric"` + MinNumeric types.Int64 `tfsdk:"min_numeric"` + MinUpper types.Int64 `tfsdk:"min_upper"` + MinLower types.Int64 `tfsdk:"min_lower"` + MinSpecial types.Int64 `tfsdk:"min_special"` + OverrideSpecial types.String `tfsdk:"override_special"` + Result types.String `tfsdk:"result"` + } + + var stringDataV2 modelV2 + + resp.Diagnostics.Append(req.State.Get(ctx, &stringDataV2)...) + if resp.Diagnostics.HasError() { + return + } + + // Setting fields that can contain null to non-null to prevent forced replacement. + // This can occur in cases where import has been used in provider versions v3.3.1 and earlier. + // If import has been used with v3.3.1, for instance then length, lower, number, special, upper, + // min_lower, min_numeric, min_special and min_upper attributes will all be null in state. + length := stringDataV2.Length + + if length.IsNull() { + length.Null = false + length.Value = int64(len(stringDataV2.Result.Value)) + } + + minNumeric := stringDataV2.MinNumeric + + if minNumeric.IsNull() { + minNumeric.Null = false + } + + minUpper := stringDataV2.MinUpper + + if minUpper.IsNull() { + minUpper.Null = false + } + + minLower := stringDataV2.MinLower + + if minLower.IsNull() { + minLower.Null = false + } + + minSpecial := stringDataV2.MinSpecial + + if minSpecial.IsNull() { + minSpecial.Null = false + } + + special := stringDataV2.Special + + if special.IsNull() { + special.Null = false + special.Value = true + } + + upper := stringDataV2.Upper + + if upper.IsNull() { + upper.Null = false + upper.Value = true + } + + lower := stringDataV2.Lower + + if lower.IsNull() { + lower.Null = false + lower.Value = true + } + + number := stringDataV2.Number + + if number.IsNull() { + number.Null = false + number.Value = true + } + + stringDataV3 := stringModelV3{ + Keepers: stringDataV2.Keepers, + Length: length, + Special: special, + Upper: upper, + Lower: lower, + Number: number, + Numeric: number, + MinNumeric: minNumeric, + MinUpper: minUpper, + MinLower: minLower, + MinSpecial: minSpecial, + OverrideSpecial: stringDataV2.OverrideSpecial, + Result: stringDataV2.Result, + ID: stringDataV2.ID, + } + + diags := resp.State.Set(ctx, stringDataV3) + resp.Diagnostics.Append(diags...) +} + +func stringSchemaV3() tfsdk.Schema { return tfsdk.Schema{ Version: 2, Description: "The resource `random_string` generates a random permutation of alphanumeric " + @@ -195,109 +527,189 @@ func (r stringResourceType) GetSchema(context.Context) (tfsdk.Schema, diag.Diagn }, }, }, - }, nil -} - -func (r stringResourceType) NewResource(_ context.Context, p provider.Provider) (resource.Resource, diag.Diagnostics) { - return &stringResource{}, nil + } } -var ( - _ resource.Resource = (*stringResource)(nil) - _ resource.ResourceWithImportState = (*stringResource)(nil) - _ resource.ResourceWithUpgradeState = (*stringResource)(nil) -) - -type stringResource struct{} - -func (r *stringResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { - var plan stringModelV2 +func stringSchemaV2() tfsdk.Schema { + return tfsdk.Schema{ + Version: 2, + Description: "The resource `random_string` generates a random permutation of alphanumeric " + + "characters and optionally special characters.\n" + + "\n" + + "This resource *does* use a cryptographic random number generator.\n" + + "\n" + + "Historically this resource's intended usage has been ambiguous as the original example used " + + "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).", + Attributes: map[string]tfsdk.Attribute{ + "keepers": { + Description: "Arbitrary map of values that, when changed, will trigger recreation of " + + "resource. See [the main provider documentation](../index.html) for more information.", + Type: types.MapType{ + ElemType: types.StringType, + }, + Optional: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.RequiresReplaceIfValuesNotNull(), + }, + }, - diags := req.Plan.Get(ctx, &plan) - resp.Diagnostics.Append(diags...) - if resp.Diagnostics.HasError() { - return - } + "length": { + Description: "The length of the string desired. The minimum value for length is 1 and, length " + + "must also be >= (`min_upper` + `min_lower` + `min_numeric` + `min_special`).", + Type: types.Int64Type, + Required: true, + PlanModifiers: []tfsdk.AttributePlanModifier{resource.RequiresReplace()}, + Validators: []tfsdk.AttributeValidator{ + int64validator.AtLeast(1), + int64validator.AtLeastSumOf( + path.MatchRoot("min_upper"), + path.MatchRoot("min_lower"), + path.MatchRoot("min_numeric"), + path.MatchRoot("min_special"), + ), + }, + }, - params := random.StringParams{ - Length: plan.Length.Value, - Upper: plan.Upper.Value, - MinUpper: plan.MinUpper.Value, - Lower: plan.Lower.Value, - MinLower: plan.MinLower.Value, - Numeric: plan.Numeric.Value, - MinNumeric: plan.MinNumeric.Value, - Special: plan.Special.Value, - MinSpecial: plan.MinSpecial.Value, - OverrideSpecial: plan.OverrideSpecial.Value, - } + "special": { + Description: "Include special characters in the result. These are `!@#$%&*()-_=+[]{}<>:?`. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Bool{Value: true}), + planmodifiers.RequiresReplace(), + }, + }, - result, err := random.CreateString(params) - if err != nil { - resp.Diagnostics.Append(diagnostics.RandomReadError(err.Error())...) - return - } + "upper": { + Description: "Include uppercase alphabet characters in the result. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Bool{Value: true}), + planmodifiers.RequiresReplace(), + }, + }, - plan.ID = types.String{Value: string(result)} - plan.Result = types.String{Value: string(result)} + "lower": { + Description: "Include lowercase alphabet characters in the result. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Bool{Value: true}), + planmodifiers.RequiresReplace(), + }, + }, - resp.Diagnostics.Append(resp.State.Set(ctx, plan)...) -} + "number": { + Description: "Include numeric characters in the result. Default value is `true`. " + + "**NOTE**: This is deprecated, use `numeric` instead.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.NumberNumericAttributePlanModifier(), + planmodifiers.RequiresReplace(), + }, + DeprecationMessage: "**NOTE**: This is deprecated, use `numeric` instead.", + }, -// Read does not need to perform any operations as the state in ReadResourceResponse is already populated. -func (r *stringResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { -} + "numeric": { + Description: "Include numeric characters in the result. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.NumberNumericAttributePlanModifier(), + planmodifiers.RequiresReplace(), + }, + }, -// Update ensures the plan value is copied to the state to complete the update. -func (r *stringResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { - var model stringModelV2 + "min_numeric": { + Description: "Minimum number of numeric characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, - resp.Diagnostics.Append(req.Plan.Get(ctx, &model)...) + "min_upper": { + Description: "Minimum number of uppercase alphabet characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, - if resp.Diagnostics.HasError() { - return - } + "min_lower": { + Description: "Minimum number of lowercase alphabet characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, - resp.Diagnostics.Append(resp.State.Set(ctx, &model)...) -} + "min_special": { + Description: "Minimum number of special characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, -// Delete does not need to explicitly call resp.State.RemoveResource() as this is automatically handled by the -// [framework](https://github.com/hashicorp/terraform-plugin-framework/pull/301). -func (r *stringResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { -} + "override_special": { + Description: "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.", + Type: types.StringType, + Optional: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.RequiresReplaceIf( + planmodifiers.RequiresReplaceUnlessEmptyStringToNull(), + "Replace on modification unless updating from empty string (\"\") to null.", + "Replace on modification unless updating from empty string (`\"\"`) to `null`.", + ), + }, + }, -func (r *stringResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { - id := req.ID + "result": { + Description: "The generated random string.", + Type: types.StringType, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.UseStateForUnknown(), + }, + }, - state := stringModelV2{ - ID: types.String{Value: id}, - Result: types.String{Value: id}, - Length: types.Int64{Value: int64(len(id))}, - Special: types.Bool{Value: true}, - Upper: types.Bool{Value: true}, - Lower: types.Bool{Value: true}, - Number: types.Bool{Value: true}, - Numeric: types.Bool{Value: true}, - MinSpecial: types.Int64{Value: 0}, - MinUpper: types.Int64{Value: 0}, - MinLower: types.Int64{Value: 0}, - MinNumeric: types.Int64{Value: 0}, - Keepers: types.Map{ - ElemType: types.StringType, - Null: true, + "id": { + Description: "The generated random string.", + Computed: true, + Type: types.StringType, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.UseStateForUnknown(), + }, + }, }, - OverrideSpecial: types.String{Null: true}, - } - - diags := resp.State.Set(ctx, &state) - resp.Diagnostics.Append(diags...) - if resp.Diagnostics.HasError() { - return } } -func (r *stringResource) UpgradeState(context.Context) map[int64]resource.StateUpgrader { - schemaV1 := tfsdk.Schema{ +func stringSchemaV1() tfsdk.Schema { + return tfsdk.Schema{ Version: 1, Description: "The resource `random_string` generates a random permutation of alphanumeric " + "characters and optionally special characters.\n" + @@ -450,61 +862,9 @@ func (r *stringResource) UpgradeState(context.Context) map[int64]resource.StateU }, }, } - - return map[int64]resource.StateUpgrader{ - 1: { - PriorSchema: &schemaV1, - StateUpgrader: upgradeStringStateV1toV2, - }, - } -} - -func upgradeStringStateV1toV2(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { - type modelV1 struct { - ID types.String `tfsdk:"id"` - Keepers types.Map `tfsdk:"keepers"` - Length types.Int64 `tfsdk:"length"` - Special types.Bool `tfsdk:"special"` - Upper types.Bool `tfsdk:"upper"` - Lower types.Bool `tfsdk:"lower"` - Number types.Bool `tfsdk:"number"` - MinNumeric types.Int64 `tfsdk:"min_numeric"` - MinUpper types.Int64 `tfsdk:"min_upper"` - MinLower types.Int64 `tfsdk:"min_lower"` - MinSpecial types.Int64 `tfsdk:"min_special"` - OverrideSpecial types.String `tfsdk:"override_special"` - Result types.String `tfsdk:"result"` - } - - var stringDataV1 modelV1 - - resp.Diagnostics.Append(req.State.Get(ctx, &stringDataV1)...) - if resp.Diagnostics.HasError() { - return - } - - stringDataV2 := stringModelV2{ - Keepers: stringDataV1.Keepers, - Length: stringDataV1.Length, - Special: stringDataV1.Special, - Upper: stringDataV1.Upper, - Lower: stringDataV1.Lower, - Number: stringDataV1.Number, - Numeric: stringDataV1.Number, - MinNumeric: stringDataV1.MinNumeric, - MinUpper: stringDataV1.MinUpper, - MinLower: stringDataV1.MinLower, - MinSpecial: stringDataV1.MinSpecial, - OverrideSpecial: stringDataV1.OverrideSpecial, - Result: stringDataV1.Result, - ID: stringDataV1.ID, - } - - diags := resp.State.Set(ctx, stringDataV2) - resp.Diagnostics.Append(diags...) } -type stringModelV2 struct { +type stringModelV3 struct { ID types.String `tfsdk:"id"` Keepers types.Map `tfsdk:"keepers"` Length types.Int64 `tfsdk:"length"` diff --git a/internal/provider/resource_string_test.go b/internal/provider/resource_string_test.go index 4d853c20..dff14aab 100644 --- a/internal/provider/resource_string_test.go +++ b/internal/provider/resource_string_test.go @@ -1,11 +1,18 @@ package provider import ( + "context" "fmt" "regexp" "testing" + "github.com/google/go-cmp/cmp" + res "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) func TestAccResourceString(t *testing.T) { @@ -1258,6 +1265,383 @@ func TestAccResourceString_UpgradeFromVersion3_3_2(t *testing.T) { }) } +func TestUpgradeStringStateV1toV3(t *testing.T) { + t.Parallel() + + raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, 16), + "lower": tftypes.NewValue(tftypes.Bool, true), + "min_lower": tftypes.NewValue(tftypes.Number, 0), + "min_numeric": tftypes.NewValue(tftypes.Number, 0), + "min_special": tftypes.NewValue(tftypes.Number, 0), + "min_upper": tftypes.NewValue(tftypes.Number, 0), + "number": tftypes.NewValue(tftypes.Bool, true), + "override_special": tftypes.NewValue(tftypes.String, "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"), + "result": tftypes.NewValue(tftypes.String, "DZy_3*tnonj%Q%Yx"), + "special": tftypes.NewValue(tftypes.Bool, true), + "upper": tftypes.NewValue(tftypes.Bool, true), + }) + + req := res.UpgradeStateRequest{ + State: &tfsdk.State{ + Raw: raw, + Schema: stringSchemaV1(), + }, + } + + resp := &res.UpgradeStateResponse{ + State: tfsdk.State{ + Schema: stringSchemaV3(), + }, + } + + upgradeStringStateV1toV3(context.Background(), req, resp) + + expected := stringModelV3{ + ID: types.String{Value: "none"}, + Keepers: types.Map{Null: true, ElemType: types.StringType}, + Length: types.Int64{Value: 16}, + Special: types.Bool{Value: true}, + Upper: types.Bool{Value: true}, + Lower: types.Bool{Value: true}, + Number: types.Bool{Value: true}, + Numeric: types.Bool{Value: true}, + MinNumeric: types.Int64{Value: 0}, + MinUpper: types.Int64{Value: 0}, + MinLower: types.Int64{Value: 0}, + MinSpecial: types.Int64{Value: 0}, + OverrideSpecial: types.String{Value: "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"}, + Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, + } + + actual := stringModelV3{} + diags := resp.State.Get(context.Background(), &actual) + if diags.HasError() { + t.Errorf("error getting state: %v", diags) + } + + if !cmp.Equal(expected, actual) { + t.Errorf("expected: %+v, got: %+v", expected, actual) + } +} + +func TestUpgradeStringStateV1toV3_NullValues(t *testing.T) { + t.Parallel() + + raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, nil), + "lower": tftypes.NewValue(tftypes.Bool, nil), + "min_lower": tftypes.NewValue(tftypes.Number, nil), + "min_numeric": tftypes.NewValue(tftypes.Number, nil), + "min_special": tftypes.NewValue(tftypes.Number, nil), + "min_upper": tftypes.NewValue(tftypes.Number, nil), + "number": tftypes.NewValue(tftypes.Bool, nil), + "override_special": tftypes.NewValue(tftypes.String, nil), + "result": tftypes.NewValue(tftypes.String, "DZy_3*tnonj%Q%Yx"), + "special": tftypes.NewValue(tftypes.Bool, nil), + "upper": tftypes.NewValue(tftypes.Bool, nil), + }) + + req := res.UpgradeStateRequest{ + State: &tfsdk.State{ + Raw: raw, + Schema: stringSchemaV1(), + }, + } + + resp := &res.UpgradeStateResponse{ + State: tfsdk.State{ + Schema: stringSchemaV3(), + }, + } + + upgradeStringStateV1toV3(context.Background(), req, resp) + + expected := stringModelV3{ + ID: types.String{Value: "none"}, + Keepers: types.Map{Null: true, ElemType: types.StringType}, + Length: types.Int64{Value: 16}, + Special: types.Bool{Value: true}, + Upper: types.Bool{Value: true}, + Lower: types.Bool{Value: true}, + Number: types.Bool{Value: true}, + Numeric: types.Bool{Value: true}, + MinNumeric: types.Int64{Value: 0}, + MinUpper: types.Int64{Value: 0}, + MinLower: types.Int64{Value: 0}, + MinSpecial: types.Int64{Value: 0}, + OverrideSpecial: types.String{Null: true}, + Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, + } + + actual := stringModelV3{} + diags := resp.State.Get(context.Background(), &actual) + if diags.HasError() { + t.Errorf("error getting state: %v", diags) + } + + if !cmp.Equal(expected, actual) { + t.Errorf("expected: %+v, got: %+v", expected, actual) + } +} + +func TestUpgradeStringStateV2toV3(t *testing.T) { + t.Parallel() + + raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, 16), + "lower": tftypes.NewValue(tftypes.Bool, true), + "min_lower": tftypes.NewValue(tftypes.Number, 0), + "min_numeric": tftypes.NewValue(tftypes.Number, 0), + "min_special": tftypes.NewValue(tftypes.Number, 0), + "min_upper": tftypes.NewValue(tftypes.Number, 0), + "number": tftypes.NewValue(tftypes.Bool, true), + "numeric": tftypes.NewValue(tftypes.Bool, true), + "override_special": tftypes.NewValue(tftypes.String, "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"), + "result": tftypes.NewValue(tftypes.String, "DZy_3*tnonj%Q%Yx"), + "special": tftypes.NewValue(tftypes.Bool, true), + "upper": tftypes.NewValue(tftypes.Bool, true), + }) + + req := res.UpgradeStateRequest{ + State: &tfsdk.State{ + Raw: raw, + Schema: stringSchemaV2(), + }, + } + + resp := &res.UpgradeStateResponse{ + State: tfsdk.State{ + Schema: stringSchemaV3(), + }, + } + + upgradeStringStateV2toV3(context.Background(), req, resp) + + expected := stringModelV3{ + ID: types.String{Value: "none"}, + Keepers: types.Map{Null: true, ElemType: types.StringType}, + Length: types.Int64{Value: 16}, + Special: types.Bool{Value: true}, + Upper: types.Bool{Value: true}, + Lower: types.Bool{Value: true}, + Number: types.Bool{Value: true}, + Numeric: types.Bool{Value: true}, + MinNumeric: types.Int64{Value: 0}, + MinUpper: types.Int64{Value: 0}, + MinLower: types.Int64{Value: 0}, + MinSpecial: types.Int64{Value: 0}, + OverrideSpecial: types.String{Value: "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"}, + Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, + } + + actual := stringModelV3{} + diags := resp.State.Get(context.Background(), &actual) + if diags.HasError() { + t.Errorf("error getting state: %v", diags) + } + + if !cmp.Equal(expected, actual) { + t.Errorf("expected: %+v, got: %+v", expected, actual) + } +} + +func TestUpgradeStringStateV2toV3_NullValues(t *testing.T) { + t.Parallel() + + raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, nil), + "lower": tftypes.NewValue(tftypes.Bool, nil), + "min_lower": tftypes.NewValue(tftypes.Number, nil), + "min_numeric": tftypes.NewValue(tftypes.Number, nil), + "min_special": tftypes.NewValue(tftypes.Number, nil), + "min_upper": tftypes.NewValue(tftypes.Number, nil), + "number": tftypes.NewValue(tftypes.Bool, nil), + "numeric": tftypes.NewValue(tftypes.Bool, nil), + "override_special": tftypes.NewValue(tftypes.String, nil), + "result": tftypes.NewValue(tftypes.String, "DZy_3*tnonj%Q%Yx"), + "special": tftypes.NewValue(tftypes.Bool, nil), + "upper": tftypes.NewValue(tftypes.Bool, nil), + }) + + req := res.UpgradeStateRequest{ + State: &tfsdk.State{ + Raw: raw, + Schema: stringSchemaV2(), + }, + } + + resp := &res.UpgradeStateResponse{ + State: tfsdk.State{ + Schema: stringSchemaV3(), + }, + } + + upgradeStringStateV2toV3(context.Background(), req, resp) + + expected := stringModelV3{ + ID: types.String{Value: "none"}, + Keepers: types.Map{Null: true, ElemType: types.StringType}, + Length: types.Int64{Value: 16}, + Special: types.Bool{Value: true}, + Upper: types.Bool{Value: true}, + Lower: types.Bool{Value: true}, + Number: types.Bool{Value: true}, + Numeric: types.Bool{Value: true}, + MinNumeric: types.Int64{Value: 0}, + MinUpper: types.Int64{Value: 0}, + MinLower: types.Int64{Value: 0}, + MinSpecial: types.Int64{Value: 0}, + OverrideSpecial: types.String{Null: true}, + Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, + } + + actual := stringModelV3{} + diags := resp.State.Get(context.Background(), &actual) + if diags.HasError() { + t.Errorf("error getting state: %v", diags) + } + + if !cmp.Equal(expected, actual) { + t.Errorf("expected: %+v, got: %+v", expected, actual) + } +} + +// TestAccResourcePassword_String_FromVersion3_1_3 verifies behaviour when resource has been imported and stores +// null for length, lower, number, special, upper, min_lower, min_numeric, min_special, min_upper attributes in state. +// v3.1.3 was selected as this is the last provider version using schema version 1. +func TestAccResourceString_Import_FromVersion3_1_3(t *testing.T) { + var result1, result2 string + + resource.ParallelTest(t, resource.TestCase{ + Steps: []resource.TestStep{ + { + ExternalProviders: providerVersion313(), + Config: `resource "random_string" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), + testExtractResourceAttr("random_string.test", "result", &result1), + ), + }, + { + ExternalProviders: providerVersion313(), + ResourceName: "random_string.test", + // Usage of ImportStateIdFunc is required as the value passed to the `terraform import` command needs + // to be the password itself, as the password resource sets ID to "none" and "result" to the password + // supplied during import. + ImportStateIdFunc: func(s *terraform.State) (string, error) { + id := "random_string.test" + rs, ok := s.RootModule().Resources[id] + if !ok { + return "", fmt.Errorf("not found: %s", id) + } + if rs.Primary.ID == "" { + return "", fmt.Errorf("no ID is set") + } + + return rs.Primary.Attributes["result"], nil + }, + ImportState: true, + // These checks should fail as running terraform import with v3.1.3 stores null for result and number + // attributes in state. + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), + resource.TestCheckResourceAttr("random_string.test", "number", "true"), + ), + }, + { + // This test is not really verifying desired behaviour as the import is populating length, number etc + ProtoV5ProviderFactories: protoV5ProviderFactories(), + Config: `resource "random_string" "test" { + length = 12 + }`, + PlanOnly: true, + }, + { + // This test is not really verifying desired behaviour as the import is populating length, number etc + ProtoV5ProviderFactories: protoV5ProviderFactories(), + Config: `resource "random_string" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), + testExtractResourceAttr("random_string.test", "result", &result2), + testCheckAttributeValuesEqual(&result1, &result2), + ), + }, + }, + }) +} + +// TestAccResourceString_Import_FromVersion3_2_0 verifies behaviour when resource has been imported and stores +// empty map {} for keepers and empty string for override_special in state. +// v3.4.2 was selected as this is the last provider version using schema version 2. +func TestAccResourceString_Import_FromVersion3_4_2(t *testing.T) { + var result1, result2 string + + resource.ParallelTest(t, resource.TestCase{ + Steps: []resource.TestStep{ + { + ExternalProviders: providerVersion342(), + Config: `resource "random_string" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), + testExtractResourceAttr("random_string.test", "result", &result1), + ), + }, + { + ExternalProviders: providerVersion342(), + ResourceName: "random_string.test", + // Usage of ImportStateIdFunc is required as the value passed to the `terraform import` command needs + // to be the password itself, as the password resource sets ID to "none" and "result" to the password + // supplied during import. + ImportStateIdFunc: func(s *terraform.State) (string, error) { + id := "random_string.test" + rs, ok := s.RootModule().Resources[id] + if !ok { + return "", fmt.Errorf("not found: %s", id) + } + if rs.Primary.ID == "" { + return "", fmt.Errorf("no ID is set") + } + + return rs.Primary.Attributes["result"], nil + }, + ImportState: true, + // These checks should fail as running terraform import with v3.2.0 stores null for result and number + // attributes in state. + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), + resource.TestCheckResourceAttr("random_string.test", "override_special", ""), + ), + }, + { + ProtoV5ProviderFactories: protoV5ProviderFactories(), + Config: `resource "random_string" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), + testExtractResourceAttr("random_string.test", "result", &result2), + testCheckAttributeValuesEqual(&result1, &result2), + ), + }, + }, + }) +} + func testCheckLen(expectedLen int) func(input string) error { return func(input string) error { if len(input) != expectedLen { From cbe7977a9fea267550a8627f88b2323341409d8f Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Tue, 6 Sep 2022 17:05:07 +0100 Subject: [PATCH 4/9] Modifying ImportState functions for password and string to set override_special and keepers in state to null. --- internal/provider/resource_password_test.go | 2 +- internal/provider/resource_string.go | 26 +++++++++++---------- internal/provider/resource_string_test.go | 2 +- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go index aaf8f258..4e77d736 100644 --- a/internal/provider/resource_password_test.go +++ b/internal/provider/resource_password_test.go @@ -64,7 +64,7 @@ func TestGenerateHash(t *testing.T) { } } -func TestAccResourcePassword(t *testing.T) { +func TestAccResourcePassword_Import(t *testing.T) { resource.UnitTest(t, resource.TestCase{ ProtoV5ProviderFactories: protoV5ProviderFactories(), Steps: []resource.TestStep{ diff --git a/internal/provider/resource_string.go b/internal/provider/resource_string.go index 19a7d2cf..98b86bbc 100644 --- a/internal/provider/resource_string.go +++ b/internal/provider/resource_string.go @@ -96,18 +96,20 @@ func (r *stringResource) ImportState(ctx context.Context, req resource.ImportSta id := req.ID state := stringModelV3{ - ID: types.String{Value: id}, - Result: types.String{Value: id}, - Length: types.Int64{Value: int64(len(id))}, - Special: types.Bool{Value: true}, - Upper: types.Bool{Value: true}, - Lower: types.Bool{Value: true}, - Number: types.Bool{Value: true}, - Numeric: types.Bool{Value: true}, - MinSpecial: types.Int64{Value: 0}, - MinUpper: types.Int64{Value: 0}, - MinLower: types.Int64{Value: 0}, - MinNumeric: types.Int64{Value: 0}, + ID: types.String{Value: id}, + Result: types.String{Value: id}, + Length: types.Int64{Value: int64(len(id))}, + Special: types.Bool{Value: true}, + Upper: types.Bool{Value: true}, + Lower: types.Bool{Value: true}, + Number: types.Bool{Value: true}, + Numeric: types.Bool{Value: true}, + MinSpecial: types.Int64{Value: 0}, + MinUpper: types.Int64{Value: 0}, + MinLower: types.Int64{Value: 0}, + MinNumeric: types.Int64{Value: 0}, + OverrideSpecial: types.String{Null: true}, + Keepers: types.Map{Null: true}, } state.Keepers.ElemType = types.StringType diff --git a/internal/provider/resource_string_test.go b/internal/provider/resource_string_test.go index dff14aab..1f4eb87b 100644 --- a/internal/provider/resource_string_test.go +++ b/internal/provider/resource_string_test.go @@ -15,7 +15,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) -func TestAccResourceString(t *testing.T) { +func TestAccResourceString_Import(t *testing.T) { resource.UnitTest(t, resource.TestCase{ ProtoV5ProviderFactories: protoV5ProviderFactories(), Steps: []resource.TestStep{ From a0bcbe446a68e1f097499115f878938932ae65df Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 8 Sep 2022 07:31:55 +0100 Subject: [PATCH 5/9] Integrating state upgrader changes into V3 as changes will be released with state upgrader changes handling fix of bcrypt --- internal/provider/resource_password.go | 361 ++------------------ internal/provider/resource_password_test.go | 180 ++-------- 2 files changed, 48 insertions(+), 493 deletions(-) diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go index 89902b92..a99fb64e 100644 --- a/internal/provider/resource_password.go +++ b/internal/provider/resource_password.go @@ -23,7 +23,7 @@ var _ provider.ResourceType = (*passwordResourceType)(nil) type passwordResourceType struct{} func (r *passwordResourceType) GetSchema(context.Context) (tfsdk.Schema, diag.Diagnostics) { - return passwordSchemaV4(), nil + return passwordSchemaV3(), nil } func (r *passwordResourceType) NewResource(_ context.Context, _ provider.Provider) (resource.Resource, diag.Diagnostics) { @@ -39,7 +39,7 @@ var ( type passwordResource struct{} func (r *passwordResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { - var plan passwordModelV4 + var plan passwordModelV3 diags := req.Plan.Get(ctx, &plan) resp.Diagnostics.Append(diags...) @@ -84,7 +84,7 @@ func (r *passwordResource) Read(ctx context.Context, req resource.ReadRequest, r // Update ensures the plan value is copied to the state to complete the update. func (r *passwordResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { - var model passwordModelV4 + var model passwordModelV3 resp.Diagnostics.Append(req.Plan.Get(ctx, &model)...) @@ -103,7 +103,7 @@ func (r *passwordResource) Delete(ctx context.Context, req resource.DeleteReques func (r *passwordResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { id := req.ID - state := passwordModelV4{ + state := passwordModelV3{ ID: types.String{Value: "none"}, Result: types.String{Value: id}, Length: types.Int64{Value: int64(len(id))}, @@ -141,29 +141,24 @@ func (r *passwordResource) UpgradeState(context.Context) map[int64]resource.Stat schemaV0 := passwordSchemaV0() schemaV1 := passwordSchemaV1() schemaV2 := passwordSchemaV2() - schemaV3 := passwordSchemaV3() return map[int64]resource.StateUpgrader{ 0: { PriorSchema: &schemaV0, - StateUpgrader: upgradePasswordStateV0toV4, + StateUpgrader: upgradePasswordStateV0toV3, }, 1: { PriorSchema: &schemaV1, - StateUpgrader: upgradePasswordStateV1toV4, + StateUpgrader: upgradePasswordStateV1toV3, }, 2: { PriorSchema: &schemaV2, - StateUpgrader: upgradePasswordStateV2toV4, - }, - 3: { - PriorSchema: &schemaV3, - StateUpgrader: upgradePasswordStateV3toV4, + StateUpgrader: upgradePasswordStateV2toV3, }, } } -func upgradePasswordStateV0toV4(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { +func upgradePasswordStateV0toV3(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { type modelV0 struct { ID types.String `tfsdk:"id"` Keepers types.Map `tfsdk:"keepers"` @@ -250,7 +245,7 @@ func upgradePasswordStateV0toV4(ctx context.Context, req resource.UpgradeStateRe number.Value = true } - passwordDataV4 := passwordModelV4{ + passwordDataV3 := passwordModelV3{ Keepers: passwordDataV0.Keepers, Length: length, Special: special, @@ -267,19 +262,19 @@ func upgradePasswordStateV0toV4(ctx context.Context, req resource.UpgradeStateRe ID: passwordDataV0.ID, } - hash, err := generateHash(passwordDataV4.Result.Value) + hash, err := generateHash(passwordDataV3.Result.Value) if err != nil { resp.Diagnostics.Append(diagnostics.HashGenerationError(err.Error())...) return } - passwordDataV4.BcryptHash.Value = hash + passwordDataV3.BcryptHash.Value = hash - diags := resp.State.Set(ctx, passwordDataV4) + diags := resp.State.Set(ctx, passwordDataV3) resp.Diagnostics.Append(diags...) } -func upgradePasswordStateV1toV4(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { +func upgradePasswordStateV1toV3(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { type modelV1 struct { ID types.String `tfsdk:"id"` Keepers types.Map `tfsdk:"keepers"` @@ -367,7 +362,7 @@ func upgradePasswordStateV1toV4(ctx context.Context, req resource.UpgradeStateRe number.Value = true } - passwordDataV4 := passwordModelV4{ + passwordDataV3 := passwordModelV3{ Keepers: passwordDataV1.Keepers, Length: length, Special: special, @@ -385,11 +380,11 @@ func upgradePasswordStateV1toV4(ctx context.Context, req resource.UpgradeStateRe ID: passwordDataV1.ID, } - diags := resp.State.Set(ctx, passwordDataV4) + diags := resp.State.Set(ctx, passwordDataV3) resp.Diagnostics.Append(diags...) } -func upgradePasswordStateV2toV4(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { +func upgradePasswordStateV2toV3(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { type passwordModelV2 struct { ID types.String `tfsdk:"id"` Keepers types.Map `tfsdk:"keepers"` @@ -486,10 +481,10 @@ func upgradePasswordStateV2toV4(ctx context.Context, req resource.UpgradeStateRe numeric.Value = true } - // Schema version 2 to schema version 4 is a duplicate of the data, + // Schema version 2 to schema version 3 is a duplicate of the data, // however the BcryptHash value may have been incorrectly generated. - //nolint:gosimple // V4 model will expand over time so all fields are written out to help future code changes. - passwordDataV4 := passwordModelV4{ + //nolint:gosimple // V3 model will expand over time so all fields are written out to help future code changes. + passwordDataV3 := passwordModelV3{ BcryptHash: passwordDataV2.BcryptHash, ID: passwordDataV2.ID, Keepers: passwordDataV2.Keepers, @@ -509,7 +504,7 @@ func upgradePasswordStateV2toV4(ctx context.Context, req resource.UpgradeStateRe // Set the duplicated data now so we can easily return early below. // The BcryptHash value will be adjusted later if it is incorrect. - resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV4)...) + resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV3)...) if resp.Diagnostics.HasError() { return @@ -547,131 +542,9 @@ func upgradePasswordStateV2toV4(ctx context.Context, req resource.UpgradeStateRe return } - passwordDataV4.BcryptHash.Value = string(newBcryptHash) - - resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV4)...) -} - -func upgradePasswordStateV3toV4(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { - type passwordModelV3 struct { - ID types.String `tfsdk:"id"` - Keepers types.Map `tfsdk:"keepers"` - Length types.Int64 `tfsdk:"length"` - Special types.Bool `tfsdk:"special"` - Upper types.Bool `tfsdk:"upper"` - Lower types.Bool `tfsdk:"lower"` - Number types.Bool `tfsdk:"number"` - Numeric types.Bool `tfsdk:"numeric"` - MinNumeric types.Int64 `tfsdk:"min_numeric"` - MinUpper types.Int64 `tfsdk:"min_upper"` - MinLower types.Int64 `tfsdk:"min_lower"` - MinSpecial types.Int64 `tfsdk:"min_special"` - OverrideSpecial types.String `tfsdk:"override_special"` - Result types.String `tfsdk:"result"` - BcryptHash types.String `tfsdk:"bcrypt_hash"` - } - - var passwordDataV3 passwordModelV3 - - resp.Diagnostics.Append(req.State.Get(ctx, &passwordDataV3)...) - - if resp.Diagnostics.HasError() { - return - } - - // Setting fields that can contain null to non-null to prevent forced replacement. - // This can occur in cases where import has been used in provider versions v3.3.1 and earlier. - // If import has been used with v3.3.1, for instance then length, lower, number, special, upper, - // min_lower, min_numeric, min_special and min_upper attributes will all be null in state. - length := passwordDataV3.Length - - if length.IsNull() { - length.Null = false - length.Value = int64(len(passwordDataV3.Result.Value)) - } - - minNumeric := passwordDataV3.MinNumeric - - if minNumeric.IsNull() { - minNumeric.Null = false - } - - minUpper := passwordDataV3.MinUpper - - if minUpper.IsNull() { - minUpper.Null = false - } - - minLower := passwordDataV3.MinLower - - if minLower.IsNull() { - minLower.Null = false - } - - minSpecial := passwordDataV3.MinSpecial - - if minSpecial.IsNull() { - minSpecial.Null = false - } - - special := passwordDataV3.Special - - if special.IsNull() { - special.Null = false - special.Value = true - } - - upper := passwordDataV3.Upper - - if upper.IsNull() { - upper.Null = false - upper.Value = true - } - - lower := passwordDataV3.Lower - - if lower.IsNull() { - lower.Null = false - lower.Value = true - } - - number := passwordDataV3.Number - - if number.IsNull() { - number.Null = false - number.Value = true - } - - numeric := passwordDataV3.Number - - if numeric.IsNull() { - numeric.Null = false - numeric.Value = true - } - - // Schema version 3 to schema version 4 is a duplicate of the data, - // however the . - - //nolint:gosimple // V4 model will expand over time so all fields are written out to help future code changes. - passwordDataV4 := passwordModelV4{ - BcryptHash: passwordDataV3.BcryptHash, - ID: passwordDataV3.ID, - Keepers: passwordDataV3.Keepers, - Length: length, - Lower: lower, - MinLower: minLower, - MinNumeric: minNumeric, - MinSpecial: minSpecial, - MinUpper: minUpper, - Number: number, - Numeric: numeric, - OverrideSpecial: passwordDataV3.OverrideSpecial, - Result: passwordDataV3.Result, - Special: special, - Upper: upper, - } + passwordDataV3.BcryptHash.Value = string(newBcryptHash) - resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV4)...) + resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV3)...) } func generateHash(toHash string) (string, error) { @@ -680,194 +553,6 @@ func generateHash(toHash string) (string, error) { return string(hash), err } -func passwordSchemaV4() tfsdk.Schema { - return tfsdk.Schema{ - Version: 4, - 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 " + - "data handling in the " + - "[Terraform documentation](https://www.terraform.io/docs/language/state/sensitive-data.html).\n\n" + - "This resource *does* use a cryptographic random number generator.", - Attributes: map[string]tfsdk.Attribute{ - "keepers": { - Description: "Arbitrary map of values that, when changed, will trigger recreation of " + - "resource. See [the main provider documentation](../index.html) for more information.", - Type: types.MapType{ - ElemType: types.StringType, - }, - Optional: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - planmodifiers.RequiresReplaceIfValuesNotNull(), - }, - }, - - "length": { - Description: "The length of the string desired. The minimum value for length is 1 and, length " + - "must also be >= (`min_upper` + `min_lower` + `min_numeric` + `min_special`).", - Type: types.Int64Type, - Required: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - resource.RequiresReplace(), - }, - Validators: []tfsdk.AttributeValidator{ - int64validator.AtLeast(1), - int64validator.AtLeastSumOf( - path.MatchRoot("min_upper"), - path.MatchRoot("min_lower"), - path.MatchRoot("min_numeric"), - path.MatchRoot("min_special"), - ), - }, - }, - - "special": { - Description: "Include special characters in the result. These are `!@#$%&*()-_=+[]{}<>:?`. Default value is `true`.", - Type: types.BoolType, - Optional: true, - Computed: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - planmodifiers.DefaultValue(types.Bool{Value: true}), - planmodifiers.RequiresReplace(), - }, - }, - - "upper": { - Description: "Include uppercase alphabet characters in the result. Default value is `true`.", - Type: types.BoolType, - Optional: true, - Computed: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - planmodifiers.DefaultValue(types.Bool{Value: true}), - planmodifiers.RequiresReplace(), - }, - }, - - "lower": { - Description: "Include lowercase alphabet characters in the result. Default value is `true`.", - Type: types.BoolType, - Optional: true, - Computed: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - planmodifiers.DefaultValue(types.Bool{Value: true}), - planmodifiers.RequiresReplace(), - }, - }, - - "number": { - Description: "Include numeric characters in the result. Default value is `true`. " + - "**NOTE**: This is deprecated, use `numeric` instead.", - Type: types.BoolType, - Optional: true, - Computed: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - planmodifiers.NumberNumericAttributePlanModifier(), - planmodifiers.RequiresReplace(), - }, - DeprecationMessage: "**NOTE**: This is deprecated, use `numeric` instead.", - }, - - "numeric": { - Description: "Include numeric characters in the result. Default value is `true`.", - Type: types.BoolType, - Optional: true, - Computed: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - planmodifiers.NumberNumericAttributePlanModifier(), - planmodifiers.RequiresReplace(), - }, - }, - - "min_numeric": { - Description: "Minimum number of numeric characters in the result. Default value is `0`.", - Type: types.Int64Type, - Optional: true, - Computed: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - planmodifiers.DefaultValue(types.Int64{Value: 0}), - planmodifiers.RequiresReplace(), - }, - }, - - "min_upper": { - Description: "Minimum number of uppercase alphabet characters in the result. Default value is `0`.", - Type: types.Int64Type, - Optional: true, - Computed: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - planmodifiers.DefaultValue(types.Int64{Value: 0}), - planmodifiers.RequiresReplace(), - }, - }, - - "min_lower": { - Description: "Minimum number of lowercase alphabet characters in the result. Default value is `0`.", - Type: types.Int64Type, - Optional: true, - Computed: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - planmodifiers.DefaultValue(types.Int64{Value: 0}), - planmodifiers.RequiresReplace(), - }, - }, - - "min_special": { - Description: "Minimum number of special characters in the result. Default value is `0`.", - Type: types.Int64Type, - Optional: true, - Computed: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - planmodifiers.DefaultValue(types.Int64{Value: 0}), - planmodifiers.RequiresReplace(), - }, - }, - - "override_special": { - Description: "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.", - Type: types.StringType, - Optional: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - resource.RequiresReplaceIf( - planmodifiers.RequiresReplaceUnlessEmptyStringToNull(), - "Replace on modification unless updating from empty string (\"\") to null.", - "Replace on modification unless updating from empty string (`\"\"`) to `null`.", - ), - }, - }, - - "result": { - Description: "The generated random string.", - Type: types.StringType, - Computed: true, - Sensitive: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - resource.UseStateForUnknown(), - }, - }, - - "bcrypt_hash": { - Description: "A bcrypt hash of the generated random string.", - Type: types.StringType, - Computed: true, - Sensitive: true, - PlanModifiers: []tfsdk.AttributePlanModifier{ - resource.UseStateForUnknown(), - }, - }, - - "id": { - Description: "A static value used internally by Terraform, this should not be referenced in configurations.", - Computed: true, - Type: types.StringType, - PlanModifiers: []tfsdk.AttributePlanModifier{ - resource.UseStateForUnknown(), - }, - }, - }, - } -} - func passwordSchemaV3() tfsdk.Schema { return tfsdk.Schema{ Version: 3, @@ -1554,7 +1239,7 @@ func passwordSchemaV0() tfsdk.Schema { } } -type passwordModelV4 struct { +type passwordModelV3 struct { ID types.String `tfsdk:"id"` Keepers types.Map `tfsdk:"keepers"` Length types.Int64 `tfsdk:"length"` diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go index 4e77d736..02344b5d 100644 --- a/internal/provider/resource_password_test.go +++ b/internal/provider/resource_password_test.go @@ -1137,7 +1137,7 @@ func TestAccResourcePassword_UpgradeFromVersion3_3_2(t *testing.T) { }) } -func TestUpgradePasswordStateV0toV4(t *testing.T) { +func TestUpgradePasswordStateV0toV3(t *testing.T) { t.Parallel() raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ @@ -1165,13 +1165,13 @@ func TestUpgradePasswordStateV0toV4(t *testing.T) { resp := &res.UpgradeStateResponse{ State: tfsdk.State{ - Schema: passwordSchemaV4(), + Schema: passwordSchemaV3(), }, } - upgradePasswordStateV0toV4(context.Background(), req, resp) + upgradePasswordStateV0toV3(context.Background(), req, resp) - expected := passwordModelV4{ + expected := passwordModelV3{ ID: types.String{Value: "none"}, Keepers: types.Map{Null: true, ElemType: types.StringType}, Length: types.Int64{Value: 16}, @@ -1188,7 +1188,7 @@ func TestUpgradePasswordStateV0toV4(t *testing.T) { Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, } - actual := passwordModelV4{} + actual := passwordModelV3{} diags := resp.State.Get(context.Background(), &actual) if diags.HasError() { t.Errorf("error getting state: %v", diags) @@ -1207,7 +1207,7 @@ func TestUpgradePasswordStateV0toV4(t *testing.T) { } } -func TestUpgradePasswordStateV0toV4_NullValues(t *testing.T) { +func TestUpgradePasswordStateV0toV3_NullValues(t *testing.T) { t.Parallel() raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ @@ -1235,13 +1235,13 @@ func TestUpgradePasswordStateV0toV4_NullValues(t *testing.T) { resp := &res.UpgradeStateResponse{ State: tfsdk.State{ - Schema: passwordSchemaV4(), + Schema: passwordSchemaV3(), }, } - upgradePasswordStateV0toV4(context.Background(), req, resp) + upgradePasswordStateV0toV3(context.Background(), req, resp) - expected := passwordModelV4{ + expected := passwordModelV3{ ID: types.String{Value: "none"}, Keepers: types.Map{Null: true, ElemType: types.StringType}, Length: types.Int64{Value: 16}, @@ -1258,7 +1258,7 @@ func TestUpgradePasswordStateV0toV4_NullValues(t *testing.T) { Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, } - actual := passwordModelV4{} + actual := passwordModelV3{} diags := resp.State.Get(context.Background(), &actual) if diags.HasError() { t.Errorf("error getting state: %v", diags) @@ -1277,7 +1277,7 @@ func TestUpgradePasswordStateV0toV4_NullValues(t *testing.T) { } } -func TestUpgradePasswordStateV1toV4(t *testing.T) { +func TestUpgradePasswordStateV1toV3(t *testing.T) { t.Parallel() raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ @@ -1306,13 +1306,13 @@ func TestUpgradePasswordStateV1toV4(t *testing.T) { resp := &res.UpgradeStateResponse{ State: tfsdk.State{ - Schema: passwordSchemaV4(), + Schema: passwordSchemaV3(), }, } - upgradePasswordStateV1toV4(context.Background(), req, resp) + upgradePasswordStateV1toV3(context.Background(), req, resp) - expected := passwordModelV4{ + expected := passwordModelV3{ ID: types.String{Value: "none"}, Keepers: types.Map{Null: true, ElemType: types.StringType}, Length: types.Int64{Value: 16}, @@ -1330,7 +1330,7 @@ func TestUpgradePasswordStateV1toV4(t *testing.T) { Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, } - actual := passwordModelV4{} + actual := passwordModelV3{} diags := resp.State.Get(context.Background(), &actual) if diags.HasError() { t.Errorf("error getting state: %v", diags) @@ -1341,7 +1341,7 @@ func TestUpgradePasswordStateV1toV4(t *testing.T) { } } -func TestUpgradePasswordStateV1toV4_NullValues(t *testing.T) { +func TestUpgradePasswordStateV1toV3_NullValues(t *testing.T) { t.Parallel() raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ @@ -1370,13 +1370,13 @@ func TestUpgradePasswordStateV1toV4_NullValues(t *testing.T) { resp := &res.UpgradeStateResponse{ State: tfsdk.State{ - Schema: passwordSchemaV4(), + Schema: passwordSchemaV3(), }, } - upgradePasswordStateV1toV4(context.Background(), req, resp) + upgradePasswordStateV1toV3(context.Background(), req, resp) - expected := passwordModelV4{ + expected := passwordModelV3{ ID: types.String{Value: "none"}, Keepers: types.Map{Null: true, ElemType: types.StringType}, Length: types.Int64{Value: 16}, @@ -1394,7 +1394,7 @@ func TestUpgradePasswordStateV1toV4_NullValues(t *testing.T) { Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, } - actual := passwordModelV4{} + actual := passwordModelV3{} diags := resp.State.Get(context.Background(), &actual) if diags.HasError() { t.Errorf("error getting state: %v", diags) @@ -1405,7 +1405,7 @@ func TestUpgradePasswordStateV1toV4_NullValues(t *testing.T) { } } -func TestUpgradePasswordStateV2toV4(t *testing.T) { +func TestUpgradePasswordStateV2toV3(t *testing.T) { t.Parallel() testCases := map[string]struct { @@ -1492,7 +1492,7 @@ func TestUpgradePasswordStateV2toV4(t *testing.T) { "special": tftypes.NewValue(tftypes.Bool, true), "upper": tftypes.NewValue(tftypes.Bool, true), }), - Schema: passwordSchemaV4(), + Schema: passwordSchemaV3(), }, }, }, @@ -1576,7 +1576,7 @@ func TestUpgradePasswordStateV2toV4(t *testing.T) { "special": tftypes.NewValue(tftypes.Bool, true), "upper": tftypes.NewValue(tftypes.Bool, true), }), - Schema: passwordSchemaV4(), + Schema: passwordSchemaV3(), }, }, }, @@ -1660,7 +1660,7 @@ func TestUpgradePasswordStateV2toV4(t *testing.T) { "special": tftypes.NewValue(tftypes.Bool, true), "upper": tftypes.NewValue(tftypes.Bool, true), }), - Schema: passwordSchemaV4(), + Schema: passwordSchemaV3(), }, }, }, @@ -1678,7 +1678,7 @@ func TestUpgradePasswordStateV2toV4(t *testing.T) { }, } - upgradePasswordStateV2toV4(context.Background(), testCase.request, &got) + upgradePasswordStateV2toV3(context.Background(), testCase.request, &got) // Since bcrypt_hash is generated, this test is very involved to // ensure the test case is set up properly and the generated @@ -1799,136 +1799,6 @@ func TestUpgradePasswordStateV2toV4(t *testing.T) { } } -func TestUpgradePasswordStateV3toV4(t *testing.T) { - t.Parallel() - - raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "none"), - "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), - "length": tftypes.NewValue(tftypes.Number, 16), - "lower": tftypes.NewValue(tftypes.Bool, true), - "min_lower": tftypes.NewValue(tftypes.Number, 0), - "min_numeric": tftypes.NewValue(tftypes.Number, 0), - "min_special": tftypes.NewValue(tftypes.Number, 0), - "min_upper": tftypes.NewValue(tftypes.Number, 0), - "number": tftypes.NewValue(tftypes.Bool, true), - "numeric": tftypes.NewValue(tftypes.Bool, true), - "override_special": tftypes.NewValue(tftypes.String, "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"), - "result": tftypes.NewValue(tftypes.String, "DZy_3*tnonj%Q%Yx"), - "special": tftypes.NewValue(tftypes.Bool, true), - "upper": tftypes.NewValue(tftypes.Bool, true), - "bcrypt_hash": tftypes.NewValue(tftypes.String, "bcrypt_hash"), - }) - - req := res.UpgradeStateRequest{ - State: &tfsdk.State{ - Raw: raw, - Schema: passwordSchemaV3(), - }, - } - - resp := &res.UpgradeStateResponse{ - State: tfsdk.State{ - Schema: passwordSchemaV4(), - }, - } - - upgradePasswordStateV3toV4(context.Background(), req, resp) - - expected := passwordModelV4{ - ID: types.String{Value: "none"}, - Keepers: types.Map{Null: true, ElemType: types.StringType}, - Length: types.Int64{Value: 16}, - Special: types.Bool{Value: true}, - Upper: types.Bool{Value: true}, - Lower: types.Bool{Value: true}, - Number: types.Bool{Value: true}, - Numeric: types.Bool{Value: true}, - MinNumeric: types.Int64{Value: 0}, - MinUpper: types.Int64{Value: 0}, - MinLower: types.Int64{Value: 0}, - MinSpecial: types.Int64{Value: 0}, - OverrideSpecial: types.String{Value: "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"}, - BcryptHash: types.String{Value: "bcrypt_hash"}, - Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, - } - - actual := passwordModelV4{} - diags := resp.State.Get(context.Background(), &actual) - if diags.HasError() { - t.Errorf("error getting state: %v", diags) - } - - if !cmp.Equal(expected, actual) { - t.Errorf("expected: %+v, got: %+v", expected, actual) - } -} - -func TestUpgradePasswordStateV3toV4_NullValues(t *testing.T) { - t.Parallel() - - raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "none"), - "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), - "length": tftypes.NewValue(tftypes.Number, nil), - "lower": tftypes.NewValue(tftypes.Bool, true), - "min_lower": tftypes.NewValue(tftypes.Number, nil), - "min_numeric": tftypes.NewValue(tftypes.Number, nil), - "min_special": tftypes.NewValue(tftypes.Number, nil), - "min_upper": tftypes.NewValue(tftypes.Number, nil), - "number": tftypes.NewValue(tftypes.Bool, nil), - "numeric": tftypes.NewValue(tftypes.Bool, nil), - "override_special": tftypes.NewValue(tftypes.String, "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"), - "result": tftypes.NewValue(tftypes.String, "DZy_3*tnonj%Q%Yx"), - "special": tftypes.NewValue(tftypes.Bool, nil), - "upper": tftypes.NewValue(tftypes.Bool, nil), - "bcrypt_hash": tftypes.NewValue(tftypes.String, "bcrypt_hash"), - }) - - req := res.UpgradeStateRequest{ - State: &tfsdk.State{ - Raw: raw, - Schema: passwordSchemaV3(), - }, - } - - resp := &res.UpgradeStateResponse{ - State: tfsdk.State{ - Schema: passwordSchemaV4(), - }, - } - - upgradePasswordStateV3toV4(context.Background(), req, resp) - - expected := passwordModelV4{ - ID: types.String{Value: "none"}, - Keepers: types.Map{Null: true, ElemType: types.StringType}, - Length: types.Int64{Value: 16}, - Special: types.Bool{Value: true}, - Upper: types.Bool{Value: true}, - Lower: types.Bool{Value: true}, - Number: types.Bool{Value: true}, - Numeric: types.Bool{Value: true}, - MinNumeric: types.Int64{Value: 0}, - MinUpper: types.Int64{Value: 0}, - MinLower: types.Int64{Value: 0}, - MinSpecial: types.Int64{Value: 0}, - OverrideSpecial: types.String{Value: "!#$%\u0026*()-_=+[]{}\u003c\u003e:?"}, - BcryptHash: types.String{Value: "bcrypt_hash"}, - Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, - } - - actual := passwordModelV4{} - diags := resp.State.Get(context.Background(), &actual) - if diags.HasError() { - t.Errorf("error getting state: %v", diags) - } - - if !cmp.Equal(expected, actual) { - t.Errorf("expected: %+v, got: %+v", expected, actual) - } -} - func TestAccResourcePassword_NumberNumericErrors(t *testing.T) { resource.UnitTest(t, resource.TestCase{ ProtoV5ProviderFactories: protoV5ProviderFactories(), From 9f694c4830f7acf9c926b9c6582e9a4affe77792 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 8 Sep 2022 08:27:25 +0100 Subject: [PATCH 6/9] Modifying tests that verify behavior of state upgrade that handles setting defaults for lower, number, numeric, special, upper, min_lower, min_numeric, min_special, min_upper and setting length when these attributes have been set to null --- internal/provider/resource_password_test.go | 241 ++++++++++++-------- internal/provider/resource_string_test.go | 112 ++++----- 2 files changed, 194 insertions(+), 159 deletions(-) diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go index 02344b5d..ea218329 100644 --- a/internal/provider/resource_password_test.go +++ b/internal/provider/resource_password_test.go @@ -293,39 +293,24 @@ func TestAccResourcePassword_Import_FromVersion3_1_3(t *testing.T) { Config: `resource "random_password" "test" { length = 12 }`, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), - testExtractResourceAttr("random_password.test", "result", &result1), - ), - }, - { - ExternalProviders: providerVersion313(), - ResourceName: "random_password.test", - // Usage of ImportStateIdFunc is required as the value passed to the `terraform import` command needs - // to be the password itself, as the password resource sets ID to "none" and "result" to the password - // supplied during import. - ImportStateIdFunc: func(s *terraform.State) (string, error) { - id := "random_password.test" - rs, ok := s.RootModule().Resources[id] - if !ok { - return "", fmt.Errorf("not found: %s", id) - } - if rs.Primary.ID == "" { - return "", fmt.Errorf("no ID is set") - } - - return rs.Primary.Attributes["result"], nil - }, - ImportState: true, - // These checks should fail as running terraform import with v3.1.3 stores null for result and number - // attributes in state. - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), - resource.TestCheckResourceAttr("random_password.test", "number", "true"), + ResourceName: "random_password.test", + ImportState: true, + ImportStateId: "Z=:cbrJE?Ltg", + ImportStatePersist: true, + ImportStateCheck: composeImportStateCheck( + testCheckNoResourceAttrInstanceState("length"), + testCheckNoResourceAttrInstanceState("number"), + testCheckNoResourceAttrInstanceState("upper"), + testCheckNoResourceAttrInstanceState("lower"), + testCheckNoResourceAttrInstanceState("special"), + testCheckNoResourceAttrInstanceState("min_numeric"), + testCheckNoResourceAttrInstanceState("min_upper"), + testCheckNoResourceAttrInstanceState("min_lower"), + testCheckNoResourceAttrInstanceState("min_special"), + testExtractResourceAttrInstanceState("result", &result1), ), }, { - // This test is not really verifying desired behaviour as the import is populating length, number etc ProtoV5ProviderFactories: protoV5ProviderFactories(), Config: `resource "random_password" "test" { length = 12 @@ -333,13 +318,21 @@ func TestAccResourcePassword_Import_FromVersion3_1_3(t *testing.T) { PlanOnly: true, }, { - // This test is not really verifying desired behaviour as the import is populating length, number etc ProtoV5ProviderFactories: protoV5ProviderFactories(), Config: `resource "random_password" "test" { length = 12 }`, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + resource.TestCheckResourceAttr("random_password.test", "number", "true"), + resource.TestCheckResourceAttr("random_password.test", "numeric", "true"), + resource.TestCheckResourceAttr("random_password.test", "upper", "true"), + resource.TestCheckResourceAttr("random_password.test", "lower", "true"), + resource.TestCheckResourceAttr("random_password.test", "special", "true"), + resource.TestCheckResourceAttr("random_password.test", "min_numeric", "0"), + resource.TestCheckResourceAttr("random_password.test", "min_upper", "0"), + resource.TestCheckResourceAttr("random_password.test", "min_lower", "0"), + resource.TestCheckResourceAttr("random_password.test", "min_special", "0"), testExtractResourceAttr("random_password.test", "result", &result2), testCheckAttributeValuesEqual(&result1, &result2), ), @@ -361,39 +354,24 @@ func TestAccResourcePassword_Import_FromVersion3_2_0(t *testing.T) { Config: `resource "random_password" "test" { length = 12 }`, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), - testExtractResourceAttr("random_password.test", "result", &result1), + ResourceName: "random_password.test", + ImportState: true, + ImportStateId: "Z=:cbrJE?Ltg", + ImportStatePersist: true, + ImportStateCheck: composeImportStateCheck( + testCheckNoResourceAttrInstanceState("length"), + testCheckNoResourceAttrInstanceState("number"), + testCheckNoResourceAttrInstanceState("upper"), + testCheckNoResourceAttrInstanceState("lower"), + testCheckNoResourceAttrInstanceState("special"), + testCheckNoResourceAttrInstanceState("min_numeric"), + testCheckNoResourceAttrInstanceState("min_upper"), + testCheckNoResourceAttrInstanceState("min_lower"), + testCheckNoResourceAttrInstanceState("min_special"), + testExtractResourceAttrInstanceState("result", &result1), ), }, { - ExternalProviders: providerVersion320(), - ResourceName: "random_password.test", - // Usage of ImportStateIdFunc is required as the value passed to the `terraform import` command needs - // to be the password itself, as the password resource sets ID to "none" and "result" to the password - // supplied during import. - ImportStateIdFunc: func(s *terraform.State) (string, error) { - id := "random_password.test" - rs, ok := s.RootModule().Resources[id] - if !ok { - return "", fmt.Errorf("not found: %s", id) - } - if rs.Primary.ID == "" { - return "", fmt.Errorf("no ID is set") - } - - return rs.Primary.Attributes["result"], nil - }, - ImportState: true, - // These checks should fail as running terraform import with v3.2.0 stores null for result and number - // attributes in state. - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), - resource.TestCheckResourceAttr("random_password.test", "number", "true"), - ), - }, - { - // This test is not really verifying desired behaviour as the import is populating length, number etc ProtoV5ProviderFactories: protoV5ProviderFactories(), Config: `resource "random_password" "test" { length = 12 @@ -401,13 +379,21 @@ func TestAccResourcePassword_Import_FromVersion3_2_0(t *testing.T) { PlanOnly: true, }, { - // This test is not really verifying desired behaviour as the import is populating length, number etc ProtoV5ProviderFactories: protoV5ProviderFactories(), Config: `resource "random_password" "test" { length = 12 }`, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + resource.TestCheckResourceAttr("random_password.test", "number", "true"), + resource.TestCheckResourceAttr("random_password.test", "numeric", "true"), + resource.TestCheckResourceAttr("random_password.test", "upper", "true"), + resource.TestCheckResourceAttr("random_password.test", "lower", "true"), + resource.TestCheckResourceAttr("random_password.test", "special", "true"), + resource.TestCheckResourceAttr("random_password.test", "min_numeric", "0"), + resource.TestCheckResourceAttr("random_password.test", "min_upper", "0"), + resource.TestCheckResourceAttr("random_password.test", "min_lower", "0"), + resource.TestCheckResourceAttr("random_password.test", "min_special", "0"), testExtractResourceAttr("random_password.test", "result", &result2), testCheckAttributeValuesEqual(&result1, &result2), ), @@ -416,7 +402,7 @@ func TestAccResourcePassword_Import_FromVersion3_2_0(t *testing.T) { }) } -// TestAccResourcePassword_Import_FromVersion3_2_0 verifies behaviour when resource has been imported and stores +// TestAccResourcePassword_Import_FromVersion3_4_2 verifies behaviour when resource has been imported and stores // empty map {} for keepers and empty string for override_special in state. // v3.4.2 was selected as this is the last provider version using schema version 2. func TestAccResourcePassword_Import_FromVersion3_4_2(t *testing.T) { @@ -429,35 +415,22 @@ func TestAccResourcePassword_Import_FromVersion3_4_2(t *testing.T) { Config: `resource "random_password" "test" { length = 12 }`, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), - testExtractResourceAttr("random_password.test", "result", &result1), - ), - }, - { - ExternalProviders: providerVersion342(), - ResourceName: "random_password.test", - // Usage of ImportStateIdFunc is required as the value passed to the `terraform import` command needs - // to be the password itself, as the password resource sets ID to "none" and "result" to the password - // supplied during import. - ImportStateIdFunc: func(s *terraform.State) (string, error) { - id := "random_password.test" - rs, ok := s.RootModule().Resources[id] - if !ok { - return "", fmt.Errorf("not found: %s", id) - } - if rs.Primary.ID == "" { - return "", fmt.Errorf("no ID is set") - } - - return rs.Primary.Attributes["result"], nil - }, - ImportState: true, - // These checks should fail as running terraform import with v3.2.0 stores null for result and number - // attributes in state. - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), - resource.TestCheckResourceAttr("random_password.test", "override_special", ""), + ResourceName: "random_password.test", + ImportState: true, + ImportStateId: "Z=:cbrJE?Ltg", + ImportStatePersist: true, + ImportStateCheck: composeImportStateCheck( + testCheckResourceAttrInstanceState("length"), + testCheckResourceAttrInstanceState("number"), + testCheckResourceAttrInstanceState("numeric"), + testCheckResourceAttrInstanceState("upper"), + testCheckResourceAttrInstanceState("lower"), + testCheckResourceAttrInstanceState("special"), + testCheckResourceAttrInstanceState("min_numeric"), + testCheckResourceAttrInstanceState("min_upper"), + testCheckResourceAttrInstanceState("min_lower"), + testCheckResourceAttrInstanceState("min_special"), + testExtractResourceAttrInstanceState("result", &result1), ), }, { @@ -467,6 +440,15 @@ func TestAccResourcePassword_Import_FromVersion3_4_2(t *testing.T) { }`, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrWith("random_password.test", "result", testCheckLen(12)), + resource.TestCheckResourceAttr("random_password.test", "number", "true"), + resource.TestCheckResourceAttr("random_password.test", "numeric", "true"), + resource.TestCheckResourceAttr("random_password.test", "upper", "true"), + resource.TestCheckResourceAttr("random_password.test", "lower", "true"), + resource.TestCheckResourceAttr("random_password.test", "special", "true"), + resource.TestCheckResourceAttr("random_password.test", "min_numeric", "0"), + resource.TestCheckResourceAttr("random_password.test", "min_upper", "0"), + resource.TestCheckResourceAttr("random_password.test", "min_lower", "0"), + resource.TestCheckResourceAttr("random_password.test", "min_special", "0"), testExtractResourceAttr("random_password.test", "result", &result2), testCheckAttributeValuesEqual(&result1, &result2), ), @@ -475,10 +457,10 @@ func TestAccResourcePassword_Import_FromVersion3_4_2(t *testing.T) { }) } -// TestAccResourcePassword_StateUpgradeV0toV2 covers the state upgrades from V0 to V2. -// This includes the the addition of `numeric` and `bcrypt_hash` attributes. +// TestAccResourcePassword_StateUpgradeV0toV3 covers the state upgrades from V0 to V3. +// This includes the addition of `numeric` and `bcrypt_hash` attributes. // v3.1.3 is used as this is last version before `bcrypt_hash` attributed was added. -func TestAccResourcePassword_StateUpgradeV0toV2(t *testing.T) { +func TestAccResourcePassword_StateUpgradeV0toV3(t *testing.T) { t.Parallel() cases := []struct { @@ -673,11 +655,11 @@ func TestAccResourcePassword_StateUpgradeV0toV2(t *testing.T) { } } -// TestAccResourcePassword_StateUpgrade_V1toV2 covers the state upgrades from V1 to V2. +// TestAccResourcePassword_StateUpgrade_V1toV3 covers the state upgrades from V1 to V3. // This includes the addition of `numeric` attribute. // v3.2.0 was used as this is the last version before `number` was deprecated and `numeric` attribute // was added. -func TestAccResourcePassword_StateUpgradeV1toV2(t *testing.T) { +func TestAccResourcePassword_StateUpgradeV1toV3(t *testing.T) { t.Parallel() cases := []struct { @@ -2591,3 +2573,68 @@ func testBcryptHashValid(hash *string, password *string) resource.TestCheckFunc return bcrypt.CompareHashAndPassword([]byte(*hash), []byte(*password)) } } + +func composeImportStateCheck(fs ...resource.ImportStateCheckFunc) resource.ImportStateCheckFunc { + return func(s []*terraform.InstanceState) error { + for i, f := range fs { + if err := f(s); err != nil { + return fmt.Errorf("check %d/%d error: %s", i+1, len(fs), err) + } + } + + return nil + } +} + +func testExtractResourceAttrInstanceState(attributeName string, attributeValue *string) resource.ImportStateCheckFunc { + return func(is []*terraform.InstanceState) error { + if len(is) != 1 { + return fmt.Errorf("unexpected number of instance states: %d", len(is)) + } + + s := is[0] + + attrValue, ok := s.Attributes[attributeName] + if !ok { + return fmt.Errorf("attribute %s not found in instance state", attributeName) + } + + *attributeValue = attrValue + + return nil + } +} + +func testCheckNoResourceAttrInstanceState(attributeName string) resource.ImportStateCheckFunc { + return func(is []*terraform.InstanceState) error { + if len(is) != 1 { + return fmt.Errorf("unexpected number of instance states: %d", len(is)) + } + + s := is[0] + + _, ok := s.Attributes[attributeName] + if ok { + return fmt.Errorf("attribute %s found in instance state", attributeName) + } + + return nil + } +} + +func testCheckResourceAttrInstanceState(attributeName string) resource.ImportStateCheckFunc { + return func(is []*terraform.InstanceState) error { + if len(is) != 1 { + return fmt.Errorf("unexpected number of instance states: %d", len(is)) + } + + s := is[0] + + _, ok := s.Attributes[attributeName] + if !ok { + return fmt.Errorf("attribute %s not found in instance state", attributeName) + } + + return nil + } +} diff --git a/internal/provider/resource_string_test.go b/internal/provider/resource_string_test.go index 1f4eb87b..7de6a30d 100644 --- a/internal/provider/resource_string_test.go +++ b/internal/provider/resource_string_test.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" - "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) func TestAccResourceString_Import(t *testing.T) { @@ -1528,39 +1527,24 @@ func TestAccResourceString_Import_FromVersion3_1_3(t *testing.T) { Config: `resource "random_string" "test" { length = 12 }`, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), - testExtractResourceAttr("random_string.test", "result", &result1), + ResourceName: "random_string.test", + ImportState: true, + ImportStateId: "Z=:cbrJE?Ltg", + ImportStatePersist: true, + ImportStateCheck: composeImportStateCheck( + testCheckNoResourceAttrInstanceState("length"), + testCheckNoResourceAttrInstanceState("number"), + testCheckNoResourceAttrInstanceState("upper"), + testCheckNoResourceAttrInstanceState("lower"), + testCheckNoResourceAttrInstanceState("special"), + testCheckNoResourceAttrInstanceState("min_numeric"), + testCheckNoResourceAttrInstanceState("min_upper"), + testCheckNoResourceAttrInstanceState("min_lower"), + testCheckNoResourceAttrInstanceState("min_special"), + testExtractResourceAttrInstanceState("result", &result1), ), }, { - ExternalProviders: providerVersion313(), - ResourceName: "random_string.test", - // Usage of ImportStateIdFunc is required as the value passed to the `terraform import` command needs - // to be the password itself, as the password resource sets ID to "none" and "result" to the password - // supplied during import. - ImportStateIdFunc: func(s *terraform.State) (string, error) { - id := "random_string.test" - rs, ok := s.RootModule().Resources[id] - if !ok { - return "", fmt.Errorf("not found: %s", id) - } - if rs.Primary.ID == "" { - return "", fmt.Errorf("no ID is set") - } - - return rs.Primary.Attributes["result"], nil - }, - ImportState: true, - // These checks should fail as running terraform import with v3.1.3 stores null for result and number - // attributes in state. - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), - resource.TestCheckResourceAttr("random_string.test", "number", "true"), - ), - }, - { - // This test is not really verifying desired behaviour as the import is populating length, number etc ProtoV5ProviderFactories: protoV5ProviderFactories(), Config: `resource "random_string" "test" { length = 12 @@ -1568,13 +1552,21 @@ func TestAccResourceString_Import_FromVersion3_1_3(t *testing.T) { PlanOnly: true, }, { - // This test is not really verifying desired behaviour as the import is populating length, number etc ProtoV5ProviderFactories: protoV5ProviderFactories(), Config: `resource "random_string" "test" { length = 12 }`, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), + resource.TestCheckResourceAttr("random_string.test", "number", "true"), + resource.TestCheckResourceAttr("random_string.test", "numeric", "true"), + resource.TestCheckResourceAttr("random_string.test", "upper", "true"), + resource.TestCheckResourceAttr("random_string.test", "lower", "true"), + resource.TestCheckResourceAttr("random_string.test", "special", "true"), + resource.TestCheckResourceAttr("random_string.test", "min_numeric", "0"), + resource.TestCheckResourceAttr("random_string.test", "min_upper", "0"), + resource.TestCheckResourceAttr("random_string.test", "min_lower", "0"), + resource.TestCheckResourceAttr("random_string.test", "min_special", "0"), testExtractResourceAttr("random_string.test", "result", &result2), testCheckAttributeValuesEqual(&result1, &result2), ), @@ -1583,7 +1575,7 @@ func TestAccResourceString_Import_FromVersion3_1_3(t *testing.T) { }) } -// TestAccResourceString_Import_FromVersion3_2_0 verifies behaviour when resource has been imported and stores +// TestAccResourceString_Import_FromVersion3_4_2 verifies behaviour when resource has been imported and stores // empty map {} for keepers and empty string for override_special in state. // v3.4.2 was selected as this is the last provider version using schema version 2. func TestAccResourceString_Import_FromVersion3_4_2(t *testing.T) { @@ -1596,35 +1588,22 @@ func TestAccResourceString_Import_FromVersion3_4_2(t *testing.T) { Config: `resource "random_string" "test" { length = 12 }`, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), - testExtractResourceAttr("random_string.test", "result", &result1), - ), - }, - { - ExternalProviders: providerVersion342(), - ResourceName: "random_string.test", - // Usage of ImportStateIdFunc is required as the value passed to the `terraform import` command needs - // to be the password itself, as the password resource sets ID to "none" and "result" to the password - // supplied during import. - ImportStateIdFunc: func(s *terraform.State) (string, error) { - id := "random_string.test" - rs, ok := s.RootModule().Resources[id] - if !ok { - return "", fmt.Errorf("not found: %s", id) - } - if rs.Primary.ID == "" { - return "", fmt.Errorf("no ID is set") - } - - return rs.Primary.Attributes["result"], nil - }, - ImportState: true, - // These checks should fail as running terraform import with v3.2.0 stores null for result and number - // attributes in state. - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), - resource.TestCheckResourceAttr("random_string.test", "override_special", ""), + ResourceName: "random_string.test", + ImportState: true, + ImportStateId: "Z=:cbrJE?Ltg", + ImportStatePersist: true, + ImportStateCheck: composeImportStateCheck( + testCheckResourceAttrInstanceState("length"), + testCheckResourceAttrInstanceState("number"), + testCheckResourceAttrInstanceState("numeric"), + testCheckResourceAttrInstanceState("upper"), + testCheckResourceAttrInstanceState("lower"), + testCheckResourceAttrInstanceState("special"), + testCheckResourceAttrInstanceState("min_numeric"), + testCheckResourceAttrInstanceState("min_upper"), + testCheckResourceAttrInstanceState("min_lower"), + testCheckResourceAttrInstanceState("min_special"), + testExtractResourceAttrInstanceState("result", &result1), ), }, { @@ -1634,6 +1613,15 @@ func TestAccResourceString_Import_FromVersion3_4_2(t *testing.T) { }`, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrWith("random_string.test", "result", testCheckLen(12)), + resource.TestCheckResourceAttr("random_string.test", "number", "true"), + resource.TestCheckResourceAttr("random_string.test", "numeric", "true"), + resource.TestCheckResourceAttr("random_string.test", "upper", "true"), + resource.TestCheckResourceAttr("random_string.test", "lower", "true"), + resource.TestCheckResourceAttr("random_string.test", "special", "true"), + resource.TestCheckResourceAttr("random_string.test", "min_numeric", "0"), + resource.TestCheckResourceAttr("random_string.test", "min_upper", "0"), + resource.TestCheckResourceAttr("random_string.test", "min_lower", "0"), + resource.TestCheckResourceAttr("random_string.test", "min_special", "0"), testExtractResourceAttr("random_string.test", "result", &result2), testCheckAttributeValuesEqual(&result1, &result2), ), From 5f2cec19217cf3f4736d42b6740b31510a38f9d1 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 8 Sep 2022 08:50:04 +0100 Subject: [PATCH 7/9] Updating CHANGELOG --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a77fd1d5..42a4c3af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,15 +2,19 @@ NOTES: +* resource/random_password: The values for `lower`, `number`, `special`, `upper`, `min_lower`, `min_numeric`, `min_special`, `min_upper` and `length` could be null if the resource was imported using version 3.3.1 or before. The value for `length` will be automatically calculated and assigned and default values will be assigned for the other attributes listed after this upgrade ([#313](https://github.com/hashicorp/terraform-provider-random/pull/313)) +* resource/random_string: The values for `lower`, `number`, `special`, `upper`, `min_lower`, `min_numeric`, `min_special`, `min_upper` and `length` could be null if the resource was imported using version 3.3.1 or before. The value for `length` will be automatically calculated and assigned and default values will be assigned for the other attributes listed after this upgrade ([#313](https://github.com/hashicorp/terraform-provider-random/pull/313)) * resource/random_password: If the resource was created between versions 3.4.0 and 3.4.2, the `bcrypt_hash` value would not correctly verify against the `result` value. Affected resources will automatically regenerate a valid `bcrypt_hash` after this upgrade. ([#308](https://github.com/hashicorp/terraform-provider-random/pull/308)) * resource/random_password: The `override_special` attribute may show a plan difference from empty string (`""`) to `null` if previously applied with version 3.4.2. The plan should show this as an in-place update and it should occur only once after upgrading. ([#312](https://github.com/hashicorp/terraform-provider-random/pull/312)) * resource/random_string: The `override_special` attribute may show a plan difference from empty string (`""`) to `null` if previously applied with version 3.4.2. The plan should show this as an in-place update and it should occur only once after upgrading. ([#312](https://github.com/hashicorp/terraform-provider-random/pull/312)) BUG FIXES: +* resource/random_password: Assign default values to `lower`, `number`, `special`, `upper`, `min_lower`, `min_numeric`, `min_special` and `min_upper` if null. Assign length of `result` to `length` if null ([#313](https://github.com/hashicorp/terraform-provider-random/pull/313)) +* resource/random_string: Assign default values to `lower`, `number`, `special`, `upper`, `min_lower`, `min_numeric`, `min_special` and `min_upper` if null. Assign length of `result` to `length` if null ([#313](https://github.com/hashicorp/terraform-provider-random/pull/313)) * resource/random_password: Fixed incorrect `bcrypt_hash` generation since version 3.4.0 ([#308](https://github.com/hashicorp/terraform-provider-random/pull/308)) * resource/random_password: Prevented difference with `override_special` when upgrading from version 3.3.2 and earlier ([#312](https://github.com/hashicorp/terraform-provider-random/pull/312)) -* resource/random_password: Prevented difference with `override_special` when upgrading from version 3.3.2 and earlier ([#312](https://github.com/hashicorp/terraform-provider-random/pull/312)) +* resource/random_string: Prevented difference with `override_special` when upgrading from version 3.3.2 and earlier ([#312](https://github.com/hashicorp/terraform-provider-random/pull/312)) ## 3.4.2 (September 02, 2022) From 75f3cd034cea295fe6f3f61ed4bc55d4aefd2381 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 8 Sep 2022 14:38:50 +0100 Subject: [PATCH 8/9] Bumping terraform-plugin-sdk to v2.22.0 (#302) --- go.mod | 8 ++++---- go.sum | 15 ++++++++------- internal/provider/resource_password_test.go | 1 + 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index fc4d39b9..328856c0 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/hashicorp/terraform-plugin-framework v0.11.1 github.com/hashicorp/terraform-plugin-framework-validators v0.5.0 github.com/hashicorp/terraform-plugin-go v0.14.0 - github.com/hashicorp/terraform-plugin-sdk/v2 v2.21.0 + github.com/hashicorp/terraform-plugin-sdk/v2 v2.22.0 golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d ) @@ -35,9 +35,9 @@ require ( github.com/hashicorp/go-plugin v1.4.4 // indirect github.com/hashicorp/go-version v1.6.0 // indirect github.com/hashicorp/hc-install v0.4.0 // indirect - github.com/hashicorp/hcl/v2 v2.13.0 // indirect + github.com/hashicorp/hcl/v2 v2.14.0 // indirect github.com/hashicorp/logutils v1.0.0 // indirect - github.com/hashicorp/terraform-exec v0.17.2 // indirect + github.com/hashicorp/terraform-exec v0.17.3 // indirect github.com/hashicorp/terraform-json v0.14.0 // indirect github.com/hashicorp/terraform-plugin-log v0.7.0 // indirect github.com/hashicorp/terraform-registry-address v0.0.0-20220623143253-7d51757b572c // indirect @@ -61,7 +61,7 @@ require ( github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect github.com/vmihailenco/msgpack/v4 v4.3.12 // indirect github.com/vmihailenco/tagparser v0.1.2 // indirect - github.com/zclconf/go-cty v1.10.0 // indirect + github.com/zclconf/go-cty v1.11.0 // indirect golang.org/x/net v0.0.0-20220708220712-1185a9018129 // indirect golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 // indirect golang.org/x/text v0.3.7 // indirect diff --git a/go.sum b/go.sum index e0ce3483..cdfb6ba5 100644 --- a/go.sum +++ b/go.sum @@ -130,12 +130,12 @@ github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mO github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/hc-install v0.4.0 h1:cZkRFr1WVa0Ty6x5fTvL1TuO1flul231rWkGH92oYYk= github.com/hashicorp/hc-install v0.4.0/go.mod h1:5d155H8EC5ewegao9A4PUTMNPZaq+TbOzkJJZ4vrXeI= -github.com/hashicorp/hcl/v2 v2.13.0 h1:0Apadu1w6M11dyGFxWnmhhcMjkbAiKCv7G1r/2QgCNc= -github.com/hashicorp/hcl/v2 v2.13.0/go.mod h1:e4z5nxYlWNPdDSNYX+ph14EvWYMFm3eP0zIUqPc2jr0= +github.com/hashicorp/hcl/v2 v2.14.0 h1:jX6+Q38Ly9zaAJlAjnFVyeNSNCKKW8D0wvyg7vij5Wc= +github.com/hashicorp/hcl/v2 v2.14.0/go.mod h1:e4z5nxYlWNPdDSNYX+ph14EvWYMFm3eP0zIUqPc2jr0= github.com/hashicorp/logutils v1.0.0 h1:dLEQVugN8vlakKOUE3ihGLTZJRB4j+M2cdTm/ORI65Y= github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= -github.com/hashicorp/terraform-exec v0.17.2 h1:EU7i3Fh7vDUI9nNRdMATCEfnm9axzTnad8zszYZ73Go= -github.com/hashicorp/terraform-exec v0.17.2/go.mod h1:tuIbsL2l4MlwwIZx9HPM+LOV9vVyEfBYu2GsO1uH3/8= +github.com/hashicorp/terraform-exec v0.17.3 h1:MX14Kvnka/oWGmIkyuyvL6POx25ZmKrjlaclkx3eErU= +github.com/hashicorp/terraform-exec v0.17.3/go.mod h1:+NELG0EqQekJzhvikkeQsOAZpsw0cv/03rbeQJqscAI= github.com/hashicorp/terraform-json v0.14.0 h1:sh9iZ1Y8IFJLx+xQiKHGud6/TSUCM0N8e17dKDpqV7s= github.com/hashicorp/terraform-json v0.14.0/go.mod h1:5A9HIWPkk4e5aeeXIBbkcOvaZbIYnAIkEyqP2pNSckM= github.com/hashicorp/terraform-plugin-docs v0.13.0 h1:6e+VIWsVGb6jYJewfzq2ok2smPzZrt1Wlm9koLeKazY= @@ -148,8 +148,8 @@ github.com/hashicorp/terraform-plugin-go v0.14.0 h1:ttnSlS8bz3ZPYbMb84DpcPhY4F5D github.com/hashicorp/terraform-plugin-go v0.14.0/go.mod h1:2nNCBeRLaenyQEi78xrGrs9hMbulveqG/zDMQSvVJTE= github.com/hashicorp/terraform-plugin-log v0.7.0 h1:SDxJUyT8TwN4l5b5/VkiTIaQgY6R+Y2BQ0sRZftGKQs= github.com/hashicorp/terraform-plugin-log v0.7.0/go.mod h1:p4R1jWBXRTvL4odmEkFfDdhUjHf9zcs/BCoNHAc7IK4= -github.com/hashicorp/terraform-plugin-sdk/v2 v2.21.0 h1:eIJjFlI4k6BMso6Wq/bq56U0RukXc4JbwJJ8Oze2/tg= -github.com/hashicorp/terraform-plugin-sdk/v2 v2.21.0/go.mod h1:mYPs/uchNcBq7AclQv9QUtSf9iNcfp1Ag21jqTlDf2M= +github.com/hashicorp/terraform-plugin-sdk/v2 v2.22.0 h1:MzfNfrheTt24xbEbA4npUSbX3GYu4xjXS7czcpJFyQY= +github.com/hashicorp/terraform-plugin-sdk/v2 v2.22.0/go.mod h1:q1XKSxXg9nDmhV0IvNZNZxe3gcTAHzMqrjs8wX1acng= github.com/hashicorp/terraform-registry-address v0.0.0-20220623143253-7d51757b572c h1:D8aRO6+mTqHfLsK/BC3j5OAoogv1WLRWzY1AaTo3rBg= github.com/hashicorp/terraform-registry-address v0.0.0-20220623143253-7d51757b572c/go.mod h1:Wn3Na71knbXc1G8Lh+yu/dQWWJeFQEpDeJMtWMtlmNI= github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 h1:HKLsbzeOsfXmKNpr3GiT18XAblV0BjCbzL8KQAMZGa0= @@ -252,8 +252,9 @@ github.com/xanzy/ssh-agent v0.3.0 h1:wUMzuKtKilRgBAD1sUb8gOwwRr2FGoBVumcjoOACClI github.com/xanzy/ssh-agent v0.3.0/go.mod h1:3s9xbODqPuuhK9JV1R321M/FlMZSBvE5aY6eAcqrDh0= github.com/zclconf/go-cty v1.1.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= -github.com/zclconf/go-cty v1.10.0 h1:mp9ZXQeIcN8kAwuqorjH+Q+njbJKjLrvB2yIh4q7U+0= github.com/zclconf/go-cty v1.10.0/go.mod h1:vVKLxnk3puL4qRAv72AO+W99LUD4da90g3uUAzyuvAk= +github.com/zclconf/go-cty v1.11.0 h1:726SxLdi2SDnjY+BStqB9J1hNp4+2WlzyXLuimibIe0= +github.com/zclconf/go-cty v1.11.0/go.mod h1:s9IfD1LK5ccNMSWCVFCE2rJfHiZgi7JijgeWIMfhLvA= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8= go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI= golang.org/x/crypto v0.0.0-20190219172222-a4c6cb3142f2/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go index ea218329..e640cda0 100644 --- a/internal/provider/resource_password_test.go +++ b/internal/provider/resource_password_test.go @@ -2586,6 +2586,7 @@ func composeImportStateCheck(fs ...resource.ImportStateCheckFunc) resource.Impor } } +//nolint:unparam func testExtractResourceAttrInstanceState(attributeName string, attributeValue *string) resource.ImportStateCheckFunc { return func(is []*terraform.InstanceState) error { if len(is) != 1 { From b2c170a18b941967058be4021f458998ae89e84f Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 8 Sep 2022 15:21:51 +0100 Subject: [PATCH 9/9] Update CHANGELOG release date (#302) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42a4c3af..e42b69f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## 3.4.3 (Unreleased) +## 3.4.3 (September 08, 2022) NOTES: