Skip to content

Commit ab2846c

Browse files
DanielMSchmidtDanielMSchmidt
authored
Merge pull request #36031 from hashicorp/TF-18617
ephemeral: support write-only attributes
2 parents 84c1bb5 + 3a962e8 commit ab2846c

29 files changed

+2506
-1334
lines changed

‎docs/plugin-protocol/tfplugin5.8.proto‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,10 @@ message ClientCapabilities {
166166
// The deferral_allowed capability signals that the client is able to
167167
// handle deferred responses from the provider.
168168
bool deferral_allowed = 1;
169+
170+
// The write_only_attributes_allowed capability signals that the client
171+
// is able to handle write_only attributes for managed resources.
172+
bool write_only_attributes_allowed = 2;
169173
}
170174

171175
message Function {

‎docs/plugin-protocol/tfplugin6.8.proto‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,10 @@ message ClientCapabilities {
241241
// The deferral_allowed capability signals that the client is able to
242242
// handle deferred responses from the provider.
243243
bool deferral_allowed = 1;
244+
245+
// The write_only_attributes_allowed capability signals that the client
246+
// is able to handle write_only attributes for managed resources.
247+
bool write_only_attributes_allowed = 2;
244248
}
245249

246250
// Deferred is a message that indicates that change is deferred for a reason.

‎internal/lang/ephemeral/marshal.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: BUSL-1.1
3+
4+
package ephemeral
5+
6+
import (
7+
"github.com/hashicorp/terraform/internal/lang/marks"
8+
"github.com/zclconf/go-cty/cty"
9+
)
10+
11+
// RemoveEphemeralValues takes a value that possibly contains ephemeral
12+
// values and returns an equal value without ephemeral values. If an attribute contains
13+
// an ephemeral value it will be set to null.
14+
func RemoveEphemeralValues(value cty.Value) cty.Value {
15+
// We currently have no error case, so we can ignore the error
16+
val, _ := cty.Transform(value, func(p cty.Path, v cty.Value) (cty.Value, error) {
17+
_, givenMarks := v.Unmark()
18+
if _, isEphemeral := givenMarks[marks.Ephemeral]; isEphemeral {
19+
// We'll strip the ephemeral mark but retain any other marks
20+
// that might be present on the input.
21+
delete(givenMarks, marks.Ephemeral)
22+
if !v.IsKnown() {
23+
// If the source value is unknown then we must leave it
24+
// unknown because its final type might be more precise
25+
// than the associated type constraint and returning a
26+
// typed null could therefore over-promise on what the
27+
// final result type will be.
28+
// We're deliberately constructing a fresh unknown value
29+
// here, rather than returning the one we were given,
30+
// because we need to discard any refinements that the
31+
// unknown value might be carrying that definitely won't
32+
// be honored when we force the final result to be null.
33+
return cty.UnknownVal(v.Type()).WithMarks(givenMarks), nil
34+
}
35+
return cty.NullVal(v.Type()).WithMarks(givenMarks), nil
36+
}
37+
return v, nil
38+
})
39+
return val
40+
}

‎internal/lang/ephemeral/marshal_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: BUSL-1.1
3+
4+
package ephemeral
5+
6+
import (
7+
"testing"
8+
9+
"github.com/hashicorp/terraform/internal/lang/marks"
10+
"github.com/zclconf/go-cty/cty"
11+
)
12+
13+
func TestEphemeral_removeEphemeralValues(t *testing.T) {
14+
for name, tc := range map[string]struct {
15+
input cty.Value
16+
want cty.Value
17+
}{
18+
"empty case": {
19+
input: cty.NullVal(cty.DynamicPseudoType),
20+
want: cty.NullVal(cty.DynamicPseudoType),
21+
},
22+
"ephemeral marks case": {
23+
input: cty.ObjectVal(map[string]cty.Value{
24+
"ephemeral": cty.StringVal("ephemeral_value").Mark(marks.Ephemeral),
25+
"normal": cty.StringVal("normal_value"),
26+
}),
27+
want: cty.ObjectVal(map[string]cty.Value{
28+
"ephemeral": cty.NullVal(cty.String),
29+
"normal": cty.StringVal("normal_value"),
30+
}),
31+
},
32+
"sensitive marks case": {
33+
input: cty.ObjectVal(map[string]cty.Value{
34+
"sensitive": cty.StringVal("sensitive_value").Mark(marks.Sensitive),
35+
"normal": cty.StringVal("normal_value"),
36+
}),
37+
want: cty.ObjectVal(map[string]cty.Value{
38+
"sensitive": cty.StringVal("sensitive_value").Mark(marks.Sensitive),
39+
"normal": cty.StringVal("normal_value"),
40+
}),
41+
},
42+
"sensitive and ephemeral marks case": {
43+
input: cty.ObjectVal(map[string]cty.Value{
44+
"sensitive_and_ephemeral": cty.StringVal("sensitive_and_ephemeral_value").Mark(marks.Sensitive).Mark(marks.Ephemeral),
45+
"normal": cty.StringVal("normal_value"),
46+
}),
47+
want: cty.ObjectVal(map[string]cty.Value{
48+
"sensitive_and_ephemeral": cty.NullVal(cty.String).Mark(marks.Sensitive),
49+
"normal": cty.StringVal("normal_value"),
50+
}),
51+
},
52+
} {
53+
t.Run(name, func(t *testing.T) {
54+
got := RemoveEphemeralValues(tc.input)
55+
56+
if !got.RawEquals(tc.want) {
57+
t.Errorf("got %#v, want %#v", got, tc.want)
58+
}
59+
})
60+
}
61+
}

‎internal/lang/ephemeral/validate.go‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: BUSL-1.1
3+
4+
package ephemeral
5+
6+
import (
7+
"fmt"
8+
9+
"github.com/hashicorp/terraform/internal/addrs"
10+
"github.com/hashicorp/terraform/internal/configs/configschema"
11+
"github.com/hashicorp/terraform/internal/tfdiags"
12+
"github.com/zclconf/go-cty/cty"
13+
)
14+
15+
func ValidateWriteOnlyAttributes(newVal cty.Value, schema *configschema.Block, provider addrs.AbsProviderConfig, addr addrs.AbsResourceInstance) (diags tfdiags.Diagnostics) {
16+
if writeOnlyPaths := NonNullWriteOnlyPaths(newVal, schema, nil); len(writeOnlyPaths) != 0 {
17+
for _, p := range writeOnlyPaths {
18+
diags = diags.Append(tfdiags.Sourceless(
19+
tfdiags.Error,
20+
"Write-only attribute set",
21+
fmt.Sprintf(
22+
"Provider %q returned a value for the write-only attribute \"%s%s\". Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.",
23+
provider.String(), addr.String(), tfdiags.FormatCtyPath(p),
24+
),
25+
))
26+
}
27+
}
28+
return diags
29+
}
30+
31+
// NonNullWriteOnlyPaths returns a list of paths to attributes that are write-only
32+
// and non-null in the given value.
33+
func NonNullWriteOnlyPaths(val cty.Value, schema *configschema.Block, p cty.Path) (paths []cty.Path) {
34+
if schema == nil {
35+
return paths
36+
}
37+
38+
for name, attr := range schema.Attributes {
39+
attrPath := append(p, cty.GetAttrStep{Name: name})
40+
attrVal, _ := attrPath.Apply(val)
41+
if attr.WriteOnly && !attrVal.IsNull() {
42+
paths = append(paths, attrPath)
43+
}
44+
}
45+
46+
for name, blockS := range schema.BlockTypes {
47+
blockPath := append(p, cty.GetAttrStep{Name: name})
48+
x := NonNullWriteOnlyPaths(val, &blockS.Block, blockPath)
49+
paths = append(paths, x...)
50+
}
51+
52+
return paths
53+
}

‎internal/lang/ephemeral/validate_test.go‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: BUSL-1.1
3+
4+
package ephemeral
5+
6+
import (
7+
"testing"
8+
9+
"github.com/hashicorp/terraform/internal/configs/configschema"
10+
"github.com/zclconf/go-cty/cty"
11+
)
12+
13+
func TestNonNullWriteOnlyPaths(t *testing.T) {
14+
for name, tc := range map[string]struct {
15+
val cty.Value
16+
schema *configschema.Block
17+
18+
expectedPaths []cty.Path
19+
}{
20+
"no write-only attributes": {
21+
val: cty.ObjectVal(map[string]cty.Value{
22+
"id": cty.StringVal("i-abc123"),
23+
}),
24+
schema: &configschema.Block{
25+
Attributes: map[string]*configschema.Attribute{
26+
"id": {
27+
Type: cty.String,
28+
},
29+
},
30+
},
31+
},
32+
33+
"write-only attribute with null value": {
34+
val: cty.ObjectVal(map[string]cty.Value{
35+
"id": cty.NullVal(cty.String),
36+
}),
37+
schema: &configschema.Block{
38+
Attributes: map[string]*configschema.Attribute{
39+
"id": {
40+
Type: cty.String,
41+
Optional: true,
42+
WriteOnly: true,
43+
},
44+
},
45+
},
46+
},
47+
48+
"write-only attribute with non-null value": {
49+
val: cty.ObjectVal(map[string]cty.Value{
50+
"valid": cty.NullVal(cty.String),
51+
"id": cty.StringVal("i-abc123"),
52+
}),
53+
schema: &configschema.Block{
54+
Attributes: map[string]*configschema.Attribute{
55+
"valid": {
56+
Type: cty.String,
57+
Optional: true,
58+
WriteOnly: true,
59+
},
60+
"id": {
61+
Type: cty.String,
62+
Optional: true,
63+
WriteOnly: true,
64+
},
65+
},
66+
},
67+
expectedPaths: []cty.Path{cty.GetAttrPath("id")},
68+
},
69+
70+
"write-only attributes in blocks": {
71+
val: cty.ObjectVal(map[string]cty.Value{
72+
"foo": cty.ObjectVal(map[string]cty.Value{
73+
"valid-write-only": cty.NullVal(cty.String),
74+
"valid": cty.StringVal("valid"),
75+
"id": cty.StringVal("i-abc123"),
76+
"bar": cty.ObjectVal(map[string]cty.Value{
77+
"valid-write-only": cty.NullVal(cty.String),
78+
"valid": cty.StringVal("valid"),
79+
"id": cty.StringVal("i-abc123"),
80+
}),
81+
}),
82+
}),
83+
schema: &configschema.Block{
84+
BlockTypes: map[string]*configschema.NestedBlock{
85+
"foo": {
86+
Block: configschema.Block{
87+
Attributes: map[string]*configschema.Attribute{
88+
"valid-write-only": {
89+
Type: cty.String,
90+
Optional: true,
91+
WriteOnly: true,
92+
},
93+
"valid": {
94+
Type: cty.String,
95+
Optional: true,
96+
},
97+
"id": {
98+
Type: cty.String,
99+
Optional: true,
100+
WriteOnly: true,
101+
},
102+
},
103+
BlockTypes: map[string]*configschema.NestedBlock{
104+
"bar": {
105+
Block: configschema.Block{
106+
Attributes: map[string]*configschema.Attribute{
107+
"valid-write-only": {
108+
Type: cty.String,
109+
Optional: true,
110+
WriteOnly: true,
111+
},
112+
"valid": {
113+
Type: cty.String,
114+
Optional: true,
115+
},
116+
"id": {
117+
Type: cty.String,
118+
Optional: true,
119+
WriteOnly: true,
120+
},
121+
},
122+
},
123+
},
124+
},
125+
},
126+
},
127+
},
128+
},
129+
expectedPaths: []cty.Path{cty.GetAttrPath("foo").GetAttr("id"), cty.GetAttrPath("foo").GetAttr("bar").GetAttr("id")},
130+
},
131+
} {
132+
t.Run(name, func(t *testing.T) {
133+
paths := NonNullWriteOnlyPaths(tc.val, tc.schema, nil)
134+
135+
if len(paths) != len(tc.expectedPaths) {
136+
t.Fatalf("expected %d write-only paths, got %d", len(tc.expectedPaths), len(paths))
137+
}
138+
139+
for i, path := range paths {
140+
if !path.Equals(tc.expectedPaths[i]) {
141+
t.Fatalf("expected path %#v, got %#v", tc.expectedPaths[i], path)
142+
}
143+
}
144+
})
145+
}
146+
}

‎internal/lang/funcs/conversion.go‎

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package funcs
66
import (
77
"strconv"
88

9+
"github.com/hashicorp/terraform/internal/lang/ephemeral"
910
"github.com/hashicorp/terraform/internal/lang/marks"
1011
"github.com/hashicorp/terraform/internal/lang/types"
1112
"github.com/zclconf/go-cty/cty"
@@ -126,29 +127,7 @@ var EphemeralAsNullFunc = function.New(&function.Spec{
126127
return args[0].Type(), nil
127128
},
128129
Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
129-
return cty.Transform(args[0], func(p cty.Path, v cty.Value) (cty.Value, error) {
130-
_, givenMarks := v.Unmark()
131-
if _, isEphemeral := givenMarks[marks.Ephemeral]; isEphemeral {
132-
// We'll strip the ephemeral mark but retain any other marks
133-
// that might be present on the input.
134-
delete(givenMarks, marks.Ephemeral)
135-
if !v.IsKnown() {
136-
// If the source value is unknown then we must leave it
137-
// unknown because its final type might be more precise
138-
// than the associated type constraint and returning a
139-
// typed null could therefore over-promise on what the
140-
// final result type will be.
141-
// We're deliberately constructing a fresh unknown value
142-
// here, rather than returning the one we were given,
143-
// because we need to discard any refinements that the
144-
// unknown value might be carrying that definitely won't
145-
// be honored when we force the final result to be null.
146-
return cty.UnknownVal(v.Type()).WithMarks(givenMarks), nil
147-
}
148-
return cty.NullVal(v.Type()).WithMarks(givenMarks), nil
149-
}
150-
return v, nil
151-
})
130+
return ephemeral.RemoveEphemeralValues(args[0]), nil
152131
},
153132
})
154133

‎internal/plans/objchange/plan_valid.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,16 @@ func assertPlannedValueValid(attrS *configschema.Attribute, priorV, configV, pla
301301
return errs
302302
}
303303

304+
if attrS.WriteOnly {
305+
// The provider is not allowed to return non-null values for write-only attributes
306+
if !plannedV.IsNull() {
307+
errs = append(errs, path.NewErrorf("planned value for write-only attribute is not null"))
308+
}
309+
310+
// We don't want to evaluate further if the attribute is write-only and null
311+
return errs
312+
}
313+
304314
switch {
305315
// The provider can plan any value for a computed-only attribute. There may
306316
// be a config value here in the case where a user used `ignore_changes` on

0 commit comments

Comments
 (0)