[General Usage]: Why do I need to fully qualify snowflake_grant_privileges_to_account_role.account_role_name?

[jamiekt]

Terraform CLI Version

1.9.3

Terraform Provider Version

0.94.1

Terraform Configuration

I have some Snowflake role grants that I want to bring under the control of terraform, so I need to import them. Here is my code:

locals {
   monitor_usage_task_roles = toset([
    data.snowflake_role.MY_ROLE1.name,
    data.snowflake_role.MY_ROLE2.name,
  ])
}
import {
  for_each = local.monitor_usage_task_roles
  to       = snowflake_grant_privileges_to_account_role.monitor_usage_task[each.key]
  id       = "${each.key}|false|false|MONITOR USAGE|OnAccount"
}
resource "snowflake_grant_privileges_to_account_role" "monitor_usage_task" {
  for_each          = local.monitor_usage_task_roles
  privileges        = ["MONITOR USAGE"]
  account_role_name = each.key
  with_grant_option = false
  on_account        = true
}

The resulting plan looks like this for all those roles:

    # snowflake_grant_privileges_to_account_role.monitor_usage_task["MY_ROLE1"] must be replaced
    # (imported from "MY_ROLE1|false|false|MONITOR USAGE|OnAccount")
    # Warning: this will destroy the imported resource
  -/+ resource "snowflake_grant_privileges_to_account_role" "monitor_usage_task" {
        ~ account_role_name = "\"MY_ROLE1\"" -> "MY_ROLE1" # forces replacement
          all_privileges    = false
          always_apply      = false
        ~ id                = "MY_ROLE1|false|false|MONITOR USAGE|OnAccount" -> (known after apply)
          on_account        = true
          privileges        = [
              "MONITOR USAGE",
          ]
          with_grant_option = false
      }

Obviously # Warning: this will destroy the imported resource is very worrying, I definitely don't want to do that.

I note from https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/grant_privileges_to_account_role#import that

All the …_name parts should be fully qualified names (where every part is quoted)

So I change my import block to this (note the new escaped quotes in the id of the import):

import {
  for_each = local.monitor_usage_task_roles
  to       = snowflake_grant_privileges_to_account_role.monitor_usage_task[each.key]
  id       = "\"${each.key}\"|false|false|MONITOR USAGE|OnAccount"
}

The new plan is:

    # snowflake_grant_privileges_to_account_role.monitor_usage_task["MY_ROLE1"] must be replaced
    # (imported from ""MY_ROLE1"|false|false|MONITOR USAGE|OnAccount")
    # Warning: this will destroy the imported resource
  -/+ resource "snowflake_grant_privileges_to_account_role" "monitor_usage_task" {
        ~ account_role_name = "\"MY_ROLE1\"" -> "MY_ROLE1" # forces replacement
          all_privileges    = false
          always_apply      = false
        ~ id                = "\"MY_ROLE1\"|false|false|MONITOR USAGE|OnAccount" -> (known after apply)
          on_account        = true
          privileges        = [
              "MONITOR USAGE",
          ]
          with_grant_option = false
      }

Note the warning, my resource is still going to be destroyed due to the altered account_role_name

Hence I add the same escaping to account_role_name

resource "snowflake_grant_privileges_to_account_role" "monitor_usage_task" {
  for_each          = local.monitor_usage_task_roles
  privileges        = ["MONITOR USAGE"]
  account_role_name = "\"${each.key}\""
  with_grant_option = false
  on_account        = true
}

This time my plan looks better

    # snowflake_grant_privileges_to_account_role.monitor_usage_task["MY_ROLE1"] will be imported
      resource "snowflake_grant_privileges_to_account_role" "monitor_usage_task" {
          account_role_name = "\"MY_ROLE1\""
          all_privileges    = false
          always_apply      = false
          id                = "\"MY_ROLE1\"|false|false|MONITOR USAGE|OnAccount"
          on_account        = true
          privileges        = [
              "MONITOR USAGE",
          ]
          with_grant_option = false
      }

This doesn't feel right though. If I was defining brand new privileges for a role I wouldn't have to wrap account_role_name with escaped quotes, so why do I have to do it when importing? That import block will be removed after the objects are imported so I'm left with escaped quotes around account_role_name with no real reason for them being there (other than they were required for importing). Moreover, I'm concerned that the role refered to when this gets applied will be

• "MY_ROLE1" as opposed to
• MY_ROLE1

