Skip to content

Commit

Permalink
fix: grants on external volumes (#2538)
Browse files Browse the repository at this point in the history
Fixes:
#2533
Terraform couldn't read privileges for External volumes, because
Snowflake returns a shorter `VOLUME` name where we expect `EXTERNAL
VOLUME` to be returned. The proposed fix relies on replacing `EXTERNAL
VOLUME` with `VOLUME` in the prepare read request function so that the
Read operation will be untouched and will work for external volumes.

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested
-->
* [x] acceptance tests to show the fix works (didn't pass without the
fix)
<!-- add more below if you think they are relevant -->

## Other
Wrote to the docs team to add this case to the SHOW GRANTS page.

**Update**: added missing privileges (CREATE MODEL needed for
#2563)
  • Loading branch information
sfc-gh-jcieslak committed Feb 28, 2024
1 parent bfe488a commit 1de9a29
Show file tree
Hide file tree
Showing 5 changed files with 239 additions and 124 deletions.
71 changes: 71 additions & 0 deletions pkg/resources/grant_privileges_to_account_role_acceptance_test.go
Expand Up @@ -964,6 +964,52 @@ func TestAcc_GrantPrivilegesToAccountRole_MultiplePartsInRoleName(t *testing.T)
})
}

// proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2533 is fixed
func TestAcc_GrantPrivilegesToAccountRole_OnExternalVolume(t *testing.T) {
name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName()
externalVolumeName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
configVariables := config.Variables{
"name": config.StringVariable(roleName),
"external_volume": config.StringVariable(externalVolumeName),
"privileges": config.ListVariable(
config.StringVariable(string(sdk.AccountObjectPrivilegeUsage)),
),
"with_grant_option": config.BoolVariable(true),
}
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: testAccCheckAccountRolePrivilegesRevoked(name),
Steps: []resource.TestStep{
{
PreConfig: func() {
createAccountRoleOutsideTerraform(t, name)
cleanupExternalVolume := createExternalVolume(t, externalVolumeName)
t.Cleanup(cleanupExternalVolume)
},
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToAccountRole/OnExternalVolume"),
ConfigVariables: configVariables,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "account_role_name", roleName),
resource.TestCheckResourceAttr(resourceName, "privileges.#", "1"),
resource.TestCheckResourceAttr(resourceName, "privileges.0", string(sdk.AccountObjectPrivilegeUsage)),
resource.TestCheckResourceAttr(resourceName, "with_grant_option", "true"),
resource.TestCheckResourceAttr(resourceName, "on_account_object.#", "1"),
resource.TestCheckResourceAttr(resourceName, "on_account_object.0.object_type", "EXTERNAL VOLUME"),
resource.TestCheckResourceAttr(resourceName, "on_account_object.0.object_name", externalVolumeName),
resource.TestCheckResourceAttr(resourceName, "id", fmt.Sprintf("%s|true|false|USAGE|OnAccountObject|EXTERNAL VOLUME|\"%s\"", roleName, externalVolumeName)),
),
},
},
})
}

func getSecondaryAccountName(t *testing.T) (string, error) {
t.Helper()
config, err := sdk.ProfileConfig(testprofiles.Secondary)
Expand Down Expand Up @@ -1100,3 +1146,28 @@ func queriedAccountRolePrivilegesContainAtLeast(roleName sdk.AccountObjectIdenti
})
}, roleName, privileges...)
}

func createExternalVolume(t *testing.T, externalVolumeName string) func() {
t.Helper()

client, err := sdk.NewDefaultClient()
require.NoError(t, err)

ctx := context.Background()
_, err = client.ExecForTests(ctx, fmt.Sprintf(`create external volume "%s" storage_locations = (
(
name = 'test'
storage_provider = 's3'
storage_base_url = 's3://my_example_bucket/'
storage_aws_role_arn = 'arn:aws:iam::123456789012:role/myrole'
encryption=(type='aws_sse_kms' kms_key_id='1234abcd-12ab-34cd-56ef-1234567890ab')
)
)
`, externalVolumeName))
require.NoError(t, err)

return func() {
_, err = client.ExecForTests(ctx, fmt.Sprintf(`drop external volume "%s"`, externalVolumeName))
require.NoError(t, err)
}
}
@@ -0,0 +1,9 @@
resource "snowflake_grant_privileges_to_account_role" "test" {
account_role_name = var.name
privileges = var.privileges
with_grant_option = var.with_grant_option
on_account_object {
object_type = "EXTERNAL VOLUME"
object_name = var.external_volume
}
}
@@ -0,0 +1,15 @@
variable "name" {
type = string
}

variable "external_volume" {
type = string
}

variable "privileges" {
type = list(string)
}

variable "with_grant_option" {
type = bool
}
7 changes: 7 additions & 0 deletions pkg/sdk/grants.go
Expand Up @@ -242,11 +242,18 @@ func (row grantRow) convert() *Grant {
if row.GrantedOn != "" {
grantedOn = ObjectType(strings.ReplaceAll(row.GrantedOn, "_", " "))
}
if row.GrantedOn == "VOLUME" {
grantedOn = ObjectTypeExternalVolume
}

var grantOn ObjectType
// true for future grants
if row.GrantOn != "" {
grantOn = ObjectType(strings.ReplaceAll(row.GrantOn, "_", " "))
}
if row.GrantOn == "VOLUME" {
grantOn = ObjectTypeExternalVolume
}

return &Grant{
CreatedOn: row.CreatedOn,
Expand Down

0 comments on commit 1de9a29

Please sign in to comment.