Skip to content

Commit 1d88bb8

Browse files
fix(storage-bucket-object): do not delete object on update content, j… (#10038) (#18479)
[upstream:672734df9f9288e14d854871815242b7af410e9f] Signed-off-by: Modular Magician <[email protected]>
1 parent 5508d74 commit 1d88bb8

File tree

3 files changed

+109
-33
lines changed

3 files changed

+109
-33
lines changed

.changelog/10038.txt

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
```release-note:enhancement
2+
storage: The update on `google_storage_bucket_object` content is now no more delete to create object, but just push a new object if the content change
3+
4+
```

google/services/storage/resource_storage_bucket_object.go

+40-33
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Copyright (c) HashiCorp, Inc.
22
// SPDX-License-Identifier: MPL-2.0
3+
// SPDX-License-Identifier: MPL-2.0
34
package storage
45

56
import (
@@ -90,7 +91,6 @@ func ResourceStorageBucketObject() *schema.Resource {
9091
"content": {
9192
Type: schema.TypeString,
9293
Optional: true,
93-
ForceNew: true,
9494
ExactlyOneOf: []string{"source"},
9595
Sensitive: true,
9696
Computed: true,
@@ -122,7 +122,6 @@ func ResourceStorageBucketObject() *schema.Resource {
122122
Type: schema.TypeString,
123123
// This field is not Computed because it needs to trigger a diff.
124124
Optional: true,
125-
ForceNew: true,
126125
// Makes the diff message nicer:
127126
// detect_md5hash: "1XcnP/iFw/hNrbhXi7QTmQ==" => "different hash" (forces new resource)
128127
// Instead of the more confusing:
@@ -378,45 +377,53 @@ func resourceStorageBucketObjectUpdate(d *schema.ResourceData, meta interface{})
378377
bucket := d.Get("bucket").(string)
379378
name := d.Get("name").(string)
380379

381-
objectsService := storage.NewObjectsService(config.NewStorageClientWithTimeoutOverride(userAgent, d.Timeout(schema.TimeoutUpdate)))
382-
getCall := objectsService.Get(bucket, name)
380+
if d.HasChange("content") || d.HasChange("detect_md5hash") {
381+
// The KMS key name are not able to be set on create :
382+
// or you get error: Error uploading object test-maarc: googleapi: Error 400: Malformed Cloud KMS crypto key: projects/myproject/locations/myregion/keyRings/mykeyring/cryptoKeys/mykeyname/cryptoKeyVersions/1, invalid
383+
d.Set("kms_key_name", nil)
384+
return resourceStorageBucketObjectCreate(d, meta)
385+
} else {
383386

384-
res, err := getCall.Do()
385-
if err != nil {
386-
return fmt.Errorf("Error retrieving object during update %s: %s", name, err)
387-
}
387+
objectsService := storage.NewObjectsService(config.NewStorageClientWithTimeoutOverride(userAgent, d.Timeout(schema.TimeoutUpdate)))
388+
getCall := objectsService.Get(bucket, name)
388389

389-
hasRetentionChanges := d.HasChange("retention")
390-
if hasRetentionChanges {
391-
if v, ok := d.GetOk("retention"); ok {
392-
res.Retention = expandObjectRetention(v)
393-
} else {
394-
res.Retention = nil
395-
res.NullFields = append(res.NullFields, "Retention")
390+
res, err := getCall.Do()
391+
if err != nil {
392+
return fmt.Errorf("Error retrieving object during update %s: %s", name, err)
396393
}
397-
}
398394

399-
if d.HasChange("event_based_hold") {
400-
v := d.Get("event_based_hold")
401-
res.EventBasedHold = v.(bool)
402-
}
395+
hasRetentionChanges := d.HasChange("retention")
396+
if hasRetentionChanges {
397+
if v, ok := d.GetOk("retention"); ok {
398+
res.Retention = expandObjectRetention(v)
399+
} else {
400+
res.Retention = nil
401+
res.NullFields = append(res.NullFields, "Retention")
402+
}
403+
}
403404

404-
if d.HasChange("temporary_hold") {
405-
v := d.Get("temporary_hold")
406-
res.TemporaryHold = v.(bool)
407-
}
405+
if d.HasChange("event_based_hold") {
406+
v := d.Get("event_based_hold")
407+
res.EventBasedHold = v.(bool)
408+
}
408409

409-
updateCall := objectsService.Update(bucket, name, res)
410-
if hasRetentionChanges {
411-
updateCall.OverrideUnlockedRetention(true)
412-
}
413-
_, err = updateCall.Do()
410+
if d.HasChange("temporary_hold") {
411+
v := d.Get("temporary_hold")
412+
res.TemporaryHold = v.(bool)
413+
}
414414

415-
if err != nil {
416-
return fmt.Errorf("Error updating object %s: %s", name, err)
417-
}
415+
updateCall := objectsService.Update(bucket, name, res)
416+
if hasRetentionChanges {
417+
updateCall.OverrideUnlockedRetention(true)
418+
}
419+
_, err = updateCall.Do()
418420

419-
return nil
421+
if err != nil {
422+
return fmt.Errorf("Error updating object %s: %s", name, err)
423+
}
424+
425+
return nil
426+
}
420427
}
421428

422429
func resourceStorageBucketObjectRead(d *schema.ResourceData, meta interface{}) error {

google/services/storage/resource_storage_bucket_object_test.go

+65
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,55 @@ func TestAccStorageObject_retention(t *testing.T) {
459459
})
460460
}
461461

462+
func TestResourceStorageBucketObjectUpdate_ContentChange(t *testing.T) {
463+
t.Parallel()
464+
465+
bucketName := acctest.TestBucketName(t)
466+
initialContent := []byte("initial content")
467+
updatedContent := []byte("updated content")
468+
h := md5.New()
469+
if _, err := h.Write(initialContent); err != nil {
470+
t.Errorf("error calculating md5: %v", err)
471+
}
472+
dataMd5 := base64.StdEncoding.EncodeToString(h.Sum(nil))
473+
474+
h2 := md5.New()
475+
if _, err := h2.Write(updatedContent); err != nil {
476+
t.Errorf("error calculating md5: %v", err)
477+
}
478+
newDataMd5 := base64.StdEncoding.EncodeToString(h2.Sum(nil))
479+
// Update the object content and verify
480+
acctest.VcrTest(t, resource.TestCase{
481+
PreCheck: func() { acctest.AccTestPreCheck(t) },
482+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
483+
CheckDestroy: testAccStorageObjectDestroyProducer(t),
484+
Steps: []resource.TestStep{
485+
{
486+
Config: testGoogleStorageBucketsObjectCustomContent(bucketName, string(initialContent)),
487+
Check: resource.ComposeTestCheckFunc(
488+
testAccCheckGoogleStorageObject(t, bucketName, objectName, dataMd5),
489+
resource.TestCheckResourceAttr(
490+
"google_storage_bucket_object.object",
491+
"content",
492+
string(initialContent),
493+
),
494+
),
495+
},
496+
{
497+
Config: testGoogleStorageBucketsObjectCustomContent(bucketName, string(updatedContent)),
498+
Check: resource.ComposeTestCheckFunc(
499+
testAccCheckGoogleStorageObject(t, bucketName, objectName, newDataMd5),
500+
resource.TestCheckResourceAttr(
501+
"google_storage_bucket_object.object",
502+
"content",
503+
string(updatedContent),
504+
),
505+
),
506+
},
507+
},
508+
})
509+
}
510+
462511
func testAccCheckGoogleStorageObject(t *testing.T, bucket, object, md5 string) resource.TestCheckFunc {
463512
return testAccCheckGoogleStorageObjectWithEncryption(t, bucket, object, md5, "")
464513
}
@@ -539,6 +588,22 @@ func testAccStorageObjectDestroyProducer(t *testing.T) func(s *terraform.State)
539588
}
540589
}
541590

591+
func testGoogleStorageBucketsObjectCustomContent(bucketName string, customContent string) string {
592+
return fmt.Sprintf(`
593+
resource "google_storage_bucket" "bucket" {
594+
name = "%s"
595+
location = "US"
596+
force_destroy = true
597+
}
598+
599+
resource "google_storage_bucket_object" "object" {
600+
name = "%s"
601+
bucket = google_storage_bucket.bucket.name
602+
content = "%s"
603+
}
604+
`, bucketName, objectName, customContent)
605+
}
606+
542607
func testGoogleStorageBucketsObjectContent(bucketName string) string {
543608
return fmt.Sprintf(`
544609
resource "google_storage_bucket" "bucket" {

0 commit comments

Comments
 (0)