Skip to content

Commit a74b21b

Browse files
ycohen278ycohen278
authored
Update bucket attributes in order to disable soft delete (#155)
* Update bucket attributes in order to disable soft delete. * Update bucket attributes in order to disable soft delete
1 parent 8abc663 commit a74b21b

File tree

5 files changed

+77
-12
lines changed

5 files changed

+77
-12
lines changed

‎cli_tools/common/domain/interfaces.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
// StorageClientInterface represents GCS storage client
2626
type StorageClientInterface interface {
2727
CreateBucket(bucketName string, project string, attrs *storage.BucketAttrs) error
28+
UpdateBucket(bucketName string, attrs storage.BucketAttrsToUpdate) error
2829
Buckets(projectID string) *storage.BucketIterator
2930
GetBucketAttrs(bucket string) (*storage.BucketAttrs, error)
3031
GetBucket(bucket string) *storage.BucketHandle

‎cli_tools/common/utils/storage/scratch_bucket_creator.go‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,14 @@ func (c *ScratchBucketCreator) getBucketAttrs(fileGcsPath string, project string
7777
// if file is not provided, fallback to input / default zone.
7878
bucketAttrs, err = c.getBucketAttrsOnFallbackZone(project, fallbackZone)
7979
}
80-
81-
if err == nil {
82-
// Enable Uniform-Bucket-Level-Access by default in image-import/export tools.
83-
bucketAttrs.UniformBucketLevelAccess.Enabled = enableUniformBucketLevelAccess
80+
if err != nil {
81+
return nil, err
8482
}
8583

86-
if bucketAttrs != nil {
87-
bucketAttrs.SoftDeletePolicy = &storage.SoftDeletePolicy{RetentionDuration: 0}
88-
}
84+
// Enable Uniform-Bucket-Level-Access by default in image-import/export tools.
85+
bucketAttrs.UniformBucketLevelAccess.Enabled = enableUniformBucketLevelAccess
86+
// Disable soft delete.
87+
bucketAttrs.SoftDeletePolicy = &storage.SoftDeletePolicy{RetentionDuration: 0}
8988

9089
return bucketAttrs, err
9190
}
@@ -134,16 +133,30 @@ func (c *ScratchBucketCreator) createBucketIfNotExisting(project string,
134133
if err != nil {
135134
return "", err
136135
}
137-
if foundBucketAttrs != nil {
138-
return foundBucketAttrs.Location, nil
136+
137+
if foundBucketAttrs == nil {
138+
log.Printf("Creating scratch bucket `%v` in %v region", bucketAttrs.Name, bucketAttrs.Location)
139+
if err := c.StorageClient.CreateBucket(bucketAttrs.Name, project, bucketAttrs); err != nil {
140+
return "", err
141+
}
139142
}
140-
log.Printf("Creating scratch bucket `%v` in %v region", bucketAttrs.Name, bucketAttrs.Location)
141-
if err := c.StorageClient.CreateBucket(bucketAttrs.Name, project, bucketAttrs); err != nil {
143+
log.Printf("Updating soft delete property of scratch bucket `%v` in %v region", bucketAttrs.Name, bucketAttrs.Location)
144+
if err = c.removeSoftDeleteFromBucket(bucketAttrs.Name); err != nil {
142145
return "", err
143146
}
147+
if foundBucketAttrs != nil {
148+
return foundBucketAttrs.Location, nil
149+
}
144150
return bucketAttrs.Location, nil
145151
}
146152

153+
func (c *ScratchBucketCreator) removeSoftDeleteFromBucket(bucketName string) error {
154+
bucketAttrToUpdate := storage.BucketAttrsToUpdate{
155+
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
156+
}
157+
return c.StorageClient.UpdateBucket(bucketName, bucketAttrToUpdate)
158+
}
159+
147160
// IsBucketInProject checks if bucket belongs to a project
148161
func (c *ScratchBucketCreator) IsBucketInProject(project string, bucketName string) bool {
149162
foundBucketAttrs, _ := c.getBucketAttrsIfInProject(project, bucketName)

‎cli_tools/common/utils/storage/scratch_bucket_creator_test.go‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ func TestCreateScratchBucketNoSourceFileDefaultBucketCreatedBasedOnDefaultRegion
5858
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
5959
}).Return(nil)
6060

61+
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
62+
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
63+
}).Return(nil)
64+
6165
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
6266
bucket, region, err := c.CreateScratchBucket("", project, "", true)
6367
assert.Equal(t, expectedBucket, bucket)
@@ -83,6 +87,10 @@ func TestCreateScratchBucketNoSourceFileTranslateGoogleDomainDefaultBucketCreate
8387
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
8488
}).Return(nil)
8589

