Skip to content

fix(sfs): guard against nil responses from the API #1741

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

Open
devpie wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(sfs): guard against nil responses from the API #1741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions stackit/internal/services/sfs/resourcepool/datasource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
27 changes: 20 additions & 7 deletions stackit/internal/services/sfs/resourcepool/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
98 changes: 98 additions & 0 deletions stackit/internal/services/sfs/sfs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
},
},
})
}
5 changes: 5 additions & 0 deletions stackit/internal/services/sfs/share/datasource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
29 changes: 21 additions & 8 deletions stackit/internal/services/sfs/share/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions stackit/internal/services/sfs/snapshots/datasource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down