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

Crash when MFA used on 2 providers #2681

Closed
Oracen opened this issue Apr 8, 2024 · 15 comments
Closed

Crash when MFA used on 2 providers #2681

Oracen opened this issue Apr 8, 2024 · 15 comments
Labels
bug Used to mark issues with provider's incorrect behavior category:provider_config

Comments

@Oracen
Copy link

Oracen commented Apr 8, 2024

Terraform CLI and Provider Versions

Terraform v1.7.5
on linux_amd64
+ provider registry.terraform.io/hashicorp/null v3.2.2
+ provider registry.terraform.io/snowflake-labs/snowflake v0.87.3-pre

Terraform Configuration

terraform {
  required_providers {
    snowflake = {
      source  = "Snowflake-Labs/snowflake"
      version = "0.87.3-pre"
    }
  }
}

provider "snowflake" {
  alias         = "security"
  account       = "account"
  user          = var.snowflake_username
  password      = var.snowflake_password
  role          = "SECURITYADMIN"
  authenticator = "UsernamePasswordMFA"

  client_request_mfa_token = true
}


provider "snowflake" {
  alias         = "system"
  account       = "account"
  user          = var.snowflake_username
  password      = var.snowflake_password
  role          = "SYSADMIN"
  authenticator = "UsernamePasswordMFA"

  client_request_mfa_token = true
}

Also requires a whole application's definition. You need a non-trivial number of resources to trigger this race condition. The key is having both client_request_mfa_token = true

Expected Behavior

Provider should cache MFA response to prevent hundreds of Duo messages. Each provider should independently store the provider.

Actual Behavior

Plan operation crashes, stack trace attached. I assume this is providers in child modules not namespacing credentials properly, or some kind of operation that needs a Mutex guard.

Steps to Reproduce

  1. terraform plan

How much impact is this issue causing?

Medium

Logs

https://gist.github.com/Oracen/2b818c2d25dabe7bffbd366176b39fea

Additional Information

Workaround for now is to use accounts without 2FA for deployment of our core Snowflake architecture. This will work short term but is obviously undesirable.

@Oracen Oracen added the bug Used to mark issues with provider's incorrect behavior label Apr 8, 2024
@Oracen
Copy link
Author

Oracen commented Apr 8, 2024

Having dug into this a bit deeper, I don't know if this is an issue with the provider or the underlying Go Snowflake library. In Python they handle multiple reads/writes to the temp credential cache using semaphores. The Go stacktrace code appears to be hitting the same file, but I don't know if the issue is in how the TF provider accesses the secret or how Go handles access to the filesystem.

@sfc-gh-asawicki
Copy link
Collaborator

Hey @Oracen. Thanks for reaching out to us.

From the logs provided it seems more like a bug in the underlying gosnowflake driver library. Please file a ticket in https://github.com/snowflakedb/gosnowflake.

@sfc-gh-dszmolka
Copy link
Contributor

hi folks - so the issue is handled over there in gosnowflake so please head to the linked issue for progress tracking; I'll update that one with the progress.

For any viable workaround, I'm not sure if we have any, if your operations on the (tons of) resources cannot complete due to the race condition on the temporal credential cache file.
Until the underlying issue is fixed, could implementing a (little) sleep every now and then between resources help maybe, to reduce the contention on the same credential cache?

@sfc-gh-asawicki
Copy link
Collaborator

A possible workaround would be also to split the current configuration into two separate workspaces so that only a single provider is used in the given run. It may require some work, though.

@sfc-gh-dszmolka
Copy link
Contributor

short update regarding the timeline and progress: gosnowflake fix merged, and the new version of the driver is expected to be released towards end of April 2024

afterwards, the TF provider also needs to bump the gosnowflake dependency + a new version released which carries the fixed gosnowflake

@sfc-gh-asawicki
Copy link
Collaborator

Thanks, @sfc-gh-dszmolka, for the update and for the fast fix. I will schedule a bump of the version on the provider side.

@Oracen
Copy link
Author

Oracen commented Apr 25, 2024

I'll confirm that splitting into separate TF configs with single providers does work, but may require modifications to your RBAC strategy. If you have a nice divide between resources + policies it works well, with only the additional CICD time + mental overhead as a cost.

If you're blocked, probably worth doing, but the easier way is to be nice to your DBAs and get them to set MINS_TO_BYPASS_MFA= X for a suitable value of X

@sfc-gh-dszmolka
Copy link
Contributor

fix in gosnowflake released with 1.10.0

@sfc-gh-jcieslak sfc-gh-jcieslak mentioned this issue May 10, 2024
1 task
sfc-gh-asawicki added a commit that referenced this issue May 15, 2024
- Bumped gosnowflake driver to the newest 1.10.0 (References #2681)
- Bumped some other terraform-related deps
- Bumped deps with security vulnerabilities
- Changed the dependabot config (it was spamming too much and we were
not looking)
@sfc-gh-asawicki
Copy link
Collaborator

Hey @Oracen @sfc-gh-dszmolka. We have bumped the driver to v1.10.0, we should do the release tomorrow.

@sfc-gh-asawicki
Copy link
Collaborator

Hey @Oracen, we released v0.91.0 yesterday. Please upgrade to the newest version and let us know if this fixes your issues.

@Oracen
Copy link
Author

Oracen commented May 17, 2024

Will do so monday

@sfc-gh-dszmolka
Copy link
Contributor

would be really curious to hear about how it went; did the fix really help ?

@sfc-gh-asawicki
Copy link
Collaborator

Hey @Oracen. Was your problem fixed with the version bump? :)

@Oracen
Copy link
Author

Oracen commented Jun 22, 2024

Yes, it was. Worked like a charm. Appreciate it!

@sfc-gh-asawicki
Copy link
Collaborator

Great to hear that! I am closing the ticket then. Thanks for the help @sfc-gh-dszmolka.

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

4 participants