Skip to content

Commit

Permalink
feat: Handle IMPORTED PRIVILEGES privileges in privilege-to-role gran…
Browse files Browse the repository at this point in the history
…ting resources (#2471)

IMPORTED PRIVILEGES privilege is a special case where returned (IMPORTED
PRIVILEGES) privilege from SHOW GRANTS shows up as a USAGE privilege.
Thus, I had to create a logic that would swap those two in the Read
operation. As IMPORTED PRIVILEGES is a privilege that can only be
applied to the database created from share, I had to create a test with
share and the database created on the second account.

References
-
#1890 (comment)
-
#1998
  • Loading branch information
sfc-gh-jcieslak authored Feb 8, 2024
1 parent 93af462 commit eb20051
Show file tree
Hide file tree
Showing 8 changed files with 318 additions and 11 deletions.
32 changes: 26 additions & 6 deletions pkg/resources/grant_privileges_to_account_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"database/sql"
"fmt"
"slices"
"strings"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/logging"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
Expand All @@ -22,6 +23,10 @@ var grantPrivilegesToAccountRoleSchema = map[string]*schema.Schema{
Description: "The fully qualified name of the account role to which privileges will be granted.",
ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](),
},
// According to docs https://docs.snowflake.com/en/user-guide/data-exchange-marketplace-privileges#usage-notes IMPORTED PRIVILEGES
// will be returned as USAGE in SHOW GRANTS command. In addition, USAGE itself is a valid privilege, but both cannot be set at the
// same time (IMPORTED PRIVILEGES can only be granted to the database created from SHARE and USAGE in every other case).
// To handle both cases, additional logic was added in read operation where IMPORTED PRIVILEGES is replaced with USAGE.
"privileges": {
Type: schema.TypeSet,
Optional: true,
Expand Down Expand Up @@ -746,7 +751,15 @@ func ReadGrantPrivilegesToAccountRole(ctx context.Context, d *schema.ResourceDat
}
}

var privileges []string
actualPrivileges := make([]string, 0)
expectedPrivileges := make([]string, 0)
expectedPrivileges = append(expectedPrivileges, id.Privileges...)

if slices.ContainsFunc(expectedPrivileges, func(s string) bool {
return strings.ToUpper(s) == sdk.AccountObjectPrivilegeImportedPrivileges.String()
}) {
expectedPrivileges = append(expectedPrivileges, sdk.AccountObjectPrivilegeUsage.String())
}

logging.DebugLogger.Printf("[DEBUG] Filtering grants to be set on account: count = %d", len(grants))
for _, grant := range grants {
Expand All @@ -756,7 +769,7 @@ func ReadGrantPrivilegesToAccountRole(ctx context.Context, d *schema.ResourceDat
}
// Only consider privileges that are already present in the ID, so we
// don't delete privileges managed by other resources.
if !slices.Contains(id.Privileges, grant.Privilege) {
if !slices.Contains(expectedPrivileges, grant.Privilege) {
continue
}
if grant.GrantOption == id.WithGrantOption && grant.GranteeName.Name() == id.RoleName.Name() {
Expand All @@ -768,18 +781,25 @@ func ReadGrantPrivilegesToAccountRole(ctx context.Context, d *schema.ResourceDat
// grant_on is for future grants, granted_on is for current grants.
// They function the same way though in a test for matching the object type
if grantedOn == grant.GrantedOn || grantedOn == grant.GrantOn {
privileges = append(privileges, grant.Privilege)
actualPrivileges = append(actualPrivileges, grant.Privilege)
}
}
}

logging.DebugLogger.Printf("[DEBUG] Setting privileges: %v", privileges)
if err := d.Set("privileges", privileges); err != nil {
usageIndex := slices.IndexFunc(actualPrivileges, func(s string) bool { return strings.ToUpper(s) == sdk.AccountObjectPrivilegeUsage.String() })
if slices.ContainsFunc(expectedPrivileges, func(s string) bool {
return strings.ToUpper(s) == sdk.AccountObjectPrivilegeImportedPrivileges.String()
}) && usageIndex >= 0 {
actualPrivileges[usageIndex] = sdk.AccountObjectPrivilegeImportedPrivileges.String()
}

logging.DebugLogger.Printf("[DEBUG] Setting privileges: %v", actualPrivileges)
if err := d.Set("privileges", actualPrivileges); err != nil {
return diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "Error setting privileges for account role",
Detail: fmt.Sprintf("Id: %s\nPrivileges: %v\nError: %s", d.Id(), privileges, err.Error()),
Detail: fmt.Sprintf("Id: %s\nPrivileges: %v\nError: %s", d.Id(), actualPrivileges, err.Error()),
},
}
}
Expand Down
126 changes: 125 additions & 1 deletion pkg/resources/grant_privileges_to_account_role_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ package resources_test
import (
"context"
"database/sql"
"errors"
"fmt"
"log"
"regexp"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/hashicorp/terraform-plugin-testing/helper/acctest"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
Expand Down Expand Up @@ -821,6 +825,123 @@ func TestAcc_GrantPrivilegesToAccountRole_AlwaysApply(t *testing.T) {
})
}

