[WIP] Add optional "safe-reload" mode

.kitchen.yml

@@ -62,22 +62,21 @@ suites:
     run_list:
     - recipe[minitest-handler]
     - recipe[iptables_ng_test::lwrp_rule_check_order]
-
   - name: recipe_default
     run_list:
     - recipe[minitest-handler]
     - recipe[iptables_ng_test::recipe_default]
     attributes:
       iptables-ng:
-        rules:
+        rules: &rules
           filter:
-            INPUT:
+            INPUT: &input
               ssh:
                 rule: '--protocol tcp --dport 22 --match state --state NEW --jump ACCEPT'
               ipv4_only:
                 rule: '--protocol tcp --source 1.2.3.4 --dport 123 --jump ACCEPT'
                 ip_version: 4
-            OUTPUT:
+            OUTPUT: &output
                 testrule:
                   rule: '--protocol icmp --jump ACCEPT'
             FORWARD:
@@ -89,7 +88,14 @@ suites:
           mangle:
             FORWARD:
               default: 'DROP [0:0]'
-
+  - name: safe_reload_off
+    run_list:
+    - recipe[minitest-handler]
+    - recipe[iptables_ng_test::recipe_default]
+    attributes:
+      iptables-ng:
+        rules: *rules
+        safe_reload: false
   - name: recipe_install
     run_list:
     - recipe[minitest-handler]

attributes/default.rb

@@ -34,6 +34,10 @@
 # Configure whether to automatically clean up unused rules
 default['iptables-ng']['auto_prune_attribute_rules'] = false
 
+# Configure whether to opt-in to safer restart behavior by skipping restarts
+# in case of converge failure. Only available on Chef client versions >= 12.5.
+default['iptables-ng']['safe_reload'] = true
+
 # Enable nat support for ipv6
 # Older distributions do not support ipv6 nat, but recent Ubuntu does
 default['iptables-ng']['ip6tables_nat_support'] = value_for_platform(
@@ -42,21 +46,22 @@
 )
 
 # Packages to install
-default['iptables-ng']['packages'] = case node['platform_family']
-when 'debian'
-  %w(iptables iptables-persistent)
-when 'rhel'
-  if node['platform'] == 'amazon'
-    # Amazon Linux doesn't include "iptables-services" or "iptables-ipv6"
-    %w(iptables)
-  elsif node['platform_version'].to_f >= 7.0
-    %w(iptables iptables-services)
+default['iptables-ng']['packages'] =
+  case node['platform_family']
+  when 'debian'
+    %w(iptables iptables-persistent)
+  when 'rhel'
+    if node['platform'] == 'amazon'
+      # Amazon Linux doesn't include "iptables-services" or "iptables-ipv6"
+      %w(iptables)
+    elsif node['platform_version'].to_f >= 7.0
+      %w(iptables iptables-services)
+    else
+      %w(iptables iptables-ipv6)
+    end
   else
-    %w(iptables iptables-ipv6)
+    %w(iptables)
   end
-else
-  %w(iptables)
-end
 
 # Where the rules are stored and how they are executed
 case node['platform']

libraries/create_iptables_rules.rb

@@ -24,7 +24,7 @@
 
 module Iptables
   module Manage
-    def create_iptables_rules(ip_version)
+    def create_iptables_rules(ip_version, run_context)
       rules = {}
 
       # Retrieve all iptables rules for this ip_version,
@@ -38,11 +38,11 @@ def create_iptables_rules(ip_version)
 
         # Skip nat table if ip6tables doesn't support it
         next if table == 'nat' &&
-                node['iptables-ng']['ip6tables_nat_support'] == false &&
+                run_context.node['iptables-ng']['ip6tables_nat_support'] == false &&
                 ip_version == 6
 
         # Skip deactivated tables
-        next unless node['iptables-ng']['enabled_tables'].include?(table)
+        next unless run_context.node['iptables-ng']['enabled_tables'].include?(table)
 
         # Create hashes unless they already exist, and add the rule
         rules[table] ||= {}
@@ -76,13 +76,15 @@ def create_iptables_rules(ip_version)
         iptables_restore << "COMMIT\n"
       end
 
-      Chef::Resource::File.new(node['iptables-ng']["script_ipv#{ip_version}"], run_context).tap do |file|
+      Chef::Resource::File.new(run_context.node['iptables-ng']["script_ipv#{ip_version}"], run_context).tap do |file|
         file.owner('root')
-        file.group(node['root_group'])
+        file.group(run_context.node['root_group'])
         file.mode(00600)
         file.content(iptables_restore)
         file.run_action(:create)
       end
     end
+
+    module_function :create_iptables_rules
   end
 end

libraries/restart_service.rb

@@ -24,28 +24,43 @@
 
 module Iptables
   module Manage
-    def restart_service(ip_version)
-      # Restart iptables service if available
-      if node['iptables-ng']["service_ipv#{ip_version}"]
+    def restart(ip_version, run_context)
+      if run_context.node['iptables-ng']["service_ipv#{ip_version}"]
+        restart_service(ip_version, run_context)
+      else
+        apply_manually(ip_version, run_context)
+      end
+    end
 
