resource(instance): fix provider crash when boot_disk_id is empty

[sudomateo]

Added a nil pointer check to prevent a nil pointer dereference causing the provider to crash when boot_disk_id is empty and disk_attachments is non-empty.

The following configuration now successfully applies.

resource "oxide_instance" "example" {
  project_id       = data.oxide_project.example.id
  description      = "example"
  name             = "example"
  host_name        = "example"
  memory           = 1024 * 1024 * 1024
  ncpus            = 2
  disk_attachments = [oxide_disk.example.id]
}

Fixes #466.

[sudomateo]

I'll get either a new acceptance test or an updated acceptance test pushed to this tomorrow. Long day!

[karencfv]

Thanks for fixing this! I'll wait for the acceptance test to review again :)

[karencfv]

Looks like there's already a test for this case

terraform-provider-oxide/internal/provider/resource_instance_test.go

Lines 46 to 55 in 7af9fa6

	resource "oxide_instance" "{{.BlockName}}" {
	project_id = data.oxide_project.{{.SupportBlockName}}.id
	description = "a test instance"
	name = "{{.InstanceName}}"
	host_name = "terraform-acc-myhost"
	memory = 1073741824
	ncpus = 1
	start_on_create = false
	}
	`

I'm surprised this only just started failing! Must have been some change in the API we didn't catch. I guess just running the existing instance tests should be fine to test this change then!

[sudomateo]

I'm surprised this only just started failing! Must have been some change in the API we didn't catch. I guess just running the existing instance tests should be fine to test this change then!

Sorry I should have been more descriptive in the pull request description. This issue only occurred when boot_disk_id is empty and disk_attachments is non-empty since the loop where the nil pointer dereference occurs won't be entered on an empty disk_attachments.

We don't have a specific test for this since the existing tests either omit both boot_disk_id and disk_attachments or include both of them. I'll add a new test for this.

[sudomateo]

Added the acceptance test.

Without this change.

> TEST_ACC_NAME=TestAccCloudResourceInstance_no_boot_disk make testacc
-> Running terraform acceptance tests
=== RUN   TestAccCloudResourceInstance_no_boot_disk
=== PAUSE TestAccCloudResourceInstance_no_boot_disk
=== CONT  TestAccCloudResourceInstance_no_boot_disk
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xf1f013]

goroutine 292 [running]:
github.com/oxidecomputer/terraform-provider-oxide/internal/provider.filterBootDiskFromDisks(...)
        /home/sudomateo/oxide/terraform-provider-oxide/internal/provider/resource_instance.go:1225
github.com/oxidecomputer/terraform-provider-oxide/internal/provider.(*instanceResource).Create(0xc0001655f8, {0x15134d0, 0xc0007a79e0}, {{{{0x1519d90, 0xc0007b1a10}, {0x11a20a0, 0xc0007b1320}}, {0x151bce0, 0xc0006c7d10}}, {{{0x1519d90, ...}, ...}, ...}, ...}, ...)
        /home/sudomateo/oxide/terraform-provider-oxide/internal/provider/resource_instance.go:417 +0xc73
github.com/hashicorp/terraform-plugin-framework/internal/fwserver.(*Server).CreateResource(0xc00043c288, {0x15134d0, 0xc0007a79e0}, 0xc0000494c0, 0xc000049490)
        /home/sudomateo/.local/gopath/pkg/mod/github.com/hashicorp/terraform-plugin-framework@v1.15.0/internal/fwserver/server_createresource.go:127 +0x88e
github.com/hashicorp/terraform-plugin-framework/internal/fwserver.(*Server).ApplyResourceChange(0xc00043c288, {0x15134d0, 0xc0007a79e0}, 0xc0002af110, 0xc0000495e8)
        /home/sudomateo/.local/gopath/pkg/mod/github.com/hashicorp/terraform-plugin-framework@v1.15.0/internal/fwserver/server_applyresourcechange.go:63 +0x586
github.com/hashicorp/terraform-plugin-framework/internal/proto6server.(*Server).ApplyResourceChange(0xc00043c288, {0x15134d0?, 0xc0007a7920?}, 0xc00078d860)
        /home/sudomateo/.local/gopath/pkg/mod/github.com/hashicorp/terraform-plugin-framework@v1.15.0/internal/proto6server/server_applyresourcechange.go:71 +0x585
github.com/hashicorp/terraform-plugin-go/tfprotov6/tf6server.(*server).ApplyResourceChange(0xc0002a5540, {0x15134d0?, 0xc0007a7140?}, 0xc000144800)
        /home/sudomateo/.local/gopath/pkg/mod/github.com/hashicorp/terraform-plugin-go@v0.28.0/tfprotov6/tf6server/server.go:941 +0x3b9
github.com/hashicorp/terraform-plugin-go/tfprotov6/internal/tfplugin6._Provider_ApplyResourceChange_Handler({0x12e9160, 0xc0002a5540}, {0x15134d0, 0xc0007a7140}, 0xc000144700, 0x0)
        /home/sudomateo/.local/gopath/pkg/mod/github.com/hashicorp/terraform-plugin-go@v0.28.0/tfprotov6/internal/tfplugin6/tfplugin6_grpc.pb.go:687 +0x1a6
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0006ae000, {0x15134d0, 0xc0007a70b0}, 0xc0004c15c0, 0xc0004eec30, 0x1dc1d88, 0x0)
        /home/sudomateo/.local/gopath/pkg/mod/google.golang.org/grpc@v1.72.1/server.go:1405 +0x1036
google.golang.org/grpc.(*Server).handleStream(0xc0006ae000, {0x1514340, 0xc0006ba340}, 0xc0004c15c0)
        /home/sudomateo/.local/gopath/pkg/mod/google.golang.org/grpc@v1.72.1/server.go:1815 +0xb88
google.golang.org/grpc.(*Server).serveStreams.func2.1()
        /home/sudomateo/.local/gopath/pkg/mod/google.golang.org/grpc@v1.72.1/server.go:1035 +0x7f
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 279
        /home/sudomateo/.local/gopath/pkg/mod/google.golang.org/grpc@v1.72.1/server.go:1046 +0x11d
FAIL    github.com/oxidecomputer/terraform-provider-oxide/internal/provider     5.911s
FAIL
make: *** [Makefile:82: testacc] Error 1

With this change.

> TEST_ACC_NAME=TestAccCloudResourceInstance_no_boot_disk make testacc
-> Running terraform acceptance tests
=== RUN   TestAccCloudResourceInstance_no_boot_disk
=== PAUSE TestAccCloudResourceInstance_no_boot_disk
=== CONT  TestAccCloudResourceInstance_no_boot_disk
--- PASS: TestAccCloudResourceInstance_no_boot_disk (16.28s)
PASS
ok      github.com/oxidecomputer/terraform-provider-oxide/internal/provider     16.289s

All instance tests.

> TEST_ACC_NAME=TestAccCloudResourceInstance OXIDE_TEST_IP_POOL_NAME=private make test
acc
-> Running terraform acceptance tests
=== RUN   TestAccCloudResourceInstance_full
=== PAUSE TestAccCloudResourceInstance_full
=== RUN   TestAccCloudResourceInstance_extIPs
=== PAUSE TestAccCloudResourceInstance_extIPs
=== RUN   TestAccCloudResourceInstance_sshKeys
=== PAUSE TestAccCloudResourceInstance_sshKeys
=== RUN   TestAccCloudResourceInstance_nic
=== PAUSE TestAccCloudResourceInstance_nic
=== RUN   TestAccCloudResourceInstance_disk
=== PAUSE TestAccCloudResourceInstance_disk
=== RUN   TestAccCloudResourceInstance_update
=== PAUSE TestAccCloudResourceInstance_update
=== RUN   TestAccCloudResourceInstance_no_boot_disk
=== PAUSE TestAccCloudResourceInstance_no_boot_disk
=== RUN   TestAccCloudResourceInstance_antiAffinityGroups
=== PAUSE TestAccCloudResourceInstance_antiAffinityGroups
=== CONT  TestAccCloudResourceInstance_full
=== CONT  TestAccCloudResourceInstance_disk
=== CONT  TestAccCloudResourceInstance_no_boot_disk
=== CONT  TestAccCloudResourceInstance_antiAffinityGroups
=== CONT  TestAccCloudResourceInstance_update
=== CONT  TestAccCloudResourceInstance_nic
--- PASS: TestAccCloudResourceInstance_no_boot_disk (29.61s)
=== CONT  TestAccCloudResourceInstance_sshKeys
--- PASS: TestAccCloudResourceInstance_full (42.26s)
=== CONT  TestAccCloudResourceInstance_extIPs
--- PASS: TestAccCloudResourceInstance_sshKeys (15.71s)
--- PASS: TestAccCloudResourceInstance_antiAffinityGroups (47.63s)
--- PASS: TestAccCloudResourceInstance_nic (54.03s)
--- PASS: TestAccCloudResourceInstance_disk (69.60s)
--- PASS: TestAccCloudResourceInstance_update (74.57s)
--- PASS: TestAccCloudResourceInstance_extIPs (68.25s)
PASS
ok      github.com/oxidecomputer/terraform-provider-oxide/internal/provider     110.524s

[karencfv]

Thanks for the fix!