diff --git a/internal/provider/data_source_packer_image.go b/internal/provider/data_source_packer_image.go index a17dc243a..2f938f283 100644 --- a/internal/provider/data_source_packer_image.go +++ b/internal/provider/data_source_packer_image.go @@ -114,7 +114,7 @@ func dataSourcePackerImageRead(ctx context.Context, d *schema.ResourceData, meta if !time.Time(iteration.RevokeAt).IsZero() { // If RevokeAt is not a zero date, it means this iteration is revoked and should not be used // to build new images. - return diag.Errorf("the iteration %s is revoked and can not be used", iteration.ID) + return diag.Errorf("the iteration %s is revoked and can not be used. A valid iteration should be used instead.", iteration.ID) } found := false diff --git a/internal/provider/data_source_packer_image_iteration.go b/internal/provider/data_source_packer_image_iteration.go index e28b04763..e1a6963e1 100644 --- a/internal/provider/data_source_packer_image_iteration.go +++ b/internal/provider/data_source_packer_image_iteration.go @@ -171,7 +171,8 @@ func dataSourcePackerImageIterationRead(ctx context.Context, d *schema.ResourceD if !time.Time(iteration.RevokeAt).IsZero() { // If RevokeAt is not a zero date, it means this iteration is revoked and should not be used // to build new images. - return diag.Errorf("the iteration %s is revoked and can not be used", iteration.ID) + return diag.Errorf("the iteration %s assigned to channel %s is revoked and can not be used. A valid iteration"+ + " must be assigned to this channel before proceeding", iteration.ID, channelSlug) } d.SetId(iteration.ID) diff --git a/internal/provider/data_source_packer_image_iteration_test.go b/internal/provider/data_source_packer_image_iteration_test.go index 19160457c..119a84d8b 100644 --- a/internal/provider/data_source_packer_image_iteration_test.go +++ b/internal/provider/data_source_packer_image_iteration_test.go @@ -208,7 +208,7 @@ func revokeIteration(t *testing.T, iterationID, bucketSlug, revokeIn string) { } } -func getIterationIDFromFingerPrint(t *testing.T, bucketSlug, fingerprint string) string { +func getIterationIDFromFingerPrint(t *testing.T, bucketSlug string, fingerprint string) (string, error) { t.Helper() client := testAccProvider.Meta().(*clients.Client) @@ -225,9 +225,9 @@ func getIterationIDFromFingerPrint(t *testing.T, bucketSlug, fingerprint string) ok, err := client.Packer.PackerServiceGetIteration(getItParams, nil) if err != nil { - t.Fatal(err) + return "", err } - return ok.Payload.Iteration.ID + return ok.Payload.Iteration.ID, nil } func upsertBuild(t *testing.T, bucketSlug, fingerprint, iterationID string) { @@ -357,7 +357,7 @@ func updateChannel(t *testing.T, bucketSlug, channelSlug, iterationID string) { t.Errorf("unexpected UpdateChannel error, expected nil. Got %v", err) } -func deleteBucket(t *testing.T, bucketSlug string) { +func deleteBucket(t *testing.T, bucketSlug string, logOnError bool) { t.Helper() client := testAccProvider.Meta().(*clients.Client) @@ -375,10 +375,12 @@ func deleteBucket(t *testing.T, bucketSlug string) { if err == nil { return } - t.Errorf("unexpected DeleteBucket error, expected nil. Got %v", err) + if logOnError { + t.Logf("unexpected DeleteBucket error, expected nil. Got %v", err) + } } -func deleteIteration(t *testing.T, bucketSlug string, iterationID string) { +func deleteIteration(t *testing.T, bucketSlug string, iterationFingerprint string, logOnError bool) { t.Helper() client := testAccProvider.Meta().(*clients.Client) @@ -387,20 +389,30 @@ func deleteIteration(t *testing.T, bucketSlug string, iterationID string) { ProjectID: client.Config.ProjectID, } + iterationID, err := getIterationIDFromFingerPrint(t, acctestIterationUbuntuBucket, iterationFingerprint) + if err != nil { + if logOnError { + t.Logf(err.Error()) + } + return + } + deleteItParams := packer_service.NewPackerServiceDeleteIterationParams() deleteItParams.LocationOrganizationID = loc.OrganizationID deleteItParams.LocationProjectID = loc.ProjectID deleteItParams.BucketSlug = &bucketSlug deleteItParams.IterationID = iterationID - _, err := client.Packer.PackerServiceDeleteIteration(deleteItParams, nil) + _, err = client.Packer.PackerServiceDeleteIteration(deleteItParams, nil) if err == nil { return } - t.Errorf("unexpected DeleteIteration error, expected nil. Got %v", err) + if logOnError { + t.Logf("unexpected DeleteIteration error, expected nil. Got %v", err) + } } -func deleteChannel(t *testing.T, bucketSlug string, channelSlug string) { +func deleteChannel(t *testing.T, bucketSlug string, channelSlug string, logOnError bool) { t.Helper() client := testAccProvider.Meta().(*clients.Client) @@ -419,7 +431,9 @@ func deleteChannel(t *testing.T, bucketSlug string, channelSlug string) { if err == nil { return } - t.Errorf("unexpected DeleteChannel error, expected nil. Got %v", err) + if logOnError { + t.Logf("unexpected DeleteChannel error, expected nil. Got %v", err) + } } func TestAcc_dataSourcePacker(t *testing.T) { @@ -430,10 +444,9 @@ func TestAcc_dataSourcePacker(t *testing.T) { PreCheck: func() { testAccPreCheck(t, false) }, ProviderFactories: providerFactories, CheckDestroy: func(*terraform.State) error { - itID := getIterationIDFromFingerPrint(t, acctestAlpineBucket, fingerprint) - deleteChannel(t, acctestAlpineBucket, acctestProductionChannel) - deleteIteration(t, acctestAlpineBucket, itID) - deleteBucket(t, acctestAlpineBucket) + deleteChannel(t, acctestAlpineBucket, acctestProductionChannel, false) + deleteIteration(t, acctestAlpineBucket, fingerprint, false) + deleteBucket(t, acctestAlpineBucket, false) return nil }, @@ -445,7 +458,10 @@ func TestAcc_dataSourcePacker(t *testing.T) { upsertRegistry(t) upsertBucket(t, acctestAlpineBucket) upsertIteration(t, acctestAlpineBucket, fingerprint) - itID := getIterationIDFromFingerPrint(t, acctestAlpineBucket, fingerprint) + itID, err := getIterationIDFromFingerPrint(t, acctestAlpineBucket, fingerprint) + if err != nil { + t.Fatal(err.Error()) + } upsertBuild(t, acctestAlpineBucket, fingerprint, itID) createChannel(t, acctestAlpineBucket, acctestProductionChannel, itID) }, @@ -465,21 +481,24 @@ func TestAcc_dataSourcePacker_revokedIteration(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t, false) }, ProviderFactories: providerFactories, - CheckDestroy: func(*terraform.State) error { - itID := getIterationIDFromFingerPrint(t, acctestUbuntuBucket, fingerprint) - deleteChannel(t, acctestUbuntuBucket, acctestProductionChannel) - deleteIteration(t, acctestUbuntuBucket, itID) - deleteBucket(t, acctestUbuntuBucket) - return nil - }, Steps: []resource.TestStep{ // testing that getting a revoked iteration fails properly { PreConfig: func() { + // CheckDestroy doesn't get called when the test fails and doesn't + // produce any tf state. In this case we destroy any existing resource + // before creating them. + deleteChannel(t, acctestUbuntuBucket, acctestProductionChannel, false) + deleteIteration(t, acctestUbuntuBucket, fingerprint, false) + deleteBucket(t, acctestUbuntuBucket, false) + upsertRegistry(t) upsertBucket(t, acctestUbuntuBucket) upsertIteration(t, acctestUbuntuBucket, fingerprint) - itID := getIterationIDFromFingerPrint(t, acctestUbuntuBucket, fingerprint) + itID, err := getIterationIDFromFingerPrint(t, acctestUbuntuBucket, fingerprint) + if err != nil { + t.Fatal(err.Error()) + } upsertBuild(t, acctestUbuntuBucket, fingerprint, itID) createChannel(t, acctestUbuntuBucket, acctestProductionChannel, itID) // Schedule revocation to the future, otherwise we won't be able to revoke an iteration that @@ -489,8 +508,7 @@ func TestAcc_dataSourcePacker_revokedIteration(t *testing.T) { time.Sleep(5 * time.Second) }, Config: testConfig(testAccPackerUbuntuProductionImage), - PlanOnly: true, - ExpectError: regexp.MustCompile(`Error: the iteration (\d|\w){26} is revoked and can not be used`), + ExpectError: regexp.MustCompile(`Error: the iteration (\d|\w){26} assigned to channel (\w|\W)* is revoked and can not be used. A valid iteration must be assigned to this channel before proceeding`), }, }, }) diff --git a/internal/provider/data_source_packer_image_test.go b/internal/provider/data_source_packer_image_test.go index 23c40f9cd..8821cfd01 100644 --- a/internal/provider/data_source_packer_image_test.go +++ b/internal/provider/data_source_packer_image_test.go @@ -1,6 +1,7 @@ package provider import ( + "context" "fmt" "math/rand" "regexp" @@ -30,18 +31,13 @@ var ( iteration_id = data.hcp_packer_iteration.alpine-imagetest.id region = "us-east-1" }`, acctestImageBucket, acctestImageChannel, acctestImageBucket) - testAccPackerImageUbuntuProduction = fmt.Sprintf(` - data "hcp_packer_iteration" "ubuntu-imagetest" { - bucket_name = %q - channel = %q - } - + testAccPackerImageUbuntuProduction = ` data "hcp_packer_image" "foo" { bucket_name = %q cloud_provider = "aws" - iteration_id = data.hcp_packer_iteration.ubuntu-imagetest.id + iteration_id = %q region = "us-east-1" - }`, acctestImageUbuntuBucket, acctestImageChannel, acctestImageUbuntuBucket) + }` ) func TestAcc_dataSourcePackerImage(t *testing.T) { @@ -52,10 +48,9 @@ func TestAcc_dataSourcePackerImage(t *testing.T) { PreCheck: func() { testAccPreCheck(t, false) }, ProviderFactories: providerFactories, CheckDestroy: func(*terraform.State) error { - itID := getIterationIDFromFingerPrint(t, acctestImageBucket, fingerprint) - deleteChannel(t, acctestImageBucket, acctestImageChannel) - deleteIteration(t, acctestImageBucket, itID) - deleteBucket(t, acctestImageBucket) + deleteChannel(t, acctestImageBucket, acctestImageChannel, false) + deleteIteration(t, acctestImageBucket, fingerprint, false) + deleteBucket(t, acctestImageBucket, false) return nil }, PreventPostDestroyRefresh: true, @@ -66,7 +61,10 @@ func TestAcc_dataSourcePackerImage(t *testing.T) { PreConfig: func() { upsertBucket(t, acctestImageBucket) upsertIteration(t, acctestImageBucket, fingerprint) - itID := getIterationIDFromFingerPrint(t, acctestImageBucket, fingerprint) + itID, err := getIterationIDFromFingerPrint(t, acctestImageBucket, fingerprint) + if err != nil { + t.Fatal(err.Error()) + } upsertBuild(t, acctestImageBucket, fingerprint, itID) createChannel(t, acctestImageBucket, acctestImageChannel, itID) }, @@ -84,36 +82,42 @@ func TestAcc_dataSourcePackerImage(t *testing.T) { func TestAcc_dataSourcePackerImage_revokedIteration(t *testing.T) { fingerprint := fmt.Sprintf("%d", rand.Int()) + testAccProvider = New()() + errC := testAccProvider.Configure(context.Background(), terraform.NewResourceConfigRaw(nil)) + if errC != nil { + t.Fatal(errC) + } + + // CheckDestroy doesn't get called when the test fails and doesn't + // produce any tf state. In this case we destroy any existing resource + // before creating them. + deleteBucket(t, acctestImageUbuntuBucket, false) + deleteIteration(t, acctestImageUbuntuBucket, fingerprint, false) + + upsertBucket(t, acctestImageUbuntuBucket) + upsertIteration(t, acctestImageUbuntuBucket, fingerprint) + itID, err := getIterationIDFromFingerPrint(t, acctestImageUbuntuBucket, fingerprint) + if err != nil { + t.Fatal(err.Error()) + } + resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t, false) }, - ProviderFactories: providerFactories, - CheckDestroy: func(*terraform.State) error { - itID := getIterationIDFromFingerPrint(t, acctestImageUbuntuBucket, fingerprint) - deleteChannel(t, acctestImageUbuntuBucket, acctestImageChannel) - deleteIteration(t, acctestImageUbuntuBucket, itID) - deleteBucket(t, acctestImageUbuntuBucket) - return nil - }, + PreCheck: func() { testAccPreCheck(t, false) }, + ProviderFactories: providerFactories, PreventPostDestroyRefresh: true, Steps: []resource.TestStep{ // Testing that getting the production channel of the alpine image // works. { PreConfig: func() { - upsertBucket(t, acctestImageUbuntuBucket) - upsertIteration(t, acctestImageUbuntuBucket, fingerprint) - itID := getIterationIDFromFingerPrint(t, acctestImageUbuntuBucket, fingerprint) - upsertBuild(t, acctestImageUbuntuBucket, fingerprint, itID) - createChannel(t, acctestImageUbuntuBucket, 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, acctestImageUbuntuBucket, "5s") // Sleep to make sure the iteration is revoked when we test time.Sleep(5 * time.Second) }, - Config: testAccPackerImageUbuntuProduction, - PlanOnly: true, - ExpectError: regexp.MustCompile(`Error: the iteration (\d|\w){26} is revoked and can not be used`), + Config: fmt.Sprintf(testAccPackerImageUbuntuProduction, acctestImageUbuntuBucket, itID), + ExpectError: regexp.MustCompile(`Error: the iteration (\d|\w){26} is revoked and can not be used. A valid iteration should be used instead.`), }, }, }) diff --git a/internal/provider/data_source_packer_iteration.go b/internal/provider/data_source_packer_iteration.go index 2fcabf15f..3a736f5fb 100644 --- a/internal/provider/data_source_packer_iteration.go +++ b/internal/provider/data_source_packer_iteration.go @@ -107,7 +107,8 @@ func dataSourcePackerIterationRead(ctx context.Context, d *schema.ResourceData, if !time.Time(iteration.RevokeAt).IsZero() { // If RevokeAt is not a zero date, it means this iteration is revoked and should not be used // to build new images. - return diag.Errorf("the iteration %s is revoked and can not be used", iteration.ID) + return diag.Errorf("the iteration %s assigned to channel %s is revoked and can not be used. A valid iteration"+ + " must be assigned to this channel before proceeding", iteration.ID, channelSlug) } d.SetId(iteration.ID) diff --git a/internal/provider/data_source_packer_iteration_test.go b/internal/provider/data_source_packer_iteration_test.go index 1b36d18e4..20eea2e78 100644 --- a/internal/provider/data_source_packer_iteration_test.go +++ b/internal/provider/data_source_packer_iteration_test.go @@ -38,10 +38,9 @@ func TestAcc_dataSourcePackerIteration(t *testing.T) { PreCheck: func() { testAccPreCheck(t, false) }, ProviderFactories: providerFactories, CheckDestroy: func(*terraform.State) error { - itID := getIterationIDFromFingerPrint(t, acctestIterationBucket, fingerprint) - deleteChannel(t, acctestIterationBucket, acctestIterationChannel) - deleteIteration(t, acctestIterationBucket, itID) - deleteBucket(t, acctestIterationBucket) + deleteChannel(t, acctestIterationBucket, acctestIterationChannel, false) + deleteIteration(t, acctestIterationBucket, fingerprint, false) + deleteBucket(t, acctestIterationBucket, false) return nil }, @@ -52,7 +51,10 @@ func TestAcc_dataSourcePackerIteration(t *testing.T) { PreConfig: func() { upsertBucket(t, acctestIterationBucket) upsertIteration(t, acctestIterationBucket, fingerprint) - itID := getIterationIDFromFingerPrint(t, acctestIterationBucket, fingerprint) + itID, err := getIterationIDFromFingerPrint(t, acctestIterationBucket, fingerprint) + if err != nil { + t.Fatal(err.Error()) + } upsertBuild(t, acctestIterationBucket, fingerprint, itID) createChannel(t, acctestIterationBucket, acctestIterationChannel, itID) }, @@ -73,10 +75,9 @@ func TestAcc_dataSourcePackerIteration_revokedIteration(t *testing.T) { PreCheck: func() { testAccPreCheck(t, false) }, ProviderFactories: providerFactories, CheckDestroy: func(*terraform.State) error { - itID := getIterationIDFromFingerPrint(t, acctestIterationUbuntuBucket, fingerprint) - deleteChannel(t, acctestIterationUbuntuBucket, acctestIterationChannel) - deleteIteration(t, acctestIterationUbuntuBucket, itID) - deleteBucket(t, acctestIterationUbuntuBucket) + deleteChannel(t, acctestIterationUbuntuBucket, acctestIterationChannel, false) + deleteIteration(t, acctestIterationUbuntuBucket, fingerprint, false) + deleteBucket(t, acctestIterationUbuntuBucket, false) return nil }, @@ -85,9 +86,19 @@ func TestAcc_dataSourcePackerIteration_revokedIteration(t *testing.T) { // works. { PreConfig: func() { + // CheckDestroy doesn't get called when the test fails and doesn't + // produce any tf state. In this case we destroy any existing resource + // before creating them. + deleteChannel(t, acctestIterationUbuntuBucket, acctestIterationChannel, false) + deleteIteration(t, acctestIterationUbuntuBucket, fingerprint, false) + deleteBucket(t, acctestIterationUbuntuBucket, false) + upsertBucket(t, acctestIterationUbuntuBucket) upsertIteration(t, acctestIterationUbuntuBucket, fingerprint) - itID := getIterationIDFromFingerPrint(t, acctestIterationUbuntuBucket, fingerprint) + itID, err := getIterationIDFromFingerPrint(t, acctestIterationUbuntuBucket, fingerprint) + if err != nil { + t.Fatal(err.Error()) + } upsertBuild(t, acctestIterationUbuntuBucket, fingerprint, itID) createChannel(t, acctestIterationUbuntuBucket, acctestIterationChannel, itID) // Schedule revocation to the future, otherwise we won't be able to revoke an iteration that @@ -97,8 +108,7 @@ func TestAcc_dataSourcePackerIteration_revokedIteration(t *testing.T) { time.Sleep(5 * time.Second) }, Config: testConfig(testAccPackerIterationUbuntuProduction), - PlanOnly: true, - ExpectError: regexp.MustCompile(`Error: the iteration (\d|\w){26} is revoked and can not be used`), + ExpectError: regexp.MustCompile(`Error: the iteration (\d|\w){26} assigned to channel (\w|\W)* is revoked and can not be used. A valid iteration must be assigned to this channel before proceeding`), }, }, })