Merge pull request #247 from traylenator/overarch

New clobber_default_config paramater
voxpupuli · Jun 27, 2024 · de945d0 · de945d0
2 parents 1331dc3 + a528bf5
commit de945d0
Show file tree

Hide file tree

Showing 6 changed files with 186 additions and 13 deletions.
diff --git a/REFERENCE.md b/REFERENCE.md
@@ -178,6 +178,7 @@ The following parameters are available in the `nftables` class:
 * [`nft_path`](#-nftables--nft_path)
 * [`echo`](#-nftables--echo)
 * [`default_config_mode`](#-nftables--default_config_mode)
+* [`clobber_default_config`](#-nftables--clobber_default_config)
 
 ##### <a name="-nftables--out_all"></a>`out_all`
 
@@ -404,6 +405,16 @@ Data type: `Stdlib::Filemode`
 The default file & dir mode for configuration files and directories. The
 default varies depending on the system, and is set in the module's data.
 
+##### <a name="-nftables--clobber_default_config"></a>`clobber_default_config`
+
+Data type: `Boolean`
+
+Should the existing OS provided rules in the `configuration_path` be removed? If
+they are not being removed this module will add all of its configuration to the end of
+the existing rules.
+
+Default value: `false`
+
 ### <a name="nftables--bridges"></a>`nftables::bridges`
 
 allow forwarding traffic on bridges

diff --git a/manifests/init.pp b/manifests/init.pp
@@ -109,11 +109,17 @@
 #   The default file & dir mode for configuration files and directories. The
 #   default varies depending on the system, and is set in the module's data.
 #
+# @param clobber_default_config
+#   Should the existing OS provided rules in the `configuration_path` be removed? If
+#   they are not being removed this module will add all of its configuration to the end of
+#   the existing rules.
+#
 class nftables (
   Stdlib::Unixpath $echo,
   Stdlib::Unixpath $configuration_path,
   Stdlib::Unixpath $nft_path,
   Stdlib::Filemode $default_config_mode,
+  Boolean $clobber_default_config = false,
   Boolean $in_ssh = true,
   Boolean $in_icmp = true,
   Boolean $out_ntp = true,
@@ -140,12 +146,30 @@
 ) {
   package { 'nftables':
     ensure => installed,
-  } -> file_line {
-    'enable_nftables':
-      line   => 'include "/etc/nftables/puppet.nft"',
-      path   => $configuration_path,
-      notify => Service['nftables'],
-  } -> file {
+  }
+
+  if $clobber_default_config {
+    file { $configuration_path:
+      ensure  => file,
+      owner   => 'root',
+      group   => 'root',
+      mode    => $default_config_mode,
+      content => "#Puppet Managed\ninclude \"/etc/nftables/puppet.nft\"\n",
+      require => Package['nftables'],
+      before  => File['/etc/nftables'],
+      notify  => Service['nftables'],
+    }
+  } else {
+    file_line { 'enable_nftables':
+      line    => 'include "/etc/nftables/puppet.nft"',
+      path    => $configuration_path,
+      require => Package['nftables'],
+      before  => File['/etc/nftables'],
+      notify  => Service['nftables'],
+    }
+  }
+
+  file {
     default:
       owner => 'root',
       group => 'root',

diff --git a/spec/acceptance/all_rules_spec.rb b/spec/acceptance/all_rules_spec.rb
@@ -6,17 +6,27 @@
   context 'configure all nftables rules' do
     it 'works idempotently with no errors' do
       pp = <<-EOS
+      # Default ArchLinux rules contain "destroy" that requires kernel >= 6.3
+      # https://gitlab.archlinux.org/archlinux/packaging/packages/nftables/-/commit/f26a7145b2885d298925819782a5302905332dbe
+      # When running on docker this may not be the case.
+      if $facts['os']['family'] == 'Archlinux' and versioncmp($facts['kernelrelease'],'6.3') < 0 {
+        $_clobber_default_config = true
+      } else {
+        $_clobber_default_config = undef
+      }
+
       # default mask of firewalld service fails if service is not installed.
       # https://tickets.puppetlabs.com/browse/PUP-10814
       # Disable all default rules and include below explicitly
       class { 'nftables':
-        firewalld_enable => false,
-        out_ntp          => false,
-        out_http         => false,
-        out_https        => false,
-        out_icmp         => false,
-        in_ssh           => false,
-        in_icmp          => false,
+        firewalld_enable       => false,
+        out_ntp                => false,
+        out_http               => false,
+        out_https              => false,
+        out_icmp               => false,
+        in_ssh                 => false,
+        in_icmp                => false,
+        clobber_default_config => $_clobber_default_config,
       }
       include nftables::rules::icmp
       include nftables::rules::dns

diff --git a/spec/acceptance/default_spec.rb b/spec/acceptance/default_spec.rb
@@ -6,10 +6,21 @@
   context 'configure default nftables service' do
     it 'works idempotently with no errors' do
       pp = <<-EOS
+
+      # Default ArchLinux rules contain "destroy" that requires kernel >= 6.3
+      # https://gitlab.archlinux.org/archlinux/packaging/packages/nftables/-/commit/f26a7145b2885d298925819782a5302905332dbe
+      # When running on docker this may not be the case.
+      if $facts['os']['family'] == 'Archlinux' and versioncmp($facts['kernelrelease'],'6.3') < 0 {
+        $_clobber_default_config = true
+      } else {
+        $_clobber_default_config = undef
+      }
+
       # default mask of firewalld service fails if service is not installed.
       # https://tickets.puppetlabs.com/browse/PUP-10814
       class { 'nftables':
         firewalld_enable => false,
+        clobber_default_config => $_clobber_default_config,
       }
       $config_path = $facts['os']['family'] ? {
         'Archlinux' => '/etc/nftables.conf',
@@ -61,8 +72,14 @@ class { 'nftables':
   context 'with bad invalid nft rules' do
     it 'puppet fails but should leave nft service running' do
       pp = <<-EOS
+      if $facts['os']['family'] == 'Archlinux' and versioncmp($facts['kernelrelease'],'6.3') < 0 {
+        $_clobber_default_config = true
+      } else {
+        $_clobber_default_config = undef
+      }
       class{'nftables':
         firewalld_enable => false,
+        clobber_default_config => $_clobber_default_config,
       }
       nftables::rule{'default_out-junk':
         content => 'A load of junk',
@@ -103,10 +120,16 @@ class { 'nftables':
   context 'with totally empty firewall' do
     it 'no rules validate okay' do
       pp = <<-EOS
+      if $facts['os']['family'] == 'Archlinux' and versioncmp($facts['kernelrelease'],'6.3') < 0 {
+        $_clobber_default_config = true
+      } else {
+        $_clobber_default_config = undef
+      }
       class{'nftables':
         firewalld_enable => false,
         inet_filter => false,
         nat => false,
+        clobber_default_config => $_clobber_default_config,
       }
       $config_path = $facts['os']['family'] ? {
         'Archlinux' => '/etc/nftables.conf',
@@ -144,10 +167,16 @@ class { 'nftables':
   context 'with custom nat_table_name' do
     it 'no rules validate okay' do
       pp = <<-EOS
+      if $facts['os']['family'] == 'Archlinux' and versioncmp($facts['kernelrelease'],'6.3') < 0 {
+        $_clobber_default_config = true
+      } else {
+        $_clobber_default_config = undef
+      }
       class{'nftables':
         firewalld_enable => false,
         nat => true,
         nat_table_name => 'mycustomtablename',
+        clobber_default_config => $_clobber_default_config,
       }
       $config_path = $facts['os']['family'] ? {
         'Archlinux' => '/etc/nftables.conf',
@@ -185,10 +214,16 @@ class { 'nftables':
   context 'with only an empty netdev table' do
     it 'rules validate okay' do
       pp = <<-EOS
+      if $facts['os']['family'] == 'Archlinux' and versioncmp($facts['kernelrelease'],'6.3') < 0 {
+        $_clobber_default_config = true
+      } else {
+        $_clobber_default_config = undef
+      }
       class{'nftables':
         firewalld_enable => false,
         inet_filter => false,
         nat => false,
+        clobber_default_config => $_clobber_default_config,
       }
       nftables::config {
         'netdev-filter':

diff --git a/spec/acceptance/destroy_spec.rb b/spec/acceptance/destroy_spec.rb
@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+require 'spec_helper_acceptance'
+
+describe 'nftables class' do
+  context 'configure defaults destroyed nftables service' do
+    it 'works idempotently with no errors' do
+      pp = <<-EOS
+      # default mask of firewalld service fails if service is not installed.
+      # https://tickets.puppetlabs.com/browse/PUP-10814
+      class { 'nftables':
+        firewalld_enable       => false,
+        clobber_default_config => true,
+      }
+      $config_path = $facts['os']['family'] ? {
+        'Archlinux' => '/etc/nftables.conf',
+        'Debian' => '/etc/nftables.conf',
+        default => '/etc/sysconfig/nftables.conf',
+      }
+      $nft_path = $facts['os']['family'] ? {
+        'Archlinux' => '/usr/bin/nft',
+        default => '/usr/sbin/nft',
+      }
+      # nftables cannot be started in docker so replace service with a validation only.
+      systemd::dropin_file{"zzz_docker_nft.conf":
+        ensure  => present,
+        unit    => "nftables.service",
+        content => [
+          "[Service]",
+          "ExecStart=",
+          "ExecStart=${nft_path} -c -I /etc/nftables/puppet -f ${config_path}",
+          "ExecReload=",
+          "ExecReload=${nft_path} -c -I /etc/nftables/puppet -f ${config_path}",
+          "",
+          ].join("\n"),
+        notify  => Service["nftables"],
+      }
+      EOS
+      # Run it twice and test for idempotency
+      apply_manifest(pp, catch_failures: true)
+      apply_manifest(pp, catch_changes: true)
+    end
+
+    describe package('nftables') do
+      it { is_expected.to be_installed }
+    end
+
+    describe service('nftables') do
+      it { is_expected.to be_running }
+      it { is_expected.to be_enabled }
+    end
+
+    describe file('/etc/nftables/puppet.nft', '/etc/systemd/system/nftables.service.d/puppet_nft.conf') do
+      it { is_expected.to be_file }
+    end
+
+    describe file('/etc/nftables/puppet') do
+      it { is_expected.to be_directory }
+    end
+  end
+end
diff --git a/spec/classes/nftables_spec.rb b/spec/classes/nftables_spec.rb
@@ -33,6 +33,38 @@
 
       it { is_expected.to contain_package('nftables') }
 
+      context 'with clobber_default_config false' do
+        let(:params) do
+          { clobber_default_config: false }
+        end
+
+        it {
+          is_expected.to contain_file_line('enable_nftables').with(
+            line: 'include "/etc/nftables/puppet.nft"',
+            path: nft_config
+          )
+        }
+
+        it { is_expected.not_to contain_file(nft_config) }
+      end
+
+      context 'with clobber_default_config true' do
+        let(:params) do
+          { clobber_default_config: true }
+        end
+
+        it {
+          is_expected.to contain_file(nft_config).with(
+            ensure: 'file',
+            content: %r{^include "/etc/nftables/puppet.nft"$},
+            owner: 'root',
+            group: 'root'
+          )
+        }
+
+        it { is_expected.not_to contain_file_line('enable_nftables') }
+      end
+
       it {
         is_expected.to contain_file('/etc/nftables').with(
           ensure: 'directory',