Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Share resource to use REFERENCE_USAGE instead of USAGE #762

Merged
merged 4 commits into from
Nov 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@ bin
dist

# JetBrains
.idea/
.idea/
*.iml
7 changes: 3 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,17 @@ To run all tests, including the acceptance tests, run `make test-acceptance`.

Our CI jobs run the full acceptence test suite, which involves creating and destroying resources in a live snowflake account. Github Actions is configured with environment variables to authenticate to our test snowflake account. For security reasons, those variables are not available to forks of this repo.

If you are making a PR from a forked repo, you can create a new Snowflake trial account and set up Travis to build it by setting these environement variables:
If you are making a PR from a forked repo, you can create a new Snowflake Enterprise trial account and set up Travis to build it by setting these environment variables:

* `SNOWFLAKE_ACCOUNT` - The account name
* `SNOWFLAKE_USER` - A snowflake user for running tests.
* `SNOWFLAKE_PASSWORD` - Password for that user.
* `SNOWFLAKE_ROLE` - Needs to be ACCOUNTADMIN or similar.
* `SNOWFLAKE_REGION` - Default is us-west-2, set this if your snowflake account is in a different region.

If you are using the Standard Snowflake plan, it's recommended you also set up the following environment variables to skip tests for features not enabled for it:
You will also need to generate a Github API token and add the secret:

* `SKIP_DATABASE_TESTS` - to skip tests with retention time larger than 1 day
* `SKIP_WAREHOUSE_TESTS` - to skip tests with multi warehouses
* `REVIEWDOG_GITHUB_API_TOKEN` - A token for reviewdog to use to access your github account with privileges to read/write discussion.

## Releasing

Expand Down
24 changes: 22 additions & 2 deletions pkg/resources/share.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,21 +100,41 @@ func setAccounts(d *schema.ResourceData, meta interface{}) error {

// 2. Create temporary DB grant to the share
tempDBGrant := snowflake.DatabaseGrant(tempName)
err = snowflake.Exec(db, tempDBGrant.Share(name).Grant("USAGE", false))

// USAGE can only be granted to one database - granting USAGE on the temp db here
// conflicts (and errors) with having a database already shared (i.e. when you
// already have a share and are just adding or removing accounts). Instead, use
// REFERENCE_USAGE which is intended for multi-database sharing as per Snowflake
// documentation here:
// https://docs.snowflake.com/en/sql-reference/sql/grant-privilege-share.html#usage-notes
// Note however that USAGE will be granted automatically on the temp db for the
// case where the main db doesn't already exist, so it will need to be revoked
// before deleting the temp db. Where USAGE hasn't been already granted it is not
// an error to revoke it, so it's ok to just do the revoke every time.
err = snowflake.Exec(db, tempDBGrant.Share(name).Grant("REFERENCE_USAGE", false))
if err != nil {
return errors.Wrapf(err, "error creating temporary DB grant %v", tempName)
return errors.Wrapf(err, "error creating temporary DB REFERENCE_USAGE grant %v", tempName)
}

// 3. Add the accounts to the share
q := fmt.Sprintf(`ALTER SHARE "%v" SET ACCOUNTS=%v`, name, strings.Join(accs, ","))
err = snowflake.Exec(db, q)
if err != nil {
return errors.Wrapf(err, "error adding accounts to share %v", name)
}

// 4. Revoke temporary DB grant to the share
err = snowflake.ExecMulti(db, tempDBGrant.Share(name).Revoke("REFERENCE_USAGE"))
if err != nil {
return errors.Wrapf(err, "error revoking temporary DB REFERENCE_USAGE grant %v", tempName)
}

// revoke the maybe automatically granted USAGE privilege.
err = snowflake.ExecMulti(db, tempDBGrant.Share(name).Revoke("USAGE"))
if err != nil {
return errors.Wrapf(err, "error revoking temporary DB grant %v", tempName)
}

// 5. Remove the temporary DB
err = snowflake.Exec(db, tempDB.Drop())
if err != nil {
Expand Down
6 changes: 5 additions & 1 deletion pkg/resources/share_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,13 @@ func TestShareCreate(t *testing.T) {
WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
mock.ExpectExec(`^CREATE SHARE "test-share" COMMENT='great comment'$`).WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectExec(`^CREATE DATABASE "TEMP_test-share_\d*"$`).WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectExec(`^GRANT USAGE ON DATABASE "TEMP_test-share_\d*" TO SHARE "test-share"$`).WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectExec(`^GRANT REFERENCE_USAGE ON DATABASE "TEMP_test-share_\d*" TO SHARE "test-share"$`).WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectExec(`^ALTER SHARE "test-share" SET ACCOUNTS=bob123,sue456$`).WillReturnResult(sqlmock.NewResult(1, 1))

mock.ExpectBegin()
mock.ExpectExec(`^REVOKE REFERENCE_USAGE ON DATABASE "TEMP_test-share_\d*" FROM SHARE "test-share"$`).WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectCommit()

mock.ExpectBegin()
mock.ExpectExec(`^REVOKE USAGE ON DATABASE "TEMP_test-share_\d*" FROM SHARE "test-share"$`).WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectCommit()
Expand Down