This seems like a strange stipulation. What is the peculiar circumstance that requires me to use escaped quotes when importing?

Category

category:resource

Object type(s)

resource:grant_privileges_to_account_role

Expected Behavior

I would have expected this code to be sufficient

resource "snowflake_grant_privileges_to_account_role" "monitor_usage_task" {
  for_each          = local.monitor_usage_task_roles
  privileges        = ["MONITOR USAGE"]
  account_role_name = each.key
  with_grant_option = false
  on_account        = true
}

Actual Behavior

I'm required to use

resource "snowflake_grant_privileges_to_account_role" "monitor_usage_task" {
  for_each          = local.monitor_usage_task_roles
  privileges        = ["MONITOR USAGE"]
  account_role_name = "\"${each.key}\""
  with_grant_option = false
  on_account        = true
}

Steps to Reproduce

Create a role in the Snowflake UI Use the code provided above to try to import it

How much impact is this issue causing?

Low

Logs

No response

Additional Information

I don't think so, but if other info is required please let me know

[sfc-gh-jcieslak]

Hey @jamiekt 👋 Thank you for reporting the issue. As I'm currently working on identifiers, I'll take a look at it as soon as possible and try to fix it. At first glance at your config and the code we have right now, I don't see any reason why this

import {
  for_each = local.monitor_usage_task_roles
  to       = snowflake_grant_privileges_to_account_role.monitor_usage_task[each.key]
  id       = "\"${each.key}\"|false|false|MONITOR USAGE|OnAccount"
}

wouldn't work. After I take care of other pending cases, I'll take a closer look at this, but it seems like a typical case of quote mismatch case that we have in some of the other resources we're currently fixing.

[jamiekt]

Hi @sfc-gh-jcieslak , thank you very much that is much appreciated.

Just to clarify, this code:

import {
  for_each = local.monitor_usage_task_roles
  to       = snowflake_grant_privileges_to_account_role.monitor_usage_task[each.key]
  id       = "\"${each.key}\"|false|false|MONITOR USAGE|OnAccount"
}

does work. The issue is that I need to use embedded quotes in the value assigned to account_role_name in order for the resource not to be destroyed-and-recreated.

[jamiekt]

Hi @sfc-gh-jcieslak , Are you able to give an update on this issue?

Thanks in advance.

[sfc-gh-jcieslak]

Yeah, I'm currently working on it, and after that change the quoting should not matter. It'll most likely be a part of the next release (somewhere around next week).

[jamiekt]

@sfc-gh-jcieslak Awesome, thank you so much.

[jamiekt]

@sfc-gh-jcieslak When you release this fix will you also do a patch release for 0.92.X? The reason for my asking is that in my PR which is currently blocked by this I am using removed blocks to remove deprecated resources such as snowflake_account_grant, e.g.:

removed {
  from = snowflake_account_grant.monitor_task_execution
  lifecycle {
    destroy = false
  }
}

For that reason I need to use provider version "<0.93.0". If I use provider version 0.93.0 or later I get an error when running terraform plan:

Error: no schema available for snowflake_account_grant.apply_masking_policy while reading state

However, in the same PR I'm also importing the objects created by the resources that are being removed.

import {
  for_each = local.monitor_task_execution_roles
  to       = snowflake_grant_privileges_to_account_role.monitor_task_execution[each.key]
  id       = "\"${each.key}\"|false|false|MONITOR EXECUTION|OnAccount"
}

and this is what is causing the problem that we've discussed above.

Hence we'd need a new version of the provider with the fix you're working on now but which is still "<0.93.0". Hope that makes sense.

[sfc-gh-jcieslak]

Hey I'm sorry, but currently, as we are 0.X.0 we're not supporting backward bugfixes. Even If we would like to do it, our release process is pretty limited at the moment, and it would take us some time to release such a version and make sure that the next versions also have that bugfix. Please, for now, use the quoted version. Starting from the v0.95.0 quoting shouldn't matter.

[sfc-gh-jcieslak]

Hey 👋 Yesterday, we released a new version of the provider (v0.95.0) that shouldn't produce any plans related to quoting. We also prepared a document concluding identifiers rework (here) that should clear things up regarding identifiers and how they behave.

[jamiekt]

Great, thanks @sfc-gh-jcieslak . I'll close this issue and re-open if I find any issues in v.0.95.0

[jamiekt]

