Skip to content

Commit 7873f91

Browse files
jbardinjbardin
authored
Merge pull request #36523 from hashicorp/backport/jbardin/write-only-validation/strongly-legal-snake
Backport of `WriteOnly` validation on deeply nested ephemeral values into v1.11
2 parents 6e6ee99 + d06bebc commit 7873f91

File tree

6 files changed

+449
-21
lines changed

6 files changed

+449
-21
lines changed

‎internal/configs/configschema/coerce_value.go‎

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,15 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
110110
continue
111111
}
112112

113+
coll, marks := coll.Unmark()
114+
113115
if !coll.CanIterateElements() {
114116
return cty.UnknownVal(impliedType), path.NewErrorf("must be a list")
115117
}
116118
l := coll.LengthInt()
117119

118120
if l == 0 {
119-
attrs[typeName] = cty.ListValEmpty(blockS.ImpliedType())
121+
attrs[typeName] = cty.ListValEmpty(blockS.ImpliedType()).WithMarks(marks)
120122
continue
121123
}
122124
elems := make([]cty.Value, 0, l)
@@ -132,7 +134,7 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
132134
elems = append(elems, val)
133135
}
134136
}
135-
attrs[typeName] = cty.ListVal(elems)
137+
attrs[typeName] = cty.ListVal(elems).WithMarks(marks)
136138
default:
137139
attrs[typeName] = cty.ListValEmpty(blockS.ImpliedType())
138140
}
@@ -150,14 +152,15 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
150152
attrs[typeName] = cty.UnknownVal(cty.Set(blockS.ImpliedType()))
151153
continue
152154
}
155+
coll, marks := coll.Unmark()
153156

154157
if !coll.CanIterateElements() {
155-
return cty.UnknownVal(impliedType), path.NewErrorf("must be a set")
158+
return cty.UnknownVal(impliedType), path.NewErrorf("cannot iterate over %#v", coll)
156159
}
157160
l := coll.LengthInt()
158161

159162
if l == 0 {
160-
attrs[typeName] = cty.SetValEmpty(blockS.ImpliedType())
163+
attrs[typeName] = cty.SetValEmpty(blockS.ImpliedType()).WithMarks(marks)
161164
continue
162165
}
163166
elems := make([]cty.Value, 0, l)
@@ -173,7 +176,7 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
173176
elems = append(elems, val)
174177
}
175178
}
176-
attrs[typeName] = cty.SetVal(elems)
179+
attrs[typeName] = cty.SetVal(elems).WithMarks(marks)
177180
default:
178181
attrs[typeName] = cty.SetValEmpty(blockS.ImpliedType())
179182
}
@@ -191,13 +194,14 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
191194
attrs[typeName] = cty.UnknownVal(cty.Map(blockS.ImpliedType()))
192195
continue
193196
}
197+
coll, marks := coll.Unmark()
194198

195199
if !coll.CanIterateElements() {
196200
return cty.UnknownVal(impliedType), path.NewErrorf("must be a map")
197201
}
198202
l := coll.LengthInt()
199203
if l == 0 {
200-
attrs[typeName] = cty.MapValEmpty(blockS.ImpliedType())
204+
attrs[typeName] = cty.MapValEmpty(blockS.ImpliedType()).WithMarks(marks)
201205
continue
202206
}
203207
elems := make(map[string]cty.Value)
@@ -220,24 +224,14 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
220224
// If the attribute values here contain any DynamicPseudoTypes,
221225
// the concrete type must be an object.
222226
useObject := false
223-
switch {
224-
case coll.Type().IsObjectType():
227+
if coll.Type().IsObjectType() || blockS.ImpliedType().HasDynamicTypes() {
225228
useObject = true
226-
default:
227-
// It's possible that we were given a map, and need to coerce it to an object
228-
ety := coll.Type().ElementType()
229-
for _, v := range elems {
230-
if !v.Type().Equals(ety) {
231-
useObject = true
232-
break
233-
}
234-
}
235229
}
236230

237231
if useObject {
238-
attrs[typeName] = cty.ObjectVal(elems)
232+
attrs[typeName] = cty.ObjectVal(elems).WithMarks(marks)
239233
} else {
240-
attrs[typeName] = cty.MapVal(elems)
234+
attrs[typeName] = cty.MapVal(elems).WithMarks(marks)
241235
}
242236
default:
243237
attrs[typeName] = cty.MapValEmpty(blockS.ImpliedType())

‎internal/configs/configschema/internal_validate.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,12 @@ func (b *Block) internalValidate(prefix string) error {
7777
// properly hash them, and so can't support mixed types.
7878
multiErr = errors.Join(multiErr, fmt.Errorf("%s%s: NestingSet blocks may not contain attributes of cty.DynamicPseudoType", prefix, name))
7979
}
80+
if blockS.Block.ContainsWriteOnly() {
81+
// This is not permitted because any marks within sets will
82+
// be hoisted up the outer set value, so only the set itself
83+
// can be WriteOnly.
84+
multiErr = errors.Join(multiErr, fmt.Errorf("%s%s: NestingSet blocks may not contain WriteOnly attributes", prefix, name))
85+
}
8086
}
8187
case NestingMap:
8288
if blockS.MinItems != 0 || blockS.MaxItems != 0 {
@@ -142,7 +148,13 @@ func (a *Attribute) internalValidate(name, prefix string) error {
142148
// This is not permitted because the HCL (cty) set implementation
143149
// needs to know the exact type of set elements in order to
144150
// properly hash them, and so can't support mixed types.
145-
err = errors.Join(err, fmt.Errorf("%s%s: NestingSet blocks may not contain attributes of cty.DynamicPseudoType", prefix, name))
151+
err = errors.Join(err, fmt.Errorf("%s%s: NestingSet attributes may not contain attributes of cty.DynamicPseudoType", prefix, name))
152+
}
153+
if a.NestedType.ContainsWriteOnly() {
154+
// This is not permitted because any marks within sets will
155+
// be hoisted up the outer set value, so only the set itself
156+
// can be WriteOnly.
157+
err = errors.Join(err, fmt.Errorf("%s%s: NestingSet attributes may not contain WriteOnly attributes", prefix, name))
146158
}
147159
}
148160
default:

