From 17fba7e83e61f034ae013970b4c8fd2039b36ca4 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 10:38:11 +0200 Subject: [PATCH 01/42] rework users datasource starts here From a45ac4ff5fc5f6a61e07c7e8a8ba513da6778b2c Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 11:32:51 +0200 Subject: [PATCH 02/42] Add user describe schema and mapper; rework users datasource --- pkg/datasources/users.go | 235 ++++++++++++++++++----------------- pkg/schemas/user_describe.go | 231 ++++++++++++++++++++++++++++++++++ 2 files changed, 355 insertions(+), 111 deletions(-) create mode 100644 pkg/schemas/user_describe.go diff --git a/pkg/datasources/users.go b/pkg/datasources/users.go index e620ac6ee5..18fe56371a 100644 --- a/pkg/datasources/users.go +++ b/pkg/datasources/users.go @@ -2,93 +2,87 @@ package datasources import ( "context" - "fmt" - "log" - "strings" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" - - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/resources" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) var usersSchema = map[string]*schema.Schema{ - "pattern": { - Type: schema.TypeString, - Required: true, - Description: "Users pattern for which to return metadata. Please refer to LIKE keyword from " + - "snowflake documentation : https://docs.snowflake.com/en/sql-reference/sql/show-users.html#parameters", + "with_describe": { + Type: schema.TypeBool, + Optional: true, + Default: true, + Description: "Runs DESC USER for each user returned by SHOW USERS. The output of describe is saved to the description field. By default this value is set to true.", }, - "users": { + "with_parameters": { + Type: schema.TypeBool, + Optional: true, + Default: true, + Description: "Runs SHOW PARAMETERS FOR USER for each user returned by SHOW USERS. The output of describe is saved to the parameters field as a map. By default this value is set to true.", + }, + "like": { + Type: schema.TypeString, + Optional: true, + Description: "Filters the output with **case-insensitive** pattern, with support for SQL wildcard characters (`%` and `_`).", + }, + "starts_with": { + Type: schema.TypeString, + Optional: true, + Description: "Filters the output with **case-sensitive** characters indicating the beginning of the object name.", + }, + "limit": { Type: schema.TypeList, - Computed: true, - Description: "The users in the database", + Optional: true, + Description: "Limits the number of rows returned. If the `limit.from` is set, then the limit wll start from the first element matched by the expression. The expression is only used to match with the first element, later on the elements are not matched by the prefix, but you can enforce a certain pattern with `starts_with` or `like`.", + MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "name": { - Type: schema.TypeString, - Computed: true, - }, - "login_name": { - Type: schema.TypeString, - Optional: true, - Computed: true, - }, - "comment": { - Type: schema.TypeString, - Optional: true, - Computed: true, - }, - "disabled": { - Type: schema.TypeBool, - Optional: true, - Computed: true, - }, - "default_warehouse": { - Type: schema.TypeString, - Optional: true, - Computed: true, + "rows": { + Type: schema.TypeInt, + Required: true, + Description: "The maximum number of rows to return.", }, - "default_namespace": { - Type: schema.TypeString, - Optional: true, - Computed: true, + "from": { + Type: schema.TypeString, + Optional: true, + Description: "Specifies a **case-sensitive** pattern that is used to match object name. After the first match, the limit on the number of rows will be applied.", }, - "default_role": { - Type: schema.TypeString, - Optional: true, - Computed: true, - }, - "default_secondary_roles": { - Type: schema.TypeSet, - Elem: &schema.Schema{Type: schema.TypeString}, - Optional: true, - Computed: true, - }, - "has_rsa_public_key": { - Type: schema.TypeBool, - Computed: true, - }, - "email": { - Type: schema.TypeString, - Optional: true, - Computed: true, - }, - "display_name": { - Type: schema.TypeString, - Computed: true, - Optional: true, + }, + }, + }, + "users": { + Type: schema.TypeList, + Computed: true, + Description: "Holds the aggregated output of all user details queries.", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + resources.ShowOutputAttributeName: { + Type: schema.TypeList, + Computed: true, + Description: "Holds the output of SHOW USERS.", + Elem: &schema.Resource{ + Schema: schemas.ShowUserSchema, + }, }, - "first_name": { - Type: schema.TypeString, - Optional: true, - Computed: true, + resources.DescribeOutputAttributeName: { + Type: schema.TypeList, + Computed: true, + Description: "Holds the output of DESCRIBE USER.", + Elem: &schema.Resource{ + Schema: schemas.UserDescribeSchema, + }, }, - "last_name": { - Type: schema.TypeString, - Optional: true, - Computed: true, + resources.ParametersAttributeName: { + Type: schema.TypeList, + Computed: true, + Description: "Holds the output of SHOW PARAMETERS FOR USER.", + Elem: &schema.Resource{ + Schema: schemas.ShowUserParametersSchema, + }, }, }, }, @@ -97,57 +91,76 @@ var usersSchema = map[string]*schema.Schema{ func Users() *schema.Resource { return &schema.Resource{ - Read: ReadUsers, - Schema: usersSchema, - Importer: &schema.ResourceImporter{ - StateContext: schema.ImportStatePassthroughContext, - }, + 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.", } } -func ReadUsers(d *schema.ResourceData, meta interface{}) error { +func ReadUsers(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client - ctx := context.Background() + var opts sdk.ShowUserOptions - userPattern := d.Get("pattern").(string) + if likePattern, ok := d.GetOk("like"); ok { + opts.Like = &sdk.Like{ + Pattern: sdk.String(likePattern.(string)), + } + } - account, err1 := client.ContextFunctions.CurrentAccount(ctx) - region, err2 := client.ContextFunctions.CurrentRegion(ctx) - if err1 != nil || err2 != nil { - log.Print("[DEBUG] unable to retrieve current account") - d.SetId("") - return nil + if startsWith, ok := d.GetOk("starts_with"); ok { + opts.StartsWith = sdk.String(startsWith.(string)) } - d.SetId(fmt.Sprintf("%s.%s", account, region)) - extractedUsers, err := client.Users.Show(ctx, &sdk.ShowUserOptions{ - Like: &sdk.Like{Pattern: sdk.String(userPattern)}, - }) + if limit, ok := d.GetOk("limit"); ok && len(limit.([]any)) == 1 { + limitMap := limit.([]any)[0].(map[string]any) + + rows := limitMap["rows"].(int) + opts.Limit = &rows + + if from, ok := limitMap["from"].(string); ok { + opts.From = &from + } + } + + users, err := client.Users.Show(ctx, &opts) if err != nil { - log.Printf("[DEBUG] no users found in account (%s)", d.Id()) - d.SetId("") - return nil + return diag.FromErr(err) } + d.SetId("users_read") + + flattenedUsers := make([]map[string]any, len(users)) - users := make([]map[string]any, len(extractedUsers)) - - for i, user := range extractedUsers { - users[i] = map[string]any{ - "name": user.Name, - "login_name": user.LoginName, - "comment": user.Comment, - "disabled": user.Disabled, - "default_warehouse": user.DefaultWarehouse, - "default_namespace": user.DefaultNamespace, - "default_role": user.DefaultRole, - "default_secondary_roles": strings.Split(helpers.ListContentToString(user.DefaultSecondaryRoles), ","), - "has_rsa_public_key": user.HasRsaPublicKey, - "email": user.Email, - "display_name": user.DisplayName, - "first_name": user.FirstName, - "last_name": user.LastName, + for i, user := range users { + user := user + var userDescription []map[string]any + if d.Get("with_describe").(bool) { + describeResult, err := client.Users.Describe(ctx, user.ID()) + if err != nil { + return diag.FromErr(err) + } + userDescription = schemas.UserDescriptionToSchema(*describeResult) } + + var userParameters []map[string]any + if d.Get("with_parameters").(bool) { + parameters, err := client.Users.ShowParameters(ctx, user.ID()) + if err != nil { + return diag.FromErr(err) + } + userParameters = []map[string]any{schemas.UserParametersToSchema(parameters)} + } + + flattenedUsers[i] = map[string]any{ + resources.ShowOutputAttributeName: []map[string]any{schemas.UserToSchema(&user)}, + resources.DescribeOutputAttributeName: userDescription, + resources.ParametersAttributeName: userParameters, + } + } + + err = d.Set("users", flattenedUsers) + if err != nil { + return diag.FromErr(err) } - return d.Set("users", users) + return nil } diff --git a/pkg/schemas/user_describe.go b/pkg/schemas/user_describe.go new file mode 100644 index 0000000000..8292999d39 --- /dev/null +++ b/pkg/schemas/user_describe.go @@ -0,0 +1,231 @@ +package schemas + +import ( + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +// UserDescribeSchema represents output of DESCRIBE query for the single UserDetails. +var UserDescribeSchema = map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Computed: true, + }, + "comment": { + Type: schema.TypeString, + Computed: true, + }, + "display_name": { + Type: schema.TypeString, + Computed: true, + }, + "login_name": { + Type: schema.TypeString, + Computed: true, + }, + "first_name": { + Type: schema.TypeString, + Computed: true, + }, + "middle_name": { + Type: schema.TypeString, + Computed: true, + }, + "last_name": { + Type: schema.TypeString, + Computed: true, + }, + "email": { + Type: schema.TypeString, + Computed: true, + }, + "password": { + Type: schema.TypeString, + Computed: true, + }, + "must_change_password": { + Type: schema.TypeBool, + Computed: true, + }, + "disabled": { + Type: schema.TypeBool, + Computed: true, + }, + "snowflake_lock": { + Type: schema.TypeBool, + Computed: true, + }, + "snowflake_support": { + Type: schema.TypeBool, + Computed: true, + }, + "days_to_expiry": { + Type: schema.TypeFloat, + Computed: true, + }, + "mins_to_unlock": { + Type: schema.TypeInt, + Computed: true, + }, + "default_warehouse": { + Type: schema.TypeString, + Computed: true, + }, + "default_namespace": { + Type: schema.TypeString, + Computed: true, + }, + "default_role": { + Type: schema.TypeString, + Computed: true, + }, + "default_secondary_roles": { + Type: schema.TypeString, + Computed: true, + }, + "ext_authn_duo": { + Type: schema.TypeBool, + Computed: true, + }, + "ext_authn_uid": { + Type: schema.TypeString, + Computed: true, + }, + "mins_to_bypass_mfa": { + Type: schema.TypeInt, + Computed: true, + }, + "mins_to_bypass_network_policy": { + Type: schema.TypeInt, + Computed: true, + }, + "rsa_public_key": { + Type: schema.TypeString, + Computed: true, + }, + "rsa_public_key_fp": { + Type: schema.TypeString, + Computed: true, + }, + "rsa_public_key2": { + Type: schema.TypeString, + Computed: true, + }, + "rsa_public_key2_fp": { + Type: schema.TypeString, + Computed: true, + }, + "password_last_set_time": { + Type: schema.TypeString, + Computed: true, + }, + "custom_landing_page_url": { + Type: schema.TypeString, + Computed: true, + }, + "custom_landing_page_url_flush_next_ui_load": { + Type: schema.TypeBool, + Computed: true, + }, +} + +var _ = UserDescribeSchema + +func UserDescriptionToSchema(userDetails sdk.UserDetails) []map[string]any { + userDetailsSchema := make(map[string]any) + if userDetails.Name != nil { + userDetailsSchema["name"] = userDetails.Name.Value + } + if userDetails.Comment != nil { + userDetailsSchema["comment"] = userDetails.Comment.Value + } + if userDetails.DisplayName != nil { + userDetailsSchema["display_name"] = userDetails.DisplayName.Value + } + if userDetails.LoginName != nil { + userDetailsSchema["login_name"] = userDetails.LoginName.Value + } + if userDetails.FirstName != nil { + userDetailsSchema["first_name"] = userDetails.FirstName.Value + } + if userDetails.MiddleName != nil { + userDetailsSchema["middle_name"] = userDetails.MiddleName.Value + } + if userDetails.LastName != nil { + userDetailsSchema["last_name"] = userDetails.LastName.Value + } + if userDetails.Email != nil { + userDetailsSchema["email"] = userDetails.Email.Value + } + if userDetails.Password != nil { + userDetailsSchema["password"] = userDetails.Password.Value + } + if userDetails.MustChangePassword != nil { + userDetailsSchema["must_change_password"] = userDetails.MustChangePassword.Value + } + if userDetails.Disabled != nil { + userDetailsSchema["disabled"] = userDetails.Disabled.Value + } + if userDetails.SnowflakeLock != nil { + userDetailsSchema["snowflake_lock"] = userDetails.SnowflakeLock.Value + } + if userDetails.SnowflakeSupport != nil { + userDetailsSchema["snowflake_support"] = userDetails.SnowflakeSupport.Value + } + if userDetails.DaysToExpiry != nil && userDetails.DaysToExpiry.Value != nil { + userDetailsSchema["days_to_expiry"] = *userDetails.DaysToExpiry.Value + } + if userDetails.MinsToUnlock != nil && userDetails.MinsToUnlock.Value != nil { + userDetailsSchema["mins_to_unlock"] = *userDetails.MinsToUnlock.Value + } + if userDetails.DefaultWarehouse != nil { + userDetailsSchema["default_warehouse"] = userDetails.DefaultWarehouse.Value + } + if userDetails.DefaultNamespace != nil { + userDetailsSchema["default_namespace"] = userDetails.DefaultNamespace.Value + } + if userDetails.DefaultRole != nil { + userDetailsSchema["default_role"] = userDetails.DefaultRole.Value + } + if userDetails.DefaultSecondaryRoles != nil { + userDetailsSchema["default_secondary_roles"] = userDetails.DefaultSecondaryRoles.Value + } + if userDetails.ExtAuthnDuo != nil { + userDetailsSchema["ext_authn_duo"] = userDetails.ExtAuthnDuo.Value + } + if userDetails.ExtAuthnUid != nil { + userDetailsSchema["ext_authn_uid"] = userDetails.ExtAuthnUid.Value + } + if userDetails.MinsToBypassMfa != nil && userDetails.MinsToBypassMfa.Value != nil { + userDetailsSchema["mins_to_bypass_mfa"] = userDetails.MinsToBypassMfa.Value + } + if userDetails.MinsToBypassNetworkPolicy != nil && userDetails.MinsToBypassNetworkPolicy.Value != nil { + userDetailsSchema["mins_to_bypass_network_policy"] = userDetails.MinsToBypassNetworkPolicy.Value + } + if userDetails.RsaPublicKey != nil { + userDetailsSchema["rsa_public_key"] = userDetails.RsaPublicKey.Value + } + if userDetails.RsaPublicKeyFp != nil { + userDetailsSchema["rsa_public_key_fp"] = userDetails.RsaPublicKeyFp.Value + } + if userDetails.RsaPublicKey2 != nil { + userDetailsSchema["rsa_public_key2"] = userDetails.RsaPublicKey2.Value + } + if userDetails.RsaPublicKey2Fp != nil { + userDetailsSchema["rsa_public_key2_fp"] = userDetails.RsaPublicKey2Fp.Value + } + if userDetails.PasswordLastSetTime != nil { + userDetailsSchema["password_last_set_time"] = userDetails.PasswordLastSetTime.Value + } + if userDetails.CustomLandingPageUrl != nil { + userDetailsSchema["custom_landing_page_url"] = userDetails.CustomLandingPageUrl.Value + } + if userDetails.CustomLandingPageUrlFlushNextUiLoad != nil { + userDetailsSchema["custom_landing_page_url_flush_next_ui_load"] = userDetails.CustomLandingPageUrlFlushNextUiLoad.Value + } + return []map[string]any{ + userDetailsSchema, + } +} + +var _ = UserDescriptionToSchema From 10a3e7b993ba1fdae16f4694fc2d8846b68312ad Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 11:50:49 +0200 Subject: [PATCH 03/42] Pass really basic test --- pkg/datasources/users_acceptance_test.go | 75 +++++++++++++++--------- 1 file changed, 46 insertions(+), 29 deletions(-) diff --git a/pkg/datasources/users_acceptance_test.go b/pkg/datasources/users_acceptance_test.go index 4f06e8419d..661641b841 100644 --- a/pkg/datasources/users_acceptance_test.go +++ b/pkg/datasources/users_acceptance_test.go @@ -1,56 +1,73 @@ package datasources_test import ( - "fmt" "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config/model" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/tfversion" ) -func TestAcc_Users(t *testing.T) { - userName := acc.TestClient().Ids.Alpha() +func TestAcc_Users_Complete(t *testing.T) { + _ = testenvs.GetOrSkipTest(t, testenvs.ConfigureClientOnce) + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + + comment := random.Comment() + pass := random.Password() + key1, _ := random.GenerateRSAPublicKey(t) + key2, _ := random.GenerateRSAPublicKey(t) + + userModelAllAttributes := model.User("u", id.Name()). + WithPassword(pass). + WithLoginName(id.Name() + "_login"). + WithDisplayName("Display Name"). + WithFirstName("Jan"). + WithMiddleName("Jakub"). + WithLastName("Testowski"). + WithEmail("fake@email.com"). + WithMustChangePassword("true"). + WithDisabled("false"). + WithDaysToExpiry(8). + WithMinsToUnlock(9). + WithDefaultWarehouse("some_warehouse"). + WithDefaultNamespace("some.namespace"). + WithDefaultRole("some_role"). + WithDefaultSecondaryRolesStringList("ALL"). + WithMinsToBypassMfa(10). + WithRsaPublicKey(key1). + WithRsaPublicKey2(key2). + WithComment(comment). + WithDisableMfa("true") + resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, + CheckDestroy: acc.CheckDestroy(t, resources.User), Steps: []resource.TestStep{ { - Config: users(userName), + Config: config.FromModel(t, userModelAllAttributes) + datasourceWithLike(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrSet("data.snowflake_users.u", "users.#"), - resource.TestCheckResourceAttr("data.snowflake_users.u", "users.#", "1"), - resource.TestCheckResourceAttr("data.snowflake_users.u", "users.0.name", userName), - resource.TestCheckResourceAttr("data.snowflake_users.u", "users.0.disabled", "false"), + resource.TestCheckResourceAttr("data.snowflake_users.test", "users.#", "1"), + + resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.name", id.Name()), ), }, }, }) } -func users(userName string) string { - return fmt.Sprintf(` - resource "snowflake_user" "u" { - name = "%s" - comment = "test comment" - login_name = "%s_login" - display_name = "Display Name" - first_name = "Alex" - last_name = "Kita" - email = "fake@email.com" - disabled = false - default_warehouse="foo" - default_role="FOO" - default_secondary_roles = ["ALL"] - default_namespace="FOO" - } - - data snowflake_users "u" { - pattern = "%s" - depends_on = [snowflake_user.u] +func datasourceWithLike() string { + return ` + data "snowflake_users" "test" { + like = snowflake_user.u.name } - `, userName, userName, userName) +` } From 6f1361ed9fe468b59fb05bb7a511a2bcdaecd42e Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 11:52:52 +0200 Subject: [PATCH 04/42] Add comment to handle sensitive in show output schemas --- pkg/schemas/user_ext.go | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 pkg/schemas/user_ext.go diff --git a/pkg/schemas/user_ext.go b/pkg/schemas/user_ext.go new file mode 100644 index 0000000000..696d7b7124 --- /dev/null +++ b/pkg/schemas/user_ext.go @@ -0,0 +1,3 @@ +package schemas + +// add sensitive to show output/describe output From 9fef8f1f0c944633d779b064272785a2ca96de31 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 13:33:04 +0200 Subject: [PATCH 05/42] Add a way to use show_output assertions in data sources test --- .../assert/resource_assertions.go | 20 +++++++++++++++---- .../users_datasource_show_output_ext.go | 18 +++++++++++++++++ pkg/datasources/users_acceptance_test.go | 17 +++++++++++++--- pkg/schemas/user_ext.go | 16 ++++++++++++++- 4 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/users_datasource_show_output_ext.go diff --git a/pkg/acceptance/bettertestspoc/assert/resource_assertions.go b/pkg/acceptance/bettertestspoc/assert/resource_assertions.go index 13fa6b1ae2..11cdedf6eb 100644 --- a/pkg/acceptance/bettertestspoc/assert/resource_assertions.go +++ b/pkg/acceptance/bettertestspoc/assert/resource_assertions.go @@ -21,10 +21,11 @@ var ( // ResourceAssert is an embeddable struct that should be used to construct new resource assertions (for resource, show output, parameters, etc.). // It implements both TestCheckFuncProvider and ImportStateCheckFuncProvider which makes it easy to create new resource assertions. type ResourceAssert struct { - name string - id string - prefix string - assertions []ResourceAssertion + name string + id string + prefix string + assertions []ResourceAssertion + additionalPrefix string } // NewResourceAssert creates a ResourceAssert where the resource name should be used as a key for assertions. @@ -45,6 +46,16 @@ func NewImportedResourceAssert(id string, prefix string) *ResourceAssert { } } +// NewDatasourceAssert creates a ResourceAssert for data sources. +func NewDatasourceAssert(name string, prefix string, additionalPrefix string) *ResourceAssert { + return &ResourceAssert{ + name: name, + prefix: prefix, + assertions: make([]ResourceAssertion, 0), + additionalPrefix: additionalPrefix, + } +} + type resourceAssertionType string const ( @@ -59,6 +70,7 @@ type ResourceAssertion struct { } func (r *ResourceAssert) AddAssertion(assertion ResourceAssertion) { + assertion.fieldName = r.additionalPrefix + assertion.fieldName r.assertions = append(r.assertions, assertion) } diff --git a/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/users_datasource_show_output_ext.go b/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/users_datasource_show_output_ext.go new file mode 100644 index 0000000000..76887e6bcd --- /dev/null +++ b/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/users_datasource_show_output_ext.go @@ -0,0 +1,18 @@ +package resourceshowoutputassert + +import ( + "testing" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" +) + +// UsersDatasourceShowOutput is a temporary workaround to have better show output assertions in data source acceptance tests. +func UsersDatasourceShowOutput(t *testing.T, name string) *UserShowOutputAssert { + t.Helper() + + u := UserShowOutputAssert{ + ResourceAssert: assert.NewDatasourceAssert("data."+name, "show_output", "users.0."), + } + u.AddAssertion(assert.ValueSet("show_output.#", "1")) + return &u +} diff --git a/pkg/datasources/users_acceptance_test.go b/pkg/datasources/users_acceptance_test.go index 661641b841..52f603227c 100644 --- a/pkg/datasources/users_acceptance_test.go +++ b/pkg/datasources/users_acceptance_test.go @@ -1,10 +1,13 @@ package datasources_test import ( + "fmt" "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config/model" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" @@ -54,10 +57,18 @@ func TestAcc_Users_Complete(t *testing.T) { Steps: []resource.TestStep{ { Config: config.FromModel(t, userModelAllAttributes) + datasourceWithLike(), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("data.snowflake_users.test", "users.#", "1"), + Check: assert.AssertThat(t, + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.#", "1")), - resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.name", id.Name()), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.name", id.Name())), + assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.show_output.0.created_on")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.login_name", fmt.Sprintf("%s_LOGIN", id.Name()))), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.display_name", "Display Name")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.first_name", "Jan")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.last_name", "Testowski")), + + resourceshowoutputassert.UsersDatasourceShowOutput(t, "snowflake_users.test"). + HasName(id.Name()), ), }, }, diff --git a/pkg/schemas/user_ext.go b/pkg/schemas/user_ext.go index 696d7b7124..2b261bb08f 100644 --- a/pkg/schemas/user_ext.go +++ b/pkg/schemas/user_ext.go @@ -1,3 +1,17 @@ package schemas -// add sensitive to show output/describe output +// This init is currently necessary to add sensitiveness to schemas which are generated (without such an underlying functionality yet). +func init() { + // show output does not contain password or middle_name + ShowUserSchema["login_name"].Sensitive = true + ShowUserSchema["first_name"].Sensitive = true + ShowUserSchema["last_name"].Sensitive = true + ShowUserSchema["email"].Sensitive = true + + UserDescribeSchema["login_name"].Sensitive = true + UserDescribeSchema["first_name"].Sensitive = true + UserDescribeSchema["middle_name"].Sensitive = true + UserDescribeSchema["last_name"].Sensitive = true + UserDescribeSchema["email"].Sensitive = true + UserDescribeSchema["password"].Sensitive = true +} From db4b05ac774a841892d486861a338a7e66ca392f Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 13:50:50 +0200 Subject: [PATCH 06/42] Add assertions to show output in users data source acc test --- .../assert/resource_assertions.go | 17 ++++++++++-- .../user_show_output_ext.go | 25 +++++++++++++++++ pkg/datasources/users_acceptance_test.go | 27 ++++++++++++------- 3 files changed, 58 insertions(+), 11 deletions(-) create mode 100644 pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/user_show_output_ext.go diff --git a/pkg/acceptance/bettertestspoc/assert/resource_assertions.go b/pkg/acceptance/bettertestspoc/assert/resource_assertions.go index 11cdedf6eb..3a404a4bcf 100644 --- a/pkg/acceptance/bettertestspoc/assert/resource_assertions.go +++ b/pkg/acceptance/bettertestspoc/assert/resource_assertions.go @@ -59,8 +59,9 @@ func NewDatasourceAssert(name string, prefix string, additionalPrefix string) *R type resourceAssertionType string const ( - resourceAssertionTypeValueSet = "VALUE_SET" - resourceAssertionTypeValueNotSet = "VALUE_NOT_SET" + resourceAssertionTypeValueSet = "VALUE_SET" + resourceAssertionTypeValueNotSet = "VALUE_NOT_SET" + resourceAssertionTypeValuePresent = "VALUE_PRESENT" ) type ResourceAssertion struct { @@ -104,6 +105,11 @@ func ResourceShowOutputValueSet(fieldName string, expected string) ResourceAsser return ResourceAssertion{fieldName: showOutputPrefix + fieldName, expectedValue: expected, resourceAssertionType: resourceAssertionTypeValueSet} } +// TODO [SNOW-1501905]: generate assertions with resourceAssertionTypeValuePresent +func ResourceShowOutputValuePresent(fieldName string) ResourceAssertion { + return ResourceAssertion{fieldName: showOutputPrefix + fieldName, resourceAssertionType: resourceAssertionTypeValuePresent} +} + const ( parametersPrefix = "parameters.0." parametersValueSuffix = ".0.value" @@ -149,6 +155,11 @@ func (r *ResourceAssert) ToTerraformTestCheckFunc(t *testing.T) resource.TestChe errCut, _ := strings.CutPrefix(err.Error(), fmt.Sprintf("%s: ", r.name)) result = append(result, fmt.Errorf("%s %s assertion [%d/%d]: failed with error: %s", r.name, r.prefix, i+1, len(r.assertions), errCut)) } + case resourceAssertionTypeValuePresent: + if err := resource.TestCheckResourceAttrSet(r.name, a.fieldName)(s); err != nil { + errCut, _ := strings.CutPrefix(err.Error(), fmt.Sprintf("%s: ", r.name)) + result = append(result, fmt.Errorf("%s %s assertion [%d/%d]: failed with error: %s", r.name, r.prefix, i+1, len(r.assertions), errCut)) + } } } @@ -171,6 +182,8 @@ func (r *ResourceAssert) ToTerraformImportStateCheckFunc(t *testing.T) resource. } case resourceAssertionTypeValueNotSet: panic("implement") + case resourceAssertionTypeValuePresent: + panic("implement") } } diff --git a/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/user_show_output_ext.go b/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/user_show_output_ext.go new file mode 100644 index 0000000000..9daf9a672f --- /dev/null +++ b/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/user_show_output_ext.go @@ -0,0 +1,25 @@ +package resourceshowoutputassert + +import ( + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" +) + +func (u *UserShowOutputAssert) HasCreatedOnNotEmpty() *UserShowOutputAssert { + u.AddAssertion(assert.ResourceShowOutputValuePresent("created_on")) + return u +} + +func (u *UserShowOutputAssert) HasDaysToExpiryNotEmpty() *UserShowOutputAssert { + u.AddAssertion(assert.ResourceShowOutputValuePresent("days_to_expiry")) + return u +} + +func (u *UserShowOutputAssert) HasMinsToUnlockNotEmpty() *UserShowOutputAssert { + u.AddAssertion(assert.ResourceShowOutputValuePresent("mins_to_unlock")) + return u +} + +func (u *UserShowOutputAssert) HasMinsToBypassMfaNotEmpty() *UserShowOutputAssert { + u.AddAssertion(assert.ResourceShowOutputValuePresent("mins_to_bypass_mfa")) + return u +} diff --git a/pkg/datasources/users_acceptance_test.go b/pkg/datasources/users_acceptance_test.go index 52f603227c..bd17cab69a 100644 --- a/pkg/datasources/users_acceptance_test.go +++ b/pkg/datasources/users_acceptance_test.go @@ -59,16 +59,25 @@ func TestAcc_Users_Complete(t *testing.T) { Config: config.FromModel(t, userModelAllAttributes) + datasourceWithLike(), Check: assert.AssertThat(t, assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.#", "1")), - - assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.name", id.Name())), - assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.show_output.0.created_on")), - assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.login_name", fmt.Sprintf("%s_LOGIN", id.Name()))), - assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.display_name", "Display Name")), - assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.first_name", "Jan")), - assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.show_output.0.last_name", "Testowski")), - resourceshowoutputassert.UsersDatasourceShowOutput(t, "snowflake_users.test"). - HasName(id.Name()), + HasName(id.Name()). + HasCreatedOnNotEmpty(). + HasLoginName(fmt.Sprintf("%s_LOGIN", id.Name())). + HasDisplayName("Display Name"). + HasFirstName("Jan"). + HasLastName("Testowski"). + HasEmail("fake@email.com"). + HasMustChangePassword(true). + HasDisabled(false). + HasDaysToExpiryNotEmpty(). + HasMinsToUnlockNotEmpty(). + HasDefaultWarehouse("some_warehouse"). + HasDefaultNamespace("some.namespace"). + HasDefaultRole("some_role"). + HasDefaultSecondaryRoles(`["ALL"]`). + HasMinsToBypassMfaNotEmpty(). + HasHasRsaPublicKey(true). + HasComment(comment), ), }, }, From 09ae8f621a25dca26188553a7cad30fb5da4d47f Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 13:55:39 +0200 Subject: [PATCH 07/42] Add assertions to parameters in users data source acc test --- .../users_datasource_parameters_ext.go | 18 ++++++++++++++++++ pkg/datasources/users_acceptance_test.go | 3 +++ 2 files changed, 21 insertions(+) create mode 100644 pkg/acceptance/bettertestspoc/assert/resourceparametersassert/users_datasource_parameters_ext.go diff --git a/pkg/acceptance/bettertestspoc/assert/resourceparametersassert/users_datasource_parameters_ext.go b/pkg/acceptance/bettertestspoc/assert/resourceparametersassert/users_datasource_parameters_ext.go new file mode 100644 index 0000000000..77e2af50b9 --- /dev/null +++ b/pkg/acceptance/bettertestspoc/assert/resourceparametersassert/users_datasource_parameters_ext.go @@ -0,0 +1,18 @@ +package resourceparametersassert + +import ( + "testing" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" +) + +// UsersDatasourceParameters is a temporary workaround to have better parameter assertions in data source acceptance tests. +func UsersDatasourceParameters(t *testing.T, name string) *UserResourceParametersAssert { + t.Helper() + + u := UserResourceParametersAssert{ + ResourceAssert: assert.NewDatasourceAssert("data."+name, "parameters", "users.0."), + } + u.AddAssertion(assert.ValueSet("parameters.#", "1")) + return &u +} diff --git a/pkg/datasources/users_acceptance_test.go b/pkg/datasources/users_acceptance_test.go index bd17cab69a..6989f05a6d 100644 --- a/pkg/datasources/users_acceptance_test.go +++ b/pkg/datasources/users_acceptance_test.go @@ -7,6 +7,7 @@ import ( acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceparametersassert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config/model" @@ -78,6 +79,8 @@ func TestAcc_Users_Complete(t *testing.T) { HasMinsToBypassMfaNotEmpty(). HasHasRsaPublicKey(true). HasComment(comment), + resourceparametersassert.UsersDatasourceParameters(t, "snowflake_users.test"). + HasAllDefaults(), ), }, }, From 71af1da23500aa7d11e1a69038082e35127f06a4 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 14:07:23 +0200 Subject: [PATCH 08/42] Check user details --- pkg/datasources/users_acceptance_test.go | 35 ++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/pkg/datasources/users_acceptance_test.go b/pkg/datasources/users_acceptance_test.go index 6989f05a6d..f04c74a0cc 100644 --- a/pkg/datasources/users_acceptance_test.go +++ b/pkg/datasources/users_acceptance_test.go @@ -24,8 +24,8 @@ func TestAcc_Users_Complete(t *testing.T) { comment := random.Comment() pass := random.Password() - key1, _ := random.GenerateRSAPublicKey(t) - key2, _ := random.GenerateRSAPublicKey(t) + key1, key1Fp := random.GenerateRSAPublicKey(t) + key2, key2Fp := random.GenerateRSAPublicKey(t) userModelAllAttributes := model.User("u", id.Name()). WithPassword(pass). @@ -70,6 +70,7 @@ func TestAcc_Users_Complete(t *testing.T) { HasEmail("fake@email.com"). HasMustChangePassword(true). HasDisabled(false). + HasSnowflakeLock(false). HasDaysToExpiryNotEmpty(). HasMinsToUnlockNotEmpty(). HasDefaultWarehouse("some_warehouse"). @@ -81,6 +82,36 @@ func TestAcc_Users_Complete(t *testing.T) { HasComment(comment), resourceparametersassert.UsersDatasourceParameters(t, "snowflake_users.test"). HasAllDefaults(), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.name", id.Name())), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.comment", comment)), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.display_name", "Display Name")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.login_name", fmt.Sprintf("%s_LOGIN", id.Name()))), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.first_name", "Jan")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.middle_name", "Jakub")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.last_name", "Testowski")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.email", "fake@email.com")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.password", "********")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.must_change_password", "true")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.disabled", "false")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.snowflake_lock", "false")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.snowflake_support", "false")), + assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.describe_output.0.days_to_expiry")), + assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.describe_output.0.mins_to_unlock")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.default_warehouse", "some_warehouse")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.default_namespace", "some.namespace")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.default_role", "some_role")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.default_secondary_roles", `["ALL"]`)), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.ext_authn_duo", "false")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.ext_authn_uid", "")), + assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.describe_output.0.mins_to_bypass_mfa")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.mins_to_bypass_network_policy", "0")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.rsa_public_key", key1)), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.rsa_public_key_fp", "SHA256:"+key1Fp)), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.rsa_public_key2", key2)), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.rsa_public_key2_fp", "SHA256:"+key2Fp)), + 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")), ), }, }, From e477ebf1d2275914ce35bc4f4c0aa31017fd47ff Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 14:26:42 +0200 Subject: [PATCH 09/42] Finish users datasource acceptance tests --- .../TestAcc_Users/without_user/test.tf | 10 ++ pkg/datasources/users_acceptance_test.go | 149 +++++++++++++++++- 2 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 pkg/datasources/testdata/TestAcc_Users/without_user/test.tf diff --git a/pkg/datasources/testdata/TestAcc_Users/without_user/test.tf b/pkg/datasources/testdata/TestAcc_Users/without_user/test.tf new file mode 100644 index 0000000000..622994fe7f --- /dev/null +++ b/pkg/datasources/testdata/TestAcc_Users/without_user/test.tf @@ -0,0 +1,10 @@ +data "snowflake_users" "test" { + like = "non-existing-user" + + lifecycle { + postcondition { + condition = length(self.users) > 0 + error_message = "there should be at least one user" + } + } +} diff --git a/pkg/datasources/users_acceptance_test.go b/pkg/datasources/users_acceptance_test.go index f04c74a0cc..1ea103e079 100644 --- a/pkg/datasources/users_acceptance_test.go +++ b/pkg/datasources/users_acceptance_test.go @@ -2,6 +2,8 @@ package datasources_test import ( "fmt" + "regexp" + "strings" "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" @@ -12,14 +14,12 @@ import ( "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config/model" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/tfversion" ) func TestAcc_Users_Complete(t *testing.T) { - _ = testenvs.GetOrSkipTest(t, testenvs.ConfigureClientOnce) id := acc.TestClient().Ids.RandomAccountObjectIdentifier() comment := random.Comment() @@ -27,6 +27,7 @@ func TestAcc_Users_Complete(t *testing.T) { key1, key1Fp := random.GenerateRSAPublicKey(t) key2, key2Fp := random.GenerateRSAPublicKey(t) + userModelNoAttributes := model.User("u", id.Name()) userModelAllAttributes := model.User("u", id.Name()). WithPassword(pass). WithLoginName(id.Name() + "_login"). @@ -114,6 +115,105 @@ func TestAcc_Users_Complete(t *testing.T) { assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.custom_landing_page_url_flush_next_ui_load", "false")), ), }, + { + Config: config.FromModel(t, userModelNoAttributes) + datasourceWithLike(), + Check: assert.AssertThat(t, + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.#", "1")), + resourceshowoutputassert.UsersDatasourceShowOutput(t, "snowflake_users.test"). + HasName(id.Name()). + HasCreatedOnNotEmpty(). + HasLoginName(strings.ToUpper(id.Name())). + HasDisplayName(""). + HasFirstName(""). + HasLastName(""). + HasEmail(""). + HasMustChangePassword(false). + HasDisabled(false). + HasSnowflakeLock(false). + HasDaysToExpiry(""). + HasMinsToUnlock(""). + HasDefaultWarehouse(""). + HasDefaultNamespace(""). + HasDefaultRole(""). + HasDefaultSecondaryRoles(""). + HasMinsToBypassMfa(""). + HasHasRsaPublicKey(false). + HasComment(""), + resourceparametersassert.UsersDatasourceParameters(t, "snowflake_users.test"). + HasAllDefaults(), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.name", id.Name())), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.comment", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.display_name", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.login_name", strings.ToUpper(id.Name()))), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.first_name", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.middle_name", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.last_name", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.email", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.password", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.must_change_password", "false")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.disabled", "false")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.snowflake_lock", "false")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.snowflake_support", "false")), + assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.describe_output.0.days_to_expiry")), + assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.describe_output.0.mins_to_unlock")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.default_warehouse", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.default_namespace", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.default_role", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.default_secondary_roles", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.ext_authn_duo", "false")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.ext_authn_uid", "")), + assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.describe_output.0.mins_to_bypass_mfa")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.mins_to_bypass_network_policy", "0")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.rsa_public_key", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.rsa_public_key_fp", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.rsa_public_key2", "")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.rsa_public_key2_fp", "")), + 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")), + ), + }, + }, + }) +} + +func TestAcc_Users_DifferentFiltering(t *testing.T) { + prefix := random.AlphaN(4) + id := acc.TestClient().Ids.RandomAccountObjectIdentifierWithPrefix(prefix) + id2 := acc.TestClient().Ids.RandomAccountObjectIdentifierWithPrefix(prefix) + id3 := acc.TestClient().Ids.RandomAccountObjectIdentifier() + + userModel := model.User("u", id.Name()) + user2Model := model.User("u2", id2.Name()) + user3Model := model.User("u3", id3.Name()) + + allUsersConfig := config.FromModel(t, userModel) + config.FromModel(t, user2Model) + config.FromModel(t, user3Model) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.User), + Steps: []resource.TestStep{ + { + Config: allUsersConfig + datasourceWithLikeMultipleUsers(), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.snowflake_users.test", "users.#", "1"), + ), + }, + { + Config: allUsersConfig + datasourceWithStartsWithMultipleUsers(prefix), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.snowflake_users.test", "users.#", "2"), + ), + }, + { + Config: allUsersConfig + datasourceWithLimitMultipleUsers(1, prefix), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.snowflake_users.test", "users.#", "1"), + ), + }, }, }) } @@ -125,3 +225,48 @@ func datasourceWithLike() string { } ` } + +func datasourceWithLikeMultipleUsers() string { + return ` + data "snowflake_users" "test" { + depends_on = [snowflake_user.u, snowflake_user.u2, snowflake_user.u3] + like = snowflake_user.u.name + } +` +} + +func datasourceWithStartsWithMultipleUsers(startsWith string) string { + return fmt.Sprintf(` + data "snowflake_users" "test" { + depends_on = [snowflake_user.u, snowflake_user.u2, snowflake_user.u3] + starts_with = "%s" + } +`, startsWith) +} + +func datasourceWithLimitMultipleUsers(rows int, from string) string { + return fmt.Sprintf(` + data "snowflake_users" "test" { + depends_on = [snowflake_user.u, snowflake_user.u2, snowflake_user.u3] + limit { + rows = %d + from = "%s" + } + } +`, rows, from) +} + +func TestAcc_Users_UserNotFound_WithPostConditions(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + Steps: []resource.TestStep{ + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Users/without_user"), + ExpectError: regexp.MustCompile("there should be at least one user"), + }, + }, + }) +} From fa7156e63d568b009e68e473882325115c91e2a3 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 14:36:04 +0200 Subject: [PATCH 10/42] Add users data source changes to the migration guide References: #2902 --- MIGRATION_GUIDE.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index eec752d556..90a84e5c2b 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -255,6 +255,19 @@ Type changes: 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 +Changes: +- account checking logic was entirely removed +- `pattern` renamed to `like` +- `like`, `starts_with`, and `limit` filters added +- `SHOW USERS` output is enclosed in `show_output` field inside `users` (all the previous fields in `users` map were removed) +- Added outputs from **DESC USER** and **SHOW PARAMETERS IN USER** (they can be turned off by declaring `with_describe = false` and `with_parameters = false`, **they're turned on by default**). + The additional parameters call **DESC USER** (with `with_describe` turned on) and **SHOW PARAMETERS IN USER** (with `with_parameters` turned on) **per user** returned by **SHOW USERS**. + The outputs of both commands are held in `users` entry, where **DESC USER** is saved in the `describe_output` field, and **SHOW PARAMETERS IN USER** in the `parameters` field. + It's important to limit the records and calls to Snowflake to the minimum. That's why we recommend assessing which information you need from the data source and then providing strong filters and turning off additional fields for better plan performance. + +Connected issues: [#2902](https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/2902) + ## v0.94.0 ➞ v0.94.1 ### changes in snowflake_schema From 1787d5efdbb854cd0d5cc0208ddbda15a2f9b451 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 14:41:09 +0200 Subject: [PATCH 11/42] Update users data source documentation --- docs/data-sources/users.md | 926 +++++++++++++++++- .../snowflake_users/data-source.tf | 76 +- templates/data-sources/users.md.tmpl | 24 + 3 files changed, 1015 insertions(+), 11 deletions(-) create mode 100644 templates/data-sources/users.md.tmpl diff --git a/docs/data-sources/users.md b/docs/data-sources/users.md index 6a3bc45366..3ba291158e 100644 --- a/docs/data-sources/users.md +++ b/docs/data-sources/users.md @@ -2,48 +2,958 @@ 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. --- -# snowflake_users (Data Source) +!> **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. ## Example Usage ```terraform -data "snowflake_users" "current" { - pattern = "user1" +# Simple usage +data "snowflake_users" "simple" { +} + +output "simple_output" { + value = data.snowflake_users.simple.users +} + +# Filtering (like) +data "snowflake_users" "like" { + like = "user-name" +} + +output "like_output" { + value = data.snowflake_users.like.users +} + +# Filtering (starts_with) +data "snowflake_users" "starts_with" { + starts_with = "user-" +} + +output "starts_with_output" { + value = data.snowflake_users.starts_with.users +} + +# Filtering (limit) +data "snowflake_users" "limit" { + limit { + rows = 10 + from = "user-" + } +} + +output "limit_output" { + value = data.snowflake_users.limit.users +} + +# Without additional data (to limit the number of calls make for every found user) +data "snowflake_users" "only_show" { + # with_describe is turned on by default and it calls DESCRIBE USER for every user found and attaches its output to users.*.describe_output field + with_describe = false + + # with_parameters is turned on by default and it calls SHOW PARAMETERS FOR USER for every user found and attaches its output to users.*.parameters field + with_parameters = false +} + +output "only_show_output" { + value = data.snowflake_users.only_show.users +} + +# Ensure the number of users is equal to at least one element (with the use of postcondition) +data "snowflake_users" "assert_with_postcondition" { + starts_with = "user-name" + lifecycle { + postcondition { + condition = length(self.users) > 0 + error_message = "there should be at least one user" + } + } +} + +# Ensure the number of users is equal to at exactly one element (with the use of check block) +check "user_check" { + data "snowflake_users" "assert_with_check_block" { + like = "user-name" + } + + assert { + condition = length(data.snowflake_users.assert_with_check_block.users) == 1 + error_message = "users filtered by '${data.snowflake_users.assert_with_check_block.like}' returned ${length(data.snowflake_users.assert_with_check_block.users)} users where one was expected" + } } ``` ## Schema -### Required +### Optional -- `pattern` (String) Users pattern for which to return metadata. Please refer to LIKE keyword from snowflake documentation : https://docs.snowflake.com/en/sql-reference/sql/show-users.html#parameters +- `like` (String) Filters the output with **case-insensitive** pattern, with support for SQL wildcard characters (`%` and `_`). +- `limit` (Block List, Max: 1) Limits the number of rows returned. If the `limit.from` is set, then the limit wll start from the first element matched by the expression. The expression is only used to match with the first element, later on the elements are not matched by the prefix, but you can enforce a certain pattern with `starts_with` or `like`. (see [below for nested schema](#nestedblock--limit)) +- `starts_with` (String) Filters the output with **case-sensitive** characters indicating the beginning of the object name. +- `with_describe` (Boolean) Runs DESC USER for each user returned by SHOW USERS. The output of describe is saved to the description field. By default this value is set to true. +- `with_parameters` (Boolean) Runs SHOW PARAMETERS FOR USER for each user returned by SHOW USERS. The output of describe is saved to the parameters field as a map. By default this value is set to true. ### Read-Only - `id` (String) The ID of this resource. -- `users` (List of Object) The users in the database (see [below for nested schema](#nestedatt--users)) +- `users` (List of Object) Holds the aggregated output of all user details queries. (see [below for nested schema](#nestedatt--users)) + + +### Nested Schema for `limit` + +Required: + +- `rows` (Number) The maximum number of rows to return. + +Optional: + +- `from` (String) Specifies a **case-sensitive** pattern that is used to match object name. After the first match, the limit on the number of rows will be applied. + ### Nested Schema for `users` Read-Only: +- `describe_output` (List of Object) (see [below for nested schema](#nestedobjatt--users--describe_output)) +- `parameters` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters)) +- `show_output` (List of Object) (see [below for nested schema](#nestedobjatt--users--show_output)) + + +### Nested Schema for `users.describe_output` + +Read-Only: + +- `comment` (String) +- `custom_landing_page_url` (String) +- `custom_landing_page_url_flush_next_ui_load` (Boolean) +- `days_to_expiry` (Number) +- `default_namespace` (String) +- `default_role` (String) +- `default_secondary_roles` (String) +- `default_warehouse` (String) +- `disabled` (Boolean) +- `display_name` (String) +- `email` (String) +- `ext_authn_duo` (Boolean) +- `ext_authn_uid` (String) +- `first_name` (String) +- `last_name` (String) +- `login_name` (String) +- `middle_name` (String) +- `mins_to_bypass_mfa` (Number) +- `mins_to_bypass_network_policy` (Number) +- `mins_to_unlock` (Number) +- `must_change_password` (Boolean) +- `name` (String) +- `password` (String) +- `password_last_set_time` (String) +- `rsa_public_key` (String) +- `rsa_public_key2` (String) +- `rsa_public_key2_fp` (String) +- `rsa_public_key_fp` (String) +- `snowflake_lock` (Boolean) +- `snowflake_support` (Boolean) + + + +### Nested Schema for `users.parameters` + +Read-Only: + +- `abort_detached_query` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--abort_detached_query)) +- `autocommit` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--autocommit)) +- `binary_input_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--binary_input_format)) +- `binary_output_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--binary_output_format)) +- `client_memory_limit` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--client_memory_limit)) +- `client_metadata_request_use_connection_ctx` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--client_metadata_request_use_connection_ctx)) +- `client_prefetch_threads` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--client_prefetch_threads)) +- `client_result_chunk_size` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--client_result_chunk_size)) +- `client_result_column_case_insensitive` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--client_result_column_case_insensitive)) +- `client_session_keep_alive` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--client_session_keep_alive)) +- `client_session_keep_alive_heartbeat_frequency` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--client_session_keep_alive_heartbeat_frequency)) +- `client_timestamp_type_mapping` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--client_timestamp_type_mapping)) +- `date_input_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--date_input_format)) +- `date_output_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--date_output_format)) +- `enable_unload_physical_type_optimization` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--enable_unload_physical_type_optimization)) +- `enable_unredacted_query_syntax_error` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--enable_unredacted_query_syntax_error)) +- `error_on_nondeterministic_merge` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--error_on_nondeterministic_merge)) +- `error_on_nondeterministic_update` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--error_on_nondeterministic_update)) +- `geography_output_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--geography_output_format)) +- `geometry_output_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--geometry_output_format)) +- `jdbc_treat_decimal_as_int` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--jdbc_treat_decimal_as_int)) +- `jdbc_treat_timestamp_ntz_as_utc` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--jdbc_treat_timestamp_ntz_as_utc)) +- `jdbc_use_session_timezone` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--jdbc_use_session_timezone)) +- `json_indent` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--json_indent)) +- `lock_timeout` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--lock_timeout)) +- `log_level` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--log_level)) +- `multi_statement_count` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--multi_statement_count)) +- `network_policy` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--network_policy)) +- `noorder_sequence_as_default` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--noorder_sequence_as_default)) +- `odbc_treat_decimal_as_int` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--odbc_treat_decimal_as_int)) +- `prevent_unload_to_internal_stages` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--prevent_unload_to_internal_stages)) +- `query_tag` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--query_tag)) +- `quoted_identifiers_ignore_case` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--quoted_identifiers_ignore_case)) +- `rows_per_resultset` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--rows_per_resultset)) +- `s3_stage_vpce_dns_name` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--s3_stage_vpce_dns_name)) +- `search_path` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--search_path)) +- `simulated_data_sharing_consumer` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--simulated_data_sharing_consumer)) +- `statement_queued_timeout_in_seconds` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--statement_queued_timeout_in_seconds)) +- `statement_timeout_in_seconds` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--statement_timeout_in_seconds)) +- `strict_json_output` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--strict_json_output)) +- `time_input_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--time_input_format)) +- `time_output_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--time_output_format)) +- `timestamp_day_is_always_24h` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--timestamp_day_is_always_24h)) +- `timestamp_input_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--timestamp_input_format)) +- `timestamp_ltz_output_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--timestamp_ltz_output_format)) +- `timestamp_ntz_output_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--timestamp_ntz_output_format)) +- `timestamp_output_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--timestamp_output_format)) +- `timestamp_type_mapping` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--timestamp_type_mapping)) +- `timestamp_tz_output_format` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--timestamp_tz_output_format)) +- `timezone` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--timezone)) +- `trace_level` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--trace_level)) +- `transaction_abort_on_error` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--transaction_abort_on_error)) +- `transaction_default_isolation_level` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--transaction_default_isolation_level)) +- `two_digit_century_start` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--two_digit_century_start)) +- `unsupported_ddl_action` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--unsupported_ddl_action)) +- `use_cached_result` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--use_cached_result)) +- `week_of_year_policy` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--week_of_year_policy)) +- `week_start` (List of Object) (see [below for nested schema](#nestedobjatt--users--parameters--week_start)) + + +### Nested Schema for `users.parameters.abort_detached_query` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.autocommit` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.binary_input_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.binary_output_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.client_memory_limit` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.client_metadata_request_use_connection_ctx` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.client_prefetch_threads` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.client_result_chunk_size` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.client_result_column_case_insensitive` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.client_session_keep_alive` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.client_session_keep_alive_heartbeat_frequency` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.client_timestamp_type_mapping` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.date_input_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.date_output_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.enable_unload_physical_type_optimization` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.enable_unredacted_query_syntax_error` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.error_on_nondeterministic_merge` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.error_on_nondeterministic_update` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.geography_output_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.geometry_output_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.jdbc_treat_decimal_as_int` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.jdbc_treat_timestamp_ntz_as_utc` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.jdbc_use_session_timezone` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.json_indent` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.lock_timeout` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.log_level` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.multi_statement_count` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.network_policy` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.noorder_sequence_as_default` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.odbc_treat_decimal_as_int` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.prevent_unload_to_internal_stages` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.query_tag` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.quoted_identifiers_ignore_case` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.rows_per_resultset` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.s3_stage_vpce_dns_name` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.search_path` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.simulated_data_sharing_consumer` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.statement_queued_timeout_in_seconds` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.statement_timeout_in_seconds` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.strict_json_output` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.time_input_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.time_output_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.timestamp_day_is_always_24h` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.timestamp_input_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.timestamp_ltz_output_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.timestamp_ntz_output_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.timestamp_output_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.timestamp_type_mapping` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.timestamp_tz_output_format` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.timezone` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.trace_level` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.transaction_abort_on_error` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.transaction_default_isolation_level` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.two_digit_century_start` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.unsupported_ddl_action` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.use_cached_result` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.week_of_year_policy` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + +### Nested Schema for `users.parameters.week_start` + +Read-Only: + +- `default` (String) +- `description` (String) +- `key` (String) +- `level` (String) +- `value` (String) + + + + +### Nested Schema for `users.show_output` + +Read-Only: + - `comment` (String) +- `created_on` (String) +- `days_to_expiry` (String) - `default_namespace` (String) - `default_role` (String) -- `default_secondary_roles` (Set of String) +- `default_secondary_roles` (String) - `default_warehouse` (String) - `disabled` (Boolean) - `display_name` (String) - `email` (String) +- `expires_at_time` (String) +- `ext_authn_duo` (Boolean) +- `ext_authn_uid` (String) - `first_name` (String) +- `has_mfa` (Boolean) +- `has_password` (Boolean) - `has_rsa_public_key` (Boolean) - `last_name` (String) +- `last_success_login` (String) +- `locked_until_time` (String) - `login_name` (String) +- `mins_to_bypass_mfa` (String) +- `mins_to_unlock` (String) +- `must_change_password` (Boolean) - `name` (String) +- `owner` (String) +- `snowflake_lock` (Boolean) +- `type` (String) diff --git a/examples/data-sources/snowflake_users/data-source.tf b/examples/data-sources/snowflake_users/data-source.tf index 117dd0d224..1cce26ac20 100644 --- a/examples/data-sources/snowflake_users/data-source.tf +++ b/examples/data-sources/snowflake_users/data-source.tf @@ -1,3 +1,73 @@ -data "snowflake_users" "current" { - pattern = "user1" -} \ No newline at end of file +# Simple usage +data "snowflake_users" "simple" { +} + +output "simple_output" { + value = data.snowflake_users.simple.users +} + +# Filtering (like) +data "snowflake_users" "like" { + like = "user-name" +} + +output "like_output" { + value = data.snowflake_users.like.users +} + +# Filtering (starts_with) +data "snowflake_users" "starts_with" { + starts_with = "user-" +} + +output "starts_with_output" { + value = data.snowflake_users.starts_with.users +} + +# Filtering (limit) +data "snowflake_users" "limit" { + limit { + rows = 10 + from = "user-" + } +} + +output "limit_output" { + value = data.snowflake_users.limit.users +} + +# Without additional data (to limit the number of calls make for every found user) +data "snowflake_users" "only_show" { + # with_describe is turned on by default and it calls DESCRIBE USER for every user found and attaches its output to users.*.describe_output field + with_describe = false + + # with_parameters is turned on by default and it calls SHOW PARAMETERS FOR USER for every user found and attaches its output to users.*.parameters field + with_parameters = false +} + +output "only_show_output" { + value = data.snowflake_users.only_show.users +} + +# Ensure the number of users is equal to at least one element (with the use of postcondition) +data "snowflake_users" "assert_with_postcondition" { + starts_with = "user-name" + lifecycle { + postcondition { + condition = length(self.users) > 0 + error_message = "there should be at least one user" + } + } +} + +# Ensure the number of users is equal to at exactly one element (with the use of check block) +check "user_check" { + data "snowflake_users" "assert_with_check_block" { + like = "user-name" + } + + assert { + condition = length(data.snowflake_users.assert_with_check_block.users) == 1 + error_message = "users filtered by '${data.snowflake_users.assert_with_check_block.like}' returned ${length(data.snowflake_users.assert_with_check_block.users)} users where one was expected" + } +} diff --git a/templates/data-sources/users.md.tmpl b/templates/data-sources/users.md.tmpl new file mode 100644 index 0000000000..d3ff8d9c6c --- /dev/null +++ b/templates/data-sources/users.md.tmpl @@ -0,0 +1,24 @@ +--- +page_title: "{{.Name}} {{.Type}} - {{.ProviderName}}" +subcategory: "" +description: |- +{{ if gt (len (split .Description "")) 1 -}} +{{ index (split .Description "") 1 | plainmarkdown | trimspace | prefixlines " " }} +{{- else -}} +{{ .Description | plainmarkdown | trimspace | prefixlines " " }} +{{- end }} +--- + +!> **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. + +# {{.Name}} ({{.Type}}) + +{{ .Description | trimspace }} + +{{ if .HasExample -}} +## Example Usage + +{{ tffile (printf "examples/data-sources/%s/data-source.tf" .Name)}} +{{- end }} + +{{ .SchemaMarkdown | trimspace }} From 06438510ca5469e96f37f2831fadc18148741c2b Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 17:02:29 +0200 Subject: [PATCH 12/42] Fix failing test --- pkg/resources/database_acceptance_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/resources/database_acceptance_test.go b/pkg/resources/database_acceptance_test.go index 9debdb1a3a..ee0efcf51e 100644 --- a/pkg/resources/database_acceptance_test.go +++ b/pkg/resources/database_acceptance_test.go @@ -815,7 +815,7 @@ func TestAcc_Database_IntParameter(t *testing.T) { }, Check: assert.AssertThat(t, assert.Check(resource.TestCheckResourceAttr("snowflake_database.test", "data_retention_time_in_days", "1")), - objectparametersassert.DatabaseParameters(t, id).HasDataRetentionTimeInDays(25).HasDataRetentionTimeInDaysLevel(sdk.ParameterTypeDatabase), + objectparametersassert.DatabaseParameters(t, id).HasDataRetentionTimeInDays(1).HasDataRetentionTimeInDaysLevel(sdk.ParameterTypeDatabase), ), }, // remove the param from config From 728350fe5d00e8fb210eca7a766c5a3362a6d4de Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 17:27:30 +0200 Subject: [PATCH 13/42] fix custom diffs for fields with diff supression starts here From f0ea06411ffb67775dc7bd4e0f895b43d1921261 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 17:47:17 +0200 Subject: [PATCH 14/42] Prepare new custom diff function taking the diff suppress into consideration --- pkg/resources/custom_diffs.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index d495e7cd93..b516ef567b 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -97,7 +97,33 @@ func ComputedIfAnyAttributeChanged(key string, changedAttributeKeys ...string) s }) } -// TODO(SNOW-1629468): Adjust the function to make it more flexible +func ComputedIfAnyAttributeChangedCheckingDiffSuppression(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) { + oldValue, newValue := diff.GetChange(changedKey) + 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 { + if !diffSuppressFunc(key, oldValue.(string), newValue.(string), nil) { + 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 result + }) +} + 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 { From 31b17e7bc7281746275750588d2d9732c173e3d2 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 18:10:33 +0200 Subject: [PATCH 15/42] Use the improved computed if any attribute changed implementation --- pkg/resources/account.go | 2 +- pkg/resources/account_role.go | 5 ++--- ...ntegration_with_authorization_code_grant.go | 4 ++-- ...tion_integration_with_client_credentials.go | 4 ++-- ...thentication_integration_with_jwt_bearer.go | 4 ++-- pkg/resources/custom_diffs.go | 18 +++--------------- pkg/resources/database.go | 2 +- pkg/resources/database_role.go | 3 +-- pkg/resources/external_oauth_integration.go | 4 ++-- pkg/resources/file_format.go | 2 +- pkg/resources/function.go | 2 +- pkg/resources/masking_policy.go | 2 +- pkg/resources/materialized_view.go | 2 +- pkg/resources/network_policy.go | 4 +++- .../oauth_integration_for_custom_clients.go | 2 ++ ...uth_integration_for_partner_applications.go | 2 ++ pkg/resources/password_policy.go | 2 +- pkg/resources/procedure.go | 2 +- pkg/resources/saml2_integration.go | 4 ++-- pkg/resources/schema.go | 9 ++++----- pkg/resources/scim_integration.go | 4 ++-- pkg/resources/secondary_database.go | 2 +- pkg/resources/shared_database.go | 2 +- pkg/resources/streamlit.go | 7 +++---- pkg/resources/table.go | 2 +- pkg/resources/user.go | 8 ++++---- pkg/resources/view.go | 4 ++-- pkg/resources/warehouse.go | 7 +++---- 28 files changed, 52 insertions(+), 63 deletions(-) diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 2f41737bdd..0fef63ce97 100644 --- a/pkg/resources/account.go +++ b/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, diff --git a/pkg/resources/account_role.go b/pkg/resources/account_role.go index e7cc6f7fcf..28922572db 100644 --- a/pkg/resources/account_role.go +++ b/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{ diff --git a/pkg/resources/api_authentication_integration_with_authorization_code_grant.go b/pkg/resources/api_authentication_integration_with_authorization_code_grant.go index a1fb3df121..c2086e4608 100644 --- a/pkg/resources/api_authentication_integration_with_authorization_code_grant.go +++ b/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"), ), diff --git a/pkg/resources/api_authentication_integration_with_client_credentials.go b/pkg/resources/api_authentication_integration_with_client_credentials.go index 53231d8cf3..c98039dcfc 100644 --- a/pkg/resources/api_authentication_integration_with_client_credentials.go +++ b/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{ diff --git a/pkg/resources/api_authentication_integration_with_jwt_bearer.go b/pkg/resources/api_authentication_integration_with_jwt_bearer.go index 23c4a68ee5..b3a508cb46 100644 --- a/pkg/resources/api_authentication_integration_with_jwt_bearer.go +++ b/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"), ), diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index b516ef567b..01b30fd31b 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -83,21 +83,8 @@ func ForceNewIfChangeToEmptyString(key string) schema.CustomizeDiffFunc { }) } -func ComputedIfAnyAttributeChanged(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 - }) -} - -func ComputedIfAnyAttributeChangedCheckingDiffSuppression(resourceSchema map[string]*schema.Schema, key string, changedAttributeKeys ...string) schema.CustomizeDiffFunc { +// TODO: test +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 { @@ -124,6 +111,7 @@ func ComputedIfAnyAttributeChangedCheckingDiffSuppression(resourceSchema map[str }) } +// TODO: remove 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 { diff --git a/pkg/resources/database.go b/pkg/resources/database.go index ec89d89db9..80923faaf2 100644 --- a/pkg/resources/database.go +++ b/pkg/resources/database.go @@ -101,7 +101,7 @@ func Database() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(databaseSchema, FullyQualifiedNameAttributeName, "name"), databaseParametersCustomDiff, ), diff --git a/pkg/resources/database_role.go b/pkg/resources/database_role.go index ee98900a7b..c070152ea9 100644 --- a/pkg/resources/database_role.go +++ b/pkg/resources/database_role.go @@ -60,8 +60,7 @@ func DatabaseRole() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment"), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(databaseRoleSchema, ShowOutputAttributeName, "comment", "name"), ), SchemaVersion: 1, diff --git a/pkg/resources/external_oauth_integration.go b/pkg/resources/external_oauth_integration.go index c8b7ffbead..b454e3fa67 100644 --- a/pkg/resources/external_oauth_integration.go +++ b/pkg/resources/external_oauth_integration.go @@ -167,8 +167,8 @@ func ExternalOauthIntegration() *schema.Resource { ForceNewIfChangeToEmptyString("external_oauth_scope_mapping_attribute"), ForceNewIfChangeToEmptySet("external_oauth_jws_keys_url"), ForceNewIfChangeToEmptySet("external_oauth_token_user_mapping_claim"), - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "external_oauth_type", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "external_oauth_issuer", "external_oauth_jws_keys_url", "external_oauth_any_role_mode", + ComputedIfAnyAttributeChanged(externalOauthIntegrationSchema, ShowOutputAttributeName, "enabled", "external_oauth_type", "comment"), + ComputedIfAnyAttributeChanged(externalOauthIntegrationSchema, DescribeOutputAttributeName, "enabled", "external_oauth_issuer", "external_oauth_jws_keys_url", "external_oauth_any_role_mode", "external_oauth_rsa_public_key", "external_oauth_rsa_public_key_2", "external_oauth_blocked_roles_list", "external_oauth_allowed_roles_list", "external_oauth_audience_list", "external_oauth_token_user_mapping_claim", "external_oauth_snowflake_user_mapping_attribute", "external_oauth_scope_delimiter", "comment"), diff --git a/pkg/resources/file_format.go b/pkg/resources/file_format.go index adc751fca6..35b2a08205 100644 --- a/pkg/resources/file_format.go +++ b/pkg/resources/file_format.go @@ -319,7 +319,7 @@ func FileFormat() *schema.Resource { Delete: DeleteFileFormat, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(fileFormatSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: fileFormatSchema, diff --git a/pkg/resources/function.go b/pkg/resources/function.go index e9f1b43c53..40709a0feb 100644 --- a/pkg/resources/function.go +++ b/pkg/resources/function.go @@ -173,7 +173,7 @@ func Function() *schema.Resource { CustomizeDiff: customdiff.All( // TODO(SNOW-1348103): add `arguments` to ComputedIfAnyAttributeChanged. This can't be done now because this function compares values without diff suppress. - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(functionSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: functionSchema, diff --git a/pkg/resources/masking_policy.go b/pkg/resources/masking_policy.go index b962f8cf81..f10a1321c9 100644 --- a/pkg/resources/masking_policy.go +++ b/pkg/resources/masking_policy.go @@ -122,7 +122,7 @@ func MaskingPolicy() *schema.Resource { Delete: DeleteMaskingPolicy, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(maskingPolicySchema, FullyQualifiedNameAttributeName, "name"), ), Schema: maskingPolicySchema, diff --git a/pkg/resources/materialized_view.go b/pkg/resources/materialized_view.go index e280150418..a280aef202 100644 --- a/pkg/resources/materialized_view.go +++ b/pkg/resources/materialized_view.go @@ -76,7 +76,7 @@ func MaterializedView() *schema.Resource { Delete: DeleteMaterializedView, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(materializedViewSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: materializedViewSchema, diff --git a/pkg/resources/network_policy.go b/pkg/resources/network_policy.go index d04b44bf86..8283f22b63 100644 --- a/pkg/resources/network_policy.go +++ b/pkg/resources/network_policy.go @@ -99,6 +99,7 @@ func NetworkPolicy() *schema.Resource { // for complex types like Sets, Lists, and Maps. When every element of the Set is suppressed in custom diff, // it returns true for d.HasChange anyway (it returns false for suppressed changes on primitive types like Number, Bool, String, etc.). ComputedIfAnyAttributeChanged( + networkPolicySchema, ShowOutputAttributeName, // "allowed_network_rule_list", // "blocked_network_rule_list", @@ -107,13 +108,14 @@ func NetworkPolicy() *schema.Resource { "comment", ), ComputedIfAnyAttributeChanged( + networkPolicySchema, DescribeOutputAttributeName, // "allowed_network_rule_list", // "blocked_network_rule_list", "allowed_ip_list", "blocked_ip_list", ), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(networkPolicySchema, FullyQualifiedNameAttributeName, "name"), ), Importer: &schema.ResourceImporter{ diff --git a/pkg/resources/oauth_integration_for_custom_clients.go b/pkg/resources/oauth_integration_for_custom_clients.go index 7d7828626b..4462651bf2 100644 --- a/pkg/resources/oauth_integration_for_custom_clients.go +++ b/pkg/resources/oauth_integration_for_custom_clients.go @@ -161,11 +161,13 @@ func OauthIntegrationForCustomClients() *schema.Resource { CustomizeDiff: customdiff.All( ComputedIfAnyAttributeChanged( + oauthIntegrationForCustomClientsSchema, ShowOutputAttributeName, "enabled", "comment", ), ComputedIfAnyAttributeChanged( + oauthIntegrationForCustomClientsSchema, DescribeOutputAttributeName, "oauth_client_type", "oauth_redirect_uri", diff --git a/pkg/resources/oauth_integration_for_partner_applications.go b/pkg/resources/oauth_integration_for_partner_applications.go index de031dab6c..d92d9d4dee 100644 --- a/pkg/resources/oauth_integration_for_partner_applications.go +++ b/pkg/resources/oauth_integration_for_partner_applications.go @@ -121,11 +121,13 @@ func OauthIntegrationForPartnerApplications() *schema.Resource { CustomizeDiff: customdiff.All( ComputedIfAnyAttributeChanged( + oauthIntegrationForPartnerApplicationsSchema, ShowOutputAttributeName, "enabled", "comment", ), ComputedIfAnyAttributeChanged( + oauthIntegrationForPartnerApplicationsSchema, DescribeOutputAttributeName, "oauth_client", "enabled", diff --git a/pkg/resources/password_policy.go b/pkg/resources/password_policy.go index 91ebb5b03b..0aa5aaff98 100644 --- a/pkg/resources/password_policy.go +++ b/pkg/resources/password_policy.go @@ -145,7 +145,7 @@ func PasswordPolicy() *schema.Resource { Delete: DeletePasswordPolicy, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(passwordPolicySchema, FullyQualifiedNameAttributeName, "name"), ), Schema: passwordPolicySchema, diff --git a/pkg/resources/procedure.go b/pkg/resources/procedure.go index 295f3a1b67..f1337acd2b 100644 --- a/pkg/resources/procedure.go +++ b/pkg/resources/procedure.go @@ -189,7 +189,7 @@ func Procedure() *schema.Resource { // TODO(SNOW-1348106): add `arguments` to ComputedIfAnyAttributeChanged for FullyQualifiedNameAttributeName. // This can't be done now because this function compares values without diff suppress. CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(procedureSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: procedureSchema, diff --git a/pkg/resources/saml2_integration.go b/pkg/resources/saml2_integration.go index 3939d2249a..6712fd23fe 100644 --- a/pkg/resources/saml2_integration.go +++ b/pkg/resources/saml2_integration.go @@ -172,8 +172,8 @@ func SAML2Integration() *schema.Resource { ForceNewIfChangeToEmptyString("saml2_snowflake_issuer_url"), ForceNewIfChangeToEmptyString("saml2_snowflake_acs_url"), ForceNewIfChangeToEmptyString("saml2_sp_initiated_login_page_label"), - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "saml2_issuer", "saml2_sso_url", "saml2_provider", "saml2_x509_cert", + ComputedIfAnyAttributeChanged(saml2IntegrationSchema, ShowOutputAttributeName, "enabled", "comment"), + ComputedIfAnyAttributeChanged(saml2IntegrationSchema, DescribeOutputAttributeName, "saml2_issuer", "saml2_sso_url", "saml2_provider", "saml2_x509_cert", "saml2_sp_initiated_login_page_label", "saml2_enable_sp_initiated", "saml2_sign_request", "saml2_requtedted_nameid_format", "saml2_post_logout_redirect_url", "saml2_force_authn", "saml2_snowflake_issuer_url", "saml2_snowflake_acs_url", "allowed_user_domains", "allowed_email_patterns"), diff --git a/pkg/resources/schema.go b/pkg/resources/schema.go index 55f54d02cf..de3c4e2e48 100644 --- a/pkg/resources/schema.go +++ b/pkg/resources/schema.go @@ -96,11 +96,10 @@ func Schema() *schema.Resource { Description: "Resource used to manage schema objects. For more information, check [schema documentation](https://docs.snowflake.com/en/sql-reference/sql/create-schema).", CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment", "with_managed_access", "is_transient"), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChangedWithSuppressDiff(DescribeOutputAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChanged(ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllSchemaParameters), strings.ToLower)...), + ComputedIfAnyAttributeChanged(schemaSchema, ShowOutputAttributeName, "name", "comment", "with_managed_access", "is_transient"), + ComputedIfAnyAttributeChanged(schemaSchema, DescribeOutputAttributeName, "name"), + ComputedIfAnyAttributeChanged(schemaSchema, FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(schemaParametersSchema, ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllSchemaParameters), strings.ToLower)...), schemaParametersCustomDiff, ), diff --git a/pkg/resources/scim_integration.go b/pkg/resources/scim_integration.go index fab3d51ae0..8ca1aeedb2 100644 --- a/pkg/resources/scim_integration.go +++ b/pkg/resources/scim_integration.go @@ -109,8 +109,8 @@ func SCIMIntegration() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "scim_client", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "comment", "network_policy", "run_as_role", "sync_password"), + ComputedIfAnyAttributeChanged(scimIntegrationSchema, ShowOutputAttributeName, "enabled", "scim_client", "comment"), + ComputedIfAnyAttributeChanged(scimIntegrationSchema, DescribeOutputAttributeName, "enabled", "comment", "network_policy", "run_as_role", "sync_password"), ), SchemaVersion: 2, diff --git a/pkg/resources/secondary_database.go b/pkg/resources/secondary_database.go index 361db4ab48..155699d1c9 100644 --- a/pkg/resources/secondary_database.go +++ b/pkg/resources/secondary_database.go @@ -53,7 +53,7 @@ func SecondaryDatabase() *schema.Resource { CustomizeDiff: customdiff.All( databaseParametersCustomDiff, - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(secondaryDatabaseSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: helpers.MergeMaps(secondaryDatabaseSchema, databaseParametersSchema), Importer: &schema.ResourceImporter{ diff --git a/pkg/resources/shared_database.go b/pkg/resources/shared_database.go index b6da4eaf36..8339148656 100644 --- a/pkg/resources/shared_database.go +++ b/pkg/resources/shared_database.go @@ -53,7 +53,7 @@ func SharedDatabase() *schema.Resource { Description: "A shared database creates a database from a share provided by another Snowflake account. For more information about shares, see [Introduction to Secure Data Sharing](https://docs.snowflake.com/en/user-guide/data-sharing-intro).", CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(sharedDatabaseSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: helpers.MergeMaps(sharedDatabaseSchema, sharedDatabaseParametersSchema), diff --git a/pkg/resources/streamlit.go b/pkg/resources/streamlit.go index e386f10f6c..830e7c0924 100644 --- a/pkg/resources/streamlit.go +++ b/pkg/resources/streamlit.go @@ -118,10 +118,9 @@ func Streamlit() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "title", "comment", "query_warehouse"), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "title", "comment", "root_location", "main_file", "query_warehouse", "external_access_integrations"), + ComputedIfAnyAttributeChanged(streamlitSchema, ShowOutputAttributeName, "name", "title", "comment", "query_warehouse"), + ComputedIfAnyAttributeChanged(streamlitSchema, FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(streamlitSchema, DescribeOutputAttributeName, "title", "comment", "root_location", "main_file", "query_warehouse", "external_access_integrations"), ), SchemaVersion: 1, diff --git a/pkg/resources/table.go b/pkg/resources/table.go index ad10836b74..100d89cffa 100644 --- a/pkg/resources/table.go +++ b/pkg/resources/table.go @@ -212,7 +212,7 @@ func Table() *schema.Resource { Delete: DeleteTable, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(tableSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: tableSchema, diff --git a/pkg/resources/user.go b/pkg/resources/user.go index 05108193e4..58af3bae16 100644 --- a/pkg/resources/user.go +++ b/pkg/resources/user.go @@ -199,10 +199,10 @@ func User() *schema.Resource { }, CustomizeDiff: customdiff.All( - // TODO [SNOW-1629468 - next pr]: handle diff suppression correctly; add "default_role", "default_secondary_roles" - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "password", "login_name", "display_name", "first_name", "middle_name", "last_name", "email", "must_change_password", "disabled", "days_to_expiry", "mins_to_unlock", "default_warehouse", "default_namespace", "mins_to_bypass_mfa", "rsa_public_key", "rsa_public_key_2", "comment", "disable_mfa"), - ComputedIfAnyAttributeChanged(ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllUserParameters), strings.ToLower)...), - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + // TODO [SNOW-1629468 - next pr]: test "default_role", "default_secondary_roles" + ComputedIfAnyAttributeChanged(userSchema, ShowOutputAttributeName, "password", "login_name", "display_name", "first_name", "middle_name", "last_name", "email", "must_change_password", "disabled", "days_to_expiry", "mins_to_unlock", "default_warehouse", "default_namespace", "default_role", "default_secondary_roles", "mins_to_bypass_mfa", "rsa_public_key", "rsa_public_key_2", "comment", "disable_mfa"), + ComputedIfAnyAttributeChanged(userParametersSchema, ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllUserParameters), strings.ToLower)...), + ComputedIfAnyAttributeChanged(userSchema, FullyQualifiedNameAttributeName, "name"), userParametersCustomDiff, // TODO [SNOW-1645348]: revisit with service user work func(_ context.Context, diff *schema.ResourceDiff, _ interface{}) error { diff --git a/pkg/resources/view.go b/pkg/resources/view.go index e9e929f2da..5090e2b9ee 100644 --- a/pkg/resources/view.go +++ b/pkg/resources/view.go @@ -280,8 +280,8 @@ func View() *schema.Resource { Description: "Resource used to manage view objects. For more information, check [view documentation](https://docs.snowflake.com/en/sql-reference/sql/create-view).", CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment", "change_tracking", "is_secure", "is_temporary", "is_recursive", "statement"), - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(viewSchema, ShowOutputAttributeName, "comment", "change_tracking", "is_secure", "is_temporary", "is_recursive", "statement"), + ComputedIfAnyAttributeChanged(viewSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: viewSchema, diff --git a/pkg/resources/warehouse.go b/pkg/resources/warehouse.go index 9623cd947b..6232b0bac1 100644 --- a/pkg/resources/warehouse.go +++ b/pkg/resources/warehouse.go @@ -204,10 +204,9 @@ func Warehouse() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "warehouse_type", "warehouse_size", "max_cluster_count", "min_cluster_count", "scaling_policy", "auto_suspend", "auto_resume", "comment", "enable_query_acceleration", "query_acceleration_max_scale_factor"), - ComputedIfAnyAttributeChanged(ParametersAttributeName, strings.ToLower(string(sdk.ObjectParameterMaxConcurrencyLevel)), strings.ToLower(string(sdk.ObjectParameterStatementQueuedTimeoutInSeconds)), strings.ToLower(string(sdk.ObjectParameterStatementTimeoutInSeconds))), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name", "resource_monitor"), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(warehouseSchema, ShowOutputAttributeName, "name", "warehouse_type", "warehouse_size", "max_cluster_count", "min_cluster_count", "scaling_policy", "auto_suspend", "auto_resume", "resource_monitor", "comment", "enable_query_acceleration", "query_acceleration_max_scale_factor"), + ComputedIfAnyAttributeChanged(warehouseSchema, ParametersAttributeName, strings.ToLower(string(sdk.ObjectParameterMaxConcurrencyLevel)), strings.ToLower(string(sdk.ObjectParameterStatementQueuedTimeoutInSeconds)), strings.ToLower(string(sdk.ObjectParameterStatementTimeoutInSeconds))), + ComputedIfAnyAttributeChanged(warehouseSchema, FullyQualifiedNameAttributeName, "name"), customdiff.ForceNewIfChange("warehouse_size", func(ctx context.Context, old, new, meta any) bool { return old.(string) != "" && new.(string) == "" From c54ad5e751f516666e7877ba6b890f7df2c5b04e Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 18:34:09 +0200 Subject: [PATCH 16/42] Unit test improved computed if any attribute changed --- pkg/resources/custom_diffs.go | 16 ---- pkg/resources/custom_diffs_test.go | 149 ++++++++++++++++++++--------- 2 files changed, 103 insertions(+), 62 deletions(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index 01b30fd31b..d31a012ab8 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -111,22 +111,6 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key }) } -// TODO: remove -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 - } - } - } - return false - }) -} - type parameter[T ~string] struct { parameterName T valueType valueType diff --git a/pkg/resources/custom_diffs_test.go b/pkg/resources/custom_diffs_test.go index 5bd28733aa..ca847e492b 100644 --- a/pkg/resources/custom_diffs_test.go +++ b/pkg/resources/custom_diffs_test.go @@ -345,10 +345,40 @@ func TestForceNewIfChangeToEmptySet(t *testing.T) { } } -func TestComputedIfAnyAttributeChangedWithSuppressDiff(t *testing.T) { - suppressFunc := schema.SchemaDiffSuppressFunc(func(k, oldValue, newValue string, d *schema.ResourceData) bool { +func Test_ComputedIfAnyAttributeChanged(t *testing.T) { + testSuppressFunc := schema.SchemaDiffSuppressFunc(func(k, oldValue, newValue string, d *schema.ResourceData) bool { return strings.Trim(oldValue, `"`) == strings.Trim(newValue, `"`) }) + testSchema := map[string]*schema.Schema{ + "value_with_diff_suppress": { + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: testSuppressFunc, + }, + "value_without_diff_suppress": { + Type: schema.TypeString, + Optional: true, + }, + "computed_value": { + Type: schema.TypeString, + Computed: true, + }, + } + testCustomDiff := resources.ComputedIfAnyAttributeChanged( + testSchema, + "computed_value", + "value_with_diff_suppress", + "value_without_diff_suppress", + ) + testProvider := &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "test": { + Schema: testSchema, + CustomizeDiff: testCustomDiff, + }, + }, + } + tests := []struct { name string stateValue map[string]string @@ -356,89 +386,116 @@ func TestComputedIfAnyAttributeChangedWithSuppressDiff(t *testing.T) { expectDiff bool }{ { - name: "no change", + name: "no change in both values", stateValue: map[string]string{ - "value": "foo", - "computed_value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", }, expectDiff: false, }, { - name: "no change - quotes in config", + name: "change on field with diff suppression - suppressed (quotes in config added)", stateValue: map[string]string{ - "value": "foo", - "computed_value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "\"foo\"", + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "foo", }, expectDiff: false, }, { - name: "no change - quotes in state", + name: "change on field with diff suppression - suppressed (quotes in config removed)", stateValue: map[string]string{ - "value": "\"foo\"", - "computed_value": "foo", + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", }, expectDiff: false, }, { - name: "name change", + name: "change on field with diff suppression - not suppressed (value change)", stateValue: map[string]string{ - "value": "foo", - "computed_value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "bar", + "value_with_diff_suppress": "bar", + "value_without_diff_suppress": "foo", }, expectDiff: true, }, { - name: "name and quoting change", + name: "change on field with diff suppression - not suppressed (value and quotes changed)", stateValue: map[string]string{ - "value": "\"foo\"", - "computed_value": "foo", + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "bar", + "value_with_diff_suppress": "bar", + "value_without_diff_suppress": "foo", + }, + expectDiff: true, + }, + { + name: "change on field without diff suppression", + stateValue: map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + rawConfigValue: map[string]any{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "bar", + }, + expectDiff: true, + }, + { + name: "change on field without diff suppression, suppressed change on field with diff suppression", + stateValue: map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + rawConfigValue: map[string]any{ + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "bar", + }, + expectDiff: true, + }, + { + name: "change on field without diff suppression, not suppressed change on field with diff suppression", + stateValue: map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + rawConfigValue: map[string]any{ + "value_with_diff_suppress": "\"bar\"", + "value_without_diff_suppress": "bar", }, expectDiff: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - customDiff := resources.ComputedIfAnyAttributeChangedWithSuppressDiff( - "computed_value", - suppressFunc, - "value", - ) - provider := &schema.Provider{ - ResourcesMap: map[string]*schema.Resource{ - "test": { - Schema: map[string]*schema.Schema{ - "value": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: suppressFunc, - }, - "computed_value": { - Type: schema.TypeString, - Computed: true, - }, - }, - CustomizeDiff: customDiff, - }, - }, - } + tt := tt diff := calculateDiffFromAttributes( t, - provider, + testProvider, tt.stateValue, tt.rawConfigValue, ) From 720a04b42c794490cee23ea6a2581a54fa2cd181 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 18:40:59 +0200 Subject: [PATCH 17/42] Add negative test --- pkg/resources/custom_diffs.go | 1 - pkg/resources/custom_diffs_test.go | 47 +++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index d31a012ab8..3561b049d9 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -83,7 +83,6 @@ func ForceNewIfChangeToEmptyString(key string) schema.CustomizeDiffFunc { }) } -// TODO: test 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 diff --git a/pkg/resources/custom_diffs_test.go b/pkg/resources/custom_diffs_test.go index ca847e492b..e27a3af73a 100644 --- a/pkg/resources/custom_diffs_test.go +++ b/pkg/resources/custom_diffs_test.go @@ -386,7 +386,7 @@ func Test_ComputedIfAnyAttributeChanged(t *testing.T) { expectDiff bool }{ { - name: "no change in both values", + name: "no change on both fields", stateValue: map[string]string{ "value_with_diff_suppress": "foo", "value_without_diff_suppress": "foo", @@ -507,4 +507,49 @@ func Test_ComputedIfAnyAttributeChanged(t *testing.T) { } }) } + + t.Run("attributes not found in schema, both fields changed", func(t *testing.T) { + otherTestSchema := map[string]*schema.Schema{ + "value": { + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: testSuppressFunc, + }, + "computed_value": { + Type: schema.TypeString, + Computed: true, + }, + } + otherTestCustomDiff := resources.ComputedIfAnyAttributeChanged( + otherTestSchema, + "computed_value", + "value_with_diff_suppress", + "value_without_diff_suppress", + ) + otherTestProvider := &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "test": { + Schema: testSchema, + CustomizeDiff: otherTestCustomDiff, + }, + }, + } + + diff := calculateDiffFromAttributes( + t, + otherTestProvider, + map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + map[string]any{ + "value_with_diff_suppress": "\"bar\"", + "value_without_diff_suppress": "bar", + }, + ) + + require.NotNil(t, diff) + assert.Nil(t, diff.Attributes["computed_value"]) + }) } From f6162763b3b3756b3a044b4f147781ff30454895 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 18:48:21 +0200 Subject: [PATCH 18/42] Add description --- pkg/resources/custom_diffs.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index 3561b049d9..442b4c5180 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -83,6 +83,9 @@ func ForceNewIfChangeToEmptyString(key 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 From 480305a9cf85f5b11bf0fec6d3802c25b70e0154 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 18:53:34 +0200 Subject: [PATCH 19/42] Add important TODO --- pkg/resources/custom_diffs.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index 442b4c5180..ff6a46ce61 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -86,6 +86,7 @@ func ForceNewIfChangeToEmptyString(key 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. +// TODO: diffSuppressFunc sometimes use the last param (d) and we can't get it from the resource diff (ResourceDiff != ResourceData) 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 From 31b25449ad2838774d78b4891a7d873bdd65b008 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 19:28:46 +0200 Subject: [PATCH 20/42] Hack resource data --- pkg/resources/custom_diffs.go | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index ff6a46ce61..b1f2a66f66 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -3,14 +3,16 @@ package resources import ( "context" "log" + "reflect" "strconv" "strings" + "unsafe" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" - "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" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) func StringParameterValueComputedIf[T ~string](key string, params []*sdk.Parameter, parameterLevel sdk.ParameterType, parameter T) schema.CustomizeDiffFunc { @@ -86,7 +88,6 @@ func ForceNewIfChangeToEmptyString(key 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. -// TODO: diffSuppressFunc sometimes use the last param (d) and we can't get it from the resource diff (ResourceDiff != ResourceData) 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 @@ -97,7 +98,11 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key if v, ok := resourceSchema[changedKey]; ok { if diffSuppressFunc := v.DiffSuppressFunc; diffSuppressFunc != nil { - if !diffSuppressFunc(key, oldValue.(string), newValue.(string), nil) { + hackedResourceData, hackedCorrectly := hackResourceData(resourceSchema, diff) + if !hackedCorrectly { + continue + } + if !diffSuppressFunc(key, oldValue.(string), newValue.(string), hackedResourceData) { log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: key %s was changed and the diff is not suppressed", changedKey) result = true } else { @@ -114,6 +119,27 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key }) } +// TODO: extract hacky stuff outside this directory? +func hackResourceData(resourceSchema schema.InternalMap, diff *schema.ResourceDiff) (*schema.ResourceData, bool) { + unexportedState := reflect.ValueOf(diff).Elem().FieldByName("state") + hackedState := reflect.NewAt(unexportedState.Type(), unsafe.Pointer(unexportedState.UnsafeAddr())).Interface() + unexportedDiff := reflect.ValueOf(diff).Elem().FieldByName("diff") + hackedDiff := reflect.NewAt(unexportedDiff.Type(), unsafe.Pointer(unexportedDiff.UnsafeAddr())).Interface() + castState, ok := hackedState.(*terraform.InstanceState) + if !ok { + return nil, false + } + castDiff, ok := hackedDiff.(*terraform.InstanceDiff) + if !ok { + return nil, false + } + hackedResourceData, err := resourceSchema.Data(castState, castDiff) + if err != nil { + return nil, false + } + return hackedResourceData, true +} + type parameter[T ~string] struct { parameterName T valueType valueType From a05e0413f0d76ed88ebaf14e9761a37141db8dd6 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 19:46:22 +0200 Subject: [PATCH 21/42] Fix unit test --- pkg/resources/custom_diffs.go | 1 + pkg/resources/custom_diffs_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index b1f2a66f66..1b14db9139 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -100,6 +100,7 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key if diffSuppressFunc := v.DiffSuppressFunc; diffSuppressFunc != nil { hackedResourceData, hackedCorrectly := hackResourceData(resourceSchema, diff) if !hackedCorrectly { + log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: did not create resource data correctly, skipping\n") continue } if !diffSuppressFunc(key, oldValue.(string), newValue.(string), hackedResourceData) { diff --git a/pkg/resources/custom_diffs_test.go b/pkg/resources/custom_diffs_test.go index e27a3af73a..acecdfee11 100644 --- a/pkg/resources/custom_diffs_test.go +++ b/pkg/resources/custom_diffs_test.go @@ -501,6 +501,7 @@ func Test_ComputedIfAnyAttributeChanged(t *testing.T) { ) if tt.expectDiff { require.NotNil(t, diff) + require.NotNil(t, diff.Attributes["computed_value"]) assert.True(t, diff.Attributes["computed_value"].NewComputed) } else { require.Nil(t, diff) From dd6671819ede9f673a79317c2530e448528d0e93 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 19:53:05 +0200 Subject: [PATCH 22/42] Fix hack impl --- pkg/resources/custom_diffs.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index 1b14db9139..4f37fb23d9 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -123,9 +123,9 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key // TODO: extract hacky stuff outside this directory? func hackResourceData(resourceSchema schema.InternalMap, diff *schema.ResourceDiff) (*schema.ResourceData, bool) { unexportedState := reflect.ValueOf(diff).Elem().FieldByName("state") - hackedState := reflect.NewAt(unexportedState.Type(), unsafe.Pointer(unexportedState.UnsafeAddr())).Interface() + hackedState := reflect.NewAt(unexportedState.Type(), unsafe.Pointer(unexportedState.UnsafeAddr())).Elem().Interface() unexportedDiff := reflect.ValueOf(diff).Elem().FieldByName("diff") - hackedDiff := reflect.NewAt(unexportedDiff.Type(), unsafe.Pointer(unexportedDiff.UnsafeAddr())).Interface() + hackedDiff := reflect.NewAt(unexportedDiff.Type(), unsafe.Pointer(unexportedDiff.UnsafeAddr())).Elem().Interface() castState, ok := hackedState.(*terraform.InstanceState) if !ok { return nil, false From 3e4112144672b45c6a950082c4aa22ddcba77b69 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 22:37:50 +0200 Subject: [PATCH 23/42] Fix old and new values passed to diff suppress --- pkg/resources/custom_diffs.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index 4f37fb23d9..e807127807 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -2,6 +2,7 @@ package resources import ( "context" + "fmt" "log" "reflect" "strconv" @@ -103,7 +104,7 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: did not create resource data correctly, skipping\n") continue } - if !diffSuppressFunc(key, oldValue.(string), newValue.(string), hackedResourceData) { + if !diffSuppressFunc(key, fmt.Sprintf("%v", oldValue), fmt.Sprintf("%v", newValue), hackedResourceData) { log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: key %s was changed and the diff is not suppressed", changedKey) result = true } else { From 99fa60313cffd41bb4388ab0b7fc49cdf50679de Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 17:27:30 +0200 Subject: [PATCH 24/42] fix custom diffs for fields with diff supression starts here From 0d0d598d4605e008788838a49ee8a0e948536984 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 17:47:17 +0200 Subject: [PATCH 25/42] Prepare new custom diff function taking the diff suppress into consideration --- pkg/resources/custom_diffs.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index d495e7cd93..b516ef567b 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -97,7 +97,33 @@ func ComputedIfAnyAttributeChanged(key string, changedAttributeKeys ...string) s }) } -// TODO(SNOW-1629468): Adjust the function to make it more flexible +func ComputedIfAnyAttributeChangedCheckingDiffSuppression(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) { + oldValue, newValue := diff.GetChange(changedKey) + 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 { + if !diffSuppressFunc(key, oldValue.(string), newValue.(string), nil) { + 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 result + }) +} + 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 { From e3563701f2e2db64f570c5c500eddc58ca2c63c9 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 18:10:33 +0200 Subject: [PATCH 26/42] Use the improved computed if any attribute changed implementation --- pkg/resources/account.go | 2 +- pkg/resources/account_role.go | 5 ++--- ...ntegration_with_authorization_code_grant.go | 4 ++-- ...tion_integration_with_client_credentials.go | 4 ++-- ...thentication_integration_with_jwt_bearer.go | 4 ++-- pkg/resources/custom_diffs.go | 18 +++--------------- pkg/resources/database.go | 2 +- pkg/resources/database_role.go | 3 +-- pkg/resources/external_oauth_integration.go | 4 ++-- pkg/resources/file_format.go | 2 +- pkg/resources/function.go | 2 +- pkg/resources/masking_policy.go | 2 +- pkg/resources/materialized_view.go | 2 +- pkg/resources/network_policy.go | 4 +++- .../oauth_integration_for_custom_clients.go | 2 ++ ...uth_integration_for_partner_applications.go | 2 ++ pkg/resources/password_policy.go | 2 +- pkg/resources/procedure.go | 2 +- pkg/resources/saml2_integration.go | 4 ++-- pkg/resources/schema.go | 9 ++++----- pkg/resources/scim_integration.go | 4 ++-- pkg/resources/secondary_database.go | 2 +- pkg/resources/shared_database.go | 2 +- pkg/resources/streamlit.go | 7 +++---- pkg/resources/table.go | 2 +- pkg/resources/user.go | 8 ++++---- pkg/resources/view.go | 4 ++-- pkg/resources/warehouse.go | 7 +++---- 28 files changed, 52 insertions(+), 63 deletions(-) diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 2f41737bdd..0fef63ce97 100644 --- a/pkg/resources/account.go +++ b/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, diff --git a/pkg/resources/account_role.go b/pkg/resources/account_role.go index e7cc6f7fcf..28922572db 100644 --- a/pkg/resources/account_role.go +++ b/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{ diff --git a/pkg/resources/api_authentication_integration_with_authorization_code_grant.go b/pkg/resources/api_authentication_integration_with_authorization_code_grant.go index a1fb3df121..c2086e4608 100644 --- a/pkg/resources/api_authentication_integration_with_authorization_code_grant.go +++ b/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"), ), diff --git a/pkg/resources/api_authentication_integration_with_client_credentials.go b/pkg/resources/api_authentication_integration_with_client_credentials.go index 53231d8cf3..c98039dcfc 100644 --- a/pkg/resources/api_authentication_integration_with_client_credentials.go +++ b/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{ diff --git a/pkg/resources/api_authentication_integration_with_jwt_bearer.go b/pkg/resources/api_authentication_integration_with_jwt_bearer.go index 23c4a68ee5..b3a508cb46 100644 --- a/pkg/resources/api_authentication_integration_with_jwt_bearer.go +++ b/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"), ), diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index b516ef567b..01b30fd31b 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -83,21 +83,8 @@ func ForceNewIfChangeToEmptyString(key string) schema.CustomizeDiffFunc { }) } -func ComputedIfAnyAttributeChanged(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 - }) -} - -func ComputedIfAnyAttributeChangedCheckingDiffSuppression(resourceSchema map[string]*schema.Schema, key string, changedAttributeKeys ...string) schema.CustomizeDiffFunc { +// TODO: test +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 { @@ -124,6 +111,7 @@ func ComputedIfAnyAttributeChangedCheckingDiffSuppression(resourceSchema map[str }) } +// TODO: remove 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 { diff --git a/pkg/resources/database.go b/pkg/resources/database.go index ec89d89db9..80923faaf2 100644 --- a/pkg/resources/database.go +++ b/pkg/resources/database.go @@ -101,7 +101,7 @@ func Database() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(databaseSchema, FullyQualifiedNameAttributeName, "name"), databaseParametersCustomDiff, ), diff --git a/pkg/resources/database_role.go b/pkg/resources/database_role.go index ee98900a7b..c070152ea9 100644 --- a/pkg/resources/database_role.go +++ b/pkg/resources/database_role.go @@ -60,8 +60,7 @@ func DatabaseRole() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment"), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(databaseRoleSchema, ShowOutputAttributeName, "comment", "name"), ), SchemaVersion: 1, diff --git a/pkg/resources/external_oauth_integration.go b/pkg/resources/external_oauth_integration.go index c8b7ffbead..b454e3fa67 100644 --- a/pkg/resources/external_oauth_integration.go +++ b/pkg/resources/external_oauth_integration.go @@ -167,8 +167,8 @@ func ExternalOauthIntegration() *schema.Resource { ForceNewIfChangeToEmptyString("external_oauth_scope_mapping_attribute"), ForceNewIfChangeToEmptySet("external_oauth_jws_keys_url"), ForceNewIfChangeToEmptySet("external_oauth_token_user_mapping_claim"), - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "external_oauth_type", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "external_oauth_issuer", "external_oauth_jws_keys_url", "external_oauth_any_role_mode", + ComputedIfAnyAttributeChanged(externalOauthIntegrationSchema, ShowOutputAttributeName, "enabled", "external_oauth_type", "comment"), + ComputedIfAnyAttributeChanged(externalOauthIntegrationSchema, DescribeOutputAttributeName, "enabled", "external_oauth_issuer", "external_oauth_jws_keys_url", "external_oauth_any_role_mode", "external_oauth_rsa_public_key", "external_oauth_rsa_public_key_2", "external_oauth_blocked_roles_list", "external_oauth_allowed_roles_list", "external_oauth_audience_list", "external_oauth_token_user_mapping_claim", "external_oauth_snowflake_user_mapping_attribute", "external_oauth_scope_delimiter", "comment"), diff --git a/pkg/resources/file_format.go b/pkg/resources/file_format.go index adc751fca6..35b2a08205 100644 --- a/pkg/resources/file_format.go +++ b/pkg/resources/file_format.go @@ -319,7 +319,7 @@ func FileFormat() *schema.Resource { Delete: DeleteFileFormat, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(fileFormatSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: fileFormatSchema, diff --git a/pkg/resources/function.go b/pkg/resources/function.go index e9f1b43c53..40709a0feb 100644 --- a/pkg/resources/function.go +++ b/pkg/resources/function.go @@ -173,7 +173,7 @@ func Function() *schema.Resource { CustomizeDiff: customdiff.All( // TODO(SNOW-1348103): add `arguments` to ComputedIfAnyAttributeChanged. This can't be done now because this function compares values without diff suppress. - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(functionSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: functionSchema, diff --git a/pkg/resources/masking_policy.go b/pkg/resources/masking_policy.go index b962f8cf81..f10a1321c9 100644 --- a/pkg/resources/masking_policy.go +++ b/pkg/resources/masking_policy.go @@ -122,7 +122,7 @@ func MaskingPolicy() *schema.Resource { Delete: DeleteMaskingPolicy, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(maskingPolicySchema, FullyQualifiedNameAttributeName, "name"), ), Schema: maskingPolicySchema, diff --git a/pkg/resources/materialized_view.go b/pkg/resources/materialized_view.go index e280150418..a280aef202 100644 --- a/pkg/resources/materialized_view.go +++ b/pkg/resources/materialized_view.go @@ -76,7 +76,7 @@ func MaterializedView() *schema.Resource { Delete: DeleteMaterializedView, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(materializedViewSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: materializedViewSchema, diff --git a/pkg/resources/network_policy.go b/pkg/resources/network_policy.go index d04b44bf86..8283f22b63 100644 --- a/pkg/resources/network_policy.go +++ b/pkg/resources/network_policy.go @@ -99,6 +99,7 @@ func NetworkPolicy() *schema.Resource { // for complex types like Sets, Lists, and Maps. When every element of the Set is suppressed in custom diff, // it returns true for d.HasChange anyway (it returns false for suppressed changes on primitive types like Number, Bool, String, etc.). ComputedIfAnyAttributeChanged( + networkPolicySchema, ShowOutputAttributeName, // "allowed_network_rule_list", // "blocked_network_rule_list", @@ -107,13 +108,14 @@ func NetworkPolicy() *schema.Resource { "comment", ), ComputedIfAnyAttributeChanged( + networkPolicySchema, DescribeOutputAttributeName, // "allowed_network_rule_list", // "blocked_network_rule_list", "allowed_ip_list", "blocked_ip_list", ), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(networkPolicySchema, FullyQualifiedNameAttributeName, "name"), ), Importer: &schema.ResourceImporter{ diff --git a/pkg/resources/oauth_integration_for_custom_clients.go b/pkg/resources/oauth_integration_for_custom_clients.go index 7d7828626b..4462651bf2 100644 --- a/pkg/resources/oauth_integration_for_custom_clients.go +++ b/pkg/resources/oauth_integration_for_custom_clients.go @@ -161,11 +161,13 @@ func OauthIntegrationForCustomClients() *schema.Resource { CustomizeDiff: customdiff.All( ComputedIfAnyAttributeChanged( + oauthIntegrationForCustomClientsSchema, ShowOutputAttributeName, "enabled", "comment", ), ComputedIfAnyAttributeChanged( + oauthIntegrationForCustomClientsSchema, DescribeOutputAttributeName, "oauth_client_type", "oauth_redirect_uri", diff --git a/pkg/resources/oauth_integration_for_partner_applications.go b/pkg/resources/oauth_integration_for_partner_applications.go index de031dab6c..d92d9d4dee 100644 --- a/pkg/resources/oauth_integration_for_partner_applications.go +++ b/pkg/resources/oauth_integration_for_partner_applications.go @@ -121,11 +121,13 @@ func OauthIntegrationForPartnerApplications() *schema.Resource { CustomizeDiff: customdiff.All( ComputedIfAnyAttributeChanged( + oauthIntegrationForPartnerApplicationsSchema, ShowOutputAttributeName, "enabled", "comment", ), ComputedIfAnyAttributeChanged( + oauthIntegrationForPartnerApplicationsSchema, DescribeOutputAttributeName, "oauth_client", "enabled", diff --git a/pkg/resources/password_policy.go b/pkg/resources/password_policy.go index 91ebb5b03b..0aa5aaff98 100644 --- a/pkg/resources/password_policy.go +++ b/pkg/resources/password_policy.go @@ -145,7 +145,7 @@ func PasswordPolicy() *schema.Resource { Delete: DeletePasswordPolicy, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(passwordPolicySchema, FullyQualifiedNameAttributeName, "name"), ), Schema: passwordPolicySchema, diff --git a/pkg/resources/procedure.go b/pkg/resources/procedure.go index 295f3a1b67..f1337acd2b 100644 --- a/pkg/resources/procedure.go +++ b/pkg/resources/procedure.go @@ -189,7 +189,7 @@ func Procedure() *schema.Resource { // TODO(SNOW-1348106): add `arguments` to ComputedIfAnyAttributeChanged for FullyQualifiedNameAttributeName. // This can't be done now because this function compares values without diff suppress. CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(procedureSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: procedureSchema, diff --git a/pkg/resources/saml2_integration.go b/pkg/resources/saml2_integration.go index 3939d2249a..6712fd23fe 100644 --- a/pkg/resources/saml2_integration.go +++ b/pkg/resources/saml2_integration.go @@ -172,8 +172,8 @@ func SAML2Integration() *schema.Resource { ForceNewIfChangeToEmptyString("saml2_snowflake_issuer_url"), ForceNewIfChangeToEmptyString("saml2_snowflake_acs_url"), ForceNewIfChangeToEmptyString("saml2_sp_initiated_login_page_label"), - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "saml2_issuer", "saml2_sso_url", "saml2_provider", "saml2_x509_cert", + ComputedIfAnyAttributeChanged(saml2IntegrationSchema, ShowOutputAttributeName, "enabled", "comment"), + ComputedIfAnyAttributeChanged(saml2IntegrationSchema, DescribeOutputAttributeName, "saml2_issuer", "saml2_sso_url", "saml2_provider", "saml2_x509_cert", "saml2_sp_initiated_login_page_label", "saml2_enable_sp_initiated", "saml2_sign_request", "saml2_requtedted_nameid_format", "saml2_post_logout_redirect_url", "saml2_force_authn", "saml2_snowflake_issuer_url", "saml2_snowflake_acs_url", "allowed_user_domains", "allowed_email_patterns"), diff --git a/pkg/resources/schema.go b/pkg/resources/schema.go index 55f54d02cf..de3c4e2e48 100644 --- a/pkg/resources/schema.go +++ b/pkg/resources/schema.go @@ -96,11 +96,10 @@ func Schema() *schema.Resource { Description: "Resource used to manage schema objects. For more information, check [schema documentation](https://docs.snowflake.com/en/sql-reference/sql/create-schema).", CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment", "with_managed_access", "is_transient"), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChangedWithSuppressDiff(DescribeOutputAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChanged(ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllSchemaParameters), strings.ToLower)...), + ComputedIfAnyAttributeChanged(schemaSchema, ShowOutputAttributeName, "name", "comment", "with_managed_access", "is_transient"), + ComputedIfAnyAttributeChanged(schemaSchema, DescribeOutputAttributeName, "name"), + ComputedIfAnyAttributeChanged(schemaSchema, FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(schemaParametersSchema, ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllSchemaParameters), strings.ToLower)...), schemaParametersCustomDiff, ), diff --git a/pkg/resources/scim_integration.go b/pkg/resources/scim_integration.go index fab3d51ae0..8ca1aeedb2 100644 --- a/pkg/resources/scim_integration.go +++ b/pkg/resources/scim_integration.go @@ -109,8 +109,8 @@ func SCIMIntegration() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "scim_client", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "comment", "network_policy", "run_as_role", "sync_password"), + ComputedIfAnyAttributeChanged(scimIntegrationSchema, ShowOutputAttributeName, "enabled", "scim_client", "comment"), + ComputedIfAnyAttributeChanged(scimIntegrationSchema, DescribeOutputAttributeName, "enabled", "comment", "network_policy", "run_as_role", "sync_password"), ), SchemaVersion: 2, diff --git a/pkg/resources/secondary_database.go b/pkg/resources/secondary_database.go index 361db4ab48..155699d1c9 100644 --- a/pkg/resources/secondary_database.go +++ b/pkg/resources/secondary_database.go @@ -53,7 +53,7 @@ func SecondaryDatabase() *schema.Resource { CustomizeDiff: customdiff.All( databaseParametersCustomDiff, - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(secondaryDatabaseSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: helpers.MergeMaps(secondaryDatabaseSchema, databaseParametersSchema), Importer: &schema.ResourceImporter{ diff --git a/pkg/resources/shared_database.go b/pkg/resources/shared_database.go index b6da4eaf36..8339148656 100644 --- a/pkg/resources/shared_database.go +++ b/pkg/resources/shared_database.go @@ -53,7 +53,7 @@ func SharedDatabase() *schema.Resource { Description: "A shared database creates a database from a share provided by another Snowflake account. For more information about shares, see [Introduction to Secure Data Sharing](https://docs.snowflake.com/en/user-guide/data-sharing-intro).", CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(sharedDatabaseSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: helpers.MergeMaps(sharedDatabaseSchema, sharedDatabaseParametersSchema), diff --git a/pkg/resources/streamlit.go b/pkg/resources/streamlit.go index e386f10f6c..830e7c0924 100644 --- a/pkg/resources/streamlit.go +++ b/pkg/resources/streamlit.go @@ -118,10 +118,9 @@ func Streamlit() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "title", "comment", "query_warehouse"), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "title", "comment", "root_location", "main_file", "query_warehouse", "external_access_integrations"), + ComputedIfAnyAttributeChanged(streamlitSchema, ShowOutputAttributeName, "name", "title", "comment", "query_warehouse"), + ComputedIfAnyAttributeChanged(streamlitSchema, FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(streamlitSchema, DescribeOutputAttributeName, "title", "comment", "root_location", "main_file", "query_warehouse", "external_access_integrations"), ), SchemaVersion: 1, diff --git a/pkg/resources/table.go b/pkg/resources/table.go index ad10836b74..100d89cffa 100644 --- a/pkg/resources/table.go +++ b/pkg/resources/table.go @@ -212,7 +212,7 @@ func Table() *schema.Resource { Delete: DeleteTable, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(tableSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: tableSchema, diff --git a/pkg/resources/user.go b/pkg/resources/user.go index 05108193e4..58af3bae16 100644 --- a/pkg/resources/user.go +++ b/pkg/resources/user.go @@ -199,10 +199,10 @@ func User() *schema.Resource { }, CustomizeDiff: customdiff.All( - // TODO [SNOW-1629468 - next pr]: handle diff suppression correctly; add "default_role", "default_secondary_roles" - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "password", "login_name", "display_name", "first_name", "middle_name", "last_name", "email", "must_change_password", "disabled", "days_to_expiry", "mins_to_unlock", "default_warehouse", "default_namespace", "mins_to_bypass_mfa", "rsa_public_key", "rsa_public_key_2", "comment", "disable_mfa"), - ComputedIfAnyAttributeChanged(ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllUserParameters), strings.ToLower)...), - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + // TODO [SNOW-1629468 - next pr]: test "default_role", "default_secondary_roles" + ComputedIfAnyAttributeChanged(userSchema, ShowOutputAttributeName, "password", "login_name", "display_name", "first_name", "middle_name", "last_name", "email", "must_change_password", "disabled", "days_to_expiry", "mins_to_unlock", "default_warehouse", "default_namespace", "default_role", "default_secondary_roles", "mins_to_bypass_mfa", "rsa_public_key", "rsa_public_key_2", "comment", "disable_mfa"), + ComputedIfAnyAttributeChanged(userParametersSchema, ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllUserParameters), strings.ToLower)...), + ComputedIfAnyAttributeChanged(userSchema, FullyQualifiedNameAttributeName, "name"), userParametersCustomDiff, // TODO [SNOW-1645348]: revisit with service user work func(_ context.Context, diff *schema.ResourceDiff, _ interface{}) error { diff --git a/pkg/resources/view.go b/pkg/resources/view.go index e9e929f2da..5090e2b9ee 100644 --- a/pkg/resources/view.go +++ b/pkg/resources/view.go @@ -280,8 +280,8 @@ func View() *schema.Resource { Description: "Resource used to manage view objects. For more information, check [view documentation](https://docs.snowflake.com/en/sql-reference/sql/create-view).", CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment", "change_tracking", "is_secure", "is_temporary", "is_recursive", "statement"), - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(viewSchema, ShowOutputAttributeName, "comment", "change_tracking", "is_secure", "is_temporary", "is_recursive", "statement"), + ComputedIfAnyAttributeChanged(viewSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: viewSchema, diff --git a/pkg/resources/warehouse.go b/pkg/resources/warehouse.go index 9623cd947b..6232b0bac1 100644 --- a/pkg/resources/warehouse.go +++ b/pkg/resources/warehouse.go @@ -204,10 +204,9 @@ func Warehouse() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "warehouse_type", "warehouse_size", "max_cluster_count", "min_cluster_count", "scaling_policy", "auto_suspend", "auto_resume", "comment", "enable_query_acceleration", "query_acceleration_max_scale_factor"), - ComputedIfAnyAttributeChanged(ParametersAttributeName, strings.ToLower(string(sdk.ObjectParameterMaxConcurrencyLevel)), strings.ToLower(string(sdk.ObjectParameterStatementQueuedTimeoutInSeconds)), strings.ToLower(string(sdk.ObjectParameterStatementTimeoutInSeconds))), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name", "resource_monitor"), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(warehouseSchema, ShowOutputAttributeName, "name", "warehouse_type", "warehouse_size", "max_cluster_count", "min_cluster_count", "scaling_policy", "auto_suspend", "auto_resume", "resource_monitor", "comment", "enable_query_acceleration", "query_acceleration_max_scale_factor"), + ComputedIfAnyAttributeChanged(warehouseSchema, ParametersAttributeName, strings.ToLower(string(sdk.ObjectParameterMaxConcurrencyLevel)), strings.ToLower(string(sdk.ObjectParameterStatementQueuedTimeoutInSeconds)), strings.ToLower(string(sdk.ObjectParameterStatementTimeoutInSeconds))), + ComputedIfAnyAttributeChanged(warehouseSchema, FullyQualifiedNameAttributeName, "name"), customdiff.ForceNewIfChange("warehouse_size", func(ctx context.Context, old, new, meta any) bool { return old.(string) != "" && new.(string) == "" From 1ae8ab47b8c5619e322720306470b26833d4bfd6 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 18:34:09 +0200 Subject: [PATCH 27/42] Unit test improved computed if any attribute changed --- pkg/resources/custom_diffs.go | 16 ---- pkg/resources/custom_diffs_test.go | 149 ++++++++++++++++++++--------- 2 files changed, 103 insertions(+), 62 deletions(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index 01b30fd31b..d31a012ab8 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -111,22 +111,6 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key }) } -// TODO: remove -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 - } - } - } - return false - }) -} - type parameter[T ~string] struct { parameterName T valueType valueType diff --git a/pkg/resources/custom_diffs_test.go b/pkg/resources/custom_diffs_test.go index 5bd28733aa..ca847e492b 100644 --- a/pkg/resources/custom_diffs_test.go +++ b/pkg/resources/custom_diffs_test.go @@ -345,10 +345,40 @@ func TestForceNewIfChangeToEmptySet(t *testing.T) { } } -func TestComputedIfAnyAttributeChangedWithSuppressDiff(t *testing.T) { - suppressFunc := schema.SchemaDiffSuppressFunc(func(k, oldValue, newValue string, d *schema.ResourceData) bool { +func Test_ComputedIfAnyAttributeChanged(t *testing.T) { + testSuppressFunc := schema.SchemaDiffSuppressFunc(func(k, oldValue, newValue string, d *schema.ResourceData) bool { return strings.Trim(oldValue, `"`) == strings.Trim(newValue, `"`) }) + testSchema := map[string]*schema.Schema{ + "value_with_diff_suppress": { + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: testSuppressFunc, + }, + "value_without_diff_suppress": { + Type: schema.TypeString, + Optional: true, + }, + "computed_value": { + Type: schema.TypeString, + Computed: true, + }, + } + testCustomDiff := resources.ComputedIfAnyAttributeChanged( + testSchema, + "computed_value", + "value_with_diff_suppress", + "value_without_diff_suppress", + ) + testProvider := &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "test": { + Schema: testSchema, + CustomizeDiff: testCustomDiff, + }, + }, + } + tests := []struct { name string stateValue map[string]string @@ -356,89 +386,116 @@ func TestComputedIfAnyAttributeChangedWithSuppressDiff(t *testing.T) { expectDiff bool }{ { - name: "no change", + name: "no change in both values", stateValue: map[string]string{ - "value": "foo", - "computed_value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", }, expectDiff: false, }, { - name: "no change - quotes in config", + name: "change on field with diff suppression - suppressed (quotes in config added)", stateValue: map[string]string{ - "value": "foo", - "computed_value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "\"foo\"", + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "foo", }, expectDiff: false, }, { - name: "no change - quotes in state", + name: "change on field with diff suppression - suppressed (quotes in config removed)", stateValue: map[string]string{ - "value": "\"foo\"", - "computed_value": "foo", + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", }, expectDiff: false, }, { - name: "name change", + name: "change on field with diff suppression - not suppressed (value change)", stateValue: map[string]string{ - "value": "foo", - "computed_value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "bar", + "value_with_diff_suppress": "bar", + "value_without_diff_suppress": "foo", }, expectDiff: true, }, { - name: "name and quoting change", + name: "change on field with diff suppression - not suppressed (value and quotes changed)", stateValue: map[string]string{ - "value": "\"foo\"", - "computed_value": "foo", + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "bar", + "value_with_diff_suppress": "bar", + "value_without_diff_suppress": "foo", + }, + expectDiff: true, + }, + { + name: "change on field without diff suppression", + stateValue: map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + rawConfigValue: map[string]any{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "bar", + }, + expectDiff: true, + }, + { + name: "change on field without diff suppression, suppressed change on field with diff suppression", + stateValue: map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + rawConfigValue: map[string]any{ + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "bar", + }, + expectDiff: true, + }, + { + name: "change on field without diff suppression, not suppressed change on field with diff suppression", + stateValue: map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + rawConfigValue: map[string]any{ + "value_with_diff_suppress": "\"bar\"", + "value_without_diff_suppress": "bar", }, expectDiff: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - customDiff := resources.ComputedIfAnyAttributeChangedWithSuppressDiff( - "computed_value", - suppressFunc, - "value", - ) - provider := &schema.Provider{ - ResourcesMap: map[string]*schema.Resource{ - "test": { - Schema: map[string]*schema.Schema{ - "value": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: suppressFunc, - }, - "computed_value": { - Type: schema.TypeString, - Computed: true, - }, - }, - CustomizeDiff: customDiff, - }, - }, - } + tt := tt diff := calculateDiffFromAttributes( t, - provider, + testProvider, tt.stateValue, tt.rawConfigValue, ) From cddde7d1953df3e65a7c6e3990d5af3b60842571 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 18:40:59 +0200 Subject: [PATCH 28/42] Add negative test --- pkg/resources/custom_diffs.go | 1 - pkg/resources/custom_diffs_test.go | 47 +++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index d31a012ab8..3561b049d9 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -83,7 +83,6 @@ func ForceNewIfChangeToEmptyString(key string) schema.CustomizeDiffFunc { }) } -// TODO: test 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 diff --git a/pkg/resources/custom_diffs_test.go b/pkg/resources/custom_diffs_test.go index ca847e492b..e27a3af73a 100644 --- a/pkg/resources/custom_diffs_test.go +++ b/pkg/resources/custom_diffs_test.go @@ -386,7 +386,7 @@ func Test_ComputedIfAnyAttributeChanged(t *testing.T) { expectDiff bool }{ { - name: "no change in both values", + name: "no change on both fields", stateValue: map[string]string{ "value_with_diff_suppress": "foo", "value_without_diff_suppress": "foo", @@ -507,4 +507,49 @@ func Test_ComputedIfAnyAttributeChanged(t *testing.T) { } }) } + + t.Run("attributes not found in schema, both fields changed", func(t *testing.T) { + otherTestSchema := map[string]*schema.Schema{ + "value": { + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: testSuppressFunc, + }, + "computed_value": { + Type: schema.TypeString, + Computed: true, + }, + } + otherTestCustomDiff := resources.ComputedIfAnyAttributeChanged( + otherTestSchema, + "computed_value", + "value_with_diff_suppress", + "value_without_diff_suppress", + ) + otherTestProvider := &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "test": { + Schema: testSchema, + CustomizeDiff: otherTestCustomDiff, + }, + }, + } + + diff := calculateDiffFromAttributes( + t, + otherTestProvider, + map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + map[string]any{ + "value_with_diff_suppress": "\"bar\"", + "value_without_diff_suppress": "bar", + }, + ) + + require.NotNil(t, diff) + assert.Nil(t, diff.Attributes["computed_value"]) + }) } From 3c36974fce82abeff7d22113c6bb0cd62056d652 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 18:48:21 +0200 Subject: [PATCH 29/42] Add description --- pkg/resources/custom_diffs.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index 3561b049d9..442b4c5180 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -83,6 +83,9 @@ func ForceNewIfChangeToEmptyString(key 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 From 4b90b491463ff98eec78f28e9dbbf62fd382862d Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 18:53:34 +0200 Subject: [PATCH 30/42] Add important TODO --- pkg/resources/custom_diffs.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index 442b4c5180..ff6a46ce61 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -86,6 +86,7 @@ func ForceNewIfChangeToEmptyString(key 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. +// TODO: diffSuppressFunc sometimes use the last param (d) and we can't get it from the resource diff (ResourceDiff != ResourceData) 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 From a7cb8cfb4d08b856c25ea84d1b6dc968cea33d5a Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 19:28:46 +0200 Subject: [PATCH 31/42] Hack resource data --- pkg/resources/custom_diffs.go | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index ff6a46ce61..b1f2a66f66 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -3,14 +3,16 @@ package resources import ( "context" "log" + "reflect" "strconv" "strings" + "unsafe" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" - "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" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) func StringParameterValueComputedIf[T ~string](key string, params []*sdk.Parameter, parameterLevel sdk.ParameterType, parameter T) schema.CustomizeDiffFunc { @@ -86,7 +88,6 @@ func ForceNewIfChangeToEmptyString(key 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. -// TODO: diffSuppressFunc sometimes use the last param (d) and we can't get it from the resource diff (ResourceDiff != ResourceData) 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 @@ -97,7 +98,11 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key if v, ok := resourceSchema[changedKey]; ok { if diffSuppressFunc := v.DiffSuppressFunc; diffSuppressFunc != nil { - if !diffSuppressFunc(key, oldValue.(string), newValue.(string), nil) { + hackedResourceData, hackedCorrectly := hackResourceData(resourceSchema, diff) + if !hackedCorrectly { + continue + } + if !diffSuppressFunc(key, oldValue.(string), newValue.(string), hackedResourceData) { log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: key %s was changed and the diff is not suppressed", changedKey) result = true } else { @@ -114,6 +119,27 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key }) } +// TODO: extract hacky stuff outside this directory? +func hackResourceData(resourceSchema schema.InternalMap, diff *schema.ResourceDiff) (*schema.ResourceData, bool) { + unexportedState := reflect.ValueOf(diff).Elem().FieldByName("state") + hackedState := reflect.NewAt(unexportedState.Type(), unsafe.Pointer(unexportedState.UnsafeAddr())).Interface() + unexportedDiff := reflect.ValueOf(diff).Elem().FieldByName("diff") + hackedDiff := reflect.NewAt(unexportedDiff.Type(), unsafe.Pointer(unexportedDiff.UnsafeAddr())).Interface() + castState, ok := hackedState.(*terraform.InstanceState) + if !ok { + return nil, false + } + castDiff, ok := hackedDiff.(*terraform.InstanceDiff) + if !ok { + return nil, false + } + hackedResourceData, err := resourceSchema.Data(castState, castDiff) + if err != nil { + return nil, false + } + return hackedResourceData, true +} + type parameter[T ~string] struct { parameterName T valueType valueType From 50706e51c75942c086ef1a2e15504fcaf3126b66 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 19:46:22 +0200 Subject: [PATCH 32/42] Fix unit test --- pkg/resources/custom_diffs.go | 1 + pkg/resources/custom_diffs_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index b1f2a66f66..1b14db9139 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -100,6 +100,7 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key if diffSuppressFunc := v.DiffSuppressFunc; diffSuppressFunc != nil { hackedResourceData, hackedCorrectly := hackResourceData(resourceSchema, diff) if !hackedCorrectly { + log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: did not create resource data correctly, skipping\n") continue } if !diffSuppressFunc(key, oldValue.(string), newValue.(string), hackedResourceData) { diff --git a/pkg/resources/custom_diffs_test.go b/pkg/resources/custom_diffs_test.go index e27a3af73a..acecdfee11 100644 --- a/pkg/resources/custom_diffs_test.go +++ b/pkg/resources/custom_diffs_test.go @@ -501,6 +501,7 @@ func Test_ComputedIfAnyAttributeChanged(t *testing.T) { ) if tt.expectDiff { require.NotNil(t, diff) + require.NotNil(t, diff.Attributes["computed_value"]) assert.True(t, diff.Attributes["computed_value"].NewComputed) } else { require.Nil(t, diff) From a9831f7b147ece150c376f04ed3d33ac8b436639 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 19:53:05 +0200 Subject: [PATCH 33/42] Fix hack impl --- pkg/resources/custom_diffs.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index 1b14db9139..4f37fb23d9 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -123,9 +123,9 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key // TODO: extract hacky stuff outside this directory? func hackResourceData(resourceSchema schema.InternalMap, diff *schema.ResourceDiff) (*schema.ResourceData, bool) { unexportedState := reflect.ValueOf(diff).Elem().FieldByName("state") - hackedState := reflect.NewAt(unexportedState.Type(), unsafe.Pointer(unexportedState.UnsafeAddr())).Interface() + hackedState := reflect.NewAt(unexportedState.Type(), unsafe.Pointer(unexportedState.UnsafeAddr())).Elem().Interface() unexportedDiff := reflect.ValueOf(diff).Elem().FieldByName("diff") - hackedDiff := reflect.NewAt(unexportedDiff.Type(), unsafe.Pointer(unexportedDiff.UnsafeAddr())).Interface() + hackedDiff := reflect.NewAt(unexportedDiff.Type(), unsafe.Pointer(unexportedDiff.UnsafeAddr())).Elem().Interface() castState, ok := hackedState.(*terraform.InstanceState) if !ok { return nil, false From 85d1e2780cf2c52cf43b8d4cf049feb3bec7b42f Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 22:37:50 +0200 Subject: [PATCH 34/42] Fix old and new values passed to diff suppress --- pkg/resources/custom_diffs.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index 4f37fb23d9..e807127807 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -2,6 +2,7 @@ package resources import ( "context" + "fmt" "log" "reflect" "strconv" @@ -103,7 +104,7 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: did not create resource data correctly, skipping\n") continue } - if !diffSuppressFunc(key, oldValue.(string), newValue.(string), hackedResourceData) { + if !diffSuppressFunc(key, fmt.Sprintf("%v", oldValue), fmt.Sprintf("%v", newValue), hackedResourceData) { log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: key %s was changed and the diff is not suppressed", changedKey) result = true } else { From 82649d4a1b71124b80b85cfdf6c93366a662e292 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Tue, 3 Sep 2024 10:59:11 +0200 Subject: [PATCH 35/42] Fix saml2 tests after changes --- .../saml2_integration_acceptance_test.go | 48 +++++++++++++------ .../non_zero_values/test.tf | 11 +++++ .../non_zero_values/variables.tf | 15 ++++++ 3 files changed, 60 insertions(+), 14 deletions(-) create mode 100644 pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/test.tf create mode 100644 pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/variables.tf diff --git a/pkg/resources/saml2_integration_acceptance_test.go b/pkg/resources/saml2_integration_acceptance_test.go index d27061762e..352c58c884 100644 --- a/pkg/resources/saml2_integration_acceptance_test.go +++ b/pkg/resources/saml2_integration_acceptance_test.go @@ -416,7 +416,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { }, CheckDestroy: acc.CheckDestroy(t, resources.Saml2SecurityIntegration), Steps: []resource.TestStep{ - // set up with concrete type + // set up with concrete saml2_force_authn { ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ @@ -432,7 +432,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "true"), ), }, - // import when type in config + // import when saml2_force_authn in config { ResourceName: "snowflake_saml2_integration.test", ImportState: true, @@ -442,7 +442,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { importchecks.TestCheckResourceAttrInstanceState(resourcehelpers.EncodeResourceIdentifier(id), "describe_output.0.saml2_force_authn.0.value", "true"), ), }, - // change type in config + // change saml2_force_authn in config { ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ @@ -462,7 +462,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { { Config: saml2ConfigWithAuthn(id.Name(), issuer, string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom), validUrl, cert, true), }, - // remove non-default type from config + // remove non-default saml2_force_authn from config { Config: saml2Config(id.Name(), issuer, string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom), validUrl, cert), ConfigPlanChecks: resource.ConfigPlanChecks{ @@ -479,23 +479,26 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"), ), }, - // add config + // add saml2_force_authn to config (false - which is a default in Snowflake) - no changes expected { ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ planchecks.PrintPlanDetails("snowflake_saml2_integration.test", "saml2_force_authn", "describe_output"), - planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String(r.BooleanDefault), sdk.String(r.BooleanDefault)), - planchecks.ExpectComputed("snowflake_saml2_integration.test", "describe_output", true), + plancheck.ExpectEmptyPlan(), }, }, Config: saml2ConfigWithAuthn(id.Name(), issuer, string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom), validUrl, cert, false), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", "false"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", r.BooleanDefault), resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.#", "1"), resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"), ), }, - // remove type from config but update externally to default (still expecting non-empty plan because we do not know the default) + // change back to non-default + { + Config: saml2ConfigWithAuthn(id.Name(), issuer, string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom), validUrl, cert, true), + }, + // remove saml2_force_authn from config but update externally to default (still expecting non-empty plan because we do not know the default) { PreConfig: func() { acc.TestClient().SecurityIntegration.UpdateSaml2ForceAuthn(t, id, false) @@ -515,7 +518,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"), ), }, - // change the size externally + // change the saml2_force_authn externally { PreConfig: func() { // we change the type to the type different from default, expecting action @@ -536,7 +539,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"), ), }, - // import when no type in config + // import when no saml2_force_authn in config { ResourceName: "snowflake_saml2_integration.test", ImportState: true, @@ -1002,15 +1005,32 @@ func TestAcc_Saml2Integration_DefaultValues(t *testing.T) { resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"), ), }, + // set to "non-zero" values + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Saml2Integration/non_zero_values"), + ConfigVariables: configVariables, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "enabled", "true"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", "true"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_post_logout_redirect_url", "http://example.com"), + + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "show_output.#", "1"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "show_output.0.enabled", "true"), + + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.#", "1"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_post_logout_redirect_url.0.value", "http://example.com"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "true"), + ), + }, // add valid "zero" values again (to validate if set is run correctly) { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Saml2Integration/zero_values"), ConfigVariables: configVariables, ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ - planchecks.ExpectChange("snowflake_saml2_integration.test", "enabled", tfjson.ActionUpdate, sdk.String(r.BooleanDefault), sdk.String(r.BooleanDefault)), - planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String(r.BooleanDefault), sdk.String(r.BooleanDefault)), - planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_post_logout_redirect_url", tfjson.ActionUpdate, sdk.String(""), sdk.String("")), + planchecks.ExpectChange("snowflake_saml2_integration.test", "enabled", tfjson.ActionUpdate, sdk.String(r.BooleanTrue), sdk.String(r.BooleanFalse)), + planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String(r.BooleanTrue), sdk.String(r.BooleanFalse)), + planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_post_logout_redirect_url", tfjson.ActionUpdate, sdk.String("http://example.com"), sdk.String("")), planchecks.ExpectComputed("snowflake_saml2_integration.test", "show_output", true), planchecks.ExpectComputed("snowflake_saml2_integration.test", "describe_output", true), }, diff --git a/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/test.tf b/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/test.tf new file mode 100644 index 0000000000..7cc95b2ebe --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/test.tf @@ -0,0 +1,11 @@ +resource "snowflake_saml2_integration" "test" { + name = var.name + saml2_issuer = var.saml2_issuer + saml2_sso_url = var.saml2_sso_url + saml2_provider = var.saml2_provider + saml2_x509_cert = var.saml2_x509_cert + + enabled = true + saml2_force_authn = true + saml2_post_logout_redirect_url = "http://example.com" +} diff --git a/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/variables.tf b/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/variables.tf new file mode 100644 index 0000000000..351581f56c --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/variables.tf @@ -0,0 +1,15 @@ +variable "name" { + type = string +} +variable "saml2_issuer" { + type = string +} +variable "saml2_provider" { + type = string +} +variable "saml2_sso_url" { + type = string +} +variable "saml2_x509_cert" { + type = string +} From 4b5dc1050ea13657ce1f69eb264ef0de2b8bcc48 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Tue, 3 Sep 2024 12:16:40 +0200 Subject: [PATCH 36/42] Fix warehouse and user --- pkg/resources/user.go | 3 ++- pkg/resources/warehouse_acceptance_test.go | 30 ++-------------------- 2 files changed, 4 insertions(+), 29 deletions(-) diff --git a/pkg/resources/user.go b/pkg/resources/user.go index 58af3bae16..eebb3dbf5e 100644 --- a/pkg/resources/user.go +++ b/pkg/resources/user.go @@ -200,7 +200,8 @@ func User() *schema.Resource { CustomizeDiff: customdiff.All( // TODO [SNOW-1629468 - next pr]: test "default_role", "default_secondary_roles" - ComputedIfAnyAttributeChanged(userSchema, ShowOutputAttributeName, "password", "login_name", "display_name", "first_name", "middle_name", "last_name", "email", "must_change_password", "disabled", "days_to_expiry", "mins_to_unlock", "default_warehouse", "default_namespace", "default_role", "default_secondary_roles", "mins_to_bypass_mfa", "rsa_public_key", "rsa_public_key_2", "comment", "disable_mfa"), + // TODO [SNOW-TODO]: "default_secondary_roles" have to stay commented out because of how the SDKv2 handles diff suppressions and custom diffs for sets + ComputedIfAnyAttributeChanged(userSchema, ShowOutputAttributeName, "password", "login_name", "display_name", "first_name", "middle_name", "last_name", "email", "must_change_password", "disabled", "days_to_expiry", "mins_to_unlock", "default_warehouse", "default_namespace", "default_role", "mins_to_bypass_mfa", "rsa_public_key", "rsa_public_key_2", "comment", "disable_mfa"), ComputedIfAnyAttributeChanged(userParametersSchema, ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllUserParameters), strings.ToLower)...), ComputedIfAnyAttributeChanged(userSchema, FullyQualifiedNameAttributeName, "name"), userParametersCustomDiff, diff --git a/pkg/resources/warehouse_acceptance_test.go b/pkg/resources/warehouse_acceptance_test.go index 3e96684a7b..1d737d39cd 100644 --- a/pkg/resources/warehouse_acceptance_test.go +++ b/pkg/resources/warehouse_acceptance_test.go @@ -188,41 +188,15 @@ func TestAcc_Warehouse_BasicFlows(t *testing.T) { resource.TestCheckResourceAttr("snowflake_warehouse.w", "fully_qualified_name", warehouseId2.FullyQualifiedName()), ), }, - // Change config but use defaults for every attribute (but not the parameters) - expect no changes (because these are already SF values) except computed show_output (follow-up why suppress diff is not taken into account in has changes?) + // Change config but use defaults for every attribute (but not the parameters) - expect no changes (because these are already SF values) { ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ planchecks.PrintPlanDetails("snowflake_warehouse.w", "warehouse_type", "warehouse_size", "max_cluster_count", "min_cluster_count", "scaling_policy", "auto_suspend", "auto_resume", "enable_query_acceleration", "query_acceleration_max_scale_factor", "max_concurrency_level", "statement_queued_timeout_in_seconds", "statement_timeout_in_seconds", r.ShowOutputAttributeName), - plancheck.ExpectResourceAction("snowflake_warehouse.w", plancheck.ResourceActionUpdate), - planchecks.ExpectComputed("snowflake_warehouse.w", r.ShowOutputAttributeName, true), + plancheck.ExpectEmptyPlan(), }, }, Config: warehouseFullDefaultWithoutParametersConfig(name2, comment), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_warehouse.w", "warehouse_type", string(sdk.WarehouseTypeStandard)), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "warehouse_size", string(sdk.WarehouseSizeXSmall)), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "max_cluster_count", "1"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "min_cluster_count", "1"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "scaling_policy", string(sdk.ScalingPolicyStandard)), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "auto_suspend", "600"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "auto_resume", "true"), - resource.TestCheckNoResourceAttr("snowflake_warehouse.w", "initially_suspended"), - resource.TestCheckNoResourceAttr("snowflake_warehouse.w", "resource_monitor"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "comment", comment), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "enable_query_acceleration", "false"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "query_acceleration_max_scale_factor", "8"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "max_concurrency_level", "8"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "statement_queued_timeout_in_seconds", "0"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "statement_timeout_in_seconds", "172800"), - - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.#", "1"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.max_concurrency_level.0.value", "8"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.max_concurrency_level.0.level", ""), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.statement_queued_timeout_in_seconds.0.value", "0"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.statement_queued_timeout_in_seconds.0.level", ""), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.statement_timeout_in_seconds.0.value", "172800"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.statement_timeout_in_seconds.0.level", ""), - ), }, // add parameters - update expected (different level even with same values) { From 0d9c1b09d9bf80de92b8c6001d6ecb704b7bbd71 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Tue, 3 Sep 2024 14:40:40 +0200 Subject: [PATCH 37/42] Fix tests --- .../sdkv2enhancements/resource_data.go | 36 +++++++++++++++++++ pkg/resources/custom_diffs.go | 31 +++------------- pkg/resources/warehouse_acceptance_test.go | 34 ++++++++++++++++++ 3 files changed, 74 insertions(+), 27 deletions(-) create mode 100644 pkg/internal/provider/sdkv2enhancements/resource_data.go diff --git a/pkg/internal/provider/sdkv2enhancements/resource_data.go b/pkg/internal/provider/sdkv2enhancements/resource_data.go new file mode 100644 index 0000000000..c2f450c908 --- /dev/null +++ b/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 +} diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index e807127807..a10c78d4d6 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -4,16 +4,14 @@ import ( "context" "fmt" "log" - "reflect" "strconv" "strings" - "unsafe" "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" - "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) func StringParameterValueComputedIf[T ~string](key string, params []*sdk.Parameter, parameterLevel sdk.ParameterType, parameter T) schema.CustomizeDiffFunc { @@ -99,12 +97,12 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key if v, ok := resourceSchema[changedKey]; ok { if diffSuppressFunc := v.DiffSuppressFunc; diffSuppressFunc != nil { - hackedResourceData, hackedCorrectly := hackResourceData(resourceSchema, diff) - if !hackedCorrectly { + 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), hackedResourceData) { + 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 { @@ -121,27 +119,6 @@ func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key }) } -// TODO: extract hacky stuff outside this directory? -func hackResourceData(resourceSchema schema.InternalMap, diff *schema.ResourceDiff) (*schema.ResourceData, bool) { - unexportedState := reflect.ValueOf(diff).Elem().FieldByName("state") - hackedState := reflect.NewAt(unexportedState.Type(), unsafe.Pointer(unexportedState.UnsafeAddr())).Elem().Interface() - unexportedDiff := reflect.ValueOf(diff).Elem().FieldByName("diff") - hackedDiff := reflect.NewAt(unexportedDiff.Type(), unsafe.Pointer(unexportedDiff.UnsafeAddr())).Elem().Interface() - castState, ok := hackedState.(*terraform.InstanceState) - if !ok { - return nil, false - } - castDiff, ok := hackedDiff.(*terraform.InstanceDiff) - if !ok { - return nil, false - } - hackedResourceData, err := resourceSchema.Data(castState, castDiff) - if err != nil { - return nil, false - } - return hackedResourceData, true -} - type parameter[T ~string] struct { parameterName T valueType valueType diff --git a/pkg/resources/warehouse_acceptance_test.go b/pkg/resources/warehouse_acceptance_test.go index 1d737d39cd..2d49146eed 100644 --- a/pkg/resources/warehouse_acceptance_test.go +++ b/pkg/resources/warehouse_acceptance_test.go @@ -3,6 +3,7 @@ package resources_test import ( "fmt" "regexp" + "strconv" "strings" "testing" @@ -290,6 +291,15 @@ func TestAcc_Warehouse_BasicFlows(t *testing.T) { resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.statement_timeout_in_seconds.0.level", string(sdk.ParameterTypeWarehouse)), ), }, + // change resource monitor - wrap in quotes (no change expected) + { + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + Config: warehouseFullConfigNoDefaultsStringId(name2, newComment, strconv.Quote(resourceMonitorId.FullyQualifiedName())), + }, // CHANGE max_concurrency_level EXTERNALLY (proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2318) { PreConfig: func() { acc.TestClient().Warehouse.UpdateMaxConcurrencyLevel(t, warehouseId2, 10) }, @@ -2110,6 +2120,30 @@ resource "snowflake_warehouse" "w" { `, name, comment, id.Name()) } +func warehouseFullConfigNoDefaultsStringId(name string, comment string, id string) string { + return fmt.Sprintf(` +resource "snowflake_warehouse" "w" { + name = "%[1]s" + warehouse_type = "SNOWPARK-OPTIMIZED" + warehouse_size = "MEDIUM" + max_cluster_count = 4 + min_cluster_count = 2 + scaling_policy = "ECONOMY" + auto_suspend = 1200 + auto_resume = false + initially_suspended = false + resource_monitor = %[3]s + comment = "%[2]s" + enable_query_acceleration = true + query_acceleration_max_scale_factor = 4 + + max_concurrency_level = 4 + statement_queued_timeout_in_seconds = 5 + statement_timeout_in_seconds = 86400 +} +`, name, comment, id) +} + func warehouseWithSizeConfig(name string, size string) string { return fmt.Sprintf(` resource "snowflake_warehouse" "w" { From 24aaeebac42eb8e872c99245287901e94c54bc7a Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 10:38:11 +0200 Subject: [PATCH 38/42] rework users datasource starts here From b803f0b9d29f34d95177cc2cc91ea769e9743f5d Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Mon, 2 Sep 2024 17:27:30 +0200 Subject: [PATCH 39/42] fix custom diffs for fields with diff supression starts here From da45481f96e1661abfbd97e6e4615d82ab9212ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Tue, 3 Sep 2024 13:47:26 +0200 Subject: [PATCH 40/42] Add user improvements --- docs/data-sources/users.md | 1 + .../assert/objectassert/user_snowflake_ext.go | 11 ++ pkg/datasources/users_acceptance_test.go | 2 + pkg/resources/user_acceptance_test.go | 105 ++++++++++++++ pkg/resources/user_parameters.go | 38 +++-- pkg/schemas/user_describe.go | 7 + pkg/sdk/parameters.go | 2 +- pkg/sdk/testint/users_integration_test.go | 133 +++++++++++++++++- pkg/sdk/users.go | 8 +- 9 files changed, 282 insertions(+), 25 deletions(-) diff --git a/docs/data-sources/users.md b/docs/data-sources/users.md index 3ba291158e..4332954bda 100644 --- a/docs/data-sources/users.md +++ b/docs/data-sources/users.md @@ -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 index f6dcf68819..7648c2403e 100644 --- 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: %v", o.Owner) + } + 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_acceptance_test.go b/pkg/datasources/users_acceptance_test.go index 1ea103e079..4b2b5e1390 100644 --- a/pkg/datasources/users_acceptance_test.go +++ b/pkg/datasources/users_acceptance_test.go @@ -96,6 +96,7 @@ func TestAcc_Users_Complete(t *testing.T) { assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.disabled", "false")), assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.snowflake_lock", "false")), assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.snowflake_support", "false")), + assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.has_mfa", "false")), assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.describe_output.0.days_to_expiry")), assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.describe_output.0.mins_to_unlock")), assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.default_warehouse", "some_warehouse")), @@ -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 index 50dace3a99..2cef1cea23 100644 --- 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())), + ), + }, + }, + }) +} diff --git a/pkg/resources/user_parameters.go b/pkg/resources/user_parameters.go index 498c8d7bcd..968f2c7c3c 100644 --- a/pkg/resources/user_parameters.go +++ b/pkg/resources/user_parameters.go @@ -76,13 +76,12 @@ var ( ) ) -// TODO [SNOW-1348101 - next PR]: uncomment DiffSuppress and ValidateDiag type parameterDef[T ~string] struct { - Name T - Type schema.ValueType - Description string - // DiffSuppress schema.SchemaDiffSuppressFunc - // ValidateDiag schema.SchemaValidateDiagFunc + Name T + Type schema.ValueType + Description string + DiffSuppress schema.SchemaDiffSuppressFunc + ValidateDiag schema.SchemaValidateDiagFunc } func init() { @@ -91,8 +90,8 @@ func init() { // session params {Name: sdk.UserParameterAbortDetachedQuery, Type: schema.TypeBool, Description: "Specifies the action that Snowflake performs for in-progress queries if connectivity is lost due to abrupt termination of a session (e.g. network outage, browser termination, service interruption)."}, {Name: sdk.UserParameterAutocommit, Type: schema.TypeBool, Description: "Specifies whether autocommit is enabled for the session. Autocommit determines whether a DML statement, when executed without an active transaction, is automatically committed after the statement successfully completes. For more information, see [Transactions](https://docs.snowflake.com/en/sql-reference/transactions)."}, - {Name: sdk.UserParameterBinaryInputFormat, Type: schema.TypeString, Description: "The format of VARCHAR values passed as input to VARCHAR-to-BINARY conversion functions. For more information, see [Binary input and output](https://docs.snowflake.com/en/sql-reference/binary-input-output)."}, - {Name: sdk.UserParameterBinaryOutputFormat, Type: schema.TypeString, Description: "The format for VARCHAR values returned as output by BINARY-to-VARCHAR conversion functions. For more information, see [Binary input and output](https://docs.snowflake.com/en/sql-reference/binary-input-output)."}, + {Name: sdk.UserParameterBinaryInputFormat, Type: schema.TypeString, ValidateDiag: sdkValidation(sdk.ToBinaryInputFormat), DiffSuppress: NormalizeAndCompare(sdk.ToBinaryInputFormat), Description: "The format of VARCHAR values passed as input to VARCHAR-to-BINARY conversion functions. For more information, see [Binary input and output](https://docs.snowflake.com/en/sql-reference/binary-input-output)."}, + {Name: sdk.UserParameterBinaryOutputFormat, Type: schema.TypeString, ValidateDiag: sdkValidation(sdk.ToBinaryOutputFormat), DiffSuppress: NormalizeAndCompare(sdk.ToBinaryOutputFormat), Description: "The format for VARCHAR values returned as output by BINARY-to-VARCHAR conversion functions. For more information, see [Binary input and output](https://docs.snowflake.com/en/sql-reference/binary-input-output)."}, {Name: sdk.UserParameterClientMemoryLimit, Type: schema.TypeInt, Description: "Parameter that specifies the maximum amount of memory the JDBC driver or ODBC driver should use for the result set from queries (in MB)."}, {Name: sdk.UserParameterClientMetadataRequestUseConnectionCtx, Type: schema.TypeBool, Description: "For specific ODBC functions and JDBC methods, this parameter can change the default search scope from all databases/schemas to the current database/schema. The narrower search typically returns fewer rows and executes more quickly."}, {Name: sdk.UserParameterClientPrefetchThreads, Type: schema.TypeInt, Description: "Parameter that specifies the number of threads used by the client to pre-fetch large result sets. The driver will attempt to honor the parameter value, but defines the minimum and maximum values (depending on your system’s resources) to improve performance."}, @@ -106,14 +105,14 @@ func init() { {Name: sdk.UserParameterEnableUnloadPhysicalTypeOptimization, Type: schema.TypeBool, Description: "Specifies whether to set the schema for unloaded Parquet files based on the logical column data types (i.e. the types in the unload SQL query or source table) or on the unloaded column values (i.e. the smallest data types and precision that support the values in the output columns of the unload SQL statement or source table)."}, {Name: sdk.UserParameterErrorOnNondeterministicMerge, Type: schema.TypeBool, Description: "Specifies whether to return an error when the [MERGE](https://docs.snowflake.com/en/sql-reference/sql/merge) command is used to update or delete a target row that joins multiple source rows and the system cannot determine the action to perform on the target row."}, {Name: sdk.UserParameterErrorOnNondeterministicUpdate, Type: schema.TypeBool, Description: "Specifies whether to return an error when the [UPDATE](https://docs.snowflake.com/en/sql-reference/sql/update) command is used to update a target row that joins multiple source rows and the system cannot determine the action to perform on the target row."}, - {Name: sdk.UserParameterGeographyOutputFormat, Type: schema.TypeString, Description: "Display format for [GEOGRAPHY values](https://docs.snowflake.com/en/sql-reference/data-types-geospatial.html#label-data-types-geography)."}, - {Name: sdk.UserParameterGeometryOutputFormat, Type: schema.TypeString, Description: "Display format for [GEOMETRY values](https://docs.snowflake.com/en/sql-reference/data-types-geospatial.html#label-data-types-geometry)."}, + {Name: sdk.UserParameterGeographyOutputFormat, Type: schema.TypeString, ValidateDiag: sdkValidation(sdk.ToGeographyOutputFormat), DiffSuppress: NormalizeAndCompare(sdk.ToGeographyOutputFormat), Description: "Display format for [GEOGRAPHY values](https://docs.snowflake.com/en/sql-reference/data-types-geospatial.html#label-data-types-geography)."}, + {Name: sdk.UserParameterGeometryOutputFormat, Type: schema.TypeString, ValidateDiag: sdkValidation(sdk.ToGeometryOutputFormat), DiffSuppress: NormalizeAndCompare(sdk.ToGeometryOutputFormat), Description: "Display format for [GEOMETRY values](https://docs.snowflake.com/en/sql-reference/data-types-geospatial.html#label-data-types-geometry)."}, {Name: sdk.UserParameterJdbcTreatDecimalAsInt, Type: schema.TypeBool, Description: "Specifies how JDBC processes columns that have a scale of zero (0)."}, {Name: sdk.UserParameterJdbcTreatTimestampNtzAsUtc, Type: schema.TypeBool, Description: "Specifies how JDBC processes TIMESTAMP_NTZ values."}, {Name: sdk.UserParameterJdbcUseSessionTimezone, Type: schema.TypeBool, Description: "Specifies whether the JDBC Driver uses the time zone of the JVM or the time zone of the session (specified by the [TIMEZONE](https://docs.snowflake.com/en/sql-reference/parameters#label-timezone) parameter) for the getDate(), getTime(), and getTimestamp() methods of the ResultSet class."}, {Name: sdk.UserParameterJsonIndent, Type: schema.TypeInt, Description: "Specifies the number of blank spaces to indent each new element in JSON output in the session. Also specifies whether to insert newline characters after each element."}, {Name: sdk.UserParameterLockTimeout, Type: schema.TypeInt, Description: "Number of seconds to wait while trying to lock a resource, before timing out and aborting the statement."}, - {Name: sdk.UserParameterLogLevel, Type: schema.TypeString, Description: "Specifies the severity level of messages that should be ingested and made available in the active event table. Messages at the specified level (and at more severe levels) are ingested. For more information about log levels, see [Setting log level](https://docs.snowflake.com/en/developer-guide/logging-tracing/logging-log-level)."}, + {Name: sdk.UserParameterLogLevel, Type: schema.TypeString, ValidateDiag: sdkValidation(sdk.ToLogLevel), DiffSuppress: NormalizeAndCompare(sdk.ToLogLevel), Description: "Specifies the severity level of messages that should be ingested and made available in the active event table. Messages at the specified level (and at more severe levels) are ingested. For more information about log levels, see [Setting log level](https://docs.snowflake.com/en/developer-guide/logging-tracing/logging-log-level)."}, {Name: sdk.UserParameterMultiStatementCount, Type: schema.TypeInt, Description: "Number of statements to execute when using the multi-statement capability."}, {Name: sdk.UserParameterNoorderSequenceAsDefault, Type: schema.TypeBool, Description: "Specifies whether the ORDER or NOORDER property is set by default when you create a new sequence or add a new table column. The ORDER and NOORDER properties determine whether or not the values are generated for the sequence or auto-incremented column in [increasing or decreasing order](https://docs.snowflake.com/en/user-guide/querying-sequences.html#label-querying-sequences-increasing-values)."}, {Name: sdk.UserParameterOdbcTreatDecimalAsInt, Type: schema.TypeBool, Description: "Specifies how ODBC processes columns that have a scale of zero (0)."}, @@ -131,12 +130,12 @@ func init() { {Name: sdk.UserParameterTimestampLtzOutputFormat, Type: schema.TypeString, Description: "Specifies the display format for the TIMESTAMP_LTZ data type. If no format is specified, defaults to [TIMESTAMP_OUTPUT_FORMAT](https://docs.snowflake.com/en/sql-reference/parameters#label-timestamp-output-format). For more information, see [Date and time input and output formats](https://docs.snowflake.com/en/sql-reference/date-time-input-output)."}, {Name: sdk.UserParameterTimestampNtzOutputFormat, Type: schema.TypeString, Description: "Specifies the display format for the TIMESTAMP_NTZ data type."}, {Name: sdk.UserParameterTimestampOutputFormat, Type: schema.TypeString, Description: "Specifies the display format for the TIMESTAMP data type alias. For more information, see [Date and time input and output formats](https://docs.snowflake.com/en/sql-reference/date-time-input-output)."}, - {Name: sdk.UserParameterTimestampTypeMapping, Type: schema.TypeString, Description: "Specifies the TIMESTAMP_* variation that the TIMESTAMP data type alias maps to."}, + {Name: sdk.UserParameterTimestampTypeMapping, Type: schema.TypeString, ValidateDiag: sdkValidation(sdk.ToTimestampTypeMapping), DiffSuppress: NormalizeAndCompare(sdk.ToTimestampTypeMapping), Description: "Specifies the TIMESTAMP_* variation that the TIMESTAMP data type alias maps to."}, {Name: sdk.UserParameterTimestampTzOutputFormat, Type: schema.TypeString, Description: "Specifies the display format for the TIMESTAMP_TZ data type. If no format is specified, defaults to [TIMESTAMP_OUTPUT_FORMAT](https://docs.snowflake.com/en/sql-reference/parameters#label-timestamp-output-format). For more information, see [Date and time input and output formats](https://docs.snowflake.com/en/sql-reference/date-time-input-output)."}, {Name: sdk.UserParameterTimezone, Type: schema.TypeString, Description: "Specifies the time zone for the session. You can specify a [time zone name](https://data.iana.org/time-zones/tzdb-2021a/zone1970.tab) or a [link name](https://data.iana.org/time-zones/tzdb-2021a/backward) from release 2021a of the [IANA Time Zone Database](https://www.iana.org/time-zones) (e.g. America/Los_Angeles, Europe/London, UTC, Etc/GMT, etc.)."}, {Name: sdk.UserParameterTimeInputFormat, Type: schema.TypeString, Description: "Specifies the input format for the TIME data type. For more information, see [Date and time input and output formats](https://docs.snowflake.com/en/sql-reference/date-time-input-output). Any valid, supported time format or AUTO (AUTO specifies that Snowflake attempts to automatically detect the format of times stored in the system during the session)."}, {Name: sdk.UserParameterTimeOutputFormat, Type: schema.TypeString, Description: "Specifies the display format for the TIME data type. For more information, see [Date and time input and output formats](https://docs.snowflake.com/en/sql-reference/date-time-input-output)."}, - {Name: sdk.UserParameterTraceLevel, Type: schema.TypeString, Description: "Controls how trace events are ingested into the event table. For more information about trace levels, see [Setting trace level](https://docs.snowflake.com/en/developer-guide/logging-tracing/tracing-trace-level)."}, + {Name: sdk.UserParameterTraceLevel, Type: schema.TypeString, ValidateDiag: sdkValidation(sdk.ToTraceLevel), DiffSuppress: NormalizeAndCompare(sdk.ToTraceLevel), Description: "Controls how trace events are ingested into the event table. For more information about trace levels, see [Setting trace level](https://docs.snowflake.com/en/developer-guide/logging-tracing/tracing-trace-level)."}, {Name: sdk.UserParameterTransactionAbortOnError, Type: schema.TypeBool, Description: "Specifies the action to perform when a statement issued within a non-autocommit transaction returns with an error."}, {Name: sdk.UserParameterTransactionDefaultIsolationLevel, Type: schema.TypeString, Description: "Specifies the isolation level for transactions in the user session."}, {Name: sdk.UserParameterTwoDigitCenturyStart, Type: schema.TypeInt, Description: "Specifies the “century start” year for 2-digit years (i.e. the earliest year such dates can represent). This parameter prevents ambiguous dates when importing or converting data with the `YY` date format component (i.e. years represented as 2 digits)."}, @@ -154,13 +153,12 @@ func init() { fieldName := strings.ToLower(string(field.Name)) userParametersSchema[fieldName] = &schema.Schema{ - Type: field.Type, - Description: enrichWithReferenceToParameterDocs(field.Name, field.Description), - Computed: true, - Optional: true, - // TODO [SNOW-1348101 - next PR]: uncomment and fill out - // ValidateDiagFunc: field.ValidateDiag, - // DiffSuppressFunc: field.DiffSuppress, + Type: field.Type, + Description: enrichWithReferenceToParameterDocs(field.Name, field.Description), + Computed: true, + Optional: true, + ValidateDiagFunc: field.ValidateDiag, + DiffSuppressFunc: field.DiffSuppress, } } } diff --git a/pkg/schemas/user_describe.go b/pkg/schemas/user_describe.go index 8292999d39..acefcfed12 100644 --- a/pkg/schemas/user_describe.go +++ b/pkg/schemas/user_describe.go @@ -127,6 +127,10 @@ var UserDescribeSchema = map[string]*schema.Schema{ Type: schema.TypeBool, Computed: true, }, + "has_mfa": { + Type: schema.TypeBool, + Computed: true, + }, } var _ = UserDescribeSchema @@ -223,6 +227,9 @@ func UserDescriptionToSchema(userDetails sdk.UserDetails) []map[string]any { if userDetails.CustomLandingPageUrlFlushNextUiLoad != nil { userDetailsSchema["custom_landing_page_url_flush_next_ui_load"] = userDetails.CustomLandingPageUrlFlushNextUiLoad.Value } + if userDetails.HasMfa != nil { + userDetailsSchema["has_mfa"] = userDetails.HasMfa.Value + } return []map[string]any{ userDetailsSchema, } diff --git a/pkg/sdk/parameters.go b/pkg/sdk/parameters.go index 53a2a2bed4..626dfcf1d7 100644 --- a/pkg/sdk/parameters.go +++ b/pkg/sdk/parameters.go @@ -832,7 +832,7 @@ func ToBinaryInputFormat(s string) (BinaryInputFormat, error) { return BinaryInputFormatHex, nil case string(BinaryInputFormatBase64): return BinaryInputFormatBase64, nil - case string(BinaryInputFormatUTF8): + case string(BinaryInputFormatUTF8), "UTF-8": return BinaryInputFormatUTF8, nil default: return "", fmt.Errorf("invalid binary input format: %s", s) diff --git a/pkg/sdk/testint/users_integration_test.go b/pkg/sdk/testint/users_integration_test.go index 43c17f4328..759ea08189 100644 --- a/pkg/sdk/testint/users_integration_test.go +++ b/pkg/sdk/testint/users_integration_test.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" "testing" + "time" assertions "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" @@ -1034,7 +1035,6 @@ func TestInt_Users(t *testing.T) { // This test proves issue https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2817. // sql: Scan error on column index 10, name "disabled": sql/driver: couldn't convert "null" into type bool - // TODO [SNOW-1348101 - next PR]: test what is visible if everything is set if there is no ownership on the active role t.Run("issue #2817: handle show properly without OWNERSHIP and MANAGE GRANTS", func(t *testing.T) { disabledUser, disabledUserCleanup := testClientHelper().User.CreateUserWithOptions(t, testClientHelper().Ids.RandomAccountObjectIdentifier(), &sdk.CreateUserOptions{ObjectProperties: &sdk.UserObjectProperties{Disable: sdk.Bool(true)}}) t.Cleanup(disabledUserCleanup) @@ -1074,6 +1074,137 @@ func TestInt_Users(t *testing.T) { require.Nil(t, fetchedDisabledUserDetails) }) + t.Run("issue #2817: check what fields are available when using a user with insufficient privileges to fully inspect another user", func(t *testing.T) { + id := testClientHelper().Ids.RandomAccountObjectIdentifier() + err := client.Users.Create(ctx, id, &sdk.CreateUserOptions{ + SessionParameters: &sdk.SessionParameters{ + AbortDetachedQuery: sdk.Bool(true), + Autocommit: sdk.Bool(false), + BinaryInputFormat: sdk.Pointer(sdk.BinaryInputFormatUTF8), + BinaryOutputFormat: sdk.Pointer(sdk.BinaryOutputFormatBase64), + ClientMemoryLimit: sdk.Int(1024), + ClientMetadataRequestUseConnectionCtx: sdk.Bool(true), + ClientPrefetchThreads: sdk.Int(2), + ClientResultChunkSize: sdk.Int(48), + ClientResultColumnCaseInsensitive: sdk.Bool(true), + ClientSessionKeepAlive: sdk.Bool(true), + ClientSessionKeepAliveHeartbeatFrequency: sdk.Int(2400), + ClientTimestampTypeMapping: sdk.Pointer(sdk.ClientTimestampTypeMappingNtz), + DateInputFormat: sdk.String("YYYY-MM-DD"), + DateOutputFormat: sdk.String("YY-MM-DD"), + EnableUnloadPhysicalTypeOptimization: sdk.Bool(false), + ErrorOnNondeterministicMerge: sdk.Bool(false), + ErrorOnNondeterministicUpdate: sdk.Bool(true), + GeographyOutputFormat: sdk.Pointer(sdk.GeographyOutputFormatWKB), + GeometryOutputFormat: sdk.Pointer(sdk.GeometryOutputFormatWKB), + JdbcTreatDecimalAsInt: sdk.Bool(false), + JdbcTreatTimestampNtzAsUtc: sdk.Bool(true), + JdbcUseSessionTimezone: sdk.Bool(false), + JSONIndent: sdk.Int(4), + LockTimeout: sdk.Int(21222), + LogLevel: sdk.Pointer(sdk.LogLevelError), + MultiStatementCount: sdk.Int(0), + NoorderSequenceAsDefault: sdk.Bool(false), + OdbcTreatDecimalAsInt: sdk.Bool(true), + QueryTag: sdk.String("some_tag"), + QuotedIdentifiersIgnoreCase: sdk.Bool(true), + RowsPerResultset: sdk.Int(2), + S3StageVpceDnsName: sdk.String("vpce-id.s3.region.vpce.amazonaws.com"), + SearchPath: sdk.String("$public, $current"), + SimulatedDataSharingConsumer: sdk.String("some_consumer"), + StatementQueuedTimeoutInSeconds: sdk.Int(10), + StatementTimeoutInSeconds: sdk.Int(10), + StrictJSONOutput: sdk.Bool(true), + TimestampDayIsAlways24h: sdk.Bool(true), + TimestampInputFormat: sdk.String("YYYY-MM-DD"), + TimestampLTZOutputFormat: sdk.String("YYYY-MM-DD HH24:MI:SS"), + TimestampNTZOutputFormat: sdk.String("YYYY-MM-DD HH24:MI:SS"), + TimestampOutputFormat: sdk.String("YYYY-MM-DD HH24:MI:SS"), + TimestampTypeMapping: sdk.Pointer(sdk.TimestampTypeMappingLtz), + TimestampTZOutputFormat: sdk.String("YYYY-MM-DD HH24:MI:SS"), + Timezone: sdk.String("Europe/Warsaw"), + TimeInputFormat: sdk.String("HH24:MI"), + TimeOutputFormat: sdk.String("HH24:MI"), + TraceLevel: sdk.Pointer(sdk.TraceLevelOnEvent), + TransactionAbortOnError: sdk.Bool(true), + TransactionDefaultIsolationLevel: sdk.Pointer(sdk.TransactionDefaultIsolationLevelReadCommitted), + TwoDigitCenturyStart: sdk.Int(1980), + UnsupportedDDLAction: sdk.Pointer(sdk.UnsupportedDDLActionFail), + UseCachedResult: sdk.Bool(false), + WeekOfYearPolicy: sdk.Int(1), + WeekStart: sdk.Int(1), + }, + ObjectParameters: &sdk.UserObjectParameters{ + EnableUnredactedQuerySyntaxError: sdk.Bool(true), + NetworkPolicy: sdk.Pointer(networkPolicy.ID()), + PreventUnloadToInternalStages: sdk.Bool(true), + }, + ObjectProperties: &sdk.UserObjectProperties{ + Password: sdk.String(password), + LoginName: sdk.String(newValue), + DisplayName: sdk.String(newValue), + FirstName: sdk.String(newValue), + MiddleName: sdk.String(newValue), + LastName: sdk.String(newValue), + Email: sdk.String(email), + MustChangePassword: sdk.Bool(true), + Disable: sdk.Bool(true), + DaysToExpiry: sdk.Int(5), + MinsToUnlock: sdk.Int(15), + DefaultWarehouse: sdk.Pointer(warehouseId), + DefaultNamespace: sdk.Pointer(schemaIdObjectIdentifier), + DefaultRole: sdk.Pointer(roleId), + DefaultSecondaryRoles: &sdk.SecondaryRoles{}, + MinsToBypassMFA: sdk.Int(30), + RSAPublicKey: sdk.String(key), + RSAPublicKey2: sdk.String(key2), + Comment: sdk.String("some comment"), + }, + }) + require.NoError(t, err) + t.Cleanup(testClientHelper().User.DropUserFunc(t, id)) + + role, roleCleanup := testClientHelper().Role.CreateRoleGrantedToCurrentUser(t) + t.Cleanup(roleCleanup) + + revertRole := testClientHelper().Role.UseRole(t, role.ID()) + t.Cleanup(revertRole) + + // Describe won't work and parameters are not affected by that fact + assertParametersSet(objectparametersassert.UserParameters(t, id)) + + assertions.AssertThatObject(t, objectassert.UserForIntegrationTests(t, id, testClientHelper()). + HasName(id.Name()). + HasCreatedOnNotEmpty(). + HasLoginName(""). + HasDisplayName(""). + HasFirstName(""). + HasLastName(""). + HasEmail(""). + HasMinsToUnlock(""). + HasDaysToExpiry(""). + HasComment(""). + HasDisabled(false). // underlying null + HasMustChangePassword(false). // underlying null + HasSnowflakeLock(false). // underlying null + HasDefaultWarehouse(""). + HasDefaultNamespace(""). + HasDefaultRole(""). + HasDefaultSecondaryRoles(""). + HasExtAuthnDuo(false). // underlying null + HasExtAuthnUid(""). + HasMinsToBypassMfa(""). + HasOwnerNotEmpty(). + HasLastSuccessLogin(time.Time{}). // underlying null + HasExpiresAtTimeNotEmpty(). + HasLockedUntilTimeNotEmpty(). + HasHasPassword(false). + HasHasRsaPublicKey(false). + HasType(""). // underlying null + HasHasMfa(false), + ) + }) + t.Run("login_name and display_name inconsistencies", func(t *testing.T) { id := testClientHelper().Ids.RandomAccountObjectIdentifier() diff --git a/pkg/sdk/users.go b/pkg/sdk/users.go index 8db4239978..ba1b98ef3d 100644 --- a/pkg/sdk/users.go +++ b/pkg/sdk/users.go @@ -91,9 +91,8 @@ type userDBRow struct { HasPassword bool `db:"has_password"` HasRsaPublicKey bool `db:"has_rsa_public_key"` // TODO [SNOW-1645348]: test type thoroughly - Type sql.NullString `db:"type"` - // TODO [SNOW-1348101 - next PR]: test if hasMfa is always non-nullable, check if this has mfa helps with disable mfa, add to the describe output too - HasMfa bool `db:"has_mfa"` + Type sql.NullString `db:"type"` + HasMfa bool `db:"has_mfa"` } func (row userDBRow) convert() *User { @@ -465,6 +464,7 @@ type UserDetails struct { PasswordLastSetTime *StringProperty CustomLandingPageUrl *StringProperty CustomLandingPageUrlFlushNextUiLoad *BoolProperty + HasMfa *BoolProperty } func userDetailsFromRows(rows []propertyRow) *UserDetails { @@ -531,6 +531,8 @@ func userDetailsFromRows(rows []propertyRow) *UserDetails { v.CustomLandingPageUrl = row.toStringProperty() case "CUSTOM_LANDING_PAGE_URL_FLUSH_NEXT_UI_LOAD": v.CustomLandingPageUrlFlushNextUiLoad = row.toBoolProperty() + case "HAS_MFA": + v.HasMfa = row.toBoolProperty() } } return v From fc1e8ce207806eabf850937bfa7c170893b7c9ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Tue, 3 Sep 2024 13:50:06 +0200 Subject: [PATCH 41/42] Add user improvements --- pkg/datasources/users_acceptance_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/datasources/users_acceptance_test.go b/pkg/datasources/users_acceptance_test.go index 4b2b5e1390..cb5c39e3d7 100644 --- 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())), @@ -96,7 +97,6 @@ func TestAcc_Users_Complete(t *testing.T) { assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.disabled", "false")), assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.snowflake_lock", "false")), assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.snowflake_support", "false")), - assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.has_mfa", "false")), assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.describe_output.0.days_to_expiry")), assert.Check(resource.TestCheckResourceAttrSet("data.snowflake_users.test", "users.0.describe_output.0.mins_to_unlock")), assert.Check(resource.TestCheckResourceAttr("data.snowflake_users.test", "users.0.describe_output.0.default_warehouse", "some_warehouse")), From 74107d8fc469b27f27fac1be63803e9a2a903c93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Tue, 3 Sep 2024 14:23:18 +0200 Subject: [PATCH 42/42] Add user improvements --- MIGRATION_GUIDE.md | 4 ++++ docs/data-sources/users.md | 4 ++-- .../bettertestspoc/assert/objectassert/user_snowflake_ext.go | 2 +- pkg/datasources/users.go | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 90a84e5c2b..de776066e7 100644 --- a/MIGRATION_GUIDE.md +++ b/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` diff --git a/docs/data-sources/users.md b/docs/data-sources/users.md index 4332954bda..e59fc18b17 100644 --- 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 diff --git a/pkg/acceptance/bettertestspoc/assert/objectassert/user_snowflake_ext.go b/pkg/acceptance/bettertestspoc/assert/objectassert/user_snowflake_ext.go index 7648c2403e..328f7a7c56 100644 --- a/pkg/acceptance/bettertestspoc/assert/objectassert/user_snowflake_ext.go +++ b/pkg/acceptance/bettertestspoc/assert/objectassert/user_snowflake_ext.go @@ -55,7 +55,7 @@ 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: %v", o.Owner) + return fmt.Errorf("expected owner not empty; got empty") } return nil }) diff --git a/pkg/datasources/users.go b/pkg/datasources/users.go index 18fe56371a..a2e576a2ac 100644 --- 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.", } }