Skip to content

Commit

Permalink
fix: Fix default secondary roles after BCR 2024_07 (#3040)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
sfc-gh-asawicki authored Sep 4, 2024
1 parent 6cb0b4e commit 2ca465a
Show file tree
Hide file tree
Showing 21 changed files with 821 additions and 163 deletions.
20 changes: 17 additions & 3 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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`
Expand All @@ -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`.
Expand Down
4 changes: 2 additions & 2 deletions docs/resources/user.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "..."
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion examples/resources/snowflake_user/resource.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "..."
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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))
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 2 additions & 4 deletions pkg/acceptance/bettertestspoc/config/model/user_model_ext.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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))
}
11 changes: 7 additions & 4 deletions pkg/acceptance/bettertestspoc/config/model/user_model_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 43 additions & 0 deletions pkg/acceptance/helpers/bcr_bundles_client.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
2 changes: 2 additions & 0 deletions pkg/acceptance/helpers/test_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type TestClient struct {
ApiIntegration *ApiIntegrationClient
Application *ApplicationClient
ApplicationPackage *ApplicationPackageClient
BcrBundles *BcrBundlesClient
Context *ContextClient
CortexSearchService *CortexSearchServiceClient
CatalogIntegration *CatalogIntegrationClient
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 2 additions & 1 deletion pkg/datasources/users_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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).
Expand Down
4 changes: 2 additions & 2 deletions pkg/resources/diff_suppressions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand All @@ -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)
}
Expand Down
Loading

0 comments on commit 2ca465a

Please sign in to comment.