Optum / dce-cli

Disposable Cloud Environment CLI
Apache License 2.0
37 stars 20 forks source link

Provide CLI flags for backend configuration #54

Closed nathanagood closed 4 years ago

nathanagood commented 4 years ago

Is your feature request related to a problem? Please describe.

The new backend will be local by default, and there is a feature request for using the YAML file as a source (see #15 ). This feature is to provide CLI flags for configuring the backend right on the CLI and without having to crack open a YAML file and edit it by hand if the user doesn't want.

Describe the solution you'd like

Suggests for flag names are welcome, and so also is the possibility that perhaps a single arg should be used that accepts a JSON configuration string (like some of the AWS CLI args). We should design this out before writing it to get the best user experience.

Describe alternatives you've considered

Additional context

Something like:

dce system deploy --backend-config="type=s3" --backend-config="bucket=mybucket"

would be pretty consistent with the Terraform configuration, but it feels a little inconsistent and clunky compared to the other CLI args existing.

nathanagood commented 4 years ago

For now, I'm feeling like defaulting to supporting JSON being passed in is the easiest way to an MVP. In other words, something like this:

$ dce system deploy --backend='{"type":"s3", "bucket":"mybucket"}'

Thoughts? @marinatedpork, @eschwartz

nathanagood commented 4 years ago

Make sure to persist configuration in DCE config, along with the vars.

eschwartz commented 4 years ago

I kind of like following Terraform syntax, for terraform-specific config options. For anyone with experience using Terraform, it will make the the CLI easier to understand. And for anyone contributing to dce-cli, it's more obvious what all the flags do behind the scenes.

in other words, Less abstraction == easier to understand.

marinatedpork commented 4 years ago

How about allowing a parameter to simply pass in all TF specific options, thus exposing all TF options to the command line

dce system deploy --tfoptions="--tf-specfic-option1 value1 --tf-specfic-option2 value2 ...."

joshmarsh commented 4 years ago

Having a parameter to pass in TF options could be a good escape hatch for users to do things we haven't anticipated. @eschwartz's suggestion would be better for usability. I would prioritize the former over the latter, but maybe we can do both?

nathanagood commented 4 years ago

Then instead of a backend configuration per se in the config file, we could just store the value of that --tfoptions param?

nathanagood commented 4 years ago

How about allowing a parameter to simply pass in all TF specific options, thus exposing all TF options to the command line

dce system deploy --tfoptions="--tf-specfic-option1 value1 --tf-specfic-option2 value2 ...."

I really like this approach. we seem to be aligned here (@eschwartz's point re: abstraction is also aligned with this). Thoughts on how we handle bad options? Do we show the TF help, or do we error with a message to look at the logs? Or do we build an array of tfoptions we support and pass on?

eschwartz commented 4 years ago

parameter to simply pass in all TF specific options

I'd thought of that, and there's definitely some benefits.

The thing then is that we lose things like cobra-generated help text, autocompletion, cobra param validation, etc. This also wouldn't handle cases where we have functionality that's beyond what TF provides -- for example, if we want to support a backend type flag.

nathanagood commented 4 years ago

Merged into master