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

Connection caching like in the python driver #486

Closed
sfc-gh-pkommini opened this issue Oct 14, 2021 · 13 comments
Closed

Connection caching like in the python driver #486

sfc-gh-pkommini opened this issue Oct 14, 2021 · 13 comments
Labels
enhancement The issue is a request for improvement or a new feature

Comments

@sfc-gh-pkommini
Copy link

sfc-gh-pkommini commented Oct 14, 2021

Issue description

Hi Team,

We use the terraform snowflake provider implemented by CZI and it's pretty much unusable with browser_auth due to every terraform plan and terraform apply creating a new browser tab for authentication.

This issue is due to the underlying dependency on the gosnowflake module which doesn't cache a connection.

It would be great if we could implement connection caching using secure-local-storage as implemented in the python connector?

This allow the snowflake terraform provider to use this module without opening 100s of tabs when doing CRUD for 100s of resources.

@sfc-gh-kterada sfc-gh-kterada added the enhancement The issue is a request for improvement or a new feature label Oct 21, 2021
@sfc-gh-afedorov
Copy link

also tracked in SNOW-505514

@github-actions github-actions bot closed this as completed Jul 1, 2022
@sfc-gh-jfan sfc-gh-jfan reopened this Jul 1, 2022
@github-actions github-actions bot closed this as completed Jul 2, 2022
@sfc-gh-jfan sfc-gh-jfan reopened this Jul 6, 2022
@larspettermadsstuen
Copy link

any progress on this?

@sfc-gh-dszmolka
Copy link
Contributor

#661 should implement this if i'm correct; which is released with 1.6.15

Add MultiFactor Authentication mechanism and caching for MFA/Id token.
Fix issue where 405 error is thrown when S3 bucket acceleration is disabled.

(currently we're at 1.6.16)

@larspettermadsstuen
Copy link

#661 should implement this if i'm correct; which is released with 1.6.15

Add MultiFactor Authentication mechanism and caching for MFA/Id token.
Fix issue where 405 error is thrown when S3 bucket acceleration is disabled.

(currently we're at 1.6.16)

Hey, thanks for the reply 🙌

I just bumped Snowflake-Labs/snowflake to 0.55, which appears to be using 1.6.16.

Running a terraform plan, I still get a browser tab popup for each thread (terraform parallelism parameter). Does this speak to an issue with the fix, or does the terraform provider not make use of the caching properly?

@sfc-gh-dszmolka
Copy link
Contributor

hi @larspettermadsstuen so for the ODBC, JDBC, and Python connectors for this feature to work , one needs to also enable it on the Snowflake side (documentation: Using Connection Caching to Minimize the Number of Prompts for Authentication)
would it be possible to retry this after issuing

use role accountadmin;
alter account set allow_id_token = true;  --if you wish to ever revert: alter account unset allow_id_token;

and see if it reduces the browser popups now? if it's still an issue, we'll look into it.

@larspettermadsstuen
Copy link

Hi, allow_id_token is already set to true in our account.

In any case, this occurs via the snowflake terraform provider, which doesn't use either of the mentioned connectors (?)

@sfc-gh-dszmolka
Copy link
Contributor

no, the provider uses the gosnowflake connector under the hood. we'll take a look what might be wrong with the connection caching behaviour even after 1.6.15, or what am I missing. Thank you for bearing with us while this is investigated!

@larspettermadsstuen
Copy link

👋 Checking in again - wondering if there has been any progress on this issue?

@sfc-gh-dszmolka
Copy link
Contributor

apparently i missed an update here - apologies ! connection/mfa caching for the gosnowflake connector has been implemented with #705 couple of weeks ago, and released with 1.6.17

this resolves the issue from the standpoint of the gosnowflake connector - however for it to take effect in the terraform provider too, i see 2 steps necessary

  • TF provider needs to be rebased to gosnowflake 1.6.17 (currently it's based on 1.6.16 which does not have yet this capability)
  • this should automatically enable connection/mfa caching on Windows and Mac. For Linux, it might be necessary to expose the corresponding configuration settings ClientStoreTemporaryCredential / ClientRequestMfaToken which are not automatically enabled on this OS

but from the perspective of this library, the capability to enable (with 1.6.17) an disable (with upcoming next release) the feature is already there / will be shortly there.

@larspettermadsstuen
Copy link

Thank you!

@sfc-gh-dszmolka
Copy link
Contributor

closing as it has been released with v1.6.17

@mwufigma
Copy link

mwufigma commented May 3, 2023

I don't think the latest version of the provider has been rebased on the v1.6.17 of gosnowflake, is there any workaround I can do at the moment?

@sfc-gh-dszmolka
Copy link
Contributor

this is the gosnowflake driver's repository, and the above comment seems to be about the snowflake terraform provider, so i feel it being in a little inappropriate place ;)

anyways, snowflake terraform provider versions 0.58.1 and up already based on gosnowflake 1.6.17+ . but there seems to be an issue still, which you can follow at Snowflake-Labs/terraform-provider-snowflake#1700

workaround is not to use externalbrowser authentication i'm afraid, but e.g. keypair instead. but i still feel the snowflake terraform provider related discussions should be kept at the appropriate repo if that's possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is a request for improvement or a new feature
Projects
None yet
Development

No branches or pull requests

7 participants