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

Have logins to multiple platforms at the same time #819

Merged
merged 3 commits into from
Oct 17, 2022

Conversation

itowlson
Copy link
Contributor

This follows on from @radu-matei expressing a wish to have Spin retain multiple logins that the user could select between without needing to go through a re-authentication each time.

In this proof of concept, spin login and spin deploy gain an optional --deployment-id argument. spin login --deployment-id saves the login connection info under the given key (instead of in the default config); spin deploy --deployment-id uses the URLs and credentials from the given key.

NOTE: this is predicated on the login refactoring PR. So this PR includes all those commits. But the multiple environments one is just the last commit in here (pretty much just modifying the config_file_path() function to consider a new field).

This POC doesn't really do any error handling, and handles environment choice on a per-command basis rather than switching between environments (so you have multiple saved but only one active at any time). But hopefully demonstrates how simple it should be to build it according to whatever UI we prefer.

ivan@hecate:~/testing/rustbiscuits$ spin deploy
https://canary.platform.fermyon.link/api/registry is responding slowly or not responding...

Deployed rustbiscuits733 version 0.1.0+q24cf147
Available Routes:
  rustbiscuits: https://rustbiscuits733-i6axuunl.canary.platform.fermyon.link (wildcard)

ivan@hecate:~/testing/rustbiscuits$ spin deploy --deployment-id pantoufles
http://bindle.3.91.166.66.sslip.io/v1 is responding slowly or not responding...

Deployed rustbiscuits733 version 0.1.0+q24cf147
Waiting for application to become ready......... ready
Available Routes:
  rustbiscuits: http://spin-deploy.rustbiscuits733.hippo.3.91.166.66.sslip.io (wildcard)

@radu-matei
Copy link
Member

This is looking great!

I realize not all of these are covered by this particular PR, but I imagine this could be a slightly larger refactoring, so I am sharing them here:

  • --deployment-id might be confusing, as it's not related to entity I deployed during spin deploy, but rather to the environment. Perhaps --environment might be more intuitive?

Extra:

  • instead of hippo-url, hippo-username, hippo-password, I think a few of us have been talking about changing them to simply url, username, and password. It's clear what they mean (and in the future, we will most likely proxy Bindle through the Platform in all cases, so bindle-username and bindle-password will not be required).
  • since spin login defaults to using GitHub auth, I think spin login --url should too, unless I provide a username and password pair.

(manually tested with multiple environments and things work great, thank you for working on this!)

long = "deployment-id",
env = DEPLOYMENT_ID_ENV
)]
pub deployment_id: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

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

"Deployment ID" for this is rather unintuitive.
Would you consider something like "environment' instead?

I would associate "deployment ID" with an instance of a deployed application that I am in the process of running spin deploy for, rather than the environment I am logged into.

}

impl DeployCommand {
pub async fn run(self) -> Result<()> {
let path = dirs::config_dir()
.context("Cannot find config directory")?
.join("spin")
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to put this under a fermyon directory instead?
There might be other tools (that we build) that might need to share that configuration file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was just following the existing convention. I don't have a strong opinion either way.

@itowlson
Copy link
Contributor Author

Ha ha, I considered environment which is a term I use in VS Code, but it's one of those terms that's absurdly overloaded already. Completely agree deployment-id is wrong though.

spin login --url defaults to asking for the method of login. I think that's the right thing to do in the general case until we can auto-detect (or until GH login is widely supported), but if at some point we have well-known domains we could default those. Happy to discuss though.

@radu-matei
Copy link
Member

Fair enough on --url asking for the method.

(again, not blocking this PR, could we use the same picker we have in spin new eventually, please?)

@radu-matei
Copy link
Member

Also unrelated. After the deployment is complete, a user sees:

Deployed rustbiscuits733 version 0.1.0+q24cf147
Available Routes:
  rustbiscuits: https://rustbiscuits733-i6axuunl.canary.platform.fermyon.link/ (wildcard)

As we know the link of the platform UI, could we also print that as well?

Something like:

Deployed rustbiscuits733 version 0.1.0+q24cf147
To manage your application, visit <link to UI>

@itowlson itowlson marked this pull request as ready for review October 17, 2022 00:05
@itowlson
Copy link
Contributor Author

All right, this now includes --url / --username / --password which is a breaking change to previous iterations of login. @mikkelhegn @tpmccallum @karthik2804 for docs visibility.

I (slightly reluctantly) added a --list option too, because if VS Code is going to use this rather than an internal system, then it needs a way to display the list. It's pretty rudimentary though!

Signed-off-by: itowlson <ivan.towlson@fermyon.com>
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants