Use UnmarkDeepWithPaths and MarkWithPaths

Updates existing code to use the new Value
methods for unmarking/marking and removes
panics/workarounds in cty marshall methods

command/format/diff.go

@@ -126,16 +126,11 @@ func ResourceChange(
 	changeV.Change.After = objchange.NormalizeObjectFromLegacySDK(changeV.Change.After, schema)
 
 	// Now that the change is decoded, add back the marks at the defined paths
-	// change.Markinfo
-	if len(change.ValMarks.Path) != 0 {
-		changeV.Change.After, _ = cty.Transform(changeV.Change.After, func(p cty.Path, v cty.Value) (cty.Value, error) {
-			if p.Equals(change.ValMarks.Path) {
-				// TODO The mark is at change.Markinfo.Marks and it would be proper
-				// to iterate through that set here
-				return v.Mark("sensitive"), nil
+	if len(change.BeforeValMarks) > 0 {
+		changeV.Change.Before = changeV.Change.Before.MarkWithPaths(change.BeforeValMarks)
 	}
-			return v, nil
-		})
+	if len(change.AfterValMarks) > 0 {
+		changeV.Change.After = changeV.Change.After.MarkWithPaths(change.AfterValMarks)
 	}
 
 	bodyWritten := p.writeBlockBodyDiff(schema, changeV.Before, changeV.After, 6, path)

plans/changes.go

@@ -337,11 +337,24 @@ type Change struct {
 // to call the corresponding Encode method of that struct rather than working
 // directly with its embedded Change.
 func (c *Change) Encode(ty cty.Type) (*ChangeSrc, error) {
-	beforeDV, err := NewDynamicValue(c.Before, ty)
+	// Storing unmarked values so that we can encode unmarked values
+	// and save the PathValueMarks for re-marking the values later
+	var beforeVM, afterVM []cty.PathValueMarks
+	unmarkedBefore := c.Before
+	unmarkedAfter := c.After
+
+	if c.Before.ContainsMarked() {
+		unmarkedBefore, beforeVM = c.Before.UnmarkDeepWithPaths()
+	}
+	beforeDV, err := NewDynamicValue(unmarkedBefore, ty)
 	if err != nil {
 		return nil, err
 	}
-	afterDV, err, marks := NewDynamicValueMarks(c.After, ty)
+
+	if c.After.ContainsMarked() {
+		unmarkedAfter, afterVM = c.After.UnmarkDeepWithPaths()
+	}
+	afterDV, err := NewDynamicValue(unmarkedAfter, ty)
 	if err != nil {
 		return nil, err
 	}
@@ -350,6 +363,7 @@ func (c *Change) Encode(ty cty.Type) (*ChangeSrc, error) {
 		Action:         c.Action,
 		Before:         beforeDV,
 		After:          afterDV,
-		ValMarks: marks,
+		BeforeValMarks: beforeVM,
+		AfterValMarks:  afterVM,
 	}, nil
 }

plans/changes_src.go

@@ -6,7 +6,6 @@ import (
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/states"
 	"github.com/zclconf/go-cty/cty"
-	ctymsgpack "github.com/zclconf/go-cty/cty/msgpack"
 )
 
 // ResourceInstanceChangeSrc is a not-yet-decoded ResourceInstanceChange.
@@ -159,7 +158,7 @@ type ChangeSrc struct {
 	Before, After DynamicValue
 
 	// Marked Paths
-	ValMarks *ctymsgpack.MarkInfo
+	BeforeValMarks, AfterValMarks []cty.PathValueMarks
 }
 
 // Decode unmarshals the raw representations of the before and after values

plans/dynamic_value.go

@@ -55,26 +55,6 @@ func NewDynamicValue(val cty.Value, ty cty.Type) (DynamicValue, error) {
 	return DynamicValue(buf), nil
 }
 
-// NewDynamicValueMarks returns a new dynamic value along with a
-// associated marking info for the value
-func NewDynamicValueMarks(val cty.Value, ty cty.Type) (DynamicValue, error, *ctymsgpack.MarkInfo) {
-	// If we're given cty.NilVal (the zero value of cty.Value, which is
-	// distinct from a typed null value created by cty.NullVal) then we'll
-	// assume the caller is trying to represent the _absense_ of a value,
-	// and so we'll return a nil DynamicValue.
-	if val == cty.NilVal {
-		return DynamicValue(nil), nil, nil
-	}
-
-	// Currently our internal encoding is msgpack, via ctymsgpack.
-	buf, err, marks := ctymsgpack.MarshalWithMarks(val, ty)
-	if err != nil {
-		return nil, err, marks
-	}
-
-	return DynamicValue(buf), nil, marks
-}
-
 // Decode retrieves the effective value from the receiever by interpreting the
 // serialized form against the given type constraint. For correct results,
 // the type constraint must match (or be consistent with) the one that was

states/instance_object.go

@@ -19,6 +19,9 @@ type ResourceInstanceObject struct {
 	// Terraform.
 	Value cty.Value
 
+	// PathValueMarks is a slice of paths and value marks of the value
+	PathValueMarks []cty.PathValueMarks
+
 	// Private is an opaque value set by the provider when this object was
 	// last created or updated. Terraform Core does not use this value in
 	// any way and it is not exposed anywhere in the user interface, so
@@ -98,7 +101,14 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res
 	// and raise an error about that.
 	val := cty.UnknownAsNull(o.Value)
 
-	src, err := ctyjson.Marshal(val, ty)
+	// If it contains marks, dump those now
+	unmarked := val
+	if val.ContainsMarked() {
+		var pvm []cty.PathValueMarks
+		unmarked, pvm = val.UnmarkDeepWithPaths()
+		o.PathValueMarks = pvm
+	}
+	src, err := ctyjson.Marshal(unmarked, ty)
 	if err != nil {
 		return nil, err
 	}

terraform/eval_diff.go

@@ -141,23 +141,17 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, diags.Err()
 	}
 
-	var markedPath cty.Path
+	// Create an unmarked version of our config val, defaulting
+	// to the configVal so we don't do the work of unmarking unless
+	// necessary
+	unmarkedConfigVal := configVal
+	var unmarkedPaths []cty.PathValueMarks
 	// var marks cty.ValueMarks
 	if configVal.ContainsMarked() {
 		// store the marked values so we can re-mark them later after
 		// we've sent things over the wire. Right now this stores
 		// one path for proof of concept, but we should store multiple
-		cty.Walk(configVal, func(p cty.Path, v cty.Value) (bool, error) {
-			if v.IsMarked() {
-				markedPath = p
-				return false, nil
-				// marks = v.Marks()
-			}
-			return true, nil
-		})
-		// Unmark the value for sending over the wire
-		// to providers as marks cannot be serialized
-		configVal, _ = configVal.UnmarkDeep()
+		unmarkedConfigVal, unmarkedPaths = configVal.UnmarkDeepWithPaths()
 	}
 
 	metaConfigVal := cty.NullVal(cty.DynamicPseudoType)
@@ -203,7 +197,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		priorVal = cty.NullVal(schema.ImpliedType())
 	}
 
-	proposedNewVal := objchange.ProposedNewObject(schema, priorVal, configVal)
+	proposedNewVal := objchange.ProposedNewObject(schema, priorVal, unmarkedConfigVal)
 
 	// Call pre-diff hook
 	if !n.Stub {
@@ -222,7 +216,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 	validateResp := provider.ValidateResourceTypeConfig(
 		providers.ValidateResourceTypeConfigRequest{
 			TypeName: n.Addr.Resource.Type,
-			Config:   configVal,
+			Config:   unmarkedConfigVal,
 		},
 	)
 	if validateResp.Diagnostics.HasErrors() {
@@ -242,7 +236,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 
 	resp := provider.PlanResourceChange(providers.PlanResourceChangeRequest{
 		TypeName:         n.Addr.Resource.Type,
-		Config:           configVal,
+		Config:           unmarkedConfigVal,
 		PriorState:       priorVal,
 		ProposedNewState: proposedNewVal,
 		PriorPrivate:     priorPrivate,
@@ -255,14 +249,9 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 
 	plannedNewVal := resp.PlannedState
 
-	// Add the mark back to the planned new value
-	if len(markedPath) != 0 {
-		plannedNewVal, _ = cty.Transform(plannedNewVal, func(p cty.Path, v cty.Value) (cty.Value, error) {
-			if p.Equals(markedPath) {
-				return v.Mark("sensitive"), nil
-			}
-			return v, nil
-		})
+	// Add the marks back to the planned new value
+	if configVal.ContainsMarked() {
+		plannedNewVal = plannedNewVal.MarkWithPaths(unmarkedPaths)
 	}
 
 	plannedPrivate := resp.PlannedPrivate

vendor/github.com/zclconf/go-cty/cty/json/marshal.go

@@ -10,8 +10,7 @@ import (
 
 func marshal(val cty.Value, t cty.Type, path cty.Path, b *bytes.Buffer) error {
 	if val.IsMarked() {
-		// For now, dump the marks when serializing JSON for POC purposes
-		val, _ = val.UnmarkDeep()
+		return path.NewErrorf("value has marks, so it cannot be serialized as JSON")
 	}
 
 	// If we're going to decode as DynamicPseudoType then we need to save

vendor/github.com/zclconf/go-cty/cty/marks.go

@@ -67,6 +67,23 @@ func (m ValueMarks) GoString() string {
 	return s.String()
 }
 
+// PathValueMarks is a structure that enables tracking marks
+// and the paths where they are located in one type
+type PathValueMarks struct {
+	Path  Path
+	Marks ValueMarks
+}
+
+func (p PathValueMarks) Equal(o PathValueMarks) bool {
+	if !p.Path.Equals(o.Path) {
+		return false
+	}
+	if !p.Marks.Equal(o.Marks) {
+		return false
+	}
+	return true
+}
+
 // IsMarked returns true if and only if the receiving value carries at least
 // one mark. A marked value cannot be used directly with integration methods
 // without explicitly unmarking it (and retrieving the markings) first.
@@ -174,6 +191,21 @@ func (val Value) Mark(mark interface{}) Value {
 	}
 }
 
+// MarkWithPaths accepts a slice of PathValueMarks to apply
+// marker particular paths
+func (val Value) MarkWithPaths(pvm []PathValueMarks) Value {
+	ret, _ := Transform(val, func(p Path, v Value) (Value, error) {
+		for _, path := range pvm {
+			if p.Equals(path.Path) {
+				return v.WithMarks(path.Marks), nil
+
+			}
+		}
+		return v, nil
+	})
+	return ret
+}
+
 // Unmark separates the marks of the receiving value from the value itself,
 // removing a new unmarked value and a map (representing a set) of the marks.
 //
@@ -209,6 +241,18 @@ func (val Value) UnmarkDeep() (Value, ValueMarks) {
 	return ret, marks
 }
 
+func (val Value) UnmarkDeepWithPaths() (Value, []PathValueMarks) {
+	var marks []PathValueMarks
+	ret, _ := Transform(val, func(p Path, v Value) (Value, error) {
+		unmarkedV, valueMarks := v.Unmark()
+		if v.IsMarked() {
+			marks = append(marks, PathValueMarks{p, valueMarks})
+		}
+		return unmarkedV, nil
+	})
+	return ret, marks
+}
+
 func (val Value) unmarkForce() Value {
 	unw, _ := val.Unmark()
 	return unw

vendor/github.com/zclconf/go-cty/cty/msgpack/marshal.go

@@ -41,39 +41,9 @@ func Marshal(val cty.Value, ty cty.Type) ([]byte, error) {
 	return buf.Bytes(), nil
 }
 
-// This type should (likely) be a map of paths
-// and marks, so that multiple marks can be found
-// in case of a value containing multiple marked values
-type MarkInfo struct {
-	Marks cty.ValueMarks
-	Path  cty.Path
-}
-
-func MarshalWithMarks(val cty.Value, ty cty.Type) ([]byte, error, *MarkInfo) {
-	markInfo := MarkInfo{}
-	if val.ContainsMarked() {
-		// store the marked values so we can re-mark them later after
-		// we've sent things over the wire
-		cty.Walk(val, func(p cty.Path, v cty.Value) (bool, error) {
-			if v.IsMarked() {
-				markInfo.Path = p
-				markInfo.Marks = v.Marks()
-			}
-			return true, nil
-		})
-		val, _ = val.UnmarkDeep()
-	}
-	by, err := Marshal(val, ty)
-	if err != nil {
-		return nil, err, &markInfo
-	}
-
-	return by, nil, &markInfo
-}
-
 func marshal(val cty.Value, ty cty.Type, path cty.Path, enc *msgpack.Encoder) error {
 	if val.IsMarked() {
-		return path.NewErrorf("value has marks, so it cannot be seralized")
+		return path.NewErrorf("value has marks, so it cannot be serialized")
 	}
 
 	// If we're going to decode as DynamicPseudoType then we need to save