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

Using external auth with ~/.snowflake/config #2145

Closed
erikallmer opened this issue Oct 23, 2023 · 5 comments
Closed

Using external auth with ~/.snowflake/config #2145

erikallmer opened this issue Oct 23, 2023 · 5 comments
Assignees
Labels
bug Used to mark issues with provider's incorrect behavior category:provider_config

Comments

@erikallmer
Copy link

Provider Version
Tested on 0.67.0, 0.73.0 and 0.74.0

Terraform Version
1.2.9

Describe the bug
I'm trying to use the ~/.snowflake/config file instead of using a tfvars file to provide auth settings. However, I was hoping to use browser auth which I have not managed to get working.

My ~/.snowflake/config file looks like this:

[default]
account = "<our Snowflake account name>"
user = "<my email>"
authenticator = "EXTERNALBROWSER"
role = "ACCOUNTADMIN"

With Terraform debug information I see the following error (0.67.0):
[DEBUG] error unmarshalling config file: toml: cannot store TOML string into a Go int

With 0.73.0 this error is instead:
[DEBUG] error unmarshalling config file: toml: cannot decode TOML string into struct field gosnowflake.Config.Authenticator of type gosnowflake.AuthType

Removing the authenticator line also removes this error from the log - however then it obviously complains that a password is not set, which is what I'm trying to avoid.

Expected behavior
The Terraform provider reads my config file and allows me to authenticate using the browser.

Would greatly appreciate any pointers/help!
Thank you!

@erikallmer erikallmer added the bug Used to mark issues with provider's incorrect behavior label Oct 23, 2023
@imre-kerr-sb1
Copy link

I'm not a Go expert, but I did a bit of digging. It seems like this provider tries to use go-toml to directly parse the config file into a gosnowflake.Config.

func loadConfigFile() (map[string]*gosnowflake.Config, error) {
path, err := configFile()
if err != nil {
return nil, err
}
dat, err := os.ReadFile(path)
if err != nil {
return nil, err
}
var s map[string]*gosnowflake.Config
err = toml.Unmarshal(dat, &s)
if err != nil {
log.Printf("[DEBUG] error unmarshalling config file: %v\n", err)
return nil, nil
}
return s, nil
}

Since the authenticator field is an enum (i.e. an int), a string value like "externalbrowser" or "oauth" (which we would like to use) isn't usable.

I don't know if you could use some custom unmarshaling with TextUnmarshaler, or if you would have to parse into an intermediate type that could be converted into a gosnowflake.Config

@sfc-gh-asawicki
Copy link
Collaborator

sfc-gh-asawicki commented Jun 27, 2024

Hey @imre-kerr-sb1. The reason for such behavior is

func MergeConfig(baseConfig *gosnowflake.Config, mergeConfig *gosnowflake.Config) *gosnowflake.Config {
(params should be already translated correctly but they are not used in the merging func).

For now, only a handful of parameters are allowed in the config. We will address this in the upcoming https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#providers-configuration-rework.

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 @erikallmer 👋

We've released a new v0.98.0 version (release, migration guide) with reworked config, which now supports setting authenticator in a TOML file. Please migrate to this version and adjust your configuration files.

@erikallmer
Copy link
Author

Hi @sfc-gh-jmichalak! Thank you! I'm no longer working at the workplace where we had this issue so can't try it out myself 😊 but I'll forward it to my former team!

@sfc-gh-jmichalak
Copy link
Collaborator

Closing due to inactivity.

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