chore: Use new identifier with arguments in function, external functi…

…on and procedure grants (#3002)

<!-- Feel free to delete comments as you fill this in -->
- handle unusual identifiers returned in SHOW GRANTS 
- handle function, external function and procedure identifiers in
ownership and privilege grants properly
- add tests for these objects with and without arguments
- remove IsValidIdentifier from validations - it doesn't work for ids
with arguments

Note: base branch of this PR is
`identifier-with-arguments-for-procedure-and-external-function` and
should be merged soon in
#2987
 <!-- summary of changes -->

TODO:
- handle `granteeName` for shares in next PR (instead of using a
fallback in `convert`), parse the ID outside the function when we know
SHOW GRANT request details, so we can parse based on that.

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested
-->
* [x] acceptance tests
* [x] integration tests
* [x] unit tests

## References
<!-- issues documentation links, etc  -->
https://docs.snowflake.com/en/sql-reference/sql/grant-privilege and
other grant docs

---------

Co-authored-by: Jan Cieślak <[email protected]>

docs/resources/grant_privileges_to_share.md

@@ -108,6 +108,7 @@ resource "snowflake_grant_privileges_to_share" "example" {
 
 - `on_all_tables_in_schema` (String) The fully qualified identifier for the schema for which the specified privilege will be granted for all tables.
 - `on_database` (String) The fully qualified name of the database on which privileges will be granted.
+- `on_function` (String) The fully qualified name of the function on which privileges will be granted.
 - `on_schema` (String) The fully qualified name of the schema on which privileges will be granted.
 - `on_table` (String) The fully qualified name of the table on which privileges will be granted.
 - `on_tag` (String) The fully qualified name of the tag on which privileges will be granted.

pkg/acceptance/helpers/function_client.go

@@ -31,18 +31,37 @@ func (c *FunctionClient) Create(t *testing.T, arguments ...sdk.DataType) *sdk.Fu
 
 func (c *FunctionClient) CreateWithIdentifier(t *testing.T, id sdk.SchemaObjectIdentifierWithArguments) *sdk.Function {
 	t.Helper()
-	ctx := context.Background()
-	argumentRequests := make([]sdk.FunctionArgumentRequest, len(id.ArgumentDataTypes()))
-	for i, argumentDataType := range id.ArgumentDataTypes() {
-		argumentRequests[i] = *sdk.NewFunctionArgumentRequest(c.ids.Alpha(), argumentDataType)
-	}
-	err := c.client().CreateForSQL(ctx,
+
+	return c.CreateWithRequest(t, id,
 		sdk.NewCreateForSQLFunctionRequest(
 			id.SchemaObjectId(),
 			*sdk.NewFunctionReturnsRequest().WithResultDataType(*sdk.NewFunctionReturnsResultDataTypeRequest(sdk.DataTypeInt)),
 			"SELECT 1",
-		).WithArguments(argumentRequests),
+		),
 	)
+}
+
+func (c *FunctionClient) CreateSecure(t *testing.T, arguments ...sdk.DataType) *sdk.Function {
+	t.Helper()
+	id := c.ids.RandomSchemaObjectIdentifierWithArguments(arguments...)
+
+	return c.CreateWithRequest(t, id,
+		sdk.NewCreateForSQLFunctionRequest(
+			id.SchemaObjectId(),
+			*sdk.NewFunctionReturnsRequest().WithResultDataType(*sdk.NewFunctionReturnsResultDataTypeRequest(sdk.DataTypeInt)),
+			"SELECT 1",
+		).WithSecure(true),
+	)
+}
+
+func (c *FunctionClient) CreateWithRequest(t *testing.T, id sdk.SchemaObjectIdentifierWithArguments, req *sdk.CreateForSQLFunctionRequest) *sdk.Function {
+	t.Helper()
+	ctx := context.Background()
+	argumentRequests := make([]sdk.FunctionArgumentRequest, len(id.ArgumentDataTypes()))
+	for i, argumentDataType := range id.ArgumentDataTypes() {
+		argumentRequests[i] = *sdk.NewFunctionArgumentRequest(c.ids.Alpha(), argumentDataType)
+	}
+	err := c.client().CreateForSQL(ctx, req.WithArguments(argumentRequests))
 	require.NoError(t, err)
 
 	t.Cleanup(func() {

pkg/acceptance/helpers/share_client.go

@@ -26,7 +26,6 @@ func (c *ShareClient) client() sdk.Shares {
 
 func (c *ShareClient) CreateShare(t *testing.T) (*sdk.Share, func()) {
 	t.Helper()
-	// TODO(SNOW-1058419): Try with identifier containing dot during identifiers rework
 	return c.CreateShareWithIdentifier(t, c.ids.RandomAccountObjectIdentifier())
 }
 

pkg/resources/grant_ownership.go

@@ -6,8 +6,6 @@ import (
 	"log"
 	"strings"
 
-	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
-
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/logging"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
@@ -400,11 +398,6 @@ func ReadGrantOwnership(ctx context.Context, d *schema.ResourceData, meta any) d
 
 // TODO(SNOW-1229218): Make sdk.ObjectType + string objectName to sdk.ObjectIdentifier mapping available in the sdk (for all object types).
 func getOnObjectIdentifier(objectType sdk.ObjectType, objectName string) (sdk.ObjectIdentifier, error) {
-	identifier, err := helpers.DecodeSnowflakeParameterID(objectName)
-	if err != nil {
-		return nil, err
-	}
-
 	switch objectType {
 	case sdk.ObjectTypeComputePool,
 		sdk.ObjectTypeDatabase,
@@ -416,12 +409,10 @@ func getOnObjectIdentifier(objectType sdk.ObjectType, objectName string) (sdk.Ob
 		sdk.ObjectTypeRole,
 		sdk.ObjectTypeUser,
 		sdk.ObjectTypeWarehouse:
-		return sdk.NewAccountObjectIdentifier(objectName), nil
+		return sdk.ParseAccountObjectIdentifier(objectName)
 	case sdk.ObjectTypeDatabaseRole,
 		sdk.ObjectTypeSchema:
-		if _, ok := identifier.(sdk.DatabaseObjectIdentifier); !ok {
-			return nil, sdk.NewError(fmt.Sprintf("invalid object_name %s, expected database object identifier", objectName))
-		}
+		return sdk.ParseDatabaseObjectIdentifier(objectName)
 	case sdk.ObjectTypeAggregationPolicy,
 		sdk.ObjectTypeAlert,
 		sdk.ObjectTypeAuthenticationPolicy,
@@ -430,7 +421,6 @@ func getOnObjectIdentifier(objectType sdk.ObjectType, objectName string) (sdk.Ob
 		sdk.ObjectTypeEventTable,
 		sdk.ObjectTypeExternalTable,
 		sdk.ObjectTypeFileFormat,
-		sdk.ObjectTypeFunction,
 		sdk.ObjectTypeGitRepository,
 		sdk.ObjectTypeHybridTable,
 		sdk.ObjectTypeIcebergTable,
@@ -439,7 +429,6 @@ func getOnObjectIdentifier(objectType sdk.ObjectType, objectName string) (sdk.Ob
 		sdk.ObjectTypeNetworkRule,
 		sdk.ObjectTypePackagesPolicy,
 		sdk.ObjectTypePipe,
-		sdk.ObjectTypeProcedure,
 		sdk.ObjectTypeMaskingPolicy,
 		sdk.ObjectTypePasswordPolicy,
 		sdk.ObjectTypeProjectionPolicy,
@@ -453,14 +442,14 @@ func getOnObjectIdentifier(objectType sdk.ObjectType, objectName string) (sdk.Ob
 		sdk.ObjectTypeTag,
 		sdk.ObjectTypeTask,
 		sdk.ObjectTypeView:
-		if _, ok := identifier.(sdk.SchemaObjectIdentifier); !ok {
-			return nil, sdk.NewError(fmt.Sprintf("invalid object_name %s, expected schema object identifier", objectName))
-		}
+		return sdk.ParseSchemaObjectIdentifier(objectName)
+	case sdk.ObjectTypeFunction,
+		sdk.ObjectTypeProcedure,
+		sdk.ObjectTypeExternalFunction:
+		return sdk.ParseSchemaObjectIdentifierWithArguments(objectName)
 	default:
 		return nil, sdk.NewError(fmt.Sprintf("object_type %s is not supported, please create a feature request for the provider if given object_type should be supported", objectType))
 	}
-
-	return identifier, nil
 }
 
 func getOwnershipGrantOn(d *schema.ResourceData) (*sdk.OwnershipGrantOn, error) {

pkg/resources/grant_ownership_acceptance_test.go

@@ -323,6 +323,101 @@ func TestAcc_GrantOwnership_OnObject_Table_ToDatabaseRole(t *testing.T) {
 	})
 }
 
+func TestAcc_GrantOwnership_OnObject_ProcedureWithArguments_ToAccountRole(t *testing.T) {
+	databaseId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
+	schemaId := acc.TestClient().Ids.RandomDatabaseObjectIdentifierInDatabase(databaseId)
+	procedureId := acc.TestClient().Ids.NewSchemaObjectIdentifierWithArgumentsInSchema(acc.TestClient().Ids.Alpha(), schemaId, sdk.DataTypeFloat)
+	accountRoleId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
+
+	configVariables := config.Variables{
+		"account_role_name": config.StringVariable(accountRoleId.Name()),
+		"database_name":     config.StringVariable(databaseId.Name()),
+		"schema_name":       config.StringVariable(schemaId.Name()),
+		"procedure_name":    config.StringVariable(procedureId.Name()),
+	}
+	resourceName := "snowflake_grant_ownership.test"
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		Steps: []resource.TestStep{
+			{
+				ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantOwnership/OnObject_Procedure_ToAccountRole"),
+				ConfigVariables: configVariables,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "account_role_name", accountRoleId.Name()),
+					resource.TestCheckResourceAttr(resourceName, "on.0.object_type", "PROCEDURE"),
+					resource.TestCheckResourceAttr(resourceName, "on.0.object_name", procedureId.FullyQualifiedName()),
+					resource.TestCheckResourceAttr(resourceName, "id", fmt.Sprintf("ToAccountRole|%s||OnObject|PROCEDURE|%s", accountRoleId.FullyQualifiedName(), procedureId.FullyQualifiedName())),
+					checkResourceOwnershipIsGranted(&sdk.ShowGrantOptions{
+						To: &sdk.ShowGrantsTo{
+							Role: accountRoleId,
+						},
+					}, sdk.ObjectTypeProcedure, accountRoleId.Name(), procedureId.FullyQualifiedName()),
+				),
+			},
+			{
+				ConfigDirectory:   acc.ConfigurationDirectory("TestAcc_GrantOwnership/OnObject_Procedure_ToAccountRole"),
+				ConfigVariables:   configVariables,
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAcc_GrantOwnership_OnObject_ProcedureWithoutArguments_ToDatabaseRole(t *testing.T) {
+	databaseId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
+	schemaId := acc.TestClient().Ids.RandomDatabaseObjectIdentifierInDatabase(databaseId)
+	procedureId := acc.TestClient().Ids.NewSchemaObjectIdentifierWithArgumentsInSchema(acc.TestClient().Ids.Alpha(), schemaId)
+
+	databaseRoleId := acc.TestClient().Ids.RandomDatabaseObjectIdentifierInDatabase(databaseId)
+
+	configVariables := config.Variables{
+		"database_role_name": config.StringVariable(databaseRoleId.Name()),
+		"database_name":      config.StringVariable(databaseId.Name()),
+		"schema_name":        config.StringVariable(schemaId.Name()),
+		"procedure_name":     config.StringVariable(procedureId.Name()),
+	}
+	resourceName := "snowflake_grant_ownership.test"
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		Steps: []resource.TestStep{
+			{
+				ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantOwnership/OnObject_Procedure_ToDatabaseRole"),
+				ConfigVariables: configVariables,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "database_role_name", databaseRoleId.FullyQualifiedName()),
+					resource.TestCheckResourceAttr(resourceName, "on.0.object_type", "PROCEDURE"),
+					resource.TestCheckResourceAttr(resourceName, "on.0.object_name", procedureId.FullyQualifiedName()),
+					resource.TestCheckResourceAttr(resourceName, "id", fmt.Sprintf("ToDatabaseRole|%s||OnObject|PROCEDURE|%s", databaseRoleId.FullyQualifiedName(), procedureId.FullyQualifiedName())),
+					checkResourceOwnershipIsGranted(&sdk.ShowGrantOptions{
+						To: &sdk.ShowGrantsTo{
+							DatabaseRole: databaseRoleId,
+						},
+					}, sdk.ObjectTypeProcedure, databaseRoleId.Name(), procedureId.FullyQualifiedName()),
+				),
+			},
+			{
+				ConfigDirectory:   acc.ConfigurationDirectory("TestAcc_GrantOwnership/OnObject_Procedure_ToDatabaseRole"),
+				ConfigVariables:   configVariables,
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func TestAcc_GrantOwnership_OnAll_InDatabase_ToAccountRole(t *testing.T) {
 	databaseId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
 	accountRoleId := acc.TestClient().Ids.RandomAccountObjectIdentifier()

pkg/resources/grant_ownership_identifier_test.go

@@ -130,6 +130,34 @@ func TestParseGrantOwnershipId(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name:       "grant ownership on function to account role",
+			Identifier: `ToAccountRole|"account-role"|COPY|OnObject|FUNCTION|"database-name"."schema-name"."function-name"(FLOAT)`,
+			Expected: GrantOwnershipId{
+				GrantOwnershipTargetRoleKind: ToAccountGrantOwnershipTargetRoleKind,
+				AccountRoleName:              sdk.NewAccountObjectIdentifier("account-role"),
+				OutboundPrivilegesBehavior:   sdk.Pointer(CopyOutboundPrivilegesBehavior),
+				Kind:                         OnObjectGrantOwnershipKind,
+				Data: &OnObjectGrantOwnershipData{
+					ObjectType: sdk.ObjectTypeFunction,
+					ObjectName: sdk.NewSchemaObjectIdentifierWithArguments("database-name", "schema-name", "function-name", sdk.DataTypeFloat),
+				},
+			},
+		},
+		{
+			Name:       "grant ownership on function without arguments to database role",
+			Identifier: `ToDatabaseRole|"database-name"."database-role"|REVOKE|OnObject|FUNCTION|"database-name"."schema-name"."function-name"()`,
+			Expected: GrantOwnershipId{
+				GrantOwnershipTargetRoleKind: ToDatabaseGrantOwnershipTargetRoleKind,
+				DatabaseRoleName:             sdk.NewDatabaseObjectIdentifier("database-name", "database-role"),
+				OutboundPrivilegesBehavior:   sdk.Pointer(RevokeOutboundPrivilegesBehavior),
+				Kind:                         OnObjectGrantOwnershipKind,
+				Data: &OnObjectGrantOwnershipData{
+					ObjectType: sdk.ObjectTypeFunction,
+					ObjectName: sdk.NewSchemaObjectIdentifierWithArguments("database-name", "schema-name", "function-name", []sdk.DataType{}...),
+				},
+			},
+		},
 		{
 			Name:       "validation: not enough parts",
 			Identifier: `ToDatabaseRole|"database-name"."role-name"|`,

pkg/resources/grant_ownership_test.go

@@ -55,14 +55,14 @@ func TestGetOnObjectIdentifier(t *testing.T) {
 		{
 			Name:       "account object identifier with dots",
 			ObjectType: sdk.ObjectTypeDatabase,
-			ObjectName: "database.name.with.dots",
+			ObjectName: "\"database.name.with.dots\"",
 			Expected:   sdk.NewAccountObjectIdentifier("database.name.with.dots"),
 		},
 		{
 			Name:       "validation - valid identifier",
 			ObjectType: sdk.ObjectTypeDatabase,
 			ObjectName: "to.many.parts.in.this.identifier",
-			Error:      "unable to classify identifier",
+			Error:      "unexpected number of parts 6 in identifier to.many.parts.in.this.identifier, expected 1 in a form of \"<account_object_name>\"",
 		},
 		{
 			Name:       "validation - unsupported type",
@@ -74,13 +74,13 @@ func TestGetOnObjectIdentifier(t *testing.T) {
 			Name:       "validation - invalid database object identifier",
 			ObjectType: sdk.ObjectTypeSchema,
 			ObjectName: "test_database.test_schema.test_table",
-			Error:      "invalid object_name test_database.test_schema.test_table, expected database object identifier",
+			Error:      "unexpected number of parts 3 in identifier test_database.test_schema.test_table, expected 2 in a form of \"<database_name>.<database_object_name>\"",
 		},
 		{
 			Name:       "validation - invalid schema object identifier",
 			ObjectType: sdk.ObjectTypeTable,
 			ObjectName: "test_database.test_schema.test_table.column_name",
-			Error:      "invalid object_name test_database.test_schema.test_table.column_name, expected schema object identifier",
+			Error:      "unexpected number of parts 4 in identifier test_database.test_schema.test_table.column_name, expected 3 in a form of \"<database_name>.<schema_name>.<schema_object_name>\"",
 		},
 	}
 
@@ -248,7 +248,7 @@ func TestGetOwnershipGrantOn(t *testing.T) {
 				"object_type": "SCHEMA",
 				"object_name": "test_database.test_schema.test_table",
 			},
-			Error: "invalid object_name test_database.test_schema.test_table, expected database object identifier",
+			Error: "unexpected number of parts 3 in identifier test_database.test_schema.test_table, expected 2 in a form of \"<database_name>.<database_object_name>\"",
 		},
 	}