From c2ecfbefaf8c381eea139db761b90f0b067a819c Mon Sep 17 00:00:00 2001 From: Michel Rouly Date: Tue, 24 Oct 2023 22:27:37 -0400 Subject: [PATCH 01/14] Mark Grafana API key as sensitive. --- internal/service/grafana/workspace_api_key.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/service/grafana/workspace_api_key.go b/internal/service/grafana/workspace_api_key.go index 14fc7fa53ec..0779b3af0a5 100644 --- a/internal/service/grafana/workspace_api_key.go +++ b/internal/service/grafana/workspace_api_key.go @@ -29,8 +29,9 @@ func ResourceWorkspaceAPIKey() *schema.Resource { Schema: map[string]*schema.Schema{ "key": { - Type: schema.TypeString, - Computed: true, + Type: schema.TypeString, + Computed: true, + Sensitive: true, }, "key_name": { Type: schema.TypeString, From 99a1eda013ca83355315c9d185f9693808350afc Mon Sep 17 00:00:00 2001 From: Michel Rouly Date: Wed, 25 Oct 2023 17:01:11 -0400 Subject: [PATCH 02/14] changelog entry --- .changelog/34105.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/34105.txt diff --git a/.changelog/34105.txt b/.changelog/34105.txt new file mode 100644 index 00000000000..6f37a493431 --- /dev/null +++ b/.changelog/34105.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_grafana_workspace_api_key: Mark API key value as sensitive. +``` From 861fbb37610982e4dc45fd3748ca04062a13408f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 15 Nov 2023 12:03:18 -0500 Subject: [PATCH 03/14] Update 34105.txt --- .changelog/34105.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/34105.txt b/.changelog/34105.txt index 6f37a493431..0ca7447e843 100644 --- a/.changelog/34105.txt +++ b/.changelog/34105.txt @@ -1,3 +1,3 @@ ```release-note:bug -resource/aws_grafana_workspace_api_key: Mark API key value as sensitive. +resource/aws_grafana_workspace_api_key: Change `key` to [`Sensitive`](https://developer.hashicorp.com/terraform/plugin/best-practices/sensitive-state#using-sensitive-flag-functionality) ``` From 118e152c138322b160a4a7e4ef38f04e3addfe7b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 15 Nov 2023 14:52:18 -0500 Subject: [PATCH 04/14] Move 'ujson' from 'service/fms'. --- internal/service/fms/managed_service_data.go | 2 +- internal/{service/fms => }/ujson/LICENSE | 0 internal/{service/fms => }/ujson/quote.go | 0 internal/{service/fms => }/ujson/quote_test.go | 2 +- .../ujson/\302\265json.go" => "internal/ujson/\302\265json.go" | 0 .../ujson/\302\265json_test.go" | 0 6 files changed, 2 insertions(+), 2 deletions(-) rename internal/{service/fms => }/ujson/LICENSE (100%) rename internal/{service/fms => }/ujson/quote.go (100%) rename internal/{service/fms => }/ujson/quote_test.go (98%) rename "internal/service/fms/ujson/\302\265json.go" => "internal/ujson/\302\265json.go" (100%) rename "internal/service/fms/ujson/\302\265json_test.go" => "internal/ujson/\302\265json_test.go" (100%) diff --git a/internal/service/fms/managed_service_data.go b/internal/service/fms/managed_service_data.go index 46591259947..dfe0409ad02 100644 --- a/internal/service/fms/managed_service_data.go +++ b/internal/service/fms/managed_service_data.go @@ -7,7 +7,7 @@ import ( "encoding/json" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-provider-aws/internal/service/fms/ujson" + "github.com/hashicorp/terraform-provider-aws/internal/ujson" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) diff --git a/internal/service/fms/ujson/LICENSE b/internal/ujson/LICENSE similarity index 100% rename from internal/service/fms/ujson/LICENSE rename to internal/ujson/LICENSE diff --git a/internal/service/fms/ujson/quote.go b/internal/ujson/quote.go similarity index 100% rename from internal/service/fms/ujson/quote.go rename to internal/ujson/quote.go diff --git a/internal/service/fms/ujson/quote_test.go b/internal/ujson/quote_test.go similarity index 98% rename from internal/service/fms/ujson/quote_test.go rename to internal/ujson/quote_test.go index e0ec6064b45..9c7c47748e5 100644 --- a/internal/service/fms/ujson/quote_test.go +++ b/internal/ujson/quote_test.go @@ -7,7 +7,7 @@ import ( "errors" "testing" - "github.com/hashicorp/terraform-provider-aws/internal/service/fms/ujson" + "github.com/hashicorp/terraform-provider-aws/internal/ujson" ) type quoteTest struct { diff --git "a/internal/service/fms/ujson/\302\265json.go" "b/internal/ujson/\302\265json.go" similarity index 100% rename from "internal/service/fms/ujson/\302\265json.go" rename to "internal/ujson/\302\265json.go" diff --git "a/internal/service/fms/ujson/\302\265json_test.go" "b/internal/ujson/\302\265json_test.go" similarity index 100% rename from "internal/service/fms/ujson/\302\265json_test.go" rename to "internal/ujson/\302\265json_test.go" From 5b46570db2efa772724d0a736ab1be89b456884a Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 15 Nov 2023 15:47:31 -0500 Subject: [PATCH 05/14] Add 'acctest.CheckResourceAttrContains'. --- internal/acctest/acctest.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/acctest/acctest.go b/internal/acctest/acctest.go index d53a3dd3cd5..0a0cf6437f1 100644 --- a/internal/acctest/acctest.go +++ b/internal/acctest/acctest.go @@ -744,6 +744,16 @@ func CheckResourceAttrJMESPair(nameFirst, keyFirst, jmesPath, nameSecond, keySec } } +// CheckResourceAttrContains ensures the Terraform state value contains the specified substr. +func CheckResourceAttrContains(name, key, substr string) resource.TestCheckFunc { + return resource.TestCheckResourceAttrWith(name, key, func(value string) error { + if strings.Contains(value, substr) { + return nil + } + return fmt.Errorf("%s: Attribute '%s' expected contains %#v, got %#v", name, key, substr, value) + }) +} + // CheckResourceAttrHasPrefix ensures the Terraform state value has the specified prefix. func CheckResourceAttrHasPrefix(name, key, prefix string) resource.TestCheckFunc { return resource.TestCheckResourceAttrWith(name, key, func(value string) error { From 7698db5a7c963dabe5a9bbe07062dab033ee45fc Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 15 Nov 2023 15:49:57 -0500 Subject: [PATCH 06/14] r/aws_grafana_workspace: Suppress 'configuration' diffs for read-only settings. --- internal/service/grafana/exports_test.go | 9 +++ internal/service/grafana/grafana_test.go | 55 ++++++++++++++++ internal/service/grafana/workspace.go | 63 ++++++++++++++++-- internal/service/grafana/workspace_test.go | 75 ++++++++++------------ 4 files changed, 156 insertions(+), 46 deletions(-) create mode 100644 internal/service/grafana/exports_test.go create mode 100644 internal/service/grafana/grafana_test.go diff --git a/internal/service/grafana/exports_test.go b/internal/service/grafana/exports_test.go new file mode 100644 index 00000000000..a966649ab58 --- /dev/null +++ b/internal/service/grafana/exports_test.go @@ -0,0 +1,9 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package grafana + +// Exports for use in tests only. +var ( + RemoveWorkspaceConfigurationReadOnlyFieldsFromJSON = removeWorkspaceConfigurationReadOnlyFieldsFromJSON +) diff --git a/internal/service/grafana/grafana_test.go b/internal/service/grafana/grafana_test.go new file mode 100644 index 00000000000..012910479f7 --- /dev/null +++ b/internal/service/grafana/grafana_test.go @@ -0,0 +1,55 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package grafana_test + +import ( + "testing" + + "github.com/hashicorp/terraform-provider-aws/internal/acctest" +) + +func TestAccGrafana_serial(t *testing.T) { + t.Parallel() + + testCases := map[string]map[string]func(t *testing.T){ + "Workspace": { + "saml": testAccWorkspace_saml, + "sso": testAccWorkspace_sso, + "disappears": testAccWorkspace_disappears, + "organization": testAccWorkspace_organization, + "dataSources": testAccWorkspace_dataSources, + "permissionType": testAccWorkspace_permissionType, + "notificationDestinations": testAccWorkspace_notificationDestinations, + "tags": testAccWorkspace_tags, + "vpc": testAccWorkspace_vpc, + "configuration": testAccWorkspace_configuration, + "networkAccess": testAccWorkspace_networkAccess, + "version": testAccWorkspace_version, + }, + "ApiKey": { + "basic": testAccWorkspaceAPIKey_basic, + }, + "DataSource": { + "basic": testAccWorkspaceDataSource_basic, + }, + "LicenseAssociation": { + "enterpriseFreeTrial": testAccLicenseAssociation_freeTrial, + }, + "SamlConfiguration": { + "basic": testAccWorkspaceSAMLConfiguration_basic, + "loginValidity": testAccWorkspaceSAMLConfiguration_loginValidity, + "assertions": testAccWorkspaceSAMLConfiguration_assertions, + }, + "RoleAssociation": { + "usersAdmin": testAccRoleAssociation_usersAdmin, + "usersEditor": testAccRoleAssociation_usersEditor, + "groupsAdmin": testAccRoleAssociation_groupsAdmin, + "groupsEditor": testAccRoleAssociation_groupsEditor, + "usersAndGroupsAdmin": testAccRoleAssociation_usersAndGroupsAdmin, + "usersAndGroupsEditor": testAccRoleAssociation_usersAndGroupsEditor, + }, + } + + acctest.RunSerialTests2Levels(t, testCases, 0) +} diff --git a/internal/service/grafana/workspace.go b/internal/service/grafana/workspace.go index 2d001daf864..61810ab8deb 100644 --- a/internal/service/grafana/workspace.go +++ b/internal/service/grafana/workspace.go @@ -4,7 +4,9 @@ package grafana import ( + "bytes" "context" + "encoding/json" "fmt" "log" "time" @@ -23,6 +25,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/flex" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" + "github.com/hashicorp/terraform-provider-aws/internal/ujson" "github.com/hashicorp/terraform-provider-aws/internal/verify" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -65,11 +68,12 @@ func ResourceWorkspace() *schema.Resource { }, }, "configuration": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validation.StringIsJSON, - DiffSuppressFunc: verify.SuppressEquivalentJSONDiffs, + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateFunc: validation.StringIsJSON, + DiffSuppressFunc: suppressEquivalentWorkspaceConfiguration, + DiffSuppressOnRefresh: true, StateFunc: func(v interface{}) string { json, _ := structure.NormalizeJsonString(v) return json @@ -524,3 +528,52 @@ func flattenNetworkAccessControl(rs *managedgrafana.NetworkAccessConfiguration) return []interface{}{m} } + +// suppressEquivalentWorkspaceConfiguration provides custom difference suppression +// for strings that are equivalent once read-only fields have been removed. +func suppressEquivalentWorkspaceConfiguration(k, old, new string, d *schema.ResourceData) bool { + if !json.Valid([]byte(old)) || !json.Valid([]byte(new)) { + return old == new + } + + old, new = removeWorkspaceConfigurationReadOnlyFieldsFromJSON(old), removeWorkspaceConfigurationReadOnlyFieldsFromJSON(new) + + return verify.JSONStringsEqual(old, new) +} + +// removeWorkspaceConfigurationReadOnlyFieldsFromJSON removes read-only (can't be specified in configuration) fields from a valid JSON string. +func removeWorkspaceConfigurationReadOnlyFieldsFromJSON(in string) string { + roFields := [][]byte{ + []byte(`"plugins"`), + } + out := make([]byte, 0, len(in)) + + err := ujson.Walk([]byte(in), func(_ int, key, value []byte) bool { + if len(key) != 0 { + for _, roField := range roFields { + if bytes.Equal(key, roField) { + // Remove the key and value from the output. + return false + } + } + } + + // Write to output. + if len(out) != 0 && ujson.ShouldAddComma(value, out[len(out)-1]) { + out = append(out, ',') + } + if len(key) > 0 { + out = append(out, key...) + out = append(out, ':') + } + out = append(out, value...) + + return true + }) + + if err != nil { + return "" + } + + return string(out) +} diff --git a/internal/service/grafana/workspace_test.go b/internal/service/grafana/workspace_test.go index e987fdc6cae..8713288b9e7 100644 --- a/internal/service/grafana/workspace_test.go +++ b/internal/service/grafana/workspace_test.go @@ -21,49 +21,41 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -func TestAccGrafana_serial(t *testing.T) { +func TestRemoveWorkspaceConfigurationReadOnlyFieldsFromJSON(t *testing.T) { t.Parallel() - testCases := map[string]map[string]func(t *testing.T){ - "Workspace": { - "saml": testAccWorkspace_saml, - "sso": testAccWorkspace_sso, - "disappears": testAccWorkspace_disappears, - "organization": testAccWorkspace_organization, - "dataSources": testAccWorkspace_dataSources, - "permissionType": testAccWorkspace_permissionType, - "notificationDestinations": testAccWorkspace_notificationDestinations, - "tags": testAccWorkspace_tags, - "vpc": testAccWorkspace_vpc, - "configuration": testAccWorkspace_configuration, - "networkAccess": testAccWorkspace_networkAccess, - "version": testAccWorkspace_version, + testCases := []struct { + testName string + input string + want string + }{ + { + testName: "empty JSON", + input: "{}", + want: "{}", }, - "ApiKey": { - "basic": testAccWorkspaceAPIKey_basic, + { + testName: "single field", + input: `{ "key": 42 }`, + want: `{"key":42}`, }, - "DataSource": { - "basic": testAccWorkspaceDataSource_basic, - }, - "LicenseAssociation": { - "enterpriseFreeTrial": testAccLicenseAssociation_freeTrial, - }, - "SamlConfiguration": { - "basic": testAccWorkspaceSAMLConfiguration_basic, - "loginValidity": testAccWorkspaceSAMLConfiguration_loginValidity, - "assertions": testAccWorkspaceSAMLConfiguration_assertions, - }, - "RoleAssociation": { - "usersAdmin": testAccRoleAssociation_usersAdmin, - "usersEditor": testAccRoleAssociation_usersEditor, - "groupsAdmin": testAccRoleAssociation_groupsAdmin, - "groupsEditor": testAccRoleAssociation_groupsEditor, - "usersAndGroupsAdmin": testAccRoleAssociation_usersAndGroupsAdmin, - "usersAndGroupsEditor": testAccRoleAssociation_usersAndGroupsEditor, + { + testName: "with read-only field", + input: "{\"unifiedAlerting\": {\"enabled\": true}, \"plugins\": {\"pluginAdminEnabled\" :false}}", + want: "{\"unifiedAlerting\":{\"enabled\":true}}", }, } - acctest.RunSerialTests2Levels(t, testCases, 0) + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.testName, func(t *testing.T) { + t.Parallel() + + if got, want := tfgrafana.RemoveWorkspaceConfigurationReadOnlyFieldsFromJSON(testCase.input), testCase.want; got != want { + t.Errorf("RemoveEmptyFieldsFromJSON(%q) = %q, want %q", testCase.input, got, want) + } + }) + } } func testAccWorkspace_saml(t *testing.T) { @@ -453,19 +445,20 @@ func testAccWorkspace_configuration(t *testing.T) { Config: testAccWorkspaceConfig_configuration(rName, `{"unifiedAlerting": { "enabled": true }}`), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckWorkspaceExists(ctx, resourceName, &v), - resource.TestCheckResourceAttr(resourceName, "configuration", `{"unifiedAlerting":{"enabled":true}}`), + acctest.CheckResourceAttrContains(resourceName, "configuration", `"unifiedAlerting":{"enabled":true}`), ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configuration"}, }, { Config: testAccWorkspaceConfig_configuration(rName, `{"unifiedAlerting": { "enabled": false }}`), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckWorkspaceExists(ctx, resourceName, &v), - resource.TestCheckResourceAttr(resourceName, "configuration", `{"unifiedAlerting":{"enabled":false}}`), + acctest.CheckResourceAttrContains(resourceName, "configuration", `"unifiedAlerting":{"enabled":false}`), ), }, }, From e44a213e4dac8dbed2c03ac9b58e60912d1e4011 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 15 Nov 2023 16:37:57 -0500 Subject: [PATCH 07/14] Acceptance test output: % make testacc TESTARGS='-run=TestAccGrafana_serial/Workspace/configuration' PKG=grafana ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/grafana/... -v -count 1 -parallel 20 -run=TestAccGrafana_serial/Workspace/configuration -timeout 360m === RUN TestAccGrafana_serial === PAUSE TestAccGrafana_serial === CONT TestAccGrafana_serial === RUN TestAccGrafana_serial/Workspace === RUN TestAccGrafana_serial/Workspace/configuration --- PASS: TestAccGrafana_serial (621.17s) --- PASS: TestAccGrafana_serial/Workspace (621.17s) --- PASS: TestAccGrafana_serial/Workspace/configuration (621.17s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/grafana 631.729s From d5dbad14620f1a777ff9b40d6a806397ee40f2d8 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 15 Nov 2023 16:39:57 -0500 Subject: [PATCH 08/14] Add CHANGELOG entry. --- .changelog/34105.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.changelog/34105.txt b/.changelog/34105.txt index 0ca7447e843..bb5e3dd04d9 100644 --- a/.changelog/34105.txt +++ b/.changelog/34105.txt @@ -1,3 +1,7 @@ ```release-note:bug resource/aws_grafana_workspace_api_key: Change `key` to [`Sensitive`](https://developer.hashicorp.com/terraform/plugin/best-practices/sensitive-state#using-sensitive-flag-functionality) ``` + +```release-note:bug +resource/aws_grafana_workspace: Prevent erroneous diffs on `configuration` +``` \ No newline at end of file From 133951bf4fec503cb17031fd8833ecccf01cc930 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 15 Nov 2023 17:24:48 -0500 Subject: [PATCH 09/14] Run 'go generate ./internal/generate/teamcity'. --- .teamcity/scripts/provider_tests/acceptance_tests.sh | 1 + .teamcity/scripts/provider_tests/unit_tests.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/.teamcity/scripts/provider_tests/acceptance_tests.sh b/.teamcity/scripts/provider_tests/acceptance_tests.sh index 2a7a03f2444..d4dd04d9dc3 100644 --- a/.teamcity/scripts/provider_tests/acceptance_tests.sh +++ b/.teamcity/scripts/provider_tests/acceptance_tests.sh @@ -53,6 +53,7 @@ TF_ACC=1 go test \ ./internal/tags/... \ ./internal/tfresource/... \ ./internal/types/... \ + ./internal/ujson/... \ ./internal/vault/... \ ./internal/verify/... \ -json -v -count=1 -parallel "%ACCTEST_PARALLELISM%" -timeout=0 -run=TestAcc diff --git a/.teamcity/scripts/provider_tests/unit_tests.sh b/.teamcity/scripts/provider_tests/unit_tests.sh index 3a496319630..c31c865cd48 100644 --- a/.teamcity/scripts/provider_tests/unit_tests.sh +++ b/.teamcity/scripts/provider_tests/unit_tests.sh @@ -25,6 +25,7 @@ go test \ ./internal/tags/... \ ./internal/tfresource/... \ ./internal/types/... \ + ./internal/ujson/... \ ./internal/vault/... \ ./internal/verify/... \ -json From e45df1d749064459d4699f236ad2ad618f44b773 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 16 Nov 2023 05:44:46 -0500 Subject: [PATCH 10/14] Acceptance test output: % make testacc TESTARGS='-run=TestAccGrafana_serial/Workspace/' PKG=grafana ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/grafana/... -v -count 1 -parallel 20 -run=TestAccGrafana_serial/Workspace/ -timeout 360m === RUN TestAccGrafana_serial === PAUSE TestAccGrafana_serial === CONT TestAccGrafana_serial === RUN TestAccGrafana_serial/Workspace === RUN TestAccGrafana_serial/Workspace/tags === RUN TestAccGrafana_serial/Workspace/saml === RUN TestAccGrafana_serial/Workspace/organization acctest.go:1009: this AWS account must be the management account of an AWS Organization === RUN TestAccGrafana_serial/Workspace/dataSources === RUN TestAccGrafana_serial/Workspace/permissionType === RUN TestAccGrafana_serial/Workspace/notificationDestinations === RUN TestAccGrafana_serial/Workspace/version === RUN TestAccGrafana_serial/Workspace/sso acctest.go:1051: skipping tests; no SSO Instances found. === RUN TestAccGrafana_serial/Workspace/disappears === RUN TestAccGrafana_serial/Workspace/vpc workspace_test.go:114: Step 1/4 error: Error running apply: exit status 1 Error: creating EC2 VPC: operation error EC2: CreateVpc, https response error StatusCode: 400, RequestID: ebca4a7c-ac66-436c-b2aa-1975e1b66e08, api error VpcLimitExceeded: The maximum number of VPCs has been reached. with aws_vpc.test, on terraform_plugin_test.tf line 30, in resource "aws_vpc" "test": 30: resource "aws_vpc" "test" { === RUN TestAccGrafana_serial/Workspace/configuration === RUN TestAccGrafana_serial/Workspace/networkAccess --- FAIL: TestAccGrafana_serial (4103.41s) --- FAIL: TestAccGrafana_serial/Workspace (4103.41s) --- PASS: TestAccGrafana_serial/Workspace/tags (358.91s) --- PASS: TestAccGrafana_serial/Workspace/saml (320.42s) --- SKIP: TestAccGrafana_serial/Workspace/organization (0.70s) --- PASS: TestAccGrafana_serial/Workspace/dataSources (321.41s) --- PASS: TestAccGrafana_serial/Workspace/permissionType (348.85s) --- PASS: TestAccGrafana_serial/Workspace/notificationDestinations (349.90s) --- PASS: TestAccGrafana_serial/Workspace/version (601.81s) --- SKIP: TestAccGrafana_serial/Workspace/sso (0.45s) --- PASS: TestAccGrafana_serial/Workspace/disappears (318.63s) --- FAIL: TestAccGrafana_serial/Workspace/vpc (11.98s) --- PASS: TestAccGrafana_serial/Workspace/configuration (666.38s) --- PASS: TestAccGrafana_serial/Workspace/networkAccess (803.97s) FAIL FAIL github.com/hashicorp/terraform-provider-aws/internal/service/grafana 4109.237s FAIL make: *** [testacc] Error 1 From d5943cfa607bab875ecc0192f7eaf03dea8e6c83 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 16 Nov 2023 10:25:34 -0500 Subject: [PATCH 11/14] Revert "Add CHANGELOG entry." This reverts commit d5dbad14620f1a777ff9b40d6a806397ee40f2d8. --- .changelog/34105.txt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.changelog/34105.txt b/.changelog/34105.txt index bb5e3dd04d9..0ca7447e843 100644 --- a/.changelog/34105.txt +++ b/.changelog/34105.txt @@ -1,7 +1,3 @@ ```release-note:bug resource/aws_grafana_workspace_api_key: Change `key` to [`Sensitive`](https://developer.hashicorp.com/terraform/plugin/best-practices/sensitive-state#using-sensitive-flag-functionality) ``` - -```release-note:bug -resource/aws_grafana_workspace: Prevent erroneous diffs on `configuration` -``` \ No newline at end of file From f8078059500fd666cc3818f1e9255aa8c556e4cd Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 16 Nov 2023 10:29:36 -0500 Subject: [PATCH 12/14] r/aws_grafana_workspace:: Revert removal of 'plugins' from 'configuration'. --- internal/service/grafana/exports_test.go | 9 ---- internal/service/grafana/workspace.go | 54 +--------------------- internal/service/grafana/workspace_test.go | 41 +--------------- 3 files changed, 3 insertions(+), 101 deletions(-) delete mode 100644 internal/service/grafana/exports_test.go diff --git a/internal/service/grafana/exports_test.go b/internal/service/grafana/exports_test.go deleted file mode 100644 index a966649ab58..00000000000 --- a/internal/service/grafana/exports_test.go +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package grafana - -// Exports for use in tests only. -var ( - RemoveWorkspaceConfigurationReadOnlyFieldsFromJSON = removeWorkspaceConfigurationReadOnlyFieldsFromJSON -) diff --git a/internal/service/grafana/workspace.go b/internal/service/grafana/workspace.go index 61810ab8deb..6391e3006ec 100644 --- a/internal/service/grafana/workspace.go +++ b/internal/service/grafana/workspace.go @@ -4,9 +4,7 @@ package grafana import ( - "bytes" "context" - "encoding/json" "fmt" "log" "time" @@ -25,7 +23,6 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/flex" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" - "github.com/hashicorp/terraform-provider-aws/internal/ujson" "github.com/hashicorp/terraform-provider-aws/internal/verify" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -72,7 +69,7 @@ func ResourceWorkspace() *schema.Resource { Optional: true, Computed: true, ValidateFunc: validation.StringIsJSON, - DiffSuppressFunc: suppressEquivalentWorkspaceConfiguration, + DiffSuppressFunc: verify.SuppressEquivalentJSONDiffs, DiffSuppressOnRefresh: true, StateFunc: func(v interface{}) string { json, _ := structure.NormalizeJsonString(v) @@ -528,52 +525,3 @@ func flattenNetworkAccessControl(rs *managedgrafana.NetworkAccessConfiguration) return []interface{}{m} } - -// suppressEquivalentWorkspaceConfiguration provides custom difference suppression -// for strings that are equivalent once read-only fields have been removed. -func suppressEquivalentWorkspaceConfiguration(k, old, new string, d *schema.ResourceData) bool { - if !json.Valid([]byte(old)) || !json.Valid([]byte(new)) { - return old == new - } - - old, new = removeWorkspaceConfigurationReadOnlyFieldsFromJSON(old), removeWorkspaceConfigurationReadOnlyFieldsFromJSON(new) - - return verify.JSONStringsEqual(old, new) -} - -// removeWorkspaceConfigurationReadOnlyFieldsFromJSON removes read-only (can't be specified in configuration) fields from a valid JSON string. -func removeWorkspaceConfigurationReadOnlyFieldsFromJSON(in string) string { - roFields := [][]byte{ - []byte(`"plugins"`), - } - out := make([]byte, 0, len(in)) - - err := ujson.Walk([]byte(in), func(_ int, key, value []byte) bool { - if len(key) != 0 { - for _, roField := range roFields { - if bytes.Equal(key, roField) { - // Remove the key and value from the output. - return false - } - } - } - - // Write to output. - if len(out) != 0 && ujson.ShouldAddComma(value, out[len(out)-1]) { - out = append(out, ',') - } - if len(key) > 0 { - out = append(out, key...) - out = append(out, ':') - } - out = append(out, value...) - - return true - }) - - if err != nil { - return "" - } - - return string(out) -} diff --git a/internal/service/grafana/workspace_test.go b/internal/service/grafana/workspace_test.go index 8713288b9e7..3ef01f7af31 100644 --- a/internal/service/grafana/workspace_test.go +++ b/internal/service/grafana/workspace_test.go @@ -21,43 +21,6 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -func TestRemoveWorkspaceConfigurationReadOnlyFieldsFromJSON(t *testing.T) { - t.Parallel() - - testCases := []struct { - testName string - input string - want string - }{ - { - testName: "empty JSON", - input: "{}", - want: "{}", - }, - { - testName: "single field", - input: `{ "key": 42 }`, - want: `{"key":42}`, - }, - { - testName: "with read-only field", - input: "{\"unifiedAlerting\": {\"enabled\": true}, \"plugins\": {\"pluginAdminEnabled\" :false}}", - want: "{\"unifiedAlerting\":{\"enabled\":true}}", - }, - } - - for _, testCase := range testCases { - testCase := testCase - t.Run(testCase.testName, func(t *testing.T) { - t.Parallel() - - if got, want := tfgrafana.RemoveWorkspaceConfigurationReadOnlyFieldsFromJSON(testCase.input), testCase.want; got != want { - t.Errorf("RemoveEmptyFieldsFromJSON(%q) = %q, want %q", testCase.input, got, want) - } - }) - } -} - func testAccWorkspace_saml(t *testing.T) { ctx := acctest.Context(t) var v managedgrafana.WorkspaceDescription @@ -445,7 +408,7 @@ func testAccWorkspace_configuration(t *testing.T) { Config: testAccWorkspaceConfig_configuration(rName, `{"unifiedAlerting": { "enabled": true }}`), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckWorkspaceExists(ctx, resourceName, &v), - acctest.CheckResourceAttrContains(resourceName, "configuration", `"unifiedAlerting":{"enabled":true}`), + resource.TestCheckResourceAttr(resourceName, "configuration", `{"unifiedAlerting":{"enabled":true}}`), ), }, { @@ -458,7 +421,7 @@ func testAccWorkspace_configuration(t *testing.T) { Config: testAccWorkspaceConfig_configuration(rName, `{"unifiedAlerting": { "enabled": false }}`), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckWorkspaceExists(ctx, resourceName, &v), - acctest.CheckResourceAttrContains(resourceName, "configuration", `"unifiedAlerting":{"enabled":false}`), + resource.TestCheckResourceAttr(resourceName, "configuration", `{"unifiedAlerting":{"enabled":false}}`), ), }, }, From 60be560ec5889519203a7f654a7ec1d87426fa7f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 16 Nov 2023 08:43:06 -0500 Subject: [PATCH 13/14] Fix 'testAccWorkspace_configuration'. --- internal/service/grafana/workspace_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/service/grafana/workspace_test.go b/internal/service/grafana/workspace_test.go index 3ef01f7af31..b7757343b46 100644 --- a/internal/service/grafana/workspace_test.go +++ b/internal/service/grafana/workspace_test.go @@ -405,10 +405,10 @@ func testAccWorkspace_configuration(t *testing.T) { ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, Steps: []resource.TestStep{ { - Config: testAccWorkspaceConfig_configuration(rName, `{"unifiedAlerting": { "enabled": true }}`), + Config: testAccWorkspaceConfig_configuration(rName, `{"unifiedAlerting": { "enabled": true }, "plugins": {"pluginAdminEnabled": false}}`), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckWorkspaceExists(ctx, resourceName, &v), - resource.TestCheckResourceAttr(resourceName, "configuration", `{"unifiedAlerting":{"enabled":true}}`), + resource.TestCheckResourceAttr(resourceName, "configuration", `{"unifiedAlerting":{"enabled":true},"plugins":{"pluginAdminEnabled":false}}`), ), }, { @@ -418,10 +418,10 @@ func testAccWorkspace_configuration(t *testing.T) { ImportStateVerifyIgnore: []string{"configuration"}, }, { - Config: testAccWorkspaceConfig_configuration(rName, `{"unifiedAlerting": { "enabled": false }}`), + Config: testAccWorkspaceConfig_configuration(rName, `{"unifiedAlerting": { "enabled": false }, "plugins": {"pluginAdminEnabled": true}}`), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckWorkspaceExists(ctx, resourceName, &v), - resource.TestCheckResourceAttr(resourceName, "configuration", `{"unifiedAlerting":{"enabled":false}}`), + resource.TestCheckResourceAttr(resourceName, "configuration", `{"unifiedAlerting":{"enabled":false},"plugins":{"pluginAdminEnabled":true}}`), ), }, }, From be2a8be0f34e21506cf0e3ea0d97815d280fbb80 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 16 Nov 2023 10:32:38 -0500 Subject: [PATCH 14/14] testAccWorkspace_configuration: No need for 'ImportStateVerifyIgnore'. --- internal/service/grafana/workspace_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/service/grafana/workspace_test.go b/internal/service/grafana/workspace_test.go index b7757343b46..28b51c50ac2 100644 --- a/internal/service/grafana/workspace_test.go +++ b/internal/service/grafana/workspace_test.go @@ -412,10 +412,9 @@ func testAccWorkspace_configuration(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"configuration"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, }, { Config: testAccWorkspaceConfig_configuration(rName, `{"unifiedAlerting": { "enabled": false }, "plugins": {"pluginAdminEnabled": true}}`),