REVOKE runs before DROP ROLE when role is deleted

[dlouseiro]

Provider Version

0.25.32

Terraform Version

v1.1.5

Describe the bug

When a snowflake_role resource and all grants associated with it are removed, terraform fails with SQL compilation error: Role '<role_name>' does not exist or not authorized.

This happens because the DROP ROLE command runs before the REVOKE command.

Expected behavior

Either the REVOKE command should run before the DROP ROLE or the REVOKE should be ignored if the role was already dropped.

Code samples and commands

Example of the initial state (already applied in the database):

resource "snowflake_role" "x" {
  name    = "X"
}

resource "snowflake_role" "y" {
  name    = "Y"
}

resource "snowflake_role" "z" {
  name    = "Z"
}

resource "snowflake_role_grants" "x" {
  role_name = snowflake_role.x.name

  roles = [
    snowflake_role.y.name,
    snowflake_role.z.name
  ]
}

Example of changes made (y role and all its dependent grants deleted):

resource "snowflake_role" "x" {
  name    = "X"
}

resource "snowflake_role" "z" {
  name    = "Z"
}

resource "snowflake_role_grants" "x" {
  role_name = snowflake_role.x.name

  roles = [
    snowflake_role.z.name
  ]
}

Code executed in the database (in this order):

DROP ROLE y;
REVOKE ROLE x FROM ROLE y;

[ricardobf]

Any updates on this one?
I have the same issue.

[danu165]

Have you tried adding a depends_on argument to role_grants to make it depend on the roles?

[ricardobf]

I did, still doesn't worked.

[bfalk-phr]

Also having this same issue. REVOKE is being called after DROP

[remymaillot]

Same issue but on a case of a role name change, here is a print of snowflake queries.

I will say pattern error is same, revoke is after drop or rename.

[hanss0n]

Any update on when this will be fixed?

[Kaniel12]

I was having this issue when I renamed roles, the way I solved the issue was to include a lifecycle block in all my role_grant blocks to force resource re-creation upon change in roles. For more on lifecycles see https://www.terraform.io/language/meta-arguments/lifecycle.

This will not work in OPs case where he actually deletes the entire terraform resource definition but the following will work if you rename roles.

resource "snowflake_role" "y" {
  name    = "Y"
}

resource "snowflake_role" "x" {
  name    = "X"
}
resource "snowflake_role_grants" "x" {
  role_name = snowflake_role.x.name

  roles = [
    snowflake_role.y.name,
  ]
  depends_on = [
    snowflake_role.x,
    snowflake_role.y,
  ]
  lifecycle {
    replace_triggered_by = [
      snowflake_role.x,
      snowflake_role.y,
    ]
  }

}

I am not 100% sure the depends_on is needed for the lifecycle to work haven't tested it, but I create most of my roles using for_each for a large number of roles so I always have it anyways.

[rjoelnorgren]

This issue makes running the snowflake provider in CI difficult. User is forced to either run these types of plans twice, first with failure but state syncing to desired state, then again so you can report a successful deploy.

Alternatively, you could make the change to remove the role in 2 phases. First apply removal of all the grants, then apply a removal the role, but that seems pretty cludgy to me.

The provider should be aware of the dependency between these resources and plan accordingly, not require the user to jump through hoops to get successful applies.

[matt-tyler]

Alternatively check that the user or role exists before attempting to revoke the permission seeing as the grant can't possibly exist if the user/role doesn't.

[matt-tyler]

Same issue is mentioned in #1204.

I think this probably affects most grants.

The grant resources represent some object x being granted to another object y.

