ephemeral: validate provider responses for write-only attributes

internal/lang/ephemeral/validate.go

@@ -0,0 +1,53 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package ephemeral
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/terraform/internal/addrs"
+	"github.com/hashicorp/terraform/internal/configs/configschema"
+	"github.com/hashicorp/terraform/internal/tfdiags"
+	"github.com/zclconf/go-cty/cty"
+)
+
+func ValidateWriteOnlyAttributes(newVal cty.Value, schema *configschema.Block, provider addrs.AbsProviderConfig, addr addrs.AbsResourceInstance) (diags tfdiags.Diagnostics) {
+	if writeOnlyPaths := NonNullWriteOnlyPaths(newVal, schema, nil); len(writeOnlyPaths) != 0 {
+		for _, p := range writeOnlyPaths {
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Write-only attribute set",
+				fmt.Sprintf(
+					"Provider %q set the write-only attribute \"%s%s\". Write-only attributes must not be set by the provider, so this is a bug in the provider, which should be reported in the provider's own issue tracker.",
+					provider.String(), addr.String(), tfdiags.FormatCtyPath(p),
+				),
+			))
+		}
+	}
+	return diags
+}
+
+// NonNullWriteOnlyPaths returns a list of paths to attributes that are write-only
+// and non-null in the given value.
+func NonNullWriteOnlyPaths(val cty.Value, schema *configschema.Block, p cty.Path) (paths []cty.Path) {
+	if schema == nil {
+		return paths
+	}
+
+	for name, attr := range schema.Attributes {
+		attrPath := append(p, cty.GetAttrStep{Name: name})
+		attrVal, _ := attrPath.Apply(val)
+		if attr.WriteOnly && !attrVal.IsNull() {
+			paths = append(paths, attrPath)
+		}
+	}
+
+	for name, blockS := range schema.BlockTypes {
+		blockPath := append(p, cty.GetAttrStep{Name: name})
+		x := NonNullWriteOnlyPaths(val, &blockS.Block, blockPath)
+		paths = append(paths, x...)
+	}
+
+	return paths
+}

internal/lang/ephemeral/validate_test.go

