chore: User improvements

MIGRATION_GUIDE.md

@@ -256,6 +256,10 @@ Validation changes:
 - `default_secondary_roles` - only 1-element lists with `"ALL"` element are now supported. Check [Snowflake docs](https://docs.snowflake.com/en/sql-reference/sql/create-user#optional-object-properties-objectproperties) for more details.
 
 #### *(breaking change)* refactored snowflake_users datasource
+> **IMPORTANT NOTE:** when querying users you don't have permissions to, the querying options are limited. 
+You won't get almost any field in `show_output` (only empty or default values), the DESCRIBE command cannot be called, so you have to set `with_describe = false`. 
+Only `parameters` output is not affected by the lack of privileges.
+
 Changes:
 - account checking logic was entirely removed
 - `pattern` renamed to `like`

docs/data-sources/users.md

@@ -2,14 +2,14 @@
 page_title: "snowflake_users Data Source - terraform-provider-snowflake"
 subcategory: ""
 description: |-
-  Datasource used to get details of filtered users. Filtering is aligned with the current possibilities for SHOW USERS https://docs.snowflake.com/en/sql-reference/sql/show-users query. The results of SHOW, DESCRIBE, and SHOW PARAMETERS IN are encapsulated in one output collection.
+  Datasource used to get details of filtered users. Filtering is aligned with the current possibilities for SHOW USERS https://docs.snowflake.com/en/sql-reference/sql/show-users query. The results of SHOW, DESCRIBE, and SHOW PARAMETERS IN are encapsulated in one output collection. Important note is that when querying users you don't have permissions to, the querying options are limited. You won't get almost any field in show_output (only empty or default values), the DESCRIBE command cannot be called, so you have to set with_describe = false. Only parameters output is not affected by the lack of privileges.
 ---
 
 !> **V1 release candidate** This data source was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the data source if needed. Any errors reported will be resolved with a higher priority. We encourage checking this data source out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0920--v0930) to use it.
 
 # snowflake_users (Data Source)
 
-Datasource used to get details of filtered users. Filtering is aligned with the current possibilities for [SHOW USERS](https://docs.snowflake.com/en/sql-reference/sql/show-users) query. The results of SHOW, DESCRIBE, and SHOW PARAMETERS IN are encapsulated in one output collection.
+Datasource used to get details of filtered users. Filtering is aligned with the current possibilities for [SHOW USERS](https://docs.snowflake.com/en/sql-reference/sql/show-users) query. The results of SHOW, DESCRIBE, and SHOW PARAMETERS IN are encapsulated in one output collection. Important note is that when querying users you don't have permissions to, the querying options are limited. You won't get almost any field in `show_output` (only empty or default values), the DESCRIBE command cannot be called, so you have to set `with_describe = false`. Only `parameters` output is not affected by the lack of privileges.
 
 ## Example Usage
 
@@ -145,6 +145,7 @@ Read-Only:
 - `ext_authn_duo` (Boolean)
 - `ext_authn_uid` (String)
 - `first_name` (String)
+- `has_mfa` (Boolean)
 - `last_name` (String)
 - `login_name` (String)
 - `middle_name` (String)

pkg/acceptance/bettertestspoc/assert/objectassert/user_snowflake_ext.go

@@ -51,6 +51,17 @@ func (w *UserAssert) HasCreatedOnNotEmpty() *UserAssert {
 	return w
 }
 
+func (w *UserAssert) HasOwnerNotEmpty() *UserAssert {
+	w.AddAssertion(func(t *testing.T, o *sdk.User) error {
+		t.Helper()
+		if o.Owner == "" {
+			return fmt.Errorf("expected owner not empty; got empty")
+		}
+		return nil
+	})
+	return w
+}
+
 func (w *UserAssert) HasLastSuccessLoginEmpty() *UserAssert {
 	w.AddAssertion(func(t *testing.T, o *sdk.User) error {
 		t.Helper()

pkg/datasources/users.go

@@ -93,7 +93,7 @@ func Users() *schema.Resource {
 	return &schema.Resource{
 		ReadContext: ReadUsers,
 		Schema:      usersSchema,
-		Description: "Datasource used to get details of filtered users. Filtering is aligned with the current possibilities for [SHOW USERS](https://docs.snowflake.com/en/sql-reference/sql/show-users) query. The results of SHOW, DESCRIBE, and SHOW PARAMETERS IN are encapsulated in one output collection.",
+		Description: "Datasource used to get details of filtered users. Filtering is aligned with the current possibilities for [SHOW USERS](https://docs.snowflake.com/en/sql-reference/sql/show-users) query. The results of SHOW, DESCRIBE, and SHOW PARAMETERS IN are encapsulated in one output collection. Important note is that when querying users you don't have permissions to, the querying options are limited. You won't get almost any field in `show_output` (only empty or default values), the DESCRIBE command cannot be called, so you have to set `with_describe = false`. Only `parameters` output is not affected by the lack of privileges.",
 	}
 }
 

pkg/datasources/users_acceptance_test.go

@@ -80,7 +80,8 @@ func TestAcc_Users_Complete(t *testing.T) {
 						HasDefaultSecondaryRoles(`["ALL"]`).
 						HasMinsToBypassMfaNotEmpty().
 						HasHasRsaPublicKey(true).
-						HasComment(comment),
+						HasComment(comment).
+						HasHasMfa(false),
 					resourceparametersassert.UsersDatasourceParameters(t, "snowflake_users.test").
 						HasAllDefaults(),
 					assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.name", id.Name())),
@@ -113,6 +114,7 @@ func TestAcc_Users_Complete(t *testing.T) {
 					assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.describe_output.0.password_last_set_time")),
 					assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.custom_landing_page_url", "")),
 					assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.custom_landing_page_url_flush_next_ui_load", "false")),
+					assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.has_mfa", "false")),
 				),
 			},
 			{

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
 	})
 }