internal: reorganise registry/HCP code

This commit reorganises the code for both the registry/API and the
Orchestrator/Registry.

The main difference with the previous version is how stuff is exposed.
Now we only expose a Registry interface to the outside (previously named
Orchestrator), which has several implementations: null is the default,
and is returned if HCP is not enabled.

The other implementations being HCL/JSON, both private to the hcp
sub-package.

The api (previously `registry') is the set of functionality that
abstracts and calls the HCP API.
It was meant to be merged with the `hcp' package, but because of a
dependency loop with the datasources, both are separated for now.

command/build.go

@@ -14,7 +14,7 @@ import (
 
 	"github.com/hashicorp/hcl/v2"
 	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
-	"github.com/hashicorp/packer/internal/hcp"
+	"github.com/hashicorp/packer/internal/hcp/registry"
 	"github.com/hashicorp/packer/packer"
 	"golang.org/x/sync/semaphore"
 
@@ -90,13 +90,13 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, cla *BuildArgs) int
 		return ret
 	}
 
-	hcpHandler, diags := hcp.GetOrchestrator(packerStarter)
+	hcpRegistry, diags := registry.New(packerStarter)
 	ret = writeDiags(c.Ui, nil, diags)
 	if ret != 0 {
 		return ret
 	}
 
-	err := hcpHandler.PopulateIteration(buildCtx)
+	err := hcpRegistry.PopulateIteration(buildCtx)
 	if err != nil {
 		return writeDiags(c.Ui, nil, hcl.Diagnostics{
 			&hcl.Diagnostic{
@@ -219,14 +219,24 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, cla *BuildArgs) int
 
 			defer limitParallel.Release(1)
 
-			err := hcpHandler.BuildStart(buildCtx, hcpMap[name])
+			err := hcpRegistry.StartBuild(buildCtx, hcpMap[name])
+			// Seems odd to require this error check here. Now that it is an error we can just exit with diag
 			if err != nil {
-				if errors.As(err, &hcp.BuildDone{}) {
-					ui.Say(fmt.Sprintf(
-						"skipping HCP-enabled build %q: already done.",
-						name))
+				// If the build is already done, we skip without a warning
+				if errors.As(err, &registry.ErrBuildAlreadyDone{}) {
+					ui.Say(fmt.Sprintf("skipping already done build %q", name))
 					return
 				}
+				writeDiags(c.Ui, nil, hcl.Diagnostics{
+					&hcl.Diagnostic{
+						Summary: fmt.Sprintf(
+							"hcp: failed to start build %q",
+							name),
+						Severity: hcl.DiagError,
+						Detail:   err.Error(),
+					},
+				})
+				return
 			}
 
 			log.Printf("Starting build run: %s", name)
@@ -237,7 +247,7 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, cla *BuildArgs) int
 			buildDuration := buildEnd.Sub(buildStart)
 			fmtBuildDuration := durafmt.Parse(buildDuration).LimitFirstN(2)
 
-			runArtifacts, hcperr := hcpHandler.BuildDone(
+			runArtifacts, hcperr := hcpRegistry.CompleteBuild(
 				buildCtx,
 				hcpMap[name],
 				runArtifacts,

datasource/hcp-packer-image/data.go

@@ -17,7 +17,7 @@ import (
 	"github.com/hashicorp/packer-plugin-sdk/hcl2helper"
 	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
 	"github.com/hashicorp/packer-plugin-sdk/template/config"
-	packerregistry "github.com/hashicorp/packer/internal/registry"
+	hcpapi "github.com/hashicorp/packer/internal/hcp/api"
 )
 
 type Datasource struct {
@@ -139,7 +139,7 @@ func (d *Datasource) OutputSpec() hcldec.ObjectSpec {
 func (d *Datasource) Execute() (cty.Value, error) {
 	ctx := context.TODO()
 
-	cli, err := packerregistry.NewClient()
+	cli, err := hcpapi.NewClient()
 	if err != nil {
 		return cty.NullVal(cty.EmptyObject), err
 	}
@@ -150,7 +150,7 @@ func (d *Datasource) Execute() (cty.Value, error) {
 		log.Printf("[INFO] Reading info from HCP Packer registry (%s) [project_id=%s, organization_id=%s, iteration_id=%s]",
 			d.config.Bucket, cli.ProjectID, cli.OrganizationID, d.config.IterationID)
 
-		iter, err := cli.GetIteration(ctx, d.config.Bucket, packerregistry.GetIteration_byID(d.config.IterationID))
+		iter, err := cli.GetIteration(ctx, d.config.Bucket, hcpapi.GetIteration_byID(d.config.IterationID))
 		if err != nil {
 			return cty.NullVal(cty.EmptyObject), fmt.Errorf(
 				"error retrieving image iteration from HCP Packer registry: %s",

datasource/hcp-packer-iteration/data.go

@@ -15,7 +15,7 @@ import (
 	"github.com/hashicorp/packer-plugin-sdk/hcl2helper"
 	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
 	"github.com/hashicorp/packer-plugin-sdk/template/config"
-	packerregistry "github.com/hashicorp/packer/internal/registry"
+	hcpapi "github.com/hashicorp/packer/internal/hcp/api"
 )
 
 type Datasource struct {
@@ -99,7 +99,7 @@ func (d *Datasource) OutputSpec() hcldec.ObjectSpec {
 func (d *Datasource) Execute() (cty.Value, error) {
 	ctx := context.TODO()
 
-	cli, err := packerregistry.NewClient()
+	cli, err := hcpapi.NewClient()
 	if err != nil {
 		return cty.NullVal(cty.EmptyObject), err
 	}

datasource/hcp-packer-iteration/data_acc_test.go

@@ -8,11 +8,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/packer-plugin-sdk/acctest"
-)
-
-const (
-	HCPClientID     string = "HCP_CLIENT_ID"
-	HCPClientSecret string = "HCP_CLIENT_SECRET"
+	"github.com/hashicorp/packer/internal/hcp/env"
 )
 
 //go:embed test-fixtures/template.pkr.hcl
@@ -31,8 +27,8 @@ var testDatasourceBasic string
 // as defined above.
 
 func TestAccDatasource_HCPPackerIteration(t *testing.T) {
-	if os.Getenv(HCPClientID) == "" && os.Getenv(HCPClientSecret) == "" {
-		t.Skip(fmt.Sprintf("Acceptance tests skipped unless envs %q and %q are set", HCPClientID, HCPClientSecret))
+	if os.Getenv(env.HCPClientID) == "" && os.Getenv(env.HCPClientSecret) == "" {
+		t.Skipf(fmt.Sprintf("Acceptance tests skipped unless envs %q and %q are set", env.HCPClientID, env.HCPClientSecret))
 		return
 	}
 

datasource/packer-image-iteration/data.go

@@ -16,7 +16,7 @@ import (
 	"github.com/hashicorp/packer-plugin-sdk/hcl2helper"
 	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
 	"github.com/hashicorp/packer-plugin-sdk/template/config"
-	packerregistry "github.com/hashicorp/packer/internal/registry"
+	hcpapi "github.com/hashicorp/packer/internal/hcp/api"
 )
 
 // Type for Packer datasource has been renamed temporarily to prevent it from being
@@ -136,7 +136,7 @@ func (d *DeactivatedDatasource) OutputSpec() hcldec.ObjectSpec {
 func (d *DeactivatedDatasource) Execute() (cty.Value, error) {
 	ctx := context.TODO()
 
-	cli, err := packerregistry.NewClient()
+	cli, err := hcpapi.NewClient()
 	if err != nil {
 		return cty.NullVal(cty.EmptyObject), err
 	}

hcl2template/common_test.go

@@ -15,7 +15,6 @@ import (
 	dnull "github.com/hashicorp/packer/datasource/null"
 	. "github.com/hashicorp/packer/hcl2template/internal"
 	hcl2template "github.com/hashicorp/packer/hcl2template/internal"
-	packerregistry "github.com/hashicorp/packer/internal/registry"
 	"github.com/hashicorp/packer/packer"
 	"github.com/zclconf/go-cty/cty"
 )
@@ -357,19 +356,11 @@ var cmpOpts = []cmp.Option{
 		packer.CoreBuildProvisioner{},
 		packer.CoreBuildPostProcessor{},
 		null.Builder{},
-		packerregistry.Bucket{},
-		packerregistry.Iteration{},
 	),
 	cmpopts.IgnoreFields(PackerConfig{},
 		"Cwd",     // Cwd will change for every os type
 		"HCPVars", // HCPVars will not be filled-in during parsing
 	),
-	cmpopts.IgnoreFields(packerregistry.Iteration{},
-		"Fingerprint", // Fingerprint will change everytime
-	),
-	cmpopts.IgnoreFields(packerregistry.Bucket{},
-		"SourceImagesToParentIterations", // Requires execution of datasource at this time
-	),
 	cmpopts.IgnoreFields(VariableAssignment{},
 		"Expr", // its an interface
 	),

hcl2template/types.build.hcp_packer_registry.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/gohcl"
-	packerregistry "github.com/hashicorp/packer/internal/registry"
 )
 
 type HCPPackerRegistryBlock struct {
@@ -21,20 +20,6 @@ type HCPPackerRegistryBlock struct {
 	HCL2Ref
 }
 
-func (b *HCPPackerRegistryBlock) WriteToBucketConfig(bucket *packerregistry.Bucket) {
-	if b == nil {
-		return
-	}
-	bucket.Description = b.Description
-	bucket.BucketLabels = b.BucketLabels
-	bucket.BuildLabels = b.BuildLabels
-	// If there's already a Slug this was set from env variable.
-	// In Packer, env variable overrides config values so we keep it that way for consistency.
-	if bucket.Slug == "" && b.Slug != "" {
-		bucket.Slug = b.Slug
-	}
-}
-
 func (p *Parser) decodeHCPRegistry(block *hcl.Block, cfg *PackerConfig) (*HCPPackerRegistryBlock, hcl.Diagnostics) {
 	par := &HCPPackerRegistryBlock{}
 	body := block.Body

internal/registry/client.go → internal/hcp/api/client.go

@@ -1,4 +1,5 @@
-package registry
+// Package api provides access to the HCP Packer Registry API.
+package api
 
 import (
 	"fmt"
@@ -8,7 +9,7 @@ import (
 	projectSvc "github.com/hashicorp/hcp-sdk-go/clients/cloud-resource-manager/preview/2019-12-10/client/project_service"
 	rmmodels "github.com/hashicorp/hcp-sdk-go/clients/cloud-resource-manager/preview/2019-12-10/models"
 	"github.com/hashicorp/hcp-sdk-go/httpclient"
-	"github.com/hashicorp/packer/internal/registry/env"
+	"github.com/hashicorp/packer/internal/hcp/env"
 	"github.com/hashicorp/packer/version"
 )
 

internal/registry/errors.go → internal/hcp/api/errors.go

@@ -1,4 +1,4 @@
-package registry
+package api
 
 import (
 	"fmt"
@@ -23,11 +23,11 @@ func (c *ClientError) Error() string {
 	return fmt.Sprintf("status %d: err %v", c.StatusCode, c.Err)
 }
 
-// checkErrorCode checks the error string for err for some code and returns true
+// CheckErrorCode checks the error string for err for some code and returns true
 // if the code is found. Ideally this function should use status.FromError
 // https://pkg.go.dev/google.golang.org/grpc/status#pkg-functions but that
 // doesn't appear to work for all of the Cloud Packer Service response errors.
-func checkErrorCode(err error, code codes.Code) bool {
+func CheckErrorCode(err error, code codes.Code) bool {
 	if err == nil {
 		return false
 	}

internal/registry/mock_service.go → internal/hcp/api/mock_service.go

@@ -1,4 +1,4 @@
-package registry
+package api
 
 import (
 	"errors"

internal/registry/service.go → internal/hcp/api/service.go

@@ -1,4 +1,4 @@
-package registry
+package api
 
 import (
 	"context"
@@ -55,7 +55,7 @@ func (client *Client) UpsertBucket(
 	// Create bucket if exist we continue as is, eventually we want to treat
 	// this like an upsert
 	_, err := client.CreateBucket(ctx, bucketSlug, bucketDescription, bucketLabels)
-	if err != nil && !checkErrorCode(err, codes.AlreadyExists) {
+	if err != nil && !CheckErrorCode(err, codes.AlreadyExists) {
 		return err
 	}
 

internal/registry/env/env.go → internal/hcp/env/env.go

@@ -1,3 +1,4 @@
+// Package env provides HCP Packer environment variables.
 package env
 
 import (

internal/hcp/registry/artifact.go

@@ -0,0 +1,37 @@
+package registry
+
+import (
+	"fmt"
+)
+
+const BuilderId = "packer.post-processor.hpc-packer-registry"
+
+type registryArtifact struct {
+	BucketSlug  string
+	IterationID string
+	BuildName   string
+}
+
+func (a *registryArtifact) BuilderId() string {
+	return BuilderId
+}
+
+func (*registryArtifact) Id() string {
+	return ""
+}
+
+func (a *registryArtifact) Files() []string {
+	return []string{}
+}
+
+func (a *registryArtifact) String() string {
+	return fmt.Sprintf("Published metadata to HCP Packer registry packer/%s/iterations/%s", a.BucketSlug, a.IterationID)
+}
+
+func (*registryArtifact) State(name string) interface{} {
+	return nil
+}
+
+func (a *registryArtifact) Destroy() error {
+	return nil
+}