Skip to content

Commit

Permalink
improve error message and tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@sylviamoss
sylviamoss committed Jan 25, 2022
1 parent f38309a commit 019d747
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 70 deletions.
2 changes: 1 addition & 1 deletion internal/provider/data_source_packer_image.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion internal/provider/data_source_packer_image_iteration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
68 changes: 43 additions & 25 deletions internal/provider/data_source_packer_image_iteration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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) {
Expand All @@ -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
},

Expand All @@ -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)
},
Expand All @@ -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
Expand All @@ -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`),
},
},
})
Expand Down
64 changes: 34 additions & 30 deletions internal/provider/data_source_packer_image_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package provider

import (
"context"
"fmt"
"math/rand"
"regexp"
Expand Down Expand Up @@ -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) {
Expand All @@ -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,
Expand All @@ -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)
},
Expand All @@ -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.`),
},
},
})
Expand Down
3 changes: 2 additions & 1 deletion internal/provider/data_source_packer_iteration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
34 changes: 22 additions & 12 deletions internal/provider/data_source_packer_iteration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
},

Expand All @@ -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)
},
Expand All @@ -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
},

Expand All @@ -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
Expand All @@ -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`),
},
},
})
Expand Down

0 comments on commit 019d747

Please sign in to comment.