Skip to content

Commit

Permalink
fix: Add fix for model grants (#3070)
Browse files Browse the repository at this point in the history
## Changes
- Address the issue:
#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.
  • Loading branch information
sfc-gh-jcieslak authored Sep 17, 2024
1 parent 01b66cf commit cbc3d74
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/acceptance/helpers/database_role_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
54 changes: 54 additions & 0 deletions pkg/resources/grant_privileges_to_account_role_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
55 changes: 55 additions & 0 deletions pkg/resources/grant_privileges_to_database_role_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
}
6 changes: 6 additions & 0 deletions pkg/sdk/grants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit cbc3d74

Please sign in to comment.