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

.gitignore

@@ -24,4 +24,5 @@ bin
 dist
 
 # JetBrains
-.idea/
+.idea/
+*.iml

README.md

@@ -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
 

pkg/resources/share.go

@@ -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 {

pkg/resources/share_test.go

@@ -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()