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

feat: create snowflake_role_ownership_grant resource #917

Merged
merged 19 commits into from
Mar 21, 2022
Merged

feat: create snowflake_role_ownership_grant resource #917

merged 19 commits into from
Mar 21, 2022

Conversation

aidanmelen
Copy link
Contributor

@aidanmelen aidanmelen commented Mar 12, 2022

Create a snowflake_role_ownership_grant resource.

Test Plan

  • unit tests
  • acceptance tests
  • tested in my snowflake account
  • tested read/update by manually deleting the to_role after creation, the ownership changed back to ACCOUNTADMIN. the following apply rebuilt the role and re-granted role ownership.

References

Example

I created a new example under examples/resources/snowflake_role_ownership_grant

resource "snowflake_role" "role" {
  name    = "rking_test_role"
  comment = "for testing"
}

resource "snowflake_role" "other_role" {
  name = "rking_test_role2"
}

# ensure the Terraform user inherits ownership privileges for the rking_test_role role
# otherwise Terraform will fail to destroy the rking_test_role2 role due to insufficient privileges
resource "snowflake_role_grants" "grants" {
  role_name = snowflake_role.role.name

  roles = [
    "ACCOUNTADMIN",
  ]
}

resource "snowflake_role_ownership_grant" "grant" {
  on_role_name = snowflake_role.role.name
  to_role_name = snowflake_role.other_role.name
  current_grants = "COPY"
}

resulting role ownership:

Screen Shot 2022-03-14 at 10 26 38 PM

@aidanmelen aidanmelen changed the title create snowflake_role_ownership_grants resource create snowflake_role_ownership_grant resource Mar 15, 2022
@aidanmelen
Copy link
Contributor Author

@alldoami I think this is ready for peer review.

@alldoami alldoami changed the title create snowflake_role_ownership_grant resource feat: create snowflake_role_ownership_grant resource Mar 15, 2022
@alldoami
Copy link
Contributor

If we wanted to use this GRANT OWNERSHIP query on other objects ( { ROLE | USER | WAREHOUSE | INTEGRATION | NETWORK POLICY | SESSION POLICY | DATABASE | SCHEMA | TABLE | VIEW | STAGE | FILE FORMAT | STREAM | TASK | MASKING POLICY | ROW ACCESS POLICY | PIPE | FUNCTION | PROCEDURE | SEQUENCE }), do you think we'd need to define new resources to create that? Or could we somehow make this a more generic snowflake_ownership_grant resource that isn't just for role ownership?

@aidanmelen
Copy link
Contributor Author

aidanmelen commented Mar 15, 2022

If we wanted to use this GRANT OWNERSHIP query on other objects ( { ROLE | USER | WAREHOUSE | INTEGRATION | NETWORK POLICY | SESSION POLICY | DATABASE | SCHEMA | TABLE | VIEW | STAGE | FILE FORMAT | STREAM | TASK | MASKING POLICY | ROW ACCESS POLICY | PIPE | FUNCTION | PROCEDURE | SEQUENCE }), do you think we'd need to define new resources to create that? Or could we somehow make this a more generic snowflake_ownership_grant resource that isn't just for role ownership?

@alldoami I considered adding multiple types but that implementation is very nuanced so I opted to only support on role ownership initially since that is what the #460 called for.

That being said, I wrote this resource so that it could be expanded to support multiple ON object types in the future if that made sense.

Do you want a single resource handling GRANT OWNERSHIP? If so, that could look like this

resource "snowflake_ownership_grant" "role" {
  type      = "ROLE"
  name      = snowflake_role.role.name
  role_name = snowflake_role.other_role.name
}

resource "snowflake_ownership_grant" "user" {
  type      = "USER"
  name      = snowflake_user.user.name
  role_name = snowflake_role.other_role.name
}

However, we won't get much mileage out of the added complexity of this "generic" resource implementation because it would only support ROLE and USER types. Additional objects would collide with functionality offered by other grant resources. For example:

resource "snowflake_ownership_grant" "warehouse" {
  type      = "WAREHOUSE"
  name      = "wh"
  role_name = "role1"
}

would be redundant to the existing snowflake_warehouse_grant resource

resource "snowflake_warehouse_grant" "grant" {
  warehouse_name = "wh"
  privilege      = "OWNERSHIP"

  roles = [
    "role1",
  ]
}

Given the considerations above, I think it is best to have two special resources. The snowflake_role_ownership_grant that I am proposing in this PR

# GRANT OWNERSHIP ON ROLE "role" TO ROLE "role1" COPY CURRENT GRANTS 

resource "snowflake_role_ownership_grant" "grant" {
  on_role_name   = "role"
  to_role_name   = "role1",
  current_grants = "COPY"
}

# not to be confused with the existing `snowflake_role_grants` resource which handles `GRANT ROLE`

and a snowflake_user_ownership_grant that I could also add to this PR.

# GRANT OWNERSHIP ON USER "user" TO ROLE "role1" COPY CURRENT GRANTS 

resource "snowflake_user_ownership_grant" "grant" {
  on_user_name   = "user"
  to_role_name   = "role1",
  current_grants = "COPY"
}

Both special resources could share the common pkg/snowflake/snowflake_ownership_grant.go code.

@alldoami
Copy link
Contributor

Got it @aidanmelen, this makes sense. Thank you for contributing!

@alldoami
Copy link
Contributor

/ok-to-test sha=5c1de73

@github-actions
Copy link

Integration tests success for 5c1de73

@alldoami
Copy link
Contributor

can you run make docs?

@aidanmelen
Copy link
Contributor Author

aidanmelen commented Mar 16, 2022

can you run make docs?

I keep trying to add an example to the docs after it generates, but the make docs command eally doesn't like that... I will remove the example and re-run make docs

@aidanmelen
Copy link
Contributor Author

@alldoami docs should be passing now

@aidanmelen
Copy link
Contributor Author

Got it @aidanmelen, this makes sense. Thank you for contributing!

That was fun! Thanks for considering my contribution!

@aidanmelen
Copy link
Contributor Author

I will plan on making another PR for the snowflake_user_ownership_grant resource in the near future.

@alldoami
Copy link
Contributor

/ok-to-test sha=04b1260

@github-actions
Copy link

Integration tests success for 04b1260

@alldoami
Copy link
Contributor

@aidanmelen GHA broke today and I think we need to rekick off the GHA jobs, do you think you can commit an empty commit to your branch?

@aidanmelen
Copy link
Contributor Author

@alldoami done

@alldoami
Copy link
Contributor

/ok-to-test sha=d734035

@github-actions
Copy link

Integration tests success for d734035

@alldoami
Copy link
Contributor

/ok-to-test sha=274b54b

@github-actions
Copy link

Integration tests failure for 274b54b

@aidanmelen
Copy link
Contributor Author

aidanmelen commented Mar 19, 2022

Integration tests failure for 274b54b

@alldoami looks like the integration test failure was unrelated to my PR:

Screen Shot 2022-03-18 at 8 26 20 PM

all my tests passed:

Screen Shot 2022-03-18 at 8 28 57 PM

@alldoami
Copy link
Contributor

/ok-to-test sha=65f7a5b

@github-actions
Copy link

Integration tests success for 65f7a5b

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.

2 participants