fix: Apply various fixes (#3176)

Apply various fixes:
- Fix handling compute pool privileges (#2717)
- Fail to reproduce the problem with password policy user attachment
(#3005)
- Adapt user to BCR Bundle 2024_08 (#3125)
- Loosen identifier validations - parentheses (#3127) - check below
- Prove MANAGE SHARE TARGET works correctly (#3153)

On the identifier validation topic:
ParseIdentifierString should generally allow parentheses. It should
validate them for the identifiers for functions, procedures, etc.
Because of that:
- this validation was removed
- method usages were analyzed to check what consequences it has
throughout the provider
  - DecodeSnowflakeAccountIdentifier - OK, account level identifier
  - DecodeSnowflakeParameterID
- buildOptsForGrantsOn (grants datasource) - NOK, had to fix the logic
- ContainsIdentifierIgnoringQuotes - OK, transitively used only in
network policies
    - TestDecodeSnowflakeParameterID - OK
    - IsValidIdentifier - OK, used for other identifier types
- pkg/resource - OK, used in streams, table constraints and tag masking
policy associations
  - suppressIdentifierQuoting
- used in non-grant resources with non-argument identifier types - OK
- used in grant resources - OK, the validation will be relaxed for now,
diff suppression won't work correctly for the identifiers with
arguments, will be addressed with functions/procedures rework
(multi-field validation could be handled for such cases, issue added;
references:
hashicorp/terraform-plugin-sdk#354,
hashicorp/terraform-plugin-sdk#233)
- suppressIdentifierQuotingPartiallyQualifiedName - as above; currently
used only for streams
- parseIdentifier - used by other identifier types (type constraints
added)
- ParseObjectIdentifierString - OK, used for other identifier types
(ParseSchemaObjectIdentifierWithArguments is dedicated for identifier
with arguments)
- ParseSchemaObjectIdentifierWithArguments - OK, we split the input
string on first opening paren (so there are no other opening parens
there)
- Test_ParseIdentifierString - tests adjusted for the removed validation

Others:
- Remove unused privileges.go file
- Fix preview resources list for V1

References:
-
#2717
-
#3005
-
#3125
-
#3127
-
#3153

MIGRATION_GUIDE.md

@@ -159,6 +159,14 @@ This segregation was based on the secret flows in CREATE SECRET. i.e.:
 
 See reference [docs](https://docs.snowflake.com/en/sql-reference/sql/create-secret).
 
+### *(bugfix)* Handle BCR Bundle 2024_08 in snowflake_user resource
+
+[bcr 2024_08](https://docs.snowflake.com/en/release-notes/bcr-bundles/2024_08/bcr-1798) changed the "empty" response in the `SHOW USERS` query. This provider version adapts to the new result types; it should be used if you want to have 2024_08 Bundle enabled on your account.
+
+Note: Because [bcr 2024_07](https://docs.snowflake.com/en/release-notes/bcr-bundles/2024_07/bcr-1692) changes the way how the `default_secondary_roles` attribute behaves, drift may be reported when enabling 2024_08 Bundle. Check [Handling default secondary roles](#breaking-change-handling-default-secondary-roles) for more context.
+
+Connected issues: [#3125](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/3125)
+
 ## v0.96.0 ➞ v0.97.0
 
 ### *(new feature)* snowflake_stream_on_table, snowflake_stream_on_external_table resource

pkg/acceptance/bettertestspoc/assert/resourceassert/user_resource_ext.go

@@ -3,6 +3,8 @@ package resourceassert
 import (
 	"strconv"
 
+	r "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/resources"
+
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 )
@@ -25,3 +27,29 @@ func (u *UserResourceAssert) HasMustChangePassword(expected bool) *UserResourceA
 func (u *UserResourceAssert) HasDefaultSecondaryRolesOption(expected sdk.SecondaryRolesOption) *UserResourceAssert {
 	return u.HasDefaultSecondaryRolesOptionString(string(expected))
 }
+
+func (u *UserResourceAssert) HasAllDefaults(userId sdk.AccountObjectIdentifier, expectedDefaultSecondaryRoles sdk.SecondaryRolesOption) *UserResourceAssert {
+	return u.
+		HasNameString(userId.Name()).
+		HasNoPassword().
+		HasNoLoginName().
+		HasNoDisplayName().
+		HasNoFirstName().
+		HasNoMiddleName().
+		HasNoLastName().
+		HasNoEmail().
+		HasMustChangePasswordString(r.BooleanDefault).
+		HasDisabledString(r.BooleanDefault).
+		HasNoDaysToExpiry().
+		HasMinsToUnlockString(r.IntDefaultString).
+		HasNoDefaultWarehouse().
+		HasNoDefaultNamespace().
+		HasNoDefaultRole().
+		HasDefaultSecondaryRolesOption(expectedDefaultSecondaryRoles).
+		HasMinsToBypassMfaString(r.IntDefaultString).
+		HasNoRsaPublicKey().
+		HasNoRsaPublicKey2().
+		HasNoComment().
+		HasDisableMfaString(r.BooleanDefault).
+		HasFullyQualifiedNameString(userId.FullyQualifiedName())
+}

pkg/acceptance/helpers/compute_pool_client.go

@@ -0,0 +1,47 @@
+package helpers
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
+	"github.com/stretchr/testify/require"
+)
+
+// TODO [SNOW-1790174]: change raw sqls to proper client
+type ComputePoolClient struct {
+	context *TestClientContext
+	ids     *IdsGenerator
+}
+
+func NewComputePoolClient(context *TestClientContext, idsGenerator *IdsGenerator) *ComputePoolClient {
+	return &ComputePoolClient{
+		context: context,
+		ids:     idsGenerator,
+	}
+}
+
+func (c *ComputePoolClient) client() *sdk.Client {
+	return c.context.client
+}
+
+func (c *ComputePoolClient) CreateComputePool(t *testing.T) (sdk.AccountObjectIdentifier, func()) {
+	t.Helper()
+	ctx := context.Background()
+
+	id := c.ids.RandomAccountObjectIdentifier()
+	_, err := c.client().ExecForTests(ctx, fmt.Sprintf(`CREATE COMPUTE POOL %s MIN_NODES = 1 MAX_NODES = 1 INSTANCE_FAMILY = CPU_X64_XS`, id.FullyQualifiedName()))
+	require.NoError(t, err)
+	return id, c.DropComputePoolFunc(t, id)
+}
+
+func (c *ComputePoolClient) DropComputePoolFunc(t *testing.T, id sdk.AccountObjectIdentifier) func() {
+	t.Helper()
+	ctx := context.Background()
+
+	return func() {
+		_, err := c.client().ExecForTests(ctx, fmt.Sprintf(`DROP COMPUTE POOL IF EXISTS %s`, id.FullyQualifiedName()))
+		require.NoError(t, err)
+	}
+}

pkg/acceptance/helpers/function_client.go

@@ -64,12 +64,20 @@ func (c *FunctionClient) CreateWithRequest(t *testing.T, id sdk.SchemaObjectIden
 	err := c.client().CreateForSQL(ctx, req.WithArguments(argumentRequests))
 	require.NoError(t, err)
 
-	t.Cleanup(func() {
-		require.NoError(t, c.context.client.Functions.Drop(ctx, sdk.NewDropFunctionRequest(id).WithIfExists(true)))
-	})
+	t.Cleanup(c.DropFunctionFunc(t, id))
 
 	function, err := c.client().ShowByID(ctx, id)
 	require.NoError(t, err)
 
 	return function
 }
+
+func (c *FunctionClient) DropFunctionFunc(t *testing.T, id sdk.SchemaObjectIdentifierWithArguments) func() {
+	t.Helper()
+	ctx := context.Background()
+
+	return func() {
+		err := c.client().Drop(ctx, sdk.NewDropFunctionRequest(id).WithIfExists(true))
+		require.NoError(t, err)
+	}
+}

pkg/acceptance/helpers/test_client.go

@@ -17,6 +17,7 @@ type TestClient struct {
 	ApplicationPackage           *ApplicationPackageClient
 	AuthenticationPolicy         *AuthenticationPolicyClient
 	BcrBundles                   *BcrBundlesClient
+	ComputePool                  *ComputePoolClient
 	Connection                   *ConnectionClient
 	Context                      *ContextClient
 	CortexSearchService          *CortexSearchServiceClient
@@ -85,6 +86,7 @@ func NewTestClient(c *sdk.Client, database string, schema string, warehouse stri
 		ApplicationPackage:           NewApplicationPackageClient(context, idsGenerator),
 		AuthenticationPolicy:         NewAuthenticationPolicyClient(context, idsGenerator),
 		BcrBundles:                   NewBcrBundlesClient(context),
+		ComputePool:                  NewComputePoolClient(context, idsGenerator),
 		Connection:                   NewConnectionClient(context, idsGenerator),
 		Context:                      NewContextClient(context),
 		CortexSearchService:          NewCortexSearchServiceClient(context, idsGenerator),

pkg/datasources/grants.go

@@ -378,13 +378,27 @@ func buildOptsForGrantsOn(grantsOn map[string]any) (*sdk.ShowGrantOptions, error
 		if objectType == "" || objectName == "" {
 			return nil, fmt.Errorf("object_type (%s) or object_name (%s) missing", objectType, objectName)
 		}
-		objectId, err := helpers.DecodeSnowflakeParameterID(objectName)
-		if err != nil {
-			return nil, err
+
+		sdkObjectType := sdk.ObjectType(objectType)
+		var objectId sdk.ObjectIdentifier
+		var err error
+		// TODO [SNOW-1569535]: use a mapper from object type to parsing function
+		// TODO [SNOW-1569535]: grant_ownership#getOnObjectIdentifier could be used but it is limited only to ownership-transferable objects (according to the docs) - we should add an integration test to verify if the docs are complete
+		if sdkObjectType.IsWithArguments() {
+			objectId, err = sdk.ParseSchemaObjectIdentifierWithArguments(objectName)
+			if err != nil {
+				return nil, err
+			}
+		} else {
+			objectId, err = helpers.DecodeSnowflakeParameterID(objectName)
+			if err != nil {
+				return nil, err
+			}
 		}
+
 		opts.On = &sdk.ShowGrantsOn{
 			Object: &sdk.Object{
-				ObjectType: sdk.ObjectType(objectType),
+				ObjectType: sdkObjectType,
 				Name:       objectId,
 			},
 		}

pkg/datasources/grants_acceptance_test.go

@@ -6,6 +6,8 @@ import (
 
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
 
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs"
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 	"github.com/hashicorp/terraform-plugin-testing/config"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/tfversion"
@@ -99,6 +101,32 @@ func TestAcc_Grants_On_SchemaObject(t *testing.T) {
 	})
 }
 
+func TestAcc_Grants_On_SchemaObject_WithArguments(t *testing.T) {
+	_ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance)
+	acc.TestAccPreCheck(t)
+
+	function := acc.TestClient().Function.Create(t, sdk.DataTypeVARCHAR)
+	configVariables := config.Variables{
+		"fully_qualified_function_name": config.StringVariable(function.ID().FullyQualifiedName()),
+	}
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: nil,
+		Steps: []resource.TestStep{
+			{
+				ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Grants/On/SchemaObject_WithArguments"),
+				ConfigVariables: configVariables,
+				Check:           checkAtLeastOneGrantPresent(),
+			},
+		},
+	})
+}
+
 func TestAcc_Grants_On_Invalid_NoAttribute(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,

pkg/datasources/testdata/TestAcc_Grants/On/SchemaObject_WithArguments/snowflake_grants_on_schema_object.tf

@@ -0,0 +1,6 @@
+data "snowflake_grants" "test" {
+  grants_on {
+    object_name = var.fully_qualified_function_name
+    object_type = "FUNCTION"
+  }
+}

pkg/datasources/testdata/TestAcc_Grants/On/SchemaObject_WithArguments/variables.tf

@@ -0,0 +1,3 @@
+variable "fully_qualified_function_name" {
+  type = string
+}

pkg/resources/custom_diffs_test.go

@@ -2,14 +2,14 @@ package resources_test
 
 import (
 	"context"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"strings"
 	"testing"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/resources"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 	"github.com/hashicorp/go-cty/cty"
+	"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"
 	"github.com/stretchr/testify/assert"

pkg/resources/grant_privileges_to_account_role.go

@@ -1183,6 +1183,9 @@ func createGrantPrivilegesToAccountRoleIdFromSchema(d *schema.ResourceData) (id
 		case on.AccountObject.ReplicationGroup != nil:
 			onAccountObjectGrantData.ObjectType = sdk.ObjectTypeReplicationGroup
 			onAccountObjectGrantData.ObjectName = *on.AccountObject.ReplicationGroup
+		case on.AccountObject.ComputePool != nil:
+			onAccountObjectGrantData.ObjectType = sdk.ObjectTypeComputePool
+			onAccountObjectGrantData.ObjectName = *on.AccountObject.ComputePool
 		case on.AccountObject.ExternalVolume != nil:
 			onAccountObjectGrantData.ObjectType = sdk.ObjectTypeExternalVolume
 			onAccountObjectGrantData.ObjectName = *on.AccountObject.ExternalVolume

pkg/resources/grant_privileges_to_account_role_acceptance_test.go

@@ -8,11 +8,10 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections"
-
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs"
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 	"github.com/hashicorp/terraform-plugin-testing/config"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
@@ -70,6 +69,45 @@ func TestAcc_GrantPrivilegesToAccountRole_OnAccount(t *testing.T) {
 	})
 }
 
+func TestAcc_GrantPrivilegesToAccountRole_OnAccount_gh3153(t *testing.T) {
+	roleId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
+	roleFullyQualifiedName := roleId.FullyQualifiedName()
+	configVariables := config.Variables{
+		"name": config.StringVariable(roleFullyQualifiedName),
+		"privileges": config.ListVariable(
+			config.StringVariable(string(sdk.GlobalPrivilegeManageShareTarget)),
+		),
+	}
+	resourceName := "snowflake_grant_privileges_to_account_role.test"
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: acc.CheckAccountRolePrivilegesRevoked(t),
+		Steps: []resource.TestStep{
+			{
+				PreConfig: func() {
+					_, roleCleanup := acc.TestClient().Role.CreateRoleWithIdentifier(t, roleId)
+					t.Cleanup(roleCleanup)
+					acc.TestClient().BcrBundles.EnableBcrBundle(t, "2024_07")
+				},
+				ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToAccountRole/OnAccount_gh3153"),
+				ConfigVariables: configVariables,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "account_role_name", roleFullyQualifiedName),
+					resource.TestCheckResourceAttr(resourceName, "privileges.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "privileges.0", string(sdk.GlobalPrivilegeManageShareTarget)),
+					resource.TestCheckResourceAttr(resourceName, "on_account", "true"),
+					resource.TestCheckResourceAttr(resourceName, "id", fmt.Sprintf("%s|false|false|%s|OnAccount", roleFullyQualifiedName, sdk.GlobalPrivilegeManageShareTarget)),
+				),
+			},
+		},
+	})
+}
+
 func TestAcc_GrantPrivilegesToAccountRole_OnAccount_PrivilegesReversed(t *testing.T) {
 	roleId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
 	roleFullyQualifiedName := roleId.FullyQualifiedName()
@@ -177,6 +215,53 @@ func TestAcc_GrantPrivilegesToAccountRole_OnAccountObject(t *testing.T) {
 	})
 }
 
+func TestAcc_GrantPrivilegesToAccountRole_OnAccountObject_gh2717(t *testing.T) {
+	_ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance)
+	acc.TestAccPreCheck(t)
+
+	computePoolId, computePoolCleanup := acc.TestClient().ComputePool.CreateComputePool(t)
+	t.Cleanup(computePoolCleanup)
+
+	roleId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
+	roleFullyQualifiedName := roleId.FullyQualifiedName()
+	configVariables := config.Variables{
+		"name":         config.StringVariable(roleFullyQualifiedName),
+		"compute_pool": config.StringVariable(computePoolId.Name()),
+		"privileges": config.ListVariable(
+			config.StringVariable(string(sdk.AccountObjectPrivilegeUsage)),
+		),
+	}
+	resourceName := "snowflake_grant_privileges_to_account_role.test"
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: acc.CheckAccountRolePrivilegesRevoked(t),
+		Steps: []resource.TestStep{
+			{
+				PreConfig: func() {
+					_, roleCleanup := acc.TestClient().Role.CreateRoleWithIdentifier(t, roleId)
+					t.Cleanup(roleCleanup)
+				},
+				ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToAccountRole/OnAccountObject_gh2717"),
+				ConfigVariables: configVariables,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "account_role_name", roleFullyQualifiedName),
+					resource.TestCheckResourceAttr(resourceName, "privileges.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "privileges.0", string(sdk.AccountObjectPrivilegeUsage)),
+					resource.TestCheckResourceAttr(resourceName, "on_account_object.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "on_account_object.0.object_type", string(sdk.ObjectTypeComputePool)),
+					resource.TestCheckResourceAttr(resourceName, "on_account_object.0.object_name", computePoolId.Name()),
+					resource.TestCheckResourceAttr(resourceName, "id", fmt.Sprintf("%s|false|false|USAGE|OnAccountObject|%s|%s", roleFullyQualifiedName, sdk.ObjectTypeComputePool, computePoolId.FullyQualifiedName())),
+				),
+			},
+		},
+	})
+}
+
 // This proves that infinite plan is not produced as in snowflake_grant_privileges_to_role.
 // More details can be found in the fix pr https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/2364.
 func TestAcc_GrantPrivilegesToApplicationRole_OnAccountObject_InfinitePlan(t *testing.T) {