fix(tgc): Fixing the if block for existingConverterAsset and add test…

… case (GoogleCloudPlatform#9914)
tdbhacks · Feb 6, 2024 · ca4b513 · ca4b513
1 parent e441d02
commit ca4b513
Show file tree

Hide file tree

Showing 2 changed files with 148 additions and 18 deletions.
diff --git a/mmv1/third_party/tgc/convert.go b/mmv1/third_party/tgc/convert.go
@@ -172,7 +172,6 @@ func (c *Converter) addDelete(rc *tfjson.ResourceChange) error {
 		}
 
 		for _, converted := range convertedItems {
-
 			key := converted.Type + converted.Name
 			var existingConverterAsset *resources.Asset
 			if existing, exists := c.assets[key]; exists {
@@ -190,14 +189,14 @@ func (c *Converter) addDelete(rc *tfjson.ResourceChange) error {
 				} else {
 					existingConverterAsset = &asset
 				}
-				if existingConverterAsset != nil {
-					converted = converter.MergeDelete(*existingConverterAsset, converted)
-					augmented, err := c.augmentAsset(rd, c.cfg, converted)
-					if err != nil {
-						return err
-					}
-					c.assets[key] = augmented
+			}
+			if existingConverterAsset != nil {
+				converted = converter.MergeDelete(*existingConverterAsset, converted)
+				augmented, err := c.augmentAsset(rd, c.cfg, converted)
+				if err != nil {
+					return err
 				}
+				c.assets[key] = augmented
 			}
 		}
 	}

diff --git a/mmv1/third_party/tgc/convert_test.go b/mmv1/third_party/tgc/convert_test.go
@@ -12,7 +12,10 @@ import (
 	"github.com/GoogleCloudPlatform/terraform-google-conversion/v5/caiasset"
 	"github.com/GoogleCloudPlatform/terraform-google-conversion/v5/tfplan2cai/ancestrymanager"
 	resources "github.com/GoogleCloudPlatform/terraform-google-conversion/v5/tfplan2cai/converters/google/resources"
+	"github.com/GoogleCloudPlatform/terraform-google-conversion/v5/tfplan2cai/converters/google/resources/cai"
+	"github.com/GoogleCloudPlatform/terraform-google-conversion/v5/tfplan2cai/converters/google/resources/services/resourcemanager"
 	"github.com/GoogleCloudPlatform/terraform-google-conversion/v5/tfplan2cai/tfdata"
+	"github.com/google/go-cmp/cmp"
 	tfjson "github.com/hashicorp/terraform-json"
 	provider "github.com/hashicorp/terraform-provider-google-beta/google-beta/provider"
 	"github.com/hashicorp/terraform-provider-google-beta/google-beta/tpgresource"
@@ -635,18 +638,146 @@ func TestConvertWrapper(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		_, err := convertWrapper(test.converter, test.rd, test.cfg)
-		if !test.wantErr {
+		t.Run(test.name, func(t *testing.T) {
+			_, err := convertWrapper(test.converter, test.rd, test.cfg)
+			if !test.wantErr {
+				if err != nil {
+					t.Errorf("convertWrapper() = %q, want = nil", err)
+				}
+			} else {
+				if err == nil {
+					t.Errorf("convertWrapper() = nil, want = %v", test.wantErrMsg)
+				} else if !strings.Contains(err.Error(), test.wantErrMsg) {
+					t.Errorf("convertWrapper() = %q, want containing %q", err, test.wantErrMsg)
+				}
+			}
+		})
+	}
+}
+
+func TestConvertMergingWithExistAsset(t *testing.T) {
+	tests := []struct {
+		name    string
+		changes []*tfjson.ResourceChange
+		want    []caiasset.Asset
+	}{
+		{
+			name: "CreateOrUpdateOrNoop",
+			changes: []*tfjson.ResourceChange{
+				{
+					Address:      "google_project_iam_binding.test",
+					Mode:         "managed",
+					Type:         "google_project_iam_binding",
+					Name:         "test",
+					ProviderName: "registry.terraform.io/hashicorp/google-beta",
+					Change: &tfjson.Change{
+						Actions: tfjson.Actions{"create"},
+						Before:  nil,
+						After: map[string]interface{}{
+							"members": []interface{}{
+								"user:[email protected]",
+							},
+							"project": "example-project",
+							"role":    "roles/editor",
+						},
+					},
+				},
+			},
+			want: []caiasset.Asset{
+				{
+					Name: "//cloudresourcemanager.googleapis.com/projects/example-project",
+					Type: "cloudresourcemanager.googleapis.com/Project",
+					IAMPolicy: &caiasset.IAMPolicy{
+						Bindings: []caiasset.IAMBinding{
+							{
+								Role:    "roles/editor",
+								Members: []string{"user:[email protected]"},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "Delete",
+			changes: []*tfjson.ResourceChange{
+				{
+					Address:      "google_project_iam_binding.test",
+					Mode:         "managed",
+					Type:         "google_project_iam_binding",
+					Name:         "test",
+					ProviderName: "registry.terraform.io/hashicorp/google-beta",
+					Change: &tfjson.Change{
+						Actions: tfjson.Actions{"delete"},
+						After:   nil,
+						Before: map[string]interface{}{
+							"members": []interface{}{
+								"user:[email protected]",
+							},
+							"project": "example-project",
+							"role":    "roles/editor",
+						},
+					},
+				},
+			},
+			want: []caiasset.Asset{
+				{
+					Name:      "//cloudresourcemanager.googleapis.com/projects/example-project",
+					Type:      "cloudresourcemanager.googleapis.com/Project",
+					IAMPolicy: &caiasset.IAMPolicy{},
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			converter, _, err := newTestConverter(false)
+			assert.Nil(t, err)
+
+			fetchFuncCalled := 0
+			fetchFunc := func(d tpgresource.TerraformResourceData, config *transport_tpg.Config) (cai.Asset, error) {
+				fetchFuncCalled++
+				return resourcemanager.FetchProjectIamPolicy(d, config)
+			}
+
+			converter.converters["google_project_iam_binding"] = []cai.ResourceConverter{
+				{
+					AssetType:         "cloudresourcemanager.googleapis.com/Project",
+					Convert:           resourcemanager.GetProjectIamBindingCaiObject,
+					FetchFullResource: fetchFunc,
+					MergeCreateUpdate: resourcemanager.MergeProjectIamBinding,
+					MergeDelete:       resourcemanager.MergeProjectIamBindingDelete,
+				},
+			}
+
+			// pre-populate the converter's asset cache
+			asset, err := converter.augmentAsset(nil, converter.cfg, cai.Asset{
+				Name: "//cloudresourcemanager.googleapis.com/projects/example-project",
+				Type: "cloudresourcemanager.googleapis.com/Project",
+				IAMPolicy: &cai.IAMPolicy{
+					Bindings: []cai.IAMBinding{
+						{
+							Role:    "roles/editor",
+							Members: []string{"user:[email protected]"},
+						},
+					},
+				},
+			})
 			if err != nil {
-				t.Errorf("convertWrapper() = %q, want = nil", err)
+				t.Fatal(err)
 			}
-		} else {
-			if err == nil {
-				t.Errorf("convertWrapper() = nil, want = %v", test.wantErrMsg)
-			} else if !strings.Contains(err.Error(), test.wantErrMsg) {
-				t.Errorf("convertWrapper() = %q, want containing %q", err, test.wantErrMsg)
+			converter.assets = map[string]Asset{
+				"cloudresourcemanager.googleapis.com/Project//cloudresourcemanager.googleapis.com/projects/example-project": asset,
 			}
-		}
-	}
 
+			err = converter.AddResourceChanges(test.changes)
+			if err != nil {
+				t.Fatalf("AddResourceChanges() = %s, want = nil", err)
+			}
+			if diff := cmp.Diff(test.want, converter.Assets()); diff != "" {
+				t.Errorf("AddResourceChanges() returned unexpected diff (-want +got):\n%s", diff)
+			}
+			assert.Equal(t, 0, fetchFuncCalled)
+		})
+	}
 }