HPR-1208: Fix panic when hcp_packer_image points to an empty channel (

#533)

* Add test for empty channel

* Fix panics

* Add changelog

---------

Co-authored-by: Wilken Rivera <dev@wilkenrivera.com>

.changelog/533.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+Fixed panic when `hcp_packer_image` points to a channel without an assigned iteration.
+```

internal/provider/data_source_packer_image.go

@@ -59,12 +59,13 @@ If a project is not configured in the HCP Provider config block, the oldest proj
 				Type:         schema.TypeString,
 				Optional:     true,
 				Computed:     true,
-				ExactlyOneOf: []string{"channel"},
+				ExactlyOneOf: []string{"iteration_id", "channel"},
 			},
 			"channel": {
-				Description: "The channel that points to the version of the image being retrieved. Either this or `iteration_id` must be specified. Note: will incur a billable request",
-				Type:        schema.TypeString,
-				Optional:    true,
+				Description:  "The channel that points to the version of the image being retrieved. Either this or `iteration_id` must be specified. Note: will incur a billable request",
+				Type:         schema.TypeString,
+				Optional:     true,
+				ExactlyOneOf: []string{"iteration_id", "channel"},
 			},
 			"component_type": {
 				Description: "Name of the builder that built this image. Ex: `amazon-ebs.example`.",
@@ -150,10 +151,8 @@ func dataSourcePackerImageRead(ctx context.Context, d *schema.ResourceData, meta
 		}
 	}
 
-	var channel *packermodels.HashicorpCloudPackerChannel
-
 	if channelSlug != "" {
-		channel, err = clients.GetPackerChannelBySlug(
+		channel, err := clients.GetPackerChannelBySlug(
 			ctx,
 			client,
 			loc,
@@ -162,11 +161,9 @@ func dataSourcePackerImageRead(ctx context.Context, d *schema.ResourceData, meta
 		if err != nil {
 			return diag.FromErr(err)
 		}
-	}
-
-	// Assuming we passed the above check, the rest of the channel is not
-	// used after that,
-	if channel != nil {
+		if channel.Iteration == nil {
+			return diag.Errorf("Channel does not have an assigned iteration (channel: %s)", channelSlug)
+		}
 		iteration = channel.Iteration
 	}
 

internal/provider/data_source_packer_image_iteration_test.go

@@ -320,7 +320,7 @@ func upsertBuild(t *testing.T, bucketSlug, fingerprint, iterationID string) {
 	}
 }
 
-func createChannel(t *testing.T, bucketSlug, channelSlug, iterationID string) {
+func upsertChannel(t *testing.T, bucketSlug, channelSlug, iterationID string) {
 	t.Helper()
 
 	client := testAccProvider.Meta().(*clients.Client)
@@ -484,7 +484,7 @@ func TestAcc_dataSourcePacker(t *testing.T) {
 						t.Fatal(err.Error())
 					}
 					upsertBuild(t, acctestAlpineBucket, fingerprint, itID)
-					createChannel(t, acctestAlpineBucket, acctestProductionChannel, itID)
+					upsertChannel(t, acctestAlpineBucket, acctestProductionChannel, itID)
 				},
 				Config: testConfig(testAccPackerAlpineProductionImage),
 				Check: resource.ComposeTestCheckFunc(
@@ -522,7 +522,7 @@ func TestAcc_dataSourcePacker_revokedIteration(t *testing.T) {
 						t.Fatal(err.Error())
 					}
 					upsertBuild(t, acctestUbuntuBucket, fingerprint, itID)
-					createChannel(t, acctestUbuntuBucket, acctestProductionChannel, itID)
+					upsertChannel(t, acctestUbuntuBucket, acctestProductionChannel, itID)
 					// Schedule revocation to the future, otherwise we won't be able to revoke an iteration that
 					// it's assigned to a channel
 					revokeIteration(t, itID, acctestUbuntuBucket, revokeAt)

internal/provider/data_source_packer_image_test.go

@@ -139,7 +139,7 @@ func TestAcc_dataSourcePackerImage(t *testing.T) {
 						t.Fatal(err.Error())
 					}
 					upsertBuild(t, acctestImageBucket, fingerprint, itID)
-					createChannel(t, acctestImageBucket, acctestImageChannel, itID)
+					upsertChannel(t, acctestImageBucket, acctestImageChannel, itID)
 				},
 				Config: testAccPackerImageAlpineProduction,
 				Check: resource.ComposeTestCheckFunc(
@@ -183,12 +183,10 @@ func TestAcc_dataSourcePackerImage_revokedIteration(t *testing.T) {
 						t.Fatal(err.Error())
 					}
 					upsertBuild(t, acctestUbuntuImageBucket, fingerprint, itID)
-					createChannel(t, acctestUbuntuImageBucket, acctestImageChannel, itID)
+					upsertChannel(t, acctestUbuntuImageBucket, acctestImageChannel, itID)
 					// Schedule revocation to the future, otherwise we won't be able to revoke an iteration that
 					// it's assigned to a channel
 					revokeIteration(t, itID, acctestUbuntuImageBucket, revokeAt)
-					// Sleep to make sure the iteration is revoked when we test
-					time.Sleep(5 * time.Second)
 				},
 				Config: testConfig(testAccPackerImageUbuntuProduction),
 				Check: resource.ComposeTestCheckFunc(
@@ -200,6 +198,30 @@ func TestAcc_dataSourcePackerImage_revokedIteration(t *testing.T) {
 	})
 }
 
+func TestAcc_dataSourcePackerImage_emptyChannel(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t, map[string]bool{"aws": false, "azure": false}) },
+		ProviderFactories: providerFactories,
+		CheckDestroy: func(*terraform.State) error {
+			deleteChannel(t, acctestArchImageBucket, acctestImageChannel, true)
+			deleteBucket(t, acctestArchImageBucket, true)
+			return nil
+		},
+		Steps: []resource.TestStep{
+			{
+				PlanOnly: true,
+				PreConfig: func() {
+					upsertRegistry(t)
+					upsertBucket(t, acctestArchImageBucket)
+					upsertChannel(t, acctestArchImageBucket, acctestImageChannel, "")
+				},
+				Config:      testConfig(testAccPackerImageArchProduction),
+				ExpectError: regexp.MustCompile(`.*Channel does not have an assigned iteration.*`),
+			},
+		},
+	})
+}
+
 func TestAcc_dataSourcePackerImage_channelAndIterationIDReject(t *testing.T) {
 	fingerprint := "rejectIterationAndChannel"
 	configs := []string{
@@ -228,7 +250,7 @@ func TestAcc_dataSourcePackerImage_channelAndIterationIDReject(t *testing.T) {
 							t.Fatal(err.Error())
 						}
 						upsertBuild(t, acctestArchImageBucket, fingerprint, itID)
-						createChannel(t, acctestArchImageBucket, acctestImageChannel, itID)
+						upsertChannel(t, acctestArchImageBucket, acctestImageChannel, itID)
 					},
 					Config:      testConfig(cfg),
 					ExpectError: regexp.MustCompile("Error: Invalid combination of arguments"),
@@ -261,7 +283,7 @@ func TestAcc_dataSourcePackerImage_channelAccept(t *testing.T) {
 						t.Fatal(err.Error())
 					}
 					upsertBuild(t, acctestArchImageBucket, fingerprint, itID)
-					createChannel(t, acctestArchImageBucket, acctestImageChannel, itID)
+					upsertChannel(t, acctestArchImageBucket, acctestImageChannel, itID)
 				},
 				Config: testConfig(testAccPackerImageArchProduction),
 				Check: resource.ComposeTestCheckFunc(

internal/provider/data_source_packer_iteration_test.go

@@ -65,7 +65,7 @@ func TestAcc_dataSourcePackerIteration(t *testing.T) {
 						t.Fatal(err.Error())
 					}
 					upsertBuild(t, acctestIterationBucket, fingerprint, itID)
-					createChannel(t, acctestIterationBucket, acctestIterationChannel, itID)
+					upsertChannel(t, acctestIterationBucket, acctestIterationChannel, itID)
 				},
 				Config: testConfig(testAccPackerIterationAlpineProduction),
 				Check: resource.ComposeTestCheckFunc(
@@ -103,7 +103,7 @@ func TestAcc_dataSourcePackerIteration_revokedIteration(t *testing.T) {
 						t.Fatal(err.Error())
 					}
 					upsertBuild(t, acctestIterationUbuntuBucket, fingerprint, itID)
-					createChannel(t, acctestIterationUbuntuBucket, acctestIterationChannel, itID)
+					upsertChannel(t, acctestIterationUbuntuBucket, acctestIterationChannel, itID)
 					// Schedule revocation to the future, otherwise we won't be able to revoke an iteration that
 					// it's assigned to a channel
 					revokeIteration(t, itID, acctestIterationUbuntuBucket, revokeAt)