bazaarvoice / cloudformation-ruby-dsl

Ruby DSL for creating Cloudformation templates
Apache License 2.0
210 stars 76 forks source link

aws_region() "pseudo" parameter is not a pseudo parameter #127

Open fshields opened 6 years ago

fshields commented 6 years ago

aws_region() is listed in the README as a pseudo parameter. But it is actually implemented as an initialization option that pulls the region from the local user's environment variable, if it exists. If not, it sets it to us-east-1:

spec/spec_helper.rb

def region
  ENV["AWS_REGION"] || "us-east-1"
end

lib/cloudformation-ruby-dsl/dsl.rb

@aws_region  = options.fetch(:region, default_region)

If this is to actually act as a pseudo parameter that will automatically pick up the region where the template is deployed, then it would need to be defined the way the other pseudo parameters are:

def aws_region() ref("AWS::Region") end

However an issue arises if you change the functionality that users have come to rely on. A way around this would be to define the pseudo parameter as aws_region_pseudo(), but that is messy because it doesn't match the naming convention of the other pseudo parameters. Alternatively, you could rename the current functionality to aws_region_env (which would break backward compatibility) and then fix up aws_region to be a pseudo_parameter.

It's a hard choice, but ultimately up to you.

zhelyan commented 5 years ago

IMHO all code related to setting AWS credentials and region must be removed. Aws already provides a flexible idiomatic mechnaism for setting these and there's no point duplicating the logic here. It can be as easy as:

AWS_PROFILE=prod ./template.rb ....