Fix identity type comparison for service perimeters (GoogleCloudPlatf…

…orm#12267)
amanMahendroo · Dec 17, 2024 · bed7acc · bed7acc
1 parent 47bf263
commit bed7acc
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 1 deletion.
diff --git a/mmv1/products/accesscontextmanager/ServicePerimeter.yaml b/mmv1/products/accesscontextmanager/ServicePerimeter.yaml
@@ -69,6 +69,7 @@ async:
     path: 'error'
     message: 'message'
 custom_code:
+  constants: 'templates/terraform/constants/access_context_manager.go.tmpl'
   encoder: 'templates/terraform/encoders/access_level_never_send_parent.go.tmpl'
   custom_import: 'templates/terraform/custom_import/set_access_policy_parent_from_self_link.go.tmpl'
 # Skipping the sweeper due to the non-standard base_url
@@ -256,6 +257,7 @@ properties:
                     - 'ANY_IDENTITY'
                     - 'ANY_USER_ACCOUNT'
                     - 'ANY_SERVICE_ACCOUNT'
+                  diff_suppress_func: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc
                 - name: 'identities'
                   type: Array
                   description: |
@@ -376,6 +378,7 @@ properties:
                     - 'ANY_IDENTITY'
                     - 'ANY_USER_ACCOUNT'
                     - 'ANY_SERVICE_ACCOUNT'
+                  diff_suppress_func: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc
                 - name: 'sources'
                   type: Array
                   description: 'Sources that this EgressPolicy authorizes access from.'
@@ -564,6 +567,7 @@ properties:
                     - 'ANY_IDENTITY'
                     - 'ANY_USER_ACCOUNT'
                     - 'ANY_SERVICE_ACCOUNT'
+                  diff_suppress_func: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc
                 - name: 'identities'
                   type: Array
                   description: |
@@ -681,6 +685,7 @@ properties:
                     - 'ANY_IDENTITY'
                     - 'ANY_USER_ACCOUNT'
                     - 'ANY_SERVICE_ACCOUNT'
+                  diff_suppress_func: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc
                 - name: 'sources'
                   type: Array
                   description: 'Sources that this EgressPolicy authorizes access from.'

diff --git a/mmv1/products/accesscontextmanager/ServicePerimeterDryRunEgressPolicy.yaml b/mmv1/products/accesscontextmanager/ServicePerimeterDryRunEgressPolicy.yaml
@@ -108,6 +108,7 @@ properties:
           - 'ANY_IDENTITY'
           - 'ANY_USER_ACCOUNT'
           - 'ANY_SERVICE_ACCOUNT'
+        diff_suppress_func: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc
       - name: 'identities'
         type: Array
         description: |

diff --git a/mmv1/products/accesscontextmanager/ServicePerimeterDryRunIngressPolicy.yaml b/mmv1/products/accesscontextmanager/ServicePerimeterDryRunIngressPolicy.yaml
@@ -110,6 +110,7 @@ properties:
           - 'ANY_IDENTITY'
           - 'ANY_USER_ACCOUNT'
           - 'ANY_SERVICE_ACCOUNT'
+        diff_suppress_func: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc
       - name: 'identities'
         type: Array
         description: |

diff --git a/mmv1/products/accesscontextmanager/ServicePerimeterEgressPolicy.yaml b/mmv1/products/accesscontextmanager/ServicePerimeterEgressPolicy.yaml
@@ -105,6 +105,7 @@ properties:
           - 'ANY_IDENTITY'
           - 'ANY_USER_ACCOUNT'
           - 'ANY_SERVICE_ACCOUNT'
+        diff_suppress_func: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc
       - name: 'identities'
         type: Array
         description: |

diff --git a/mmv1/products/accesscontextmanager/ServicePerimeterIngressPolicy.yaml b/mmv1/products/accesscontextmanager/ServicePerimeterIngressPolicy.yaml
@@ -107,6 +107,7 @@ properties:
           - 'ANY_IDENTITY'
           - 'ANY_USER_ACCOUNT'
           - 'ANY_SERVICE_ACCOUNT'
+        diff_suppress_func: AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc
       - name: 'identities'
         type: Array
         description: |

diff --git a/mmv1/templates/terraform/constants/access_context_manager.go.tmpl b/mmv1/templates/terraform/constants/access_context_manager.go.tmpl
@@ -38,4 +38,12 @@ func {{$.ResourceName}}IngressToResourcesDiffSupressFunc(_, _, _ string, d *sche
     sort.Strings(newResources)
 
     return slices.Equal(oldResources, newResources)
-}
+}
+
+func {{$.ResourceName}}IdentityTypeDiffSupressFunc(_, old, new string, _ *schema.ResourceData) bool {
+    if old == "" && new == "IDENTITY_TYPE_UNSPECIFIED" {
+       return true
+    }
+
+    return old == new
+}
diff --git a/...vices/accesscontextmanager/resource_access_context_manager_service_perimeter_test.go.tmpl b/...vices/accesscontextmanager/resource_access_context_manager_service_perimeter_test.go.tmpl
@@ -7,6 +7,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 
+	"github.com/hashicorp/terraform-provider-google/google/services/accesscontextmanager"
 	"github.com/hashicorp/terraform-provider-google/google/acctest"
 	"github.com/hashicorp/terraform-provider-google/google/envvar"
 	"github.com/hashicorp/terraform-provider-google/google/tpgresource"
@@ -410,3 +411,60 @@ resource "google_access_context_manager_service_perimeter" "test-access" {
 `, org, policyTitle, levelTitleName, levelTitleName, perimeterTitleName, perimeterTitleName)
 }
 
+
+type IdentityTypeDiffSupressFuncDiffSuppressTestCase struct {
+	Name               string
+	AreEqual bool
+	Before         	   string
+	After              string
+}
+
+
+var identityTypeDiffSuppressTestCases = []IdentityTypeDiffSupressFuncDiffSuppressTestCase{
+	{
+		AreEqual: false,
+		Before: "A",
+		After: "B",
+	},
+	{
+		AreEqual: true,
+		Before: "A",
+		After: "A",
+	},
+	{
+		AreEqual: false,
+		Before: "",
+		After: "A",
+	},
+	{
+		AreEqual: false,
+		Before: "A",
+		After: "",
+	},
+	{
+		AreEqual: true,
+		Before: "",
+		After: "IDENTITY_TYPE_UNSPECIFIED",
+	},
+	{
+		AreEqual: false,
+		Before: "IDENTITY_TYPE_UNSPECIFIED",
+		After: "",
+	},
+}
+
+func TestUnitAccessContextManagerServicePerimeter_identityTypeDiff(t *testing.T) {
+	for _, tc := range identityTypeDiffSuppressTestCases {
+		tc.Test(t)
+	}
+}
+
+
+func (tc *IdentityTypeDiffSupressFuncDiffSuppressTestCase) Test(t *testing.T) {
+	actual := accesscontextmanager.AccessContextManagerServicePerimeterIdentityTypeDiffSupressFunc("", tc.Before, tc.After, nil)
+	if actual != tc.AreEqual {
+		t.Errorf(
+			"Unexpected difference found. Before: \"%s\", after: \"%s\", actual: %t, expected: %t",
+			tc.Before, tc.After, actual, tc.AreEqual)
+	}
+}