Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tfsdk: Added framework-specific error diagnostics when Resource implementations errantly return no errors and empty state after Create and Update methods #406

Merged
merged 4 commits into from
Jul 8, 2022
Merged

tfsdk: Added framework-specific error diagnostics when Resource implementations errantly return no errors and empty state after Create and Update methods #406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7df7468
tfsdk: Added framework-specific error diagnostics when `Resource` imp…
bflad Jul 8, 2022
6ef587a
Update CHANGELOG for #406
bflad Jul 8, 2022
530333f
internal/fwserver: Expand Missing Resource State After Create diagnos…
bflad Jul 8, 2022
a28cbf2
Merge branch 'main' into bflad-create-update-empty-state-diags
bflad Jul 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/406.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
tfsdk: Added framework-specific error diagnostics when `Resource` implementations errantly return no errors and empty state after `Create` and `Update` methods
```
164 changes: 154 additions & 10 deletions internal/fwserver/server_applyresourcechange_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ func TestServerApplyResourceChange(t *testing.T) {
if data.TestRequired.Value != "test-config-value" {
resp.Diagnostics.AddError("unexpected req.Config value: %s", data.TestRequired.Value)
}

// Prevent missing resource state error diagnostic
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
},
DeleteMethod: func(_ context.Context, _ tfsdk.DeleteResourceRequest, resp *tfsdk.DeleteResourceResponse) {
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Delete")
Expand All @@ -124,8 +127,13 @@ func TestServerApplyResourceChange(t *testing.T) {
},
},
expectedResponse: &fwserver.ApplyResourceChangeResponse{
// Intentionally empty, Create implementation does not call resp.State.Set()
NewState: testEmptyState,
NewState: &tfsdk.State{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, nil),
"test_required": tftypes.NewValue(tftypes.String, "test-config-value"),
}),
Schema: testSchema,
},
},
},
"create-request-plannedstate": {
Expand Down Expand Up @@ -156,6 +164,9 @@ func TestServerApplyResourceChange(t *testing.T) {
if data.TestRequired.Value != "test-plannedstate-value" {
resp.Diagnostics.AddError("unexpected req.Plan value: %s", data.TestRequired.Value)
}

// Prevent missing resource state error diagnostic
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
},
DeleteMethod: func(_ context.Context, _ tfsdk.DeleteResourceRequest, resp *tfsdk.DeleteResourceResponse) {
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Delete")
Expand All @@ -168,15 +179,27 @@ func TestServerApplyResourceChange(t *testing.T) {
},
},
expectedResponse: &fwserver.ApplyResourceChangeResponse{
// Intentionally empty, Create implementation does not call resp.State.Set()
NewState: testEmptyState,
NewState: &tfsdk.State{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, nil),
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
}),
Schema: testSchema,
},
},
},
"create-request-providermeta": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
},
request: &fwserver.ApplyResourceChangeRequest{
PlannedState: &tfsdk.Plan{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, nil),
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
}),
Schema: testSchema,
},
PriorState: testEmptyState,
ResourceSchema: testSchema,
ResourceType: &testprovider.ResourceType{
Expand All @@ -186,13 +209,19 @@ func TestServerApplyResourceChange(t *testing.T) {
NewResourceMethod: func(_ context.Context, _ tfsdk.Provider) (tfsdk.Resource, diag.Diagnostics) {
return &testprovider.Resource{
CreateMethod: func(ctx context.Context, req tfsdk.CreateResourceRequest, resp *tfsdk.CreateResourceResponse) {
var data testProviderMetaData
var metadata testProviderMetaData

resp.Diagnostics.Append(req.ProviderMeta.Get(ctx, &data)...)
resp.Diagnostics.Append(req.ProviderMeta.Get(ctx, &metadata)...)

if data.TestProviderMetaAttribute.Value != "test-provider-meta-value" {
resp.Diagnostics.AddError("unexpected req.ProviderMeta value: %s", data.TestProviderMetaAttribute.Value)
if metadata.TestProviderMetaAttribute.Value != "test-provider-meta-value" {
resp.Diagnostics.AddError("Unexpected req.ProviderMeta Value", "Got: "+metadata.TestProviderMetaAttribute.Value)
}

// Prevent missing resource state error diagnostic
var data testSchemaData

resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
},
DeleteMethod: func(_ context.Context, _ tfsdk.DeleteResourceRequest, resp *tfsdk.DeleteResourceResponse) {
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Delete")
Expand All @@ -206,8 +235,13 @@ func TestServerApplyResourceChange(t *testing.T) {
ProviderMeta: testProviderMetaConfig,
},
expectedResponse: &fwserver.ApplyResourceChangeResponse{
// Intentionally empty, Create implementation does not call resp.State.Set()
NewState: testEmptyState,
NewState: &tfsdk.State{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, nil),
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
}),
Schema: testSchema,
},
},
},
"create-response-diagnostics": {
Expand Down Expand Up @@ -305,6 +339,59 @@ func TestServerApplyResourceChange(t *testing.T) {
},
},
},
"create-response-newstate-null": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
},
request: &fwserver.ApplyResourceChangeRequest{
Config: &tfsdk.Config{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, nil),
"test_required": tftypes.NewValue(tftypes.String, "test-config-value"),
}),
Schema: testSchema,
},
PlannedState: &tfsdk.Plan{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
"test_required": tftypes.NewValue(tftypes.String, "test-config-value"),
}),
Schema: testSchema,
},
PriorState: testEmptyState,
ResourceSchema: testSchema,
ResourceType: &testprovider.ResourceType{
GetSchemaMethod: func(_ context.Context) (tfsdk.Schema, diag.Diagnostics) {
return testSchema, nil
},
NewResourceMethod: func(_ context.Context, _ tfsdk.Provider) (tfsdk.Resource, diag.Diagnostics) {
return &testprovider.Resource{
CreateMethod: func(ctx context.Context, req tfsdk.CreateResourceRequest, resp *tfsdk.CreateResourceResponse) {
// Intentionally missing resp.State.Set()
},
DeleteMethod: func(_ context.Context, _ tfsdk.DeleteResourceRequest, resp *tfsdk.DeleteResourceResponse) {
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Delete")
},
UpdateMethod: func(_ context.Context, _ tfsdk.UpdateResourceRequest, resp *tfsdk.UpdateResourceResponse) {
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Update")
},
}, nil
},
},
},
expectedResponse: &fwserver.ApplyResourceChangeResponse{
Diagnostics: diag.Diagnostics{
diag.NewErrorDiagnostic(
"Missing Resource State After Create",
"The Terraform Provider unexpectedly returned no resource state after having no errors in the resource creation. "+
"This is always an issue in the Terraform Provider and should be reported to the provider developers.\n\n"+
"The resource may have been successfully created, but Terraform is not tracking it. "+
"Applying the configuration again with no other action may result in duplicate resource errors.",
),
},
NewState: testEmptyState,
},
},
"delete-request-priorstate": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
Expand Down Expand Up @@ -862,6 +949,63 @@ func TestServerApplyResourceChange(t *testing.T) {
},
},
},
"update-response-newstate-null": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
},
request: &fwserver.ApplyResourceChangeRequest{
Config: &tfsdk.Config{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, nil),
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
}),
Schema: testSchema,
},
PlannedState: &tfsdk.Plan{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
}),
Schema: testSchema,
},
PriorState: &tfsdk.State{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, nil),
"test_required": tftypes.NewValue(tftypes.String, "test-old-value"),
}),
Schema: testSchema,
},
ResourceSchema: testSchema,
ResourceType: &testprovider.ResourceType{
GetSchemaMethod: func(_ context.Context) (tfsdk.Schema, diag.Diagnostics) {
return testSchema, nil
},
NewResourceMethod: func(_ context.Context, _ tfsdk.Provider) (tfsdk.Resource, diag.Diagnostics) {
return &testprovider.Resource{
CreateMethod: func(_ context.Context, _ tfsdk.CreateResourceRequest, resp *tfsdk.CreateResourceResponse) {
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Create")
},
DeleteMethod: func(_ context.Context, _ tfsdk.DeleteResourceRequest, resp *tfsdk.DeleteResourceResponse) {
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Delete")
},
UpdateMethod: func(ctx context.Context, req tfsdk.UpdateResourceRequest, resp *tfsdk.UpdateResourceResponse) {
resp.State.RemoveResource(ctx)
},
}, nil
},
},
},
expectedResponse: &fwserver.ApplyResourceChangeResponse{
Diagnostics: diag.Diagnostics{
diag.NewErrorDiagnostic(
"Missing Resource State After Update",
"The Terraform Provider unexpectedly returned no resource state after having no errors in the resource update. "+
"This is always an issue in the Terraform Provider and should be reported to the provider developers.",
),
},
NewState: testEmptyState,
},
},
}

for name, testCase := range testCases {
Expand Down
24 changes: 21 additions & 3 deletions internal/fwserver/server_createresource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,22 @@ func (s *Server) CreateResource(ctx context.Context, req *CreateResourceRequest,
return
}

nullSchemaData := tftypes.NewValue(req.ResourceSchema.TerraformType(ctx), nil)

createReq := tfsdk.CreateResourceRequest{
Config: tfsdk.Config{
Schema: req.ResourceSchema,
Raw: tftypes.NewValue(req.ResourceSchema.TerraformType(ctx), nil),
Raw: nullSchemaData,
},
Plan: tfsdk.Plan{
Schema: req.ResourceSchema,
Raw: tftypes.NewValue(req.ResourceSchema.TerraformType(ctx), nil),
Raw: nullSchemaData,
},
}
createResp := tfsdk.CreateResourceResponse{
State: tfsdk.State{
Schema: req.ResourceSchema,
Raw: tftypes.NewValue(req.ResourceSchema.TerraformType(ctx), nil),
Raw: nullSchemaData,
},
}

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

resp.Diagnostics = createResp.Diagnostics
resp.NewState = &createResp.State

if !resp.Diagnostics.HasError() && createResp.State.Raw.Equal(nullSchemaData) {
detail := "The Terraform Provider unexpectedly returned no resource state after having no errors in the resource creation. " +
"This is always an issue in the Terraform Provider and should be reported to the provider developers.\n\n" +
"The resource may have been successfully created, but Terraform is not tracking it. " +
"Applying the configuration again with no other action may result in duplicate resource errors."

if _, ok := resource.(tfsdk.ResourceWithImportState); ok {
detail += " Import the resource if the resource was actually created and Terraform should be tracking it."
}

resp.Diagnostics.AddError(
"Missing Resource State After Create",
detail,
)
}
}
Loading