From 6d4333ae2bee336f2a19330e430d105d73ebbc10 Mon Sep 17 00:00:00 2001 From: Patrick Rauchfuss Date: Wed, 7 Sep 2022 09:31:10 +0200 Subject: [PATCH 1/6] suppress dataplex labels --- .../resources/resource_storage_bucket.go.erb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/mmv1/third_party/terraform/resources/resource_storage_bucket.go.erb b/mmv1/third_party/terraform/resources/resource_storage_bucket.go.erb index ffc758bffb5a..8533dc5c3219 100644 --- a/mmv1/third_party/terraform/resources/resource_storage_bucket.go.erb +++ b/mmv1/third_party/terraform/resources/resource_storage_bucket.go.erb @@ -82,6 +82,8 @@ func resourceStorageBucket() *schema.Resource { "labels": { Type: schema.TypeMap, Optional: true, + // GCP (Dataplex) automatically adds labels + DiffSuppressFunc: resourceDataplexLabelDiffSuppress, Elem: &schema.Schema{Type: schema.TypeString}, Description: `A set of key/value label pairs to assign to the bucket.`, }, @@ -376,6 +378,22 @@ func resourceStorageBucket() *schema.Resource { } } +const resourceDataplexGoogleProvidedLabelPrefix = "labels.goog-dataplex" + +func resourceDataplexLabelDiffSuppress(k, old, new string, d *schema.ResourceData) bool { + if strings.HasPrefix(k, resourceDataplexGoogleProvidedLabelPrefix) && new == "" { + return true + } + + // Let diff be determined by labels (above) + if strings.HasPrefix(k, "labels.%") { + return true + } + + // For other keys, don't suppress diff. + return false +} + // Is the old bucket retention policy locked? func isPolicyLocked(_ context.Context, old, new, _ interface{}) bool { if old == nil || new == nil { From 9ad55e91f44d30987fc651431075e22d26b42ee6 Mon Sep 17 00:00:00 2001 From: Patrick Rauchfuss Date: Mon, 12 Sep 2022 10:55:32 +0200 Subject: [PATCH 2/6] add test label suppress tests --- .../tests/resource_storage_bucket_test.go.erb | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb b/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb index 0c08fb33e0ce..12e02b0ba1f2 100644 --- a/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb @@ -8,6 +8,7 @@ import ( "regexp" "testing" "time" + "strings" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" @@ -980,6 +981,109 @@ func TestAccStorageBucket_retentionPolicyLocked(t *testing.T) { }) } +func TestLabelDiffSuppress(t *testing.T) { + cases := map[string]struct { + K, Old, New string + ExpectDiffSuppress bool + }{ + "missing goog-dataplex-asset-id": { + K: "labels.goog-dataplex-asset-id", + Old: "test-bucket", + New: "", + ExpectDiffSuppress: true, + }, + "explicit missing goog-dataplex-asset-id": { + K: "labels.goog-dataplex-asset-id", + Old: "test-bucket", + New: "test-bucket", + ExpectDiffSuppress: false, + }, + "missing goog-dataplex-lake-id": { + K: "labels.goog-dataplex-lake-id", + Old: "test-lake", + New: "", + ExpectDiffSuppress: true, + }, + "explicit goog-dataplex-lake-id": { + K: "labels.goog-dataplex-lake-id", + Old: "test-lake", + New: "test-lake", + ExpectDiffSuppress: false, + }, + "missing goog-dataplex-project-id": { + K: "labels.goog-dataplex-project-id", + Old: "test-project-12345", + New: "", + ExpectDiffSuppress: true, + }, + "explicit goog-dataplex-project-id": { + K: "labels.goog-dataplex-project-id", + Old: "test-project-12345", + New: "test-project-12345", + ExpectDiffSuppress: false, + }, + "missing goog-dataplex-zone-id": { + K: "labels.goog-dataplex-zone-id", + Old: "test-zone1", + New: "", + ExpectDiffSuppress: true, + }, + "explicit goog-dataplex-zone-id": { + K: "labels.goog-dataplex-zone-id", + Old: "test-zone1", + New: "test-zone1", + ExpectDiffSuppress: false, + }, + "labels.%": { + K: "labels.%", + Old: "5", + New: "1", + ExpectDiffSuppress: true, + }, + "deleted custom key": { + K: "labels.my-label", + Old: "my-value", + New: "", + ExpectDiffSuppress: false, + }, + "added custom key": { + K: "labels.my-label", + Old: "", + New: "my-value", + ExpectDiffSuppress: false, + }, + } + for tn, tc := range cases { + if testLabelDiffSuppress(tc.K, tc.Old, tc.New) != tc.ExpectDiffSuppress { + t.Errorf("bad: %s, %q: %q => %q expect DiffSuppress to return %t", tn, tc.K, tc.Old, tc.New, tc.ExpectDiffSuppress) + } + } +} + +var googleProvidedLabels = []string{ + "goog-dataplex-asset-id", + "goog-dataplex-lake-id", + "goog-dataplex-project-id", + "goog-dataplex-zone-id", +} + +func testLabelDiffSuppress(k, old, new string) bool { + // Suppress diffs for the labels provided by Google + for _, label := range googleProvidedLabels { + if strings.Contains(k, label) && new == "" { + return true + } + } + + // Let diff be determined by labels (above) + if strings.Contains(k, "labels.%") { + return true + } + + // For other keys, don't suppress diff. + return false +} + func testAccCheckStorageBucketExists(t *testing.T, n string, bucketName string, bucket *storage.Bucket) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] From 5916bbe840bf6dcb3abe4ccad997dee05c933077 Mon Sep 17 00:00:00 2001 From: Patrick Rauchfuss Date: Wed, 28 Sep 2022 10:42:24 +0200 Subject: [PATCH 3/6] fix test --- .../tests/resource_storage_bucket_test.go.erb | 26 +------------------ 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb b/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb index 12e02b0ba1f2..746963bc5c99 100644 --- a/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb @@ -1054,36 +1054,12 @@ func TestLabelDiffSuppress(t *testing.T) { }, } for tn, tc := range cases { - if testLabelDiffSuppress(tc.K, tc.Old, tc.New) != tc.ExpectDiffSuppress { + if resourceDataplexLabelDiffSuppress(tc.K, tc.Old, tc.New) != tc.ExpectDiffSuppress { t.Errorf("bad: %s, %q: %q => %q expect DiffSuppress to return %t", tn, tc.K, tc.Old, tc.New, tc.ExpectDiffSuppress) } } } -var googleProvidedLabels = []string{ - "goog-dataplex-asset-id", - "goog-dataplex-lake-id", - "goog-dataplex-project-id", - "goog-dataplex-zone-id", -} - -func testLabelDiffSuppress(k, old, new string) bool { - // Suppress diffs for the labels provided by Google - for _, label := range googleProvidedLabels { - if strings.Contains(k, label) && new == "" { - return true - } - } - - // Let diff be determined by labels (above) - if strings.Contains(k, "labels.%") { - return true - } - - // For other keys, don't suppress diff. - return false -} - func testAccCheckStorageBucketExists(t *testing.T, n string, bucketName string, bucket *storage.Bucket) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] From bfda4b0234f574dfe715563c3686d7d8ccf4dbd6 Mon Sep 17 00:00:00 2001 From: Patrick Rauchfuss Date: Wed, 28 Sep 2022 10:44:15 +0200 Subject: [PATCH 4/6] fix test --- .../terraform/tests/resource_storage_bucket_test.go.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb b/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb index 746963bc5c99..92e137449dbb 100644 --- a/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb @@ -1054,7 +1054,7 @@ func TestLabelDiffSuppress(t *testing.T) { }, } for tn, tc := range cases { - if resourceDataplexLabelDiffSuppress(tc.K, tc.Old, tc.New) != tc.ExpectDiffSuppress { + if resourceDataplexLabelDiffSuppress(tc.K, tc.Old, tc.New, nil) != tc.ExpectDiffSuppress { t.Errorf("bad: %s, %q: %q => %q expect DiffSuppress to return %t", tn, tc.K, tc.Old, tc.New, tc.ExpectDiffSuppress) } } From 29b3dbc334c8cf077e1094c91e2f0e8f4b88e871 Mon Sep 17 00:00:00 2001 From: Patrick Rauchfuss Date: Wed, 28 Sep 2022 13:07:23 +0200 Subject: [PATCH 5/6] remove unnecessary package from import --- .../terraform/tests/resource_storage_bucket_test.go.erb | 1 - 1 file changed, 1 deletion(-) diff --git a/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb b/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb index 92e137449dbb..dfc85482c21b 100644 --- a/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb @@ -8,7 +8,6 @@ import ( "regexp" "testing" "time" - "strings" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" From d91dc325aa0b96276f05b52bba094cfdfa1b009e Mon Sep 17 00:00:00 2001 From: Patrick Rauchfuss Date: Thu, 29 Sep 2022 09:10:38 +0200 Subject: [PATCH 6/6] adjust testcases --- .../tests/resource_storage_bucket_test.go.erb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb b/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb index dfc85482c21b..e33fa5329937 100644 --- a/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_storage_bucket_test.go.erb @@ -991,10 +991,10 @@ func TestLabelDiffSuppress(t *testing.T) { New: "", ExpectDiffSuppress: true, }, - "explicit missing goog-dataplex-asset-id": { + "explicit goog-dataplex-asset-id": { K: "labels.goog-dataplex-asset-id", Old: "test-bucket", - New: "test-bucket", + New: "test-bucket-1", ExpectDiffSuppress: false, }, "missing goog-dataplex-lake-id": { @@ -1006,7 +1006,7 @@ func TestLabelDiffSuppress(t *testing.T) { "explicit goog-dataplex-lake-id": { K: "labels.goog-dataplex-lake-id", Old: "test-lake", - New: "test-lake", + New: "test-lake-1", ExpectDiffSuppress: false, }, "missing goog-dataplex-project-id": { @@ -1018,7 +1018,7 @@ func TestLabelDiffSuppress(t *testing.T) { "explicit goog-dataplex-project-id": { K: "labels.goog-dataplex-project-id", Old: "test-project-12345", - New: "test-project-12345", + New: "test-project-12345-1", ExpectDiffSuppress: false, }, "missing goog-dataplex-zone-id": { @@ -1030,7 +1030,7 @@ func TestLabelDiffSuppress(t *testing.T) { "explicit goog-dataplex-zone-id": { K: "labels.goog-dataplex-zone-id", Old: "test-zone1", - New: "test-zone1", + New: "test-zone1-1", ExpectDiffSuppress: false, }, "labels.%": {