intuit / simple_deploy

Maintenance Mode - Simple Deploy is an opinionated CLI tool for managing AWS Cloud Formation Stacks.
MIT License
64 stars 22 forks source link

support for env var and aws temporary credentials #235

Closed thbishop closed 10 years ago

thbishop commented 10 years ago

this enables both support for specifying credentials using env vars as well as for temporary aws credentials.

the behavior is:

does this behavior make sense from a user perspective? what sucks?

bw-intuit commented 10 years ago

@thbishop all makes sense, going to test using it with ENV vars and see how it feels

bw-intuit commented 10 years ago

I went to use it, and got triped up as it over-rode my YALM settings with my env vars and returned data for a different AWS account. Need to think more about it, but I'm wondering if we don't allow people to specify the ENV var in the YAML file rather than have it automatically override?

thbishop commented 10 years ago

@brettweavnet-intuit i think i understand. elaborate on specify the ENV var in the YAML file?

do you mean like this (in the config file)?:

environments:
  preprod_west_2:
    access_key: AWS_ACCESS_KEY_ID
    secret_key: AWS_SECRET_ACCESS_KEY
    secret_token: AWS_SECRET_TOKEN
    region: us-west-2

what if we just specified ENV and used the common aws env vars? less flexible, but fits more with community standards, harder to mess up for users, and easier to code.

environments:
  preprod_west_2:
    access_key: ENV
    secret_key: ENV
    secret_token: ENV
    region: us-west-2
thbishop commented 10 years ago

@brettweavnet-intuit latest update adds support for only reading env vars when specifying ENV in config file

bw-intuit commented 10 years ago

@thbishop update readme?

bw-intuit commented 10 years ago

@thbishop used this approach a bit and it works well. One thing I've noticed is I'll probably never have more than one environment for each region (as there is nothing else which can be configured).

I'm good merging it and moving forwards, but one thought, would it make sense to offer some sort of flag that just read the three envs and region rather than put them in the config file?

For example

simple_deploy --read-from-env

Thoughts?

thbishop commented 10 years ago

@brettweavnet-intuit not sure i fully grok your statement. are you saying, specify --read-from-env instead of passing -e for commands?

bw-intuit commented 10 years ago

@thbishop Yes, or some flag. What I noticed is that when I was adding my creds to the yaml file, I basically just have a entry per region that points to ENV. Since nothing is different per environment, what is the value of reading it from there? Would it be cleaner just to add a flag that would read those env variables, as well as AWS_REGION and make that call rather than maintain that in the config file? Customers could do a -e ENV or -r (or something to read from env vars).

Make sense?

thbishop commented 10 years ago

@brettweavnet-intuit yep, i follow. the original reason for naming the envs was to be able to have a friendly name associated with otherwise opaque items (i.e. credentials). so i think with the "read from env" approach, we lose some of that. that being said, you pretty much lose it anyway if your config file ends up being nothing but ENV for the values. i'm good giving the "read from env" approach a shot

bw-intuit commented 10 years ago

@thbishop exactly, I don't see it being anything except read from ENV as we use roles. Give it a shot, I think it will be cleaner, but would like the bishop feedback.

thbishop commented 10 years ago

@brettweavnet-intuit yeah. it somewhat just pushes the issue up to whatever is calling/coordinating the env vars.

thbishop commented 10 years ago

closing in favor of https://github.com/intuit/simple_deploy/pull/236/files