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

GRANT IMPORTED PRIVILEGES TO ROLE introduced permadiff #2366

Closed
jrobison-sb opened this issue Jan 17, 2024 · 14 comments
Closed

GRANT IMPORTED PRIVILEGES TO ROLE introduced permadiff #2366

jrobison-sb opened this issue Jan 17, 2024 · 14 comments
Assignees
Labels
bug Used to mark issues with provider's incorrect behavior

Comments

@jrobison-sb
Copy link
Contributor

jrobison-sb commented Jan 17, 2024

Terraform CLI and Provider Versions

Terraform v1.6.6
on darwin_arm64
+ provider registry.terraform.io/snowflake-labs/snowflake v0.73.0

Terraform Configuration

resource "snowflake_database_grant" "staff_imported_priviliges_on_snowflake_database" {
  # `GRANT IMPORTED PRIVILEGES ON DATABASE SNOWFLAKE TO ROLE STAFF_READ_WRITE`
  # This allows staff to query billing metrics in the snowflake database.
  # https://docs.snowflake.com/en/sql-reference/account-usage#enabling-the-snowflake-database-usage-for-other-roles
  database_name = "SNOWFLAKE"
  privilege     = "IMPORTED PRIVILEGES"
  roles = [
    snowflake_role.staff_read_write.name,
    snowflake_role.staff_read_only.name,
  ]
  with_grant_option = false
}


### Expected Behavior

I expect to be able to apply this with the usual terraform workflow:
1. `terraform apply` # This should apply cleanly, which it does
2. `terraform plan`  # This should report no diffs, because I already applied the grants

### Actual Behavior

Results in a permadiff:
1. `terraform apply` # This should apply cleanly, which it does
2. `terraform plan`  # This reports a permadiff

### Steps to Reproduce

Use the above HCL code, and then:

1. `terraform apply`
2. `terraform plan` # permadiff seen here


### How much impact is this issue causing?

Low

### Logs

_No response_

### Additional Information

_No response_
@jrobison-sb jrobison-sb added the bug Used to mark issues with provider's incorrect behavior label Jan 17, 2024
@sfc-gh-jcieslak
Copy link
Collaborator

Hey @jrobison-sb
The fix for that should be introduced soon. We are aware of the issue with IMPORTED PRIVILEGES. We have to handle it differently than other privileges as this privilege is a "pseudo-role" and it cannot be read from Snowflake.

@sfc-gh-jcieslak sfc-gh-jcieslak self-assigned this Jan 22, 2024
@sfc-gh-jcieslak
Copy link
Collaborator

Hey 👋, the fix for imported privileges has been merged in #2471 and will be available in the next release. As soon as the latest version appears (next week), please let us know if it fixes your bug and the issue can be closed.

@chrisweis
Copy link

chrisweis commented Feb 25, 2024

@sfc-gh-jcieslak I'm running v0.86.0 but I'm still seeing a permadiff for IMPORTED PRIVILEGES. Is it working properly now for you @jrobison-sb ?

My resource is slightly different than above, maybe that's why?:

resource "snowflake_grant_privileges_to_account_role" "SNOWFLAKE__DEV_DBT_ROLE" {
  provider = snowflake.snowflake_terraform_securityadmin
  privileges        = ["IMPORTED PRIVILEGES"]
  account_role_name = snowflake_role.DEV_DBT_ROLE.name
  on_account_object {
    object_type = "DATABASE"
    object_name = "SNOWFLAKE"
  }
  with_grant_option = false
  depends_on = [ snowflake_role.DEV_DBT_ROLE ]
  # lifecycle {
  #   ignore_changes = all # disables updates due to what seems like a bug - https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/1573
  # }
}

The repeating difference looks like this for me:

