From a2ffde0a4f3e38b73c41d1956033061c900dca10 Mon Sep 17 00:00:00 2001 From: Modular Magician Date: Tue, 26 Sep 2023 22:21:48 +0000 Subject: [PATCH] 5.0.0 - remove computed keys from JSON in monitoring dashboard (#9065) Signed-off-by: Modular Magician --- .changelog/9065.txt | 3 ++ .../resource_monitoring_dashboard.go | 37 +++++++++++++++---- .../resource_monitoring_dashboard_test.go | 2 - .../docs/r/monitoring_dashboard.html.markdown | 6 +++ 4 files changed, 38 insertions(+), 10 deletions(-) create mode 100644 .changelog/9065.txt diff --git a/.changelog/9065.txt b/.changelog/9065.txt new file mode 100644 index 0000000000..c9592896f2 --- /dev/null +++ b/.changelog/9065.txt @@ -0,0 +1,3 @@ +```release-note:breaking-change +monitoring: fixed perma-diffs in `google_monitoring_dashboard.dashboard_json` by suppressing values returned by the API that are not in configuration +``` diff --git a/google-beta/services/monitoring/resource_monitoring_dashboard.go b/google-beta/services/monitoring/resource_monitoring_dashboard.go index ccc5caccab..a22665212e 100644 --- a/google-beta/services/monitoring/resource_monitoring_dashboard.go +++ b/google-beta/services/monitoring/resource_monitoring_dashboard.go @@ -16,24 +16,45 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) -func monitoringDashboardDiffSuppress(k, old, new string, d *schema.ResourceData) bool { - computedFields := []string{"etag", "name"} +// This recursive function takes an old map and a new map and is intended to remove the computed keys +// from the old json string (stored in state) so that it doesn't show a diff if it's not defined in the +// new map's json string (defined in config) +func removeComputedKeys(old map[string]interface{}, new map[string]interface{}) map[string]interface{} { + for k, v := range old { + if _, ok := old[k]; ok && new[k] == nil { + delete(old, k) + continue + } + + if reflect.ValueOf(v).Kind() == reflect.Map { + old[k] = removeComputedKeys(v.(map[string]interface{}), new[k].(map[string]interface{})) + continue + } + + if reflect.ValueOf(v).Kind() == reflect.Slice { + for i, j := range v.([]interface{}) { + if reflect.ValueOf(j).Kind() == reflect.Map { + old[k].([]interface{})[i] = removeComputedKeys(j.(map[string]interface{}), new[k].([]interface{})[i].(map[string]interface{})) + } + } + continue + } + } + + return old +} +func monitoringDashboardDiffSuppress(k, old, new string, d *schema.ResourceData) bool { oldMap, err := structure.ExpandJsonFromString(old) if err != nil { return false } - newMap, err := structure.ExpandJsonFromString(new) if err != nil { return false } - for _, f := range computedFields { - delete(oldMap, f) - delete(newMap, f) - } - + oldMap = removeComputedKeys(oldMap, newMap) return reflect.DeepEqual(oldMap, newMap) } diff --git a/google-beta/services/monitoring/resource_monitoring_dashboard_test.go b/google-beta/services/monitoring/resource_monitoring_dashboard_test.go index 4f09c76583..0fff4a3777 100644 --- a/google-beta/services/monitoring/resource_monitoring_dashboard_test.go +++ b/google-beta/services/monitoring/resource_monitoring_dashboard_test.go @@ -85,8 +85,6 @@ func TestAccMonitoringDashboard_rowLayout(t *testing.T) { } func TestAccMonitoringDashboard_update(t *testing.T) { - // TODO: Fix requires a breaking change https://github.com/hashicorp/terraform-provider-google/issues/9976 - t.Skip() t.Parallel() acctest.VcrTest(t, resource.TestCase{ diff --git a/website/docs/r/monitoring_dashboard.html.markdown b/website/docs/r/monitoring_dashboard.html.markdown index b3d0e0dd63..97c32f85b9 100644 --- a/website/docs/r/monitoring_dashboard.html.markdown +++ b/website/docs/r/monitoring_dashboard.html.markdown @@ -114,6 +114,12 @@ The following arguments are supported: The JSON representation of a dashboard, following the format at https://cloud.google.com/monitoring/api/ref_v3/rest/v1/projects.dashboards. The representation of an existing dashboard can be found by using the [API Explorer](https://cloud.google.com/monitoring/api/ref_v3/rest/v1/projects.dashboards/get) + ~> **Warning:** Because this is represented as a JSON string, Terraform doesn't have underlying information to know + which fields in the string have defaults. To prevent permanent diffs from default values, Terraform will attempt to + suppress diffs where the value is returned in the JSON string but doesn't exist in the configuration. Consequently, + legitmate remove-only diffs will also be suppressed. For Terraform to detect the diff, key removals must also be + accompanied by a non-removal change (trivial or not). + - - -