From 4e7dd87b3b28c45f9639f3a8f5a2fb7a30d52dc4 Mon Sep 17 00:00:00 2001 From: Jan Schreier Date: Fri, 4 Sep 2026 15:20:18 +0200 Subject: [PATCH] fix(sfs): guard against nil responses from the API The generated SDK decoder returns early for an empty body, so Execute() hands back (nil, nil) for a 2xx response that carries no body, and a wait handler returns (nil, nil) when a single poll hits a retryable 502 or 504: WaiterHelper reports (true, nil, err), handleError swallows the retryable status and returns a nil error, and the loop then returns the nil response because it is already done. The SFS resources and data sources dereferenced those responses unchecked and crashed the provider process. Several of the checks meant to validate a response were themselves the dereference - they tested `x.Field == nil` without first testing `x == nil` - and in create the wait handler result was read by tflog.SetField one line before its guard. Reproduced for resourcepool Read with terraform-plugin-testing: a refresh against a 200 with an empty body panics with "invalid memory address or nil pointer dereference". The guards follow the shape already used in the package, one combined condition covering the response and the fields actually used. Two error titles in the resource pool and share update paths said "creating" and are corrected while their lines are touched. Left unchanged: export-policy and snapshot-policy already guard their responses, project-lock uses nil-receiver-safe getters, and every Delete discards the response, so the delete waiter's legitimate nil return on a 404 is harmless. --- .../services/sfs/resourcepool/datasource.go | 5 + .../services/sfs/resourcepool/resource.go | 27 +++-- stackit/internal/services/sfs/sfs_test.go | 98 +++++++++++++++++++ .../internal/services/sfs/share/datasource.go | 5 + .../internal/services/sfs/share/resource.go | 29 ++++-- .../services/sfs/snapshots/datasource.go | 5 + 6 files changed, 154 insertions(+), 15 deletions(-) diff --git a/stackit/internal/services/sfs/resourcepool/datasource.go b/stackit/internal/services/sfs/resourcepool/datasource.go index 65c2c89af..474b10ccf 100644 --- a/stackit/internal/services/sfs/resourcepool/datasource.go +++ b/stackit/internal/services/sfs/resourcepool/datasource.go @@ -114,6 +114,11 @@ func (r *resourcePoolDataSource) Read(ctx context.Context, req datasource.ReadRe ctx = core.LogResponse(ctx) + if response == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading resource pool", "Calling API: Empty response") + return + } + // Map response body to schema err = mapDataSourceFields(ctx, region, response.ResourcePool, &model) if err != nil { diff --git a/stackit/internal/services/sfs/resourcepool/resource.go b/stackit/internal/services/sfs/resourcepool/resource.go index 096510ceb..37733d6d0 100644 --- a/stackit/internal/services/sfs/resourcepool/resource.go +++ b/stackit/internal/services/sfs/resourcepool/resource.go @@ -304,20 +304,24 @@ func (r *resourcePoolResource) Create(ctx context.Context, req resource.CreateRe core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", fmt.Sprintf("resource pool creation waiting: %v", err)) return } + if response == nil || response.ResourcePool == nil || response.ResourcePool.Id == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", "Calling API: Incomplete response (id missing)") + return + } ctx = tflog.SetField(ctx, "resource_pool_id", response.ResourcePool.Id) // the responses of create and update are not compatible, so we can't use a unified // mapFields function. Therefore, we issue a GET request after the create // to get a compatible structure - if response.ResourcePool == nil || response.ResourcePool.Id == nil { - core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", "response did not contain an ID") - return - } getResponse, err := r.client.DefaultAPI.GetResourcePool(ctx, projectId, region, *response.ResourcePool.Id).Execute() if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", fmt.Sprintf("resource pool get: %v", err)) return } + if getResponse == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", "Calling API: Empty response") + return + } // Map response body to schema err = mapFields(ctx, region, getResponse.ResourcePool, &model) @@ -371,6 +375,11 @@ func (r *resourcePoolResource) Read(ctx context.Context, req resource.ReadReques ctx = core.LogResponse(ctx) + if response == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading resource pool", "Calling API: Empty response") + return + } + // Map response body to schema err = mapFields(ctx, region, response.ResourcePool, &model) if err != nil { @@ -438,14 +447,18 @@ func (r *resourcePoolResource) Update(ctx context.Context, req resource.UpdateRe // the responses of create and update are not compatible, so we can't use a unified // mapFields function. Therefore, we issue a GET request after the create // to get a compatible structure - if response.ResourcePool == nil || response.ResourcePool.Id == nil { - core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", "response did not contain an ID") + if response == nil || response.ResourcePool == nil || response.ResourcePool.Id == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating resource pool", "Calling API: Incomplete response (id missing)") return } getResponse, err := wait.UpdateResourcePoolWaitHandler(ctx, r.client.DefaultAPI, projectId, region, resourcePoolId).WaitWithContext(ctx) if err != nil { - core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", fmt.Sprintf("resource pool get: %v", err)) + core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating resource pool", fmt.Sprintf("resource pool update waiting: %v", err)) + return + } + if getResponse == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating resource pool", "Calling API: Empty response") return } err = mapFields(ctx, region, getResponse.ResourcePool, &model) diff --git a/stackit/internal/services/sfs/sfs_test.go b/stackit/internal/services/sfs/sfs_test.go index 5567f9c54..295960ba6 100644 --- a/stackit/internal/services/sfs/sfs_test.go +++ b/stackit/internal/services/sfs/sfs_test.go @@ -162,3 +162,101 @@ resource "stackit_sfs_share" "example" { }, }) } + +// TestSfsResourcePoolReadHandlesEmptyResponse covers a 2xx answer with an empty body. The generated SDK decoder +// returns early for an empty body, so Execute hands back (nil, nil) and the resource used to dereference that nil. +func TestSfsResourcePoolReadHandlesEmptyResponse(t *testing.T) { + projectId := uuid.NewString() + resourcePoolId := uuid.NewString() + const region = "eu01" + + s := testutil.NewMockServer(t) + defer s.Server.Close() + tfConfig := fmt.Sprintf(` +provider "stackit" { + default_region = "%s" + sfs_custom_endpoint = "%s" + service_account_token = "mock-server-needs-no-auth" + enable_beta_resources = true +} +resource "stackit_sfs_resource_pool" "resourcepool" { + project_id = "%s" + name = "sfs-instance" + availability_zone = "eu01-m" + performance_class = "Standard" + size_gigabytes = 512 + ip_acl = ["192.168.2.0/24"] +} +`, region, s.Server.URL, projectId) + + resource.UnitTest(t, resource.TestCase{ + ProtoV6ProviderFactories: testutil.TestAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + // Fail the create wait so the IDs are in state and the next step can refresh them. + PreConfig: func() { + s.Reset( + testutil.MockResponse{ + Description: "create resource pool", + ToJsonBody: sfs.CreateResourcePoolResponse{ + ResourcePool: &sfs.ResourcePool{Id: new(resourcePoolId)}, + }, + }, + testutil.MockResponse{Description: "failing waiter", StatusCode: http.StatusInternalServerError}, + ) + }, + Config: tfConfig, + ExpectError: regexp.MustCompile("Error creating resource pool"), + }, + { + PreConfig: func() { + s.Reset( + testutil.MockResponse{Description: "refresh with an empty body", StatusCode: http.StatusOK}, + testutil.MockResponse{Description: "delete", StatusCode: http.StatusAccepted}, + testutil.MockResponse{Description: "delete waiter", StatusCode: http.StatusNotFound}, + ) + }, + RefreshState: true, + ExpectError: regexp.MustCompile("Error reading resource pool"), + }, + }, + }) +} + +// TestSfsShareCreateHandlesEmptyResponse covers the same empty-body case on the share create call, where the check +// meant to validate the response was itself the dereference. +func TestSfsShareCreateHandlesEmptyResponse(t *testing.T) { + projectId := uuid.NewString() + resourcePoolId := uuid.NewString() + const region = "eu01" + + s := testutil.NewMockServer(t, + testutil.MockResponse{Description: "create share with an empty body", StatusCode: http.StatusOK}, + ) + defer s.Server.Close() + tfConfig := fmt.Sprintf(` +provider "stackit" { + default_region = "%s" + sfs_custom_endpoint = "%s" + service_account_token = "mock-server-needs-no-auth" + enable_beta_resources = true +} +resource "stackit_sfs_share" "example" { + project_id = "%s" + resource_pool_id = "%s" + name = "my-nfs-share" + export_policy = "high-performance-class" + space_hard_limit_gigabytes = 32 +} +`, region, s.Server.URL, projectId, resourcePoolId) + + resource.UnitTest(t, resource.TestCase{ + ProtoV6ProviderFactories: testutil.TestAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: tfConfig, + ExpectError: regexp.MustCompile("Incomplete response"), + }, + }, + }) +} diff --git a/stackit/internal/services/sfs/share/datasource.go b/stackit/internal/services/sfs/share/datasource.go index 28899614e..6353047d6 100644 --- a/stackit/internal/services/sfs/share/datasource.go +++ b/stackit/internal/services/sfs/share/datasource.go @@ -108,6 +108,11 @@ func (r *shareDataSource) Read(ctx context.Context, req datasource.ReadRequest, ctx = core.LogResponse(ctx) + if response == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading share", "Calling API: Empty response") + return + } + // Map response body to schema err = mapDataSourceFields(ctx, region, response.Share, &model) if err != nil { diff --git a/stackit/internal/services/sfs/share/resource.go b/stackit/internal/services/sfs/share/resource.go index e5814636b..413750a4e 100644 --- a/stackit/internal/services/sfs/share/resource.go +++ b/stackit/internal/services/sfs/share/resource.go @@ -243,7 +243,7 @@ func (r *shareResource) Create(ctx context.Context, req resource.CreateRequest, ctx = core.LogResponse(ctx) - if share.Share == nil || share.Share.Id == nil { + if share == nil || share.Share == nil || share.Share.Id == nil { core.LogAndAddError(ctx, &resp.Diagnostics, "error creating share", "Calling API: Incomplete response (id missing)") return } @@ -264,20 +264,24 @@ func (r *shareResource) Create(ctx context.Context, req resource.CreateRequest, core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating share", fmt.Sprintf("share creation waiting: %v", err)) return } + if response == nil || response.Share == nil || response.Share.Id == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating share", "Calling API: Incomplete response (id missing)") + return + } ctx = tflog.SetField(ctx, "share_id", response.Share.Id) // the responses of create and update are not compatible, so we can't use a unified // mapFields function. Therefore, we issue a GET request after the create // to get a compatible structure - if response.Share == nil || response.Share.Id == nil { - core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating share", "response did not contain an ID") - return - } getResponse, err := r.client.DefaultAPI.GetShare(ctx, projectId, region, resourcePoolId, *response.Share.Id).Execute() if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating share", fmt.Sprintf("share get: %v", err)) return } + if getResponse == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating share", "Calling API: Empty response") + return + } // Map response body to schema err = mapFields(ctx, getResponse.Share, region, &model) @@ -334,6 +338,11 @@ func (r *shareResource) Read(ctx context.Context, req resource.ReadRequest, resp ctx = core.LogResponse(ctx) + if response == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading share", "Calling API: Empty response") + return + } + // Map response body to schema err = mapFields(ctx, response.Share, region, &model) if err != nil { @@ -403,14 +412,18 @@ func (r *shareResource) Update(ctx context.Context, req resource.UpdateRequest, // the responses of create and update are not compatible, so we can't use a unified // mapFields function. Therefore, we issue a GET request after the create // to get a compatible structure - if response.Share == nil || response.Share.Id == nil { - core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating share", "response did not contain an ID") + if response == nil || response.Share == nil || response.Share.Id == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating share", "Calling API: Incomplete response (id missing)") return } getResponse, err := wait.UpdateShareWaitHandler(ctx, r.client.DefaultAPI, projectId, region, resourcePoolId, shareId).WaitWithContext(ctx) if err != nil { - core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating share", fmt.Sprintf("share get: %v", err)) + core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating share", fmt.Sprintf("share update waiting: %v", err)) + return + } + if getResponse == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating share", "Calling API: Empty response") return } err = mapFields(ctx, getResponse.Share, region, &model) diff --git a/stackit/internal/services/sfs/snapshots/datasource.go b/stackit/internal/services/sfs/snapshots/datasource.go index c4ce23f42..df4f053d1 100644 --- a/stackit/internal/services/sfs/snapshots/datasource.go +++ b/stackit/internal/services/sfs/snapshots/datasource.go @@ -124,6 +124,11 @@ func (r *resourcePoolSnapshotDataSource) Read(ctx context.Context, req datasourc ctx = core.LogResponse(ctx) + if response == nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading resource pool snapshot", "Calling API: Empty response") + return + } + // Map response body to schema err = mapDataSourceFields(ctx, region, &response.ResourcePoolSnapshots, &model) if err != nil {