-        # Do not restart twice if the command is the same for ipv4 and ipv6
-        return if node['iptables-ng']['service_ipv4'] == node['iptables-ng']['service_ipv6'] && ip_version == 6
+    # Restart if any resources in provided run_context have been updated
+    def conditionally_restart(ip_version, run_context)
+      our_resources = run_context.resource_collection.select do |r|
+        r.is_a?(Chef::Resource::IptablesNgRule) || r.is_a?(Chef::Resource::IptablesNgChain)
+      end
 
-        Chef::Resource::Service.new(node['iptables-ng']["service_ipv#{ip_version}"], run_context).tap do |service|
-          service.supports(status: true, restart: true)
-          service.run_action(:enable)
-          service.run_action(:restart)
-        end
+      restart(ip_version, run_context) if our_resources.any?(&:updated_by_last_action?)
+    end
 
-      # If no service is available, apply the rules manually
-      else
-        Chef::Log.info 'applying rules manually, as no service is specified'
-        Chef::Resource::Execute.new("iptables-restore for ipv#{ip_version}", run_context).tap do |execute|
-          execute.command("iptables-restore < #{node['iptables-ng']['script_ipv4']}") if ip_version == 4
-          execute.command("ip6tables-restore < #{node['iptables-ng']['script_ipv6']}") if ip_version == 6
-          execute.run_action(:run)
-        end
+    def restart_service(ip_version, run_context)
+      # Do not restart twice if the command is the same for ipv4 and ipv6
+      return if run_context.node['iptables-ng']['service_ipv4'] == run_context.node['iptables-ng']['service_ipv6'] && ip_version == 6
+
+      Chef::Resource::Service.new(run_context.node['iptables-ng']["service_ipv#{ip_version}"], run_context).tap do |service|
+        service.supports(status: true, restart: true)
+        service.run_action(:enable)
+        service.run_action(:restart)
       end
     end
+
+    def apply_manually(ip_version, run_context)
+      Chef::Log.info 'applying rules manually, as no service is specified'
+      Chef::Resource::Execute.new("iptables-restore for ipv#{ip_version}", run_context).tap do |execute|
+        execute.command("iptables-restore < #{run_context.node['iptables-ng']['script_ipv4']}") if ip_version == 4
+        execute.command("ip6tables-restore < #{run_context.node['iptables-ng']['script_ipv6']}") if ip_version == 6
+        execute.run_action(:run)
+      end
+    end
+
+    module_function :restart, :conditionally_restart, :restart_service, :apply_manually
   end
 end

recipes/manage.rb

@@ -21,32 +21,37 @@
 # This recipe only creates ruby_blocks that are called later by the LWRPs
 # Do not use it by its own
 
-ruby_block 'create_rules' do
-  block do
-    class Chef::Resource::RubyBlock
-      include Iptables::Manage
+modern_and_paranoid = node['iptables-ng']['safe_reload'] && Chef::VERSION.to_f >= 12.5
+
+if modern_and_paranoid
+  require 'chef/event_dispatch/dsl'
+
+  Chef.event_handler do
+    on :converge_complete do
+      Chef.run_context.node['iptables-ng']['enabled_ip_versions'].each do |ip_version|
+        Iptables::Manage.create_iptables_rules(ip_version, Chef.run_context)
+        Iptables::Manage.conditionally_restart(ip_version, Chef.run_context)
+      end if Chef.run_context.node['iptables-ng']['managed_service']
     end
+  end
+end
 
+ruby_block 'create_rules' do
+  block do
     Array(node['iptables-ng']['enabled_ip_versions']).each do |ip_version|
-      create_iptables_rules(ip_version)
+      Iptables::Manage.create_iptables_rules(ip_version, run_context)
     end
   end
-
+  not_if { modern_and_paranoid }
   action :nothing
 end
 
 ruby_block 'restart_iptables' do
   block do
-    class Chef::Resource::RubyBlock
-      include Iptables::Manage
-    end
-
-    if node['iptables-ng']['managed_service']
-      Array(node['iptables-ng']['enabled_ip_versions']).each do |ip_version|
-        restart_service(ip_version)
-      end
-    end
+    Array(node['iptables-ng']['enabled_ip_versions']).each do |ip_version|
+      Iptables::Manage.restart(ip_version, run_context)
+    end if node['iptables-ng']['managed_service']
   end
-
+  not_if { modern_and_paranoid }
   action :nothing
 end

spec/default_spec.rb

@@ -2,7 +2,7 @@
 
 describe 'iptables-ng::default' do
   let(:chef_run) do
-    ChefSpec::Runner.new do |node|
+    ChefSpec::ServerRunner.new do |node|
       node.automatic.merge!(JSON.parse(File.read('test/nodes/test.json')))
     end.converge(described_recipe)
   end

spec/install_spec.rb

@@ -4,7 +4,7 @@
   describe 'debian' do
 
     let(:chef_run) do
-      ChefSpec::Runner.new do |node|
+      ChefSpec::ServerRunner.new do |node|
         node.automatic['platform_family'] = 'debian'
       end.converge(described_recipe)
     end
@@ -15,7 +15,7 @@
   end
 
   let(:chef_run) do
-    ChefSpec::Runner.new do |node|
+    ChefSpec::ServerRunner.new do |node|
       node.automatic.merge!(JSON.parse(File.read('test/nodes/test.json')))
     end.converge(described_recipe)
   end

test/cookbooks/iptables_ng_test/recipes/recipe_default.rb

@@ -1 +1,4 @@
 include_recipe 'iptables-ng::default'
+
+# Uncomment to verify 'safe-reload' mode
+# execute 'false'