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

[Bug]: Behaviour change bundle 2024_08 breaks the user resource #3125

Closed
1 task done
Relativity74205 opened this issue Oct 10, 2024 · 8 comments
Closed
1 task done
Labels
bug Used to mark issues with provider's incorrect behavior

Comments

@Relativity74205
Copy link
Contributor

Terraform CLI Version

0.96.0

Terraform Provider Version

1.8.4

Terraform Configuration

terraform {
  required_version = ">= 1.2.9"
}

terraform {
  required_providers {
    snowflake = {
      source  = "Snowflake-Labs/snowflake"
      version = "0.96.0"
    }
  }
  backend "http" {}
}

provider "snowflake" {
  user          = var.USER
  private_key   = file(var.SSH_KEY_SNOWFLAKE_TF_P8_KEY_FILE)
  account       = var.account
  role          = "PUBLIC"
  authenticator = "JWT"
}

provider "snowflake" {
  alias         = "security"
  user          = var.USER
  private_key   = file(var.SSH_KEY_SNOWFLAKE_TF_P8_KEY_FILE)
  account       = var.account
  role          = "SECURITYADMIN"
  authenticator = "JWT"
}

provider "snowflake" {
  alias         = "sys"
  user          = var.USER
  private_key   = file(var.SSH_KEY_SNOWFLAKE_TF_P8_KEY_FILE)
  account       = var.account
  role          = "SYSADMIN"
  authenticator = "JWT"
}

provider "snowflake" {
  alias         = "account"
  user          = var.USER
  private_key   = file(var.SSH_KEY_SNOWFLAKE_TF_P8_KEY_FILE)
  account       = var.account
  role          = "ACCOUNTADMIN"
  authenticator = "JWT" # only needed because of terraform provider bug. May be removed in future.
}

provider "snowflake" {
  alias         = "user"
  user          = var.USER
  private_key   = file(var.SSH_KEY_SNOWFLAKE_TF_P8_KEY_FILE)
  account       = var.account
  role          = "USERADMIN"
  authenticator = "JWT"
}

Category

category:resource

Object type(s)

resource:user

Expected Behavior

No terraform plan errors for the user resource with behaviour change bundle 2024_08.

Actual Behavior

