From 2ca465adcc6d28f4e10f42bae15462075a6010de Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Wed, 4 Sep 2024 18:41:56 +0200 Subject: [PATCH] fix: Fix default secondary roles after BCR 2024_07 (#3040) - fix warehouse resume - add system functions to enable/disable BCR - change default secondary roles handling - address BCR 2024_07 for default secondary roles References: #3038 --- MIGRATION_GUIDE.md | 20 +- docs/resources/user.md | 4 +- examples/resources/snowflake_user/resource.tf | 2 +- .../resourceassert/user_resource_ext.go | 18 +- .../resourceassert/user_resource_gen.go | 8 +- .../config/model/user_model_ext.go | 6 +- .../config/model/user_model_gen.go | 11 +- pkg/acceptance/helpers/bcr_bundles_client.go | 43 +++ pkg/acceptance/helpers/test_client.go | 2 + pkg/datasources/users_acceptance_test.go | 3 +- pkg/resources/diff_suppressions.go | 4 +- pkg/resources/user.go | 80 ++-- pkg/resources/user_acceptance_test.go | 361 ++++++++++++++++-- pkg/resources/user_state_upgraders.go | 27 ++ pkg/resources/validators_test.go | 51 --- pkg/sdk/system_functions.go | 12 + .../system_functions_integration_test.go | 16 + pkg/sdk/testint/users_integration_test.go | 93 ++++- pkg/sdk/users.go | 64 +++- pkg/sdk/users_test.go | 157 +++++++- pkg/sdk/warehouses.go | 2 +- 21 files changed, 821 insertions(+), 163 deletions(-) create mode 100644 pkg/acceptance/helpers/bcr_bundles_client.go create mode 100644 pkg/resources/user_state_upgraders.go diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index d6a0c292c2..22d421a5e7 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -153,6 +153,8 @@ Connected issues: [#3007](https://github.com/Snowflake-Labs/terraform-provider-s ### snowflake_user resource changes +Because of the multiple changes in the resource, the easiest migration way is to follow our [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/resource_migration.md) to perform zero downtime migration. Alternatively, it is possible to follow some pointers below. Either way, familiarize yourself with the resource changes before version bumping. Also, check the [design decisions](./v1-preparations/CHANGES_BEFORE_V1.md). + #### *(breaking change)* user parameters added to snowflake_user resource On our road to V1 we changed the approach to Snowflake parameters on the object level; now, we add them directly to the resource. This is a **breaking change** because now: @@ -243,6 +245,19 @@ Not every attribute can be updated in the state during read (like `password` in Connected issues: [#2970](https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/2970) +#### *(breaking change)* Handling default secondary roles + +Old field `default_secondary_roles` was removed in favour of the new, easier, `default_secondary_roles_option` because the only possible options that can be currently set are `('ALL')` and `()`. The logic to handle set element changes was convoluted and error-prone. Additionally, [bcr 2024_07](https://docs.snowflake.com/en/release-notes/bcr-bundles/2024_07/bcr-1692) complicated the matter even more. + +Now: +- the default value is `DEFAULT` - it falls back to Snowflake default (so `()` before and `('ALL')` after the BCR) +- to explicitly set to `('ALL')` use `ALL` +- to explicitly set to `()` use `NONE` + +While migrating, the old `default_secondary_roles` will be removed from the state automatically and `default_secondary_roles_option` will be constructed based on the previous value (in some cases apply may be necessary). + +Connected issues: [#3038](https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/3038) + #### *(breaking change)* Attributes changes Attributes that are no longer computed: @@ -257,11 +272,13 @@ New fields: - `mins_to_unlock` - `mins_to_bypass_mfa` - `disable_mfa` +- `default_secondary_roles_option` - `show_output` - holds the response from `SHOW USERS`. Remember that the field will be only recomputed if one of the user attributes is changed. - `parameters` - holds the response from `SHOW PARAMETERS IN USER`. Removed fields: - `has_rsa_public_key` +- `default_secondary_roles` - replaced with `default_secondary_roles_option` Default changes: - `must_change_password` @@ -271,9 +288,6 @@ Type changes: - `must_change_password`: bool -> string (To easily handle three-value logic (true, false, unknown) in provider's configs, read more in https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/751239b7d2fee4757471db6c03b952d4728ee099/v1-preparations/CHANGES_BEFORE_V1.md?plain=1#L24) - `disabled`: bool -> string (To easily handle three-value logic (true, false, unknown) in provider's configs, read more in https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/751239b7d2fee4757471db6c03b952d4728ee099/v1-preparations/CHANGES_BEFORE_V1.md?plain=1#L24) -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`. diff --git a/docs/resources/user.md b/docs/resources/user.md index d28d978cbe..c265d27851 100644 --- a/docs/resources/user.md +++ b/docs/resources/user.md @@ -34,7 +34,7 @@ resource "snowflake_user" "user" { last_name = "User" default_warehouse = "warehouse" - default_secondary_roles = ["ALL"] + default_secondary_roles = "ALL" default_role = "role1" rsa_public_key = "..." @@ -73,7 +73,7 @@ resource "snowflake_user" "user" { - `days_to_expiry` (Number) Specifies the number of days after which the user status is set to `Expired` and the user is no longer allowed to log in. This is useful for defining temporary users (i.e. users who should only have access to Snowflake for a limited time period). In general, you should not set this property for [account administrators](https://docs.snowflake.com/en/user-guide/security-access-control-considerations.html#label-accountadmin-users) (i.e. users with the `ACCOUNTADMIN` role) because Snowflake locks them out when they become `Expired`. External changes for this field won't be detected. In case you want to apply external changes, you can re-create the resource manually using "terraform taint". - `default_namespace` (String) Specifies the namespace (database only or database and schema) that is active by default for the user’s session upon login. Note that the CREATE USER operation does not verify that the namespace exists. - `default_role` (String) Specifies the role that is active by default for the user’s session upon login. Note that specifying a default role for a user does **not** grant the role to the user. The role must be granted explicitly to the user using the [GRANT ROLE](https://docs.snowflake.com/en/sql-reference/sql/grant-role) command. In addition, the CREATE USER operation does not verify that the role exists. -- `default_secondary_roles` (Set of String) Specifies the set of secondary roles that are active for the user’s session upon login. Currently only ["ALL"] value is supported - more information can be found in [doc](https://docs.snowflake.com/en/sql-reference/sql/create-user#optional-object-properties-objectproperties). +- `default_secondary_roles_option` (String) Specifies the secondary roles that are active for the user’s session upon login. Valid values are (case-insensitive): `DEFAULT` | `NONE` | `ALL`. More information can be found in [doc](https://docs.snowflake.com/en/sql-reference/sql/create-user#optional-object-properties-objectproperties). - `default_warehouse` (String) Specifies the virtual warehouse that is active by default for the user’s session upon login. Note that the CREATE USER operation does not verify that the warehouse exists. - `disable_mfa` (String) Allows enabling or disabling [multi-factor authentication](https://docs.snowflake.com/en/user-guide/security-mfa). Available options are: "true" or "false". When the value is not set in the configuration the provider will put "default" there which means to use the Snowflake default for this value. External changes for this field won't be detected. In case you want to apply external changes, you can re-create the resource manually using "terraform taint". - `disabled` (String) Specifies whether the user is disabled, which prevents logging in and aborts all the currently-running queries for the user. Available options are: "true" or "false". When the value is not set in the configuration the provider will put "default" there which means to use the Snowflake default for this value. diff --git a/examples/resources/snowflake_user/resource.tf b/examples/resources/snowflake_user/resource.tf index 40eee9da30..3c44d99314 100644 --- a/examples/resources/snowflake_user/resource.tf +++ b/examples/resources/snowflake_user/resource.tf @@ -10,7 +10,7 @@ resource "snowflake_user" "user" { last_name = "User" default_warehouse = "warehouse" - default_secondary_roles = ["ALL"] + default_secondary_roles = "ALL" default_role = "role1" rsa_public_key = "..." diff --git a/pkg/acceptance/bettertestspoc/assert/resourceassert/user_resource_ext.go b/pkg/acceptance/bettertestspoc/assert/resourceassert/user_resource_ext.go index 26e2511674..076102926f 100644 --- a/pkg/acceptance/bettertestspoc/assert/resourceassert/user_resource_ext.go +++ b/pkg/acceptance/bettertestspoc/assert/resourceassert/user_resource_ext.go @@ -1,10 +1,10 @@ package resourceassert import ( - "fmt" "strconv" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" ) func (u *UserResourceAssert) HasDisabled(expected bool) *UserResourceAssert { @@ -17,19 +17,11 @@ func (u *UserResourceAssert) HasEmptyPassword() *UserResourceAssert { return u } -func (u *UserResourceAssert) HasDefaultSecondaryRoles(roles ...string) *UserResourceAssert { - for idx, role := range roles { - u.AddAssertion(assert.ValueSet(fmt.Sprintf("default_secondary_roles.%d", idx), role)) - } - return u -} - -func (u *UserResourceAssert) HasDefaultSecondaryRolesEmpty() *UserResourceAssert { - u.AddAssertion(assert.ValueSet("default_secondary_roles.#", "0")) - return u -} - func (u *UserResourceAssert) HasMustChangePassword(expected bool) *UserResourceAssert { u.AddAssertion(assert.ValueSet("must_change_password", strconv.FormatBool(expected))) return u } + +func (u *UserResourceAssert) HasDefaultSecondaryRolesOption(expected sdk.SecondaryRolesOption) *UserResourceAssert { + return u.HasDefaultSecondaryRolesOptionString(string(expected)) +} diff --git a/pkg/acceptance/bettertestspoc/assert/resourceassert/user_resource_gen.go b/pkg/acceptance/bettertestspoc/assert/resourceassert/user_resource_gen.go index d709986e7b..9bc2d1eb70 100644 --- a/pkg/acceptance/bettertestspoc/assert/resourceassert/user_resource_gen.go +++ b/pkg/acceptance/bettertestspoc/assert/resourceassert/user_resource_gen.go @@ -122,8 +122,8 @@ func (u *UserResourceAssert) HasDefaultRoleString(expected string) *UserResource return u } -func (u *UserResourceAssert) HasDefaultSecondaryRolesString(expected string) *UserResourceAssert { - u.AddAssertion(assert.ValueSet("default_secondary_roles", expected)) +func (u *UserResourceAssert) HasDefaultSecondaryRolesOptionString(expected string) *UserResourceAssert { + u.AddAssertion(assert.ValueSet("default_secondary_roles_option", expected)) return u } @@ -531,8 +531,8 @@ func (u *UserResourceAssert) HasNoDefaultRole() *UserResourceAssert { return u } -func (u *UserResourceAssert) HasNoDefaultSecondaryRoles() *UserResourceAssert { - u.AddAssertion(assert.ValueNotSet("default_secondary_roles")) +func (u *UserResourceAssert) HasNoDefaultSecondaryRolesOption() *UserResourceAssert { + u.AddAssertion(assert.ValueNotSet("default_secondary_roles_option")) return u } diff --git a/pkg/acceptance/bettertestspoc/config/model/user_model_ext.go b/pkg/acceptance/bettertestspoc/config/model/user_model_ext.go index 3912d76253..9bf0cc44de 100644 --- a/pkg/acceptance/bettertestspoc/config/model/user_model_ext.go +++ b/pkg/acceptance/bettertestspoc/config/model/user_model_ext.go @@ -4,7 +4,6 @@ import ( tfconfig "github.com/hashicorp/terraform-plugin-testing/config" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" ) @@ -67,7 +66,6 @@ func (u *UserModel) WithNullPassword() *UserModel { return u.WithPasswordValue(config.NullVariable()) } -func (u *UserModel) WithDefaultSecondaryRolesStringList(items ...string) *UserModel { - itemsAsVariables := collections.Map(items, func(s string) tfconfig.Variable { return tfconfig.StringVariable(s) }) - return u.WithDefaultSecondaryRolesValue(tfconfig.ListVariable(itemsAsVariables...)) +func (u *UserModel) WithDefaultSecondaryRolesOptionEnum(option sdk.SecondaryRolesOption) *UserModel { + return u.WithDefaultSecondaryRolesOption(string(option)) } diff --git a/pkg/acceptance/bettertestspoc/config/model/user_model_gen.go b/pkg/acceptance/bettertestspoc/config/model/user_model_gen.go index a5649d334a..32eb8218ee 100644 --- a/pkg/acceptance/bettertestspoc/config/model/user_model_gen.go +++ b/pkg/acceptance/bettertestspoc/config/model/user_model_gen.go @@ -28,7 +28,7 @@ type UserModel struct { DaysToExpiry tfconfig.Variable `json:"days_to_expiry,omitempty"` DefaultNamespace tfconfig.Variable `json:"default_namespace,omitempty"` DefaultRole tfconfig.Variable `json:"default_role,omitempty"` - DefaultSecondaryRoles tfconfig.Variable `json:"default_secondary_roles,omitempty"` + DefaultSecondaryRolesOption tfconfig.Variable `json:"default_secondary_roles_option,omitempty"` DefaultWarehouse tfconfig.Variable `json:"default_warehouse,omitempty"` DisableMfa tfconfig.Variable `json:"disable_mfa,omitempty"` Disabled tfconfig.Variable `json:"disabled,omitempty"` @@ -210,7 +210,10 @@ func (u *UserModel) WithDefaultRole(defaultRole string) *UserModel { return u } -// default_secondary_roles attribute type is not yet supported, so WithDefaultSecondaryRoles can't be generated +func (u *UserModel) WithDefaultSecondaryRolesOption(defaultSecondaryRolesOption string) *UserModel { + u.DefaultSecondaryRolesOption = tfconfig.StringVariable(defaultSecondaryRolesOption) + return u +} func (u *UserModel) WithDefaultWarehouse(defaultWarehouse string) *UserModel { u.DefaultWarehouse = tfconfig.StringVariable(defaultWarehouse) @@ -616,8 +619,8 @@ func (u *UserModel) WithDefaultRoleValue(value tfconfig.Variable) *UserModel { return u } -func (u *UserModel) WithDefaultSecondaryRolesValue(value tfconfig.Variable) *UserModel { - u.DefaultSecondaryRoles = value +func (u *UserModel) WithDefaultSecondaryRolesOptionValue(value tfconfig.Variable) *UserModel { + u.DefaultSecondaryRolesOption = value return u } diff --git a/pkg/acceptance/helpers/bcr_bundles_client.go b/pkg/acceptance/helpers/bcr_bundles_client.go new file mode 100644 index 0000000000..05a130f7fb --- /dev/null +++ b/pkg/acceptance/helpers/bcr_bundles_client.go @@ -0,0 +1,43 @@ +package helpers + +import ( + "context" + "testing" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/stretchr/testify/require" +) + +type BcrBundlesClient struct { + context *TestClientContext +} + +func NewBcrBundlesClient(context *TestClientContext) *BcrBundlesClient { + return &BcrBundlesClient{ + context: context, + } +} + +func (c *BcrBundlesClient) client() sdk.SystemFunctions { + return c.context.client.SystemFunctions +} + +func (c *BcrBundlesClient) EnableBcrBundle(t *testing.T, name string) { + t.Helper() + ctx := context.Background() + + err := c.client().EnableBehaviorChangeBundle(ctx, name) + require.NoError(t, err) + + t.Cleanup(c.DisableBcrBundleFunc(t, name)) +} + +func (c *BcrBundlesClient) DisableBcrBundleFunc(t *testing.T, name string) func() { + t.Helper() + ctx := context.Background() + + return func() { + err := c.client().DisableBehaviorChangeBundle(ctx, name) + require.NoError(t, err) + } +} diff --git a/pkg/acceptance/helpers/test_client.go b/pkg/acceptance/helpers/test_client.go index 2f68673466..21bfd5d015 100644 --- a/pkg/acceptance/helpers/test_client.go +++ b/pkg/acceptance/helpers/test_client.go @@ -15,6 +15,7 @@ type TestClient struct { ApiIntegration *ApiIntegrationClient Application *ApplicationClient ApplicationPackage *ApplicationPackageClient + BcrBundles *BcrBundlesClient Context *ContextClient CortexSearchService *CortexSearchServiceClient CatalogIntegration *CatalogIntegrationClient @@ -77,6 +78,7 @@ func NewTestClient(c *sdk.Client, database string, schema string, warehouse stri ApiIntegration: NewApiIntegrationClient(context, idsGenerator), Application: NewApplicationClient(context, idsGenerator), ApplicationPackage: NewApplicationPackageClient(context, idsGenerator), + BcrBundles: NewBcrBundlesClient(context), Context: NewContextClient(context), CortexSearchService: NewCortexSearchServiceClient(context, idsGenerator), CatalogIntegration: NewCatalogIntegrationClient(context, idsGenerator), diff --git a/pkg/datasources/users_acceptance_test.go b/pkg/datasources/users_acceptance_test.go index cb5c39e3d7..16993ce3b6 100644 --- a/pkg/datasources/users_acceptance_test.go +++ b/pkg/datasources/users_acceptance_test.go @@ -15,6 +15,7 @@ import ( "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/provider/resources" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/tfversion" ) @@ -43,7 +44,7 @@ func TestAcc_Users_Complete(t *testing.T) { WithDefaultWarehouse("some_warehouse"). WithDefaultNamespace("some.namespace"). WithDefaultRole("some_role"). - WithDefaultSecondaryRolesStringList("ALL"). + WithDefaultSecondaryRolesOptionEnum(sdk.SecondaryRolesOptionAll). WithMinsToBypassMfa(10). WithRsaPublicKey(key1). WithRsaPublicKey2(key2). diff --git a/pkg/resources/diff_suppressions.go b/pkg/resources/diff_suppressions.go index 5b47345c82..7f60139b5e 100644 --- a/pkg/resources/diff_suppressions.go +++ b/pkg/resources/diff_suppressions.go @@ -63,7 +63,7 @@ func SuppressCaseInSet(key string) schema.SchemaDiffSuppressFunc { return false } - if oldValue == "" && !d.GetRawState().IsNull() { + if oldValue == "" && !d.GetRawState().IsNull() && !d.GetRawState().AsValueMap()[key].IsNull() { return slices.Contains(collections.Map(ctyValToSliceString(d.GetRawState().AsValueMap()[key].AsValueSet().Values()), strings.ToUpper), strings.ToUpper(newValue)) } @@ -81,7 +81,7 @@ func IgnoreAfterCreation(_, _, _ string, d *schema.ResourceData) bool { return d.Id() != "" } -// IgnoreChangeToCurrentSnowflakeValueInShow should be used to ignore changes to the given attribute when its value is equal to value in show_output. +// IgnoreChangeToCurrentSnowflakeValueInShowWithMapping should be used to ignore changes to the given attribute when its value is equal to value in show_output after applying the mapping. func IgnoreChangeToCurrentSnowflakeValueInShowWithMapping(keyInOutput string, mapping func(any) any) schema.SchemaDiffSuppressFunc { return IgnoreChangeToCurrentSnowflakePlainValueInOutputWithMapping(ShowOutputAttributeName, keyInOutput, mapping) } diff --git a/pkg/resources/user.go b/pkg/resources/user.go index 89c240894a..52eb1211f3 100644 --- a/pkg/resources/user.go +++ b/pkg/resources/user.go @@ -14,6 +14,7 @@ import ( "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -41,7 +42,6 @@ var userSchema = map[string]*schema.Schema{ Description: "The name users use to log in. If not supplied, snowflake will use name instead. Login names are always case-insensitive.", // login_name is case-insensitive }, - // TODO [SNOW-1348101 - next PR]: handle external changes and the default behavior correctly; same with the login_name "display_name": { Type: schema.TypeString, Optional: true, @@ -107,7 +107,6 @@ var userSchema = map[string]*schema.Schema{ DiffSuppressFunc: suppressIdentifierQuoting, Description: "Specifies the virtual warehouse that is active by default for the user’s session upon login. Note that the CREATE USER operation does not verify that the warehouse exists.", }, - // TODO [SNOW-1348101 - next PR]: check the exact behavior of default_namespace and default_role because it looks like it is handled in a case-insensitive manner on Snowflake side "default_namespace": { Type: schema.TypeString, Optional: true, @@ -120,17 +119,15 @@ var userSchema = map[string]*schema.Schema{ DiffSuppressFunc: suppressIdentifierQuoting, Description: "Specifies the role that is active by default for the user’s session upon login. Note that specifying a default role for a user does **not** grant the role to the user. The role must be granted explicitly to the user using the [GRANT ROLE](https://docs.snowflake.com/en/sql-reference/sql/grant-role) command. In addition, the CREATE USER operation does not verify that the role exists.", }, - "default_secondary_roles": { - Type: schema.TypeSet, - Elem: &schema.Schema{ - Type: schema.TypeString, - ValidateDiagFunc: isValidSecondaryRole(), - }, - DiffSuppressFunc: SuppressCaseInSet("default_secondary_roles"), - MaxItems: 1, - MinItems: 1, + "default_secondary_roles_option": { + Type: schema.TypeString, Optional: true, - Description: "Specifies the set of secondary roles that are active for the user’s session upon login. Currently only [\"ALL\"] value is supported - more information can be found in [doc](https://docs.snowflake.com/en/sql-reference/sql/create-user#optional-object-properties-objectproperties).", + Default: sdk.SecondaryRolesOptionDefault, + ValidateDiagFunc: sdkValidation(sdk.ToSecondaryRolesOption), + DiffSuppressFunc: SuppressIfAny(NormalizeAndCompare(sdk.ToSecondaryRolesOption), IgnoreChangeToCurrentSnowflakeValueInShowWithMapping("default_secondary_roles", func(x any) any { + return sdk.GetSecondaryRolesOptionFrom(x.(string)) + })), + Description: fmt.Sprintf("Specifies the secondary roles that are active for the user’s session upon login. Valid values are (case-insensitive): %s. More information can be found in [doc](https://docs.snowflake.com/en/sql-reference/sql/create-user#optional-object-properties-objectproperties).", possibleValuesListed(sdk.ValidSecondaryRolesOptionsString)), }, "mins_to_bypass_mfa": { Type: schema.TypeInt, @@ -187,6 +184,8 @@ var userSchema = map[string]*schema.Schema{ func User() *schema.Resource { return &schema.Resource{ + SchemaVersion: 1, + CreateContext: CreateUser, UpdateContext: UpdateUser, ReadContext: GetReadUserFunc(true), @@ -200,8 +199,7 @@ func User() *schema.Resource { CustomizeDiff: customdiff.All( // TODO [SNOW-1629468 - next pr]: test "default_role", "default_secondary_roles" - // TODO [SNOW-1648997]: "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", "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(userSchema, ShowOutputAttributeName, "password", "login_name", "display_name", "first_name", "last_name", "email", "must_change_password", "disabled", "days_to_expiry", "mins_to_unlock", "default_warehouse", "default_namespace", "default_role", "default_secondary_roles_option", "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, @@ -216,6 +214,15 @@ func User() *schema.Resource { return nil }, ), + + StateUpgraders: []schema.StateUpgrader{ + { + Version: 0, + // setting type to cty.EmptyObject is a bit hacky here but following https://developer.hashicorp.com/terraform/plugin/framework/migrating/resources/state-upgrade#sdkv2-1 would require lots of repetitive code; this should work with cty.EmptyObject + Type: cty.EmptyObject, + Upgrade: v094UserStateUpgrader, + }, + }, } } @@ -274,9 +281,21 @@ func CreateUser(ctx context.Context, d *schema.ResourceData, meta any) diag.Diag accountObjectIdentifierAttributeCreate(d, "default_warehouse", &opts.ObjectProperties.DefaultWarehouse), objectIdentifierAttributeCreate(d, "default_namespace", &opts.ObjectProperties.DefaultNamespace), accountObjectIdentifierAttributeCreate(d, "default_role", &opts.ObjectProperties.DefaultRole), - // We do not need value because it is validated on the schema level and ALL is the only supported value currently. - // Check more in https://docs.snowflake.com/en/sql-reference/sql/create-user#optional-object-properties-objectproperties. - attributeDirectValueCreate(d, "default_secondary_roles", &opts.ObjectProperties.DefaultSecondaryRoles, &sdk.SecondaryRoles{}), + func() error { + defaultSecondaryRolesOption, err := sdk.ToSecondaryRolesOption(d.Get("default_secondary_roles_option").(string)) + if err != nil { + return err + } + switch defaultSecondaryRolesOption { + case sdk.SecondaryRolesOptionDefault: + return nil + case sdk.SecondaryRolesOptionNone: + opts.ObjectProperties.DefaultSecondaryRoles = &sdk.SecondaryRoles{None: sdk.Bool(true)} + case sdk.SecondaryRolesOptionAll: + opts.ObjectProperties.DefaultSecondaryRoles = &sdk.SecondaryRoles{All: sdk.Bool(true)} + } + return nil + }(), intAttributeWithSpecialDefaultCreate(d, "mins_to_bypass_mfa", &opts.ObjectProperties.MinsToBypassMFA), stringAttributeCreate(d, "rsa_public_key", &opts.ObjectProperties.RSAPublicKey), stringAttributeCreate(d, "rsa_public_key_2", &opts.ObjectProperties.RSAPublicKey2), @@ -370,6 +389,7 @@ func GetReadUserFunc(withExternalChangesMarking bool) schema.ReadContextFunc { showMapping{"must_change_password", "must_change_password", u.MustChangePassword, fmt.Sprintf("%t", u.MustChangePassword), nil}, showMapping{"disabled", "disabled", u.Disabled, fmt.Sprintf("%t", u.Disabled), nil}, showMapping{"default_namespace", "default_namespace", u.DefaultNamespace, u.DefaultNamespace, nil}, + showMapping{"default_secondary_roles", "default_secondary_roles_option", u.DefaultSecondaryRoles, u.GetSecondaryRolesOption(), nil}, ); err != nil { return diag.FromErr(err) } @@ -385,10 +405,6 @@ func GetReadUserFunc(withExternalChangesMarking bool) schema.ReadContextFunc { return diag.FromErr(err) } - var defaultSecondaryRoles []string - if userDetails.DefaultSecondaryRoles != nil && len(userDetails.DefaultSecondaryRoles.Value) > 0 { - defaultSecondaryRoles = sdk.ParseCommaSeparatedStringArray(userDetails.DefaultSecondaryRoles.Value, true) - } errs := errors.Join( // not reading name on purpose (we never update the name externally) // can't read password @@ -405,7 +421,7 @@ func GetReadUserFunc(withExternalChangesMarking bool) schema.ReadContextFunc { setFromStringPropertyIfNotEmpty(d, "default_warehouse", userDetails.DefaultWarehouse), // not reading default_namespace because one-part namespace seems to be capitalized on Snowflake side (handled as external change to show output) setFromStringPropertyIfNotEmpty(d, "default_role", userDetails.DefaultRole), - d.Set("default_secondary_roles", defaultSecondaryRoles), + // not setting default_secondary_role_option (handled as external change to show output) // not reading mins_to_bypass_mfa on purpose (they always change) setFromStringPropertyIfNotEmpty(d, "rsa_public_key", userDetails.RsaPublicKey), setFromStringPropertyIfNotEmpty(d, "rsa_public_key_2", userDetails.RsaPublicKey2), @@ -464,9 +480,23 @@ func UpdateUser(ctx context.Context, d *schema.ResourceData, meta any) diag.Diag accountObjectIdentifierAttributeUpdate(d, "default_warehouse", &setObjectProperties.DefaultWarehouse, &unsetObjectProperties.DefaultWarehouse), objectIdentifierAttributeUpdate(d, "default_namespace", &setObjectProperties.DefaultNamespace, &unsetObjectProperties.DefaultNamespace), accountObjectIdentifierAttributeUpdate(d, "default_role", &setObjectProperties.DefaultRole, &unsetObjectProperties.DefaultRole), - // We do not need value because it is validated on the schema level and ALL is the only supported value currently. - // Check more in https://docs.snowflake.com/en/sql-reference/sql/create-user#optional-object-properties-objectproperties. - attributeDirectValueUpdate(d, "default_secondary_roles", &setObjectProperties.DefaultSecondaryRoles, &sdk.SecondaryRoles{}, &unsetObjectProperties.DefaultSecondaryRoles), + func() error { + if d.HasChange("default_secondary_roles_option") { + defaultSecondaryRolesOption, err := sdk.ToSecondaryRolesOption(d.Get("default_secondary_roles_option").(string)) + if err != nil { + return err + } + switch defaultSecondaryRolesOption { + case sdk.SecondaryRolesOptionDefault: + unsetObjectProperties.DefaultSecondaryRoles = sdk.Bool(true) + case sdk.SecondaryRolesOptionNone: + setObjectProperties.DefaultSecondaryRoles = &sdk.SecondaryRoles{None: sdk.Bool(true)} + case sdk.SecondaryRolesOptionAll: + setObjectProperties.DefaultSecondaryRoles = &sdk.SecondaryRoles{All: sdk.Bool(true)} + } + } + return nil + }(), intAttributeWithSpecialDefaultUpdate(d, "mins_to_bypass_mfa", &setObjectProperties.MinsToBypassMFA, &unsetObjectProperties.MinsToBypassMFA), stringAttributeUpdate(d, "rsa_public_key", &setObjectProperties.RSAPublicKey, &unsetObjectProperties.RSAPublicKey), stringAttributeUpdate(d, "rsa_public_key_2", &setObjectProperties.RSAPublicKey2, &unsetObjectProperties.RSAPublicKey2), diff --git a/pkg/resources/user_acceptance_test.go b/pkg/resources/user_acceptance_test.go index 74ece5ee28..c7155298fe 100644 --- a/pkg/resources/user_acceptance_test.go +++ b/pkg/resources/user_acceptance_test.go @@ -62,7 +62,7 @@ func TestAcc_User_BasicFlows(t *testing.T) { WithDefaultWarehouse("some_warehouse"). WithDefaultNamespace("some.namespace"). WithDefaultRole("some_role"). - WithDefaultSecondaryRolesStringList("ALL"). + WithDefaultSecondaryRolesOptionEnum(sdk.SecondaryRolesOptionAll). WithMinsToBypassMfa(10). WithRsaPublicKey(key1). WithRsaPublicKey2(key2). @@ -85,7 +85,7 @@ func TestAcc_User_BasicFlows(t *testing.T) { WithDefaultWarehouse("other_warehouse"). WithDefaultNamespace("one_part_namespace"). WithDefaultRole("other_role"). - WithDefaultSecondaryRolesStringList("ALL"). + WithDefaultSecondaryRolesOptionEnum(sdk.SecondaryRolesOptionAll). WithMinsToBypassMfa(14). WithRsaPublicKey(key2). WithRsaPublicKey2(key1). @@ -121,7 +121,7 @@ func TestAcc_User_BasicFlows(t *testing.T) { HasNoDefaultWarehouse(). HasNoDefaultNamespace(). HasNoDefaultRole(). - HasNoDefaultSecondaryRoles(). + HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionDefault). HasMinsToBypassMfaString(r.IntDefaultString). HasNoRsaPublicKey(). HasNoRsaPublicKey2(). @@ -185,7 +185,7 @@ func TestAcc_User_BasicFlows(t *testing.T) { HasDefaultWarehouseString("some_warehouse"). HasDefaultNamespaceString("some.namespace"). HasDefaultRoleString("some_role"). - HasDefaultSecondaryRoles("ALL"). + HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionAll). HasMinsToBypassMfaString("10"). HasRsaPublicKeyString(key1). HasRsaPublicKey2String(key2). @@ -214,7 +214,7 @@ func TestAcc_User_BasicFlows(t *testing.T) { HasDefaultWarehouseString("other_warehouse"). HasDefaultNamespaceString("one_part_namespace"). HasDefaultRoleString("other_role"). - HasDefaultSecondaryRoles("ALL"). + HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionAll). HasMinsToBypassMfaString("14"). HasRsaPublicKeyString(key2). HasRsaPublicKey2String(key1). @@ -267,7 +267,7 @@ func TestAcc_User_BasicFlows(t *testing.T) { HasDefaultWarehouseString(""). HasDefaultNamespaceString(""). HasDefaultRoleString(""). - HasDefaultSecondaryRolesEmpty(). + HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionDefault). HasMinsToBypassMfaString(r.IntDefaultString). HasRsaPublicKeyString(""). HasRsaPublicKey2String(""). @@ -1015,13 +1015,12 @@ func TestAcc_User_handleChangesToDefaultSecondaryRoles(t *testing.T) { userId := acc.TestClient().Ids.RandomAccountObjectIdentifier() userModelEmpty := model.UserWithDefaultMeta(userId.Name()) - userModelWithDefaultSecondaryRole := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesStringList("ALL") - userModelLowercaseValue := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesStringList("all") - userModelIncorrectValue := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesStringList("OTHER") - userModelNoValues := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesStringList() - userModelMultipleValues := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesStringList("ALL", "OTHER") - userModelRepeatedDifferentCasing := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesStringList("ALL", "all") - userModelRepeatedValues := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesStringList("ALL", "ALL") + userModelWithOptionAll := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOptionEnum(sdk.SecondaryRolesOptionAll) + userModelWithOptionNone := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOptionEnum(sdk.SecondaryRolesOptionNone) + userModelLowercaseValue := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOption("all") + userModelIncorrectValue := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOption("OTHER") + userModelEmptyValue := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOption("") + userModelNullValue := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOptionValue(config.NullVariable()) resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, @@ -1031,23 +1030,36 @@ func TestAcc_User_handleChangesToDefaultSecondaryRoles(t *testing.T) { PreCheck: func() { acc.TestAccPreCheck(t) }, CheckDestroy: acc.CheckDestroy(t, resources.User), Steps: []resource.TestStep{ - // 1. create without default secondary roles + // 1. create without default secondary roles option set (DEFAULT will be used) { Config: config.FromModel(t, userModelEmpty), Check: assert.AssertThat(t, - resourceassert.UserResource(t, userModelEmpty.ResourceReference()).HasNoDefaultSecondaryRoles(), + resourceassert.UserResource(t, userModelEmpty.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionDefault), objectassert.User(t, userId).HasDefaultSecondaryRoles(""), ), }, - // 2. add default secondary roles + // 2. add default secondary roles NONE (expecting change because null != [] on Snowflake side) { - Config: config.FromModel(t, userModelWithDefaultSecondaryRole), + Config: config.FromModel(t, userModelWithOptionNone), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectNonEmptyPlan(), + }, + }, + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelWithOptionAll.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionNone), + objectassert.User(t, userId).HasDefaultSecondaryRoles(`[]`), + ), + }, + // 3. add default secondary roles ALL + { + Config: config.FromModel(t, userModelWithOptionAll), Check: assert.AssertThat(t, - resourceassert.UserResource(t, userModelWithDefaultSecondaryRole.ResourceReference()).HasDefaultSecondaryRoles("ALL"), + resourceassert.UserResource(t, userModelWithOptionAll.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionAll), objectassert.User(t, userId).HasDefaultSecondaryRoles(`["ALL"]`), ), }, - // 3. change to lowercase (no changes) + // 4. change to lowercase (no changes) { Config: config.FromModel(t, userModelLowercaseValue), ConfigPlanChecks: resource.ConfigPlanChecks{ @@ -1056,63 +1068,332 @@ func TestAcc_User_handleChangesToDefaultSecondaryRoles(t *testing.T) { }, }, }, - // 4. unset externally + // 5. unset externally { PreConfig: func() { acc.TestClient().User.UnsetDefaultSecondaryRoles(t, userId) }, - Config: config.FromModel(t, userModelWithDefaultSecondaryRole), + Config: config.FromModel(t, userModelWithOptionAll), ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ - planchecks.ExpectChange(userModelWithDefaultSecondaryRole.ResourceReference(), "default_secondary_roles", tfjson.ActionUpdate, sdk.String("[]"), sdk.String("[ALL]")), + planchecks.ExpectChange(userModelWithOptionAll.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String("DEFAULT"), sdk.String("ALL")), }, }, Check: assert.AssertThat(t, - resourceassert.UserResource(t, userModelWithDefaultSecondaryRole.ResourceReference()).HasDefaultSecondaryRoles("ALL"), + resourceassert.UserResource(t, userModelWithOptionAll.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionAll), objectassert.User(t, userId).HasDefaultSecondaryRoles(`["ALL"]`), ), }, - // 5. unset in config to 5 (change) + // 6. unset in config (change) { Config: config.FromModel(t, userModelEmpty), ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ - planchecks.ExpectChange(userModelEmpty.ResourceReference(), "default_secondary_roles", tfjson.ActionUpdate, sdk.String("[ALL]"), sdk.String("[]")), + planchecks.ExpectChange(userModelEmpty.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String("ALL"), sdk.String("DEFAULT")), }, }, Check: assert.AssertThat(t, - resourceassert.UserResource(t, userModelEmpty.ResourceReference()).HasDefaultSecondaryRolesEmpty(), + resourceassert.UserResource(t, userModelEmpty.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionDefault), objectassert.User(t, userId).HasDefaultSecondaryRoles(""), ), }, - // 6. incorrect value used + // 7. incorrect value used { Config: config.FromModel(t, userModelIncorrectValue), - ExpectError: regexp.MustCompile("Unsupported secondary role 'OTHER'"), + ExpectError: regexp.MustCompile("invalid secondary roles option: OTHER"), }, - // 7. empty set used + // 8. set to empty in config (invalid) { - Config: config.FromModel(t, userModelNoValues), - ExpectError: regexp.MustCompile("Attribute default_secondary_roles requires 1 item minimum"), + Config: config.FromModel(t, userModelEmptyValue), + ExpectError: regexp.MustCompile("invalid secondary roles option: "), }, - // 8. multiple values (correct and incorrect) + // 9. set in config to NONE (change) { - Config: config.FromModel(t, userModelMultipleValues), - ExpectError: regexp.MustCompile("Attribute default_secondary_roles supports 1 item maximum"), + Config: config.FromModel(t, userModelWithOptionNone), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + planchecks.ExpectChange(userModelWithOptionNone.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String("DEFAULT"), sdk.String("NONE")), + }, + }, + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelWithOptionNone.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionNone), + objectassert.User(t, userId).HasDefaultSecondaryRoles("[]"), + ), }, - // 9. multiple values (different casing) + // 10. unset in config (change) { - Config: config.FromModel(t, userModelRepeatedDifferentCasing), - ExpectError: regexp.MustCompile("Attribute default_secondary_roles supports 1 item maximum"), + Config: config.FromModel(t, userModelEmpty), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + planchecks.ExpectChange(userModelEmpty.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String("NONE"), sdk.String("DEFAULT")), + }, + }, + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelEmpty.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionDefault), + objectassert.User(t, userId).HasDefaultSecondaryRoles(""), + ), }, - // 10. multiple values (two same) - no error + // 11. add default secondary roles ALL { - Config: config.FromModel(t, userModelRepeatedValues), + Config: config.FromModel(t, userModelWithOptionAll), Check: assert.AssertThat(t, - resourceassert.UserResource(t, userModelWithDefaultSecondaryRole.ResourceReference()).HasDefaultSecondaryRoles("ALL"), + resourceassert.UserResource(t, userModelWithOptionAll.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionAll), objectassert.User(t, userId).HasDefaultSecondaryRoles(`["ALL"]`), ), }, + // 12. set to null value in config (change) + { + Config: config.FromModel(t, userModelNullValue), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + planchecks.ExpectChange(userModelNullValue.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String("ALL"), sdk.String("DEFAULT")), + }, + }, + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelNullValue.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionDefault), + objectassert.User(t, userId).HasDefaultSecondaryRoles(""), + ), + }, + }, + }) +} + +func TestAcc_User_handleChangesToDefaultSecondaryRoles_bcr202407(t *testing.T) { + userId := acc.TestClient().Ids.RandomAccountObjectIdentifier() + + userModelEmpty := model.UserWithDefaultMeta(userId.Name()) + userModelWithOptionAll := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOptionEnum(sdk.SecondaryRolesOptionAll) + userModelWithOptionNone := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOptionEnum(sdk.SecondaryRolesOptionNone) + userModelLowercaseValue := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOption("all") + userModelIncorrectValue := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOption("OTHER") + userModelEmptyValue := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOption("") + userModelNullValue := model.UserWithDefaultMeta(userId.Name()).WithDefaultSecondaryRolesOptionValue(config.NullVariable()) + + 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{ + // 1. create without default secondary roles option set (DEFAULT will be used) + { + PreConfig: func() { + acc.TestClient().BcrBundles.EnableBcrBundle(t, "2024_07") + }, + Config: config.FromModel(t, userModelEmpty), + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelEmpty.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionDefault), + objectassert.User(t, userId).HasDefaultSecondaryRoles(`["ALL"]`), + ), + }, + // 2. add default secondary roles ALL (expecting no change) + { + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + Config: config.FromModel(t, userModelWithOptionAll), + }, + // 3. change to lowercase (change because we have DEFAULT in state because previous step was suppressed so none of the suppressors NormalizeAndCompare nor IgnoreChangeToCurrentSnowflakeValueInShowWithMapping suppresses it; it can be made better later) + { + Config: config.FromModel(t, userModelLowercaseValue), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + planchecks.ExpectChange(userModelWithOptionNone.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String("DEFAULT"), sdk.String("all")), + }, + }, + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelEmpty.ResourceReference()).HasDefaultSecondaryRolesOptionString("all"), + objectassert.User(t, userId).HasDefaultSecondaryRoles(`["ALL"]`), + ), + }, + // 4. add default secondary roles NONE + { + Config: config.FromModel(t, userModelWithOptionNone), + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelWithOptionNone.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionNone), + objectassert.User(t, userId).HasDefaultSecondaryRoles(`[]`), + ), + }, + // 5. unset externally + { + PreConfig: func() { + acc.TestClient().User.UnsetDefaultSecondaryRoles(t, userId) + }, + Config: config.FromModel(t, userModelWithOptionNone), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + planchecks.ExpectChange(userModelWithOptionNone.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String("ALL"), sdk.String("NONE")), + }, + }, + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelWithOptionAll.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionNone), + objectassert.User(t, userId).HasDefaultSecondaryRoles(`[]`), + ), + }, + // 6. unset in config (change) + { + Config: config.FromModel(t, userModelEmpty), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + planchecks.ExpectChange(userModelEmpty.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String("NONE"), sdk.String("DEFAULT")), + }, + }, + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelEmpty.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionDefault), + objectassert.User(t, userId).HasDefaultSecondaryRoles(`["ALL"]`), + ), + }, + // 7. incorrect value used + { + Config: config.FromModel(t, userModelIncorrectValue), + ExpectError: regexp.MustCompile("invalid secondary roles option: OTHER"), + }, + // 8. set to empty in config (invalid) + { + Config: config.FromModel(t, userModelEmptyValue), + ExpectError: regexp.MustCompile("invalid secondary roles option: "), + }, + // 9. set in config to NONE (change) + { + Config: config.FromModel(t, userModelWithOptionNone), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + planchecks.ExpectChange(userModelWithOptionNone.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String("DEFAULT"), sdk.String("NONE")), + }, + }, + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelWithOptionNone.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionNone), + objectassert.User(t, userId).HasDefaultSecondaryRoles("[]"), + ), + }, + // 10. unset in config (change) + { + Config: config.FromModel(t, userModelEmpty), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + planchecks.ExpectChange(userModelEmpty.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String("NONE"), sdk.String("DEFAULT")), + }, + }, + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelEmpty.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionDefault), + objectassert.User(t, userId).HasDefaultSecondaryRoles(`["ALL"]`), + ), + }, + // 11. add default secondary roles NONE + { + Config: config.FromModel(t, userModelWithOptionNone), + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelEmpty.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionNone), + objectassert.User(t, userId).HasDefaultSecondaryRoles(`[]`), + ), + }, + // 12. set to null value in config (change) + { + Config: config.FromModel(t, userModelNullValue), + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelEmpty.ResourceReference()).HasDefaultSecondaryRolesOption(sdk.SecondaryRolesOptionDefault), + objectassert.User(t, userId).HasDefaultSecondaryRoles(`["ALL"]`), + ), + }, + }, + }) +} + +func TestAcc_User_migrateFromVersion094_noDefaultSecondaryRolesSet(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + + userModel := model.UserWithDefaultMeta(id.Name()) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.User), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.94.1", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: config.FromModel(t, userModel), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(userModel.ResourceReference(), "name", id.Name()), + resource.TestCheckResourceAttr(userModel.ResourceReference(), "default_secondary_roles.#", "0"), + ), + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + Config: config.FromModel(t, userModel), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + // we do not have a plancheck yet to validate no changes on the given field; this is a current alternative + planchecks.ExpectChange(userModel.ResourceReference(), "default_secondary_roles", tfjson.ActionUpdate, nil, nil), + planchecks.ExpectChange(userModel.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String(string(sdk.SecondaryRolesOptionDefault)), sdk.String(string(sdk.SecondaryRolesOptionDefault))), + }, + }, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(userModel.ResourceReference(), "name", id.Name()), + resource.TestCheckResourceAttr(userModel.ResourceReference(), "default_secondary_roles_option", string(sdk.SecondaryRolesOptionDefault)), + resource.TestCheckNoResourceAttr(userModel.ResourceReference(), "default_secondary_roles"), + ), + }, + }, + }) +} + +func TestAcc_User_migrateFromVersion094_defaultSecondaryRolesSet(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + + userModelWithOptionAll := model.UserWithDefaultMeta(id.Name()).WithDefaultSecondaryRolesOptionEnum(sdk.SecondaryRolesOptionAll) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.User), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.94.1", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: fmt.Sprintf(` +resource "snowflake_user" "test" { + name = "%s" + default_secondary_roles = ["ALL"] +}`, id.Name()), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(userModelWithOptionAll.ResourceReference(), "name", id.Name()), + resource.TestCheckResourceAttr(userModelWithOptionAll.ResourceReference(), "default_secondary_roles.#", "1"), + resource.TestCheckResourceAttr(userModelWithOptionAll.ResourceReference(), "default_secondary_roles.0", "ALL"), + ), + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + Config: config.FromModel(t, userModelWithOptionAll), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + // we do not have a plancheck yet to validate no changes on the given field; this is a current alternative + planchecks.ExpectChange(userModelWithOptionAll.ResourceReference(), "default_secondary_roles", tfjson.ActionUpdate, nil, nil), + planchecks.ExpectChange(userModelWithOptionAll.ResourceReference(), "default_secondary_roles_option", tfjson.ActionUpdate, sdk.String(string(sdk.SecondaryRolesOptionAll)), sdk.String(string(sdk.SecondaryRolesOptionAll))), + }, + }, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(userModelWithOptionAll.ResourceReference(), "name", id.Name()), + resource.TestCheckResourceAttr(userModelWithOptionAll.ResourceReference(), "default_secondary_roles_option", string(sdk.SecondaryRolesOptionAll)), + resource.TestCheckNoResourceAttr(userModelWithOptionAll.ResourceReference(), "default_secondary_roles"), + ), + }, }, }) } diff --git a/pkg/resources/user_state_upgraders.go b/pkg/resources/user_state_upgraders.go new file mode 100644 index 0000000000..3802b8e98f --- /dev/null +++ b/pkg/resources/user_state_upgraders.go @@ -0,0 +1,27 @@ +package resources + +import ( + "context" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" +) + +func v094UserStateUpgrader(ctx context.Context, rawState map[string]any, meta any) (map[string]any, error) { + if rawState == nil { + return rawState, nil + } + + oldDefaultSecondaryRoles := rawState["default_secondary_roles"] + if oldDefaultSecondaryRoles == nil { + rawState["default_secondary_roles_option"] = string(sdk.SecondaryRolesOptionDefault) + } else { + if len(oldDefaultSecondaryRoles.([]any)) > 0 { + rawState["default_secondary_roles_option"] = string(sdk.SecondaryRolesOptionAll) + } else { + rawState["default_secondary_roles_option"] = string(sdk.SecondaryRolesOptionNone) + } + } + delete(rawState, "default_secondary_roles") + + return rawState, nil +} diff --git a/pkg/resources/validators_test.go b/pkg/resources/validators_test.go index 7a50eed135..6146d3c603 100644 --- a/pkg/resources/validators_test.go +++ b/pkg/resources/validators_test.go @@ -8,7 +8,6 @@ import ( "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestIsDataType(t *testing.T) { @@ -356,53 +355,3 @@ func Test_isNotEqualTo(t *testing.T) { }) } } - -func Test_isValidSecondaryRole(t *testing.T) { - testCases := []struct { - Name string - Value any - ExpectedSummary string - ExpectedDetail string - }{ - { - Name: "only supported value: 'ALL'", - Value: "ALL", - }, - { - Name: "only supported value lowercased", - Value: "all", - }, - { - Name: "nil value", - Value: nil, - ExpectedSummary: "Unsupported secondary role ''", - ExpectedDetail: "The only supported default secondary roles value is currently 'ALL'", - }, - { - Name: "empty value", - Value: "", - ExpectedSummary: "Unsupported secondary role ''", - ExpectedDetail: "The only supported default secondary roles value is currently 'ALL'", - }, - { - Name: "unsupported value", - Value: "SOME_ROLE", - ExpectedSummary: "Unsupported secondary role 'SOME_ROLE'", - ExpectedDetail: "The only supported default secondary roles value is currently 'ALL'", - }, - } - - for _, tt := range testCases { - tt := tt - t.Run(tt.Name, func(t *testing.T) { - diag := isValidSecondaryRole()(tt.Value, cty.GetAttrPath("path")) - if tt.ExpectedSummary != "" || tt.ExpectedDetail != "" { - require.Len(t, diag, 1) - assert.Contains(t, diag[0].Summary, tt.ExpectedSummary) - assert.Contains(t, diag[0].Detail, tt.ExpectedDetail) - } else { - require.Len(t, diag, 0) - } - }) - } -} diff --git a/pkg/sdk/system_functions.go b/pkg/sdk/system_functions.go index 39c4396788..0ff406ab1a 100644 --- a/pkg/sdk/system_functions.go +++ b/pkg/sdk/system_functions.go @@ -15,6 +15,8 @@ type SystemFunctions interface { // PipeForceResume unpauses a pipe after ownership transfer. Snowflake will throw an error whenever a pipe changes its owner, // and someone tries to unpause it. To unpause a pipe after ownership transfer, this system function has to be called instead of ALTER PIPE. PipeForceResume(pipeId SchemaObjectIdentifier, options []ForceResumePipeOption) error + EnableBehaviorChangeBundle(ctx context.Context, bundle string) error + DisableBehaviorChangeBundle(ctx context.Context, bundle string) error } var _ SystemFunctions = (*systemFunctions)(nil) @@ -100,3 +102,13 @@ func (c *systemFunctions) PipeForceResume(pipeId SchemaObjectIdentifier, options _, err := c.client.exec(ctx, fmt.Sprintf("SELECT SYSTEM$PIPE_FORCE_RESUME('%s')%s", pipeId.FullyQualifiedName(), functionOpts)) return err } + +func (c *systemFunctions) EnableBehaviorChangeBundle(ctx context.Context, bundle string) error { + _, err := c.client.exec(ctx, fmt.Sprintf("SELECT SYSTEM$ENABLE_BEHAVIOR_CHANGE_BUNDLE('%s')", bundle)) + return err +} + +func (c *systemFunctions) DisableBehaviorChangeBundle(ctx context.Context, bundle string) error { + _, err := c.client.exec(ctx, fmt.Sprintf("SELECT SYSTEM$DISABLE_BEHAVIOR_CHANGE_BUNDLE('%s')", bundle)) + return err +} diff --git a/pkg/sdk/testint/system_functions_integration_test.go b/pkg/sdk/testint/system_functions_integration_test.go index 247c70e2d6..4ae2311ef5 100644 --- a/pkg/sdk/testint/system_functions_integration_test.go +++ b/pkg/sdk/testint/system_functions_integration_test.go @@ -173,3 +173,19 @@ func TestInt_PipeForceResume(t *testing.T) { require.NoError(t, err) require.Equal(t, sdk.RunningPipeExecutionState, pipeExecutionState) } + +// TODO [SNOW-1650249]: add positive tests for bundle enablement (add SYSTEM$SHOW_ACTIVE_BEHAVIOR_CHANGE_BUNDLES() and use it to always pick a bundle that can be enabled/disabled) +func TestInt_BcrBundles(t *testing.T) { + client := testClient(t) + ctx := testContext(t) + + t.Run("enable non-existing bundle", func(t *testing.T) { + err := client.SystemFunctions.EnableBehaviorChangeBundle(ctx, "non-existing-bundle") + require.ErrorContains(t, err, "Invalid Change Bundle 'non-existing-bundle'") + }) + + t.Run("disable non-existing bundle", func(t *testing.T) { + err := client.SystemFunctions.DisableBehaviorChangeBundle(ctx, "non-existing-bundle") + require.ErrorContains(t, err, "Invalid Change Bundle 'non-existing-bundle'") + }) +} diff --git a/pkg/sdk/testint/users_integration_test.go b/pkg/sdk/testint/users_integration_test.go index dcba7985b0..24ad3085ed 100644 --- a/pkg/sdk/testint/users_integration_test.go +++ b/pkg/sdk/testint/users_integration_test.go @@ -254,7 +254,7 @@ func TestInt_Users(t *testing.T) { DefaultWarehouse: sdk.Pointer(warehouseId), DefaultNamespace: sdk.Pointer(schemaIdObjectIdentifier), DefaultRole: sdk.Pointer(roleId), - DefaultSecondaryRoles: &sdk.SecondaryRoles{}, + DefaultSecondaryRoles: &sdk.SecondaryRoles{All: sdk.Bool(true)}, MinsToBypassMFA: sdk.Int(30), RSAPublicKey: sdk.String(key), RSAPublicKey2: sdk.String(key2), @@ -646,7 +646,7 @@ func TestInt_Users(t *testing.T) { DefaultWarehouse: sdk.Pointer(warehouseId), DefaultNamespace: sdk.Pointer(schemaIdObjectIdentifier), DefaultRole: sdk.Pointer(roleId), - DefaultSecondaryRoles: &sdk.SecondaryRoles{}, + DefaultSecondaryRoles: &sdk.SecondaryRoles{All: sdk.Bool(true)}, MinsToBypassMFA: sdk.Int(30), RSAPublicKey: sdk.String(key), RSAPublicKey2: sdk.String(key2), @@ -1153,7 +1153,7 @@ func TestInt_Users(t *testing.T) { DefaultWarehouse: sdk.Pointer(warehouseId), DefaultNamespace: sdk.Pointer(schemaIdObjectIdentifier), DefaultRole: sdk.Pointer(roleId), - DefaultSecondaryRoles: &sdk.SecondaryRoles{}, + DefaultSecondaryRoles: &sdk.SecondaryRoles{All: sdk.Bool(true)}, MinsToBypassMFA: sdk.Int(30), RSAPublicKey: sdk.String(key), RSAPublicKey2: sdk.String(key2), @@ -1539,4 +1539,91 @@ func TestInt_Users(t *testing.T) { // mins to bypass mfa is nil require.Nil(t, userDetails.MinsToBypassMfa.Value) }) + + t.Run("default secondary roles: before bundle 2024_07", func(t *testing.T) { + id := testClientHelper().Ids.RandomAccountObjectIdentifier() + + // create, expecting null as default + err := client.Users.Create(ctx, id, nil) + require.NoError(t, err) + t.Cleanup(testClientHelper().User.DropUserFunc(t, id)) + + userDetails, err := client.Users.Describe(ctx, id) + require.NoError(t, err) + require.Equal(t, "", userDetails.DefaultSecondaryRoles.Value) + + // set to empty, expecting empty list + err = client.Users.Alter(ctx, id, &sdk.AlterUserOptions{ + Set: &sdk.UserSet{ + ObjectProperties: &sdk.UserAlterObjectProperties{ + UserObjectProperties: sdk.UserObjectProperties{ + DefaultSecondaryRoles: &sdk.SecondaryRoles{None: sdk.Bool(true)}, + }, + }, + }, + }) + require.NoError(t, err) + + userDetails, err = client.Users.Describe(ctx, id) + require.NoError(t, err) + require.Equal(t, "[]", userDetails.DefaultSecondaryRoles.Value) + + // unset, expecting null + err = client.Users.Alter(ctx, id, &sdk.AlterUserOptions{ + Unset: &sdk.UserUnset{ + ObjectProperties: &sdk.UserObjectPropertiesUnset{ + DefaultSecondaryRoles: sdk.Bool(true), + }, + }, + }) + require.NoError(t, err) + + userDetails, err = client.Users.Describe(ctx, id) + require.NoError(t, err) + require.Equal(t, "", userDetails.DefaultSecondaryRoles.Value) + }) + + t.Run("default secondary roles: with bundle 2024_07 enabled", func(t *testing.T) { + testClientHelper().BcrBundles.EnableBcrBundle(t, "2024_07") + id := testClientHelper().Ids.RandomAccountObjectIdentifier() + + // create, expecting ALL as new default + err := client.Users.Create(ctx, id, nil) + require.NoError(t, err) + t.Cleanup(testClientHelper().User.DropUserFunc(t, id)) + + userDetails, err := client.Users.Describe(ctx, id) + require.NoError(t, err) + require.Equal(t, `["ALL"]`, userDetails.DefaultSecondaryRoles.Value) + + // set to empty, expecting empty list + err = client.Users.Alter(ctx, id, &sdk.AlterUserOptions{ + Set: &sdk.UserSet{ + ObjectProperties: &sdk.UserAlterObjectProperties{ + UserObjectProperties: sdk.UserObjectProperties{ + DefaultSecondaryRoles: &sdk.SecondaryRoles{None: sdk.Bool(true)}, + }, + }, + }, + }) + require.NoError(t, err) + + userDetails, err = client.Users.Describe(ctx, id) + require.NoError(t, err) + require.Equal(t, "[]", userDetails.DefaultSecondaryRoles.Value) + + // unset, expecting ALL + err = client.Users.Alter(ctx, id, &sdk.AlterUserOptions{ + Unset: &sdk.UserUnset{ + ObjectProperties: &sdk.UserObjectPropertiesUnset{ + DefaultSecondaryRoles: sdk.Bool(true), + }, + }, + }) + require.NoError(t, err) + + userDetails, err = client.Users.Describe(ctx, id) + require.NoError(t, err) + require.Equal(t, `["ALL"]`, userDetails.DefaultSecondaryRoles.Value) + }) } diff --git a/pkg/sdk/users.go b/pkg/sdk/users.go index ba1b98ef3d..50ff90aab8 100644 --- a/pkg/sdk/users.go +++ b/pkg/sdk/users.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "strings" "time" ) @@ -63,6 +64,22 @@ type User struct { HasMfa bool } +func (v *User) GetSecondaryRolesOption() SecondaryRolesOption { + return GetSecondaryRolesOptionFrom(v.DefaultSecondaryRoles) +} + +func GetSecondaryRolesOptionFrom(text string) SecondaryRolesOption { + if text != "" { + parsedRoles := ParseCommaSeparatedStringArray(text, true) + if len(parsedRoles) > 0 { + return SecondaryRolesOptionAll + } else { + return SecondaryRolesOptionNone + } + } + return SecondaryRolesOptionDefault +} + type userDBRow struct { Name string `db:"name"` CreatedOn time.Time `db:"created_on"` @@ -187,6 +204,11 @@ func (opts *CreateUserOptions) validate() error { if !ValidObjectIdentifier(opts.name) { return errors.Join(ErrInvalidObjectIdentifier) } + if valueSet(opts.ObjectProperties) && valueSet(opts.ObjectProperties.DefaultSecondaryRoles) { + if err := opts.ObjectProperties.DefaultSecondaryRoles.validate(); err != nil { + return err + } + } return nil } @@ -236,7 +258,8 @@ type UserAlterObjectProperties struct { } type SecondaryRoles struct { - all bool `ddl:"static" sql:"('ALL')"` + None *bool `ddl:"static" sql:"()"` + All *bool `ddl:"static" sql:"('ALL')"` } type SecondaryRole struct { @@ -376,6 +399,18 @@ func (opts *UserSet) validate() error { if !anyValueSet(opts.PasswordPolicy, opts.SessionPolicy, opts.ObjectProperties, opts.ObjectParameters, opts.SessionParameters) { return errAtLeastOneOf("UserSet", "PasswordPolicy", "SessionPolicy", "ObjectProperties", "ObjectParameters", "SessionParameters") } + if valueSet(opts.ObjectProperties) && valueSet(opts.ObjectProperties.DefaultSecondaryRoles) { + if err := opts.ObjectProperties.DefaultSecondaryRoles.validate(); err != nil { + return err + } + } + return nil +} + +func (opts *SecondaryRoles) validate() error { + if !exactlyOneValueSet(opts.All, opts.None) { + return errExactlyOneOf("SecondaryRoles", "All", "None") + } return nil } @@ -629,3 +664,30 @@ func (v *users) ShowParameters(ctx context.Context, id AccountObjectIdentifier) }, }) } + +type SecondaryRolesOption string + +const ( + SecondaryRolesOptionDefault SecondaryRolesOption = "DEFAULT" + SecondaryRolesOptionNone SecondaryRolesOption = "NONE" + SecondaryRolesOptionAll SecondaryRolesOption = "ALL" +) + +func ToSecondaryRolesOption(s string) (SecondaryRolesOption, error) { + switch strings.ToUpper(s) { + case string(SecondaryRolesOptionDefault): + return SecondaryRolesOptionDefault, nil + case string(SecondaryRolesOptionNone): + return SecondaryRolesOptionNone, nil + case string(SecondaryRolesOptionAll): + return SecondaryRolesOptionAll, nil + default: + return "", fmt.Errorf("invalid secondary roles option: %s", s) + } +} + +var ValidSecondaryRolesOptionsString = []string{ + string(SecondaryRolesOptionDefault), + string(SecondaryRolesOptionNone), + string(SecondaryRolesOptionAll), +} diff --git a/pkg/sdk/users_test.go b/pkg/sdk/users_test.go index 7414f94c92..0404cbf74a 100644 --- a/pkg/sdk/users_test.go +++ b/pkg/sdk/users_test.go @@ -1,6 +1,7 @@ package sdk import ( + "fmt" "testing" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" @@ -22,6 +23,39 @@ func TestUserCreate(t *testing.T) { assertOptsValidAndSQLEquals(t, opts, `CREATE USER %s`, id.FullyQualifiedName()) }) + t.Run("empty secondary roles", func(t *testing.T) { + opts := &CreateUserOptions{ + name: id, + ObjectProperties: &UserObjectProperties{ + DefaultSecondaryRoles: &SecondaryRoles{None: Bool(true)}, + }, + } + assertOptsValidAndSQLEquals(t, opts, `CREATE USER %s DEFAULT_SECONDARY_ROLES = ()`, id.FullyQualifiedName()) + }) + + t.Run("with empty secondary roles", func(t *testing.T) { + opts := &CreateUserOptions{ + name: id, + ObjectProperties: &UserObjectProperties{ + DefaultSecondaryRoles: &SecondaryRoles{}, + }, + } + assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("SecondaryRoles", "All", "None")) + }) + + t.Run("with both options in secondary roles", func(t *testing.T) { + opts := &CreateUserOptions{ + name: id, + ObjectProperties: &UserObjectProperties{ + DefaultSecondaryRoles: &SecondaryRoles{ + All: Bool(true), + None: Bool(true), + }, + }, + } + assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("SecondaryRoles", "All", "None")) + }) + t.Run("with complete options", func(t *testing.T) { tagId := randomSchemaObjectIdentifier() tags := []TagAssociation{ @@ -41,11 +75,12 @@ func TestUserCreate(t *testing.T) { name: id, IfNotExists: Bool(true), ObjectProperties: &UserObjectProperties{ - Password: &password, - LoginName: &loginName, - DefaultRole: Pointer(defaultRoleId), - DefaultNamespace: Pointer(defaultNamespaceId), - DefaultWarehouse: Pointer(defaultWarehouseId), + Password: &password, + LoginName: &loginName, + DefaultRole: Pointer(defaultRoleId), + DefaultNamespace: Pointer(defaultNamespaceId), + DefaultWarehouse: Pointer(defaultWarehouseId), + DefaultSecondaryRoles: &SecondaryRoles{All: Bool(true)}, }, ObjectParameters: &UserObjectParameters{ EnableUnredactedQuerySyntaxError: Bool(true), @@ -57,7 +92,7 @@ func TestUserCreate(t *testing.T) { Tags: tags, } - assertOptsValidAndSQLEquals(t, opts, `CREATE OR REPLACE USER IF NOT EXISTS %s PASSWORD = '%s' LOGIN_NAME = '%s' DEFAULT_WAREHOUSE = %s DEFAULT_NAMESPACE = %s DEFAULT_ROLE = %s ENABLE_UNREDACTED_QUERY_SYNTAX_ERROR = true AUTOCOMMIT = true WITH TAG (%s = 'v1')`, id.FullyQualifiedName(), password, loginName, defaultWarehouseId.FullyQualifiedName(), defaultNamespaceId.FullyQualifiedName(), defaultRoleId.FullyQualifiedName(), tagId.FullyQualifiedName()) + assertOptsValidAndSQLEquals(t, opts, `CREATE OR REPLACE USER IF NOT EXISTS %s PASSWORD = '%s' LOGIN_NAME = '%s' DEFAULT_WAREHOUSE = %s DEFAULT_NAMESPACE = %s DEFAULT_ROLE = %s DEFAULT_SECONDARY_ROLES = ('ALL') ENABLE_UNREDACTED_QUERY_SYNTAX_ERROR = true AUTOCOMMIT = true WITH TAG (%s = 'v1')`, id.FullyQualifiedName(), password, loginName, defaultWarehouseId.FullyQualifiedName(), defaultNamespaceId.FullyQualifiedName(), defaultRoleId.FullyQualifiedName(), tagId.FullyQualifiedName()) }) } @@ -149,8 +184,10 @@ func TestUserAlter(t *testing.T) { password := random.Password() objectProperties := UserAlterObjectProperties{ UserObjectProperties: UserObjectProperties{ - Password: &password, - DefaultSecondaryRoles: &SecondaryRoles{}, + Password: &password, + DefaultSecondaryRoles: &SecondaryRoles{ + All: Bool(true), + }, }, } opts := &AlterUserOptions{ @@ -300,6 +337,37 @@ func TestUserAlter(t *testing.T) { } assertOptsValidAndSQLEquals(t, opts, "ALTER USER %s REMOVE DELEGATED AUTHORIZATION OF ROLE %s FROM SECURITY INTEGRATION %s", id.FullyQualifiedName(), role, integration) }) + + t.Run("empty secondary roles", func(t *testing.T) { + opts := &AlterUserOptions{ + name: id, + Set: &UserSet{ + ObjectProperties: &UserAlterObjectProperties{ + UserObjectProperties: UserObjectProperties{ + DefaultSecondaryRoles: &SecondaryRoles{}, + }, + }, + }, + } + assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("SecondaryRoles", "All", "None")) + }) + + t.Run("with empty secondary roles", func(t *testing.T) { + opts := &AlterUserOptions{ + name: id, + Set: &UserSet{ + ObjectProperties: &UserAlterObjectProperties{ + UserObjectProperties: UserObjectProperties{ + DefaultSecondaryRoles: &SecondaryRoles{ + All: Bool(true), + None: Bool(true), + }, + }, + }, + }, + } + assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("SecondaryRoles", "All", "None")) + }) } func TestUserDrop(t *testing.T) { @@ -711,3 +779,76 @@ func Test_User_ToUnsupportedDDLAction(t *testing.T) { }) } } + +func Test_User_ToSecondaryRolesOption(t *testing.T) { + type test struct { + input string + want SecondaryRolesOption + } + + valid := []test{ + // case insensitive. + {input: "none", want: SecondaryRolesOptionNone}, + + // Supported Values + {input: "NONE", want: SecondaryRolesOptionNone}, + {input: "ALL", want: SecondaryRolesOptionAll}, + {input: "DEFAULT", want: SecondaryRolesOptionDefault}, + } + + invalid := []test{ + // bad values + {input: ""}, + {input: "foo"}, + } + + for _, tc := range valid { + tc := tc + t.Run(tc.input, func(t *testing.T) { + got, err := ToSecondaryRolesOption(tc.input) + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } + + for _, tc := range invalid { + tc := tc + t.Run(tc.input, func(t *testing.T) { + _, err := ToSecondaryRolesOption(tc.input) + require.Error(t, err) + }) + } +} + +func Test_User_GetSecondaryRolesOptionFrom(t *testing.T) { + type test struct { + input string + want SecondaryRolesOption + } + + valid := []test{ + {input: "", want: SecondaryRolesOptionDefault}, + {input: "[]", want: SecondaryRolesOptionNone}, + {input: `["ALL"]`, want: SecondaryRolesOptionAll}, + {input: `["any"]`, want: SecondaryRolesOptionAll}, + {input: `["more", "than", "one"]`, want: SecondaryRolesOptionAll}, + {input: `no list`, want: SecondaryRolesOptionAll}, + } + + for _, tc := range valid { + tc := tc + t.Run(tc.input, func(t *testing.T) { + got := GetSecondaryRolesOptionFrom(tc.input) + require.Equal(t, tc.want, got) + }) + } + + for _, tc := range valid { + tc := tc + t.Run(fmt.Sprintf("invoked from user: %s", tc.input), func(t *testing.T) { + user := User{DefaultSecondaryRoles: tc.input} + got := user.GetSecondaryRolesOption() + require.Equal(t, tc.want, got) + }) + } +} diff --git a/pkg/sdk/warehouses.go b/pkg/sdk/warehouses.go index 26dbeb89e8..d484d3110f 100644 --- a/pkg/sdk/warehouses.go +++ b/pkg/sdk/warehouses.go @@ -322,7 +322,7 @@ func (c *warehouses) Alter(ctx context.Context, id AccountObjectIdentifier, opts return err } defer func() { - err := c.Alter(ctx, id, &AlterWarehouseOptions{Resume: Bool(true)}) + err := c.Alter(ctx, id, &AlterWarehouseOptions{Resume: Bool(true), IfSuspended: Bool(true)}) if err != nil { log.Printf("[DEBUG] error occurred during warehouse resumption, err=%v", err) }