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

OWNERSHIP can only be transferred error on destruction of snowflake_grant_privileges_to_role resource #2084

Closed
liamjamesfoley opened this issue Sep 29, 2023 · 4 comments
Labels
bug Used to mark issues with provider's incorrect behavior category:grants

Comments

@liamjamesfoley
Copy link
Contributor

liamjamesfoley commented Sep 29, 2023

Provider Version
0.71

Terraform Version
1.5.2

Describe the bug
https://docs.snowflake.com/en/sql-reference/sql/grant-ownership
Destruction of snowflake_grant_privileges_to_role produces that a query that will always fail.

image

Expected behavior
This is happening in a managed access schema module I'm working on, so TBH, I don't care as long as the resource destruction happens without using terraform state rm b/c I'm also destroying the schema, but ownership should probably go to ACCOUNTADMIN?

Since snowflake_schema_grant is being deprecated the new resource should have something similar to revert_ownership_to_role_name.
https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/pkg/resources/schema_grant.go#L106-L114

Code samples and commands

Resource:

resource "snowflake_grant_privileges_to_role" "owner_schema_grant" {
  # `GRANT OWNERSHIP ON SCHEMA ${schema_identifier} TO ROLE ${owner_role};`,
  role_name  = snowflake_role.owner_role.name
  privileges = ["OWNERSHIP"]
  on_schema {
    schema_name = "${var.db_name}.${snowflake_schema.schema.name}"
  }
  with_grant_option = true
}
@liamjamesfoley liamjamesfoley added the bug Used to mark issues with provider's incorrect behavior label Sep 29, 2023
@imre-kerr-sb1
Copy link

We also have issues with ownership grants. For now we are using the following workaround:

data "snowflake_current_role" "current" {}

resource "snowsql_exec" "owner_schema_grant" {
  name = "owner_schema_grant"

  create {
    statements           = "GRANT OWNERSHIP ON SCHEMA ${schema_identifier} TO ROLE ${snowflake_role.owner_role.name};"
    number_of_statements = 1
  }

  read {
    statements           = "SHOW GRANTS ON SCHEMA ${schema_identifier};"
    number_of_statements = 1
  }

  delete {
    statements           = "GRANT OWNERSHIP ON SCHEMA ${schema_identifier} TO ROLE ${data.snowflake_current_role.current.name};"
    number_of_statements = 1
  }
}

@entechlog
Copy link

entechlog commented Oct 6, 2023

Having the same error on schema and warehouse ownership changes. When we try to change the owner using snowflake_grant_privileges_to_role resource the plan runs fine but apply fails with below error.

Error: error revoking privileges from account role: exactly one of AllPrivileges, GlobalPrivileges, AccountObjectPrivileges, SchemaPrivileges, or SchemaObjectPrivileges must be set

Workaround
Step 1: Change the owner manually using sql, This is just setting a default role
GRANT OWNERSHIP ON WAREHOUSE "ALL_MAGIC_QUERY_WH_XS" TO ROLE "ENTECHLOG_MAGIC_ROLE" REVOKE CURRENT GRANTS;

Step 2: Remove the problem state from terrafrom state
terraform state rm module.dbt_wh_xs.snowflake_grant_privileges_to_role.ownership_warehouse_grant["sysadmin_role"]

Step 3: Rerun terrraform plan and apply to make the ownership change

@sfc-gh-jcieslak
Copy link
Collaborator

Hey @liamjamesfoley
I think we can close this one. In the newer, non-deprecated snowflake_grant_privileges_to_account_role resource the OWNERSHIP privilege is not allowed (because ownership privilege is more complex and should be managed by different resources). Luckily I started to work on this resource and in one or two next minor versions should be usable. Here's a pr to follow (#2604) it also lists other issues connected to OWNERSHIP privilege (like this one), so you can follow the progress in them. This one was specific to snowflake_grant_privileges_to_role which is now deprecated and in the newer resource OWNERSHIP is not allowed.

