Skip to content

Commit eb1ecde

Browse files
bfladbflad
authored
tfsdk: Added framework-specific error diagnostics when Resource implementations errantly return no errors and empty state after Create and Update methods (#406)
Reference: #403
1 parent 90e2017 commit eb1ecde

8 files changed

+607
-47
lines changed

‎.changelog/406.txt

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:enhancement
2+
tfsdk: Added framework-specific error diagnostics when `Resource` implementations errantly return no errors and empty state after `Create` and `Update` methods
3+
```

‎internal/fwserver/server_applyresourcechange_test.go

+154-10
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ func TestServerApplyResourceChange(t *testing.T) {
112112
if data.TestRequired.Value != "test-config-value" {
113113
resp.Diagnostics.AddError("unexpected req.Config value: %s", data.TestRequired.Value)
114114
}
115+
116+
// Prevent missing resource state error diagnostic
117+
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
115118
},
116119
DeleteMethod: func(_ context.Context, _ tfsdk.DeleteResourceRequest, resp *tfsdk.DeleteResourceResponse) {
117120
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Delete")
@@ -124,8 +127,13 @@ func TestServerApplyResourceChange(t *testing.T) {
124127
},
125128
},
126129
expectedResponse: &fwserver.ApplyResourceChangeResponse{
127-
// Intentionally empty, Create implementation does not call resp.State.Set()
128-
NewState: testEmptyState,
130+
NewState: &tfsdk.State{
131+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
132+
"test_computed": tftypes.NewValue(tftypes.String, nil),
133+
"test_required": tftypes.NewValue(tftypes.String, "test-config-value"),
134+
}),
135+
Schema: testSchema,
136+
},
129137
},
130138
},
131139
"create-request-plannedstate": {
@@ -156,6 +164,9 @@ func TestServerApplyResourceChange(t *testing.T) {
156164
if data.TestRequired.Value != "test-plannedstate-value" {
157165
resp.Diagnostics.AddError("unexpected req.Plan value: %s", data.TestRequired.Value)
158166
}
167+
168+
// Prevent missing resource state error diagnostic
169+
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
159170
},
160171
DeleteMethod: func(_ context.Context, _ tfsdk.DeleteResourceRequest, resp *tfsdk.DeleteResourceResponse) {
161172
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Delete")
@@ -168,15 +179,27 @@ func TestServerApplyResourceChange(t *testing.T) {
168179
},
169180
},
170181
expectedResponse: &fwserver.ApplyResourceChangeResponse{
171-
// Intentionally empty, Create implementation does not call resp.State.Set()
172-
NewState: testEmptyState,
182+
NewState: &tfsdk.State{
183+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
184+
"test_computed": tftypes.NewValue(tftypes.String, nil),
185+
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
186+
}),
187+
Schema: testSchema,
188+
},
173189
},
174190
},
175191
"create-request-providermeta": {
176192
server: &fwserver.Server{
177193
Provider: &testprovider.Provider{},
178194
},
179195
request: &fwserver.ApplyResourceChangeRequest{
196+
PlannedState: &tfsdk.Plan{
197+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
198+
"test_computed": tftypes.NewValue(tftypes.String, nil),
199+
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
200+
}),
201+
Schema: testSchema,
202+
},
180203
PriorState: testEmptyState,
181204
ResourceSchema: testSchema,
182205
ResourceType: &testprovider.ResourceType{
@@ -186,13 +209,19 @@ func TestServerApplyResourceChange(t *testing.T) {
186209
NewResourceMethod: func(_ context.Context, _ tfsdk.Provider) (tfsdk.Resource, diag.Diagnostics) {
187210
return &testprovider.Resource{
188211
CreateMethod: func(ctx context.Context, req tfsdk.CreateResourceRequest, resp *tfsdk.CreateResourceResponse) {
189-
var data testProviderMetaData
212+
var metadata testProviderMetaData
190213

191-
resp.Diagnostics.Append(req.ProviderMeta.Get(ctx, &data)...)
214+
resp.Diagnostics.Append(req.ProviderMeta.Get(ctx, &metadata)...)
192215

193-
if data.TestProviderMetaAttribute.Value != "test-provider-meta-value" {
194-
resp.Diagnostics.AddError("unexpected req.ProviderMeta value: %s", data.TestProviderMetaAttribute.Value)
216+
if metadata.TestProviderMetaAttribute.Value != "test-provider-meta-value" {
217+
resp.Diagnostics.AddError("Unexpected req.ProviderMeta Value", "Got: "+metadata.TestProviderMetaAttribute.Value)
195218
}
219+
220+
// Prevent missing resource state error diagnostic
221+
var data testSchemaData
222+
223+
resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
224+
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
196225
},
197226
DeleteMethod: func(_ context.Context, _ tfsdk.DeleteResourceRequest, resp *tfsdk.DeleteResourceResponse) {
198227
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Delete")
@@ -206,8 +235,13 @@ func TestServerApplyResourceChange(t *testing.T) {
206235
ProviderMeta: testProviderMetaConfig,
207236
},
208237
expectedResponse: &fwserver.ApplyResourceChangeResponse{
209-
// Intentionally empty, Create implementation does not call resp.State.Set()
210-
NewState: testEmptyState,
238+
NewState: &tfsdk.State{
239+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
240+
"test_computed": tftypes.NewValue(tftypes.String, nil),
241+
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
242+
}),
243+
Schema: testSchema,
244+
},
211245
},
212246
},
213247
"create-response-diagnostics": {
@@ -305,6 +339,59 @@ func TestServerApplyResourceChange(t *testing.T) {
305339
},
306340
},
307341
},
342+
"create-response-newstate-null": {
343+
server: &fwserver.Server{
344+
Provider: &testprovider.Provider{},
345+
},
346+
request: &fwserver.ApplyResourceChangeRequest{
347+
Config: &tfsdk.Config{
348+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
349+
"test_computed": tftypes.NewValue(tftypes.String, nil),
350+
"test_required": tftypes.NewValue(tftypes.String, "test-config-value"),
351+
}),
352+
Schema: testSchema,
353+
},
354+
PlannedState: &tfsdk.Plan{
355+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
356+
"test_computed": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
357+
"test_required": tftypes.NewValue(tftypes.String, "test-config-value"),
358+
}),
359+
Schema: testSchema,
360+
},
361+
PriorState: testEmptyState,
362+
ResourceSchema: testSchema,
363+
ResourceType: &testprovider.ResourceType{
364+
GetSchemaMethod: func(_ context.Context) (tfsdk.Schema, diag.Diagnostics) {
365+
return testSchema, nil
366+
},
367+
NewResourceMethod: func(_ context.Context, _ tfsdk.Provider) (tfsdk.Resource, diag.Diagnostics) {
368+
return &testprovider.Resource{
369+
CreateMethod: func(ctx context.Context, req tfsdk.CreateResourceRequest, resp *tfsdk.CreateResourceResponse) {
370+
// Intentionally missing resp.State.Set()
371+
},
372+
DeleteMethod: func(_ context.Context, _ tfsdk.DeleteResourceRequest, resp *tfsdk.DeleteResourceResponse) {
373+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Delete")
374+
},
375+
UpdateMethod: func(_ context.Context, _ tfsdk.UpdateResourceRequest, resp *tfsdk.UpdateResourceResponse) {
376+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Update")
377+
},
378+
}, nil
379+
},
380+
},
381+
},
382+
expectedResponse: &fwserver.ApplyResourceChangeResponse{
383+
Diagnostics: diag.Diagnostics{
384+
diag.NewErrorDiagnostic(
385+
"Missing Resource State After Create",
386+
"The Terraform Provider unexpectedly returned no resource state after having no errors in the resource creation. "+
387+
"This is always an issue in the Terraform Provider and should be reported to the provider developers.\n\n"+
388+
"The resource may have been successfully created, but Terraform is not tracking it. "+
389+
"Applying the configuration again with no other action may result in duplicate resource errors.",
390+
),
391+
},
392+
NewState: testEmptyState,
393+
},
394+
},
308395
"delete-request-priorstate": {
309396
server: &fwserver.Server{
310397
Provider: &testprovider.Provider{},
@@ -862,6 +949,63 @@ func TestServerApplyResourceChange(t *testing.T) {
862949
},
863950
},
864951
},
952+
"update-response-newstate-null": {
953+
server: &fwserver.Server{
954+
Provider: &testprovider.Provider{},
955+
},
956+
request: &fwserver.ApplyResourceChangeRequest{
957+
Config: &tfsdk.Config{
958+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
959+
"test_computed": tftypes.NewValue(tftypes.String, nil),
960+
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
961+
}),
962+
Schema: testSchema,
963+
},
964+
PlannedState: &tfsdk.Plan{
965+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
966+
"test_computed": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
967+
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
968+
}),
969+
Schema: testSchema,
970+
},
971+
PriorState: &tfsdk.State{
972+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
973+
"test_computed": tftypes.NewValue(tftypes.String, nil),
974+
"test_required": tftypes.NewValue(tftypes.String, "test-old-value"),
975+
}),
976+
Schema: testSchema,
977+
},
978+
ResourceSchema: testSchema,
979+
ResourceType: &testprovider.ResourceType{
980+
GetSchemaMethod: func(_ context.Context) (tfsdk.Schema, diag.Diagnostics) {
981+
return testSchema, nil
982+
},
983+
NewResourceMethod: func(_ context.Context, _ tfsdk.Provider) (tfsdk.Resource, diag.Diagnostics) {
984+
return &testprovider.Resource{
985+
CreateMethod: func(_ context.Context, _ tfsdk.CreateResourceRequest, resp *tfsdk.CreateResourceResponse) {
986+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Create")
987+
},
988+
DeleteMethod: func(_ context.Context, _ tfsdk.DeleteResourceRequest, resp *tfsdk.DeleteResourceResponse) {
989+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Delete")
990+
},
991+
UpdateMethod: func(ctx context.Context, req tfsdk.UpdateResourceRequest, resp *tfsdk.UpdateResourceResponse) {
992+
resp.State.RemoveResource(ctx)
993+
},
994+
}, nil
995+
},
996+
},
997+
},
998+
expectedResponse: &fwserver.ApplyResourceChangeResponse{
999+
Diagnostics: diag.Diagnostics{
1000+
diag.NewErrorDiagnostic(
1001+
"Missing Resource State After Update",
1002+
"The Terraform Provider unexpectedly returned no resource state after having no errors in the resource update. "+
1003+
"This is always an issue in the Terraform Provider and should be reported to the provider developers.",
1004+
),
1005+
},
1006+
NewState: testEmptyState,
1007+
},
1008+
},
8651009
}
8661010

8671011
for name, testCase := range testCases {

‎internal/fwserver/server_createresource.go

+21-3
Original file line numberDiff line numberDiff line change
@@ -46,20 +46,22 @@ func (s *Server) CreateResource(ctx context.Context, req *CreateResourceRequest,
4646
return
4747
}
4848

49+
nullSchemaData := tftypes.NewValue(req.ResourceSchema.TerraformType(ctx), nil)
50+
4951
createReq := tfsdk.CreateResourceRequest{
5052
Config: tfsdk.Config{
5153
Schema: req.ResourceSchema,
52-
Raw: tftypes.NewValue(req.ResourceSchema.TerraformType(ctx), nil),
54+
Raw: nullSchemaData,
5355
},
5456
Plan: tfsdk.Plan{
5557
Schema: req.ResourceSchema,
56-
Raw: tftypes.NewValue(req.ResourceSchema.TerraformType(ctx), nil),
58+
Raw: nullSchemaData,
5759
},
5860
}
5961
createResp := tfsdk.CreateResourceResponse{
6062
State: tfsdk.State{
6163
Schema: req.ResourceSchema,
62-
Raw: tftypes.NewValue(req.ResourceSchema.TerraformType(ctx), nil),
64+
Raw: nullSchemaData,
6365
},
6466
}
6567

@@ -81,4 +83,20 @@ func (s *Server) CreateResource(ctx context.Context, req *CreateResourceRequest,
8183

8284
resp.Diagnostics = createResp.Diagnostics
8385
resp.NewState = &createResp.State
86+
87+
if !resp.Diagnostics.HasError() && createResp.State.Raw.Equal(nullSchemaData) {
88+
detail := "The Terraform Provider unexpectedly returned no resource state after having no errors in the resource creation. " +
89+
"This is always an issue in the Terraform Provider and should be reported to the provider developers.\n\n" +
90+
"The resource may have been successfully created, but Terraform is not tracking it. " +
91+
"Applying the configuration again with no other action may result in duplicate resource errors."
92+
93+
if _, ok := resource.(tfsdk.ResourceWithImportState); ok {
94+
detail += " Import the resource if the resource was actually created and Terraform should be tracking it."
95+
}
96+
97+
resp.Diagnostics.AddError(
98+
"Missing Resource State After Create",
99+
detail,
100+
)
101+
}
84102
}

0 commit comments

Comments
 (0)