RSDK-9424 - CLI profile support

[stuqdog]

Adds support for API key-based profile login with the CLI.

A profile can be added with viam profiles add --key-id <id> --key <key> --profile-name <name>
A profile can be removed with viam profiles remove --profile-name <name>
A profile can be modified with viam profiles update --key-id <id> --key <key> --profile-name <name>
Existing profiles can be listed with viam profiles list

To specify a profile for a specific command, you can use the top level --profile flag, e.g. viam --profile <my-profile> organizations list will auth to my-profile and list its orgs.

To specify a profile for a shell session, you can set the VIAM_CLI_PROFILE_NAME env var. If no profile is specified with the --profile flag, the value of that env var (if it is set) will be used as a profile. If the --profile flag is set, this will override the env var.

To fall back to default behavior and ignore the env var, the top level --disable-profiles flag will cause profiles to be ignored. It will cause an error if both --disable-profiles and --profile are set.

Example:

[stuqdog]

@thegreatco fyi

[stuqdog]

Note to reviewers: I did not add unit tests. The current testing suite is pretty ill-suited to this; the inject functions don't have access to a cli context and so creating a global args to parse profile is not really feasible. I think the example functionality should be sufficient to show how this works, and the risk of this working incorrectly is fairly low (you can always roll back or just not use a profile). but if there are any concerns about safety here please let me know!

[benjirewis]

Looking good! Super cool feature. Have some initial questions + comments.

[benjirewis]

Why add this alias?

[stuqdog]

When I was testing I found myself waffling between the two and found it a little annoying when the CLI told me one was wrong, so I added the alias for simplicity of use. Definitely not necessary, happy to remove if you think it's unnecessary cluttering!

[benjirewis]

Seems fine to keep; maybe just a comment above saying it's not for backward compatibility (as most of our aliases are) but for ease of use.

[benjirewis]

So I don't think you need named returns here. Also, is there a reason to return *string instead of just string?

[stuqdog]

Yeah, I went with named returns because I worried that the significance of the return bool was ambiguous. I guess not too big a concern given the limited use here and the fact that it's not exported, I'll remove!

Re: *string, we check on nil-ness in ConfigFromCache. It could certainly be a regular string and we could check on if it's empty or not! I don't have a particular preference, do you think one is preferred?

[benjirewis]

I guess not too big a concern given the limited use here and the fact that it's not exported, I'll remove!

Yeah that comma-ok pattern you're using (value, valueExists) is common enough that I don't think you need the named returns.

Re: *string, we check on nil-ness in ConfigFromCache. It could certainly be a regular string and we could check on if it's empty or not!

I had assumed you would check the bool return value (previously profileSpecified) and assume the string represented a profile only if that bool was true (as opposed to checking if whichProf != nil.) i.e. I think you have enough expressiveness of "existence" through the boolean return value such that the string only needs to be string and not *string. But, no super strong preference, and I may be misunderstanding what you're doing.

[stuqdog]

Ahh, so the string might represent a profile even if profileSpecified is false, if the VIAM_CLI_PROFILE_NAME env var is set. Even when we know we have a profile, we care about whether it was specified via flag or not. specifically, we want to fallback on default behavior if the profile doesn't exist and wasn't specified (maybe the env var was accidentally exported), but not if the profile doesn't exist and was specified (since the user specifically asked for particular behavior, we shouldn't assume they'd be okay with different behavior).

[benjirewis]

Gotcha, maybe a comment above whichProfile explaining exactly what you've said there could be helpful. Thanks for the extra context!

[benjirewis]

LGTM! Up to you if you want to add unit tests; I think if you manually tested, it's fine.