Remove soft deletion when GCS bucket is provided as an arg (#159)

This will clear the soft deletion on GCS buckets created by gcloud.

cli_tools/common/domain/interfaces.go

@@ -104,6 +104,7 @@ type ZoneValidatorInterface interface {
 type ScratchBucketCreatorInterface interface {
 	CreateScratchBucket(sourceFileFlag string, projectFlag string, fallbackZone string,
 		enableUniformBucketLevelAccess bool) (string, string, error)
+	RemoveSoftDeleteFromBucket(bucketAttrs *storage.BucketAttrs) error
 	IsBucketInProject(project string, bucketName string) bool
 }
 

cli_tools/common/utils/param/param_utils.go

@@ -99,9 +99,15 @@ func populateScratchBucketGcsPath(scratchBucketGcsPath *string, zone string, mgc
 		}
 
 		scratchBucketAttrs, err := storageClient.GetBucketAttrs(scratchBucketName)
-		if err == nil {
-			scratchBucketRegion = scratchBucketAttrs.Location
+		if err != nil {
+			return "", err
+		}
+
+		if err := scratchBucketCreator.RemoveSoftDeleteFromBucket(scratchBucketAttrs); err != nil {
+			return "", err
 		}
+
+		scratchBucketRegion = scratchBucketAttrs.Location
 	}
 	return scratchBucketRegion, nil
 }

cli_tools/common/utils/param/populator_test.go

@@ -50,7 +50,10 @@ func TestPopulator_PopulateMissingParametersReturnsErrorWhenZoneCantBeRetrieved(
 	mockResourceLocationRetriever.EXPECT().GetZone("us-west2", project).Return("",
 		daisy.Errf("zone not found")).Times(1)
 	mockStorageClient := mocks.NewMockStorageClientInterface(mockCtrl)
-	mockStorageClient.EXPECT().GetBucketAttrs("scratchbucket").Return(&storage.BucketAttrs{Location: "us-west2"}, nil).Times(1)
+	bucketAttrs := &storage.BucketAttrs{Location: "us-west2"}
+	mockStorageClient.EXPECT().GetBucketAttrs("scratchbucket").Return(bucketAttrs, nil).Times(1)
+	mockScratchBucketCreator.EXPECT().RemoveSoftDeleteFromBucket(bucketAttrs).Return(nil).Times(1)
+
 	mockNetworkResolver := newNoOpNetworkResolver(mockCtrl)
 	mockMachineSeriesDetector := newMockN2N1MachineSeriesDetector(mockCtrl)
 	err := NewPopulator(
@@ -84,7 +87,9 @@ func TestPopulator_PropagatesErrorFromNetworkResolver(t *testing.T) {
 	mockScratchBucketCreator.EXPECT().IsBucketInProject(project, "scratchbucket").Return(true)
 	mockResourceLocationRetriever := mocks.NewMockResourceLocationRetrieverInterface(mockCtrl)
 	mockStorageClient := mocks.NewMockStorageClientInterface(mockCtrl)
-	mockStorageClient.EXPECT().GetBucketAttrs("scratchbucket").Return(&storage.BucketAttrs{Location: "us-west2"}, nil).Times(1)
+	bucketAttrs := &storage.BucketAttrs{Location: "us-west2"}
+	mockStorageClient.EXPECT().GetBucketAttrs("scratchbucket").Return(bucketAttrs, nil).Times(1)
+	mockScratchBucketCreator.EXPECT().RemoveSoftDeleteFromBucket(bucketAttrs).Return(nil).Times(1)
 	mockNetworkResolver := mocks.NewMockNetworkResolver(mockCtrl)
 	mockNetworkResolver.EXPECT().ResolveAndValidateNetworkAndSubnet("original-network", "original-subnet", "us-west2", "a_project").Return("", "", daisy.Errf("network cannot be found"))
 	mockMachineSeriesDetector := newMockN2N1MachineSeriesDetector(mockCtrl)
@@ -119,7 +124,9 @@ func TestPopulator_UsesReturnValuesFromNetworkResolver(t *testing.T) {
 	mockScratchBucketCreator.EXPECT().IsBucketInProject(project, "scratchbucket").Return(true)
 	mockResourceLocationRetriever := mocks.NewMockResourceLocationRetrieverInterface(mockCtrl)
 	mockStorageClient := mocks.NewMockStorageClientInterface(mockCtrl)
-	mockStorageClient.EXPECT().GetBucketAttrs("scratchbucket").Return(&storage.BucketAttrs{Location: "us-west2"}, nil).Times(1)
+	bucketAttrs := &storage.BucketAttrs{Location: "us-west2"}
+	mockStorageClient.EXPECT().GetBucketAttrs("scratchbucket").Return(bucketAttrs, nil).Times(1)
+	mockScratchBucketCreator.EXPECT().RemoveSoftDeleteFromBucket(bucketAttrs).Return(nil).Times(1)
 	mockNetworkResolver := mocks.NewMockNetworkResolver(mockCtrl)
 	mockNetworkResolver.EXPECT().ResolveAndValidateNetworkAndSubnet(
 		"original-network", "original-subnet", "us-west2", "a_project").Return("fixed-network", "fixed-subnet", nil)
@@ -323,7 +330,10 @@ func TestPopulator_PopulateMissingParametersReturnsErrorWhenPopulateRegionFails(
 	mockScratchBucketCreator.EXPECT().IsBucketInProject(project, "scratchbucket").Return(true)
 	mockResourceLocationRetriever := mocks.NewMockResourceLocationRetrieverInterface(mockCtrl)
 	mockStorageClient := mocks.NewMockStorageClientInterface(mockCtrl)
-	mockStorageClient.EXPECT().GetBucketAttrs("scratchbucket").Return(&storage.BucketAttrs{Location: region}, nil)
+	bucketAttrs := &storage.BucketAttrs{Location: region}
+	mockStorageClient.EXPECT().GetBucketAttrs("scratchbucket").Return(bucketAttrs, nil).Times(1)
+	mockScratchBucketCreator.EXPECT().RemoveSoftDeleteFromBucket(bucketAttrs).Return(nil).Times(1)
+
 	mockNetworkResolver := newNoOpNetworkResolver(mockCtrl)
 	mockMachineSeriesDetector := newMockN2N1MachineSeriesDetector(mockCtrl)
 	err := NewPopulator(
@@ -362,7 +372,9 @@ func TestPopulator_PopulateMissingParametersDoesNotChangeProvidedScratchBucketAn
 	mockResourceLocationRetriever := mocks.NewMockResourceLocationRetrieverInterface(mockCtrl)
 	mockResourceLocationRetriever.EXPECT().GetZone(expectedRegion, project).Return(expectedZone, nil).Times(1)
 	mockStorageClient := mocks.NewMockStorageClientInterface(mockCtrl)
-	mockStorageClient.EXPECT().GetBucketAttrs(expectedBucketName).Return(&storage.BucketAttrs{Location: expectedRegion}, nil)
+	bucketAttrs := &storage.BucketAttrs{Location: expectedRegion}
+	mockStorageClient.EXPECT().GetBucketAttrs(expectedBucketName).Return(bucketAttrs, nil).Times(1)
+	mockScratchBucketCreator.EXPECT().RemoveSoftDeleteFromBucket(bucketAttrs).Return(nil).Times(1)
 	mockNetworkResolver := newNoOpNetworkResolver(mockCtrl)
 	mockMachineSeriesDetector := newMockN2N1MachineSeriesDetector(mockCtrl)
 	err := NewPopulator(
@@ -595,7 +607,9 @@ func TestPopulator_PopulateMissingParametersPopulatesStorageLocationWithScratchB
 	mockScratchBucketCreator.EXPECT().IsBucketInProject(project, "scratchbucket").Return(true)
 	mockResourceLocationRetriever := mocks.NewMockResourceLocationRetrieverInterface(mockCtrl)
 	mockStorageClient := mocks.NewMockStorageClientInterface(mockCtrl)
-	mockStorageClient.EXPECT().GetBucketAttrs("scratchbucket").Return(&storage.BucketAttrs{Location: region}, nil)
+	bucketAttrs := &storage.BucketAttrs{Location: region}
+	mockStorageClient.EXPECT().GetBucketAttrs("scratchbucket").Return(bucketAttrs, nil).Times(1)
+	mockScratchBucketCreator.EXPECT().RemoveSoftDeleteFromBucket(bucketAttrs).Return(nil).Times(1)
 	mockResourceLocationRetriever.EXPECT().GetLargestStorageLocation(region).Return("US")
 	mockNetworkResolver := newNoOpNetworkResolver(mockCtrl)
 	mockMachineSeriesDetector := newMockN2N1MachineSeriesDetector(mockCtrl)

cli_tools/common/utils/storage/scratch_bucket_creator.go

@@ -136,15 +136,16 @@ func (c *ScratchBucketCreator) createBucketIfNotExisting(project string,
 	}
 
 	if foundBucketAttrs != nil {
-		return foundBucketAttrs.Location, c.removeSoftDeleteFromBucket(foundBucketAttrs)
+		return foundBucketAttrs.Location, c.RemoveSoftDeleteFromBucket(foundBucketAttrs)
 
 	}
 
 	log.Printf("Creating scratch bucket `%v` in %v region", bucketAttrs.Name, bucketAttrs.Location)
 	return bucketAttrs.Location, c.StorageClient.CreateBucket(bucketAttrs.Name, project, bucketAttrs)
 }
 
-func (c *ScratchBucketCreator) removeSoftDeleteFromBucket(bucketAttrs *storage.BucketAttrs) error {
+// RemoveSoftDeleteFromBucket removed the soft deletion protection from the given bucket.
+func (c *ScratchBucketCreator) RemoveSoftDeleteFromBucket(bucketAttrs *storage.BucketAttrs) error {
 	if bucketAttrs.SoftDeletePolicy.RetentionDuration == 0 {
 		return nil
 	}