From 84d118e18f709ba4caa3f69ef0c49b065ae9621b Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Fri, 7 Aug 2020 11:59:06 -0400 Subject: [PATCH] Track sensitivity through evaluation Mark sensitivity on a value. However, when the value is encoded to send to the provider to produce a changeset we must remove the marks, so unmark the value and remark it with the saved path afterwards --- command/format/diff.go | 28 +++++++++++++---- plans/changes.go | 9 +++--- plans/changes_src.go | 4 +++ plans/dynamic_value.go | 20 +++++++++++++ terraform/eval_diff.go | 28 +++++++++++++++++ terraform/evaluate.go | 4 +++ .../zclconf/go-cty/cty/json/marshal.go | 3 +- .../zclconf/go-cty/cty/msgpack/marshal.go | 30 +++++++++++++++++++ 8 files changed, 116 insertions(+), 10 deletions(-) diff --git a/command/format/diff.go b/command/format/diff.go index 901c9a081..7fa94fada 100644 --- a/command/format/diff.go +++ b/command/format/diff.go @@ -125,10 +125,21 @@ func ResourceChange( changeV.Change.Before = objchange.NormalizeObjectFromLegacySDK(changeV.Change.Before, schema) changeV.Change.After = objchange.NormalizeObjectFromLegacySDK(changeV.Change.After, schema) - result := p.writeBlockBodyDiff(schema, changeV.Before, changeV.After, 6, path) - if result.bodyWritten { - p.buf.WriteString("\n") - p.buf.WriteString(strings.Repeat(" ", 4)) + // Now that the change is decoded, add back the marks at the defined paths + // change.Markinfo + 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 + } + return v, nil + }) + + bodyWritten := p.writeBlockBodyDiff(schema, changeV.Before, changeV.After, 6, path) + if bodyWritten { + buf.WriteString("\n") + buf.WriteString(strings.Repeat(" ", 4)) } buf.WriteString("}\n") @@ -586,6 +597,12 @@ func (p *blockBodyDiffPrinter) writeNestedBlockDiff(name string, label *string, } func (p *blockBodyDiffPrinter) writeValue(val cty.Value, action plans.Action, indent int) { + // Could check specifically for the sensitivity marker + if val.IsMarked() { + p.buf.WriteString("(sensitive)") + return + } + if !val.IsKnown() { p.buf.WriteString("(known after apply)") return @@ -1284,7 +1301,8 @@ func ctyGetAttrMaybeNull(val cty.Value, name string) cty.Value { // This allows us to avoid spurious diffs // until we introduce null to the SDK. attrValue := val.GetAttr(name) - if ctyEmptyString(attrValue) { + // If the value is marked, the ctyEmptyString function will fail + if !val.ContainsMarked() && ctyEmptyString(attrValue) { return cty.NullVal(attrType) } diff --git a/plans/changes.go b/plans/changes.go index 9e8f25bae..a8ea099fe 100644 --- a/plans/changes.go +++ b/plans/changes.go @@ -341,14 +341,15 @@ func (c *Change) Encode(ty cty.Type) (*ChangeSrc, error) { if err != nil { return nil, err } - afterDV, err := NewDynamicValue(c.After, ty) + afterDV, err, marks := NewDynamicValueMarks(c.After, ty) if err != nil { return nil, err } return &ChangeSrc{ - Action: c.Action, - Before: beforeDV, - After: afterDV, + Action: c.Action, + Before: beforeDV, + After: afterDV, + ValMarks: marks, }, nil } diff --git a/plans/changes_src.go b/plans/changes_src.go index 553a84082..2b7cd9da3 100644 --- a/plans/changes_src.go +++ b/plans/changes_src.go @@ -6,6 +6,7 @@ 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. @@ -156,6 +157,9 @@ type ChangeSrc struct { // but have not yet been decoded from the serialized value used for // storage. Before, After DynamicValue + + // Marked Paths + ValMarks *ctymsgpack.MarkInfo } // Decode unmarshals the raw representations of the before and after values diff --git a/plans/dynamic_value.go b/plans/dynamic_value.go index 51fbb24cf..c9080deb0 100644 --- a/plans/dynamic_value.go +++ b/plans/dynamic_value.go @@ -55,6 +55,26 @@ 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 diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index 509bd5bbc..73e37539f 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -141,6 +141,25 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { return nil, diags.Err() } + var markedPath cty.Path + // 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() + } + metaConfigVal := cty.NullVal(cty.DynamicPseudoType) if n.ProviderMetas != nil { if m, ok := n.ProviderMetas[n.ProviderAddr.Provider]; ok && m != nil { @@ -235,6 +254,15 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } plannedNewVal := resp.PlannedState + + // Add the mark back to the planned new value + 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 + }) + plannedPrivate := resp.PlannedPrivate if plannedNewVal == cty.NilVal { diff --git a/terraform/evaluate.go b/terraform/evaluate.go index 3e77ca9b8..3c11b66ca 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -292,6 +292,10 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd val = cty.UnknownVal(wantType) } + if config.Sensitive { + val = val.Mark("sensitive") + } + return val, diags } diff --git a/vendor/github.com/zclconf/go-cty/cty/json/marshal.go b/vendor/github.com/zclconf/go-cty/cty/json/marshal.go index 75e02577b..040b20c27 100644 --- a/vendor/github.com/zclconf/go-cty/cty/json/marshal.go +++ b/vendor/github.com/zclconf/go-cty/cty/json/marshal.go @@ -10,7 +10,8 @@ import ( func marshal(val cty.Value, t cty.Type, path cty.Path, b *bytes.Buffer) error { if val.IsMarked() { - return path.NewErrorf("value has marks, so it cannot be seralized") + // For now, dump the marks when serializing JSON for POC purposes + val, _ = val.UnmarkDeep() } // If we're going to decode as DynamicPseudoType then we need to save diff --git a/vendor/github.com/zclconf/go-cty/cty/msgpack/marshal.go b/vendor/github.com/zclconf/go-cty/cty/msgpack/marshal.go index cc351f0f1..950d1c590 100644 --- a/vendor/github.com/zclconf/go-cty/cty/msgpack/marshal.go +++ b/vendor/github.com/zclconf/go-cty/cty/msgpack/marshal.go @@ -41,6 +41,36 @@ 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")