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

[Feature]: Consistency for the SNOWFLAKE_AUTHENTICATOR value #2983

Closed
1 task
vludax opened this issue Aug 7, 2024 · 5 comments
Closed
1 task

[Feature]: Consistency for the SNOWFLAKE_AUTHENTICATOR value #2983

vludax opened this issue Aug 7, 2024 · 5 comments
Assignees
Labels
category:provider_config feature-request Used to mark issues with provider's missing functionalities general-usage General help/usage questions

Comments

@vludax
Copy link

vludax commented Aug 7, 2024

Use Cases or Problem Statement

As documented in the migration guide, since version 0.74.0 of the provider, the authenticator property has to be set to "JWT" to use key-pair authentication.

In order to simplify our upgrade process, we would like to make use of the SNOWFLAKE_AUTHENTICATOR environment variable. However, we have discovered that several other connectors, such as python and nodejs expect the value of SNOWFLAKE_AUTHENTICATOR to be "SNOWFLAKE_JWT" rather than "JWT. It also looks like the "SNOWFLAKE_JWT" value is also used by the go SDK.

Category

category:provider_config

Object type(s)

No response

Proposal

Let the terraform provider treat the "SNOWFLAKE_JWT" value the same way it treats "JWT" for the authenticator property.

How much impact is this issue causing?

Medium

Additional Information

No response

Would you like to implement a fix?

  • Yeah, I'll take it 😎
@vludax vludax added the feature-request Used to mark issues with provider's missing functionalities label Aug 7, 2024
@sfc-gh-jmichalak sfc-gh-jmichalak added the general-usage General help/usage questions label Aug 7, 2024
@sfc-gh-jcieslak
Copy link
Collaborator

Hey @vludax 👋
Thanks for reporting the issue. Soon we'll be going through provider configuration rework where we'll address topics such as this one. But until that happens, please continue using JWT for now. Any breaking changes will be documented in the migration guide, but we'll ping you here once the change will be released.

@sfc-gh-jmichalak
Copy link
Collaborator

@vludax You can also use a config like this:

variable "authenticator" {
  type = string

}
provider "snowflake" {
  ...
  authenticator = var.authenticator
}

and run terraform like TF_VAR_authenticator="JWT" terraform refresh.
See https://developer.hashicorp.com/terraform/cli/config/environment-variables#tf_var_name for more details.

@vludax
Copy link
Author

vludax commented Aug 7, 2024

@sfc-gh-jmichalak The reason we wanted to use the SNOWFLAKE_AUTHENTICATOR environment variable was to avoid making additional changes in the provider config. If we need to add authenticator = var.authenticator, we may as well just set authenticator = "JWT" 🙂

sfc-gh-jmichalak added a commit that referenced this issue Nov 7, 2024
<!-- Feel free to delete comments as you fill this in -->
- add remaining fields to the schema
- deprecate `account`
- implement and use a helper function for matching provider versions in
acceptance tests
- use helpers to fill config values
- add acceptance tests for all fields in the config
- move some code to internal package
- improve documentation: describe config hierarchy and provide better
config file examples
- improve and test sdk.MergeConfig
- move mock helper to a separate package because it caused unnecessarily
registered `sqlmock` driver in one of the tests
<!-- summary of changes -->

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested
-->
* [x] acceptance tests
<!-- add more below if you think they are relevant -->
* [x] unit tests

## References
<!-- issues documentation links, etc  -->


#1881

#2145

#2925

#2983

#3104

## TODO
- acceptance test for fields regarding private keys - will be done in
SNOW-1754319
- unskip some tests after creating a compatible config for older
versions
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-jmichalak
Copy link
Collaborator

Hi @vludax 👋

We've released a new v0.98.0 version (release, migration guide). For now, we accept both of these values, but SNOWFLAKE_JWT is the preferred method and JWT is deprecated and will be removed in the future releases.

@vludax
Copy link
Author

vludax commented Nov 12, 2024

Thank you @sfc-gh-jmichalak!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:provider_config feature-request Used to mark issues with provider's missing functionalities general-usage General help/usage questions
Projects
None yet
Development

No branches or pull requests

3 participants