cli icon indicating copy to clipboard operation
cli copied to clipboard

[Request]: better behavior when defaultaccount is not set

Open jeffallen opened this issue 1 year ago • 6 comments

Your request

I managed to get my exoscale.toml file like this, I'm not sure how. Note the empty defaultaccount.

defaultaccount = ""

[[accounts]]
  account = "clusterF-play"
  defaultZone = "bg-sof-1"
  environment = ""
  name = "clusterF-play"
  (secret stuff removed)

As a result, I could not get exo to run at all, it only gave default account not defined, and it was very difficult to know what to do to get things working again. While tracing the code to find out what was going wrong, I found that this little change makes it recover silently from this situation, by taking the first account instead of dying with a borderline useless message:

diff --git a/cmd/root.go b/cmd/root.go
index 161c0515..690b2948 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -262,7 +262,11 @@ func initConfig() { //nolint:gocyclo
        }

        if config.DefaultAccount == "" && gAccountName == "" {
-               log.Fatalf("default account not defined")
+               if len(config.Accounts) == 0 {
+                       log.Fatalf("default account not defined")
+               } else {
+                       config.DefaultAccount = config.Accounts[0].Name
+               }
        }

        if gAccountName == "" {

With that change in place, I was able to use "exo config set clusterF-play" to get my default account set again.

If you'd like this proposed change in the form of a PR, I can do that. I just wanted to quickly jot down what situation I got myself in and how I got out, so I don't forget in case it happens again!

jeffallen avatar Jan 15 '24 10:01 jeffallen

Hey, thank you for the suggestion! I don't think this is a good idea though, imagine a user having two accounts one for critical infra and one for testing and development. Let's say that user has two resources named the same way in both accounts and spontaneously decides to delete one of them in the non-critical account. They forget to specify the account parameter and unfortunately the order of the accounts is such that the account with critical infra is first. So based on that random order he has now deleted something in his critical infra by accident.

But there are certainly things we can improve here. I can think of two things:

  1. Do you think the error message needs to be improved?
  2. How did you arrive at a configuration file in this state? When you add an account through the exo config add command it should set the new account as the default. Maybe we have to improve some documentation somewhere.

sauterp avatar Jan 15 '24 11:01 sauterp

Ok, definitely not a good plan to default to the first one, then.

Let me try and figure out how I got the blank default. I recall just sort of accepting the defaults from "exo config" without really paying close attention, and was then surprised to find myself completely blocked by a config file that was supposed to be completely initialized to defaults.

If the error message had been something like "the default account in file %v is empty" I would have been able to fix it much faster. In fact, I really struggled to find the config file at all, because I was on Windows, and it was not clear where to look for it.

jeffallen avatar Jan 17 '24 09:01 jeffallen

Thanks for this feedback! There is definitely room for improvement here and we'll get on it.

I went through the exo config add command flow just now and noticed this line "[+] Set [] as default account? [yN]:". because "N" is a capital letter it's the default option if the user just hits enter, so it won't set the account as a default account. I still think this behaviour is reasonable, because setting the default account should be a conscious decision, but I think we should output a clear warning after the config add command if there is no default account set.

sauterp avatar Jan 17 '24 10:01 sauterp

Here's what happened. I started with no exoscale.toml file at all. I ran "exo compute instance list" and it reminded me to run "exo config" to get started. Ok so far.

Then this:

PS C:\Users\jalle\Desktop\exoscale-clusterF> .\exo config
No Exoscale CLI configuration found

In order to set up your configuration profile, you will need to retrieve
Exoscale API credentials from your organization's IAM:

    https://portal.exoscale.com/iam/api-keys

[+] API Key [none]: EXOd860da045965539ca290aa55
[+] Secret Key [none]: xxxxx
[+] Account name [none]: clusterF-play
[+] Name [none]: <-- here I hit return to accept the default, figuring, "whatever, it's the same as the account name"

Then it had me select my default zone, which went fine. The resulting config file was:

defaultaccount = ""

[[accounts]]
  account = "clusterF-play"
  defaultZone = "bg-sof-1"
  environment = ""
  key = "EXOd860da045965539ca290aa55"
  name = ""
  secret = "xxx"

So, when starting from zero, and choosing not to give a name, then you end up in this "no default account" situation. I think I would have preferred that the default name was the same as the account name.

Thanks for listening to my feedback. I admit I was being a lazy user, having come back to the platform after using it in the past and being eager to "just go for it", trusting the defaults a bit too much, maybe!

jeffallen avatar Jan 17 '24 10:01 jeffallen

Thank you for these details, this is definitely not optimal and we will improve it. Most users actually are "lazy" and that's OK, we should make sure our tooling does what most users expect by default. We value your feedback!

sauterp avatar Jan 17 '24 13:01 sauterp

To restart on mac, just delete the file /Users/youruser/Library/Application\ Support/exoscale/exoscale.toml

col-panic avatar Mar 01 '24 16:03 col-panic

We now print a reminder after the config commands if no default account has been set. https://github.com/exoscale/cli/pull/593 With that I'll close this. Happy to reopen the issue if there are more unclarities. Thanks~

sauterp avatar Apr 23 '24 08:04 sauterp