From 98353bc87031c8a8a33f5ac3d28491b547bfc600 Mon Sep 17 00:00:00 2001 From: sylviamoss Date: Wed, 2 Mar 2022 14:43:13 +0100 Subject: [PATCH 1/2] do not fail packer datasources for iteration with revoke_at in the future --- internal/provider/data_source_packer_image.go | 5 +- .../data_source_packer_image_iteration.go | 5 +- ...data_source_packer_image_iteration_test.go | 54 ++++++++++++++++- .../provider/data_source_packer_iteration.go | 6 +- .../data_source_packer_iteration_test.go | 58 ++++++++++++++++++- 5 files changed, 116 insertions(+), 12 deletions(-) diff --git a/internal/provider/data_source_packer_image.go b/internal/provider/data_source_packer_image.go index 2f938f283..e3e5ce654 100644 --- a/internal/provider/data_source_packer_image.go +++ b/internal/provider/data_source_packer_image.go @@ -111,8 +111,9 @@ func dataSourcePackerImageRead(ctx context.Context, d *schema.ResourceData, meta return diag.FromErr(err) } - if !time.Time(iteration.RevokeAt).IsZero() { - // If RevokeAt is not a zero date, it means this iteration is revoked and should not be used + revokeAt := time.Time(iteration.RevokeAt) + if !revokeAt.IsZero() && revokeAt.Before(time.Now().UTC()) { + // If RevokeAt is not a zero date and is before NOW, 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. A valid iteration should be used instead.", iteration.ID) } diff --git a/internal/provider/data_source_packer_image_iteration.go b/internal/provider/data_source_packer_image_iteration.go index e1a6963e1..0d5e71962 100644 --- a/internal/provider/data_source_packer_image_iteration.go +++ b/internal/provider/data_source_packer_image_iteration.go @@ -168,8 +168,9 @@ func dataSourcePackerImageIterationRead(ctx context.Context, d *schema.ResourceD iteration := channel.Iteration - if !time.Time(iteration.RevokeAt).IsZero() { - // If RevokeAt is not a zero date, it means this iteration is revoked and should not be used + revokeAt := time.Time(iteration.RevokeAt) + if !revokeAt.IsZero() && revokeAt.Before(time.Now().UTC()) { + // If RevokeAt is not a zero date and is before NOW, it means this iteration is revoked and should not be used // to build new images. 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) diff --git a/internal/provider/data_source_packer_image_iteration_test.go b/internal/provider/data_source_packer_image_iteration_test.go index 5f1ca259b..0a0ce0154 100644 --- a/internal/provider/data_source_packer_image_iteration_test.go +++ b/internal/provider/data_source_packer_image_iteration_test.go @@ -21,9 +21,10 @@ import ( ) const ( - acctestAlpineBucket = "alpine-acctest" - acctestUbuntuBucket = "ubuntu-acctest" - acctestProductionChannel = "production" + acctestAlpineBucket = "alpine-acctest" + acctestUbuntuBucket = "ubuntu-acctest" + acctestAnotherUbuntuBucket = "another-ubuntu-acctest" + acctestProductionChannel = "production" ) var ( @@ -37,6 +38,12 @@ var ( bucket_name = %q channel = %q }`, acctestUbuntuBucket, acctestProductionChannel) + + testAccPackerAnotherUbuntuProductionImage = fmt.Sprintf(` + data "hcp_packer_image_iteration" "another-ubuntu" { + bucket_name = %q + channel = %q + }`, acctestAnotherUbuntuBucket, acctestProductionChannel) ) func upsertRegistry(t *testing.T) { @@ -512,3 +519,44 @@ func TestAcc_dataSourcePacker_revokedIteration(t *testing.T) { }, }) } + +func TestAcc_dataSourcePacker_iterationScheduledToBeRevoked(t *testing.T) { + resourceName := "data.hcp_packer_image_iteration.another-ubuntu" + fingerprint := fmt.Sprintf("%d", rand.Int()) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t, map[string]bool{"aws": false, "azure": false}) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + // testing that getting an iteration with scheduled revocation 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, acctestAnotherUbuntuBucket, acctestProductionChannel, false) + deleteIteration(t, acctestAnotherUbuntuBucket, fingerprint, false) + deleteBucket(t, acctestAnotherUbuntuBucket, false) + + upsertRegistry(t) + upsertBucket(t, acctestAnotherUbuntuBucket) + upsertIteration(t, acctestAnotherUbuntuBucket, fingerprint) + itID, err := getIterationIDFromFingerPrint(t, acctestAnotherUbuntuBucket, fingerprint) + if err != nil { + t.Fatal(err.Error()) + } + upsertBuild(t, acctestAnotherUbuntuBucket, fingerprint, itID) + createChannel(t, acctestAnotherUbuntuBucket, acctestProductionChannel, itID) + // Schedule revocation to the future + revokeIteration(t, itID, acctestAnotherUbuntuBucket, "1d") + + }, + Config: testConfig(testAccPackerAnotherUbuntuProductionImage), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet(resourceName, "organization_id"), + resource.TestCheckResourceAttrSet(resourceName, "project_id"), + ), + }, + }, + }) +} diff --git a/internal/provider/data_source_packer_iteration.go b/internal/provider/data_source_packer_iteration.go index 3a736f5fb..16591877c 100644 --- a/internal/provider/data_source_packer_iteration.go +++ b/internal/provider/data_source_packer_iteration.go @@ -104,8 +104,10 @@ func dataSourcePackerIterationRead(ctx context.Context, d *schema.ResourceData, } iteration := channel.Iteration - if !time.Time(iteration.RevokeAt).IsZero() { - // If RevokeAt is not a zero date, it means this iteration is revoked and should not be used + + revokeAt := time.Time(iteration.RevokeAt) + if !revokeAt.IsZero() && revokeAt.Before(time.Now().UTC()) { + // If RevokeAt is not a zero date and is before NOW, it means this iteration is revoked and should not be used // to build new images. 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) diff --git a/internal/provider/data_source_packer_iteration_test.go b/internal/provider/data_source_packer_iteration_test.go index 07b73a65b..df22f28b3 100644 --- a/internal/provider/data_source_packer_iteration_test.go +++ b/internal/provider/data_source_packer_iteration_test.go @@ -12,9 +12,10 @@ import ( ) const ( - acctestIterationBucket = "alpine-acctest-itertest" - acctestIterationUbuntuBucket = "ubuntu-acctest-itertest" - acctestIterationChannel = "production-iter-test" + acctestIterationBucket = "alpine-acctest-itertest" + acctestIterationUbuntuBucket = "ubuntu-acctest-itertest" + acctestIterationAnotherUbuntuBucket = "another-ubuntu-acctest-itertest" + acctestIterationChannel = "production-iter-test" ) var ( @@ -28,6 +29,11 @@ var ( bucket_name = %q channel = %q }`, acctestIterationUbuntuBucket, acctestIterationChannel) + testAccPackerIterationAnotherUbuntuProduction = fmt.Sprintf(` + data "hcp_packer_iteration" "another-ubuntu" { + bucket_name = %q + channel = %q + }`, acctestIterationAnotherUbuntuBucket, acctestIterationChannel) ) func TestAcc_dataSourcePackerIteration(t *testing.T) { @@ -113,3 +119,49 @@ func TestAcc_dataSourcePackerIteration_revokedIteration(t *testing.T) { }, }) } + +func TestAcc_dataSourcePackerIteration_iterationScheduledToBeRevoked(t *testing.T) { + resourceName := "data.hcp_packer_iteration.another-ubuntu" + fingerprint := fmt.Sprintf("%d", rand.Int()) + + 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, acctestIterationAnotherUbuntuBucket, acctestIterationChannel, false) + deleteIteration(t, acctestIterationAnotherUbuntuBucket, fingerprint, false) + deleteBucket(t, acctestIterationAnotherUbuntuBucket, false) + return nil + }, + + Steps: []resource.TestStep{ + // testing that getting an iteration with scheduled revocation 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, acctestIterationAnotherUbuntuBucket, acctestIterationChannel, false) + deleteIteration(t, acctestIterationAnotherUbuntuBucket, fingerprint, false) + deleteBucket(t, acctestIterationAnotherUbuntuBucket, false) + + upsertBucket(t, acctestIterationAnotherUbuntuBucket) + upsertIteration(t, acctestIterationAnotherUbuntuBucket, fingerprint) + itID, err := getIterationIDFromFingerPrint(t, acctestIterationAnotherUbuntuBucket, fingerprint) + if err != nil { + t.Fatal(err.Error()) + } + upsertBuild(t, acctestIterationAnotherUbuntuBucket, fingerprint, itID) + createChannel(t, acctestIterationAnotherUbuntuBucket, acctestIterationChannel, itID) + // Schedule revocation to the future + revokeIteration(t, itID, acctestIterationAnotherUbuntuBucket, "1d") + }, + Config: testConfig(testAccPackerIterationAnotherUbuntuProduction), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet(resourceName, "organization_id"), + resource.TestCheckResourceAttrSet(resourceName, "project_id"), + ), + }, + }, + }) +} From f5cb8b4aa0892efb3e27108a5f0c25ea2ef495fa Mon Sep 17 00:00:00 2001 From: sylviamoss Date: Wed, 2 Mar 2022 14:58:23 +0100 Subject: [PATCH 2/2] remove unecessary code from tests --- .../data_source_packer_image_iteration_test.go | 13 ++++++------- .../provider/data_source_packer_iteration_test.go | 15 --------------- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/internal/provider/data_source_packer_image_iteration_test.go b/internal/provider/data_source_packer_image_iteration_test.go index 0a0ce0154..51c4708c9 100644 --- a/internal/provider/data_source_packer_image_iteration_test.go +++ b/internal/provider/data_source_packer_image_iteration_test.go @@ -527,17 +527,16 @@ func TestAcc_dataSourcePacker_iterationScheduledToBeRevoked(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, acctestAnotherUbuntuBucket, acctestProductionChannel, false) + deleteIteration(t, acctestAnotherUbuntuBucket, fingerprint, false) + deleteBucket(t, acctestAnotherUbuntuBucket, false) + return nil + }, Steps: []resource.TestStep{ // testing that getting an iteration with scheduled revocation 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, acctestAnotherUbuntuBucket, acctestProductionChannel, false) - deleteIteration(t, acctestAnotherUbuntuBucket, fingerprint, false) - deleteBucket(t, acctestAnotherUbuntuBucket, false) - upsertRegistry(t) upsertBucket(t, acctestAnotherUbuntuBucket) upsertIteration(t, acctestAnotherUbuntuBucket, fingerprint) diff --git a/internal/provider/data_source_packer_iteration_test.go b/internal/provider/data_source_packer_iteration_test.go index df22f28b3..8494a0bf0 100644 --- a/internal/provider/data_source_packer_iteration_test.go +++ b/internal/provider/data_source_packer_iteration_test.go @@ -80,13 +80,6 @@ func TestAcc_dataSourcePackerIteration_revokedIteration(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, acctestIterationUbuntuBucket, acctestIterationChannel, false) - deleteIteration(t, acctestIterationUbuntuBucket, fingerprint, false) - deleteBucket(t, acctestIterationUbuntuBucket, false) - return nil - }, - Steps: []resource.TestStep{ // testing that getting the production channel of the alpine image // works. @@ -133,18 +126,10 @@ func TestAcc_dataSourcePackerIteration_iterationScheduledToBeRevoked(t *testing. deleteBucket(t, acctestIterationAnotherUbuntuBucket, false) return nil }, - Steps: []resource.TestStep{ // testing that getting an iteration with scheduled revocation 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, acctestIterationAnotherUbuntuBucket, acctestIterationChannel, false) - deleteIteration(t, acctestIterationAnotherUbuntuBucket, fingerprint, false) - deleteBucket(t, acctestIterationAnotherUbuntuBucket, false) - upsertBucket(t, acctestIterationAnotherUbuntuBucket) upsertIteration(t, acctestIterationAnotherUbuntuBucket, fingerprint) itID, err := getIterationIDFromFingerPrint(t, acctestIterationAnotherUbuntuBucket, fingerprint)