Hi @sfc-gh-jcieslak , Hoping I can get your advice on something. We are currently replacing the resource types that have been removed in 0.93. How we're doing that is:

• pinning to 0.92.0
• replacing the old resource types with their replacement counterparts
• using removed blocks to remove the replaced resources from the state file
• importing the definition of the new resource type using import blocks

The old code looks like this:

resource "snowflake_grant_privileges_to_role" "warehouse_usage_grant" {
  for_each = toset(
   # logic to determine a list of roles
  )
  on_account_object {
    object_type = "WAREHOUSE"
    object_name = snowflake_warehouse.warehouse.name
  }
  privileges = ["USAGE"]

  role_name = each.key

  with_grant_option = false
}

The new code looks like this:

resource "snowflake_grant_privileges_to_account_role" "warehouse_usage_grant" {
  for_each = toset(
   # logic to determine a list of roles
  )
  account_role_name = each.key
  on_account_object {
    object_type = "WAREHOUSE"
    object_name = snowflake_warehouse.warehouse.name
  }
  privileges = ["USAGE"]

  with_grant_option = false
}

removed {
  from = snowflake_grant_privileges_to_role.warehouse_usage_grant
  lifecycle {
    destroy = false
  }
}

# in consuming root module:
import {
    for_each = toset(
   # logic to determine roles
    )
    to = module.resources.module.warehouse[0].snowflake_grant_privileges_to_account_role.warehouse_usage_grant[each.key]
    id = "${each.key}|false|false|USAGE|OnAccountObject|WAREHOUSE|WAREHOUSE_NAME"
}

There are a lot of resources that we're having to do this for.

The aim here is to produce a plan with zero changes but the plan currently looks like this:

Plan: 231 to import, 206 to add, 0 to change, 205 to destroy.

and the reason is this quoted identifier problem that we discussed above. We have 205 resources that appear in the plan like this:

  # module.resources.module.warehouse[0].snowflake_grant_privileges_to_account_role.warehouse_usage_grant["ROLE_NAME"] must be replaced
  # (imported from "ROLE_NAME|false|false|USAGE|OnAccountObject|WAREHOUSE|WAREHOUSE_NAME")
  # Warning: this will destroy the imported resource
-/+ resource "snowflake_grant_privileges_to_account_role" "warehouse_usage_grant" {
      ~ account_role_name = "\"ROLE_NAME\"" -> "ROLE_NAME" # forces replacement
        all_privileges    = false
        always_apply      = false
      ~ id                = "ROLE_NAME|false|false|USAGE|OnAccountObject|WAREHOUSE|WAREHOUSE_NAME" -> (known after apply)
        on_account        = false
        privileges        = [
            "USAGE",
        ]
        with_grant_option = false

      ~ on_account_object {
          ~ object_name = "\"WAREHOUSE_NAME\"" -> "WAREHOUSE_NAME" # forces replacement
            object_type = "WAREHOUSE"
        }
    }

Notice that the provider regards these resources as having been changed because of the quoted identifiers.

We do not want the grants to be dropped and recreated because that would require downtime, which we absolutely cannot have.

Thus, we attempted to upgrade to 0.95.0 (which contains your fix for the quoted identifiers) but that failed with:

Error: no schema available for module.resources.module.warehouse[0].snowflake_grant_privileges_to_role.warehouse_usage_grant["ROLE_NAME"] while reading state;

which is because resource type snowflake_grant_privileges_to_role no longer exists in 0.95.0.

hence we're between a rock and a hard place. We can't use 0.92.0 because it would cause all of our resources to be dropped and recreated. We can't use 0.95 because it no longer contains the definition of resource types that are in the state file.

Do you have any advice about how to tackle this?

[sfc-gh-jcieslak]

Hey @jamiekt 👋 The simplest but cumbersome approach that I can think of right now is to use lifecycle meta-argument to prevent_changes on the fields that may cause replacement (also maybe prevent_destroy would help additionally). That should enable you to upgrade to v0.93.0 then after you upgrade to v0.95.0, you'll be able to drop prevent_changes meta-attributes and see no changes planned. It's worth noting that I'm not an expert in using Terraform, but that should work, afaik. Let me know If that approach works for you; if not, maybe I'll be able to find another way.

[jamiekt]

Thx @sfc-gh-jcieslak , we'll look into doing that.

[kadu-vido]

Hi @sfc-gh-jcieslak, I'm the Data Ops engineer working with Jamie on this.

