Repository: GoogleCloudPlatform/terraform-validator
Pull request #1112 (2022-11-11 19:23:45)

tfv: Fix GetOk field name in hand written iam files
<!-- Put a description of what this PR is for here, along with any references to issues that this resolves or contributes to -->

b/255831148
terraform validator: GetOk() function should not have {{}} within the parameter. 


<!--
Replace each [ ] with [X] to check it. Switch to the preview view to make it easier to click on links.
These steps will speed up the review process, and we appreciate you spending time on them before sending
your code to be reviewed.
-->
If this PR is for Terraform, I acknowledge that I have:

- [X] Searched through the [issue tracker](https://github.com/hashicorp/terraform-provider-google/issues) for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
- [ ] [Generated Terraform](https://github.com/GoogleCloudPlatform/magic-modules#generating-the-terraform-providers), and ran `make test` and `make lint` to ensure it passes unit and linter tests.
- [ ] Ensured that all new fields I added that can be set by a user appear in at least one [example](https://github.com/GoogleCloudPlatform/magic-modules/tree/main/mmv1/templates/terraform/examples) (for generated resources) or [third_party test](https://github.com/GoogleCloudPlatform/magic-modules/tree/main/mmv1/third_party/terraform/tests) (for handwritten resources or update tests).
- [ ] [Ran](https://github.com/hashicorp/terraform-provider-google/blob/main/.github/CONTRIBUTING.md#tests) relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
- [X] Read the [Release Notes Guide](https://github.com/GoogleCloudPlatform/magic-modules/blob/main/.ci/RELEASE_NOTES_GUIDE.md) before writing my release note below.

<!-- AUTOCHANGELOG for Downstream PRs.

Please select one of the following "release-note:" headings:
    - release-note:enhancement
    - release-note:bug
    - release-note:note
    - release-note:new-resource
    - release-note:new-datasource
    - release-note:deprecation
    - release-note:breaking-change
    - release-note:none
    
Unless you choose release-note:none, please add a release note.

See .ci/RELEASE_NOTES_GUIDE.md for writing good release notes.

You can add more release note blocks if you want more than one CHANGELOG
entry for this PR.
-->
**Release Note Template for Downstream PRs (will be copied)**

```release-note:none

```

Derived from https://github.com/GoogleCloudPlatform/magic-modules/pull/6745


Patch:
diff --git a/converters/google/resources/bigquery_dataset_iam.go b/converters/google/resources/bigquery_dataset_iam.go
index 0de2b9ca..3e431c79 100644
--- a/converters/google/resources/bigquery_dataset_iam.go
+++ b/converters/google/resources/bigquery_dataset_iam.go
@@ -89,7 +89,7 @@ func newBigqueryDatasetIamAsset(
 
 func FetchBigqueryDatasetIamPolicy(d TerraformResourceData, config *Config) (Asset, error) {
 	// Check if the identity field returns a value
-	if _, ok := d.GetOk("{{dataset_id}}"); !ok {
+	if _, ok := d.GetOk("dataset_id"); !ok {
 		return Asset{}, ErrEmptyIdentityField
 	}
 
diff --git a/converters/google/resources/kms_crypto_key_iam.go b/converters/google/resources/kms_crypto_key_iam.go
index b7ed8ab4..fd588935 100644
--- a/converters/google/resources/kms_crypto_key_iam.go
+++ b/converters/google/resources/kms_crypto_key_iam.go
@@ -93,7 +93,7 @@ func newKmsCryptoKeyIamAsset(
 
 func FetchKmsCryptoKeyIamPolicy(d TerraformResourceData, config *Config) (Asset, error) {
 	// Check if the identity field returns a value
-	if _, ok := d.GetOk("{{crypto_key_id}}"); !ok {
+	if _, ok := d.GetOk("crypto_key_id"); !ok {
 		return Asset{}, ErrEmptyIdentityField
 	}
 
diff --git a/converters/google/resources/pubsub_subscription_iam.go b/converters/google/resources/pubsub_subscription_iam.go
index 678e495f..280842fb 100644
--- a/converters/google/resources/pubsub_subscription_iam.go
+++ b/converters/google/resources/pubsub_subscription_iam.go
@@ -89,7 +89,7 @@ func newPubsubSubscriptionIamAsset(
 
 func FetchPubsubSubscriptionIamPolicy(d TerraformResourceData, config *Config) (Asset, error) {
 	// Check if the identity field returns a value
-	if _, ok := d.GetOk("{{subscription}}"); !ok {
+	if _, ok := d.GetOk("subscription"); !ok {
 		return Asset{}, ErrEmptyIdentityField
 	}
 
diff --git a/converters/google/resources/spanner_database_iam.go b/converters/google/resources/spanner_database_iam.go
index ba81b92b..90ad00bc 100644
--- a/converters/google/resources/spanner_database_iam.go
+++ b/converters/google/resources/spanner_database_iam.go
@@ -89,11 +89,11 @@ func newSpannerDatabaseIamAsset(
 
 func FetchSpannerDatabaseIamPolicy(d TerraformResourceData, config *Config) (Asset, error) {
 	// Check if the identity field returns a value
-	if _, ok := d.GetOk("{{instance}}"); !ok {
+	if _, ok := d.GetOk("instance"); !ok {
 		return Asset{}, ErrEmptyIdentityField
 	}
 
-	if _, ok := d.GetOk("{{database}}"); !ok {
+	if _, ok := d.GetOk("database"); !ok {
 		return Asset{}, ErrEmptyIdentityField
 	}
 
diff --git a/converters/google/resources/spanner_instance_iam.go b/converters/google/resources/spanner_instance_iam.go
index 9e2120fe..583cf614 100644
--- a/converters/google/resources/spanner_instance_iam.go
+++ b/converters/google/resources/spanner_instance_iam.go
@@ -89,7 +89,7 @@ func newSpannerInstanceIamAsset(
 
 func FetchSpannerInstanceIamPolicy(d TerraformResourceData, config *Config) (Asset, error) {
 	// Check if the identity field returns a value
-	if _, ok := d.GetOk("{{instance}}"); !ok {
+	if _, ok := d.GetOk("instance"); !ok {
 		return Asset{}, ErrEmptyIdentityField
 	}
 
diff --git a/converters/google/resources/storage_bucket_iam.go b/converters/google/resources/storage_bucket_iam.go
index d395560b..1c29e3fc 100644
--- a/converters/google/resources/storage_bucket_iam.go
+++ b/converters/google/resources/storage_bucket_iam.go
@@ -92,7 +92,7 @@ func newStorageBucketIamAsset(
 
 func FetchStorageBucketIamPolicy(d TerraformResourceData, config *Config) (Asset, error) {
 	// Check if the identity field returns a value
-	if _, ok := d.GetOk("{{bucket}}"); !ok {
+	if _, ok := d.GetOk("bucket"); !ok {
 		return Asset{}, ErrEmptyIdentityField
 	}
 


Test patch:
diff --git a/converters/google/resources/config_test_utils.go b/converters/google/resources/config_test_utils.go
new file mode 100644
index 00000000..b91052f4
--- /dev/null
+++ b/converters/google/resources/config_test_utils.go
@@ -0,0 +1,113 @@
+package google
+
+import (
+	"net/http/httptest"
+	"strings"
+)
+
+// NewTestConfig create a config using the http test server.
+func NewTestConfig(server *httptest.Server) *Config {
+	cfg := &Config{}
+	cfg.client = server.Client()
+	configureTestBasePaths(cfg, server.URL)
+	return cfg
+}
+
+func configureTestBasePaths(c *Config, url string) {
+	if !strings.HasSuffix(url, "/") {
+		url = url + "/"
+	}
+	// Generated Products
+	c.AccessApprovalBasePath = url
+	c.AccessContextManagerBasePath = url
+	c.ActiveDirectoryBasePath = url
+	c.ApigeeBasePath = url
+	c.AppEngineBasePath = url
+	c.ArtifactRegistryBasePath = url
+	c.BeyondcorpBasePath = url
+	c.BigQueryBasePath = url
+	c.BigqueryAnalyticsHubBasePath = url
+	c.BigqueryConnectionBasePath = url
+	c.BigqueryDataTransferBasePath = url
+	c.BigqueryReservationBasePath = url
+	c.BigtableBasePath = url
+	c.BillingBasePath = url
+	c.BinaryAuthorizationBasePath = url
+	c.CertificateManagerBasePath = url
+	c.CloudAssetBasePath = url
+	c.CloudBuildBasePath = url
+	c.CloudFunctionsBasePath = url
+	c.Cloudfunctions2BasePath = url
+	c.CloudIdentityBasePath = url
+	c.CloudIdsBasePath = url
+	c.CloudIotBasePath = url
+	c.CloudRunBasePath = url
+	c.CloudSchedulerBasePath = url
+	c.CloudTasksBasePath = url
+	c.ComputeBasePath = url
+	c.ContainerAnalysisBasePath = url
+	c.DataCatalogBasePath = url
+	c.DataFusionBasePath = url
+	c.DataLossPreventionBasePath = url
+	c.DataprocBasePath = url
+	c.DataprocMetastoreBasePath = url
+	c.DatastoreBasePath = url
+	c.DatastreamBasePath = url
+	c.DeploymentManagerBasePath = url
+	c.DialogflowBasePath = url
+	c.DialogflowCXBasePath = url
+	c.DNSBasePath = url
+	c.DocumentAIBasePath = url
+	c.EssentialContactsBasePath = url
+	c.FilestoreBasePath = url
+	c.FirestoreBasePath = url
+	c.GameServicesBasePath = url
+	c.GKEHubBasePath = url
+	c.HealthcareBasePath = url
+	c.IAMBetaBasePath = url
+	c.IapBasePath = url
+	c.IdentityPlatformBasePath = url
+	c.KMSBasePath = url
+	c.LoggingBasePath = url
+	c.MemcacheBasePath = url
+	c.MLEngineBasePath = url
+	c.MonitoringBasePath = url
+	c.NetworkManagementBasePath = url
+	c.NetworkServicesBasePath = url
+	c.NotebooksBasePath = url
+	c.OSConfigBasePath = url
+	c.OSLoginBasePath = url
+	c.PrivatecaBasePath = url
+	c.PubsubBasePath = url
+	c.PubsubLiteBasePath = url
+	c.RedisBasePath = url
+	c.ResourceManagerBasePath = url
+	c.SecretManagerBasePath = url
+	c.SecurityCenterBasePath = url
+	c.ServiceManagementBasePath = url
+	c.ServiceUsageBasePath = url
+	c.SourceRepoBasePath = url
+	c.SpannerBasePath = url
+	c.SQLBasePath = url
+	c.StorageBasePath = url
+	c.StorageTransferBasePath = url
+	c.TagsBasePath = url
+	c.TPUBasePath = url
+	c.VertexAIBasePath = url
+	c.VPCAccessBasePath = url
+	c.WorkflowsBasePath = url
+
+	// Handwritten Products / Versioned / Atypical Entries
+	c.CloudBillingBasePath = url
+	c.ComposerBasePath = url
+	c.ContainerBasePath = url
+	c.DataprocBasePath = url
+	c.DataflowBasePath = url
+	c.IamCredentialsBasePath = url
+	c.ResourceManagerV3BasePath = url
+	c.IAMBasePath = url
+	c.ServiceNetworkingBasePath = url
+	c.BigQueryBasePath = url
+	c.StorageTransferBasePath = url
+	c.BigtableAdminBasePath = url
+}
diff --git a/test/iam_test.go b/test/iam_test.go
new file mode 100644
index 00000000..59be04be
--- /dev/null
+++ b/test/iam_test.go
@@ -0,0 +1,126 @@
+package test
+
+import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	crmv1 "google.golang.org/api/cloudresourcemanager/v1"
+
+	resources "github.com/GoogleCloudPlatform/terraform-validator/converters/google/resources"
+	"github.com/GoogleCloudPlatform/terraform-validator/tfdata"
+	"github.com/GoogleCloudPlatform/terraform-validator/tfplan"
+	provider "github.com/hashicorp/terraform-provider-google/google"
+)
+
+func TestIAMFetchFullResource(t *testing.T) {
+	cases := []struct {
+		name string
+	}{
+		{name: "example_bigquery_dataset_iam_binding"},
+		{name: "example_bigquery_dataset_iam_member"},
+		{name: "example_cloud_run_service_iam_binding"},
+		{name: "example_cloud_run_service_iam_member"},
+		{name: "example_compute_instance_iam_binding"},
+		{name: "example_compute_instance_iam_member"},
+		{name: "example_folder_iam_binding"},
+		{name: "example_folder_iam_member"},
+		{name: "example_kms_crypto_key_iam_binding"},
+		{name: "example_kms_crypto_key_iam_member"},
+		{name: "example_kms_key_ring_iam_binding"},
+		{name: "example_kms_key_ring_iam_member"},
+		{name: "example_organization_iam_binding"},
+		{name: "example_organization_iam_member"},
+		{name: "example_project_iam_binding"},
+		{name: "example_project_iam_member"},
+		{name: "example_pubsub_subscription_iam_binding"},
+		{name: "example_pubsub_subscription_iam_member"},
+		{name: "example_secret_manager_secret_iam_binding"},
+		{name: "example_secret_manager_secret_iam_member"},
+		{name: "example_spanner_database_iam_binding"},
+		{name: "example_spanner_database_iam_member"},
+		{name: "example_spanner_instance_iam_binding"},
+		{name: "example_spanner_instance_iam_member"},
+		{name: "example_storage_bucket_iam_binding"},
+		{name: "example_storage_bucket_iam_member"},
+	}
+
+	converters := resources.ResourceConverters()
+	schema := provider.Provider()
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		var payload []byte
+		var err error
+		if strings.Contains(r.URL.String(), "dataset") {
+			obj := map[string][]interface{}{
+				"access": {&crmv1.Policy{}},
+			}
+			payload, err = json.Marshal(obj)
+		} else {
+			payload, err = (&crmv1.Policy{}).MarshalJSON()
+		}
+		if err != nil {
+			w.WriteHeader(http.StatusInternalServerError)
+			w.Write([]byte(fmt.Sprintf("failed to MarshalJSON: %s", err)))
+			return
+		}
+		w.Write(payload)
+	}))
+
+	// Using Cleanup instead of defer because t.Parallel() does not block t.Run.
+	t.Cleanup(func() {
+		server.Close()
+	})
+
+	cfg := resources.NewTestConfig(server)
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			t.Parallel()
+
+			// Create a temporary directory for generating tfplan.json from template.
+			dir, err := ioutil.TempDir(tmpDir, "terraform")
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer os.RemoveAll(dir)
+
+			generateTestFiles(t, "../testdata/templates", dir, c.name+".tfplan.json")
+			path := filepath.Join(dir, c.name+".tfplan.json")
+
+			data, err := ioutil.ReadFile(path)
+			if err != nil {
+				t.Fatalf("opening JSON plan file: %s", err)
+			}
+			changes, err := tfplan.ReadResourceChanges(data)
+			if err != nil {
+				t.Fatalf("ReadResourceChanges failed: %s", err)
+			}
+
+			for _, rc := range changes {
+				resource := schema.ResourcesMap[rc.Type]
+				rd := tfdata.NewFakeResourceData(
+					rc.Type,
+					resource.Schema,
+					rc.Change.After.(map[string]interface{}),
+				)
+				for _, converter := range converters[rd.Kind()] {
+					if converter.FetchFullResource == nil {
+						continue
+					}
+					_, err := converter.FetchFullResource(rd, cfg)
+					if err != nil {
+						t.Errorf("FetchFullResource() = %s, want = nil", err)
+					}
+				}
+			}
+
+		})
+	}
+}