Skip to content

Commit be96750

Browse files
radeksimkoradeksimko
committed
Ensure ephemeral mark is assigned to WriteOnly attribute values
1 parent 32fe8e7 commit be96750

File tree

8 files changed

+223
-33
lines changed

8 files changed

+223
-33
lines changed

‎internal/backend/local/backend_apply.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func (b *Local) opApply(
106106
// actions but that any it does include will be properly-formed.
107107
// plan.Errored will be true in this case, which our plan
108108
// renderer can rely on to tailor its messaging.
109-
if plan != nil && (len(plan.Changes.Resources) != 0 || len(plan.Changes.Outputs) != 0) {
109+
if plan != nil && plan.Changes != nil && (len(plan.Changes.Resources) != 0 || len(plan.Changes.Outputs) != 0) {
110110
op.View.Plan(plan, schemas)
111111
}
112112
op.ReportResult(runningOp, diags)

‎internal/configs/configschema/implied_type.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,30 @@ func (b *Block) ContainsSensitive() bool {
5959
return false
6060
}
6161

62+
// ContainsWriteOnly returns true if any of the attributes of the receiving
63+
// block or any of its descendant blocks are marked as WriteOnly.
64+
//
65+
// Blocks themselves cannot be WriteOnly as a whole -- sensitivity is a
66+
// per-attribute idea -- but sometimes we want to include a whole object
67+
// decoded from a block in some UI output, and that is safe to do only if
68+
// none of the contained attributes are WriteOnly.
69+
func (b *Block) ContainsWriteOnly() bool {
70+
for _, attrS := range b.Attributes {
71+
if attrS.WriteOnly {
72+
return true
73+
}
74+
if attrS.NestedType != nil && attrS.NestedType.ContainsWriteOnly() {
75+
return true
76+
}
77+
}
78+
for _, blockS := range b.BlockTypes {
79+
if blockS.ContainsWriteOnly() {
80+
return true
81+
}
82+
}
83+
return false
84+
}
85+
6286
// ImpliedType returns the cty.Type that would result from decoding a Block's
6387
// ImpliedType and getting the resulting AttributeType.
6488
//
@@ -134,3 +158,17 @@ func (o *Object) ContainsSensitive() bool {
134158
}
135159
return false
136160
}
161+
162+
// ContainsWriteOnly returns true if any of the attributes of the receiving
163+
// Object are marked as WriteOnly.
164+
func (o *Object) ContainsWriteOnly() bool {
165+
for _, attrS := range o.Attributes {
166+
if attrS.WriteOnly {
167+
return true
168+
}
169+
if attrS.NestedType != nil && attrS.NestedType.ContainsWriteOnly() {
170+
return true
171+
}
172+
}
173+
return false
174+
}

‎internal/configs/configschema/marks.go

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,72 @@ func (b *Block) SensitivePaths(val cty.Value, basePath cty.Path) []cty.Path {
8484
return ret
8585
}
8686

87+
// WriteOnlyPaths returns a set of paths into the given value that should
88+
// be marked as WriteOnly based on the static declarations in the schema.
89+
func (b *Block) WriteOnlyPaths(val cty.Value, basePath cty.Path) []cty.Path {
90+
var ret []cty.Path
91+
92+
// We can mark attributes as WriteOnly even if the value is null
93+
for name, attrS := range b.Attributes {
94+
if attrS.WriteOnly {
95+
attrPath := copyAndExtendPath(basePath, cty.GetAttrStep{Name: name})
96+
ret = append(ret, attrPath)
97+
}
98+
}
99+
100+
// If the value is null, no other marks are possible
101+
if val.IsNull() {
102+
return ret
103+
}
104+
105+
// Extract marks for nested attribute type values
106+
for name, attrS := range b.Attributes {
107+
// If the attribute has no nested type, or the nested type doesn't
108+
// contain any write-only attributes, skip inspecting it
109+
if attrS.NestedType == nil || !attrS.NestedType.ContainsWriteOnly() {
110+
continue
111+
}
112+
113+
// Create a copy of the path, with this step added, to add to our PathValueMarks slice
114+
attrPath := copyAndExtendPath(basePath, cty.GetAttrStep{Name: name})
115+
ret = append(ret, attrS.NestedType.WriteOnlyPaths(val.GetAttr(name), attrPath)...)
116+
}
117+
118+
// Extract marks for nested blocks
119+
for name, blockS := range b.BlockTypes {
120+
// If our block doesn't contain any WriteOnly attributes, skip inspecting it
121+
if !blockS.Block.ContainsWriteOnly() {
122+
continue
123+
}
124+
125+
blockV := val.GetAttr(name)
126+
if blockV.IsNull() || !blockV.IsKnown() {
127+
continue
128+
}
129+
130+
// Create a copy of the path, with this step added, to add to our PathValueMarks slice
131+
blockPath := copyAndExtendPath(basePath, cty.GetAttrStep{Name: name})
132+
133+
switch blockS.Nesting {
134+
case NestingSingle, NestingGroup:
135+
ret = append(ret, blockS.Block.WriteOnlyPaths(blockV, blockPath)...)
136+
case NestingList, NestingMap, NestingSet:
137+
blockV, _ = blockV.Unmark() // peel off one level of marking so we can iterate
138+
for it := blockV.ElementIterator(); it.Next(); {
139+
idx, blockEV := it.Element()
140+
// Create a copy of the path, with this block instance's index
141+
// step added, to add to our PathValueMarks slice
142+
blockInstancePath := copyAndExtendPath(blockPath, cty.IndexStep{Key: idx})
143+
morePaths := blockS.Block.WriteOnlyPaths(blockEV, blockInstancePath)
144+
ret = append(ret, morePaths...)
145+
}
146+
default:
147+
panic(fmt.Sprintf("unsupported nesting mode %s", blockS.Nesting))
148+
}
149+
}
150+
return ret
151+
}
152+
87153
// SensitivePaths returns a set of paths into the given value that should be
88154
// marked as sensitive based on the static declarations in the schema.
89155
func (o *Object) SensitivePaths(val cty.Value, basePath cty.Path) []cty.Path {
@@ -140,3 +206,60 @@ func (o *Object) SensitivePaths(val cty.Value, basePath cty.Path) []cty.Path {
140206
}
141207
return ret
142208
}
209+
210+
// WriteOnlyPaths returns a set of paths into the given value that should be
211+
// marked as WriteOnly based on the static declarations in the schema.
212+
func (o *Object) WriteOnlyPaths(val cty.Value, basePath cty.Path) []cty.Path {
213+
var ret []cty.Path
214+
215+
if val.IsNull() || !val.IsKnown() {
216+
return ret
217+
}
218+
219+
for name, attrS := range o.Attributes {
220+
// Skip attributes which can never produce WriteOnly path value marks
221+
if !attrS.WriteOnly && (attrS.NestedType == nil || !attrS.NestedType.ContainsWriteOnly()) {
222+
continue
223+
}
224+
225+
switch o.Nesting {
226+
case NestingSingle, NestingGroup:
227+
// Create a path to this attribute
228+
attrPath := copyAndExtendPath(basePath, cty.GetAttrStep{Name: name})
229+
230+
if attrS.WriteOnly {
231+
// If the entire attribute is WriteOnly, mark it so
232+
ret = append(ret, attrPath)
233+
} else {
234+
// The attribute has a nested type which contains WriteOnly
235+
// attributes, so recurse
236+
ret = append(ret, attrS.NestedType.WriteOnlyPaths(val.GetAttr(name), attrPath)...)
237+
}
238+
case NestingList, NestingMap, NestingSet:
239+
// For nested attribute types which have a non-single nesting mode,
240+
// we add path value marks for each element of the collection
241+
val, _ = val.Unmark() // peel off one level of marking so we can iterate
242+
for it := val.ElementIterator(); it.Next(); {
243+
idx, attrEV := it.Element()
244+
attrV := attrEV.GetAttr(name)
245+
246+
// Create a path to this element of the attribute's collection. Note
247+
// that the path is extended in opposite order to the iteration order
248+
// of the loops: index into the collection, then the contained
249+
// attribute name. This is because we have one type
250+
// representing multiple collection elements.
251+
attrPath := copyAndExtendPath(basePath, cty.IndexStep{Key: idx}, cty.GetAttrStep{Name: name})
252+
253+
if attrS.WriteOnly {
254+
// If the entire attribute is WriteOnly, mark it so
255+
ret = append(ret, attrPath)
256+
} else {
257+
ret = append(ret, attrS.NestedType.WriteOnlyPaths(attrV, attrPath)...)
258+
}
259+
}
260+
default:
261+
panic(fmt.Sprintf("unsupported nesting mode %s", attrS.NestedType.Nesting))
262+
}
263+
}
264+
return ret
265+
}

‎internal/plans/changes.go

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -611,22 +611,27 @@ func (c *Change) Encode(ty cty.Type) (*ChangeSrc, error) {
611611
// We don't accept any other marks here. The caller should have dealt
612612
// with those somehow and replaced them with unmarked placeholders before
613613
// writing the value into the state.
614-
unmarkedBefore, marksesBefore := c.Before.UnmarkDeepWithPaths()
615-
unmarkedAfter, marksesAfter := c.After.UnmarkDeepWithPaths()
616-
sensitiveAttrsBefore, unsupportedMarksesBefore := marks.PathsWithMark(marksesBefore, marks.Sensitive)
617-
sensitiveAttrsAfter, unsupportedMarksesAfter := marks.PathsWithMark(marksesAfter, marks.Sensitive)
618-
if len(unsupportedMarksesBefore) != 0 {
614+
unmarkedBefore, marksBefore := c.Before.UnmarkDeepWithPaths()
615+
unmarkedAfter, marksAfter := c.After.UnmarkDeepWithPaths()
616+
617+
sensitiveAttrsBefore, remainingMarksBefore := marks.PathsWithMark(marksBefore, marks.Sensitive)
618+
sensitiveAttrsAfter, remainingMarksAfter := marks.PathsWithMark(marksAfter, marks.Sensitive)
619+
620+
writeOnlyAttrsBefore, remainingMarksBefore := marks.PathsWithMark(remainingMarksBefore, marks.Ephemeral)
621+
writeOnlyAttrsAfter, remainingMarksAfter := marks.PathsWithMark(remainingMarksAfter, marks.Ephemeral)
622+
623+
if len(remainingMarksBefore) != 0 {
619624
return nil, fmt.Errorf(
620625
"prior value %s: can't serialize value marked with %#v (this is a bug in Terraform)",
621-
tfdiags.FormatCtyPath(unsupportedMarksesBefore[0].Path),
622-
unsupportedMarksesBefore[0].Marks,
626+
tfdiags.FormatCtyPath(remainingMarksBefore[0].Path),
627+
remainingMarksBefore[0].Marks,
623628
)
624629
}
625-
if len(unsupportedMarksesAfter) != 0 {
630+
if len(remainingMarksAfter) != 0 {
626631
return nil, fmt.Errorf(
627632
"new value %s: can't serialize value marked with %#v (this is a bug in Terraform)",
628-
tfdiags.FormatCtyPath(unsupportedMarksesAfter[0].Path),
629-
unsupportedMarksesAfter[0].Marks,
633+
tfdiags.FormatCtyPath(remainingMarksAfter[0].Path),
634+
remainingMarksAfter[0].Marks,
630635
)
631636
}
632637

@@ -645,6 +650,8 @@ func (c *Change) Encode(ty cty.Type) (*ChangeSrc, error) {
645650
After: afterDV,
646651
BeforeSensitivePaths: sensitiveAttrsBefore,
647652
AfterSensitivePaths: sensitiveAttrsAfter,
653+
BeforeWriteOnlyPaths: writeOnlyAttrsBefore,
654+
AfterWriteOnlyPaths: writeOnlyAttrsAfter,
648655
Importing: c.Importing.Encode(),
649656
GeneratedConfig: c.GeneratedConfig,
650657
}, nil

‎internal/plans/changes_src.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,14 @@ type ChangeSrc struct {
388388
// the serialized change.
389389
BeforeSensitivePaths, AfterSensitivePaths []cty.Path
390390

391+
// BeforeWriteOnlyPaths and AfterWriteOnlyPaths are the paths for any
392+
// values in Before or After (respectively) that are considered to be
393+
// WriteOnly. The WriteOnly marks are removed from the in-memory values
394+
// to enable encoding (marked values cannot be marshalled), and so we
395+
// store the WriteOnly paths to allow re-marking later when we decode
396+
// the serialized change.
397+
BeforeWriteOnlyPaths, AfterWriteOnlyPaths []cty.Path
398+
391399
// Importing is present if the resource is being imported as part of this
392400
// change.
393401
//

‎internal/states/instance_object.go

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,34 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res
9898
// If it contains marks, remove these marks before traversing the
9999
// structure with UnknownAsNull, and save the PathValueMarks
100100
// so we can save them in state.
101-
val, sensitivePaths, err := unmarkValueForStorage(o.Value)
101+
val, remainingMarks := o.Value.UnmarkDeepWithPaths()
102+
103+
sensitivePaths, remainingMarks := marks.PathsWithMark(remainingMarks, marks.Sensitive)
104+
105+
writeOnlyPaths, remainingMarks := marks.PathsWithMark(remainingMarks, marks.Ephemeral)
106+
// ensure we aren't attempting to persist real values after unmarking
107+
err := cty.Walk(val, func(p cty.Path, v cty.Value) (bool, error) {
108+
for _, woPath := range writeOnlyPaths {
109+
if p.Equals(woPath) && !v.IsNull() {
110+
return false, fmt.Errorf(
111+
"%s: cannot serialize ephemeral value %#v for inclusion in a state snapshot (this is a bug in Terraform)",
112+
tfdiags.FormatCtyPath(woPath), val.GoString(),
113+
)
114+
}
115+
}
116+
return true, nil
117+
})
102118
if err != nil {
103119
return nil, err
104120
}
105121

122+
if len(remainingMarks) != 0 {
123+
return nil, fmt.Errorf(
124+
"%s: cannot serialize value marked as %#v for inclusion in a state snapshot (this is a bug in Terraform)",
125+
tfdiags.FormatCtyPath(remainingMarks[0].Path), remainingMarks[0].Marks,
126+
)
127+
}
128+
106129
// Our state serialization can't represent unknown values, so we convert
107130
// them to nulls here. This is lossy, but nobody should be writing unknown
108131
// values here and expecting to get them out again later.
@@ -157,24 +180,3 @@ func (o *ResourceInstanceObject) AsTainted() *ResourceInstanceObject {
157180
ret.Status = ObjectTainted
158181
return ret
159182
}
160-
161-
// unmarkValueForStorage takes a value that possibly contains marked values
162-
// and returns an equal value without markings along with the separated mark
163-
// metadata that should be stored alongside the value in another field.
164-
//
165-
// This function only accepts the marks that are valid to store, and so will
166-
// return an error if other marks are present. Marks that this package doesn't
167-
// know how to store must be dealt with somehow by a caller -- presumably by
168-
// replacing each marked value with some sort of storage placeholder -- before
169-
// writing a value into the state.
170-
func unmarkValueForStorage(v cty.Value) (unmarkedV cty.Value, sensitivePaths []cty.Path, err error) {
171-
val, pvms := v.UnmarkDeepWithPaths()
172-
sensitivePaths, withOtherMarks := marks.PathsWithMark(pvms, marks.Sensitive)
173-
if len(withOtherMarks) != 0 {
174-
return cty.NilVal, nil, fmt.Errorf(
175-
"%s: cannot serialize value marked as %#v for inclusion in a state snapshot (this is a bug in Terraform)",
176-
tfdiags.FormatCtyPath(withOtherMarks[0].Path), withOtherMarks[0].Marks,
177-
)
178-
}
179-
return val, sensitivePaths, nil
180-
}

‎internal/terraform/node_resource_abstract_instance.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -748,6 +748,9 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state
748748
if moreSensitivePaths := schema.SensitivePaths(ret.Value, nil); len(moreSensitivePaths) != 0 {
749749
ret.Value = marks.MarkPaths(ret.Value, marks.Sensitive, moreSensitivePaths)
750750
}
751+
if writeOnlyPaths := schema.WriteOnlyPaths(ret.Value, nil); len(writeOnlyPaths) != 0 {
752+
ret.Value = marks.MarkPaths(ret.Value, marks.Ephemeral, writeOnlyPaths)
753+
}
751754

752755
return ret, deferred, diags
753756
}
@@ -1067,6 +1070,9 @@ func (n *NodeAbstractResourceInstance) plan(
10671070
if sensitivePaths := schema.SensitivePaths(plannedNewVal, nil); len(sensitivePaths) != 0 {
10681071
plannedNewVal = marks.MarkPaths(plannedNewVal, marks.Sensitive, sensitivePaths)
10691072
}
1073+
if writeOnlyPaths := schema.WriteOnlyPaths(plannedNewVal, nil); len(writeOnlyPaths) != 0 {
1074+
plannedNewVal = marks.MarkPaths(plannedNewVal, marks.Ephemeral, writeOnlyPaths)
1075+
}
10701076

10711077
reqRep, reqRepDiags := getRequiredReplaces(unmarkedPriorVal, unmarkedPlannedNewVal, resp.RequiresReplace, n.ResolvedProvider.Provider, n.Addr)
10721078
diags = diags.Append(reqRepDiags)
@@ -2646,6 +2652,9 @@ func (n *NodeAbstractResourceInstance) apply(
26462652
if sensitivePaths := schema.SensitivePaths(newVal, nil); len(sensitivePaths) != 0 {
26472653
newVal = marks.MarkPaths(newVal, marks.Sensitive, sensitivePaths)
26482654
}
2655+
if writeOnlyPaths := schema.WriteOnlyPaths(newVal, nil); len(writeOnlyPaths) != 0 {
2656+
newVal = marks.MarkPaths(newVal, marks.Ephemeral, writeOnlyPaths)
2657+
}
26492658

26502659
if change.Action != plans.Delete && !diags.HasErrors() {
26512660
// Only values that were marked as unknown in the planned value are allowed

‎internal/terraform/node_resource_plan_partialexp.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,9 @@ func (n *nodePlannablePartialExpandedResource) managedResourceExecute(ctx EvalCo
278278
if sensitivePaths := schema.SensitivePaths(plannedNewVal, nil); len(sensitivePaths) != 0 {
279279
plannedNewVal = marks.MarkPaths(plannedNewVal, marks.Sensitive, sensitivePaths)
280280
}
281+
if writeOnlyPaths := schema.WriteOnlyPaths(plannedNewVal, nil); len(writeOnlyPaths) != 0 {
282+
plannedNewVal = marks.MarkPaths(plannedNewVal, marks.Ephemeral, writeOnlyPaths)
283+
}
281284

282285
change.After = plannedNewVal
283286
change.Private = resp.PlannedPrivate

0 commit comments

Comments
 (0)