add install_method parameter #104

Dan33l · 2020-05-02T09:32:43Z

Pull Request (PR) description

This PR add parameter install_method that permits to choose between package and vcsrepo. This permit to install more recent version of ferm when desired.

This Pull Request (PR) fixes the following issues

Fixes #103

manifests/config.pp

Dan33l · 2020-05-04T21:42:12Z

Note to myself, the CentOS6 is failing to start service ferm with message :

starting firewall fermFATAL: Could not load /lib/modules/4.15.0-74-generic/modules.dep: No such file or directory
FATAL: Could not load /lib/modules/4.15.0-74-generic/modules.dep: No such file or directory
FATAL: Could not load /lib/modules/4.15.0-74-generic/modules.dep: No such file or directory
FATAL: Could not load /lib/modules/4.15.0-74-generic/modules.dep: No such file or directory

The directory is empty.

We already have the message on master branch.

Dan33l · 2020-05-10T17:54:11Z

On debian10, it looks that iptables-save point to /usr/sbin/xtables-nft-multi.

The acceptance test uses iptables-save to verify rules. But to check rules with install_method => vcsrepo we have to use iptables-legacy-save instead of iptables-save

Edit:
Extract from CHANGELOG file aka NEWS:

v2.5 - 22 Nov 2019

call "legacy" xtables tools because nft based tools are incompatible

MaxKellermann/ferm@47b78b6

The ferm version shipped by debian10 package is 2.4.
This explain why the result is different with last ferm sersion 2.5.1

bastelfreak · 2020-05-16T21:36:06Z

manifests/install.pp

