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 resources report perpetual diff #1514

Closed
bennylu2 opened this issue Feb 3, 2023 · 13 comments
Closed

Grant resources report perpetual diff #1514

bennylu2 opened this issue Feb 3, 2023 · 13 comments
Labels
bug Used to mark issues with provider's incorrect behavior

Comments

@bennylu2
Copy link
Contributor

bennylu2 commented Feb 3, 2023

Provider Version

0.56.3

Terraform Version

1.3.4

Describe the bug

Any plans with grant resources will show a perpetual diff that it wants to add the grant to the roles.

Running and applying the same code over and over wants to add grants to roles

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

Expected behavior

Once grants have been granted, there should be no diff

Additional context

I've identified this happened with release 0.56.3 in #1511 and #1510

It appears a line was removed that checks the privs

NOTE: as a workaround for anyone that comes across this, we will be using .terraform.lock.hcl file and set required_providers version constraint to use a prior version that does not have this bug

@bennylu2 bennylu2 added the bug Used to mark issues with provider's incorrect behavior label Feb 3, 2023
@sfc-gh-swinkler
Copy link
Collaborator

I am sorry @bennylu2 but i am not able to reproduce this. If i try to create a grant right now then i do not get any changes in the plan. Can you please provide more information? Sample code snippet, and maybe some json data from state file?

@bennylu2
Copy link
Contributor Author

bennylu2 commented Feb 6, 2023

@sfc-gh-swinkler here's some sample code

terraform plan diff for single view grant

  # module.tdm_demo.module.public_schemas["CLEANSED"].snowflake_view_grant.this["SELECT"] will be updated in-place
  ~ resource "snowflake_view_grant" "this" {
        id                     = "TDM_DEMO|CLEANSED||SELECT|false"
      ~ roles                  = [
          + "TDM_DEMO__CLEANSED__READ",
        ]
        # (6 unchanged attributes hidden)
    }

terraform state which shows it already has grant

$ tf state show 'module.tdm_demo.module.public_schemas["CLEANSED"].snowflake_view_grant.this["SELECT"]'                                                           
# module.tdm_demo.module.public_schemas["CLEANSED"].snowflake_view_grant.this["SELECT"]:
resource "snowflake_view_grant" "this" {
    database_name          = "TDM_DEMO"
    enable_multiple_grants = true
    id                     = "TDM_DEMO|CLEANSED||SELECT|false"
    on_future              = true
    privilege              = "SELECT"
    roles                  = [
        "TDM_DEMO__CLEANSED__READ",
    ]
    schema_name            = "CLEANSED"
    with_grant_option      = false
}

Associated terraform view grant in module

resource "snowflake_view_grant" "this" {
  for_each = transpose(local.grants_view)

  database_name          = snowflake_schema.this.database
  schema_name            = snowflake_schema.this.name
  privilege              = each.key
  roles                  = each.value
  on_future              = true
  with_grant_option      = false
  enable_multiple_grants = true # needed until we sunset database migrations
}

@sfc-gh-swinkler
Copy link
Collaborator

sfc-gh-swinkler commented Feb 6, 2023

@bennylu2 i see what the problem is. You have on_future enabled. The logic for performing a read and how to do the diff calculation for this resource is a bit tricky and it looks like i made a mistake when refactoring it. I have a fix here: #1522.

@bennylu2
Copy link
Contributor Author

bennylu2 commented Feb 6, 2023

When will it be released? Or I can test against the main branch but likely won't be able to get to till later this week

@sfc-gh-swinkler
Copy link
Collaborator

Currently trying to fix a related issue, having to do with import id for misc grants. Once i fix this, then i will create a new release. Next day or so.

@mark-oppenheim
Copy link

Hoping this is still on track!

@grahamwetzler
Copy link

Is this supposed to be fixed in 0.56.5? I'm still having the issue after upgrading

@mark-oppenheim
Copy link

Any news on this? This is more serious than it sounds when coupled with FUTURE OWNERSHIP grants.

@kallangerard
Copy link

Why do we need a resource id. It seems to me that it's being used inappropriately to generate the plan, instead of the actual attributes themselves.

For example,

Given a database grant is applied as so

resource "snowflake_database_grant" "this" {
  database_name = "MY_DATABASE"
  privilege     = "REFERENCE_USAGE"
  roles = [
    "OLD_ROLE"
  ]
}

This creates a resource in the state with the id of MY_DATABASE❄️REFERENCE_USAGE❄️false❄️OLD_ROLE❄️

Then the resource arguments change to

resource "snowflake_database_grant" "this" {
  database_name = "MY_DATABASE"
  privilege     = "REFERENCE_USAGE"
  roles = [
    "NEW_ROLE"
  ]
}

This then shows a perpetual diff of

~ resource "snowflake_database_grant" "this" {
  id = "MY_DATABASE❄️REFERENCE_USAGE❄️false❄️OLD_ROLE❄️"
  ~ roles = [
    + "NEW_ROLE",
  ]
  # (5 unchanged attributes hidden)
}

I'm not entirely sure on the logic happening, but it's obvious it's using the static attribute of id to handle state refresh and plan generation. Why would we even need to use a resource id at all, given that it's not a real underlying Snowflake infrastructure id.

From my understanding, we should be using the actual arguments to calculate the plan, not a write once id attribute.

@matthewoflynn-dxrx
Copy link

Downgrading to 0.55.1 has allowed me to avoid the issue for the time being until this is resolved.

@sfc-gh-swinkler
Copy link
Collaborator

@bennylu2 can i close this issue?

@grahamwetzler
Copy link

This seems to be fixed for me on 0.60.0

@bennylu2
Copy link
Contributor Author

This has been resolved in 0.60.0, thanks for the work on this!

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

6 participants