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

command: Enable terraform login command, add manual token support #23995

Merged
merged 1 commit into from Feb 4, 2020

Conversation

alisdair
Copy link
Member

terraform-login

The token generated in the above screen capture has been invalidated 🤫

This is the first in a series of PRs implementing and improving the terraform login feature.

Changes

  • Enable the stubbed-out terraform login sub-command
  • Add fallback detection of the tfe.v2 API (to ensure that we're trying to authenticate against compatible Terraform Cloud/Terraform Enterprise)
  • If login.v1 is supported, use whichever OAuth grant flow is available (note: none of these are supported by TFC/TFE yet)
  • Otherwise, if tfe.v2 is supported, open a browser to the user's token settings page, and wait for a token to be pasted back

var tokenDiags tfdiags.Diagnostics

// Prefer Terraform login if available
if clientConfig != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this if statement is redundant, since we've already exhausted the cases if clientConfig == nil - what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's still needed. The previous if statement doesn't result in an early exit, and only triggers an additional service discovery step. So we could still arrive at this line with a non-nil clientConfig, if the target host supports the login.v1 service.

I don't think we can even move this block up to an else clause, because we want to print diagnostics and exit if neither service is supported. The control flow after these changes is definitely more confusing, but I can't find any way to simplify it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotchya, that's really obvious in hindsight - thanks!

@alisdair alisdair requested a review from a team January 30, 2020 15:25
@alisdair alisdair merged commit f34cba4 into master Feb 4, 2020
@alisdair alisdair deleted the alisdair/terraform-login branch February 4, 2020 16:28
@alisdair alisdair changed the title Enable login subcommand, add manual token support command: enable login subcommand, add manual token support Feb 4, 2020
@alisdair alisdair changed the title command: enable login subcommand, add manual token support command: Enable terraform login command, add manual token support Feb 4, 2020
@ghost
Copy link

ghost commented Mar 29, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@hashicorp hashicorp locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants