From cbc3d746c7153025598b4c9d5cccfaae3a957c0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Tue, 17 Sep 2024 14:04:07 +0200 Subject: [PATCH] fix: Add fix for model grants (#3070) ## Changes - Address the issue: https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/3050 about granting usage on future models (I couldn't provide the test on non-future model because the only way of creating such model is with python snowpark and it's not that easy, I tried). - Fix other failing test in grants. --- .../helpers/database_role_client.go | 2 +- ...vileges_to_account_role_acceptance_test.go | 54 ++++++++++++++++++ ...ileges_to_database_role_acceptance_test.go | 55 +++++++++++++++++++ pkg/sdk/grants.go | 6 ++ 4 files changed, 116 insertions(+), 1 deletion(-) diff --git a/pkg/acceptance/helpers/database_role_client.go b/pkg/acceptance/helpers/database_role_client.go index 75893addd5..81afb5072f 100644 --- a/pkg/acceptance/helpers/database_role_client.go +++ b/pkg/acceptance/helpers/database_role_client.go @@ -62,7 +62,7 @@ func (c *DatabaseRoleClient) CleanupDatabaseRoleFunc(t *testing.T, id sdk.Databa return func() { // to prevent error when db was removed before the role _, err := c.context.client.Databases.ShowByID(ctx, id.DatabaseId()) - if errors.Is(err, sdk.ErrObjectNotExistOrAuthorized) { + if errors.Is(err, sdk.ErrObjectNotFound) { return } diff --git a/pkg/resources/grant_privileges_to_account_role_acceptance_test.go b/pkg/resources/grant_privileges_to_account_role_acceptance_test.go index 0b70cc4375..22a9dbeba1 100644 --- a/pkg/resources/grant_privileges_to_account_role_acceptance_test.go +++ b/pkg/resources/grant_privileges_to_account_role_acceptance_test.go @@ -4,8 +4,12 @@ import ( "context" "fmt" "regexp" + "strconv" + "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" @@ -1810,3 +1814,53 @@ func TestAcc_GrantPrivilegesToAccountRole_OnDataset_issue2807(t *testing.T) { }, }) } + +// proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/3050 +func TestAcc_GrantPrivilegesToAccountRole_OnFutureModels_issue3050(t *testing.T) { + accountRoleName := acc.TestClient().Ids.RandomAccountObjectIdentifier().Name() + databaseName := acc.TestClient().Ids.DatabaseId().Name() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckAccountRolePrivilegesRevoked(t), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.95.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: grantPrivilegesToAccountRoleOnFutureInDatabaseConfig(accountRoleName, []string{"USAGE"}, sdk.PluralObjectTypeModels, databaseName), + ExpectNonEmptyPlan: true, + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + Config: grantPrivilegesToAccountRoleOnFutureInDatabaseConfig(accountRoleName, []string{"USAGE"}, sdk.PluralObjectTypeModels, databaseName), + }, + }, + }) +} + +func grantPrivilegesToAccountRoleOnFutureInDatabaseConfig(accountRoleName string, privileges []string, objectTypePlural sdk.PluralObjectType, databaseName string) string { + return fmt.Sprintf(` +resource "snowflake_account_role" "test" { + name = "%[1]s" +} + +resource "snowflake_grant_privileges_to_account_role" "test" { + account_role_name = snowflake_account_role.test.name + privileges = [ %[2]s ] + + on_schema_object { + future { + object_type_plural = "%[3]s" + in_database = "%[4]s" + } + } +} +`, accountRoleName, strings.Join(collections.Map(privileges, strconv.Quote), ","), objectTypePlural, databaseName) +} diff --git a/pkg/resources/grant_privileges_to_database_role_acceptance_test.go b/pkg/resources/grant_privileges_to_database_role_acceptance_test.go index 47872e837e..b3b491cfd8 100644 --- a/pkg/resources/grant_privileges_to_database_role_acceptance_test.go +++ b/pkg/resources/grant_privileges_to_database_role_acceptance_test.go @@ -4,8 +4,12 @@ import ( "context" "fmt" "regexp" + "strconv" + "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" @@ -1480,3 +1484,54 @@ func TestAcc_GrantPrivilegesToDatabaseRole_IdentifierQuotingDiffSuppression(t *t }, }) } + +// proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/3050 +func TestAcc_GrantPrivilegesToDatabaseRole_OnFutureModels_issue3050(t *testing.T) { + databaseRoleId := acc.TestClient().Ids.RandomDatabaseObjectIdentifier() + databaseName := acc.TestClient().Ids.DatabaseId().Name() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckAccountRolePrivilegesRevoked(t), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.95.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: grantPrivilegesToDatabaseRoleOnFutureInDatabaseConfig(databaseRoleId, []string{"USAGE"}, sdk.PluralObjectTypeModels, databaseName), + ExpectNonEmptyPlan: true, + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + Config: grantPrivilegesToDatabaseRoleOnFutureInDatabaseConfig(databaseRoleId, []string{"USAGE"}, sdk.PluralObjectTypeModels, databaseName), + }, + }, + }) +} + +func grantPrivilegesToDatabaseRoleOnFutureInDatabaseConfig(databaseRoleId sdk.DatabaseObjectIdentifier, privileges []string, objectTypePlural sdk.PluralObjectType, databaseName string) string { + return fmt.Sprintf(` +resource "snowflake_database_role" "test" { + name = "%[1]s" + database = "%[2]s" +} + +resource "snowflake_grant_privileges_to_database_role" "test" { + database_role_name = snowflake_database_role.test.fully_qualified_name + privileges = [ %[3]s ] + + on_schema_object { + future { + object_type_plural = "%[4]s" + in_database = "%[5]s" + } + } +} +`, databaseRoleId.Name(), databaseRoleId.DatabaseName(), strings.Join(collections.Map(privileges, strconv.Quote), ","), objectTypePlural, databaseName) +} diff --git a/pkg/sdk/grants.go b/pkg/sdk/grants.go index 5660ecc4ab..e82e26f9bf 100644 --- a/pkg/sdk/grants.go +++ b/pkg/sdk/grants.go @@ -237,6 +237,9 @@ func (row grantRow) convert() *Grant { if row.GrantedOn == "VOLUME" { grantedOn = ObjectTypeExternalVolume } + if row.GrantedOn == "MODULE" { + grantedOn = ObjectTypeModel + } var grantOn ObjectType // true for future grants @@ -246,6 +249,9 @@ func (row grantRow) convert() *Grant { if row.GrantOn == "VOLUME" { grantOn = ObjectTypeExternalVolume } + if row.GrantOn == "MODULE" { + grantOn = ObjectTypeModel + } var name ObjectIdentifier var err error