‎internal/configs/configschema/internal_validate_test.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,45 @@ func TestBlockInternalValidate(t *testing.T) {
265265
},
266266
[]string{"bad: block schema is nil"},
267267
},
268+
"nested set block with write-only attribute": {
269+
&Block{
270+
BlockTypes: map[string]*NestedBlock{
271+
"bad": {
272+
Nesting: NestingSet,
273+
Block: Block{
274+
Attributes: map[string]*Attribute{
275+
"wo": {
276+
Type: cty.String,
277+
Optional: true,
278+
WriteOnly: true,
279+
},
280+
},
281+
},
282+
},
283+
},
284+
},
285+
[]string{"bad: NestingSet blocks may not contain WriteOnly attributes"},
286+
},
287+
"nested set attributes with write-only attribute": {
288+
&Block{
289+
Attributes: map[string]*Attribute{
290+
"bad": {
291+
NestedType: &Object{
292+
Attributes: map[string]*Attribute{
293+
"wo": {
294+
Type: cty.String,
295+
Optional: true,
296+
WriteOnly: true,
297+
},
298+
},
299+
Nesting: NestingSet,
300+
},
301+
Optional: true,
302+
},
303+
},
304+
},
305+
[]string{"bad: NestingSet attributes may not contain WriteOnly attributes"},
306+
},
268307
}
269308

270309
for name, test := range tests {

‎internal/configs/configschema/path_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,24 @@ func TestAttributeByPath(t *testing.T) {
3232
},
3333
},
3434
},
35+
"a4": {
36+
Description: "a3",
37+
NestedType: &Object{
38+
Nesting: NestingMap,
39+
Attributes: map[string]*Attribute{
40+
"nt1": {Description: "nt1"},
41+
"nt2": {
42+
Description: "nt2",
43+
NestedType: &Object{
44+
Nesting: NestingSingle,
45+
Attributes: map[string]*Attribute{
46+
"deeply_nested": {Description: "deeply_nested"},
47+
},
48+
},
49+
},
50+
},
51+
},
52+
},
3553
},
3654
BlockTypes: map[string]*NestedBlock{
3755
"b1": {
@@ -97,6 +115,11 @@ func TestAttributeByPath(t *testing.T) {
97115
"missing",
98116
false,
99117
},
118+
{
119+
cty.GetAttrPath("a3").IndexInt(1).GetAttr("nt2").GetAttr("deeply_nested"),
120+
"deeply_nested",
121+
true,
122+
},
100123
{
101124
cty.GetAttrPath("b1"),
102125
"block",
@@ -133,6 +156,11 @@ func TestAttributeByPath(t *testing.T) {
133156
"a9",
134157
true,
135158
},
159+
{
160+
cty.GetAttrPath("a4").IndexString("test").GetAttr("nt2").GetAttr("deeply_nested"),
161+
"deeply_nested",
162+
true,
163+
},
136164
} {
137165
t.Run(tc.attrDescription, func(t *testing.T) {
138166
attr := schema.AttributeByPath(tc.path)

‎internal/terraform/node_resource_validate.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -752,14 +752,23 @@ func validateDependsOn(ctx EvalContext, dependsOn []hcl.Traversal) (diags tfdiag
752752
func validateResourceForbiddenEphemeralValues(ctx EvalContext, value cty.Value, schema *configschema.Block) (diags tfdiags.Diagnostics) {
753753
for _, path := range ephemeral.EphemeralValuePaths(value) {
754754
attr := schema.AttributeByPath(path)
755+
756+
// If this attribute doesn't exist (usually through a dynamic type) or
757+
// itself isn't write-only, it may be nested within a more complex type
758+
// which is write-only. We need to walk upwards through the path
759+
// segments and see of something wrapping this path is write-only.
760+
for (attr == nil || !attr.WriteOnly) && len(path) > 1 {
761+
path = path[:len(path)-1]
762+
attr = schema.AttributeByPath((path))
763+
}
764+
755765
// We know the config decoded, so the "attribute" exists in the
756766
// schema somehow. If the ephemeral mark ended up being hoisted into
757767
// a container however, especially if that container is a block,
758768
// it's not actually an assignable attribute so we need to make a
759769
// generic sounding error with a little more context because the
760770
// AttributeValue diagnostic won't point to anything except the
761771
// resource block.
762-
//
763772
if attr == nil {
764773
diags = diags.Append(tfdiags.AttributeValue(
765774
tfdiags.Error,

0 commit comments

Comments
 (0)