Prompt for custom tfcloud host

[Convez]

This is a simplified version of PR #1673 , as requested
This closes #1671 and closes #1505
Allow TFC host selection during login. Hosts already in TFC credentials file are displayed. Options for insertion of new hosts added:

N.B. Logging to a new hostname does not update the TFC credentials file (Maybe to be implemented in the future?)

[jpogran]

Hi @Convez, thanks for the updated PR. I appreciate your patience with the delays in reviewing your original PR and this one. I'm grateful for your continued contributions. The delay is not a reflection of the importance of your contributions, it's a result of new priorities and the need to manage shifting timelines.

For now, with your permission, I can do a couple of things to get some forward movement here. I would like to extract the bug fix for the Terraform credential path on Windows myself into a separate PR and attribute you for it. Then I'll rebase this PR so there is not so many merge commits in it and get it in a ready to review state. My team will be discussing TFE support in our quarterly planning in 2 weeks, after that I will let you know the results of that meeting and how that applies to this PR.

I again thank you for your patience.

[jpogran]

Hey @Convez, I've extracted and merged #1735, which contains your bug fix for the credentials file.

With your permission I can rebase this PR against main and keep i up to date.

[willjprice]

Bump.

[Convez]

Hey @Convez, I've extracted and merged #1735, which contains your bug fix for the credentials file.

With your permission I can rebase this PR against main and keep i up to date.

Yes, of course. Thank you very much. Sorry for the (very) late reply

[jpogran]

Hey @Convez, thanks for your patience. There are/have been several things in the air that either needed to land or be started before I could come back to this. The good news is I am picking this back up now.

I've rebased and made a small commit updating some HCP Terraform renaming, which is minor. Then I have tested it against a test TFE instance that has over 10,000 workspaces, and the good news is it works which was a concern of ours since we didn't load test this extension with TFE workloads in mind. The bad news is I've hit two bugs, but only one is related to this PR.

The one related to this PR is it not remembering the hostname after closing out VS Code. This seems to hit mostly with the temp profile, but did happen with my personal install. I think I found the resolution by storing the hostname along with the token in the secret store. This way it is present when the extension starts again. I still have to test it with expired tokens to make sure all the auth related scenarios still work, but I think that resolves it.

The bug not related to your PR is refreshing the workspace view after loading a significant number of workspaces seems to fail with a duplicate key. I'll resolve that separately.

The next steps is to finish out the testing auth scenarios, then we can go through the review process and get this merged.

[Convez]

Hello @jpogran ,

I see, please let me know if there is anything I can do to help from my side 😄

[ansgarm]

Hey 👋

I just test drove this PR a little and noticed a small inconsistency (which seems to have existed before) when opting to use an existing token from the Terraform CLI.

Steps to reproduce:

1. Have an existing ~/.terraform.d/credentials.tfrc.json file with e.g. credentials for app.terraform.io (doesn't really matter)
2. Login to HCP Terraform
3. Select "New Instance"
4. Put in some endpoint for which we don't have credentials in the TF CLI file
5. Select "Stored user token"
6. There's no error message but we also are not logged in
   (this also happens for the "Default HCP Terraform Instance" login option if there are no credentials for app.terraform.io in the credentials.tfrc.json file)

A similar thing I noticed (that may be related) is that when selecting "Existing user token – Enter a token manually" and then entering an empty string the same behavior seems to happen (i.e. nothing happens, we are not logged in and no message is displayed). However, in this case I could see entering an empty string as a way of aborting (But the input also states "Press Esc to cancel" so there's that 😄)

Other than that, I could successfully log into a different instance 🎉 (I used it to log into the HCP Terraform staging instance for testing as I don't have a TFE instance ready).

Also: Do we need to add some note somewhere that it's only possible to log into one endpoint at a time? When I logged into the staging instance, another VS Code window of mine popped up from the background and told me that I was logged out of HCP Terraform and prompted me to login again 😅 Is this something we need to handle better? Is it already known, @jpogran?

[dbanck]

With an existing session/login present, I'm getting this error on this branch

Steps to reproduce

1. checkout main
2. start the extension
3. login with your HCP Terraform credentials
4. close the extension
5. switch to this branch
6. start the extension again

We might need something like a default fallback to HCP Terraform?

[jpogran]

I just test drove this PR a little and noticed a small inconsistency (which seems to have existed before) when opting to use an existing token from the Terraform CLI.

This was a failure of the token check inside createSession and the session handler. The session handler was supposed to throw an InvalidToken exception that is caught and re-prompts the user. In practice we threw an Error before getting to the session handler, which exited the method without an error (because createSession requires you to return a rejected promise and not throw).

This should be fixed now.

A similar thing I noticed (that may be related) is that when selecting "Existing user token – Enter a token manually" and then entering an empty string the same behavior seems to happen (i.e. nothing happens, we are not logged in and no message is displayed). However, in this case I could see entering an empty string as a way of aborting (But the input also states "Press Esc to cancel" so there's that 😄)

This should be fixed by the above fix, which handles this similar scenario where there isn't a token returned.

Also: Do we need to add some note somewhere that it's only possible to log into one endpoint at a time? When I logged into the staging instance, another VS Code window of mine popped up from the background and told me that I was logged out of HCP Terraform and prompted me to login again 😅 Is this something we need to handle better? Is it already known, @jpogran?

Previously we only supported one endpoint, so there was only one login/token allowed and we would trigger a session refresh in all windows if one window changed. Now that we are accepting custom endpoints, it is a possibility but would require more work than this PR intends to cover. I can ticket this request

[jpogran]

With an existing session/login present, I'm getting this error

We might need something like a default fallback to HCP Terraform?

This is because on load, we only checked if there was a persisted session and not if it had all of the properties we expected. This is fixed by checking to see if the stored session has a hostname, and if not adding the default HCP Terraform hostname then calling the session handler store method. This is a safe assumption because we know all prior sessions were only for HCP Terraform and we can update the stored session accordingly.

[ansgarm]

[dbanck]

Thanks for addressing the hostname issue. It now works fine when starting from an existing login (main).

I added a suggestion to reduce duplication in client creation, but feel free to ignore it.

[dbanck]

I just noticed that I have to select approve because I previously requested changes. Oops

[jpogran]

@Convez this is now merged to main, and will go out with the next release. Thank you for your patience, we really appreciate the contribution!

[github-actions]

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.