From 8166151182922540630cc0720e303c78b3ba773b Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 24 Aug 2023 17:50:45 -0400 Subject: [PATCH 1/6] initial commit --- .../resources/data-consistency-errors.mdx | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx index f555e08ca09..6e826a6d023 100644 --- a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx +++ b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx @@ -110,3 +110,140 @@ This occurs if the attribute schema definition is `Optional: true` without `Comp If the value is expected to never be set by configuration, the schema attribute `Optional: true` flag should be replaced with `Computed: true`. Otherwise, this may not be resolvable when the resource is implemented with `terraform-plugin-sdk`. Having `Optional: true` while also setting the attribute's `Computed: true` flag in the schema will also enable this SDK's behavior of keeping the prior state value if the configuration value is removed (set to null) during an update. That SDK behavior is unavoidable. This SDK will also raise an implementation error if both `Computed: true` and `Default` are set, since the value will never reset to the default value because of that behavior. If that behavior is not acceptable, this error is unavoidable until the resource is migrated to terraform-plugin-framework, which does not have implicit behaviors when enabling the `Computed: true` flag and instead provider developers are expected to decide whether the prior state preservation behavior should occur or not by using the `UseStateForUnknown` schema plan modifier. + +### Planned Value does not match Config Value + +If the resource is raising this type of error or warning log: + +```text +TIMESTAMP [WARN] Provider "TYPE" produced an invalid plan for ADDRESS, but we are tolerating it because it is using the legacy plugin SDK. + The following problems may be the cause of any confusing errors from downstream operations: + - .ATTRIBUTE: planned value cty.StringVal("VALUE") does not match config value cty.StringVal("value") +``` + +This occurs for attribute schema definitions that are `Optional: true` and `Computed: true`; where the planned value, returned by the provider, does not match the attribute's config value or prior state value. For example, value's for an attribute of type string must match byte-for-byte. + +An example root cause of this issue could be from API normalization, such as a JSON string being returned from an API and stored in state with differing whitespace then what was originally in config. + +#### SDKv2 Example + +Here is an example of an SDKv2 resource schema and terraform config that simulates this data consistency error: + +```go +func thingResource() *schema.Resource { + return &schema.Resource{ + // ... + Schema: map[string]*schema.Schema{ + "word": { + Type: schema.TypeString, + Optional: true, + Computed: true, + StateFunc: func(word interface{}) string { + // This simulates an API returning the 'word' attribute as all uppercase, + // which is stored to state even if it doesn't match the config or prior value. + return strings.ToUpper(word.(string)) + }, + }, + }, + } +} +``` + +```hcl +resource "examplecloud_thing" "this" { + word = "value" +} +``` + +A warning log will be produced and the resulting state after applying a new resource would look like: +```json +{ + //... + "resources": [ + { + "mode": "managed", + "type": "examplecloud_thing", + "name": "this", + "instances": [ + { + "attributes": { + // This is invalid! Config has this stored as "value" + "word": "VALUE" + }, + } + ] + } + ], +} +``` + +#### Migrating to Plugin Framework + +When a resource with this behavior and prior state is migrated to Plugin Framework, depending on the business logic, you could potentially see: + +- Resource drift in the plan; Terraform will always detect a change between the config and state value. If no [modification](/terraform/plugin/framework/resources/plan-modification) is implemented, you could see drift in the plan: +```hcl +resource "examplecloud_thing" "this" { + word = "value" +} +``` +```text +examplecloud_thing.this: Refreshing state... + +Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols: + ~ update in-place + +Terraform will perform the following actions: + + # examplecloud_thing.this will be updated in-place + ~ resource "examplecloud_thing" "this" { + ~ word = "VALUE" -> "value" + } + +Plan: 0 to add, 1 to change, 0 to destroy. +``` +- If you mimic the original SDKv2 behavior of storing a different value from config/prior value into state in the `Update` method, you will see an error like below: +```text +examplecloud_thing.this: Modifying... +╷ +│ Error: Provider produced inconsistent result after apply +│ +│ When applying changes to examplecloud_thing.this, provider "provider[\"TYPE\"]" produced an unexpected +│ new value: .word: was cty.StringVal("value"), but now cty.StringVal("VALUE"). +│ +│ This is a bug in the provider, which should be reported in the provider's own issue tracker. +``` + +#### Recommended Solution +To solve this issue, the provider code must preserve the config value or prior state value when producing the new state. The recommended way to implement this logic is by creating a [custom type](/terraform/plugin/framework/handling-data/types/custom) with [semantic equality logic](/terraform/plugin/framework/handling-data/types/custom#semantic-equality). A custom type can be shared across multiple resource attributes and will ensure that the semantic equality logic is invoked during the `Read`, `Create`, and `Update` methods respectively. + +For the above example, the semantic equality implementation below would resolve the resource drift and error: +```go +type CaseInsensitive struct { + basetypes.StringValue +} + +// ... custom value type implementation + +// StringSemanticEquals returns true if the given string value is semantically equal to the current string value. (case-insensitive) +func (v CaseInsensitive) StringSemanticEquals(_ context.Context, newValuable basetypes.StringValuable) (bool, diag.Diagnostics) { + var diags diag.Diagnostics + + newValue, ok := newValuable.(CaseInsensitive) + if !ok { + diags.AddError( + "Semantic Equality Check Error", + "An unexpected value type was received while performing semantic equality checks. "+ + "Please report this to the provider developers.\n\n"+ + "Expected Value Type: "+fmt.Sprintf("%T", v)+"\n"+ + "Got Value Type: "+fmt.Sprintf("%T", newValuable), + ) + + return false, diags + } + + return strings.EqualFold(newValue.ValueString(), v.ValueString()), diags +} +``` + +More examples of custom type/value logic with semantic equality can be found in the [common custom type](/terraform/plugin/framework/handling-data/types/custom#common-custom-types) repositories. From 016f347be6c62fc6172c073f45795f9cac9a356f Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 29 Aug 2023 12:58:42 -0400 Subject: [PATCH 2/6] added SDKv2 solution --- .../resources/data-consistency-errors.mdx | 86 +++++-------------- 1 file changed, 23 insertions(+), 63 deletions(-) diff --git a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx index 6e826a6d023..1951216afe5 100644 --- a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx +++ b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx @@ -139,8 +139,8 @@ func thingResource() *schema.Resource { Optional: true, Computed: true, StateFunc: func(word interface{}) string { - // This simulates an API returning the 'word' attribute as all uppercase, - // which is stored to state even if it doesn't match the config or prior value. + // This simulates an API returning the 'word' attribute as all uppercase, + // which is stored to state even if it doesn't match the config or prior value. return strings.ToUpper(word.(string)) }, }, @@ -177,73 +177,33 @@ A warning log will be produced and the resulting state after applying a new reso } ``` -#### Migrating to Plugin Framework +#### Recommended SDKv2 Solution +To solve this issue, the provider code must preserve the config value or prior state value when producing the new state. In the example above, the solution is to remove the usage of [`StateFunc`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.StateFunc). -When a resource with this behavior and prior state is migrated to Plugin Framework, depending on the business logic, you could potentially see: +In use-cases where a value is normalized on return from the remote API, SDKv2 resource can implement [`DiffSuppressFunc`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.DiffSuppressFunc) to preserve the prior state value, in the case of inconsequential differences and [`DiffSuppressOnRefresh: true`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.DiffSuppressOnRefresh) to ensure this logic preserves the prior value during refresh. -- Resource drift in the plan; Terraform will always detect a change between the config and state value. If no [modification](/terraform/plugin/framework/resources/plan-modification) is implemented, you could see drift in the plan: -```hcl -resource "examplecloud_thing" "this" { - word = "value" -} -``` -```text -examplecloud_thing.this: Refreshing state... - -Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols: - ~ update in-place - -Terraform will perform the following actions: - - # examplecloud_thing.this will be updated in-place - ~ resource "examplecloud_thing" "this" { - ~ word = "VALUE" -> "value" - } - -Plan: 0 to add, 1 to change, 0 to destroy. -``` -- If you mimic the original SDKv2 behavior of storing a different value from config/prior value into state in the `Update` method, you will see an error like below: -```text -examplecloud_thing.this: Modifying... -╷ -│ Error: Provider produced inconsistent result after apply -│ -│ When applying changes to examplecloud_thing.this, provider "provider[\"TYPE\"]" produced an unexpected -│ new value: .word: was cty.StringVal("value"), but now cty.StringVal("VALUE"). -│ -│ This is a bug in the provider, which should be reported in the provider's own issue tracker. -``` - -#### Recommended Solution -To solve this issue, the provider code must preserve the config value or prior state value when producing the new state. The recommended way to implement this logic is by creating a [custom type](/terraform/plugin/framework/handling-data/types/custom) with [semantic equality logic](/terraform/plugin/framework/handling-data/types/custom#semantic-equality). A custom type can be shared across multiple resource attributes and will ensure that the semantic equality logic is invoked during the `Read`, `Create`, and `Update` methods respectively. +Adding those functions to the above example would look like: -For the above example, the semantic equality implementation below would resolve the resource drift and error: ```go -type CaseInsensitive struct { - basetypes.StringValue -} - -// ... custom value type implementation - -// StringSemanticEquals returns true if the given string value is semantically equal to the current string value. (case-insensitive) -func (v CaseInsensitive) StringSemanticEquals(_ context.Context, newValuable basetypes.StringValuable) (bool, diag.Diagnostics) { - var diags diag.Diagnostics - - newValue, ok := newValuable.(CaseInsensitive) - if !ok { - diags.AddError( - "Semantic Equality Check Error", - "An unexpected value type was received while performing semantic equality checks. "+ - "Please report this to the provider developers.\n\n"+ - "Expected Value Type: "+fmt.Sprintf("%T", v)+"\n"+ - "Got Value Type: "+fmt.Sprintf("%T", newValuable), - ) +func thingResource() *schema.Resource { + return &schema.Resource{ + CreateContext: thingResourceCreate, + ReadContext: thingResourceRead, + UpdateContext: thingResourceUpdate, + DeleteContext: thingResourceDelete, - return false, diags + Schema: map[string]*schema.Schema{ + "word": { + Type: schema.TypeString, + Optional: true, + Computed: true, + DiffSuppressFunc: func(k, oldValue, newValue string, d *schema.ResourceData) bool { + return strings.EqualFold(oldValue, newValue) + }, + DiffSuppressOnRefresh: true, + }, + }, } - - return strings.EqualFold(newValue.ValueString(), v.ValueString()), diags } ``` -More examples of custom type/value logic with semantic equality can be found in the [common custom type](/terraform/plugin/framework/handling-data/types/custom#common-custom-types) repositories. From 429a76798c9ab0aac30beeb21eb9f3b6cde9632c Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 29 Aug 2023 13:03:29 -0400 Subject: [PATCH 3/6] weird spacing --- .../docs/plugin/sdkv2/resources/data-consistency-errors.mdx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx index 1951216afe5..e7da037a474 100644 --- a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx +++ b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx @@ -139,8 +139,8 @@ func thingResource() *schema.Resource { Optional: true, Computed: true, StateFunc: func(word interface{}) string { - // This simulates an API returning the 'word' attribute as all uppercase, - // which is stored to state even if it doesn't match the config or prior value. + // This simulates an API returning the 'word' attribute as all uppercase, + // which is stored to state even if it doesn't match the config or prior value. return strings.ToUpper(word.(string)) }, }, From 9bf580769f12de0db747ba27d942634f594ddc83 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Wed, 30 Aug 2023 11:44:11 -0400 Subject: [PATCH 4/6] Update website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx Co-authored-by: Brian Flad --- .../resources/data-consistency-errors.mdx | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx index e7da037a474..bb114b42d6d 100644 --- a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx +++ b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx @@ -155,27 +155,7 @@ resource "examplecloud_thing" "this" { } ``` -A warning log will be produced and the resulting state after applying a new resource would look like: -```json -{ - //... - "resources": [ - { - "mode": "managed", - "type": "examplecloud_thing", - "name": "this", - "instances": [ - { - "attributes": { - // This is invalid! Config has this stored as "value" - "word": "VALUE" - }, - } - ] - } - ], -} -``` +A warning log will be produced and the resulting state after applying a new resource will be `VALUE` instead of `value`. #### Recommended SDKv2 Solution To solve this issue, the provider code must preserve the config value or prior state value when producing the new state. In the example above, the solution is to remove the usage of [`StateFunc`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.StateFunc). From 7667c7fa9aa9e2041f3db66aa716e02e6b420393 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Wed, 30 Aug 2023 12:18:05 -0400 Subject: [PATCH 5/6] missing comma --- website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx index bb114b42d6d..f40ef8c68dd 100644 --- a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx +++ b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx @@ -160,7 +160,7 @@ A warning log will be produced and the resulting state after applying a new reso #### Recommended SDKv2 Solution To solve this issue, the provider code must preserve the config value or prior state value when producing the new state. In the example above, the solution is to remove the usage of [`StateFunc`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.StateFunc). -In use-cases where a value is normalized on return from the remote API, SDKv2 resource can implement [`DiffSuppressFunc`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.DiffSuppressFunc) to preserve the prior state value, in the case of inconsequential differences and [`DiffSuppressOnRefresh: true`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.DiffSuppressOnRefresh) to ensure this logic preserves the prior value during refresh. +In use-cases where a value is normalized on return from the remote API, SDKv2 resource can implement [`DiffSuppressFunc`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.DiffSuppressFunc) to preserve the prior state value, in the case of inconsequential differences, and [`DiffSuppressOnRefresh: true`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.DiffSuppressOnRefresh) to ensure this logic preserves the prior value during refresh. Adding those functions to the above example would look like: From 9fd4130bf80c3e9156e121cb0ad0349c211755eb Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Wed, 30 Aug 2023 12:18:44 -0400 Subject: [PATCH 6/6] whoops --- website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx index f40ef8c68dd..2db646bdde4 100644 --- a/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx +++ b/website/docs/plugin/sdkv2/resources/data-consistency-errors.mdx @@ -160,7 +160,7 @@ A warning log will be produced and the resulting state after applying a new reso #### Recommended SDKv2 Solution To solve this issue, the provider code must preserve the config value or prior state value when producing the new state. In the example above, the solution is to remove the usage of [`StateFunc`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.StateFunc). -In use-cases where a value is normalized on return from the remote API, SDKv2 resource can implement [`DiffSuppressFunc`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.DiffSuppressFunc) to preserve the prior state value, in the case of inconsequential differences, and [`DiffSuppressOnRefresh: true`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.DiffSuppressOnRefresh) to ensure this logic preserves the prior value during refresh. +In use-cases where a value is normalized on return from the remote API, SDKv2 resource can implement [`DiffSuppressFunc`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.DiffSuppressFunc) to preserve the prior state value in the case of inconsequential differences, and [`DiffSuppressOnRefresh: true`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#Schema.DiffSuppressOnRefresh) to ensure this logic preserves the prior value during refresh. Adding those functions to the above example would look like: