Skip to content

Commit 37e9bc3

Browse files
committed
types: Remove Attrs, AttrTypes, Elems, ElemTypes, Null, Unknown, and Value fields
Reference: #447 Reference: #502 When the framework type system was originally being developed, the value types were introduced with exported fields which also served as the internal details of whether a value was null, unknown, or a known value of a friendlier Go type. It was known that there was the potential for issues, but the simplified developer experience seemed to outweigh the potential for developer issues. Fast forward a few months, this decision appears to have two consequences that the framework maintainers hear about across various forums. One issue is that the value types directly expose their internal implementation details and support the three states of a Terraform type value: being null, unknown, or a known value. Only one state should ever be set, but provider developers can make a value that is any combination of those states. This makes the framework behavior potentially indeterminate from the provider developer perspective whether, for example, a null AND unknown value becomes null OR unknown as it works its way through the framework. ```go type ThingResourceModel struct{ Computed types.String `tfsdk:"computed"` } func (r ThingResource) Create(ctx context.Context, req resource.CreateResource, resp *resource.CreateResponse) { var data ThingResourceModel resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...) tflog.Trace(ctx, "Data Values", map[string]any{ // Unknown value: types.String{Null: false, Unknown: true, Value: ""} "computed": plan.Computed, }) // Maybe some external API responses here, but showing hardcoded updates for // brevity. This will make the value invalid by enabling Null without // disabling Unknown. data.Computed.Null = true tflog.Trace(ctx, "Data Values", map[string]any{ // Invalid value: types.String{Null: true, Unknown: true, Value: ""} "computed": data.Computed, }) // The invalid value will be either null or unknown, depending on the // type implementation. If unknown, Terraform will error, since unknown // values are never allowed in state. resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) } ``` Another issue is that it is possible to create collection value types that do not match their type definition. This issue is especially more likely with `types.Object` where it is possible to accidentially omit attributes. While the framework handling would eventually catch this issue when dealing with the invalid value, it can be caught sooner. ```go // Invalid value (missing attribute and differing attribute name) types.Object{ AttrTypes: map[string]attr.Type{ "one": types.StringType, "two": types.BoolType, }, Attrs: map[string]attr.Value{ "not_one": types.String{Value: "wrong name"}, }, } ``` Another issue is that the default (zero-value) state for an "unset" value type turns into a known value, which is confusing since these values explicitly support being null. This causes Terraform errors which would surface to practitioners (especially when untested) that provider developers then have to troubleshoot the error message containing Terraform's type system details, potentially discover the reason why it is happening by looking at the framework type source code, then figure out a workable solution. It's not intuitive. ```go type ThingResourceModel struct{ // let's assume this is left unconfigured (null in config and plan) Optional types.String `tfsdk:"optional"` } func (r ThingResource) Create(ctx context.Context, req resource.CreateResource, resp *resource.CreateResponse) { // Providers can opt to use a single variable that is updated based on an // external response, however that logic can be more difficult sometimes, // so it can be easier to split them. Showing the split way to exemplify // the "unset" problem. var plan, state ThingResourceModel resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) tflog.Trace(ctx, "Plan Values", map[string]any{ // Null value: types.String{Null: true, Unknown: false, Value: ""} "optional": plan.Optional, }) // Maybe some external API responses here, but intentionally not // doing any state.Optional setting, which might happen if the // external response for that data was null for example. tflog.Trace(ctx, "State Values", map[string]any{ // Zero-value: types.String{Null: false, Unknown: false, Value: ""} "optional": state.Optional, }) // The state zero-value will later cause Terraform to error, such as: // Error: Provider produced inconsistent result after apply // ... expected cty.NullVal(cty.String), got cty.StringVal("") // Since the plan value said it would be null. resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) } ``` This removal of the fields in preference of functions and methods aims to unexport the internal details and treat the value types as immutable once they are created. The zero-value implementations of these values is now null, instead of a known zero-value of the underlying type. While not recommended for production usage without extensive testing, provider developers can opt for `panic()` inducing versions of collection value creations, rather than dealing with `diag.Diagnostics` everywhere. Accessing value information after the migration can be accomplished with the following: | Prior Value Access | New Value Access | |----------------------|--------------------| | `(types.Bool).Value` | `(types.Bool).ValueBool()` | | `(types.Bool).Null` | `(types.Bool).IsNull()` | | `(types.Bool).Unknown` | `(types.Bool).IsUnknown()` | | `(types.Float64).Value` | `(types.Float64).ValueFloat64()` | | `(types.Float64).Null` | `(types.Float64).IsNull()` | | `(types.Float64).Unknown` | `(types.Float64).IsUnknown()` | | `(types.Int64).Value` | `(types.Int64).ValueInt64()` | | `(types.Int64).Null` | `(types.Int64).IsNull()` | | `(types.Int64).Unknown` | `(types.Int64).IsUnknown()` | | `(types.List).Elems` | `(types.List).Elements()` or `(types.List).ElementsAs()` | | `(types.List).ElemType` | `(types.List).ElementType()` | | `(types.List).Null` | `(types.List).IsNull()` | | `(types.List).Unknown` | `(types.List).IsUnknown()` | | `(types.Map).Elems` | `(types.Map).Elements()` or `(types.Map).ElementsAs()` | | `(types.Map).ElemType` | `(types.Map).ElementType()` | | `(types.Map).Null` | `(types.Map).IsNull()` | | `(types.Map).Unknown` | `(types.Map).IsUnknown()` | | `(types.Number).Value` | `(types.Number).ValueBigFloat()` | | `(types.Number).Null` | `(types.Number).IsNull()` | | `(types.Number).Unknown` | `(types.Number).IsUnknown()` | | `(types.Object).Attrs` | `(types.Object).Attributes()` or `(types.Object).As()` | | `(types.Object).AttrTypes` | `(types.Object).AttributeTypes()` | | `(types.Object).Null` | `(types.Object).IsNull()` | | `(types.Object).Unknown` | `(types.Object).IsUnknown()` | | `(types.Set).Elems` | `(types.Set).Elements()` or `(types.Set).ElementsAs()` | | `(types.Set).ElemType` | `(types.Set).ElementType()` | | `(types.Set).Null` | `(types.Set).IsNull()` | | `(types.Set).Unknown` | `(types.Set).IsUnknown()` | | `(types.String).Value` | `(types.String).ValueString()` | | `(types.String).Null` | `(types.String).IsNull()` | | `(types.String).Unknown` | `(types.String).IsUnknown()` | Creating values after the migration can be accomplished with the following: | Prior Value Creation | New Value Creation | |----------------------|--------------------| | `types.Bool{Value: /* value */}` | `types.BoolValue(/* value */)` | | `types.Bool{Null: true}` | `types.BoolNull()` | | `types.Bool{Unknown: true}` | `types.BoolUnknown()` | | `types.Float64{Value: /* value */}` | `types.Float64Value(/* value */)` | | `types.Float64{Null: true}` | `types.Float64Null()` | | `types.Float64{Unknown: true}` | `types.Float64Unknown()` | | `types.Int64{Value: /* value */}` | `types.Int64Value(/* value */)` | | `types.Int64{Null: true}` | `types.Int64Null()` | | `types.Int64{Unknown: true}` | `types.Int64Unknown()` | | `types.List{ElemType: /* element type */, Elems: /* value */}` | `list, diags := types.ListValue(/* element type */, /* value */)` or `list, diags := types.ListValueFrom(context.Context, /* element type */, any)` or `list := types.ListValueMust(/* element type */, /* value */)` | | `types.List{ElemType: /* element type */, Null: true}` | `types.ListNull(/* element type */)` | | `types.List{ElemType: /* element type */, Unknown: true}` | `types.ListUnknown(/* element type */)` | | `types.Map{ElemType: /* element type */, Elems: /* value */}` | `m, diags := types.MapValue(/* element type */, /* value */)` or `m, diags := types.MapValueFrom(context.Context, /* element type */, any)` or `m := types.MapValueMust(/* element type */, /* value */)` | | `types.Map{ElemType: /* element type */, Null: true}` | `types.MapNull(/* element type */)` | | `types.Map{ElemType: /* element type */, Unknown: true}` | `types.MapUnknown(/* element type */)` | | `types.Number{Value: /* value */}` | `types.NumberValue(/* value */)` | | `types.Number{Null: true}` | `types.NumberNull()` | | `types.Number{Unknown: true}` | `types.NumberUnknown()` | | `types.Object{AttrTypes: /* attribute types */, Attrs: /* attribute values */}` | `object, diags := types.ObjectValue(/* attribute types */, /* attribute values */)` or `object, diags := types.ObjectValueFrom(context.Context, /* attribute types */, any)` or `object := types.ObjectValueMust(/* attribute types */, /* attribute values */)` | | `types.Object{AttrTypes: /* attribute types */, Null: true}` | `types.ObjectNull(/* attribute types */)` | | `types.Object{AttrTypes: /* attribute types */, Unknown: true}` | `types.ObjectUnknown(/* attribute types */)` | | `types.Set{ElemType: /* element type */, Elems: /* value */}` | `set, diags := types.SetValue(/* element type */, /* value */)` or `set, diags := types.SetValueFrom(context.Context, /* element type */, any)` or `set := types.SetValueMust(/* element type */, /* value */)` | | `types.Set{ElemType: /* element type */, Null: true}` | `types.SetNull(/* element type */)` | | `types.Set{ElemType: /* element type */, Unknown: true}` | `types.SetUnknown(/* element type */)` | | `types.String{Value: /* value */}` | `types.StringValue(/* value */)` | | `types.String{Null: true}` | `types.StringNull()` | | `types.String{Unknown: true}` | `types.StringUnknown()` |
1 parent de565fa commit 37e9bc3

File tree

85 files changed

+2936
-8067
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

85 files changed

+2936
-8067
lines changed

.changelog/pending.txt

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
```release-note:breaking-change
2+
types: The `Bool` type `Null`, `Unknown`, and `Value` fields have been removed. Use the `BoolNull()`, `BoolUnknown()`, and `BoolValue()` creation functions and `IsNull()`, `IsUnknown()`, and `ValueBool()` methods instead.
3+
```
4+
5+
```release-note:breaking-change
6+
types: The `Float64` type `Null`, `Unknown`, and `Value` fields have been removed. Use the `Float64Null()`, `Float64Unknown()`, and `Float64Value()` creation functions and `IsNull()`, `IsUnknown()`, and `ValueFloat64()` methods instead.
7+
```
8+
9+
```release-note:breaking-change
10+
types: The `Int64` type `Null`, `Unknown`, and `Value` fields have been removed. Use the `Int64Null()`, `Int64Unknown()`, and `Int64Value()` creation functions and `IsNull()`, `IsUnknown()`, and `ValueInt64()` methods instead.
11+
```
12+
13+
```release-note:breaking-change
14+
types: The `List` type `Elems`, `ElemType`, `Null`, and `Unknown` fields have been removed. Use the `ListNull()`, `ListUnknown()`, `ListValue()`, and `ListValueMust()` creation functions and `Elements()`, `ElementsAs()`, `ElementType()`, `IsNull()`, and `IsUnknown()` methods instead.
15+
```
16+
17+
```release-note:breaking-change
18+
types: The `Map` type `Elems`, `ElemType`, `Null`, and `Unknown` fields have been removed. Use the `MapNull()`, `MapUnknown()`, `MapValue()`, and `MapValueMust()` creation functions and `Elements()`, `ElementsAs()`, `ElementType()`, `IsNull()`, and `IsUnknown()` methods instead.
19+
```
20+
21+
```release-note:breaking-change
22+
types: The `Number` type `Null`, `Unknown`, and `Value` fields have been removed. Use the `NumberNull()`, `NumberUnknown()`, and `NumberValue()` creation functions and `IsNull()`, `IsUnknown()`, and `ValueBigFloat()` methods instead.
23+
```
24+
25+
```release-note:breaking-change
26+
types: The `Object` type `Attrs`, `AttrTypes`, `Null`, and `Unknown` fields have been removed. Use the `ObjectNull()`, `ObjectUnknown()`, `ObjectValue()`, and `ObjectValueMust()` creation functions and `As()`, `Attributes()`, `AttributeTypes()`, `IsNull()`, and `IsUnknown()` methods instead.
27+
```
28+
29+
```release-note:breaking-change
30+
types: The `Set` type `Elems`, `ElemType`, `Null`, and `Unknown` fields have been removed. Use the `SetNull()`, `SetUnknown()`, `SetValue()`, and `SetValueMust()` creation functions and `Elements()`, `ElementsAs()`, `ElementType()`, `IsNull()`, and `IsUnknown()` methods instead.
31+
```
32+
33+
```release-note:breaking-change
34+
types: The `String` type `Null`, `Unknown`, and `Value` fields have been removed. Use the `StringNull()`, `StringUnknown()`, and `StringValue()` creation functions and `IsNull()`, `IsUnknown()`, and `ValueString()` methods instead.
35+
```
36+
37+
```release-note:enhancement
38+
attr: Added `ValueState` type, which custom types can use to consistently represent the three possible value states (known, null, and unknown)
39+
```
40+
41+
```release-note:bug
42+
types: Prevented Terraform errors where the zero-value for any `attr.Value` types such as `String` would be a known value instead of null
43+
```
44+
45+
```release-note:bug
46+
types: Prevented indeterminate behavior for any `attr.Value` types where they could be any combination of null, unknown, and/or known
47+
```

attr/value_state.go

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package attr
2+
3+
import "fmt"
4+
5+
const (
6+
// ValueStateNull represents a value which is null.
7+
//
8+
// This value is 0 so it is the zero-value for types implementations.
9+
ValueStateNull ValueState = 0
10+
11+
// ValueStateUnknown represents a value which is unknown.
12+
ValueStateUnknown ValueState = 1
13+
14+
// ValueStateKnown represents a value which is known (not null or unknown).
15+
ValueStateKnown ValueState = 2
16+
)
17+
18+
type ValueState uint8
19+
20+
func (s ValueState) String() string {
21+
switch s {
22+
case ValueStateKnown:
23+
return "known"
24+
case ValueStateNull:
25+
return "null"
26+
case ValueStateUnknown:
27+
return "unknown"
28+
default:
29+
panic(fmt.Sprintf("unhandled ValueState in String: %d", s))
30+
}
31+
}

internal/fromtftypes/attribute_path_step_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func TestAttributePathStep(t *testing.T) {
4343
"PathStepElementKeyValue": {
4444
tfType: tftypes.ElementKeyValue(tftypes.NewValue(tftypes.String, "test")),
4545
attrType: types.StringType,
46-
expected: path.PathStepElementKeyValue{Value: types.String{Value: "test"}},
46+
expected: path.PathStepElementKeyValue{Value: types.StringValue("test")},
4747
},
4848
"PathStepElementKeyValue-error": {
4949
tfType: tftypes.ElementKeyValue(tftypes.NewValue(tftypes.String, "test")),

internal/fromtftypes/attribute_path_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func TestAttributePath(t *testing.T) {
8888
},
8989
},
9090
},
91-
expected: path.Root("test").AtSetValue(types.String{Value: "test-value"}),
91+
expected: path.Root("test").AtSetValue(types.StringValue("test-value")),
9292
},
9393
"AttributeName-ElementKeyValue-value-conversion-error": {
9494
tfType: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyValue(tftypes.NewValue(tftypes.String, "test-value")),

internal/fromtftypes/value_test.go

+41-55
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func TestValue(t *testing.T) {
2727
"empty-tftype": {
2828
tfType: tftypes.Value{},
2929
attrType: types.BoolType,
30-
expected: types.Bool{Null: true},
30+
expected: types.BoolNull(),
3131
},
3232
"nil-attr-type": {
3333
tfType: tftypes.Value{},
@@ -44,47 +44,47 @@ func TestValue(t *testing.T) {
4444
"bool-null": {
4545
tfType: tftypes.NewValue(tftypes.Bool, nil),
4646
attrType: types.BoolType,
47-
expected: types.Bool{Null: true},
47+
expected: types.BoolNull(),
4848
},
4949
"bool-unknown": {
5050
tfType: tftypes.NewValue(tftypes.Bool, tftypes.UnknownValue),
5151
attrType: types.BoolType,
52-
expected: types.Bool{Unknown: true},
52+
expected: types.BoolUnknown(),
5353
},
5454
"bool-value": {
5555
tfType: tftypes.NewValue(tftypes.Bool, true),
5656
attrType: types.BoolType,
57-
expected: types.Bool{Value: true},
57+
expected: types.BoolValue(true),
5858
},
5959
"float64-null": {
6060
tfType: tftypes.NewValue(tftypes.Number, nil),
6161
attrType: types.Float64Type,
62-
expected: types.Float64{Null: true},
62+
expected: types.Float64Null(),
6363
},
6464
"float64-unknown": {
6565
tfType: tftypes.NewValue(tftypes.Number, tftypes.UnknownValue),
6666
attrType: types.Float64Type,
67-
expected: types.Float64{Unknown: true},
67+
expected: types.Float64Unknown(),
6868
},
6969
"float64-value": {
7070
tfType: tftypes.NewValue(tftypes.Number, big.NewFloat(1.2)),
7171
attrType: types.Float64Type,
72-
expected: types.Float64{Value: 1.2},
72+
expected: types.Float64Value(1.2),
7373
},
7474
"int64-null": {
7575
tfType: tftypes.NewValue(tftypes.Number, nil),
7676
attrType: types.Int64Type,
77-
expected: types.Int64{Null: true},
77+
expected: types.Int64Null(),
7878
},
7979
"int64-unknown": {
8080
tfType: tftypes.NewValue(tftypes.Number, tftypes.UnknownValue),
8181
attrType: types.Int64Type,
82-
expected: types.Int64{Unknown: true},
82+
expected: types.Int64Unknown(),
8383
},
8484
"int64-value": {
8585
tfType: tftypes.NewValue(tftypes.Number, 123),
8686
attrType: types.Int64Type,
87-
expected: types.Int64{Value: 123},
87+
expected: types.Int64Value(123),
8888
},
8989
"list-null": {
9090
tfType: tftypes.NewValue(
@@ -96,10 +96,7 @@ func TestValue(t *testing.T) {
9696
attrType: types.ListType{
9797
ElemType: types.StringType,
9898
},
99-
expected: types.List{
100-
ElemType: types.StringType,
101-
Null: true,
102-
},
99+
expected: types.ListNull(types.StringType),
103100
},
104101
"list-unknown": {
105102
tfType: tftypes.NewValue(
@@ -111,10 +108,7 @@ func TestValue(t *testing.T) {
111108
attrType: types.ListType{
112109
ElemType: types.StringType,
113110
},
114-
expected: types.List{
115-
ElemType: types.StringType,
116-
Unknown: true,
117-
},
111+
expected: types.ListUnknown(types.StringType),
118112
},
119113
"list-value": {
120114
tfType: tftypes.NewValue(
@@ -128,27 +122,27 @@ func TestValue(t *testing.T) {
128122
attrType: types.ListType{
129123
ElemType: types.StringType,
130124
},
131-
expected: types.List{
132-
ElemType: types.StringType,
133-
Elems: []attr.Value{
134-
types.String{Value: "test-value"},
125+
expected: types.ListValueMust(
126+
types.StringType,
127+
[]attr.Value{
128+
types.StringValue("test-value"),
135129
},
136-
},
130+
),
137131
},
138132
"number-null": {
139133
tfType: tftypes.NewValue(tftypes.Number, nil),
140134
attrType: types.NumberType,
141-
expected: types.Number{Null: true},
135+
expected: types.NumberNull(),
142136
},
143137
"number-unknown": {
144138
tfType: tftypes.NewValue(tftypes.Number, tftypes.UnknownValue),
145139
attrType: types.NumberType,
146-
expected: types.Number{Unknown: true},
140+
expected: types.NumberUnknown(),
147141
},
148142
"number-value": {
149143
tfType: tftypes.NewValue(tftypes.Number, big.NewFloat(1.2)),
150144
attrType: types.NumberType,
151-
expected: types.Number{Value: big.NewFloat(1.2)},
145+
expected: types.NumberValue(big.NewFloat(1.2)),
152146
},
153147
"object-null": {
154148
tfType: tftypes.NewValue(
@@ -164,12 +158,11 @@ func TestValue(t *testing.T) {
164158
"test_attr": types.StringType,
165159
},
166160
},
167-
expected: types.Object{
168-
AttrTypes: map[string]attr.Type{
161+
expected: types.ObjectNull(
162+
map[string]attr.Type{
169163
"test_attr": types.StringType,
170164
},
171-
Null: true,
172-
},
165+
),
173166
},
174167
"object-unknown": {
175168
tfType: tftypes.NewValue(
@@ -185,12 +178,11 @@ func TestValue(t *testing.T) {
185178
"test_attr": types.StringType,
186179
},
187180
},
188-
expected: types.Object{
189-
AttrTypes: map[string]attr.Type{
181+
expected: types.ObjectUnknown(
182+
map[string]attr.Type{
190183
"test_attr": types.StringType,
191184
},
192-
Unknown: true,
193-
},
185+
),
194186
},
195187
"object-value": {
196188
tfType: tftypes.NewValue(
@@ -208,14 +200,14 @@ func TestValue(t *testing.T) {
208200
"test_attr": types.StringType,
209201
},
210202
},
211-
expected: types.Object{
212-
AttrTypes: map[string]attr.Type{
203+
expected: types.ObjectValueMust(
204+
map[string]attr.Type{
213205
"test_attr": types.StringType,
214206
},
215-
Attrs: map[string]attr.Value{
216-
"test_attr": types.String{Value: "test-value"},
207+
map[string]attr.Value{
208+
"test_attr": types.StringValue("test-value"),
217209
},
218-
},
210+
),
219211
},
220212
"set-null": {
221213
tfType: tftypes.NewValue(
@@ -227,10 +219,7 @@ func TestValue(t *testing.T) {
227219
attrType: types.SetType{
228220
ElemType: types.StringType,
229221
},
230-
expected: types.Set{
231-
ElemType: types.StringType,
232-
Null: true,
233-
},
222+
expected: types.SetNull(types.StringType),
234223
},
235224
"set-unknown": {
236225
tfType: tftypes.NewValue(
@@ -242,10 +231,7 @@ func TestValue(t *testing.T) {
242231
attrType: types.SetType{
243232
ElemType: types.StringType,
244233
},
245-
expected: types.Set{
246-
ElemType: types.StringType,
247-
Unknown: true,
248-
},
234+
expected: types.SetUnknown(types.StringType),
249235
},
250236
"set-value": {
251237
tfType: tftypes.NewValue(
@@ -259,27 +245,27 @@ func TestValue(t *testing.T) {
259245
attrType: types.SetType{
260246
ElemType: types.StringType,
261247
},
262-
expected: types.Set{
263-
ElemType: types.StringType,
264-
Elems: []attr.Value{
265-
types.String{Value: "test-value"},
248+
expected: types.SetValueMust(
249+
types.StringType,
250+
[]attr.Value{
251+
types.StringValue("test-value"),
266252
},
267-
},
253+
),
268254
},
269255
"string-null": {
270256
tfType: tftypes.NewValue(tftypes.String, nil),
271257
attrType: types.StringType,
272-
expected: types.String{Null: true},
258+
expected: types.StringNull(),
273259
},
274260
"string-unknown": {
275261
tfType: tftypes.NewValue(tftypes.String, tftypes.UnknownValue),
276262
attrType: types.StringType,
277-
expected: types.String{Unknown: true},
263+
expected: types.StringUnknown(),
278264
},
279265
"string-value": {
280266
tfType: tftypes.NewValue(tftypes.String, "test-value"),
281267
attrType: types.StringType,
282-
expected: types.String{Value: "test-value"},
268+
expected: types.StringValue("test-value"),
283269
},
284270
}
285271

0 commit comments

Comments
 (0)