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

Role attribute is ignored in the provider if the corresponding "SNOWFLAKE_ROLE" env variable is set #2242

Closed
pashamartinenko opened this issue Dec 8, 2023 · 6 comments
Labels
bug Used to mark issues with provider's incorrect behavior category:provider_config

Comments

@pashamartinenko
Copy link

Terraform CLI and Provider Versions

Terraform v1.3.7
on linux_amd64

provider registry.terraform.io/snowflake-labs/snowflake snowflake-labs/snowflake v0.74.0

Terraform Configuration

provider "snowflake" {
  // account - pulled from env variable
  // username - pulled from env variable
  // password - pulled from env variable
  // role - pulled from env variable (SNOWFLAKE_ROLE is set to ROLE_1)
}

provider "snowflake" {
  // account - pulled from env variable
  // username - pulled from env variable
  // password - pulled from env variable
  role  = "ROLE_2"
  alias = "role2"
}

resource "snowflake_database" "database1" {
  name                        = "database_name1"
}

resource "snowflake_database" "database2" {
  name                        = "database_name2"
  provider = snowflake.role2
}

Expected Behavior

database_name1 db owner is ROLE_1, database_name2 owner is ROLE_2

Actual Behavior

both database_name1 and database_name2 dbs' owner is ROLE_1

Steps to Reproduce

  1. run terraform init, plan, apply in a new workspace using v0.73.0 provider - returns expected result
  2. run terraform init, plan, apply in a new workspace using v0.74.0 provider - produces the described issue

How much impact is this issue causing?

Low

Logs

No response

Additional Information

workaround is unsetting SNOWFLAKE_ROLE env variable and assigning role attribute to both providers explicitly

@pashamartinenko pashamartinenko added the bug Used to mark issues with provider's incorrect behavior label Dec 8, 2023
@PedroMartinSteenstrup
Copy link

Similar behavior noted after upgrading to v0.82

Terraform v1.5.7
on darwin_arm64
+ provider registry.terraform.io/snowflake-labs/snowflake v0.82.0

My provider file

provider "snowflake" {
  alias                     = "admin_user"
  role                      = "USERADMIN"
  account               = "MR-.OE-"
  user                     = "tf"
  authenticator      = "JWT"
  private_key         = data.vault_generic_secret.snowflake.data["snowflake_tf_snow_key.p8"]
}

and

$ printenv | grep -i SNOWFLAKE_ROLE
SNOWFLAKE_ROLE=SYSADMIN

will result in SYSADMIN being used.

Given we are repeating the same steps for each upgrade for the past year, and we always did export SNOWFLAKE_ROLE="SYSADMIN" as step number 1, something must have changed in the precedence of env var SNOWFLAKE_ROLE vs being set in the provider directly.

@AZenat
Copy link

AZenat commented Feb 18, 2024

Same issue with v0.86.
Which is weird because according to the code, the value written in the provider should override the environment variable.

@sfc-gh-asawicki
Copy link
Collaborator

Hey @AZenat @PedroMartinSteenstrup @pashamartinenko. This seems connected to #2294. We plan to rework the configuration completely but because this is really misleading now and can result in unpredicted behavior, we will adjust the current config behavior to what the docs say.

I hope the fix will land next week (0.87.0 or 0.88.0).

