From cbe4b9921d6c57eb77d0a02bc6979805fc6fefb1 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Mon, 19 Dec 2022 15:50:29 -0500 Subject: [PATCH] hcp: generate fingerprints on each new build Fingerprints are how we link a packer build to an iteration on HCP. These are computed automatically from the Git SHA in the current state, and are unique to the bucket/iteration. The main problem with this approach is that while sound in theory, it quickly falls apart when users want to run the same build configuration twice, but expect a new image to be created. With the current model, this fails, as the iteration with the current SHA already exists. While this is solvable through environment variables, or by committing a change to the repository, we think this is not clear enough, and causes an extra step to what should otherwise be a simple process. Therefore, to lower the barrier of entry into HCP, we change this behaviour with this commit. Now, fingerprints are randomly generated ULIDs instead of a git SHA, and a new one is always generated, unless one is already specified in the environment. This makes continuation of an existing iteration a conscious choice rather than something automatic, and virtually eliminates conflicts such as the ones described above. --- CHANGELOG.md | 9 ++ command/build.go | 2 + internal/hcp/registry/hcl.go | 13 ++ internal/hcp/registry/hcp.go | 45 ++++++- internal/hcp/registry/json.go | 13 ++ internal/hcp/registry/null_registry.go | 3 + internal/hcp/registry/registry.go | 2 +- .../hcp/registry/types.bucket_service_test.go | 21 ++-- internal/hcp/registry/types.bucket_test.go | 6 +- internal/hcp/registry/types.iterations.go | 113 +++++++++++------- .../hcp/registry/types.iterations_test.go | 29 +---- website/content/docs/hcp/index.mdx | 2 +- 12 files changed, 164 insertions(+), 94 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7371d01dcda..20da5868dbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ displayed if both were available, and now both will be displayed. This does not impact the behaviour of options such as only or except, they will continue to work as they did before. +* hcp: iteration fingerprints used to be computed from the Git SHA of the repository + where the template is located when running packer build. This changes with this + release, and now fingerprints are automatically generated as a ULID. This implies + that continuing an existing iteration will require users to define the + fingerprint in the environment manually in order to adopt this behaviour, + otherwise, by default, a new iteration will be created. This does not impact + workflows where the fingerprint was defined through the + HCP_PACKER_ITERATION_FINGERPRINT environment variable, and these builds will work + exactly as they did before. ## 1.8.5 (December 12, 2022) diff --git a/command/build.go b/command/build.go index 66a2ec4c90f..d254b68d0c4 100644 --- a/command/build.go +++ b/command/build.go @@ -96,6 +96,8 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, cla *BuildArgs) int return ret } + defer hcpRegistry.IterationStatusSummary(c.Ui) + err := hcpRegistry.PopulateIteration(buildCtx) if err != nil { return writeDiags(c.Ui, nil, hcl.Diagnostics{ diff --git a/internal/hcp/registry/hcl.go b/internal/hcp/registry/hcl.go index baa2b5cb29a..8c5115afc54 100644 --- a/internal/hcp/registry/hcl.go +++ b/internal/hcp/registry/hcl.go @@ -3,6 +3,7 @@ package registry import ( "context" "fmt" + "log" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcp-sdk-go/clients/cloud-packer-service/stable/2021-04-30/models" @@ -42,6 +43,13 @@ func (h *HCLMetadataRegistry) PopulateIteration(ctx context.Context) error { h.configuration.HCPVars["iterationID"] = cty.StringVal(iterationID) + sha, err := getGitSHA(h.configuration.Basedir) + if err != nil { + log.Printf("failed to get GIT SHA from environment, won't set as build labels") + } else { + h.bucket.Iteration.AddSHAToBuildLabels(sha) + } + return nil } @@ -70,6 +78,11 @@ func (h *HCLMetadataRegistry) CompleteBuild( return h.bucket.completeBuild(ctx, name, artifacts, buildErr) } +// IterationStatusSummary prints a status report in the UI if the iteration is not yet done +func (h *HCLMetadataRegistry) IterationStatusSummary(ui sdkpacker.Ui) { + h.bucket.Iteration.iterationStatusSummary(ui) +} + func NewHCLMetadataRegistry(config *hcl2template.PackerConfig) (*HCLMetadataRegistry, hcl.Diagnostics) { var diags hcl.Diagnostics if len(config.Builds) > 1 { diff --git a/internal/hcp/registry/hcp.go b/internal/hcp/registry/hcp.go index fe95a0a63cd..aacb56466fe 100644 --- a/internal/hcp/registry/hcp.go +++ b/internal/hcp/registry/hcp.go @@ -3,6 +3,7 @@ package registry import ( "fmt" + git "github.com/go-git/go-git/v5" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/packer/hcl2template" "github.com/hashicorp/packer/internal/hcp/env" @@ -87,9 +88,15 @@ func createConfiguredBucket(templateDir string, opts ...bucketConfigurationOpts) }) } - err := bucket.Iteration.Initialize(IterationOptions{ - TemplateBaseDir: templateDir, - }) + err := bucket.Iteration.Initialize() + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Failed to initialize iteration", + Detail: fmt.Sprintf("The iteration failed to be initialized for bucket %q: %s", + bucket.Slug, err), + }) + } if err != nil { diags = append(diags, &hcl.Diagnostic{ @@ -109,3 +116,35 @@ func withPackerEnvConfiguration(bucket *Bucket) hcl.Diagnostics { return nil } + +// getGitSHA returns the HEAD commit for some template dir defined in baseDir. +// If the base directory is not under version control an error is returned. +func getGitSHA(baseDir string) (string, error) { + r, err := git.PlainOpenWithOptions(baseDir, &git.PlainOpenOptions{ + DetectDotGit: true, + }) + + if err != nil { + return "", fmt.Errorf("Packer could not read the fingerprint from git.") + } + + // The config can be used to retrieve user identity. for example, + // c.User.Email. Leaving in but commented because I'm not sure we care + // about this identity right now. - Megan + // + // c, err := r.ConfigScoped(config.GlobalScope) + // if err != nil { + // return "", fmt.Errorf("Error setting git scope", err) + // } + ref, err := r.Head() + if err != nil { + // If we get there, we're in a Git dir, but HEAD cannot be read. + // + // This may happen when there's no commit in the git dir. + return "", fmt.Errorf("Packer could not read a git SHA in directory %q: %s", baseDir, err) + } + + // log.Printf("Author: %v, Commit: %v\n", c.User.Email, ref.Hash()) + + return ref.Hash().String(), nil +} diff --git a/internal/hcp/registry/json.go b/internal/hcp/registry/json.go index f15c79aede1..8bbacdf012f 100644 --- a/internal/hcp/registry/json.go +++ b/internal/hcp/registry/json.go @@ -3,6 +3,7 @@ package registry import ( "context" "fmt" + "log" "path/filepath" "github.com/hashicorp/hcl/v2" @@ -63,6 +64,13 @@ func (h *JSONMetadataRegistry) PopulateIteration(ctx context.Context) error { return err } + sha, err := getGitSHA(h.configuration.Template.Path) + if err != nil { + log.Printf("failed to get GIT SHA from environment, won't set as build labels") + } else { + h.bucket.Iteration.AddSHAToBuildLabels(sha) + } + return nil } @@ -80,3 +88,8 @@ func (h *JSONMetadataRegistry) CompleteBuild( ) ([]sdkpacker.Artifact, error) { return h.bucket.completeBuild(ctx, build.Name(), artifacts, buildErr) } + +// IterationStatusSummary prints a status report in the UI if the iteration is not yet done +func (h *JSONMetadataRegistry) IterationStatusSummary(ui sdkpacker.Ui) { + h.bucket.Iteration.iterationStatusSummary(ui) +} diff --git a/internal/hcp/registry/null_registry.go b/internal/hcp/registry/null_registry.go index dbab4197ded..16e27eac634 100644 --- a/internal/hcp/registry/null_registry.go +++ b/internal/hcp/registry/null_registry.go @@ -25,3 +25,6 @@ func (r nullRegistry) CompleteBuild( ) ([]sdkpacker.Artifact, error) { return artifacts, nil } + +func (r nullRegistry) IterationStatusSummary(_ sdkpacker.Ui) { +} diff --git a/internal/hcp/registry/registry.go b/internal/hcp/registry/registry.go index 16e82eb7f50..8c01aa8ef94 100644 --- a/internal/hcp/registry/registry.go +++ b/internal/hcp/registry/registry.go @@ -12,10 +12,10 @@ import ( // Registry is an entity capable to orchestrate a Packer build and upload metadata to HCP type Registry interface { - //Configure(packer.Handler) PopulateIteration(context.Context) error StartBuild(context.Context, sdkpacker.Build) error CompleteBuild(ctx context.Context, build sdkpacker.Build, artifacts []sdkpacker.Artifact, buildErr error) ([]sdkpacker.Artifact, error) + IterationStatusSummary(writer sdkpacker.Ui) } // New instanciates the appropriate registry for the Packer configuration template type. diff --git a/internal/hcp/registry/types.bucket_service_test.go b/internal/hcp/registry/types.bucket_service_test.go index 60317443707..463feb59b4c 100644 --- a/internal/hcp/registry/types.bucket_service_test.go +++ b/internal/hcp/registry/types.bucket_service_test.go @@ -9,7 +9,6 @@ import ( ) func TestInitialize_NewBucketNewIteration(t *testing.T) { - t.Setenv("HCP_PACKER_BUILD_FINGERPRINT", "testnumber") mockService := api.NewMockPackerClientService() b := &Bucket{ @@ -20,7 +19,7 @@ func TestInitialize_NewBucketNewIteration(t *testing.T) { } b.Iteration = NewIteration() - err := b.Iteration.Initialize(IterationOptions{}) + err := b.Iteration.Initialize() if err != nil { t.Errorf("unexpected failure: %v", err) } @@ -62,7 +61,6 @@ func TestInitialize_NewBucketNewIteration(t *testing.T) { } func TestInitialize_UnsetTemplateTypeError(t *testing.T) { - t.Setenv("HCP_PACKER_BUILD_FINGERPRINT", "testunsettemplate") mockService := api.NewMockPackerClientService() mockService.BucketAlreadyExist = true @@ -74,7 +72,7 @@ func TestInitialize_UnsetTemplateTypeError(t *testing.T) { } b.Iteration = NewIteration() - err := b.Iteration.Initialize(IterationOptions{}) + err := b.Iteration.Initialize() if err != nil { t.Errorf("unexpected failure: %v", err) } @@ -88,7 +86,6 @@ func TestInitialize_UnsetTemplateTypeError(t *testing.T) { } func TestInitialize_ExistingBucketNewIteration(t *testing.T) { - t.Setenv("HCP_PACKER_BUILD_FINGERPRINT", "testnumber") mockService := api.NewMockPackerClientService() mockService.BucketAlreadyExist = true @@ -100,7 +97,7 @@ func TestInitialize_ExistingBucketNewIteration(t *testing.T) { } b.Iteration = NewIteration() - err := b.Iteration.Initialize(IterationOptions{}) + err := b.Iteration.Initialize() if err != nil { t.Errorf("unexpected failure: %v", err) } @@ -142,7 +139,6 @@ func TestInitialize_ExistingBucketNewIteration(t *testing.T) { } func TestInitialize_ExistingBucketExistingIteration(t *testing.T) { - t.Setenv("HCP_PACKER_BUILD_FINGERPRINT", "testnumber") mockService := api.NewMockPackerClientService() mockService.BucketAlreadyExist = true mockService.IterationAlreadyExist = true @@ -155,7 +151,7 @@ func TestInitialize_ExistingBucketExistingIteration(t *testing.T) { } b.Iteration = NewIteration() - err := b.Iteration.Initialize(IterationOptions{}) + err := b.Iteration.Initialize() if err != nil { t.Errorf("unexpected failure: %v", err) } @@ -212,7 +208,6 @@ func TestInitialize_ExistingBucketExistingIteration(t *testing.T) { } func TestInitialize_ExistingBucketCompleteIteration(t *testing.T) { - t.Setenv("HCP_PACKER_BUILD_FINGERPRINT", "testnumber") mockService := api.NewMockPackerClientService() mockService.BucketAlreadyExist = true mockService.IterationAlreadyExist = true @@ -227,7 +222,7 @@ func TestInitialize_ExistingBucketCompleteIteration(t *testing.T) { } b.Iteration = NewIteration() - err := b.Iteration.Initialize(IterationOptions{}) + err := b.Iteration.Initialize() if err != nil { t.Errorf("unexpected failure: %v", err) } @@ -258,7 +253,6 @@ func TestInitialize_ExistingBucketCompleteIteration(t *testing.T) { } func TestUpdateBuildStatus(t *testing.T) { - t.Setenv("HCP_PACKER_BUILD_FINGERPRINT", "testnumber") mockService := api.NewMockPackerClientService() mockService.BucketAlreadyExist = true mockService.IterationAlreadyExist = true @@ -271,7 +265,7 @@ func TestUpdateBuildStatus(t *testing.T) { } b.Iteration = NewIteration() - err := b.Iteration.Initialize(IterationOptions{}) + err := b.Iteration.Initialize() if err != nil { t.Errorf("unexpected failure: %v", err) } @@ -312,7 +306,6 @@ func TestUpdateBuildStatus(t *testing.T) { } func TestUpdateBuildStatus_DONENoImages(t *testing.T) { - t.Setenv("HCP_PACKER_BUILD_FINGERPRINT", "testnumber") mockService := api.NewMockPackerClientService() mockService.BucketAlreadyExist = true mockService.IterationAlreadyExist = true @@ -325,7 +318,7 @@ func TestUpdateBuildStatus_DONENoImages(t *testing.T) { } b.Iteration = NewIteration() - err := b.Iteration.Initialize(IterationOptions{}) + err := b.Iteration.Initialize() if err != nil { t.Errorf("unexpected failure: %v", err) } diff --git a/internal/hcp/registry/types.bucket_test.go b/internal/hcp/registry/types.bucket_test.go index e7c926f6253..922916b461a 100644 --- a/internal/hcp/registry/types.bucket_test.go +++ b/internal/hcp/registry/types.bucket_test.go @@ -10,11 +10,9 @@ import ( ) func createInitialTestBucket(t testing.TB) *Bucket { - t.Setenv("HCP_PACKER_BUILD_FINGERPRINT", "no-fingerprint-here") - t.Helper() bucket := NewBucketWithIteration() - err := bucket.Iteration.Initialize(IterationOptions{}) + err := bucket.Iteration.Initialize() if err != nil { t.Errorf("failed to initialize Bucket: %s", err) return nil @@ -289,7 +287,7 @@ func TestBucket_PopulateIteration(t *testing.T) { mockService.BuildAlreadyDone = tt.buildCompleted bucket := NewBucketWithIteration() - err := bucket.Iteration.Initialize(IterationOptions{}) + err := bucket.Iteration.Initialize() if err != nil { t.Fatalf("failed when calling NewBucketWithIteration: %s", err) } diff --git a/internal/hcp/registry/types.iterations.go b/internal/hcp/registry/types.iterations.go index 410827a24a1..ef86470647b 100644 --- a/internal/hcp/registry/types.iterations.go +++ b/internal/hcp/registry/types.iterations.go @@ -3,12 +3,17 @@ package registry import ( "errors" "fmt" + "math/rand" "os" + "strings" "sync" + "time" - git "github.com/go-git/go-git/v5" + "github.com/hashicorp/hcp-sdk-go/clients/cloud-packer-service/stable/2021-04-30/models" + sdkpacker "github.com/hashicorp/packer-plugin-sdk/packer" registryimage "github.com/hashicorp/packer-plugin-sdk/packer/registry/image" "github.com/hashicorp/packer/internal/hcp/env" + "github.com/oklog/ulid" ) type Iteration struct { @@ -34,68 +39,28 @@ func NewIteration() *Iteration { } // Initialize prepares the iteration to be used with an active HCP Packer registry bucket. -func (i *Iteration) Initialize(opts IterationOptions) error { +func (i *Iteration) Initialize() error { if i == nil { return errors.New("Unexpected call to initialize for a nil Iteration") } // By default we try to load a Fingerprint from the environment variable. - // If no variable is defined we should try to load a fingerprint from Git, or other VCS. + // If no variable is defined we generate a new fingerprint. i.Fingerprint = os.Getenv(env.HCPPackerBuildFingerprint) if i.Fingerprint != "" { return nil } - fp, err := GetGitFingerprint(opts) + fp, err := ulid.New(ulid.Now(), ulid.Monotonic(rand.New(rand.NewSource(time.Now().UnixNano())), 0)) if err != nil { - return err + return fmt.Errorf("Failed to generate a fingerprint: %s", err) } - i.Fingerprint = fp + i.Fingerprint = fp.String() return nil } -// GetGitFingerprint returns the HEAD commit for some template dir defined in opt.TemplateBaseDir. -// If the base directory is not under version control an error is returned. -func GetGitFingerprint(opts IterationOptions) (string, error) { - r, err := git.PlainOpenWithOptions(opts.TemplateBaseDir, &git.PlainOpenOptions{ - DetectDotGit: true, - }) - - if err != nil { - return "", fmt.Errorf("Packer could not read the fingerprint from git." + - "\n\nIf your Packer template is not within a git managed directory, " + - "you can set the HCP_PACKER_BUILD_FINGERPRINT environment variable. " + - "The fingerprint must be less than 32 characters and can contain letters and numbers.") - } - - // The config can be used to retrieve user identity. for example, - // c.User.Email. Leaving in but commented because I'm not sure we care - // about this identity right now. - Megan - // - // c, err := r.ConfigScoped(config.GlobalScope) - // if err != nil { - // return "", fmt.Errorf("Error setting git scope", err) - // } - ref, err := r.Head() - if err != nil { - // If we get there, we're in a Git dir, but HEAD cannot be read. - // - // This may happen when there's no commit in the git dir. - return "", fmt.Errorf("Packer could not read a git SHA in directory %q.\n"+ - "This may happen if your template is in a git repository without any "+ - "commits. You can either add a commit in this directory, or set the "+ - "HCP_PACKER_BUILD_FINGERPRINT environment variable. The fingerprint "+ - "must be less than 32 characters and can contain letters and numbers.\n"+ - "Error: %s", opts.TemplateBaseDir, err) - } - - // log.Printf("Author: %v, Commit: %v\n", c.User.Email, ref.Hash()) - - return ref.Hash().String(), nil -} - //StoreBuild stores a build for buildName to an active iteration. func (i *Iteration) StoreBuild(buildName string, build *Build) { i.builds.Store(buildName, build) @@ -151,3 +116,59 @@ func (i *Iteration) AddLabelsToBuild(buildName string, data map[string]string) e i.StoreBuild(buildName, build) return nil } + +// AddSHAToBuildLabels adds the Git SHA for the current iteration (if set) as a label for all the builds of the iteration +func (i *Iteration) AddSHAToBuildLabels(sha string) { + i.builds.Range(func(_, v any) bool { + b, ok := v.(*Build) + if !ok { + return true + } + + b.MergeLabels(map[string]string{ + "git_sha": sha, + }) + + return true + }) +} + +// RemainingBuilds returns the list of builds that are not in a DONE status +func (i *Iteration) RemainingBuilds() []*Build { + var todo []*Build + + i.builds.Range(func(k, v any) bool { + build, ok := v.(*Build) + if !ok { + // Unlikely since the builds map contains only Build instances + return true + } + + if build.Status != models.HashicorpCloudPackerBuildStatusDONE { + todo = append(todo, build) + } + return true + }) + + return todo +} + +func (i *Iteration) iterationStatusSummary(ui sdkpacker.Ui) { + rem := i.RemainingBuilds() + if rem == nil { + return + } + + buf := &strings.Builder{} + + buf.WriteString(fmt.Sprintf( + "\nIteration %q is not complete, the following builds are not done:\n\n", + i.Fingerprint)) + for _, b := range rem { + buf.WriteString(fmt.Sprintf("* %q: %s\n", b.ComponentType, b.Status)) + } + buf.WriteString("\nYou may resume work on this iteration in further Packer builds by defining the following variable in your environment:\n") + buf.WriteString(fmt.Sprintf("HCP_PACKER_BUILD_FINGERPRINT=%q", i.Fingerprint)) + + ui.Say(buf.String()) +} diff --git a/internal/hcp/registry/types.iterations_test.go b/internal/hcp/registry/types.iterations_test.go index d7d0435f837..b78229454ce 100644 --- a/internal/hcp/registry/types.iterations_test.go +++ b/internal/hcp/registry/types.iterations_test.go @@ -12,7 +12,6 @@ func TestIteration_Initialize(t *testing.T) { var tc = []struct { name string fingerprint string - opts IterationOptions setupFn func(t *testing.T) errorExpected bool }{ @@ -26,9 +25,6 @@ func TestIteration_Initialize(t *testing.T) { { name: "using git fingerprint", fingerprint: "4ec004e18e977a5b8a3a28f4b24279b6993d7e7c", - opts: IterationOptions{ - TemplateBaseDir: tempdir("4ec004e18e"), - }, setupFn: func(t *testing.T) { //nolint:errcheck git.PlainClone(tempdir("4ec004e18e"), false, &git.CloneOptions{ @@ -37,33 +33,16 @@ func TestIteration_Initialize(t *testing.T) { Depth: 1, }) + t.Setenv("HCP_PACKER_BUILD_FINGERPRINT", "4ec004e18e977a5b8a3a28f4b24279b6993d7e7c") + t.Cleanup(func() { //nolint:errcheck os.RemoveAll(tempdir("4ec004e18e")) }) }, }, - { - name: "using empty git directory", - opts: IterationOptions{ - TemplateBaseDir: tempdir("empty-init"), - }, - setupFn: func(t *testing.T) { - //nolint:errcheck - git.PlainInit(tempdir("empty-init"), false) - t.Cleanup(func() { - //nolint:errcheck - os.RemoveAll(tempdir("empty-init")) - }) - }, - errorExpected: true, - }, { name: "using no fingerprint in clean directory", - opts: IterationOptions{ - TemplateBaseDir: "/dev/null", - }, - errorExpected: true, }, } @@ -75,7 +54,7 @@ func TestIteration_Initialize(t *testing.T) { } i := NewIteration() - err := i.Initialize(tt.opts) + err := i.Initialize() if tt.errorExpected { t.Logf("%v", err) if err == nil { @@ -92,7 +71,7 @@ func TestIteration_Initialize(t *testing.T) { t.Errorf("expected %q to return with no error, but it %v", tt.name, err) } - if i.Fingerprint != tt.fingerprint { + if tt.fingerprint != "" && i.Fingerprint != tt.fingerprint { t.Errorf("%q failed to load the expected fingerprint %q, but got %q", tt.name, tt.fingerprint, i.Fingerprint) } diff --git a/website/content/docs/hcp/index.mdx b/website/content/docs/hcp/index.mdx index a5ddf719683..a3e47c88752 100644 --- a/website/content/docs/hcp/index.mdx +++ b/website/content/docs/hcp/index.mdx @@ -29,7 +29,7 @@ You must set the following environment variables to enable Packer to push metada You can set these additional environment variables to control how metadata is pushed to the registry. -- `HCP_PACKER_BUILD_FINGERPRINT` - A unique identifier assigned to each build. HCP Packer uses this identifier to determine if metadata for a build on the registry is complete. By default, HCP Packer uses the HEAD Git SHA for the template file. If the template is not version controlled, you must set this environment variable to a unique value before running `packer build`. +- `HCP_PACKER_BUILD_FINGERPRINT` - A unique identifier assigned to each build. HCP Packer uses this identifier to determine if metadata for a build on the registry is complete. By default, Packer will generate a new unique fingerprint for the template file on each new build. If you want to continue building an iteration, you will need to either set a unique fingerprint as HCP_PACKER_BUILD_FINGERPRINT before starting the build or you may use the generated one on subsequent builds. - `HCP_PACKER_REGISTRY` - When set, Packer does not push image metadata to HCP Packer from an otherwise configured template. Allowed values are [0|OFF].