Fix crash when importing a resource with complex sensitive attributes (#34996)

internal/genconfig/generate_config.go

@@ -38,7 +38,6 @@ func GenerateResourceContents(addr addrs.AbsResourceInstance,
 		buf.WriteString(fmt.Sprintf("provider = %s\n", pc.StringCompact()))
 	}
 
-	stateVal = omitUnknowns(stateVal)
 	if stateVal.RawEquals(cty.NilVal) {
 		diags = diags.Append(writeConfigAttributes(addr, &buf, schema.Attributes, 2))
 		diags = diags.Append(writeConfigBlocks(addr, &buf, schema.BlockTypes, 2))
@@ -151,11 +150,17 @@ func writeConfigAttributesFromExisting(addr addrs.AbsResourceInstance, buf *stri
 				val = attrS.EmptyValue()
 			}
 			if val.Type() == cty.String {
+				// Before we inspect the string, take off any marks.
+				unmarked, marks := val.Unmark()
+
 				// SHAMELESS HACK: If we have "" for an optional value, assume
 				// it is actually null, due to the legacy SDK.
-				if !val.IsNull() && attrS.Optional && len(val.AsString()) == 0 {
-					val = attrS.EmptyValue()
+				if !unmarked.IsNull() && attrS.Optional && len(unmarked.AsString()) == 0 {
+					unmarked = attrS.EmptyValue()
 				}
+
+				// Before we carry on, add the marks back.
+				val = unmarked.WithMarks(marks)
 			}
 			if attrS.Sensitive || val.IsMarked() {
 				buf.WriteString("null # sensitive")
@@ -567,59 +572,3 @@ func ctyCollectionValues(val cty.Value) []cty.Value {
 
 	return ret
 }
-
-// omitUnknowns recursively walks the src cty.Value and returns a new cty.Value,
-// omitting any unknowns.
-//
-// The result also normalizes some types: all sequence types are turned into
-// tuple types and all mapping types are converted to object types, since we
-// assume the result of this is just going to be serialized as JSON (and thus
-// lose those distinctions) anyway.
-func omitUnknowns(val cty.Value) cty.Value {
-	ty := val.Type()
-	switch {
-	case val.IsNull():
-		return val
-	case !val.IsKnown():
-		return cty.NilVal
-	case ty.IsPrimitiveType():
-		return val
-	case ty.IsListType() || ty.IsTupleType() || ty.IsSetType():
-		var vals []cty.Value
-		it := val.ElementIterator()
-		for it.Next() {
-			_, v := it.Element()
-			newVal := omitUnknowns(v)
-			if newVal != cty.NilVal {
-				vals = append(vals, newVal)
-			} else if newVal == cty.NilVal {
-				// element order is how we correlate unknownness, so we must
-				// replace unknowns with nulls
-				vals = append(vals, cty.NullVal(v.Type()))
-			}
-		}
-		// We use tuple types always here, because the work we did above
-		// may have caused the individual elements to have different types,
-		// and we're doing this work to produce JSON anyway and JSON marshalling
-		// represents all of these sequence types as an array.
-		return cty.TupleVal(vals)
-	case ty.IsMapType() || ty.IsObjectType():
-		vals := make(map[string]cty.Value)
-		it := val.ElementIterator()
-		for it.Next() {
-			k, v := it.Element()
-			newVal := omitUnknowns(v)
-			if newVal != cty.NilVal {
-				vals[k.AsString()] = newVal
-			}
-		}
-		// We use object types always here, because the work we did above
-		// may have caused the individual elements to have different types,
-		// and we're doing this work to produce JSON anyway and JSON marshalling
-		// represents both of these mapping types as an object.
-		return cty.ObjectVal(vals)
-	default:
-		// Should never happen, since the above should cover all types
-		panic(fmt.Sprintf("omitUnknowns cannot handle %#v", val))
-	}
-}

internal/genconfig/generate_config_test.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/configs/configschema"
+	"github.com/hashicorp/terraform/internal/lang/marks"
 )
 
 func TestConfigGeneration(t *testing.T) {
@@ -536,6 +537,67 @@ resource "tfcoremock_simple_resource" "empty" {
 			expected: `
 resource "tfcoremock_simple_resource" "empty" {
   value = "[\"Hello\", \"World\""
+}`,
+		},
+		// Just try all the simple values with sensitive marks.
+		"sensitive_values": {
+			schema: &configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"string":       sensitiveAttribute(cty.String),
+					"empty_string": sensitiveAttribute(cty.String),
+					"number":       sensitiveAttribute(cty.Number),
+					"bool":         sensitiveAttribute(cty.Bool),
+					"object": sensitiveAttribute(cty.Object(map[string]cty.Type{
+						"nested": cty.String,
+					})),
+					"list": sensitiveAttribute(cty.List(cty.String)),
+					"map":  sensitiveAttribute(cty.Map(cty.String)),
+					"set":  sensitiveAttribute(cty.Set(cty.String)),
+				},
+			},
+			addr: addrs.AbsResourceInstance{
+				Module: addrs.RootModuleInstance,
+				Resource: addrs.ResourceInstance{
+					Resource: addrs.Resource{
+						Mode: addrs.ManagedResourceMode,
+						Type: "tfcoremock_sensitive_values",
+						Name: "values",
+					},
+					Key: addrs.NoKey,
+				},
+			},
+			provider: addrs.LocalProviderConfig{
+				LocalName: "tfcoremock",
+			},
+			value: cty.ObjectVal(map[string]cty.Value{
+				// Values that are sensitive will now be marked as such
+				"string":       cty.StringVal("Hello, world!").Mark(marks.Sensitive),
+				"empty_string": cty.StringVal("").Mark(marks.Sensitive),
+				"number":       cty.NumberIntVal(42).Mark(marks.Sensitive),
+				"bool":         cty.True.Mark(marks.Sensitive),
+				"object": cty.ObjectVal(map[string]cty.Value{
+					"nested": cty.StringVal("Hello, solar system!"),
+				}).Mark(marks.Sensitive),
+				"list": cty.ListVal([]cty.Value{
+					cty.StringVal("Hello, world!"),
+				}).Mark(marks.Sensitive),
+				"map": cty.MapVal(map[string]cty.Value{
+					"key": cty.StringVal("Hello, world!"),
+				}).Mark(marks.Sensitive),
+				"set": cty.SetVal([]cty.Value{
+					cty.StringVal("Hello, world!"),
+				}).Mark(marks.Sensitive),
+			}),
+			expected: `
+resource "tfcoremock_sensitive_values" "values" {
+  bool         = null # sensitive
+  empty_string = null # sensitive
+  list         = null # sensitive
+  map          = null # sensitive
+  number       = null # sensitive
+  object       = null # sensitive
+  set          = null # sensitive
+  string       = null # sensitive
 }`,
 		},
 	}
@@ -558,3 +620,11 @@ resource "tfcoremock_simple_resource" "empty" {
 		})
 	}
 }
