From 788743cba1538560d36892de20d38e06bf1a2647 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 | 7 +------ 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, 61 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 45cb1298..74deb951 100644 --- a/lib/puppet/type/ec2_autoscalinggroup.rb +++ b/lib/puppet/type/ec2_autoscalinggroup.rb @@ -87,13 +87,8 @@ desc 'Indicates whether newly launched instances are protected from termination by Auto Scaling when scaling in.' 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 9dfee1e7..839a0f6b 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 838b39f9..5587c462 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 9c66e301..f4ace4fe 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 c126badf..f2d1acf8 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 862d4c71..000a1822 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