Accenture / adop-docker-compose

Talk to us on Gitter: https://gitter.im/Accenture/ADOP
https://accenture.github.io/adop-docker-compose
Apache License 2.0
765 stars 574 forks source link

Reading variables from ~/.aws/config and ~/.aws/credentials is broken #71

Open oscarrenalias opened 8 years ago

oscarrenalias commented 8 years ago

If data is present in either ~/.aws/config or ~/.aws/credentials, whatever bash magic is happening in the scripts isn't working. I enabled debug mode in the scripts by adding #!/bin/bash -ex at the top to see what's going on, and this is what I get:

+ echo ' 
      ###    ########   #######  ########  
     ## ##   ##     ## ##     ## ##     ## 
    ##   ##  ##     ## ##     ## ##     ## 
   ##     ## ##     ## ##     ## ########  
   ######### ##     ## ##     ## ##        
   ##     ## ##     ## ##     ## ##        
   ##     ## ########   #######  ##        
'

      ###    ########   #######  ########  
     ## ##   ##     ## ##     ## ##     ## 
    ##   ##  ##     ## ##     ## ##     ## 
   ##     ## ##     ## ##     ## ########  
   ######### ##     ## ##     ## ##        
   ##     ## ##     ## ##     ## ##        
   ##     ## ########   #######  ##        

+ getopts t:m:a:s:c:z:r:u:p: opt
+ case ${opt} in
+ export MACHINE_TYPE=aws
+ MACHINE_TYPE=aws
+ getopts t:m:a:s:c:z:r:u:p: opt
+ case ${opt} in
+ export AWS_ACCESS_KEY_ID=AKIAIR4X6GNTAQLFE36Q
+ AWS_ACCESS_KEY_ID=AKIAIR4X6GNTAQLFE36Q
+ getopts t:m:a:s:c:z:r:u:p: opt
+ case ${opt} in
+ export AWS_SECRET_ACCESS_KEY=rWqNA6sq87qjusvG/vF1x/UnRmYpUDlTsQYSxiwV
+ AWS_SECRET_ACCESS_KEY=rWqNA6sq87qjusvG/vF1x/UnRmYpUDlTsQYSxiwV
+ getopts t:m:a:s:c:z:r:u:p: opt
+ case ${opt} in
+ export MACHINE_NAME=ilmarinen-adop
+ MACHINE_NAME=ilmarinen-adop
+ getopts t:m:a:s:c:z:r:u:p: opt
+ case ${opt} in
+ export AWS_VPC_ID=vpc-c303f0a7
+ AWS_VPC_ID=vpc-c303f0a7
+ getopts t:m:a:s:c:z:r:u:p: opt
+ case ${opt} in
+ export AWS_DEFAULT_REGION=eu-west-1
+ AWS_DEFAULT_REGION=eu-west-1
+ getopts t:m:a:s:c:z:r:u:p: opt
+ '[' -z aws ']'
+ CLI_COMPOSE_OPTS=
+ case ${MACHINE_TYPE} in
+ provision_aws
+ '[' -z ilmarinen-adop ']'
+ '[' -z vpc-c303f0a7 ']'
+ '[' -z ']'
+ echo 'No availability zone specified - using default [a].'
No availability zone specified - using default [a].
+ export VPC_AVAIL_ZONE=a
+ VPC_AVAIL_ZONE=a
+ '[' -f /Users/oscar.renalias/.aws/credentials ']'
+ echo 'Using default AWS credentials from ~/.aws/credentials'
Using default AWS credentials from ~/.aws/credentials
+ '[' -z AKIAIR4X6GNTAQLFE36Q ']'
++ grep -v '^\[' /Users/oscar.renalias/.aws/credentials
++ sed 's/^\(.*\)\s=\s/export \U\1=/'
+ eval aws_access_key_id = AKIAIR4X6GNTAQLFE36Q aws_secret_access_key = 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
++ aws_access_key_id = AKIAIR4X6GNTAQLFE36Q aws_secret_access_key = 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
./quickstart.sh: line 119: aws_access_key_id: command not found

The version of bash in OS X 10.11 definitely doesn't like whatever is going on.

nickdgriffin commented 8 years ago

I'm really not a fan of us trying to re-use the .aws directory as that's normally handled by some Python magic in the AWS CLI so that part might be stripped out in favour of something less inventive but not as fragile.

Also, I hope those credentials aren't active any more!

robwells57 commented 8 years ago

Aka redact your credentials before posting on public forums, i.e. don't wave your secret key around in public! :-) On 18 May 2016 12:09 a.m., "Nick Griffin" notifications@github.com wrote:

I'm really not a fan of us trying to re-use the .aws directory as that's normally handled by some Python magic in the AWS CLI so that part might be stripped out in favour of something less inventive but not as fragile.

Also, I hope those credentials aren't active any more!

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/Accenture/adop-docker-compose/issues/71#issuecomment-219880930

oscarrenalias commented 8 years ago

No, that API key is no longer valid 😇

oscarrenalias commented 8 years ago

Regarding the issue, why not drop this approach and simply force everyone to expose these values as environment variables?

nickdgriffin commented 8 years ago

That's the direction I'm heading in. We had that originally and people wanted to use the credentials file but then it turns out there are some cases where people have multiple sets of credentials that it couldn't handle, and it'd also use the contents of the file even if the parameters were provided etc.

I'd happily strip it all out in favour of environment variables and drop the parameters from the script too.

oscarrenalias commented 8 years ago

Yeah, I also had an issue with the script using credentials from the file despite the fact that I was providing them as parameters.

ddelmoli commented 8 years ago

The issue of trying to use the stored credentials even when you provide them on the command line is that you're using the bitwise operators (&, |) instead of the logical operators (&&, II) in your compound bash if tests. I'll try to fix and submit a pull request for this if I get time.