sfc-gh-jcieslak added a commit that referenced this issue Mar 14, 2024
The first part of the implementation of the `snowflake_grant_ownership`
resource. This is a "basic" version of this resource providing baseline
functionalities needed to transfer ownership in Terraform. In the next
pull request, I'll add all of the edge cases we have to cover (most of
them are described
[here](https://docs.snowflake.com/en/sql-reference/sql/grant-ownership#usage-notes)).

Changes made:
- Created a new `snowflake_grant_ownership` resource with CRUD
operations implemented (still there are TODOs left for discussion)
- Added examples and documentation needed for the resource and its
identifier

Things to do before the merge:
- remove `snowflake_grant_ownership` from the provider.go

TODO in the next pr(s):
- Add deprecation messages to old grant resources specifically made for
granting ownership
- Add edge cases and test them (and if needed describe them in the
documentation and add examples)
- Add `setId("")` in read and forcefully grant ownership in Create
operation
- Referring to
[comment](#2604 (comment)),
test different cases where the Delete operation may struggle with
- Test outside of Terraform interactions to see how it behaves in
different situations

## Test Plan
* [x] acceptance tests
* [x] unit tests for the resource identifier conversions from/to String
representation
* [x] unit tests for the helper functions needed by resource CRUD
operations

## References
* [GRANT
OWNERSHIP](https://docs.snowflake.com/en/sql-reference/sql/grant-ownership)

## Mentioned in
A list of issues requesting this resource (a big probability there's
more); notify after part 2 will be done.
- #2549
- #2199
- #2084
- #1942
- #1875
sfc-gh-swinkler pushed a commit that referenced this issue Mar 19, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.87.3-pre](v0.87.2...v0.87.3-pre)
(2024-03-18)


### 🎉 **What's new:**

* Add snowflake grant ownership resource
([#2604](#2604))
([bfadd24](bfadd24)),
closes
[#2549](#2549)
[#2199](#2199)
[#2084](#2084)
[#1942](#1942)
[#1875](#1875)


### 🔧 **Misc**

* Fix env variables for tests
([#2603](#2603))
([8bc2437](8bc2437))
* release 0.87.3-pre
([a2be7b9](a2be7b9))


### 🐛 **Bug fixes:**

* alter table column data type
([#2607](#2607))
([538b6dc](538b6dc))
* cgo goreleaser alt solution
([#2613](#2613))
([5d31856](5d31856))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>
sfc-gh-jcieslak added a commit that referenced this issue Apr 3, 2024
A follow-up for #2604. 

Done in this pr:
- Add setId("") in Read (when ownership is not found on the target
object) and forcefully grant ownership in Create (this was already
present, but added test cases for it).
- Edge cases
- Granting `ON PIPE` and `ON ALL PIPES` is handled (pipes are paused
before and resumed after ownership transfer)

Full list of things that still need to be done:
- Deprecation messages
- More documentation (explain how grant_ownership resource handles edge
cases) and examples that would show simple usage, edge cases, cases
where the resource may cause trouble
- Referring to
#2604 (comment),
test different cases where the Delete operation may struggle with
- Test outside of Terraform interactions to see how it behaves in
different situations
- A test where used role is not privileged enough to transfer ownership
- Also cases within Terraform to see how grant_ownership will act with
other grant resources within certain configurations
- Edge cases
  - Granting `ON TASK`
  - Use `VIEW` when granting on `MATERIALIZED VIEW`
  - Granting `ON EXTERNAL TABLES`

## References
[GRANT
OWNERSHIP](https://docs.snowflake.com/en/sql-reference/sql/grant-ownership)

## Mentioned in
A list of issues requesting this resource: #2549 #2199 #2084 #1942 #1875
sfc-gh-jcieslak added a commit that referenced this issue Apr 8, 2024
A follow-up for
#2604.

Done in this pr:
- All of the edge cases handled and tested (except of tasks that are
done in the separate pr):
  - Materialized views (already handled by Snowflake no changes needed)
  - RBAC hierarchy (test case added)
- Delete dependent resource (role or granted object) and remove grant
resource from the state (test case added)

Won't do:
- External tables (cannot handle this edge case, because we have to know
the auto_refresh state of the external table; it's not retrievable by
SHOW or DESC commands. It will be still possible to grant ownership of
the external table, but there may be additional manual work to do
afterward. Everything is documented.)

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested
-->
* [x] acceptance tests that show how the resource is handling certain
edge cases + RBAC use case

## References
[GRANT
OWNERSHIP](https://docs.snowflake.com/en/sql-reference/sql/grant-ownership)

## Mentioned in
A list of issues requesting this resource:
#2549
#2199
#2084
#1942
#1875
@sfc-gh-jcieslak
Copy link
Collaborator

Hey 👋
Closing, as the issue was about the deprecated/not allowed functionality. Recently, we released a new grant resource which is capable of granting ownership. Please, give it a try. If there will be any issues with it, create another GitHub issue. Also, please check our technical documentation section where you can find a migration guide that can help you with upgrading to the latest grant resources and our newly added design decision doc (regarding new grant resources).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to mark issues with provider's incorrect behavior category:grants
Projects
None yet
Development

No branches or pull requests

4 participants