From 919073e9beb8266c2784fbb64c34a86a78b967ca Mon Sep 17 00:00:00 2001 From: Gareth Rushgrove Date: Tue, 24 Mar 2015 09:46:20 +0000 Subject: [PATCH] Avoid ambiguity when using the AWS_REGION environment var If using the AWS_REGION environment variable and a manifest where the region property of one or more resources doesn't match you can see unspecified behaviour. See https://github.com/puppetlabs/puppetlabs-aws/pull/117 for discussions. This PR extracted from the above simply stops that from happening. If the env is present and doesn't match All of the region properties then we fail. This maintains the utility of the env var as an optimisation but stops us shooting ourselves in the foot. --- lib/puppet/type/cloudwatch_alarm.rb | 9 +++------ lib/puppet/type/ec2_autoscalinggroup.rb | 9 +++------ lib/puppet/type/ec2_elastic_ip.rb | 8 +++----- lib/puppet/type/ec2_launchconfiguration.rb | 9 +++------ lib/puppet/type/ec2_scalingpolicy.rb | 9 +++------ lib/puppet/type/ec2_securitygroup.rb | 9 +++------ lib/puppet/type/ec2_vpc.rb | 9 +++------ lib/puppet/type/ec2_vpc_customer_gateway.rb | 9 +++------ lib/puppet/type/ec2_vpc_dhcp_options.rb | 5 +++-- lib/puppet/type/ec2_vpc_internet_gateway.rb | 9 +++------ lib/puppet/type/ec2_vpc_routetable.rb | 9 +++------ lib/puppet/type/ec2_vpc_subnet.rb | 9 +++------ lib/puppet/type/ec2_vpc_vpn.rb | 9 +++------ lib/puppet/type/ec2_vpc_vpn_gateway.rb | 9 +++------ lib/puppet/type/elb_loadbalancer.rb | 9 +++------ lib/puppet_x/puppetlabs/property/region.rb | 15 +++++++++++++++ spec/unit/type/ec2_elastic_ip_spec.rb | 6 +++--- 17 files changed, 63 insertions(+), 88 deletions(-) create mode 100644 lib/puppet_x/puppetlabs/property/region.rb diff --git a/lib/puppet/type/cloudwatch_alarm.rb b/lib/puppet/type/cloudwatch_alarm.rb index ee358e2c..9c00fafa 100644 --- a/lib/puppet/type/cloudwatch_alarm.rb +++ b/lib/puppet/type/cloudwatch_alarm.rb @@ -1,3 +1,5 @@ +require_relative '../../puppet_x/puppetlabs/property/region' + Puppet::Type.newtype(:cloudwatch_alarm) do @doc = 'Type representing an AWS CloudWatch Alarm.' @@ -73,13 +75,8 @@ end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to launch the instances.' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should not be blank' if value == '' - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:dimensions, :array_matching => :all) do diff --git a/lib/puppet/type/ec2_autoscalinggroup.rb b/lib/puppet/type/ec2_autoscalinggroup.rb index 5983c964..35fc5eee 100644 --- a/lib/puppet/type/ec2_autoscalinggroup.rb +++ b/lib/puppet/type/ec2_autoscalinggroup.rb @@ -1,3 +1,5 @@ +require_relative '../../puppet_x/puppetlabs/property/region' + Puppet::Type.newtype(:ec2_autoscalinggroup) do @doc = 'Type representing an EC2 auto scaling group.' @@ -31,13 +33,8 @@ end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to launch the instances.' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should not be blank' if value == '' - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:launch_configuration) do diff --git a/lib/puppet/type/ec2_elastic_ip.rb b/lib/puppet/type/ec2_elastic_ip.rb index 15c022d5..29aed019 100644 --- a/lib/puppet/type/ec2_elastic_ip.rb +++ b/lib/puppet/type/ec2_elastic_ip.rb @@ -1,3 +1,5 @@ +require_relative '../../puppet_x/puppetlabs/property/region' + Puppet::Type.newtype(:ec2_elastic_ip) do @doc = "Type representing an Elastic IP and it's association." @@ -19,12 +21,8 @@ end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The name of the region in which the Elastic IP is found.' - validate do |value| - fail 'region should be a String' unless value.is_a?(String) - fail 'You must provide a region for Elastic IPs.' if value.nil? || value.empty? - end end newproperty(:instance) do diff --git a/lib/puppet/type/ec2_launchconfiguration.rb b/lib/puppet/type/ec2_launchconfiguration.rb index 1133ae9c..89d409d1 100644 --- a/lib/puppet/type/ec2_launchconfiguration.rb +++ b/lib/puppet/type/ec2_launchconfiguration.rb @@ -1,3 +1,5 @@ +require_relative '../../puppet_x/puppetlabs/property/region' + Puppet::Type.newtype(:ec2_launchconfiguration) do @doc = 'Type representing an EC2 launch configuration.' @@ -33,13 +35,8 @@ def insync?(is) end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to launch the instances.' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should not be blank' if value == '' - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:instance_type) do diff --git a/lib/puppet/type/ec2_scalingpolicy.rb b/lib/puppet/type/ec2_scalingpolicy.rb index 99698cb9..922c22ca 100644 --- a/lib/puppet/type/ec2_scalingpolicy.rb +++ b/lib/puppet/type/ec2_scalingpolicy.rb @@ -1,3 +1,5 @@ +require_relative '../../puppet_x/puppetlabs/property/region' + Puppet::Type.newtype(:ec2_scalingpolicy) do @doc = 'Type representing an EC2 scaling policy.' @@ -21,13 +23,8 @@ end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to launch the policy.' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should not be blank' if value == '' - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:adjustment_type) do diff --git a/lib/puppet/type/ec2_securitygroup.rb b/lib/puppet/type/ec2_securitygroup.rb index 74014975..4e62d143 100644 --- a/lib/puppet/type/ec2_securitygroup.rb +++ b/lib/puppet/type/ec2_securitygroup.rb @@ -1,4 +1,5 @@ -require_relative '../../puppet_x/puppetlabs/property/tag.rb' +require_relative '../../puppet_x/puppetlabs/property/tag' +require_relative '../../puppet_x/puppetlabs/property/region' require_relative '../../puppet_x/puppetlabs/aws_ingress_rules_parser' Puppet::Type.newtype(:ec2_securitygroup) do @@ -14,12 +15,8 @@ end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'the region in which to launch the security group' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:ingress, :array_matching => :all) do diff --git a/lib/puppet/type/ec2_vpc.rb b/lib/puppet/type/ec2_vpc.rb index 81e09189..f8c1f14a 100644 --- a/lib/puppet/type/ec2_vpc.rb +++ b/lib/puppet/type/ec2_vpc.rb @@ -1,4 +1,5 @@ -require_relative '../../puppet_x/puppetlabs/property/tag.rb' +require_relative '../../puppet_x/puppetlabs/property/tag' +require_relative '../../puppet_x/puppetlabs/property/region' Puppet::Type.newtype(:ec2_vpc) do @doc = 'A type representing an AWS VPC.' @@ -13,12 +14,8 @@ end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to launch the VPC.' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:cidr_block) do diff --git a/lib/puppet/type/ec2_vpc_customer_gateway.rb b/lib/puppet/type/ec2_vpc_customer_gateway.rb index f5badd64..42aeaf8a 100644 --- a/lib/puppet/type/ec2_vpc_customer_gateway.rb +++ b/lib/puppet/type/ec2_vpc_customer_gateway.rb @@ -1,4 +1,5 @@ -require_relative '../../puppet_x/puppetlabs/property/tag.rb' +require_relative '../../puppet_x/puppetlabs/property/tag' +require_relative '../../puppet_x/puppetlabs/property/region' Puppet::Type.newtype(:ec2_vpc_customer_gateway) do @doc = 'Type representing an AWS VPC customer gateways.' @@ -31,12 +32,8 @@ desc 'The tags for the customer gateway.' end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to launch the customer gateway.' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:type) do diff --git a/lib/puppet/type/ec2_vpc_dhcp_options.rb b/lib/puppet/type/ec2_vpc_dhcp_options.rb index e1305634..70189df8 100644 --- a/lib/puppet/type/ec2_vpc_dhcp_options.rb +++ b/lib/puppet/type/ec2_vpc_dhcp_options.rb @@ -1,4 +1,5 @@ -require_relative '../../puppet_x/puppetlabs/property/tag.rb' +require_relative '../../puppet_x/puppetlabs/property/tag' +require_relative '../../puppet_x/puppetlabs/property/region' Puppet::Type.newtype(:ec2_vpc_dhcp_options) do @doc = 'Type representing a DHCP option set for AWS VPC.' @@ -17,7 +18,7 @@ desc 'Tags for the DHCP option set.' end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to assign the DHCP option set.' validate do |value| fail 'region should not contain spaces' if value =~ /\s/ diff --git a/lib/puppet/type/ec2_vpc_internet_gateway.rb b/lib/puppet/type/ec2_vpc_internet_gateway.rb index 1607b57f..423d31a4 100644 --- a/lib/puppet/type/ec2_vpc_internet_gateway.rb +++ b/lib/puppet/type/ec2_vpc_internet_gateway.rb @@ -1,4 +1,5 @@ -require_relative '../../puppet_x/puppetlabs/property/tag.rb' +require_relative '../../puppet_x/puppetlabs/property/tag' +require_relative '../../puppet_x/puppetlabs/property/region' Puppet::Type.newtype(:ec2_vpc_internet_gateway) do @doc = 'Type representing an EC2 VPC Internet Gateway.' @@ -17,12 +18,8 @@ desc 'Tags to assign to the internet gateway.' end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to launch the internet gateway.' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:vpc) do diff --git a/lib/puppet/type/ec2_vpc_routetable.rb b/lib/puppet/type/ec2_vpc_routetable.rb index c37f0684..c576613c 100644 --- a/lib/puppet/type/ec2_vpc_routetable.rb +++ b/lib/puppet/type/ec2_vpc_routetable.rb @@ -1,4 +1,5 @@ -require_relative '../../puppet_x/puppetlabs/property/tag.rb' +require_relative '../../puppet_x/puppetlabs/property/tag' +require_relative '../../puppet_x/puppetlabs/property/region' Puppet::Type.newtype(:ec2_vpc_routetable) do @doc = 'Type representing a VPC route table.' @@ -20,12 +21,8 @@ end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'Region in which to launch the route table.' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:routes, :array_matching => :all) do diff --git a/lib/puppet/type/ec2_vpc_subnet.rb b/lib/puppet/type/ec2_vpc_subnet.rb index 1631d11f..a737a6c4 100644 --- a/lib/puppet/type/ec2_vpc_subnet.rb +++ b/lib/puppet/type/ec2_vpc_subnet.rb @@ -1,4 +1,5 @@ -require_relative '../../puppet_x/puppetlabs/property/tag.rb' +require_relative '../../puppet_x/puppetlabs/property/tag' +require_relative '../../puppet_x/puppetlabs/property/region' Puppet::Type.newtype(:ec2_vpc_subnet) do @doc = 'Type representing a VPC Subnet.' @@ -20,12 +21,8 @@ end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to launch the subnet.' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:cidr_block) do diff --git a/lib/puppet/type/ec2_vpc_vpn.rb b/lib/puppet/type/ec2_vpc_vpn.rb index fbdb5a47..10806a81 100644 --- a/lib/puppet/type/ec2_vpc_vpn.rb +++ b/lib/puppet/type/ec2_vpc_vpn.rb @@ -1,4 +1,5 @@ -require_relative '../../puppet_x/puppetlabs/property/tag.rb' +require_relative '../../puppet_x/puppetlabs/property/tag' +require_relative '../../puppet_x/puppetlabs/property/region' Puppet::Type.newtype(:ec2_vpc_vpn) do @doc = 'Type representing an AWS Virtual Private Networks.' @@ -53,12 +54,8 @@ def insync?(is) end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to launch the VPN.' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:tags, :parent => PuppetX::Property::AwsTag) do diff --git a/lib/puppet/type/ec2_vpc_vpn_gateway.rb b/lib/puppet/type/ec2_vpc_vpn_gateway.rb index d0276125..22e56eaf 100644 --- a/lib/puppet/type/ec2_vpc_vpn_gateway.rb +++ b/lib/puppet/type/ec2_vpc_vpn_gateway.rb @@ -1,4 +1,5 @@ -require_relative '../../puppet_x/puppetlabs/property/tag.rb' +require_relative '../../puppet_x/puppetlabs/property/tag' +require_relative '../../puppet_x/puppetlabs/property/region' Puppet::Type.newtype(:ec2_vpc_vpn_gateway) do @doc = 'A type representing a VPN gateway.' @@ -24,12 +25,8 @@ end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to launch the VPN gateway.' - validate do |value| - fail 'region should not contain spaces' if value =~ /\s/ - fail 'region should be a String' unless value.is_a?(String) - end end newparam(:availability_zone) do diff --git a/lib/puppet/type/elb_loadbalancer.rb b/lib/puppet/type/elb_loadbalancer.rb index 17ae30ec..4cc11572 100644 --- a/lib/puppet/type/elb_loadbalancer.rb +++ b/lib/puppet/type/elb_loadbalancer.rb @@ -1,4 +1,5 @@ -require_relative '../../puppet_x/puppetlabs/property/tag.rb' +require_relative '../../puppet_x/puppetlabs/property/tag' +require_relative '../../puppet_x/puppetlabs/property/region' Puppet::Type.newtype(:elb_loadbalancer) do @doc = 'Type representing an ELB load balancer.' @@ -13,12 +14,8 @@ end end - newproperty(:region) do + newproperty(:region, :parent => PuppetX::Property::AwsRegion) do desc 'The region in which to launch the load balancer.' - validate do |value| - fail 'region must not contain spaces' if value =~ /\s/ - fail 'region should be a String' unless value.is_a?(String) - end end newproperty(:listeners, :array_matching => :all) do diff --git a/lib/puppet_x/puppetlabs/property/region.rb b/lib/puppet_x/puppetlabs/property/region.rb new file mode 100644 index 00000000..efe89476 --- /dev/null +++ b/lib/puppet_x/puppetlabs/property/region.rb @@ -0,0 +1,15 @@ +module PuppetX + module Property + class AwsRegion < Puppet::Property + validate do |value| + name = resource[:name] + fail 'region should not contain spaces' if value =~ /\s/ + fail 'region should not be blank' if value == '' + fail 'region should be a String' unless value.is_a?(String) + if !ENV['AWS_REGION'].nil? && ENV['AWS_REGION'] != value + fail "if using AWS_REGION environment variable it must match the specified region value for #{name}" + end + end + end + end +end diff --git a/spec/unit/type/ec2_elastic_ip_spec.rb b/spec/unit/type/ec2_elastic_ip_spec.rb index 803abdf1..6faa3332 100644 --- a/spec/unit/type/ec2_elastic_ip_spec.rb +++ b/spec/unit/type/ec2_elastic_ip_spec.rb @@ -45,12 +45,12 @@ it 'should require a region to be specified' do expect { type_class.new({ name: '10.0.0.1', region: '' }) - }.to raise_error(Puppet::Error, /You must provide a region for Elastic IPs/) + }.to raise_error(Puppet::Error, /region should not be blank/) end it 'should require an instance to be specified' do expect { - type_class.new({ name: '10.0.0.1', region: 'us-east-1', instance: '' }) + type_class.new({ name: '10.0.0.1', region: 'sa-east-1', instance: '' }) }.to raise_error(Puppet::Error, /You must provide an instance for the Elastic IP association/) end @@ -76,7 +76,7 @@ context 'with valid properties' do it 'should create a valid elastic ip ' do - type_class.new({ name: '10.0.0.1', region: 'us-east-1', instance: 'web-1' }) + type_class.new({ name: '10.0.0.1', region: 'sa-east-1', instance: 'web-1' }) end end