90+
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
91+
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
92+
}).Return(nil)
93+
8694
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
8795
bucket, region, err := c.CreateScratchBucket("", project, "", true)
8896
assert.Equal(t, expectedBucket, bucket)
@@ -108,6 +116,10 @@ func TestCreateScratchBucketNoSourceFileBucketCreatedBasedOnInputZone(t *testing
108116
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
109117
}).Return(nil)
110118

119+
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
120+
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
121+
}).Return(nil)
122+
111123
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
112124
bucket, region, err := c.CreateScratchBucket("", project, "asia-east1-b", true)
113125
assert.Equal(t, expectedBucket, bucket)
@@ -173,7 +185,9 @@ func TestCreateScratchBucketNewBucketCreatedProject(t *testing.T) {
173185
mockStorageClient := mocks.NewMockStorageClientInterface(mockCtrl)
174186
mockStorageClient.EXPECT().GetBucketAttrs(sourceBucketAttrs.Name).Return(sourceBucketAttrs, nil).Times(1)
175187
mockStorageClient.EXPECT().CreateBucket("project1-daisy-bkt-us-west2", project, scratchBucketAttrs).Return(nil).Times(1)
176-
188+
mockStorageClient.EXPECT().UpdateBucket("project1-daisy-bkt-us-west2", storage.BucketAttrsToUpdate{
189+
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
190+
}).Return(nil).Times(1)
177191
mockBucketIterator := mocks.NewMockBucketIteratorInterface(mockCtrl)
178192
first := mockBucketIterator.EXPECT().Next().Return(anotherBucketAttrs, nil)
179193
second := mockBucketIterator.EXPECT().Next().Return(sourceBucketAttrs, nil)
@@ -230,6 +244,9 @@ func TestCreateScratchBucketErrorRetrievingSourceFileBucketMetadataDefaultBucket
230244
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
231245
}).Return(nil)
232246

247+
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
248+
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
249+
}).Return(nil).Times(1)
233250
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
234251
bucket, region, err := c.CreateScratchBucket("gs://sourcebucket/sourcefile", project, "", true)
235252
assert.Equal(t, expectedBucket, bucket)
@@ -256,6 +273,10 @@ func TestCreateScratchBucketErrorRetrievingSourceFileBucketMetadataBucketCreated
256273
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
257274
}).Return(nil)
258275

276+
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
277+
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
278+
}).Return(nil)
279+
259280
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
260281
bucket, region, err := c.CreateScratchBucket("gs://sourcebucket/sourcefile", project, "asia-east1-b", true)
261282
assert.Equal(t, expectedBucket, bucket)
@@ -282,6 +303,10 @@ func TestCreateScratchBucketNilSourceFileBucketMetadataDefaultBucketCreated(t *t
282303
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
283304
}).Return(nil)
284305

306+
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
307+
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
308+
}).Return(nil)
309+
285310
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
286311
bucket, region, err := c.CreateScratchBucket("gs://sourcebucket/sourcefile", project, "", true)
287312
assert.Equal(t, expectedBucket, bucket)
@@ -356,6 +381,9 @@ func TestCreateScratchBucketReturnsExistingScratchBucketNoCreate(t *testing.T) {
356381
Return(mockBucketIterator).
357382
Times(1)
358383

384+
mockStorageClient.EXPECT().UpdateBucket("project1-daisy-bkt-us-west2", storage.BucketAttrsToUpdate{
385+
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
386+
}).Return(nil).Times(1)
359387
c := ScratchBucketCreator{mockStorageClient, ctx, mockBucketIteratorCreator}
360388
bucket, region, err := c.CreateScratchBucket("gs://sourcebucket/sourcefile", projectID, "", true)
361389
assert.Equal(t, "project1-daisy-bkt-us-west2", bucket)

‎cli_tools/common/utils/storage/storage_client.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,15 @@ func (sc *Client) CreateBucket(
7373
return nil
7474
}
7575

76+
// UpdateBucket updates a GCS bucket
77+
func (sc *Client) UpdateBucket(
78+
bucketName string, attrs storage.BucketAttrsToUpdate) error {
79+
if _, err := sc.StorageClient.Bucket(bucketName).Update(sc.Ctx, attrs); err != nil {
80+
return daisy.Errf("Error updating bucket `%v` : %v", bucketName, err)
81+
}
82+
return nil
83+
}
84+
7685
// Buckets returns a bucket iterator for all buckets within a project
7786
func (sc *Client) Buckets(projectID string) *storage.BucketIterator {
7887
return sc.StorageClient.Buckets(sc.Ctx, projectID)

‎cli_tools/mocks/mock_storage_client.go‎

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)