diff --git a/lib/puppet/provider/ec2_securitygroup/v2.rb b/lib/puppet/provider/ec2_securitygroup/v2.rb index 64159375..12d437ee 100644 --- a/lib/puppet/provider/ec2_securitygroup/v2.rb +++ b/lib/puppet/provider/ec2_securitygroup/v2.rb @@ -14,7 +14,7 @@ def self.instances end.flatten end - read_only(:region) + read_only(:region, :ingress, :description) def self.prefetch(resources) instances.each do |prov| @@ -24,11 +24,28 @@ def self.prefetch(resources) end end + def self.format_ingress_rules(group) + group[:ip_permissions].collect do |rule| + if rule.user_id_group_pairs.empty? + { + 'protocol' => rule.ip_protocol, + 'port' => rule.to_port.to_i, + 'cidr' => rule.ip_ranges.first.cidr_ip + } + else + { + 'security_group' => rule.user_id_group_pairs.first.group_name + } + end + end.uniq + end + def self.security_group_to_hash(region, group) { name: group[:group_name], description: group[:description], ensure: :present, + ingress: format_ingress_rules(group), region: region, } end diff --git a/lib/puppet/type/ec2_securitygroup.rb b/lib/puppet/type/ec2_securitygroup.rb index b43c93f1..b91dd7b2 100644 --- a/lib/puppet/type/ec2_securitygroup.rb +++ b/lib/puppet/type/ec2_securitygroup.rb @@ -17,8 +17,16 @@ end end - newparam(:ingress, :array_mathching => :all) do + newproperty(:ingress, :array_matching => :all) do desc 'rules for ingress traffic' + def insync?(is) + should == stringify_values(is) + end + def stringify_values(rules) + rules.collect do |obj| + obj.each { |k,v| obj[k] = v.to_s } + end + end end newparam(:tags, :array_matching => :all) do @@ -32,11 +40,15 @@ end end + def should_autorequire?(rule) + !rule.nil? and rule.key? 'security_group' and rule['security_group'] != name + end + autorequire(:ec2_securitygroup) do rules = self[:ingress] rules = [rules] unless rules.is_a?(Array) rules.collect do |rule| - rule['security_group'] if !rule.nil? and rule.key? 'security_group' + rule['security_group'] if should_autorequire?(rule) end end end diff --git a/spec/acceptance/fixtures/securitygroup.pp.tmpl b/spec/acceptance/fixtures/securitygroup.pp.tmpl index fc298e2b..28510bd6 100644 --- a/spec/acceptance/fixtures/securitygroup.pp.tmpl +++ b/spec/acceptance/fixtures/securitygroup.pp.tmpl @@ -1,7 +1,16 @@ ec2_securitygroup { '{{name}}': ensure => {{ensure}}, - region => '{{region}}', description => '{{description}}', + region => '{{region}}', + ingress => [ + {{#ingress}} + { + {{#values}} + {{k}} => '{{v}}', + {{/values}} + }, + {{/ingress}} + ], tags => { {{#tags}} {{k}} => '{{v}}', diff --git a/spec/acceptance/securitygroup_spec.rb b/spec/acceptance/securitygroup_spec.rb index 2599fcdb..33dd5f63 100644 --- a/spec/acceptance/securitygroup_spec.rb +++ b/spec/acceptance/securitygroup_spec.rb @@ -23,14 +23,56 @@ def has_matching_tags(group, tags) expect(symmetric_difference).to be_empty end + def get_group_permission(ip_permissions, group, protocol) + ip_permissions.detect do |perm| + pairs = perm[:user_id_group_pairs] + pairs.any? do |pair| + pair.group_name == group && perm[:ip_protocol] == protocol + end + end + end + + # a fairly naive matching algorithm, since the shape of ip_permissions is + # quite different than the shape of our ingress rules + def has_ingress_rule(rule, ip_permissions) + if (rule.has_key? :security_group) + group_name = rule[:security_group] + # a match occurs when AWS has a TCP / UDP / ICMP perm setup for group + tcp_perm = get_group_permission(ip_permissions, group_name, 'tcp') + udp_perm = get_group_permission(ip_permissions, group_name, 'udp') + icmp_perm = get_group_permission(ip_permissions, group_name, 'icmp') + match = !tcp_perm.nil? && !udp_perm.nil? && !icmp_perm.nil? + expect(match).to eq(true), "Could not find ingress rule for #{group_name}" + else + match = ip_permissions.any? do |perm| + rule[:protocol] == perm[:ip_protocol] && + rule[:port] == perm[:from_port] && + rule[:port] == perm[:to_port] && + perm[:ip_ranges].any? { |ip| ip[:cidr_ip] == rule[:cidr] } + end + msg = "Could not find ingress rule for #{rule[:protocol]} port #{rule[:port]} and CIDR #{rule[:cidr]}" + expect(match).to eq(true), msg + end + end + describe 'should create a new security group' do before(:all) do + @name = "#{PuppetManifest.env_id}-#{SecureRandom.uuid}" @config = { - :name => "#{PuppetManifest.env_id}-#{SecureRandom.uuid}", - :region => @default_region, + :name => @name, :ensure => 'present', - :description => 'short lived group created by acceptance tests', + :description => 'aws acceptance securitygroup', + :region => @default_region, + :ingress => [ + { + :security_group => @name, + },{ + :protocol => 'tcp', + :port => 22, + :cidr => '0.0.0.0/0' + } + ], :tags => { :department => 'engineering', :project => 'cloud', @@ -45,6 +87,8 @@ def has_matching_tags(group, tags) after(:all) do new_config = @config.update({:ensure => 'absent'}) PuppetManifest.new(@template, new_config).apply + + expect(find_group(@config[:name])).to be_nil end it "with the specified name" do @@ -63,6 +107,58 @@ def has_matching_tags(group, tags) expect(@group.description).to eq(@config[:description]) end + it "with the specified ingress rules" do + # perform a naive match + @config[:ingress].all? { |rule| has_ingress_rule(rule, @group.ip_permissions)} + end + + end + + describe 'should create a new securitygroup' do + + before(:each) do + @name = "#{PuppetManifest.env_id}-#{SecureRandom.uuid}" + @config = { + :name => @name, + :ensure => 'present', + :description => 'aws acceptance sg', + :region => @default_region, + :ingress => [ + { + :security_group => @name, + },{ + :protocol => 'tcp', + :port => 22, + :cidr => '0.0.0.0/0' + } + ], + :tags => { + :department => 'engineering', + :project => 'cloud', + :created_by => 'aws-acceptance' + } + } + + PuppetManifest.new(@template, @config).apply + @group = find_group(@config[:name]) + end + + after(:each) do + @config[:ensure] = 'absent' + PuppetManifest.new(@template, @config).apply + end + + it 'that can have tags changed' do + pending 'changing tags not yet supported for security groups' + has_matching_tags(@group, @config[:tags]) + + tags = {:created_by => 'aws-tests', :foo => 'bar'} + @config[:tags].update(tags) + + PuppetManifest.new(@template, @config).apply + @group = find_group(@config[:name]) + has_matching_tags(@group, @config[:tags]) + end end end diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb index 9af7c44e..ca26a924 100644 --- a/spec/spec_helper_acceptance.rb +++ b/spec/spec_helper_acceptance.rb @@ -5,10 +5,7 @@ class PuppetManifest < Mustache def initialize(file, config) @template_file = File.join(Dir.getwd, 'spec', 'acceptance', 'fixtures', file) config.each do |key, value| - config_value = value - if (value.class == Hash) - config_value = value.map { |k, v| { :k => k, :v => v }} - end + config_value = self.class.to_generalized_data(value) instance_variable_set("@#{key}".to_sym, config_value) self.class.send(:attr_accessor, key) end @@ -18,6 +15,38 @@ def apply system("bundle exec puppet apply -e \"#{manifest}\" --modulepath ../") end + def self.to_generalized_data(val) + case val + when Hash + to_generalized_hash_list(val) + when Array + to_generalized_array_list(val) + else + val + end + end + + # returns an array of :k =>, :v => hashes given a Hash + # { :a => 'b', :c => 'd' } -> [{:k => 'a', :v => 'b'}, {:k => 'c', :v => 'd'}] + def self.to_generalized_hash_list(hash) + hash.map { |k, v| { :k => k, :v => v }} + end + + # necessary to build like [{ :values => Array }] rather than [[]] when there + # are nested hashes, for the sake of Mustache being able to render + # otherwise, simply return the item + def self.to_generalized_array_list(arr) + arr.map do |item| + if item.class == Hash + { + :values => to_generalized_hash_list(item) + } + else + item + end + end + end + def self.env_id @env_id ||= ( ENV['BUILD_DISPLAY_NAME'] || diff --git a/spec/unit/type/ec2_securitygroup_spec.rb b/spec/unit/type/ec2_securitygroup_spec.rb index d6c68d0a..0628cc9b 100644 --- a/spec/unit/type/ec2_securitygroup_spec.rb +++ b/spec/unit/type/ec2_securitygroup_spec.rb @@ -7,7 +7,7 @@ let :params do [ :name, - :ingress + :tags, ] end @@ -15,7 +15,8 @@ [ :ensure, :description, - :region + :region, + :ingress, ] end