sfc-gh-asawicki added a commit that referenced this issue Feb 23, 2024
Before fixing #2294 and #2242 we need to have a clean CI env. After this
PR it should be possible to completely remove following environment
variables from the secrets:
- AWS_EXTERNAL_BUCKET_URL (it has TEST_SF_TF_AWS_EXTERNAL_BUCKET_URL
replacement)
- AWS_EXTERNAL_KEY_ID (it has TEST_SF_TF_AWS_EXTERNAL_KEY_ID
replacement)
- AWS_EXTERNAL_ROLE_ARN (it has TEST_SF_TF_AWS_EXTERNAL_ROLE_ARN
replacement)
- AWS_EXTERNAL_SECRET_KEY (it has TEST_SF_TF_AWS_EXTERNAL_SECRET_KEY
replacement)
- AZURE_EXTERNAL_BUCKET_URL (it has TEST_SF_TF_AZURE_EXTERNAL_BUCKET_URL
replacement)
- AZURE_EXTERNAL_SAS_TOKEN (it has TEST_SF_TF_AZURE_EXTERNAL_SAS_TOKEN
replacement)
- AZURE_EXTERNAL_TENANT_ID (it has TEST_SF_TF_AZURE_EXTERNAL_TENANT_ID
replacement)
- GCS_EXTERNAL_BUCKET_URL (it has TEST_SF_TF_GCS_EXTERNAL_BUCKET_URL
replacement)
- SKIP_EMAIL_INTEGRATION_TESTS (not used)
- SKIP_EXTERNAL_TABLE_TEST (not used anymore)
- SKIP_NOTIFICATION_INTEGRATION_TESTS (not used)
- SKIP_STREAM_TEST (not used anymore)
- SNOWFLAKE_ACCOUNT (we use the config now, this shouldn't be needed)
- SNOWFLAKE_ACCOUNT_SECOND (not used anymore)
- SNOWFLAKE_ACCOUNT_THIRD (not used anymore)
- SNOWFLAKE_PASSWORD (we use the config now, this shouldn't be needed)
- SNOWFLAKE_ROLE (we use the config now, this shouldn't be needed)
- SNOWFLAKE_USER (we use the config now, this shouldn't be needed)

There are three more that should be removed but they should be
approached more carefully:
- SNOWFLAKE_WAREHOUSE (we do not have it in our config, so it is taken
from env; test what happens without setting a warehouse by running the
tests locally with the config similar to the CI one)
- SNOWFLAKE_PORT (we do not have this in our config, let's check if the
tests will work without this - they should not fail)
- SNOWFLAKE_PROTOCOL (we do not have this in our config, there is a
default, it should be fine without it)
sfc-gh-asawicki added a commit that referenced this issue Feb 23, 2024
sfc-gh-asawicki added a commit that referenced this issue Feb 26, 2024
Fix the order of precedence for the provider's config:
- removed fallback to environment variables for the SDK client (we will
address the setup of the client separately with the extraction of the
SDK client from this repository)
- fixed the merge config function to fill out only missing parameters
- added missing tests
- tested the provider setup in an acceptance test
- introduced environment helpers with tests
- started to move env to one place (the rest envs can be moved after
discussing in this PR if we all agree with that direction)
- added migration notes

References: #2242 #2294
@sfc-gh-asawicki
Copy link
Collaborator

Hey @AZenat @PedroMartinSteenstrup @pashamartinenko. We have released the fix as part of v0.87.0 release. Please follow the migration guide during the update. Please confirm that the issue is resolved in the newest version. Thanks!

@PedroMartinSteenstrup
Copy link

I set the variable export SNOWFLAKE_ROLE="SYSADMIN"

I ask Terragrunt to use a hardcoded role

%{if contains(local.provider_roles, "admin_user")}
provider "snowflake" {
  alias                = "admin_user"
  role                 = "USERADMIN"
  account          = "MR-.OE-"
  user                = "tf"
  authenticator = "JWT"
  private_key    = data.vault_generic_secret.snowflake.data["snowflake_tf_snow_key.p8"]
}
%{endif}

Generated provider block honours the hardcoded over the environment variable

provider "snowflake" {
  alias                = "admin_user"
  role                 = "USERADMIN"
  account          = "MR-.OE-"
  user                = "tf"
  authenticator = "JWT"
  private_key         = data.vault_generic_secret.snowflake.data["snowflake_tf_snow_key.p8"]
}

All good on my side ⚡
Thanks!

@sfc-gh-asawicki
Copy link
Collaborator

sfc-gh-asawicki commented Apr 10, 2024

Closing the issue as it was solved and confirmed by @PedroMartinSteenstrup.

@pashamartinenko please open a new one if you still encounter problems using the newest provider version.

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

No branches or pull requests

5 participants