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

Read a refresh token from accessTokens.json #39

Closed
wants to merge 1 commit into from

Conversation

@mikhailshilkov
Copy link

mikhailshilkov commented Sep 5, 2019

Resolves #22.

When Azure CLI authentication is used, there's no refresh token returned from the CLI. Because of that, long-running operations tend to fail.

As a workaround, try reading the refresh token from ~/.azure/accessTokens.json (or whatever other Azure folder is configured) and finding the refresh token by access token.

@hashicorp-cla

This comment has been minimized.

Copy link

hashicorp-cla commented Sep 5, 2019

CLA assistant check
All committers have signed the CLA.

@Moeser

This comment has been minimized.

Copy link

Moeser commented Sep 5, 2019

Looks good to me 👍 . I'm happy to see you also honored the AZURE_CONFIG_DIR environment variable.

@mikhailshilkov

This comment has been minimized.

Copy link
Author

mikhailshilkov commented Sep 6, 2019

I validated the change by rebuilding the terraform-provider-azurerm provider and provisioning a CosmosDB account with 6 regions, which always raised the token error for me. It ran fine for 1 hour and failed with another unrelated error. So it looks like the change works :)

@Moeser

This comment has been minimized.

Copy link

Moeser commented Sep 6, 2019

@tombuildsstuff or @katbyte any feedback here?

@pessimism

This comment has been minimized.

Copy link

pessimism commented Oct 3, 2019

Any ETA on getting this merged?

@mikhailshilkov mikhailshilkov force-pushed the mikhailshilkov:gh-22 branch from ef4b18b to 8b17192 Oct 7, 2019
@tombuildsstuff

This comment has been minimized.

Copy link
Member

tombuildsstuff commented Oct 7, 2019

hi @mikhailshilkov

Thanks for this PR - apologies for the delayed response here!

Whilst we appreciate this contribution unfortunately we're unable to use this approach since it won't consistently work in all circumstances (the tokens in the accessTokens.json` will contain an access token, but there's no guarantee it contains a valid refresh token) - as such this'd lead to harder to diagnose errors for users.

We have a fix planned which will resolve this, however this requires some external coordination to achieve and unfortunately we're unable to give a timeframe for that. Whilst this PR would resolve the error when the Azure CLI contains a refresh token as mentioned above unfortunately that's not the case in all circumstances - as such whilst I'd like to thank you for this contribution I'm going to close this PR for the moment.

Thanks!

@mikhailshilkov

This comment has been minimized.

Copy link
Author

mikhailshilkov commented Oct 7, 2019

So many people are frustrated by the cryptic error of the missing refresh token that it's hard for me to see how fixing this in 99% of Azure CLI users would make it harder to diagnose.
Good luck with the better fix, I hope it won't take another year.

@pessimism

This comment has been minimized.

Copy link

pessimism commented Oct 7, 2019

Imagine demonstrating Terraform to senior executives in your organization for potential adoption, only to have it crash with a cryptic error partway through, and the only explanation you can provide is that it is a known, solvable error which the vendor has ignored for nine months, has no ETA on a fix for and outright rejected a community provided (partial) solution. I am sure there are greater factors at play here but it is indeed a frustrating scenario.

@easkay

This comment has been minimized.

Copy link

easkay commented Oct 7, 2019

I'm very disappointed to see this closed, as this is causing me issues on pretty much a daily basis with several of Azure's long-provisioning-time resources (e.g. APIM). I really hope this gets fixed soon.

@tombuildsstuff Can you give any details of any issues or PRs which have been raised in other repos as part of fixing #22?

@choovick

This comment has been minimized.

Copy link

choovick commented Oct 8, 2019

@tombuildsstuff where can we track progress on that planned fix?
I was also hoped to relief my users with long running resources. Currently we have to az logout, followed by az login to obtain fresh token valid for an hour and hope that there will be no delay with resource creation on azure side. Alternatively we have to switch to service principal authentication, but that complicates permission configuration on our end.

@techspeque

This comment has been minimized.

Copy link

techspeque commented Oct 17, 2019

I agree with all the latest comments, it is rather difficult to manage cryptic errors such as this one. AKS alongside of previously mentioned HDI cluster is one of the resources that is taking a long time to create. I wonder if that error occurs with any other long-running jobs or is it down to creation of a single resource only? Also, does anyone have a working workaround?

@easkay

This comment has been minimized.

Copy link

easkay commented Oct 17, 2019

As @choovick said:

Currently we have to az logout, followed by az login to obtain fresh token valid for an hour and hope that there will be no delay with resource creation on azure side.

I've found that to be alright as a workaround, just a massive pain.

@AHelper

This comment has been minimized.

Copy link

AHelper commented Oct 17, 2019

Another (very ugly) workaround is to expire all of your access tokens before starting Terraform. The az cli will refresh the token without the login flow and get a new 1 hour access token:

$tokens = Get-Content $Home\.azure\accessTokens.json | ConvertFrom-Json
foreach ($token in $tokens) { $token.expiresOn = "1970-01-01 00:00:00.000000" }
$tokens | ConvertTo-json | Set-Content $Home\.azure\accessTokens.json
``
@cicorias

This comment has been minimized.

Copy link

cicorias commented Dec 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

10 participants
You can’t perform that action at this time.