re-organize CLI config files

[ahl]

This moves us from a hosts.toml indexed by the "host" used to connect to the Oxide API server to a "profile"-based approach. Profile data will span the config.toml and credentials.toml, the former being primarily user-managed, the latter CLI/auth managed. To use a profile, users will specify --profile XXX; there is a default-profile set in config.toml. Specifying the profile rather than the host will, I expect, be much more user friendly.

Note that this also simplifies the SDK <-> CLI boundary. We add the credentials.toml configuration mechanism to the SDK so that SDK consumers can use that same authentication information. But we move the CLI-specific context information back to the CLI (since it's not really a generally useful mechanism appropriate for the SDK).

Closes #6, #163, #250, #254, #301, #391, #508, #737.

[david-crespo]

We talked about this in DMs, but to sum up: I was skeptical of the need to have the SDK be able to read from the same credentials config files as the CLI, because I am used to SDKs having a more explicit mechanism for specifying credentials. Usually I see env vars plus client constructor params for passing in a token (and in our case a host). The TS and Go SDKs already have the constructor params. However, seeing that both the GCP and AWS client libraries support authing from the credentials set up by their respective CLIs, I concede it's probably useful and users may expect this from us.

So, my suggestions were:

• Let's make sure we explain the different ways of authing and how to opt in or out
• Add explicit host and token params to the SDK client constructor so the user can take matters fully into their own hands if they want (and if these params are used, do not fall back to any other mechanism)

[ahl]

@david-crespo I added the raw token auth method; still more work needed, but thanks for the suggestion.

[ahl]

@karencfv I'll be happy to let you know when this is ready for review.

[augustuswm]

In good news, updating to this version worked without any changes for cassette. It primarily uses Client::new_with_client since it needs to abstract over api tokens and session cookies (I think using session cookies should always require a custom client). The CliConfig struct makes sense to me for a new method of passing credentials.

I'm not sure about opting in to reading files by default. I don't think it is terribly onerous to ask the caller to use Client::authenticated(&CliConfig::default().with_credentials("path-to-credentials")). Or to follow something like GCP, allow the path to be specified by an ENV variable (https://cloud.google.com/docs/authentication/application-default-credentials). With a call opting in Client::authenticated(&CliConfig::default().with_env()).

[ahl]

I'm not sure about opting in to reading files by default. I don't think it is terribly onerous to ask the caller to use Client::authenticated(&CliConfig::default().with_credentials("path-to-credentials")). Or to follow something like GCP, allow the path to be specified by an ENV variable (https://cloud.google.com/docs/authentication/application-default-credentials). With a call opting in Client::authenticated(&CliConfig::default().with_env()).

I hear you, but I really wanted to make it easy to build a custom consumer. For example, if we know where creds are kept, why not make that the default? Perhaps put another way, why unintentional use might we see happening that would create friction for users of the SDK (or CLI)?

[ahl]

@karencfv and @david-crespo this is ready for your review. I see this as the foundation to fix a few outstanding issues:

• 6

• 25 by having output format specified in config.toml

• 163 (more or less obviates that)

• 250

• 255 (now that Context is cleaned up)

• 254

• 301

• 391

• 508

• 567

....etc

[david-crespo]

Maybe you could split the difference by making it opt-in (through a special constructor or whatever) to read from the default location, but you don’t have to give a path — it would know.

I admit that the Bad Thing (reading from the CLI config files when you don’t want to) is relatively unlikely, because if someone wants to specify creds they will probably do so and override that default behavior. It still feels a little weird to me for a library (as opposed to a CLI) to do something like that.

[ahl]

Maybe you could split the difference by making it opt-in (through a special constructor or whatever) to read from the default location, but you don’t have to give a path — it would know.

I think a common use case is going to be 1. oxide auth login 2. build a custom tool that uses those creds. I could have a ClientConfig::new() that didn't include defaults... but that doesn't strike me as easier or clearer. What problem would we see explicit opt-in as solving?

[augustuswm]

I do agree that using the CLI to create a token and then using the SDK is a common use case today. Some of my hesitance is that the current CLI authentication feels more of a result of a limitation in the product (i.e. device OAuth is the only method to create tokens). It also add profiles as a SDK concept, which then asks how can profiles be managed via the SDK instead of files (though we could add this later).

It may also just be me being paranoid, but something doesn't feel correct about an SDK that reads all of the credentials in a config file (even if AWS does it) when they may not be relevant to the executing program. I think Google handles this model a lot better, in that it requests that the user explicitly create an Application Default Credentials file (via a CLI command) which encodes a single credential. Their SDKs then load this file from an expected location.

[ahl]

It may also just be me being paranoid, but something doesn't feel correct about an SDK that reads all of the credentials in a config file (even if AWS does it) when they may not be relevant to the executing program.

I confess that I don't fully understand your concern. If you're building a SDK consumer for your own use, requiring more steps (or, pragmatically, probably just a larger copy-paste) seems onerous. If you're using an SDK consumer that someone else built... well they could easy write the code to consume CLI config files for authentication.

The GCP model you describe sounds like more of a pain in the neck, but if you think we should go down that path let's discuss. I'd note that the SDK today basically pulls data out of the hosts.toml file.

[augustuswm]

Initial run through of basic commands with the CLI seemed to work (mostly just reads). Generally worked as expected. Nicely handled having two different silos with the same name Two minor suggestions:

[oxide.rs] new-config : oxide system networking bgp status
no authenticated hosts; use oxide auth login to authenticate

Maybe a newline here between the statements? or something else to make oxide auth login stand out.

Was there a specific reason to make the profile flag non-global? I initially placed it at the end of the call:

[oxide.rs] new-config : oxide system networking bgp show-status --profile oxide2
error: unexpected argument '--profile' found

Usage: oxide system networking bgp show-status

For more information, try '--help'.

expecting behavior like the AWS CLI. Given that I know the behavior of clap it was easy enough to realize that I should move it to the front, though that may not be common knowledge.

[ahl]

@augustuswm

Was there a specific reason to make the profile flag non-global? I initially placed it at the end of the call:

I actually tinkered with that, but the docs put --profile under every subcommand which I thought was distracting.

[david-crespo]

We should be able to fix that in the docs somehow. I think the UX benefit of being able to put it anywhere is important. I would be absolutely baffled by that error. Think we could get a global property into the docs JSON? Then we could decide in the docs to either hide it or give its own section at the bottom. Could add that in https://github.com/oxidecomputer/oxide.rs/pull/725.

Edit: https://docs.rs/clap/latest/clap/struct.Arg.html#method.is_global_set

[ahl]

If I missed anything from the review above, please file as an issue and I'll attend to it.