From 148d5e3a2df8ab1e306878b3946c1dc5ce2054bc Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Tue, 11 Oct 2022 15:07:03 -0400 Subject: [PATCH] Add initialize method to Iteration Fingerprint initialize was previously occurring during the creation of a bucket. We want to be able to initialize a bucket and defer the setting of a fingerprint to a later point. * Update test to reflect new function signatures for Bucket and Iteration --- command/registry.go | 28 ++++++++----------- internal/registry/types.bucket.go | 11 ++------ .../registry/types.bucket_service_test.go | 26 ++++++++--------- internal/registry/types.bucket_test.go | 8 ++---- internal/registry/types.iterations.go | 19 +++++++++---- internal/registry/types.iterations_test.go | 9 +++--- 6 files changed, 49 insertions(+), 52 deletions(-) diff --git a/command/registry.go b/command/registry.go index e1482084e..d79b65597 100644 --- a/command/registry.go +++ b/command/registry.go @@ -180,20 +180,7 @@ func createConfiguredBucket(templateDir string, opts ...bucketConfigurationOpts) }) } - bucket, err := registry.NewBucketWithIteration(registry.IterationOptions{ - TemplateBaseDir: templateDir, - }) - - // This error needs to be reworded. - if err != nil { - diags = append(diags, &hcl.Diagnostic{ - Summary: "Unable to create a valid bucket object for HCP Packer Registry", - Detail: fmt.Sprintf("%s", err), - Severity: hcl.DiagError, - }) - - return nil, diags - } + bucket := registry.NewBucketWithIteration() for _, opt := range opts { if optDiags := opt(bucket); optDiags.HasErrors() { @@ -207,10 +194,19 @@ func createConfiguredBucket(templateDir string, opts ...bucketConfigurationOpts) Detail: "empty bucket name, please set it with the HCP_PACKER_BUCKET_NAME environment variable, or in a `hcp_packer_registry` block", Severity: hcl.DiagError, }) - - return nil, diags } + err := bucket.Iteration.Initialize(registry.IterationOptions{ + TemplateBaseDir: templateDir, + }) + + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Summary: "Iteration initialization failed", + Detail: fmt.Sprintf("Initialization of the iteration failed with %s", err), + Severity: hcl.DiagError, + }) + } return bucket, diags } diff --git a/internal/registry/types.bucket.go b/internal/registry/types.bucket.go index 881b5caaa..3c0696f77 100644 --- a/internal/registry/types.bucket.go +++ b/internal/registry/types.bucket.go @@ -39,20 +39,15 @@ type ParentIteration struct { // NewBucketWithIteration initializes a simple Bucket that can be used publishing Packer build // images to the HCP Packer registry. -func NewBucketWithIteration(opts IterationOptions) (*Bucket, error) { +func NewBucketWithIteration() *Bucket { b := Bucket{ BucketLabels: make(map[string]string), BuildLabels: make(map[string]string), SourceImagesToParentIterations: make(map[string]ParentIteration), } + b.Iteration = NewIteration() - i, err := NewIteration(opts) - if err != nil { - return nil, err - } - - b.Iteration = i - return &b, nil + return &b } func (b *Bucket) Validate() error { diff --git a/internal/registry/types.bucket_service_test.go b/internal/registry/types.bucket_service_test.go index 13aa5736e..6e0c27b68 100644 --- a/internal/registry/types.bucket_service_test.go +++ b/internal/registry/types.bucket_service_test.go @@ -18,8 +18,8 @@ func TestInitialize_NewBucketNewIteration(t *testing.T) { }, } - var err error - b.Iteration, err = NewIteration(IterationOptions{}) + b.Iteration = NewIteration() + err := b.Iteration.Initialize(IterationOptions{}) if err != nil { t.Errorf("unexpected failure: %v", err) } @@ -73,12 +73,11 @@ func TestInitialize_ExistingBucketNewIteration(t *testing.T) { }, } - var err error - b.Iteration, err = NewIteration(IterationOptions{}) + b.Iteration = NewIteration() + err := b.Iteration.Initialize(IterationOptions{}) if err != nil { t.Errorf("unexpected failure: %v", err) } - b.Iteration.expectedBuilds = append(b.Iteration.expectedBuilds, "happycloud.image") err = b.Initialize(context.TODO()) @@ -129,8 +128,8 @@ func TestInitialize_ExistingBucketExistingIteration(t *testing.T) { }, } - var err error - b.Iteration, err = NewIteration(IterationOptions{}) + b.Iteration = NewIteration() + err := b.Iteration.Initialize(IterationOptions{}) if err != nil { t.Errorf("unexpected failure: %v", err) } @@ -201,8 +200,8 @@ func TestInitialize_ExistingBucketCompleteIteration(t *testing.T) { }, } - var err error - b.Iteration, err = NewIteration(IterationOptions{}) + b.Iteration = NewIteration() + err := b.Iteration.Initialize(IterationOptions{}) if err != nil { t.Errorf("unexpected failure: %v", err) } @@ -245,12 +244,11 @@ func TestUpdateBuildStatus(t *testing.T) { }, } - var err error - b.Iteration, err = NewIteration(IterationOptions{}) + b.Iteration = NewIteration() + err := b.Iteration.Initialize(IterationOptions{}) if err != nil { t.Errorf("unexpected failure: %v", err) } - b.Iteration.expectedBuilds = append(b.Iteration.expectedBuilds, "happycloud.image") mockService.ExistingBuilds = append(mockService.ExistingBuilds, "happycloud.image") @@ -300,8 +298,8 @@ func TestUpdateBuildStatus_DONENoImages(t *testing.T) { }, } - var err error - b.Iteration, err = NewIteration(IterationOptions{}) + b.Iteration = NewIteration() + err := b.Iteration.Initialize(IterationOptions{}) if err != nil { t.Errorf("unexpected failure: %v", err) } diff --git a/internal/registry/types.bucket_test.go b/internal/registry/types.bucket_test.go index 397ae0e3a..6b2ca63ec 100644 --- a/internal/registry/types.bucket_test.go +++ b/internal/registry/types.bucket_test.go @@ -12,10 +12,7 @@ func createInitialTestBucket(t testing.TB) *Bucket { t.Setenv("HCP_PACKER_BUILD_FINGERPRINT", "no-fingerprint-here") t.Helper() - bucket, err := NewBucketWithIteration(IterationOptions{}) - if err != nil { - t.Fatalf("failed when calling NewBucketWithIteration: %s", err) - } + bucket := NewBucketWithIteration() mockService := NewMockPackerClientService() mockService.TrackCalledServiceMethods = false @@ -285,7 +282,8 @@ func TestBucket_PopulateIteration(t *testing.T) { mockService.IterationAlreadyExist = true mockService.BuildAlreadyDone = tt.buildCompleted - bucket, err := NewBucketWithIteration(IterationOptions{}) + bucket := NewBucketWithIteration() + err := bucket.Iteration.Initialize(IterationOptions{}) if err != nil { t.Fatalf("failed when calling NewBucketWithIteration: %s", err) } diff --git a/internal/registry/types.iterations.go b/internal/registry/types.iterations.go index ce577be29..dfcba0c61 100644 --- a/internal/registry/types.iterations.go +++ b/internal/registry/types.iterations.go @@ -26,7 +26,7 @@ type IterationOptions struct { } // NewIteration returns a pointer to an Iteration that can be used for storing Packer build details needed by PAR. -func NewIteration(opts IterationOptions) (*Iteration, error) { +func NewIteration() *Iteration { i := Iteration{ expectedBuilds: make([]string, 0), } @@ -35,17 +35,26 @@ func NewIteration(opts IterationOptions) (*Iteration, error) { // If no variable is defined we should try to load a fingerprint from Git, or other VCS. i.Fingerprint = os.Getenv(env.HCPPackerBuildFingerprint) - // get a Git SHA + return &i +} + +// Initialize prepares the iteration to be used with an active HCP Packer registry bucket. +func (i *Iteration) Initialize(opts IterationOptions) error { + if i == nil { + return errors.New("Unexpected call to initialize for a nil Iteration") + } + if i.Fingerprint != "" { - return &i, nil + return nil } fp, err := GetGitFingerprint(opts) if err != nil { - return nil, err + return err } i.Fingerprint = fp - return &i, nil + + return nil } // GetGitFingerprint returns the HEAD commit for some template dir defined in opt.TemplateBaseDir. diff --git a/internal/registry/types.iterations_test.go b/internal/registry/types.iterations_test.go index cec8321e2..d7d0435f8 100644 --- a/internal/registry/types.iterations_test.go +++ b/internal/registry/types.iterations_test.go @@ -8,7 +8,7 @@ import ( git "github.com/go-git/go-git/v5" ) -func TestNewIteration(t *testing.T) { +func TestIteration_Initialize(t *testing.T) { var tc = []struct { name string fingerprint string @@ -74,15 +74,16 @@ func TestNewIteration(t *testing.T) { tt.setupFn(t) } - i, err := NewIteration(tt.opts) + i := NewIteration() + err := i.Initialize(tt.opts) if tt.errorExpected { t.Logf("%v", err) if err == nil { t.Errorf("expected %q to result in an error, but it return no error", tt.name) } - if i != nil { - t.Errorf("expected %q to result in an error with no iteration, but got %v", tt.name, i) + if i.Fingerprint != "" { + t.Errorf("expected %q to result in an error with an empty iteration fingerprint, but got %q", tt.name, i.Fingerprint) } return }