oxidecomputer / terraform-provider-oxide

Oxide Terraform provider
Mozilla Public License 2.0
20 stars 3 forks source link

Authorization via oxide profiles #334

Open karencfv opened 2 months ago

karencfv commented 2 months ago

Overview

The cli now supports a concept of profiles https://github.com/oxidecomputer/oxide.rs/pull/727 . The terraform provider should be able to authenticate with them.

Implementation details

The main provider block should be able take an optional argument to set the profile the user wishes to authenticate with:

provider "oxide" {
  profile = "prod"
}
karencfv commented 2 months ago

Responding to @ahl in https://github.com/oxidecomputer/terraform-provider-oxide/pull/331#discussion_r1714453720

I might have my bit set the wrong way here, but I don't think this should be the domain of the terraform provider? That might be wrong or inconsistent (in that the Rust SDK is profile-aware), but my inclination is that I wouldn't ever put a profile name into a tf config: I'm probably going to check in the tf config to a git repo and I don't think I'd want everyone to be required to have the same profile name. What do you think?

I think there are multiple use cases to account for. For the "deploying to production" use case, yes, I completely agree with you. We shouldn't expect everyone to have the same profile name. For iterating and testing out tf plans, I personally found it useful to use AWS' profile argument https://registry.terraform.io/providers/hashicorp/aws/latest/docs#provider-configuration.

If in the future we decide to have support for something like AWS' credential_process, this would come in handy as well.

All that said, this would be an optional argument, and we wouldn't be removing any previous authentication methods.

ahl commented 2 months ago

If in the future we decide to have support for something like AWS' credential_process, this would come in handy as well.

All that said, this would be an optional argument, and we wouldn't be removing any previous authentication methods.

My inclination on both of those is to do it all in the CLI and let oxide env terraform .. handle the rest -- keep the complexity in the CLI.

karencfv commented 2 months ago

Hm, it appears we may have a bigger decision to make. Will "profiles" be an Oxide first class citizen to be used by any client, or an authentication configuration file to be used solely by the CLI?

I personally would like to see profiles as a first class citizen, a simplified way for our API consumers to manage authentication. But, I realise others may have other views. Perhaps we should bring in @david-crespo and @charliepark into this discussion. I'd like to hear their views.

david-crespo commented 2 months ago

Luckily for you, we already fought this out over whether the Rust SDK should be aware of CLI profiles, and I think we landed on it being first class but not automatic, like you have to opt in.

https://github.com/oxidecomputer/oxide.rs/pull/727#issuecomment-2228796188

karencfv commented 2 months ago

Luckily for you, we already fought this out over whether the Rust SDK should be aware of CLI profiles, and I think we landed on it being first class but not automatic, like you have to opt in.

I read the comment, and maybe I'm misunderstanding, but the comment seems specific to the Rust SDK. My question is a little more general. Do we want to integrate "profiles" as a concept into any client we develop, or should it remain a CLI specific thing? It may be confusing to our users if profiles are a thing in some clients, but not in others?

Tangentially, if we are going to have authorization via profiles in the Rust SDK, perhaps we should do the same with other SDKs for consistency?

karencfv commented 2 months ago

based on that discussion, it seems I should probably bring @augustuswm into the conversation as well

david-crespo commented 2 months ago

I figure any argument that the Rust SDK should use CLI profiles would apply to other clients (except for the weakest argument: that it’s easy because it can share the code with the CLI).

I hadn’t really thought about consistency across clients, but it’s immediately clear that you’re right and its better for us to commit to supporting some consistent baseline profile integration in all SDKs. The basic contract should probably be the simple thing you described above: every client supporting some way to specify a profile name in the client constructor and have it go look for it in the config files.

My inclination on both of those is to do it all in the CLI and let oxide env terraform .. handle the rest -- keep the complexity in the CLI.

Why should the terraform provider be different from the Rust SDK? Wouldn’t it just rely on the Go SDK’s implementation of profile functionality, which should be very straightforward, and which would benefit all Go SDK consumers?

karencfv commented 2 months ago

but my inclination is that I wouldn't ever put a profile name into a tf config: I'm probably going to check in the tf config to a git repo and I don't think I'd want everyone to be required to have the same profile name.

Just adding a little more information on this. Profile names wouldn't necessarily have to be checked in to the git repo. Users can leverage input variables for this use case.

ahl commented 2 months ago

There's not a wrong way here. My inclination is to keep future complexity such as MFA devices, external auth processes, etc. in one place i.e. the CLI. I don't think I'd use profiles in terraform configs based on any of my previous use of terraform, but perhaps there is a compelling use case, and I'd love to hear it. Yes, I'm familiar with tf's input variables.

Why should the terraform provider be different from the Rust SDK? Wouldn’t it just rely on the Go SDK’s implementation of profile functionality, which should be very straightforward, and which would benefit all Go SDK consumers?

I think the question is: why would the terraform provider be different than the CLI? It doesn't have to be! I think it adds complexity to terraform that doesn't need to be there. I don't expect e.g. SDK consumers to directly encode a profile name, but I suppose they could do that too e.g. if one were just building something for a one-off experiment.