func TestAcc_GrantPrivilegesToAccountRole_ImportedPrivileges(t *testing.T) {
sharedDatabaseName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
shareName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
roleName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
secondaryAccountName, err := getSecondaryAccountName(t)
require.NoError(t, err)
configVariables := config.Variables{
"role_name": config.StringVariable(roleName),
"shared_database_name": config.StringVariable(sharedDatabaseName),
"share_name": config.StringVariable(shareName),
"account_name": config.StringVariable(secondaryAccountName),
"privileges": config.ListVariable(
config.StringVariable(sdk.AccountObjectPrivilegeImportedPrivileges.String()),
),
}
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: func(state *terraform.State) error {
return errors.Join(
testAccCheckAccountRolePrivilegesRevoked(roleName)(state),
dropSharedDatabaseOnSecondaryAccount(t, sharedDatabaseName, shareName),
)
},
Steps: []resource.TestStep{
{
PreConfig: func() { assert.NoError(t, createSharedDatabaseOnSecondaryAccount(t, sharedDatabaseName, shareName)) },
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToAccountRole/ImportedPrivileges"),
ConfigVariables: configVariables,
ConfigPlanChecks: resource.ConfigPlanChecks{
PostApplyPostRefresh: []plancheck.PlanCheck{
plancheck.ExpectEmptyPlan(),
},
},
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "privileges.#", "1"),
resource.TestCheckResourceAttr(resourceName, "privileges.0", sdk.AccountObjectPrivilegeImportedPrivileges.String()),
),
},
{
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToAccountRole/ImportedPrivileges"),
ConfigVariables: configVariables,
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func getSecondaryAccountName(t *testing.T) (string, error) {
t.Helper()
config, err := sdk.ProfileConfig("secondary_test_account")
if err != nil {
t.Fatal(err)
}
client, err := sdk.NewClient(config)
if err != nil {
t.Fatal(err)
}
return client.ContextFunctions.CurrentAccount(context.Background())
}

func getAccountName(t *testing.T) (string, error) {
t.Helper()
client, err := sdk.NewDefaultClient()
if err != nil {
t.Fatal(err)
}
return client.ContextFunctions.CurrentAccount(context.Background())
}

func createSharedDatabaseOnSecondaryAccount(t *testing.T, databaseName string, shareName string) error {
t.Helper()
config, err := sdk.ProfileConfig("secondary_test_account")
if err != nil {
t.Fatal(err)
}
client, err := sdk.NewClient(config)
if err != nil {
t.Fatal(err)
}
ctx := context.Background()
accountName, err := getAccountName(t)
return errors.Join(
err,
client.Databases.Create(ctx, sdk.NewAccountObjectIdentifier(databaseName), &sdk.CreateDatabaseOptions{}),
client.Shares.Create(ctx, sdk.NewAccountObjectIdentifier(shareName), &sdk.CreateShareOptions{}),
client.Grants.GrantPrivilegeToShare(ctx, []sdk.ObjectPrivilege{sdk.ObjectPrivilegeReferenceUsage}, &sdk.ShareGrantOn{Database: sdk.NewAccountObjectIdentifier(databaseName)}, sdk.NewAccountObjectIdentifier(shareName)),
client.Shares.Alter(ctx, sdk.NewAccountObjectIdentifier(shareName), &sdk.AlterShareOptions{Set: &sdk.ShareSet{
Accounts: []sdk.AccountIdentifier{sdk.NewAccountIdentifierFromAccountLocator(accountName)},
}}),
)
}

func dropSharedDatabaseOnSecondaryAccount(t *testing.T, databaseName string, shareName string) error {
t.Helper()
config, err := sdk.ProfileConfig("secondary_test_account")
if err != nil {
t.Fatal(err)
}
client, err := sdk.NewClient(config)
if err != nil {
t.Fatal(err)
}
ctx := context.Background()
return errors.Join(
client.Shares.Drop(ctx, sdk.NewAccountObjectIdentifier(shareName)),
client.Databases.Drop(ctx, sdk.NewAccountObjectIdentifier(databaseName), &sdk.DropDatabaseOptions{}),
)
}

func createAccountRoleOutsideTerraform(t *testing.T, name string) {
t.Helper()
client, err := sdk.NewDefaultClient()
Expand All @@ -840,7 +961,7 @@ func testAccCheckAccountRolePrivilegesRevoked(name string) func(*terraform.State
client := sdk.NewClientFromDB(db)

defer func() {
err := client.Roles.Drop(context.Background(), sdk.NewDropRoleRequest(sdk.NewAccountObjectIdentifier(name)))
err := client.Roles.Drop(context.Background(), sdk.NewDropRoleRequest(sdk.NewAccountObjectIdentifier(name)).WithIfExists(true))
if err != nil {
log.Printf("failed to drop account role (%s), err = %s\n", name, err.Error())
}
Expand All @@ -859,6 +980,9 @@ func testAccCheckAccountRolePrivilegesRevoked(name string) func(*terraform.State
},
})
if err != nil {
if errors.Is(err, sdk.ErrObjectNotExistOrAuthorized) {
continue
}
return err
}
var grantedPrivileges []string
Expand Down
28 changes: 24 additions & 4 deletions pkg/resources/grant_privileges_to_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ import (
)

var grantPrivilegesToRoleSchema = map[string]*schema.Schema{
// According to docs https://docs.snowflake.com/en/user-guide/data-exchange-marketplace-privileges#usage-notes IMPORTED PRIVILEGES
// will be returned as USAGE in SHOW GRANTS command. In addition, USAGE itself is a valid privilege, but both cannot be set at the
// same time (IMPORTED PRIVILEGES can only be granted to the database created from SHARE and USAGE in every other case).
// To handle both cases, additional logic was added in read operation where IMPORTED PRIVILEGES is replaced with USAGE.
"privileges": {
Type: schema.TypeSet,
Optional: true,
Expand Down Expand Up @@ -834,14 +838,23 @@ func readRoleGrantPrivileges(ctx context.Context, client *sdk.Client, grantedOn
}

withGrantOption := d.Get("with_grant_option").(bool)
privileges := []string{}
roleName := d.Get("role_name").(string)

actualPrivileges := make([]string, 0)
expectedPrivileges := make([]string, 0)
expectedPrivileges = append(expectedPrivileges, id.Privileges...)

if slices.ContainsFunc(expectedPrivileges, func(s string) bool {
return strings.ToUpper(s) == sdk.AccountObjectPrivilegeImportedPrivileges.String()
}) {
expectedPrivileges = append(expectedPrivileges, sdk.AccountObjectPrivilegeUsage.String())
}

logging.DebugLogger.Printf("[DEBUG] Filtering grants to be set on account: count = %d", len(grants))
for _, grant := range grants {
// Only consider privileges that are already present in the ID so we
// don't delete privileges managed by other resources.
if !slices.Contains(id.Privileges, grant.Privilege) {
if !slices.Contains(expectedPrivileges, grant.Privilege) {
continue
}
if grant.GrantOption == withGrantOption && grant.GranteeName.Name() == sdk.NewAccountObjectIdentifier(roleName).Name() {
Expand All @@ -852,13 +865,20 @@ func readRoleGrantPrivileges(ctx context.Context, client *sdk.Client, grantedOn
}
// grant_on is for future grants, granted_on is for current grants. They function the same way though in a test for matching the object type
if grantedOn == grant.GrantedOn || grantedOn == grant.GrantOn {
privileges = append(privileges, grant.Privilege)
actualPrivileges = append(actualPrivileges, grant.Privilege)
}
}
}

usageIndex := slices.IndexFunc(actualPrivileges, func(s string) bool { return strings.ToUpper(s) == sdk.AccountObjectPrivilegeUsage.String() })
if slices.ContainsFunc(expectedPrivileges, func(s string) bool {
return strings.ToUpper(s) == sdk.AccountObjectPrivilegeImportedPrivileges.String()
}) && usageIndex >= 0 {
actualPrivileges[usageIndex] = sdk.AccountObjectPrivilegeImportedPrivileges.String()
}

logging.DebugLogger.Printf("[DEBUG] Setting privileges on account")
if err := d.Set("privileges", privileges); err != nil {
if err := d.Set("privileges", actualPrivileges); err != nil {
logging.DebugLogger.Printf("[DEBUG] Error setting privileges for account role: err = %v", err)
return fmt.Errorf("error setting privileges for account role: %w", err)
}
Expand Down
61 changes: 61 additions & 0 deletions pkg/resources/grant_privileges_to_role_acceptance_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
package resources_test

import (
"errors"
"fmt"
"regexp"
"strings"
"testing"

"github.com/hashicorp/terraform-plugin-testing/config"
"github.com/hashicorp/terraform-plugin-testing/terraform"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/hashicorp/terraform-plugin-testing/plancheck"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
Expand Down Expand Up @@ -980,3 +986,58 @@ resource "snowflake_grant_privileges_to_role" "test_invalidation" {
},
})
}

func TestAcc_GrantPrivilegesToRole_ImportedPrivileges(t *testing.T) {
sharedDatabaseName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
shareName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
roleName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
secondaryAccountName, err := getSecondaryAccountName(t)
require.NoError(t, err)
configVariables := config.Variables{
"role_name": config.StringVariable(roleName),
"shared_database_name": config.StringVariable(sharedDatabaseName),
"share_name": config.StringVariable(shareName),
"account_name": config.StringVariable(secondaryAccountName),
"privileges": config.ListVariable(
config.StringVariable(sdk.AccountObjectPrivilegeImportedPrivileges.String()),
),
}
resourceName := "snowflake_grant_privileges_to_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: func(state *terraform.State) error {
return errors.Join(
testAccCheckAccountRolePrivilegesRevoked(roleName)(state),
dropSharedDatabaseOnSecondaryAccount(t, sharedDatabaseName, shareName),
)
},
Steps: []resource.TestStep{
{
PreConfig: func() { assert.NoError(t, createSharedDatabaseOnSecondaryAccount(t, sharedDatabaseName, shareName)) },
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToRole/ImportedPrivileges"),
ConfigVariables: configVariables,
ConfigPlanChecks: resource.ConfigPlanChecks{
PostApplyPostRefresh: []plancheck.PlanCheck{
plancheck.ExpectEmptyPlan(),
},
},
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "privileges.#", "1"),
resource.TestCheckResourceAttr(resourceName, "privileges.0", sdk.AccountObjectPrivilegeImportedPrivileges.String()),
),
},
{
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToRole/ImportedPrivileges"),
ConfigVariables: configVariables,
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
resource "snowflake_database" "test" {
name = var.shared_database_name
data_retention_time_in_days = 0
from_share = {
provider = var.account_name
share = var.share_name
}
}

resource "snowflake_role" "test" {
name = var.role_name
}

resource "snowflake_grant_privileges_to_account_role" "test" {
depends_on = [snowflake_database.test, snowflake_role.test]
account_role_name = "\"${var.role_name}\""
privileges = var.privileges
on_account_object {
object_type = "DATABASE"
object_name = "\"${var.shared_database_name}\""
}
}
Loading

0 comments on commit eb20051

Please sign in to comment.