When activating the behaviour change bundle 2024_08 on our dev environment, we get the following errors when running terraform plan:

 Error: sql: Scan error on column index 14, name "default_namespace": converting NULL to string is unsupported
 
   with module.etl_user-exaflake.snowflake_user.user,
   on ../modules_components/technical_user/main.tf line 7, in resource "snowflake_user" "user":
    7: resource "snowflake_user" "user" {

Steps to Reproduce

  1. Create a user with some fields unset (e.g. default_namespace) with terraform
  2. Activate the bevaiour change bundle 2024_08 on an account: SELECT SYSTEM$ENABLE_BEHAVIOR_CHANGE_BUNDLE('2024_08');
  3. Run terraform apply

How much impact is this issue causing?

High

Logs

No response

Additional Information

The error is most probably the following change: https://docs.snowflake.com/en/release-notes/bcr-bundles/2024_08/bcr-1798
This causes the output of the SHOW USERS command to change:

  • Without the bundle:
    • default_namespace is an empty string
  • With the bundle
    • default_namespace is NULL

When setting the default_namespace for the user to a value, the same error happens then with other fields, e.g. ext_authn_uid or mins_to_unlock.

I would like to implement a fix, however, I haven't found so far the place in the code, which causes the error.

Would you like to implement a fix?

  • Yeah, I'll take it 😎
@Relativity74205 Relativity74205 added the bug Used to mark issues with provider's incorrect behavior label Oct 10, 2024
@Relativity74205
Copy link
Contributor Author

Perhaps this bug was already fixed by #3119 by @sfc-gh-asawicki...

Perhaps I will have some time in the next days to go with the debugger through the code to find the correct place. However, perhaps some of you guys knows it directly.

@sfc-gh-asawicki
Copy link
Collaborator

Hey @Relativity74205. Thanks for reaching out to us.

AFAIK the bundle will be enabled by default in January, so we still have some time to address this. We should be able to adjust provider logic with this breaking change next week. Until then, do not enable the bundle while using the provider.

@simonepm
Copy link

The issue is also on field 'ext_authn_uid' and all the field becoming "" -> null in bundle 2024_08

The problem for default_namespace can be solved by setting a default namespace in bundle 2024_07 and then activating bundle 2024_08 only afterward.

Unfortunately for ext_authn_uid there is not solution as is a read-only parameter!

@Relativity74205
Copy link
Contributor Author

@sfc-gh-asawicki
I think I found and fixed the problem, here is the PR: #3144. Can you please have a look? If this is fine, it would be great, if it could be merged for the next release.
In fact, we would like the activate the breaking change bundle as soon as possible, as we need one of the features in it (python 3.11 for streamlit).

sfc-gh-asawicki added a commit that referenced this issue Nov 7, 2024
sfc-gh-asawicki added a commit that referenced this issue Nov 8, 2024
Apply various fixes:
- Fix handling compute pool privileges (#2717)
- Fail to reproduce the problem with password policy user attachment
(#3005)
- Adapt user to BCR Bundle 2024_08 (#3125)
- Loosen identifier validations - parentheses (#3127) - check below
- Prove MANAGE SHARE TARGET works correctly (#3153)

On the identifier validation topic:
ParseIdentifierString should generally allow parentheses. It should
validate them for the identifiers for functions, procedures, etc.
Because of that:
- this validation was removed
- method usages were analyzed to check what consequences it has
throughout the provider
  - DecodeSnowflakeAccountIdentifier - OK, account level identifier
  - DecodeSnowflakeParameterID
- buildOptsForGrantsOn (grants datasource) - NOK, had to fix the logic
- ContainsIdentifierIgnoringQuotes - OK, transitively used only in
network policies
    - TestDecodeSnowflakeParameterID - OK
    - IsValidIdentifier - OK, used for other identifier types
- pkg/resource - OK, used in streams, table constraints and tag masking
policy associations
  - suppressIdentifierQuoting
- used in non-grant resources with non-argument identifier types - OK
- used in grant resources - OK, the validation will be relaxed for now,
diff suppression won't work correctly for the identifiers with
arguments, will be addressed with functions/procedures rework
(multi-field validation could be handled for such cases, issue added;
references:
hashicorp/terraform-plugin-sdk#354,
hashicorp/terraform-plugin-sdk#233)
- suppressIdentifierQuotingPartiallyQualifiedName - as above; currently
used only for streams
- parseIdentifier - used by other identifier types (type constraints
added)
- ParseObjectIdentifierString - OK, used for other identifier types
(ParseSchemaObjectIdentifierWithArguments is dedicated for identifier
with arguments)
- ParseSchemaObjectIdentifierWithArguments - OK, we split the input
string on first opening paren (so there are no other opening parens
there)
- Test_ParseIdentifierString - tests adjusted for the removed validation

Others:
- Remove unused privileges.go file
- Fix preview resources list for V1

References:
-
#2717
-
#3005
-
#3125
-
#3127
-
#3153
sfc-gh-jmichalak pushed a commit that referenced this issue Nov 8, 2024
##
[0.98.0](v0.97.0...v0.98.0)
(2024-11-08)

Feature scope readiness for V1:
[link](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/v1-preparations/ESSENTIAL_GA_OBJECTS.MD)
([Roadmap
reference](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#wrap-up-the-functional-scope)).
:exclamation: Migration guide: [v0.97.0 ->
v0.98.0](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0970--v0980)

### 🎉 What's new
- New resources:
- authentication_policy
([#3098](#3098)),
references
[#2880](#2880)
- external_volume
([#3106](#3106)),
partially references
[#2980](#2980)
- stream_on_directory_table
([#3129](#3129))
- stream_on_view
([#3150](#3150))
- primary_connection, secondary_connection
([#3162](#3162))
- secret_with_basic_authentication, secret_with_generic_string,
secret_with_oauth_authorization_code_grant,
secret_with_oauth_client_credentials
([#3110](#3110)),
([#3141](#3141))
- New data sources:
- connections
([#3155](#3155)),
([#3173](#3173))
- secrets
([#3131](#3131))
- Reworked:
- provider configuration hierarchy
([#3166](#3166)),
references
[#1881](#1881),
[#2145](#2145),
[#2925](#2925),
[#2983](#2983),
[#3104](#3104)
- provider configuration fields
([#3152](#3152))
streams data source
([#3151](#3151))
- SDK upgrades:
- Upgrade tag SDK
([#3126](#3126))
- Recreate streams when they are stale
([#3129](#3129))
### 🔧  Misc
- Add object renaming research summary
([#3172](#3172))
- Test support for object renaming
([#3130](#3130)),
([#3147](#3147)),
([#3154](#3154))
- Add tests to issue
[#3117](#3117)
([#3133](#3133))
- New roadmap entry
([#3158](#3158))
- Test more authentication methods
([#3178](#3178))
- Minor fixes
([#3174](#3174))
### 🐛  Bug fixes
- Apply various fixes
([#3176](#3176)),
this addresses BCR 2024_08, references
[#2717](#2717),
[#3005](#3005),
[#3125](#3125),
[#3127](#3127),
[#3153](#3153)
- Connection and secret data sources tests
([#3177](#3177))
- Fix grant import docs
([#3183](#3183)),
resolves
[#3179](https://github.com/Snowflake-Labs/terraform-provider-snowflake/discussions/3179)
- Fix user resource import
([#3181](#3181))
- Handle external type changes in stream resources
([#3164](#3164))
- Do not use OR REPLACE on initial creation in resources with
copy_grants
([#3129](#3129))
- Address issue
[#2201](#2201)
by introducing new stream resources

Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>
@sfc-gh-asawicki
Copy link
Collaborator

Hey @Relativity74205 @simonepm.

We've released a new v0.98.0 version (release, migration guide) with a fix. Please check it out :)

@Relativity74205
Copy link
Contributor Author

@sfc-gh-asawicki We updated/migrated our dev and intergration accounts without problems and haven't encountered any problems so far. Thanks for your work!

@sfc-gh-asawicki
Copy link
Collaborator

Glad to hear that! Should I close the issue then or do you want to do some more testing?

@Relativity74205
Copy link
Contributor Author

I think the issue can be closed now. If we find something, we can reopen/create a new one. Thanks for the fast implementation of the fix.

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

3 participants