+        ensure => absent,
+      }
+
+      -> package { ['iptables', 'perl', 'make']:


at least perl and make could be managed by another module, should be use ensure_package here?

The function ensure_packages() is now used.

bastelfreak · 2020-05-18T09:34:46Z

manifests/config.pp

@@ -10,6 +10,19 @@

  $_ip = join($ferm::ip_versions, ' ')

+  if $facts['systemd'] { #fact provided by systemd module
+    if $ferm::install_method == 'vcsrepo' and $ferm::manage_service {
+      include systemd::systemctl::daemon_reload


this isn't required. systemd::dropin_file does the notify:
https://github.com/camptocamp/puppet-systemd/blob/master/manifests/dropin_file.pp#L93-L103

bastelfreak · 2020-05-18T09:45:48Z

manifests/init.pp

@@ -67,6 +70,9 @@
  Hash $chains = {},
  Array[Enum['ip','ip6']] $ip_versions = ['ip','ip6'],
  Hash[String[1],Array[String[1]]] $preserve_chains_in_tables = {},
+  Enum['package','vcsrepo'] $install_method = 'package',
+  String $vcsrepo = 'https://github.com/MaxKellermann/ferm.git',
+  String $vcstag = 'v2.5.1',


Suggested change

String $vcstag = 'v2.5.1',

String[1] $vcstag = 'v2.5.1',

bastelfreak · 2020-05-18T09:46:25Z

manifests/init.pp

@@ -67,6 +70,9 @@
  Hash $chains = {},
  Array[Enum['ip','ip6']] $ip_versions = ['ip','ip6'],
  Hash[String[1],Array[String[1]]] $preserve_chains_in_tables = {},
+  Enum['package','vcsrepo'] $install_method = 'package',
+  String $vcsrepo = 'https://github.com/MaxKellermann/ferm.git',


Suggested change

String $vcsrepo = 'https://github.com/MaxKellermann/ferm.git',

Stdlib::HTTPSUrl $vcsrepo = 'https://github.com/MaxKellermann/ferm.git',

bastelfreak · 2020-05-18T09:46:57Z

manifests/install.pp

+    }
+    'vcsrepo': {
+      $_source_path = '/opt/ferm'
+      ensure_packages (['iptables', 'perl', 'make'], { ensure => present })


isn't git also a requirement that should be installed?

bastelfreak · 2020-05-18T09:47:32Z

spec/acceptance/ferm_spec.rb

@@ -43,12 +53,36 @@ class { 'ferm':
      },
    },
    ip_versions      => ['ip'], #only ipv4 available with CI
-  }


why did you delete this line?

to factorize the manifest and add at this end some parameters or not

templates/dropin_ferm.conf.epp

bastelfreak · 2020-05-18T12:08:43Z

spec/spec_helper_acceptance.rb

@@ -2,4 +2,5 @@

 configure_beaker do |host|
  install_package(host, 'epel-release') if fact_on(host, 'os.name') == 'CentOS'
+  install_package(host, 'git')


this isn't required anymore since we now manage the git package.

Dan33l · 2020-05-18T20:17:13Z

@bastelfreak thank you for the review

vox-pupuli-tasks · 2020-08-05T19:06:25Z

Dear @Dan33l, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs?
If you need any help, you can reach out to us on our IRC channel voxpupuli on Freenode or our Slack channel voxpupuli at slack.puppet.com.
You can find my sourcecode at voxpupuli/vox-pupuli-tasks

bastelfreak reviewed May 2, 2020

View reviewed changes

manifests/config.pp Outdated Show resolved Hide resolved

Dan33l changed the title ~~add install_method parameter~~ WIP : add install_method parameter May 2, 2020

Dan33l force-pushed the install_from_sources branch from e7f70c2 to 27a9214 Compare May 10, 2020 16:40

Dan33l added the enhancement New feature or request label May 10, 2020

Dan33l force-pushed the install_from_sources branch 3 times, most recently from 725d1cc to bc18180 Compare May 16, 2020 20:51

Dan33l changed the title ~~WIP : add install_method parameter~~ add install_method parameter May 16, 2020

bastelfreak reviewed May 16, 2020

View reviewed changes

Dan33l force-pushed the install_from_sources branch 5 times, most recently from b327d8c to d9369ba Compare May 18, 2020 08:45

bastelfreak reviewed May 18, 2020

View reviewed changes

templates/dropin_ferm.conf.epp Show resolved Hide resolved

Dan33l force-pushed the install_from_sources branch from d9369ba to 7605632 Compare May 18, 2020 11:01

bastelfreak reviewed May 18, 2020

View reviewed changes

Dan33l force-pushed the install_from_sources branch 4 times, most recently from 48bbd6b to 007a9bd Compare May 18, 2020 20:04

add install_method parameter

007a9bd

bastelfreak merged commit 1924c40 into voxpupuli:master May 18, 2020

Dan33l deleted the install_from_sources branch May 18, 2020 20:16

foxxx0 mentioned this pull request Jun 22, 2020

release 4.0.0 #113

Merged

vox-pupuli-tasks bot added the tests-fail label Aug 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add install_method parameter #104

add install_method parameter #104

Dan33l commented May 2, 2020

Dan33l commented May 4, 2020 •

edited

Loading

Dan33l commented May 10, 2020 •

edited

Loading

bastelfreak May 16, 2020

Dan33l May 18, 2020

bastelfreak May 18, 2020

bastelfreak May 18, 2020

bastelfreak May 18, 2020

bastelfreak May 18, 2020

bastelfreak May 18, 2020

Dan33l May 18, 2020

bastelfreak May 18, 2020

Dan33l commented May 18, 2020

vox-pupuli-tasks bot commented Aug 5, 2020

	String $vcsrepo = 'https://github.com/MaxKellermann/ferm.git',
	Stdlib::HTTPSUrl $vcsrepo = 'https://github.com/MaxKellermann/ferm.git',

add install_method parameter #104

add install_method parameter #104

Conversation

Dan33l commented May 2, 2020

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Dan33l commented May 4, 2020 • edited Loading

Dan33l commented May 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dan33l commented May 18, 2020

vox-pupuli-tasks bot commented Aug 5, 2020

Dan33l commented May 4, 2020 •

edited

Loading

Dan33l commented May 10, 2020 •

edited

Loading