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: with_grant_option diff drift #2608

Merged
merged 6 commits into from
Apr 3, 2024

Conversation

sfc-gh-jcieslak
Copy link
Collaborator

@sfc-gh-jcieslak sfc-gh-jcieslak commented Mar 8, 2024

Fixes: #2459

The issue was related to the fact that the Read operation is only concerned about privileges (with_grant_option is also taken into consideration, but it's never set). Given this configuration:

resource "snowflake_grant_privileges_to_account_role" "test" {
  account_role_name = "TEST_ROLE"
  privileges = ["TRUNCATE"]
  on_schema_object {
    object_type = "TABLE"
    object_name = "TEST_DATABASE.TEST_SCHEMA.TEST_TABLE"
  }
  with_grant_option = true
}

after apply we run the following commands by hand

revoke truncate on table test_table from role test_role;
grant truncate on table test_table to role test_role; -- notice we don't add "with grant option" which our resource should detect

Now, when we run plan or apply our resource is seeing a drift (the "TRUNCATE" privilege is not present, because with_grant_option is not matching) and tries to run the Update operation (unsuccessfully; 1. because of the "sdk.GrantPrivOptions" not set 2. because of the incorrect logic). When there're already existing grants there are two ways to update with_grant_option which depends on what is set in Snowflake. It's better to show it with SQLs, so:

-- imagine this is ran by Snowflake Terraform Plugin (with_grant_option is set to true in the config)
grant truncate on table test_table to role test_role with grant option;

-- this is ran by hand in the worksheet
revoke truncate on table test_table from role test_role;
grant truncate on table test_table to role test_role; 

-- now update tries to run the following
grant truncate on table test_table to role test_role with grant option; -- this will successfully update with_grant_option to 'true'
-- imagine this is ran by Snowflake Terraform Plugin (with_grant_option is set to false in the config)
grant truncate on table test_table to role test_role;

-- this is ran by hand in the worksheet
revoke truncate on table test_table from role test_role;
grant truncate on table test_table to role test_role with grant option; 

-- now update tries to run the following
grant truncate on table test_table to role test_role; -- this won't update the with_grant_option to false because Snowflake is not updating the value when the option is already set to true (you have to revoke it first)

The fix I opted to is to:

  • when with_grant_option is set to true in the config
    • proceed as it was (but now set option struct correctly with with_grant_option set to true)
  • when with_grant_option is set to false in the config
    • firstly revoke privileges we would like to add (just in case this issue happens; it won't fail even if the grant does not exist)
    • then proceed as it was (grant privileges we would like to add)

todo other grant privileges to database role

Test Plan

  • Acceptance tests that prove the issue has been fixed for every privilege-granting resource

Copy link

github-actions bot commented Mar 8, 2024

Integration tests success for ec831cd559ecc7343ca6fccb261eda704e2d6e34

@sfc-gh-jcieslak sfc-gh-jcieslak marked this pull request as ready for review March 22, 2024 13:17
@sfc-gh-jcieslak sfc-gh-jcieslak force-pushed the fix-with-grant-option-diff-drift branch from ea19b61 to 4a80c90 Compare March 22, 2024 13:19
Copy link

Integration tests failure for ea19b61e5dc85815508093aec7b88c83332891e9

Copy link

Integration tests failure for 4a80c90aa42f732b66a112dd6424f455f0dfe563

@sfc-gh-jcieslak sfc-gh-jcieslak force-pushed the fix-with-grant-option-diff-drift branch from 4a80c90 to 363136a Compare March 28, 2024 07:47
Copy link

Integration tests failure for 363136a2ad2ce7155e70b035b33ef5cba2116095

pkg/resources/grant_privileges_to_account_role.go Outdated Show resolved Hide resolved
# Conflicts:
#	pkg/resources/grant_privileges_to_account_role_acceptance_test.go
#	pkg/resources/grant_privileges_to_database_role_acceptance_test.go
Copy link

Integration tests failure for e26a4f9ff207a02064b4b538c9c6e5555f4eaf51

Copy link

Integration tests failure for 7dff96c2a42fb9e8aa4426381c69c01fee4fe819

Copy link

github-actions bot commented Apr 3, 2024

Integration tests failure for 3fc15c3176df920989b3f6f876aff899f92be2c8

@sfc-gh-jcieslak sfc-gh-jcieslak merged commit f0018c6 into main Apr 3, 2024
8 of 9 checks passed
@sfc-gh-jcieslak sfc-gh-jcieslak deleted the fix-with-grant-option-diff-drift branch April 3, 2024 13:15
sfc-gh-jcieslak pushed a commit that referenced this pull request Apr 9, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.88.0](v0.87.3-pre...v0.88.0)
(2024-04-09)


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

* Fix issues 2651 2656
([#2659](#2659))
([7fa09cc](7fa09cc))
* Grant ownership follow up
([#2628](#2628))
([d467e5b](d467e5b))
* grant ownership follow-up
([#2658](#2658))
([bfa2317](bfa2317))
* grant ownership on tasks
([#2684](#2684))
([2ba7889](2ba7889))
* Introduce shared function to suspend task roots
([#2650](#2650))
([d684b5d](d684b5d))
* Redesign snowflake_grants datasource
([#2667](#2667))
([918873d](918873d))
* user password policy attachment
([#2627](#2627))
([382e49d](382e49d))


### 🔧 **Misc**

* release 0.88.0
([02d60e0](02d60e0))
* Update grant examples in all resources
([#2660](#2660))
([b542b69](b542b69))


### 🐛 **Bug fixes:**

* Adjust dynamic tables after BCR-1543
([#2664](#2664))
([cf32ceb](cf32ceb))
* Fix handling secure views read and import
([#2655](#2655))
([3c3ede6](3c3ede6))
* Fix issues 2606 2642 2652 2653
([#2654](#2654))
([4a73721](4a73721))
* Fix release workflow
([#2639](#2639))
([dfd07e9](dfd07e9))
* Generate docs for dynamic table
([#2666](#2666))
([16c75b0](16c75b0))
* grant read operation
([#2665](#2665))
([0b947a5](0b947a5))
* migration guide for databases created from share
([#2637](#2637))
([f9651bc](f9651bc))
* with_grant_option diff drift
([#2608](#2608))
([f0018c6](f0018c6))

---
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

grant_privileges_to_role doesn't recognize changes to with_grant_option that were made outside of Terraform
2 participants