diff --git a/lib/puppet/provider/ec2_securitygroup/v2.rb b/lib/puppet/provider/ec2_securitygroup/v2.rb index d7ddb347..50908ac7 100644 --- a/lib/puppet/provider/ec2_securitygroup/v2.rb +++ b/lib/puppet/provider/ec2_securitygroup/v2.rb @@ -17,7 +17,7 @@ def self.instances end.flatten end - read_only(:region, :ingress, :description) + read_only(:region, :description) def self.prefetch(resources) instances.each do |prov| @@ -78,9 +78,17 @@ def create ) unless tags.empty? rules = resource[:ingress] - rules = [rules] unless rules.is_a?(Array) + authorize_ingress(rules) + end + + def authorize_ingress(new_rules, existing_rules=[]) + ec2 = ec2_client(resource[:region]) + new_rules = [new_rules] unless new_rules.is_a?(Array) + + to_create = new_rules - existing_rules + to_delete = existing_rules - new_rules - rules.reject(&:nil?).each do |rule| + to_create.reject(&:nil?).each do |rule| if rule.key? 'security_group' ec2.authorize_security_group_ingress( group_name: name, @@ -100,6 +108,32 @@ def create ) end end + + to_delete.reject(&:nil?).each do |rule| + if rule.key? 'security_group' + ec2.revoke_security_group_ingress( + group_name: name, + source_security_group_name: rule['security_group'] + ) + else + ec2.revoke_security_group_ingress( + group_name: name, + ip_permissions: [{ + ip_protocol: rule['protocol'], + to_port: rule['port'].to_i, + from_port: rule['port'].to_i, + ip_ranges: [{ + cidr_ip: rule['cidr'] + }] + }] + ) + end + end + + end + + def ingress=(value) + authorize_ingress(value, @property_hash[:ingress]) end def destroy diff --git a/spec/acceptance/securitygroup_spec.rb b/spec/acceptance/securitygroup_spec.rb index a2bf7732..269c25c8 100644 --- a/spec/acceptance/securitygroup_spec.rb +++ b/spec/acceptance/securitygroup_spec.rb @@ -34,7 +34,7 @@ def get_group_permission(ip_permissions, group, protocol) # 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) + def check_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 @@ -42,7 +42,7 @@ def has_ingress_rule(rule, ip_permissions) 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}" + msg = "Could not find ingress rule for #{group_name}" else match = ip_permissions.any? do |perm| rule[:protocol] == perm[:ip_protocol] && @@ -51,10 +51,21 @@ def has_ingress_rule(rule, ip_permissions) 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 + [match, msg] end + def has_ingress_rule(rule, ip_permissions) + match, msg = check_ingress_rule(rule, ip_permissions) + expect(match).to eq(true), msg + end + + def doesnt_have_ingress_rule(rule, ip_permissions) + match, msg = check_ingress_rule(rule, ip_permissions) + expect(match).to eq(false), msg + end + + describe 'should create a new security group' do before(:all) do @@ -112,14 +123,21 @@ def has_ingress_rule(rule, ip_permissions) @config[:ingress].all? { |rule| has_ingress_rule(rule, @group.ip_permissions)} end - it 'and should not be able to modify the ingress rules and recreate the security group' do - new_config = @config.dup.update({:ingress => [{ :security_group => @name }]}) + it 'should be able to modify the ingress rules and recreate the security group' do + new_rules = [{ + :protocol => 'tcp', + :port => 80, + :cidr => '0.0.0.0/0' + }] + new_config = @config.dup.update({:ingress => new_rules}) success = PuppetManifest.new(@template, new_config).apply[:exit_status].success? expect(success).to eq(false) # should still have the original rules @group = find_group(@config[:name]) - @config[:ingress].all? { |rule| has_ingress_rule(rule, @group.ip_permissions)} + + new_rules.all? { |rule| has_ingress_rule(rule, @group.ip_permissions)} + @config[:ingress].all? { |rule| doesnt_have_ingress_rule(rule, @group.ip_permissions)} end describe 'that another group depends on in a secondary manifest' do