use lifecycle meta-argument to prevent_changes on the fields that may cause replacement (also maybe prevent_destroy would help additionally)

This can't work, since these lifecycle arguments just cause the plan to throw errors. From the terraform language docs:

prevent_destroy (bool) - This meta-argument, when set to true, will cause Terraform to reject with an error any plan that would destroy the infrastructure object associated with the resource, as long as the argument remains present in the configuration. (...) it will make certain configuration changes impossible to apply.

so essentially, if there would be any changes that would destroy assets, the plan and the apply would just fail. I've still tested it (the commit you see above), and confirmed this.

Similarly, I think ignore_changes=true would introduce state drift, i.e. the state would be mismatched with the code. We could arguably add this, migrate to 0.93 with the extra quotes, then remove this and remove the argument. But this is considerably harder to test, and if after both steps we're still drifting, it'll probably be considerably harder to fix 😅

as we are 0.X.0 we're not supporting backward bugfixes.

Can I make a plea for the team to reconsider this decision? Because 0.92.0 -> 0.93.0 introduced a big breaking change with the grants, and also a bug, patching 0.92 with a fix to the bug would allow for a much smoother, one-step upgrade.

For context: my team maintains a set of modules and an onboarding template used by >30 teams spread from US to Australia. A 2-step process essentially doubles the effort needed for all of them, you're essentially guaranteeing my team won't get much sleep because someone broke their grants in the middle of the (central Europe) night 😅

Have any SF clients reported issues due to these extraneous quotes? Do we know for sure whether or not this will create duplicate roles with quotes in the actual role name? In other words: if ignore this bug and just leave the extraneous quotes there in the code, will my customers suddenly have e.g. both SYSADMIN and "SYSADMIN" in their warehouses? I think this is Jamie's biggest concern, correct me if I'm wrong @jamiekt

[sfc-gh-jcieslak]

Hey 👋

This can't work, since these lifecycle arguments just cause the plan to throw errors.

Then, in this case, prevent_destroy shouldn't be used (only prevent_changes that would prevent plan changes on id and name).

Similarly, I think ignore_changes=true would introduce state drift, i.e. the state would be mismatched with the code.

Yes, but in 0.95.0, that's handled, and you can remove the prevent_changes blocks there (the plan shouldn't be produced there).

Have any SF clients reported issues due to these extraneous quotes?

Yes, that's why we planned to refactor them for a long time, and only recently did we find time to do so. Unfortunately, as the refactoring was concluded recently, only the latest version (0.95.0) has all the changes regarding identifier rework (documented here).

Do we know for sure whether or not this will create duplicate roles with quotes in the actual role name?

If you refer to snowflake_account_role, in 0.95.0, it doesn't matter if the name is equal to name = "\"role_name\" or name = "role_name" (it will be treated as the same name and won't create duplicates).

Also, I wanted to ask if migrating straight to 0.95.0 is possible? I know that it's not with removed blocks, but you can also do it from the CLI level. That would go around the error about snowflake_grant_privileges_to_role being removed.

I hope I addressed all the concerns. Please, try to proceed with migration without the prevent_destroy flag as suggested. If that fails, we'll consider other alternatives.

[jamiekt]

If you refer to snowflake_account_role, in 0.95.0, it doesn't matter if the name is equal to name = "\"role_name\" or name = "role_name" (it will be treated as the same name and won't create duplicates).

That is very good news, thank you @sfc-gh-jcieslak

[sfc-gh-jcieslak]

@jamiekt Just to show how it behaves I did a simple test

resource "snowflake_account_role" "test" {
  name = "TEST_ROLE_ONE" # results in TEST_ROLE_ONE (in Snowflake)
}

resource "snowflake_account_role" "test2" {
  name = "\"TEST_ROLE_TWO\"" # results in TEST_ROLE_TWO (in Snowflake)
}

Adding a third role with role name quoted internally would cause a naming clash with the one that is not quoted resulting in SQL compilation error: Object 'TEST_ROLE_ONE' already exists..

resource "snowflake_account_role" "test" {
  name = "TEST_ROLE_ONE"
}

resource "snowflake_account_role" "test2" {
  name = "\"TEST_ROLE_TWO\""
}

resource "snowflake_account_role" "test3" {
  name = "\"TEST_ROLE_ONE\"" # this is the same as the first one
}

Hope that helps.