From 4db615aa1ad4cd03a74ec19c2ce078f47e453fb2 Mon Sep 17 00:00:00 2001 From: Vincent Roseberry Date: Fri, 24 Nov 2017 14:46:44 -0800 Subject: [PATCH 1/3] Detect server or local changes to bucket object when using source field --- google/resource_storage_bucket_object.go | 38 ++++++++++++++- google/resource_storage_bucket_object_test.go | 46 +++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/google/resource_storage_bucket_object.go b/google/resource_storage_bucket_object.go index d3a93f88e98..3299078d4e1 100644 --- a/google/resource_storage_bucket_object.go +++ b/google/resource_storage_bucket_object.go @@ -9,8 +9,11 @@ import ( "github.com/hashicorp/terraform/helper/schema" + "crypto/md5" + "encoding/base64" "google.golang.org/api/googleapi" "google.golang.org/api/storage/v1" + "io/ioutil" ) func resourceStorageBucketObject() *schema.Resource { @@ -77,7 +80,28 @@ func resourceStorageBucketObject() *schema.Resource { "md5hash": &schema.Schema{ Type: schema.TypeString, - Computed: true, + Optional: true, + ForceNew: true, + // Makes the diff message nicer: + // md5hash: "1XcnP/iFw/hNrbhXi7QTmQ==" => "different hash" (forces new resource) + // Instead of the more confusing: + // md5hash: "1XcnP/iFw/hNrbhXi7QTmQ==" => "" (forces new resource) + Default: "different hash", + // If `content` is used, always suppress the diff. Equivalent to Computed behavior. + // If `source` is used: + // 1. Compute the md5 hash of the local file + // 2. Compare the computed md5 hash with the hash stored in Cloud Storage + // 3. Don't suppress the diff iff they don't match + DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { + // `old` is the md5 hash we retrieved from the server in the ReadFunc + if source, ok := d.GetOkExists("source"); ok { + if old != getFileMd5Hash(source.(string)) { + return false + } + } + + return true + }, }, "predefined_acl": &schema.Schema{ @@ -221,3 +245,15 @@ func resourceStorageBucketObjectDelete(d *schema.ResourceData, meta interface{}) return nil } + +func getFileMd5Hash(filename string) string { + data, err := ioutil.ReadFile(filename) + if err != nil { + log.Printf("[WARN] Failed to read source file %q. Cannot compute md5 hash for it.", filename) + return "" + } + + h := md5.New() + h.Write(data) + return base64.StdEncoding.EncodeToString(h.Sum(nil)) +} diff --git a/google/resource_storage_bucket_object_test.go b/google/resource_storage_bucket_object_test.go index b548db5d73d..494e2de28d3 100644 --- a/google/resource_storage_bucket_object_test.go +++ b/google/resource_storage_bucket_object_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform/terraform" "google.golang.org/api/storage/v1" + "os" ) var tf, err = ioutil.TempFile("", "tf-gce-test") @@ -46,6 +47,51 @@ func TestAccGoogleStorageObject_basic(t *testing.T) { }) } +func TestAccGoogleStorageObject_recreate(t *testing.T) { + t.Parallel() + + bucketName := testBucketName() + + writeFile := func(name string, data []byte) string { + h := md5.New() + h.Write(data) + data_md5 := base64.StdEncoding.EncodeToString(h.Sum(nil)) + + ioutil.WriteFile(name, data, 0644) + return data_md5 + } + data_md5 := writeFile(tf.Name(), []byte("data data data")) + updated_data_md5 := writeFile(tf.Name()+".update", []byte("datum")) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + if err != nil { + panic(err) + } + testAccPreCheck(t) + }, + Providers: testAccProviders, + CheckDestroy: testAccGoogleStorageObjectDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testGoogleStorageBucketsObjectBasic(bucketName), + Check: testAccCheckGoogleStorageObject(bucketName, objectName, data_md5), + }, + resource.TestStep{ + PreConfig: func() { + updateName := tf.Name() + ".update" + err := os.Rename(updateName, tf.Name()) + if err != nil { + t.Errorf("Failed to rename %s to %s", updateName, tf.Name()) + } + }, + Config: testGoogleStorageBucketsObjectBasic(bucketName), + Check: testAccCheckGoogleStorageObject(bucketName, objectName, updated_data_md5), + }, + }, + }) +} + func TestAccGoogleStorageObject_content(t *testing.T) { t.Parallel() From 0dbcd483e2f6919234d6659350e45d3480249593 Mon Sep 17 00:00:00 2001 From: Vincent Roseberry Date: Fri, 24 Nov 2017 15:02:46 -0800 Subject: [PATCH 2/3] Improve tests by removing global err and tf vars --- .../resource_compute_instance_migrate_test.go | 2 +- google/resource_container_cluster_test.go | 1 + google/resource_storage_bucket_object_test.go | 102 ++++++++---------- 3 files changed, 46 insertions(+), 59 deletions(-) diff --git a/google/resource_compute_instance_migrate_test.go b/google/resource_compute_instance_migrate_test.go index e608417b545..e3c4c91e0ac 100644 --- a/google/resource_compute_instance_migrate_test.go +++ b/google/resource_compute_instance_migrate_test.go @@ -810,7 +810,7 @@ func runInstanceMigrateTest(t *testing.T, id, testName string, version int, attr ID: id, Attributes: attributes, } - is, err = resourceComputeInstanceMigrateState(version, is, meta) + is, err := resourceComputeInstanceMigrateState(version, is, meta) if err != nil { t.Fatal(err) } diff --git a/google/resource_container_cluster_test.go b/google/resource_container_cluster_test.go index e064d339ad6..7bcbcefa93d 100644 --- a/google/resource_container_cluster_test.go +++ b/google/resource_container_cluster_test.go @@ -864,6 +864,7 @@ func checkMapMatch(attributes map[string]string, attr string, gcpMap map[string] func checkBoolMatch(attributes map[string]string, attr string, gcpBool bool) string { // Handle the case where an unset value defaults to false var tf bool + var err error if attributes[attr] == "" { tf = false } else { diff --git a/google/resource_storage_bucket_object_test.go b/google/resource_storage_bucket_object_test.go index 494e2de28d3..e5eaba6aef2 100644 --- a/google/resource_storage_bucket_object_test.go +++ b/google/resource_storage_bucket_object_test.go @@ -14,10 +14,10 @@ import ( "os" ) -var tf, err = ioutil.TempFile("", "tf-gce-test") -var bucketName = "tf-gce-bucket-test" -var objectName = "tf-gce-test" -var content = "now this is content!" +const ( + objectName = "tf-gce-test" + content = "now this is content!" +) func TestAccGoogleStorageObject_basic(t *testing.T) { t.Parallel() @@ -28,19 +28,15 @@ func TestAccGoogleStorageObject_basic(t *testing.T) { h.Write(data) data_md5 := base64.StdEncoding.EncodeToString(h.Sum(nil)) - ioutil.WriteFile(tf.Name(), data, 0644) + testFile := getNewTmpTestFile(t, "tf-test") + ioutil.WriteFile(testFile.Name(), data, 0644) resource.Test(t, resource.TestCase{ - PreCheck: func() { - if err != nil { - panic(err) - } - testAccPreCheck(t) - }, + PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccGoogleStorageObjectDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testGoogleStorageBucketsObjectBasic(bucketName), + Config: testGoogleStorageBucketsObjectBasic(bucketName, testFile.Name()), Check: testAccCheckGoogleStorageObject(bucketName, objectName, data_md5), }, }, @@ -60,32 +56,28 @@ func TestAccGoogleStorageObject_recreate(t *testing.T) { ioutil.WriteFile(name, data, 0644) return data_md5 } - data_md5 := writeFile(tf.Name(), []byte("data data data")) - updated_data_md5 := writeFile(tf.Name()+".update", []byte("datum")) + testFile := getNewTmpTestFile(t, "tf-test") + data_md5 := writeFile(testFile.Name(), []byte("data data data")) + updated_data_md5 := writeFile(testFile.Name()+".update", []byte("datum")) resource.Test(t, resource.TestCase{ - PreCheck: func() { - if err != nil { - panic(err) - } - testAccPreCheck(t) - }, + PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccGoogleStorageObjectDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testGoogleStorageBucketsObjectBasic(bucketName), + Config: testGoogleStorageBucketsObjectBasic(bucketName, testFile.Name()), Check: testAccCheckGoogleStorageObject(bucketName, objectName, data_md5), }, resource.TestStep{ PreConfig: func() { - updateName := tf.Name() + ".update" - err := os.Rename(updateName, tf.Name()) + updateName := testFile.Name() + ".update" + err := os.Rename(updateName, testFile.Name()) if err != nil { - t.Errorf("Failed to rename %s to %s", updateName, tf.Name()) + t.Errorf("Failed to rename %s to %s", updateName, testFile.Name()) } }, - Config: testGoogleStorageBucketsObjectBasic(bucketName), + Config: testGoogleStorageBucketsObjectBasic(bucketName, testFile.Name()), Check: testAccCheckGoogleStorageObject(bucketName, objectName, updated_data_md5), }, }, @@ -101,14 +93,10 @@ func TestAccGoogleStorageObject_content(t *testing.T) { h.Write(data) data_md5 := base64.StdEncoding.EncodeToString(h.Sum(nil)) - ioutil.WriteFile(tf.Name(), data, 0644) + testFile := getNewTmpTestFile(t, "tf-test") + ioutil.WriteFile(testFile.Name(), data, 0644) resource.Test(t, resource.TestCase{ - PreCheck: func() { - if err != nil { - panic(err) - } - testAccPreCheck(t) - }, + PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccGoogleStorageObjectDestroy, Steps: []resource.TestStep{ @@ -134,16 +122,12 @@ func TestAccGoogleStorageObject_withContentCharacteristics(t *testing.T) { h := md5.New() h.Write(data) data_md5 := base64.StdEncoding.EncodeToString(h.Sum(nil)) - ioutil.WriteFile(tf.Name(), data, 0644) + testFile := getNewTmpTestFile(t, "tf-test") + ioutil.WriteFile(testFile.Name(), data, 0644) disposition, encoding, language, content_type := "inline", "compress", "en", "binary/octet-stream" resource.Test(t, resource.TestCase{ - PreCheck: func() { - if err != nil { - panic(err) - } - testAccPreCheck(t) - }, + PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccGoogleStorageObjectDestroy, Steps: []resource.TestStep{ @@ -174,21 +158,17 @@ func TestAccGoogleStorageObject_cacheControl(t *testing.T) { h := md5.New() h.Write(data) data_md5 := base64.StdEncoding.EncodeToString(h.Sum(nil)) - ioutil.WriteFile(tf.Name(), data, 0644) + testFile := getNewTmpTestFile(t, "tf-test") + ioutil.WriteFile(testFile.Name(), data, 0644) cacheControl := "private" resource.Test(t, resource.TestCase{ - PreCheck: func() { - if err != nil { - panic(err) - } - testAccPreCheck(t) - }, + PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccGoogleStorageObjectDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testGoogleStorageBucketsObject_cacheControl(bucketName, cacheControl), + Config: testGoogleStorageBucketsObject_cacheControl(bucketName, testFile.Name(), cacheControl), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleStorageObject(bucketName, objectName, data_md5), resource.TestCheckResourceAttr( @@ -207,16 +187,12 @@ func TestAccGoogleStorageObject_storageClass(t *testing.T) { h := md5.New() h.Write(data) data_md5 := base64.StdEncoding.EncodeToString(h.Sum(nil)) - ioutil.WriteFile(tf.Name(), data, 0644) + testFile := getNewTmpTestFile(t, "tf-test") + ioutil.WriteFile(testFile.Name(), data, 0644) storageClass := "MULTI_REGIONAL" resource.Test(t, resource.TestCase{ - PreCheck: func() { - if err != nil { - panic(err) - } - testAccPreCheck(t) - }, + PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccGoogleStorageObjectDestroy, Steps: []resource.TestStep{ @@ -291,7 +267,7 @@ resource "google_storage_bucket_object" "object" { `, bucketName, objectName, content) } -func testGoogleStorageBucketsObjectBasic(bucketName string) string { +func testGoogleStorageBucketsObjectBasic(bucketName, sourceFilename string) string { return fmt.Sprintf(` resource "google_storage_bucket" "bucket" { name = "%s" @@ -302,7 +278,7 @@ resource "google_storage_bucket_object" "object" { bucket = "${google_storage_bucket.bucket.name}" source = "%s" } -`, bucketName, objectName, tf.Name()) +`, bucketName, objectName, sourceFilename) } func testGoogleStorageBucketsObject_optionalContentFields( @@ -324,7 +300,7 @@ resource "google_storage_bucket_object" "object" { `, bucketName, objectName, content, disposition, encoding, language, content_type) } -func testGoogleStorageBucketsObject_cacheControl(bucketName, cacheControl string) string { +func testGoogleStorageBucketsObject_cacheControl(bucketName, sourceFilename, cacheControl string) string { return fmt.Sprintf(` resource "google_storage_bucket" "bucket" { name = "%s" @@ -336,7 +312,7 @@ resource "google_storage_bucket_object" "object" { source = "%s" cache_control = "%s" } -`, bucketName, objectName, tf.Name(), cacheControl) +`, bucketName, objectName, sourceFilename, cacheControl) } func testGoogleStorageBucketsObject_storageClass(bucketName string, storageClass string) string { @@ -353,3 +329,13 @@ resource "google_storage_bucket_object" "object" { } `, bucketName, objectName, content, storageClass) } + +// Creates a new tmp test file. Fails the current test if we cannot create +// new tmp file in the filesystem. +func getNewTmpTestFile(t *testing.T, prefix string) *os.File { + testFile, err := ioutil.TempFile("", prefix) + if err != nil { + t.Fatalf("Cannot create temp file: %s", err) + } + return testFile +} From 9401c25438d208d695acb80e45479d2b0b144fdd Mon Sep 17 00:00:00 2001 From: Vincent Roseberry Date: Mon, 27 Nov 2017 16:22:29 -0800 Subject: [PATCH 3/3] Address Dana's comments --- google/resource_storage_bucket_object_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/google/resource_storage_bucket_object_test.go b/google/resource_storage_bucket_object_test.go index e5eaba6aef2..eefba83ed89 100644 --- a/google/resource_storage_bucket_object_test.go +++ b/google/resource_storage_bucket_object_test.go @@ -58,7 +58,8 @@ func TestAccGoogleStorageObject_recreate(t *testing.T) { } testFile := getNewTmpTestFile(t, "tf-test") data_md5 := writeFile(testFile.Name(), []byte("data data data")) - updated_data_md5 := writeFile(testFile.Name()+".update", []byte("datum")) + updatedName := testFile.Name() + ".update" + updated_data_md5 := writeFile(updatedName, []byte("datum")) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -71,10 +72,9 @@ func TestAccGoogleStorageObject_recreate(t *testing.T) { }, resource.TestStep{ PreConfig: func() { - updateName := testFile.Name() + ".update" - err := os.Rename(updateName, testFile.Name()) + err := os.Rename(updatedName, testFile.Name()) if err != nil { - t.Errorf("Failed to rename %s to %s", updateName, testFile.Name()) + t.Errorf("Failed to rename %s to %s", updatedName, testFile.Name()) } }, Config: testGoogleStorageBucketsObjectBasic(bucketName, testFile.Name()),