+
+func sensitiveAttribute(t cty.Type) *configschema.Attribute {
+	return &configschema.Attribute{
+		Type:      t,
+		Optional:  true,
+		Sensitive: true,
+	}
+}

internal/terraform/context_plan_import_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/hashicorp/terraform/internal/configs/configschema"
 	"github.com/hashicorp/terraform/internal/plans"
 	"github.com/hashicorp/terraform/internal/providers"
+	testing_provider "github.com/hashicorp/terraform/internal/providers/testing"
 	"github.com/hashicorp/terraform/internal/states"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
@@ -1413,7 +1414,7 @@ func TestContext2Plan_importGenerateNone(t *testing.T) {
 import {
   for_each = []
   to   = test_object.a
-  id   = "123"
+  id   = "81ba7c97"
 }
 `,
 	})
@@ -1437,3 +1438,66 @@ import {
 		t.Fatal("expected no resource changes")
 	}
 }
+
+// This is a test for the issue raised in #34992
+func TestContext2Plan_importWithSensitives(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+import {
+  to = test_object.a
+  id = "123"
+}
+`,
+	})
+
+	p := &testing_provider.MockProvider{
+		GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{
+			ResourceTypes: map[string]providers.Schema{
+				"test_object": {
+					Block: &configschema.Block{
+						Attributes: map[string]*configschema.Attribute{
+							"sensitive_string": {
+								Type:      cty.String,
+								Sensitive: true,
+								Optional:  true,
+							},
+							"sensitive_list": {
+								Type:      cty.List(cty.String),
+								Sensitive: true,
+								Optional:  true,
+							},
+						},
+					},
+				},
+			},
+		},
+		ImportResourceStateFn: func(request providers.ImportResourceStateRequest) providers.ImportResourceStateResponse {
+			return providers.ImportResourceStateResponse{
+				ImportedResources: []providers.ImportedResource{
+					{
+						TypeName: "test_object",
+						State: cty.ObjectVal(map[string]cty.Value{
+							"sensitive_string": cty.StringVal("sensitive"),
+							"sensitive_list":   cty.ListVal([]cty.Value{cty.StringVal("hello"), cty.StringVal("world")}),
+						}),
+					},
+				},
+			}
+		},
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	// Just don't crash!
+	_, diags := ctx.Plan(m, states.NewState(), &PlanOpts{
+		Mode:               plans.NormalMode,
+		GenerateConfigPath: "generated.tf",
+	})
+	if diags.HasErrors() {
+		t.Fatalf("unexpected errors\n%s", diags.Err().Error())
+	}
+}