@@ -0,0 +1,146 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package ephemeral
+
+import (
+	"testing"
+
+	"github.com/hashicorp/terraform/internal/configs/configschema"
+	"github.com/zclconf/go-cty/cty"
+)
+
+func TestNonNullWriteOnlyPaths(t *testing.T) {
+	for name, tc := range map[string]struct {
+		val    cty.Value
+		schema *configschema.Block
+
+		expectedPaths []cty.Path
+	}{
+		"no write-only attributes": {
+			val: cty.ObjectVal(map[string]cty.Value{
+				"id": cty.StringVal("i-abc123"),
+			}),
+			schema: &configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"id": {
+						Type: cty.String,
+					},
+				},
+			},
+		},
+
+		"write-only attribute with null value": {
+			val: cty.ObjectVal(map[string]cty.Value{
+				"id": cty.NullVal(cty.String),
+			}),
+			schema: &configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"id": {
+						Type:      cty.String,
+						Optional:  true,
+						WriteOnly: true,
+					},
+				},
+			},
+		},
+
+		"write-only attribute with non-null value": {
+			val: cty.ObjectVal(map[string]cty.Value{
+				"valid": cty.NullVal(cty.String),
+				"id":    cty.StringVal("i-abc123"),
+			}),
+			schema: &configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"valid": {
+						Type:      cty.String,
+						Optional:  true,
+						WriteOnly: true,
+					},
+					"id": {
+						Type:      cty.String,
+						Optional:  true,
+						WriteOnly: true,
+					},
+				},
+			},
+			expectedPaths: []cty.Path{cty.GetAttrPath("id")},
+		},
+
+		"write-only attributes in blocks": {
+			val: cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.ObjectVal(map[string]cty.Value{
+					"valid-write-only": cty.NullVal(cty.String),
+					"valid":            cty.StringVal("valid"),
+					"id":               cty.StringVal("i-abc123"),
+					"bar": cty.ObjectVal(map[string]cty.Value{
+						"valid-write-only": cty.NullVal(cty.String),
+						"valid":            cty.StringVal("valid"),
+						"id":               cty.StringVal("i-abc123"),
+					}),
+				}),
+			}),
+			schema: &configschema.Block{
+				BlockTypes: map[string]*configschema.NestedBlock{
+					"foo": {
+						Block: configschema.Block{
+							Attributes: map[string]*configschema.Attribute{
+								"valid-write-only": {
+									Type:      cty.String,
+									Optional:  true,
+									WriteOnly: true,
+								},
+								"valid": {
+									Type:     cty.String,
+									Optional: true,
+								},
+								"id": {
+									Type:      cty.String,
+									Optional:  true,
+									WriteOnly: true,
+								},
+							},
+							BlockTypes: map[string]*configschema.NestedBlock{
+								"bar": {
+									Block: configschema.Block{
+										Attributes: map[string]*configschema.Attribute{
+											"valid-write-only": {
+												Type:      cty.String,
+												Optional:  true,
+												WriteOnly: true,
+											},
+											"valid": {
+												Type:     cty.String,
+												Optional: true,
+											},
+											"id": {
+												Type:      cty.String,
+												Optional:  true,
+												WriteOnly: true,
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedPaths: []cty.Path{cty.GetAttrPath("foo").GetAttr("id"), cty.GetAttrPath("foo").GetAttr("bar").GetAttr("id")},
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			paths := NonNullWriteOnlyPaths(tc.val, tc.schema, nil)
+
+			if len(paths) != len(tc.expectedPaths) {
+				t.Fatalf("expected %d write-only paths, got %d", len(tc.expectedPaths), len(paths))
+			}
+
+			for i, path := range paths {
+				if !path.Equals(tc.expectedPaths[i]) {
+					t.Fatalf("expected path %#v, got %#v", tc.expectedPaths[i], path)
+				}
+			}
+		})
+	}
+}

internal/providers/testing/provider_mock.go

@@ -368,8 +368,38 @@ func (p *MockProvider) ReadResource(r providers.ReadResourceRequest) (resp provi
 		return resp
 	}
 
-	// otherwise just return the same state we received
-	resp.NewState = r.PriorState
+	// otherwise just return the same state we received without the write-only attributes
+	// since there are old tests without a schema we default to the prior state
+	if schema, ok := p.getProviderSchema().ResourceTypes[r.TypeName]; ok {
+
+		newVal, err := cty.Transform(r.PriorState, func(path cty.Path, v cty.Value) (cty.Value, error) {
+			// We're only concerned with known null values, which can be computed
+			// by the provider.
+			if !v.IsKnown() {
+				return v, nil
+			}
+
+			attrSchema := schema.Block.AttributeByPath(path)
+			if attrSchema == nil {
+				// this is an intermediate path which does not represent an attribute
+				return v, nil
+			}
+
+			// Write-only attributes always return null
+			if attrSchema.WriteOnly {
+				return cty.NullVal(v.Type()), nil
+			}
+
+			return v, nil
+		})
+		if err != nil {
+			resp.Diagnostics = resp.Diagnostics.Append(err)
+		}
+		resp.NewState = newVal
+	} else {
+		resp.NewState = r.PriorState
+	}
+
 	resp.Private = r.Private
 	return resp
 }

internal/refactoring/cross_provider_move.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/configs/configschema"
+	"github.com/hashicorp/terraform/internal/lang/ephemeral"
 	"github.com/hashicorp/terraform/internal/providers"
 	"github.com/hashicorp/terraform/internal/states"
 	"github.com/hashicorp/terraform/internal/tfdiags"
@@ -153,6 +154,14 @@ func (move *crossTypeMove) applyCrossTypeMove(stmt *MoveStatement, source, targe
 		return diags
 	}
 
+	// Providers are supposed to return null values for all write-only attributes
+	writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(resp.TargetState, move.targetResourceSchema, move.targetProviderAddr, target)
+	diags = diags.Append(writeOnlyDiags)
+
+	if writeOnlyDiags.HasErrors() {
+		return diags
+	}
+
 	if resp.TargetState == cty.NilVal {
 		diags = diags.Append(&hcl.Diagnostic{
 			Severity: hcl.DiagError,