There was a PR to fix revokes on resource changes when granting role x to user y, where y has been deleted (#1142).

This issue is now referring to role x to role y.

Compare

terraform-provider-snowflake/pkg/resources/role_grants.go

Line 219 in 0dff065

func revokeRoleFromRole(db *sql.DB, role1, role2 string) error {

with

terraform-provider-snowflake/pkg/resources/role_grants.go

Line 225 in 0dff065

func revokeRoleFromUser(db *sql.DB, role1, user string) error {

I'm unsure as to whether simply checking for an error and swallowing it is the right way to go - but it probably is.

Grant creation obviously has to fail if either sides of the grant relationship do not exist.
On update or delete, if this resolves in needing to perform a revoke, and the revoke fails from either side not existing, that should be fine - as non-existence means that the revoke has already happened anyway.

As a revoke always happens as a result of removing a grant, it probably makes sense to ignore failed revokes where either side does not exist. However, the error that is returned does mention potential auth failure, so there would need to be some way of distinguishing between something not existing and simply an auth failure. I presume this is why in the 'user' issue above, why a list call is made. This might make it difficult to handle non-existence generically.

Of note, there are some generic handlers that seem to have been created for handling revokes, of which certain grant types are (database_grant for example https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/pkg/resources/database_grant.go) but these are not being used for role_grant, despite an implementation existing for role_grant. I assume there was a refactoring effort at some point that did not complete (role_grant predates database_grant).

@sfc-gh-swinkler you seem to be most active on this repository at the moment - do you have any thoughts?

[zmaleki]

Fixing this bug would take away a lot of pain points where we have to fiddle with terraform state to get around this error. As mentioned above it might be easy enough to check if the role exists, if it doesn't then revoking the grant means nothing and can be ignored

[sfc-gh-swinkler]

@matt-tyler a simple fix would be to check if the role exists before attempting to revoke a role that has already been dropped.

If you really want this done then please file a support ticket with your Snowflake account representative. When you file a support ticket it gets priority above other backlog issues. It also helps us to make the case to allocate more engineering resources toward supporting the provider in the future.

[dlouseiro]

Thanks @sfc-gh-swinkler. I created a Snowflake support case for this purpose!

[DustinMoriarty]

I am having the same problem. For a revoke, ultimately the logic should be that if the role no longer exists then the revoke is already complete so no error should be raised.

In general, the problem I see most in this provider relates to the behavior when a resource is changed or destroyed.

[DustinMoriarty]

I have reported this in a snowflake support case and it has become Jira task SNOW-750421.

[DustinMoriarty]

Also, note that the fix should include all grant resources since they all probably follow a similar pattern.

[DustinMoriarty]

Here is an example in which a role name is changed.

terraform {
  required_version = ">=1.3.4,<1.4"
  required_providers {
    snowflake = {
      source  = "Snowflake-Labs/snowflake"
      version = ">=0.56.5,<0.57"
    }
  }
}


provider "snowflake" {
  role    = "SECURITYADMIN"
}

resource "snowflake_role" "parent" {
  name = "FOO_PARENT"
}

resource "snowflake_role" "child" {
  name = "FOO_CHILD"
}

resource "snowflake_role_grants" "child_to_parent" {
  role_name = snowflake_role.parent.name
  roles = [snowflake_role.child.name]
}

Logs after changing the name of the parent resource.

% terraform apply
snowflake_role.child: Refreshing state... [id=FOO_CHILD]
snowflake_role.parent: Refreshing state... [id=FOO_PARENT]
snowflake_role_grants.child_to_parent: Refreshing state... [id=FOO_PARENT❄️FOO_CHILD❄️]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # snowflake_role.parent will be updated in-place
  ~ resource "snowflake_role" "parent" {
        id   = "FOO_PARENT"
      ~ name = "FOO_PARENT" -> "FOO_PARENT_2"
    }

  # snowflake_role_grants.child_to_parent will be updated in-place
  ~ resource "snowflake_role_grants" "child_to_parent" {
        id                     = "FOO_PARENT❄️FOO_CHILD❄️"
      ~ role_name              = "FOO_PARENT" -> "FOO_PARENT_2"
        # (3 unchanged attributes hidden)
    }

Plan: 0 to add, 2 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

snowflake_role.parent: Modifying... [id=FOO_PARENT]
snowflake_role.parent: Modifications complete after 3s [id=FOO_PARENT_2]
snowflake_role_grants.child_to_parent: Modifying... [id=FOO_PARENT❄️FOO_CHILD❄️]
╷
│ Error: 002003 (02000): SQL compilation error:
│ Role 'FOO_PARENT' does not exist or not authorized.
│
│   with snowflake_role_grants.child_to_parent,
│   on [main.tf](http://main.tf/) line 25, in resource "snowflake_role_grants" "child_to_parent":
│   25: resource "snowflake_role_grants" "child_to_parent" {
│
╵

[KPGR]

I believe this has been fixed as of v0.58.0. See here:

terraform-provider-snowflake/pkg/resources/role_grants.go

Line 220 in 9087220

func revokeRoleFromRole(db *sql.DB, role1, role2 string) error {

[DustinMoriarty]

@KPGR and @dlouseiro: I have tested this with snowflake provider 0.59.0 and terraform 1.4.2 for both the use case of changing the name of the parent role and terraform destroy and the bug is no longer present.

Here is the configuration I used for testing.

terraform {
  required_version = ">=1.4.2,<1.5"
  required_providers {
    snowflake = {
      source  = "Snowflake-Labs/snowflake"
      version = ">=0.59.0,<0.60"
    }
  }
}


provider "snowflake" {
  role    = "SECURITYADMIN"
}

resource "snowflake_role" "parent" {
  name = "FOO_PARENT"
}

resource "snowflake_role" "child" {
  name = "FOO_CHILD"
}

resource "snowflake_role_grants" "child_to_parent" {
  role_name = snowflake_role.parent.name
  roles = [snowflake_role.child.name]
}

[sfc-gh-swinkler]

@DustinMoriarty thank you for verifying that this bug is no longer found. I am going to go ahead and close this issue then. If it is still a problem, please feel free to reopen the ticket / create a new ticket.