Repository: honeycombio/terraform-provider-honeycombio (language: python)

Issue:
plugin crash report
<!---
Thank you for taking the time to report bugs!

We love code snippets and links to repositories that reproduce the issue, but understand if you don't have the time to add them. We'll do our best with the info you provide, and might ask follow-up questions.

Please see our [OSS process document](https://github.com/honeycombio/home/blob/main/honeycomb-oss-lifecycle-and-practices.md#) to get an idea of how we operate.
--->

**Versions**
0.24.0

**Steps to reproduce**
unsure. It used to run fine on `0.24.0` and crashed as I was manually cleaning up resources (boards and triggers). Just crashed three times in a row during the planning phase.

**Additional context**
Stack trace from the terraform-provider-honeycombio_v0.24.0 plugin:

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xbc49c5]

goroutine 630 [running]:
github.com/honeycombio/terraform-provider-honeycombio/honeycombio.resourceDatasetDefinitionRead({0xf18500, 0xc0001e97a0}, 0x0?, {0xceb500?, 0xc0003ff200})
	github.com/honeycombio/terraform-provider-honeycombio/honeycombio/resource_dataset_definition.go:78 +0x685
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).read(0xc0002540e0, {0xf18458, 0xc000551da0}, 0xd?, {0xceb500, 0xc0003ff200})
	github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/schema/resource.go:823 +0x11b
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).RefreshWithoutUpgrade(0xc0002540e0, {0xf18458, 0xc000551da0}, 0xc000317520, {0xceb500, 0xc0003ff200})
	github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/schema/resource.go:1117 +0x552
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ReadResource(0xc0000117d0, {0xf18458?, 0xc000551cb0?}, 0xc0007f2b00)
	github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/schema/grpc_provider.go:708 +0x6ca
github.com/hashicorp/terraform-plugin-mux/tf5muxserver.(*muxServer).ReadResource(0xf18490?, {0xf18458?, 0xc0005519b0?}, 0xc0007f2b00)
	github.com/hashicorp/terraform-plugin-mux@v0.16.0/tf5muxserver/mux_server_ReadResource.go:35 +0x193
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ReadResource(0xc0001e57c0, {0xf18458?, 0xc000550600?}, 0xc0001e8d90)
	github.com/hashicorp/terraform-plugin-go@v0.23.0/tfprotov5/tf5server/server.go:783 +0x309
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ReadResource_Handler({0xd90920?, 0xc0001e57c0}, {0xf18458, 0xc000550600}, 0xc000889f00, 0x0)
	github.com/hashicorp/terraform-plugin-go@v0.23.0/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:482 +0x169
google.golang.org/grpc.(*Server).processUnaryRPC(0xc000117000, {0xf18458, 0xc000550570}, {0xf1e040, 0xc0001ea480}, 0xc000705200, 0xc0002f3350, 0x14c6c08, 0x0)
	google.golang.org/grpc@v1.63.2/server.go:1369 +0xe23
google.golang.org/grpc.(*Server).handleStream(0xc000117000, {0xf1e040, 0xc0001ea480}, 0xc000705200)
	google.golang.org/grpc@v1.63.2/server.go:1780 +0x1016
google.golang.org/grpc.(*Server).serveStreams.func2.1()
	google.golang.org/grpc@v1.63.2/server.go:1019 +0x8b
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 13
	google.golang.org/grpc@v1.63.2/server.go:1030 +0x135

Error: The terraform-provider-honeycombio_v0.24.0 plugin crashed!