Terraform will perform the following actions:

  # snowflake_grant_privileges_to_account_role.SNOWFLAKE__DEV_DBT_ROLE will be updated in-place
  ~ resource "snowflake_grant_privileges_to_account_role" "SNOWFLAKE__DEV_DBT_ROLE" {
        id                = "\"DEV_DBT_ROLE\"|false|false|IMPORTED PRIVILEGES|OnAccountObject|DATABASE|\"SNOWFLAKE\""
      ~ privileges        = [
          + "IMPORTED PRIVILEGES",
        ]
        # (5 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

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

I also tried the database_grant resource and it also seems to have the permadiff issue. Thanks!

@jrobison-sb
Copy link
Contributor Author

@chrisweis We haven't been able to upgrade to the new provider yet because we still have to fix some breaking changes which were introduced in a prior version of the provider. So I haven't tried this yet, sorry.

@sfc-gh-jcieslak
Copy link
Collaborator

sfc-gh-jcieslak commented Feb 26, 2024

@chrisweis That's strange, I did a minimal test (very similar to your configuration) with 0.86.0 right now and it worked. Did you manually query grants on the database to see if USAGE is there or not? (reference why USAGE and not IMPORTED PRIVILEGES) Could you try and produce a minimal configuration that would mimic this behavior, so I could debug why is it happening? My take on reproducing your configuration produced no errors (no permadiff) :/

@gdelia-pm
Copy link

@sfc-gh-jcieslak Hi Jan, not the original poster but I'm seeing the same issue with 0.86 and 0.87

My config is as follows

resource "snowflake_grant_privileges_to_account_role" "GRANT_IMPORTED_PRIVILEGES-DATA_DEVELOPER_PRD" {
  depends_on = [snowflake_role.FUNCTIONAL_ROLES["DATA_DEVELOPER_PRD"]]

  privileges        = ["IMPORTED PRIVILEGES"]
  account_role_name = "DATA_DEVELOPER_PRD"
  on_account_object {
    object_type = "DATABASE"
    object_name = "SNOWFLAKE"
  }
}

And when i query the state file things look good.

 .\terraform.exe state show snowflake_grant_privileges_to_account_role.GRANT_IMPORTED_PRIVILEGES-DATA_DEVELOPER_PRD 
# snowflake_grant_privileges_to_account_role.GRANT_IMPORTED_PRIVILEGES-DATA_DEVELOPER_PRD:
resource "snowflake_grant_privileges_to_account_role" "GRANT_IMPORTED_PRIVILEGES-DATA_DEVELOPER_PRD" {
    account_role_name = "DATA_DEVELOPER_PRD"
    all_privileges    = false
    always_apply      = false
    id                = "\"DATA_DEVELOPER_PRD\"|false|false|IMPORTED PRIVILEGES|OnAccountObject|DATABASE|\"SNOWFLAKE\""
    on_account        = false
    privileges        = [
        "IMPORTED PRIVILEGES",
    ]
    with_grant_option = false

    on_account_object {
        object_name = "SNOWFLAKE"
        object_type = "DATABASE"
    }
}

But on every plan/ apply I see

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_grant_privileges_to_account_role.GRANT_IMPORTED_PRIVILEGES-DATA_DEVELOPER_PRD will be updated in-place
  ~ resource "snowflake_grant_privileges_to_account_role" "GRANT_IMPORTED_PRIVILEGES-DATA_DEVELOPER_PRD" {
        id                = "\"DATA_DEVELOPER_PRD\"|false|false|IMPORTED PRIVILEGES|OnAccountObject|DATABASE|\"SNOWFLAKE\""
      ~ privileges        = [
          + "IMPORTED PRIVILEGES",
        ]
        # (5 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

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

When I query the snowflake database share in snowsight

Show Grants on Database SNOWFLAKE;
created_on	privilege	granted_on	name	granted_to	grantee_name	grant_option	granted_by	granted_by_role_type
2024-01-26 08:53:16.553 -0800	USAGE	APPLICATION	SNOWFLAKE	APPLICATION_ROLE	BUDGET_ADMIN	false		
2024-01-26 08:53:16.425 -0800	USAGE	APPLICATION	SNOWFLAKE	APPLICATION_ROLE	BUDGET_VIEWER	false		
2024-02-27 07:22:37.647 -0800	USAGE	APPLICATION	SNOWFLAKE	ROLE	DATA_DEVELOPER_PRD	false		

Let me know if this helps or if I can do some other testing to help clarify the issue.

@sfc-gh-jcieslak
Copy link
Collaborator

Thanks for the answer @gdelia-pm. The issue was/still is connected to bad Read operation. I solved it for shared databases, but It looks like the SNOWFLAKE database is actually of type APPLICATION (as you can see in the posted output granted_on column). It's connected to how closely related applications are to databases. We'll need additional check/convertion for this to work. I'll try to fix it case for the next release. Thanks again for the data :)

sfc-gh-jcieslak added a commit that referenced this issue Mar 6, 2024
Fix for: #1998 and #2366

Changes
- Because it's the default database it fulfills our `if` checks for
futures, so added a check for `name != SNOWFLAKE`
- The second check was added to make it possible to grant privileges on
applications by setting object_type to DATABASE
- Those cases were added to the documentation for the
`snowflake_grant_privilege_to_account_role` resource
- Added acceptance test that would check if SNOWFLAKE database could be
made with `object_type = DATABASE`
- I also added a follow-up ticket to add additional tests for
applications when they are available.
@sfc-gh-jcieslak
Copy link
Collaborator

Hi again @jrobison-sb @chrisweis @gdelia-pm 👋
Today we released a bugfix version 0.87.1. Please give it a go and see if the issue persists.

@gdelia-pm
Copy link

Confirmed that this is now working for me. Thanks y'all!

@jrobison-sb
Copy link
Contributor Author

Hi again @sfc-gh-jcieslak, thanks for your attention on this.

I was finally able to upgrade my provider version to the latest version and give this a try, but unfortunately I'm running into the same situation mentioned by @gdelia-pm, though with an additional wrinkle that I'm using terraform import for part of it.

Version:

$ terraform version
Terraform v1.6.6
on darwin_arm64
+ provider registry.terraform.io/snowflake-labs/snowflake v0.87.2

Old HCL which uses a deprecated snowflake_database_grant resource type:

resource "snowflake_database_grant" "staff_imported_priviliges_on_snowflake_database" {
  # `GRANT IMPORTED PRIVILEGES ON DATABASE SNOWFLAKE TO ROLE STAFF_READ_WRITE`
  # This allows staff to query billing metrics in the snowflake database.
  # https://docs.snowflake.com/en/sql-reference/account-usage#enabling-the-snowflake-database-usage-for-other-roles
  database_name = "SNOWFLAKE"
  privilege     = "IMPORTED PRIVILEGES"
  roles = [
    snowflake_role.role.name,
    snowflake_role.staff_read_write.name,
    snowflake_role.staff_read_only.name,
  ]
  with_grant_option = false

  lifecycle {
    # ignore_changes is needed as a temporary workaround for this bug:
    # https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2366
    ignore_changes = [roles]
  }
}

New HCL which uses the new snowflake_grant_privileges_to_account_role resource type, with this HCL mostly copy/pasted from the docs:

resource "snowflake_grant_privileges_to_account_role" "staff_imported_priviliges_on_snowflake_database" {
  for_each = toset([
    snowflake_role.role.name,
    snowflake_role.staff_read_write.name,
    snowflake_role.staff_read_only.name,
  ])
  account_role_name = jsonencode(each.key)
  privileges        = ["IMPORTED PRIVILEGES"]
  on_account_object {
    object_type = "DATABASE"
    object_name = jsonencode("SNOWFLAKE")
  }
}

Steps to reproduce:

# Save the Terraform state because I'll need to later
terraform state pull > terraform_state_before_migration.json

# Import the three new snowflake_grant_privileges_to_account_role resources
for role_name in staging_role STAGING_STAFF_READ_WRITE STAGING_STAFF_READ_ONLY
do
terraform import \
  module.staging_application.snowflake_grant_privileges_to_account_role.staff_imported_priviliges_on_snowflake_database[\"$role_name\"] \
  "\"$role_name\"|false|false|\"IMPORTED PRIVILEGES\"|OnAccountObject|DATABASE|SNOWFLAKE"
done

<snip>

Import successful!

The resources that were imported are shown above. These resources are now in
your Terraform state and will henceforth be managed by Terraform.

# Okay, it supposedly imported correctly, run a plan
terraform plan

<snip>

  # module.staging_application.snowflake_grant_privileges_to_account_role.staff_imported_priviliges_on_snowflake_database["STAGING_STAFF_READ_ONLY"] will be updated in-place
  ~ resource "snowflake_grant_privileges_to_account_role" "staff_imported_priviliges_on_snowflake_database" {
        id                = "\"STAGING_STAFF_READ_ONLY\"|false|false|\"IMPORTED PRIVILEGES\"|OnAccountObject|DATABASE|SNOWFLAKE"
      ~ privileges        = [
          + "IMPORTED PRIVILEGES",
        ]
        # (5 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

<with a similar change being shown for all three, but I'll only paste one for brevity>

# Welp, that didn't work, restore the old state
terraform state push -force terraform_state_before_migration.json

Am I doing the import wrong, perhaps? Is it possible that the bug described by @gdelia-pm still exists but it now only exists in the import code path? Anything else I should know for this one?

Thanks.

@sfc-gh-jcieslak
Copy link
Collaborator

Thanks for the response @jrobison-sb I'll try to reproduce it today.

@sfc-gh-jcieslak
Copy link
Collaborator

sfc-gh-jcieslak commented Mar 13, 2024

@jrobison-sb Could you try to import it without quotes around privileges ?
It should be IMPORTED PRIVILEGES instead of \"IMPORTED PRIVILEGES\".

@jrobison-sb
Copy link
Contributor Author

@sfc-gh-jcieslak that worked, thank you.

I don't see that anyone else has any outstanding requests in this issue so I'll go ahead and close this now.

Thanks again for your efforts.

Resolved by #2596

@chrisweis
Copy link

@sfc-gh-jcieslak Thanks for all the awesome help and improvements lately! You're restoring my faith in this Terraform provider! 😊

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
Projects
None yet
Development

No branches or pull requests

4 participants