diff --git a/internal/hcp/api/mock_service.go b/internal/hcp/api/mock_service.go index 9a5137050..db2f64a7a 100644 --- a/internal/hcp/api/mock_service.go +++ b/internal/hcp/api/mock_service.go @@ -19,7 +19,7 @@ import ( // Upon calling a service method a boolean is set to true to indicate that a method has been called. // To skip the setting of these booleans set TrackCalledServiceMethods to false; defaults to true in NewMockPackerClientService(). type MockPackerClientService struct { - CreateBucketCalled, UpdateBucketCalled, BucketAlreadyExist bool + CreateBucketCalled, UpdateBucketCalled, GetBucketCalled, BucketNotFound bool CreateVersionCalled, GetVersionCalled, VersionAlreadyExist, VersionCompleted bool CreateBuildCalled, UpdateBuildCalled, ListBuildsCalled, BuildAlreadyDone bool TrackCalledServiceMethods bool @@ -55,14 +55,6 @@ func (svc *MockPackerClientService) PackerServiceCreateBucket( params *hcpPackerService.PackerServiceCreateBucketParams, _ runtime.ClientAuthInfoWriter, opts ...hcpPackerService.ClientOption, ) (*hcpPackerService.PackerServiceCreateBucketOK, error) { - - if svc.BucketAlreadyExist { - return nil, status.Error( - codes.AlreadyExists, - fmt.Sprintf("Code:%d %s", codes.AlreadyExists, codes.AlreadyExists.String()), - ) - } - if params.Body.Name == "" { return nil, errors.New("no bucket name was passed in") } @@ -84,6 +76,19 @@ func (svc *MockPackerClientService) PackerServiceCreateBucket( return ok, nil } +func (svc *MockPackerClientService) PackerServiceGetBucket( + params *hcpPackerService.PackerServiceGetBucketParams, _ runtime.ClientAuthInfoWriter, + opts ...hcpPackerService.ClientOption, +) (*hcpPackerService.PackerServiceGetBucketOK, error) { + if svc.TrackCalledServiceMethods { + svc.GetBucketCalled = true + } + if svc.BucketNotFound { + return nil, status.Error(codes.NotFound, fmt.Sprintf("Code:%d %s", codes.NotFound, codes.NotFound.String())) + } + return hcpPackerService.NewPackerServiceGetBucketOK(), nil +} + func (svc *MockPackerClientService) PackerServiceUpdateBucket( params *hcpPackerService.PackerServiceUpdateBucketParams, _ runtime.ClientAuthInfoWriter, opts ...hcpPackerService.ClientOption, diff --git a/internal/hcp/api/service_bucket.go b/internal/hcp/api/service_bucket.go index bfdbe99f8..c512f1dee 100644 --- a/internal/hcp/api/service_bucket.go +++ b/internal/hcp/api/service_bucket.go @@ -36,20 +36,24 @@ func (c *Client) DeleteBucket( return c.Packer.PackerServiceDeleteBucket(deleteBktParams, nil) } -// UpsertBucket tries to create a bucket on a HCP Packer Registry. If the bucket exists it will -// handle the error and update the bucket with the provided details. +// UpsertBucket will create or update a bucket. It calls GetBucket first, if the bucket is not found it creates that bucket +// If GetBucket succeeded we then call UpdateBucket description and bucket labels in case they've changed. +// GetBucket is used instead of CreateBucket since users with bucket level access to specific existing buckets can not create new buckets. func (c *Client) UpsertBucket( ctx context.Context, bucketName, bucketDescription string, bucketLabels map[string]string, ) error { - // Create bucket if exist we continue as is, eventually we want to treat this like an upsert. - _, err := c.CreateBucket(ctx, bucketName, bucketDescription, bucketLabels) - if err != nil && !CheckErrorCode(err, codes.AlreadyExists) { - return err - } + getParams := hcpPackerService.NewPackerServiceGetBucketParamsWithContext(ctx) + getParams.LocationOrganizationID = c.OrganizationID + getParams.LocationProjectID = c.ProjectID + getParams.BucketName = bucketName - if err == nil { - return nil + _, err := c.Packer.PackerServiceGetBucket(getParams, nil) + if err != nil { + if CheckErrorCode(err, codes.NotFound) { + _, err = c.CreateBucket(ctx, bucketName, bucketDescription, bucketLabels) + } + return err } params := hcpPackerService.NewPackerServiceUpdateBucketParamsWithContext(ctx) diff --git a/internal/hcp/registry/types.bucket.go b/internal/hcp/registry/types.bucket.go index 9ebf42eb7..17f3e34c0 100644 --- a/internal/hcp/registry/types.bucket.go +++ b/internal/hcp/registry/types.bucket.go @@ -122,7 +122,6 @@ func (bucket *Bucket) Initialize( } bucket.Destination = fmt.Sprintf("%s/%s", bucket.client.OrganizationID, bucket.client.ProjectID) - err := bucket.client.UpsertBucket(ctx, bucket.Name, bucket.Description, bucket.BucketLabels) if err != nil { return fmt.Errorf("failed to initialize bucket %q: %w", bucket.Name, err) diff --git a/internal/hcp/registry/types.bucket_service_test.go b/internal/hcp/registry/types.bucket_service_test.go index a89233d68..16d046980 100644 --- a/internal/hcp/registry/types.bucket_service_test.go +++ b/internal/hcp/registry/types.bucket_service_test.go @@ -13,6 +13,7 @@ import ( func TestInitialize_NewBucketNewVersion(t *testing.T) { mockService := hcpPackerAPI.NewMockPackerClientService() + mockService.BucketNotFound = true b := &Bucket{ Name: "TestBucket", @@ -34,10 +35,15 @@ func TestInitialize_NewBucketNewVersion(t *testing.T) { t.Errorf("unexpected failure: %v", err) } + if !mockService.GetBucketCalled { + t.Errorf("expected a call to GetBucket but it didn't happen") + } if !mockService.CreateBucketCalled { t.Errorf("expected a call to CreateBucket but it didn't happen") } - + if mockService.UpdateBucketCalled { + t.Errorf("unexpected call to UpdateBucket") + } if !mockService.CreateVersionCalled { t.Errorf("expected a call to CreateVersion but it didn't happen") } @@ -65,8 +71,6 @@ func TestInitialize_NewBucketNewVersion(t *testing.T) { func TestInitialize_UnsetTemplateTypeError(t *testing.T) { mockService := hcpPackerAPI.NewMockPackerClientService() - mockService.BucketAlreadyExist = true - b := &Bucket{ Name: "TestBucket", client: &hcpPackerAPI.Client{ @@ -90,7 +94,6 @@ func TestInitialize_UnsetTemplateTypeError(t *testing.T) { func TestInitialize_ExistingBucketNewVersion(t *testing.T) { mockService := hcpPackerAPI.NewMockPackerClientService() - mockService.BucketAlreadyExist = true b := &Bucket{ Name: "TestBucket", @@ -110,6 +113,12 @@ func TestInitialize_ExistingBucketNewVersion(t *testing.T) { if err != nil { t.Errorf("unexpected failure: %v", err) } + if !mockService.GetBucketCalled { + t.Errorf("expected call to GetBucket but it didn't happen") + } + if mockService.CreateBucketCalled { + t.Errorf("unexpected call to CreateBucket") + } if !mockService.UpdateBucketCalled { t.Errorf("expected call to UpdateBucket but it didn't happen") @@ -143,7 +152,6 @@ func TestInitialize_ExistingBucketNewVersion(t *testing.T) { func TestInitialize_ExistingBucketExistingVersion(t *testing.T) { mockService := hcpPackerAPI.NewMockPackerClientService() - mockService.BucketAlreadyExist = true mockService.VersionAlreadyExist = true b := &Bucket{ @@ -175,12 +183,18 @@ func TestInitialize_ExistingBucketExistingVersion(t *testing.T) { t.Errorf("unexpected call to CreateBucket") } + if !mockService.GetBucketCalled { + t.Errorf("expected call to GetBucket but it didn't happen") + } + if mockService.CreateBucketCalled { + t.Errorf("unexpected call to CreateBucket") + } if !mockService.UpdateBucketCalled { t.Errorf("expected call to UpdateBucket but it didn't happen") } if mockService.CreateVersionCalled { - t.Errorf("unexpected a call to CreateVersion") + t.Errorf("unexpected call to CreateVersion") } if !mockService.GetVersionCalled { @@ -188,7 +202,7 @@ func TestInitialize_ExistingBucketExistingVersion(t *testing.T) { } if mockService.CreateBuildCalled { - t.Errorf("unexpected a call to CreateBuild") + t.Errorf("unexpected call to CreateBuild") } if b.Version.ID != "version-id" { @@ -212,7 +226,6 @@ func TestInitialize_ExistingBucketExistingVersion(t *testing.T) { func TestInitialize_ExistingBucketCompleteVersion(t *testing.T) { mockService := hcpPackerAPI.NewMockPackerClientService() - mockService.BucketAlreadyExist = true mockService.VersionAlreadyExist = true mockService.VersionCompleted = true mockService.BuildAlreadyDone = true @@ -238,6 +251,15 @@ func TestInitialize_ExistingBucketCompleteVersion(t *testing.T) { t.Errorf("unexpected failure: %v", err) } + if !mockService.GetBucketCalled { + t.Errorf("expected call to GetBucket, but it didn't happen") + } + if !mockService.UpdateBucketCalled { + t.Errorf("expected call to UpdateBucket, but it didn't happen") + } + if mockService.CreateBucketCalled { + t.Errorf("unexpected call to CreateBucket") + } if mockService.CreateVersionCalled { t.Errorf("unexpected call to CreateVersion") } @@ -257,7 +279,6 @@ func TestInitialize_ExistingBucketCompleteVersion(t *testing.T) { func TestUpdateBuildStatus(t *testing.T) { mockService := hcpPackerAPI.NewMockPackerClientService() - mockService.BucketAlreadyExist = true mockService.VersionAlreadyExist = true b := &Bucket{ @@ -310,9 +331,7 @@ func TestUpdateBuildStatus(t *testing.T) { func TestUpdateBuildStatus_DONENoImages(t *testing.T) { mockService := hcpPackerAPI.NewMockPackerClientService() - mockService.BucketAlreadyExist = true mockService.VersionAlreadyExist = true - b := &Bucket{ Name: "TestBucket", client: &hcpPackerAPI.Client{ diff --git a/internal/hcp/registry/types.bucket_test.go b/internal/hcp/registry/types.bucket_test.go index 1b50fb887..194a0c27d 100644 --- a/internal/hcp/registry/types.bucket_test.go +++ b/internal/hcp/registry/types.bucket_test.go @@ -291,7 +291,6 @@ func TestBucket_PopulateVersion(t *testing.T) { t.Setenv("HCP_PACKER_BUILD_FINGERPRINT", "test-run-"+strconv.Itoa(i)) mockService := hcpPackerAPI.NewMockPackerClientService() - mockService.BucketAlreadyExist = true mockService.VersionAlreadyExist = true mockService.BuildAlreadyDone = tt.buildCompleted