fix: Fix custom diffs for fields with diff supression (#3032)

Fix custom diffs for fields with diff impression:
- existing `ComputedIfAnyAttributeChanged` merged with
`ComputedIfAnyAttributeChangedWithSuppressDiff`
- changed the way how the suppress diff is invoked from within
`ComputedIfAnyAttributeChanged` (ResourceData and string values for old
and new)
- fix the failing tests (behavior was fixed)
- add test to validate the behavior in warehouse

pkg/internal/provider/sdkv2enhancements/resource_data.go

@@ -0,0 +1,36 @@
+package sdkv2enhancements
+
+import (
+	"reflect"
+	"unsafe"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
+)
+
+// CreateResourceDataFromResourceDiff allows to create schema.ResourceData out of schema.ResourceDiff.
+// Unfortunately, schema.resourceDiffer is an unexported interface, and functions like schema.SchemaDiffSuppressFunc do not use the interface but a concrete implementation.
+// One use case in which we needed to have schema.ResourceData from schema.ResourceDiff was to run schema.SchemaDiffSuppressFunc from inside schema.CustomizeDiffFunc.
+// This implementation uses:
+// - schema.InternalMap that exposes hidden schema.schemaMap (a wrapper over map[string]*schema.Schema)
+// - (m schemaMap) Data method allowing to create schema.ResourceData from terraform.InstanceState and terraform.InstanceDiff
+// - terraform.InstanceState and terraform.InstanceDiff are unexported in schema.ResourceDiff, so we get them using reflection
+func CreateResourceDataFromResourceDiff(resourceSchema schema.InternalMap, diff *schema.ResourceDiff) (*schema.ResourceData, bool) {
+	unexportedState := reflect.ValueOf(diff).Elem().FieldByName("state")
+	stateFromResourceDiff := reflect.NewAt(unexportedState.Type(), unsafe.Pointer(unexportedState.UnsafeAddr())).Elem().Interface()
+	unexportedDiff := reflect.ValueOf(diff).Elem().FieldByName("diff")
+	diffFroResourceDif := reflect.NewAt(unexportedDiff.Type(), unsafe.Pointer(unexportedDiff.UnsafeAddr())).Elem().Interface()
+	castState, ok := stateFromResourceDiff.(*terraform.InstanceState)
+	if !ok {
+		return nil, false
+	}
+	castDiff, ok := diffFroResourceDif.(*terraform.InstanceDiff)
+	if !ok {
+		return nil, false
+	}
+	resourceData, err := resourceSchema.Data(castState, castDiff)
+	if err != nil {
+		return nil, false
+	}
+	return resourceData, true
+}

pkg/resources/account.go

@@ -216,7 +216,7 @@ func Account() *schema.Resource {
 		Delete:      DeleteAccount,
 
 		CustomizeDiff: customdiff.All(
-			ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"),
+			ComputedIfAnyAttributeChanged(accountSchema, FullyQualifiedNameAttributeName, "name"),
 		),
 
 		Schema: accountSchema,

pkg/resources/account_role.go

@@ -50,9 +50,8 @@ func AccountRole() *schema.Resource {
 		Description:   "The resource is used for role management, where roles can be assigned privileges and, in turn, granted to users and other roles. When granted to roles they can create hierarchies of privilege structures. For more details, refer to the [official documentation](https://docs.snowflake.com/en/user-guide/security-access-control-overview).",
 
 		CustomizeDiff: customdiff.All(
-			ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment"),
-			ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"),
-			ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"),
+			ComputedIfAnyAttributeChanged(accountRoleSchema, ShowOutputAttributeName, "comment", "name"),
+			ComputedIfAnyAttributeChanged(accountRoleSchema, FullyQualifiedNameAttributeName, "name"),
 		),
 
 		Importer: &schema.ResourceImporter{

pkg/resources/api_authentication_integration_with_authorization_code_grant.go

@@ -46,8 +46,8 @@ func ApiAuthenticationIntegrationWithAuthorizationCodeGrant() *schema.Resource {
 			ForceNewIfChangeToEmptyString("oauth_token_endpoint"),
 			ForceNewIfChangeToEmptyString("oauth_authorization_endpoint"),
 			ForceNewIfChangeToEmptyString("oauth_client_auth_method"),
-			ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "comment"),
-			ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity",
+			ComputedIfAnyAttributeChanged(apiAuthAuthorizationCodeGrantSchema, ShowOutputAttributeName, "enabled", "comment"),
+			ComputedIfAnyAttributeChanged(apiAuthAuthorizationCodeGrantSchema, DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity",
 				"oauth_client_id", "oauth_client_auth_method", "oauth_authorization_endpoint",
 				"oauth_token_endpoint", "oauth_allowed_scopes"),
 		),

pkg/resources/api_authentication_integration_with_client_credentials.go

@@ -41,8 +41,8 @@ func ApiAuthenticationIntegrationWithClientCredentials() *schema.Resource {
 		CustomizeDiff: customdiff.All(
 			ForceNewIfChangeToEmptyString("oauth_token_endpoint"),
 			ForceNewIfChangeToEmptyString("oauth_client_auth_method"),
-			ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "comment"),
-			ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity",
+			ComputedIfAnyAttributeChanged(apiAuthClientCredentialsSchema, ShowOutputAttributeName, "enabled", "comment"),
+			ComputedIfAnyAttributeChanged(apiAuthClientCredentialsSchema, DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity",
 				"oauth_client_id", "oauth_client_auth_method", "oauth_token_endpoint", "oauth_allowed_scopes"),
 		),
 		Importer: &schema.ResourceImporter{

pkg/resources/api_authentication_integration_with_jwt_bearer.go

@@ -45,8 +45,8 @@ func ApiAuthenticationIntegrationWithJwtBearer() *schema.Resource {
 			ForceNewIfChangeToEmptyString("oauth_token_endpoint"),
 			ForceNewIfChangeToEmptyString("oauth_authorization_endpoint"),
 			ForceNewIfChangeToEmptyString("oauth_client_auth_method"),
-			ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "comment"),
-			ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity",
+			ComputedIfAnyAttributeChanged(apiAuthJwtBearerSchema, ShowOutputAttributeName, "enabled", "comment"),
+			ComputedIfAnyAttributeChanged(apiAuthJwtBearerSchema, DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity",
 				"oauth_client_id", "oauth_client_auth_method", "oauth_authorization_endpoint",
 				"oauth_token_endpoint", "oauth_assertion_issuer"),
 		),

pkg/resources/custom_diffs.go

@@ -2,12 +2,13 @@ package resources
 
 import (
 	"context"
+	"fmt"
 	"log"
 	"strconv"
 	"strings"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections"
-
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider/sdkv2enhancements"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -83,33 +84,38 @@ func ForceNewIfChangeToEmptyString(key string) schema.CustomizeDiffFunc {
 	})
 }
 
-func ComputedIfAnyAttributeChanged(key string, changedAttributeKeys ...string) schema.CustomizeDiffFunc {
+// ComputedIfAnyAttributeChanged marks the given fields as computed if any of the listed fields changes.
+// It takes field-level diffSuppress into consideration based on the schema passed.
+// If the field is not found in the given schema, it continues without error.
+func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key string, changedAttributeKeys ...string) schema.CustomizeDiffFunc {
 	return customdiff.ComputedIf(key, func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) bool {
 		var result bool
-		for _, changedKey := range changedAttributeKeys {
-			if diff.HasChange(changedKey) {
-				old, new := diff.GetChange(changedKey)
-				log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: changed key: %s old: %s new: %s\n", changedKey, old, new)
-			}
-			result = result || diff.HasChange(changedKey)
-		}
-		return result
-	})
-}
-
-// TODO(SNOW-1629468): Adjust the function to make it more flexible
-func ComputedIfAnyAttributeChangedWithSuppressDiff(key string, suppressDiffFunc schema.SchemaDiffSuppressFunc, changedAttributeKeys ...string) schema.CustomizeDiffFunc {
-	return customdiff.ComputedIf(key, func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) bool {
 		for _, changedKey := range changedAttributeKeys {
 			if diff.HasChange(changedKey) {
 				oldValue, newValue := diff.GetChange(changedKey)
-				if !suppressDiffFunc(key, oldValue.(string), newValue.(string), nil) {
-					log.Printf("[DEBUG] ComputedIfAnyAttributeChangedWithSuppressDiff: changed key: %s", changedKey)
-					return true
+				log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: changed key: %s old: %s new: %s\n", changedKey, oldValue, newValue)
+
+				if v, ok := resourceSchema[changedKey]; ok {
+					if diffSuppressFunc := v.DiffSuppressFunc; diffSuppressFunc != nil {
+						resourceData, resourceDataOk := sdkv2enhancements.CreateResourceDataFromResourceDiff(resourceSchema, diff)
+						if !resourceDataOk {
+							log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: did not create resource data correctly, skipping\n")
+							continue
+						}
+						if !diffSuppressFunc(key, fmt.Sprintf("%v", oldValue), fmt.Sprintf("%v", newValue), resourceData) {
+							log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: key %s was changed and the diff is not suppressed", changedKey)
+							result = true
+						} else {
+							log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: key %s was changed but the diff is suppresed", changedKey)
+						}
+					} else {
+						log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: key %s was changed and it does not have a diff suppressor", changedKey)
+						result = true
+					}
 				}
 			}
 		}
-		return false
+		return result
 	})
 }