chore: User improvements (#3034)

## Changes - Added basic DiffSuppression/Validations for user parameters that are string enums. - Added basic flow for display_name | login_name fields (should I create a ticket and include its number for the 2nd step? incorrect values after rename). - Added integration test that shows which fields are visible when querying a given user with different query commands. - Added HasMfa to describe and added new asserts in users acceptance test to show it's there and it's getting mapped (the only way to get it `true` is to set MFA manually so I can only assert it's `false`). ## Doing right now - Testing manually default_namespace / default_role --------- Co-authored-by: Artur Sawicki <[email protected]>
Snowflake-Labs · Sep 3, 2024 · 65b64d7 · 65b64d7
1 parent 195b41c
commit 65b64d7
Show file tree

Hide file tree

Showing 11 changed files with 290 additions and 29 deletions.
diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md
@@ -273,6 +273,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`

diff --git a/docs/data-sources/users.md b/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)

diff --git a/pkg/acceptance/bettertestspoc/assert/objectassert/user_snowflake_ext.go b/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()

diff --git a/pkg/datasources/users.go b/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.",
 	}
 }
 

diff --git a/pkg/datasources/users_acceptance_test.go b/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")),
 				),
 			},
 			{

diff --git a/pkg/resources/user_acceptance_test.go b/pkg/resources/user_acceptance_test.go
@@ -1102,3 +1102,108 @@ func TestAcc_User_handleChangesToDefaultSecondaryRoles(t *testing.T) {
 		},
 	})
 }
+
+func TestAcc_User_ParameterValidationsAndDiffSuppressions(t *testing.T) {
+	id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
+
+	userModel := model.User("w", id.Name()).
+		WithBinaryInputFormat(strings.ToLower(string(sdk.BinaryInputFormatHex))).
+		WithBinaryOutputFormat(strings.ToLower(string(sdk.BinaryOutputFormatHex))).
+		WithGeographyOutputFormat(strings.ToLower(string(sdk.GeographyOutputFormatGeoJSON))).
+		WithGeometryOutputFormat(strings.ToLower(string(sdk.GeometryOutputFormatGeoJSON))).
+		WithLogLevel(strings.ToLower(string(sdk.LogLevelInfo))).
+		WithTimestampTypeMapping(strings.ToLower(string(sdk.TimestampTypeMappingNtz))).
+		WithTraceLevel(strings.ToLower(string(sdk.TraceLevelAlways)))
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		PreCheck:     func() { acc.TestAccPreCheck(t) },
+		CheckDestroy: acc.CheckDestroy(t, resources.User),
+		Steps: []resource.TestStep{
+			{
+				Config: config.FromModel(t, userModel),
+				Check: assert.AssertThat(t,
+					resourceassert.UserResource(t, userModel.ResourceReference()).
+						HasBinaryInputFormatString(string(sdk.BinaryInputFormatHex)).
+						HasBinaryOutputFormatString(string(sdk.BinaryOutputFormatHex)).
+						HasGeographyOutputFormatString(string(sdk.GeographyOutputFormatGeoJSON)).
+						HasGeometryOutputFormatString(string(sdk.GeometryOutputFormatGeoJSON)).
+						HasLogLevelString(string(sdk.LogLevelInfo)).
+						HasTimestampTypeMappingString(string(sdk.TimestampTypeMappingNtz)).
+						HasTraceLevelString(string(sdk.TraceLevelAlways)),
+				),
+			},
+		},
+	})
+}
+
+func TestAcc_User_LoginNameAndDisplayName(t *testing.T) {
+	id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
+	newId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
+
+	userModelWithoutBoth := model.User("w", id.Name())
+	userModelWithNewId := model.User("w", newId.Name())
+	userModelWithBoth := model.User("w", newId.Name()).WithLoginName("login_name").WithDisplayName("display_name")
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		PreCheck:     func() { acc.TestAccPreCheck(t) },
+		CheckDestroy: acc.CheckDestroy(t, resources.User),
+		Steps: []resource.TestStep{
+			// Create without both set
+			{
+				Config: config.FromModel(t, userModelWithoutBoth),
+				Check: assert.AssertThat(t,
+					resourceassert.UserResource(t, userModelWithoutBoth.ResourceReference()).
+						HasNoDisplayName().
+						HasNoLoginName(),
+					objectassert.User(t, id).
+						HasDisplayName(strings.ToUpper(id.Name())).
+						HasLoginName(strings.ToUpper(id.Name())),
+				),
+			},
+			// Rename
+			{
+				Config: config.FromModel(t, userModelWithNewId),
+				Check: assert.AssertThat(t,
+					resourceassert.UserResource(t, userModelWithNewId.ResourceReference()).
+						HasNoDisplayName().
+						HasNoLoginName(),
+					objectassert.User(t, newId).
+						HasDisplayName(strings.ToUpper(id.Name())).
+						HasLoginName(strings.ToUpper(id.Name())),
+				),
+			},
+			// Set both params
+			{
+				Config: config.FromModel(t, userModelWithBoth),
+				Check: assert.AssertThat(t,
+					resourceassert.UserResource(t, userModelWithBoth.ResourceReference()).
+						HasDisplayNameString("display_name").
+						HasLoginNameString("login_name"),
+					objectassert.User(t, newId).
+						HasDisplayName("display_name").
+						HasLoginName("LOGIN_NAME"),
+				),
+			},
+			// Unset both params
+			{
+				Config: config.FromModel(t, userModelWithNewId),
+				Check: assert.AssertThat(t,
+					resourceassert.UserResource(t, userModelWithNewId.ResourceReference()).
+						HasDisplayNameString("").
+						HasLoginNameString(""),
+					objectassert.User(t, newId).
+						HasDisplayName("").
+						HasLoginName(strings.ToUpper(newId.Name())),
+				),
+			},
+		},
+	})
+}