This is always indicative of a bug within the plugin. It would be immensely
helpful if you could report the crash with the plugin's maintainers so that it
can be fixed. The output above should help diagnose the issue.
```


Discussion / hints:

Fix patch:
diff --git a/honeycombio/resource_dataset_definition.go b/honeycombio/resource_dataset_definition.go
index c2f0c5fa..65d4ce17 100644
--- a/honeycombio/resource_dataset_definition.go
+++ b/honeycombio/resource_dataset_definition.go
@@ -75,6 +75,12 @@ func resourceDatasetDefinitionRead(ctx context.Context, d *schema.ResourceData,
 
 	name := d.Get("name").(string)
 	column := extractDatasetDefinitionColumnByName(dd, name)
+	if column == nil {
+		// Definition used to be set but was removed without TF knowning about it,
+		// so we're trying to read a definition that has a "null" value.
+		d.SetId("")
+		return nil
+	}
 
 	d.SetId(strconv.Itoa(hashcode.String(fmt.Sprintf("%s-%s", dataset, name))))
 	d.Set("name", name)


Test patch:
diff --git a/honeycombio/resource_dataset_definition_test.go b/honeycombio/resource_dataset_definition_test.go
index 68c77f00..b3cb4762 100644
--- a/honeycombio/resource_dataset_definition_test.go
+++ b/honeycombio/resource_dataset_definition_test.go
@@ -11,21 +11,15 @@ import (
 
 	honeycombio "github.com/honeycombio/terraform-provider-honeycombio/client"
 	"github.com/honeycombio/terraform-provider-honeycombio/internal/helper/test"
-
-	"github.com/stretchr/testify/assert"
 )
 
 func TestAccHoneycombioDatasetDefinition_basic(t *testing.T) {
 	dataset := testAccDataset()
 	col1Name := test.RandomStringWithPrefix("test.", 8)
 	col2Name := test.RandomStringWithPrefix("test.", 8)
+	col3Name := test.RandomStringWithPrefix("test.", 8)
 
-	resource.Test(t, resource.TestCase{
-		PreCheck:                 testAccPreCheck(t),
-		ProtoV5ProviderFactories: testAccProtoV5ProviderFactory,
-		Steps: []resource.TestStep{
-			{
-				Config: fmt.Sprintf(`
+	config := fmt.Sprintf(`
 resource "honeycombio_column" "column_1" {
   dataset = "%[1]s"
   name    = "%[2]s"
@@ -36,6 +30,11 @@ resource "honeycombio_column" "column_2" {
   name    = "%[3]s"
 }
 
+resource "honeycombio_column" "column_3" {
+  dataset = "%[1]s"
+  name    = "%[4]s"
+}
+
 resource "honeycombio_derived_column" "log10_duration" {
   dataset = "%[1]s"
 
@@ -63,7 +62,21 @@ resource "honeycombio_dataset_definition" "route" {
   name   = "route"
   column = honeycombio_column.column_2.key_name
 }
-`, dataset, col1Name, col2Name),
+
+resource "honeycombio_dataset_definition" "error" {
+  dataset = "%[1]s"
+
+  name   = "error"
+  column = honeycombio_column.column_3.key_name
+}
+`, dataset, col1Name, col2Name, col3Name)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 testAccPreCheck(t),
+		ProtoV5ProviderFactories: testAccProtoV5ProviderFactory,
+		Steps: []resource.TestStep{
+			{
+				Config: config,
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("honeycombio_dataset_definition.name", "name", "name"),
 					resource.TestCheckResourceAttr("honeycombio_dataset_definition.name", "column", col1Name),
@@ -76,31 +89,42 @@ resource "honeycombio_dataset_definition" "route" {
 					resource.TestCheckResourceAttr("honeycombio_dataset_definition.route", "column_type", "column"),
 				),
 			},
+			{
+				// remove the 'error' definition and ensure reading the definitions still works
+				PreConfig: func() {
+					ctx := context.Background()
+					client := testAccClient(t)
+					client.DatasetDefinitions.Update(ctx, dataset, &honeycombio.DatasetDefinition{
+						Error: &honeycombio.DefinitionColumn{Name: ""},
+					})
+				},
+				Config: config,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("honeycombio_dataset_definition.error", "name", "error"),
+					resource.TestCheckResourceAttr("honeycombio_dataset_definition.error", "column", col3Name),
+					resource.TestCheckResourceAttr("honeycombio_dataset_definition.error", "column_type", "column"),
+				),
+			},
 		},
-		CheckDestroy: resource.ComposeTestCheckFunc(
+		CheckDestroy: func(s *terraform.State) error {
 			// ensure that after destroying ('deleting') the above definitions
 			// they have been reset to their default values
-			testAccCheckDatasetDefinitionResetToDefault(t, dataset, "name"),
-			testAccCheckDatasetDefinitionResetToDefault(t, dataset, "duration_ms"),
-			testAccCheckDatasetDefinitionResetToDefault(t, dataset, "route"),
-		),
+			client := testAccClient(t)
+			dd, err := client.DatasetDefinitions.Get(context.Background(), dataset)
+			if err != nil {
+				return fmt.Errorf("could not lookup dataset definitions: %w", err)
+			}
+			for _, defn := range []string{"name", "duration_ms", "route", "error"} {
+				column := extractDatasetDefinitionColumnByName(dd, defn)
+				if column == nil {
+					continue // definition was removed and does not have default
+				}
+				if slices.Contains(honeycombio.DatasetDefinitionDefaults()[defn], column.Name) {
+					continue // definition was removed and reset to default
+				}
+				return fmt.Errorf("definition %q was not reset to default: %v", defn, column)
+			}
+			return nil
+		},
 	})
 }
-
-func testAccCheckDatasetDefinitionResetToDefault(t *testing.T, dataset string, name string) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		client := testAccClient(t)
-		dd, err := client.DatasetDefinitions.Get(context.Background(), dataset)
-		if err != nil {
-			return fmt.Errorf("could not lookup dataset definitions: %w", err)
-		}
-
-		// defaults would be nil or one of the values in `DatasetDefinitionColumnDefaults`
-		column := extractDatasetDefinitionColumnByName(dd, name)
-		if column != nil {
-			assert.True(t, slices.Contains(honeycombio.DatasetDefinitionDefaults()[name], column.Name))
-		}
-
-		return nil
-	}
-}
diff --git a/scripts/setup-testsuite-dataset b/scripts/setup-testsuite-dataset
index 6ce2fc98..df0c3cd2 100755
--- a/scripts/setup-testsuite-dataset
+++ b/scripts/setup-testsuite-dataset
@@ -44,7 +44,7 @@ create_dataset_if_missing () {
       log "error creating dataset \"$name\""
     fi
 
-    log "created dataset \"$name}\""
+    log "created dataset \"${name}\""
   fi
 }