Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make before setting in jail.conf configurable #68

Merged
merged 3 commits into from
Aug 18, 2018

Conversation

arjenz
Copy link
Member

@arjenz arjenz commented Jun 17, 2018

"before" File is called 'paths-debian.conf' on osfamily Debian, add this setting to params, init and template.

Pull Request (PR) description

Account for different filenames on different operating systems fore the "before" include in jails.conf

This Pull Request (PR) fixes the following issues

@arjenz
Copy link
Member Author

arjenz commented Jun 18, 2018

Fixed the failing tests

@bastelfreak
Copy link
Member

Hi @arjenz, can you please rebase against our latest master branch?

@arjenz
Copy link
Member Author

arjenz commented Aug 1, 2018

@bastelfreak Done :)

@bastelfreak bastelfreak added enhancement New feature or request tests-fail and removed needs-rebase labels Aug 1, 2018
@@ -15,6 +15,7 @@
String $config_file_owner = $::fail2ban::params::config_file_owner,
String $config_file_group = $::fail2ban::params::config_file_group,
String $config_file_mode = $::fail2ban::params::config_file_mode,
String $config_file_before = $::fail2ban::params::before_file,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please use String[1]? This ensures people don't provide an empty string.

@@ -41,6 +41,12 @@
default => 'Package[fail2ban]',
}

$before_file = $::osfamily ? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use $facts['os']['family'] instead. The new facts hash is preferred to access facts.

@@ -41,6 +41,12 @@
default => 'Package[fail2ban]',
}

$before_file = $::osfamily ? {
'Debian' => 'paths-debian.conf',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome that you use a selector here to determine the value for $before_file. That is the right way to go, but rarely used.

@@ -19,7 +19,7 @@

[INCLUDES]

before = paths-fedora.conf
before = <%= scope['::fail2ban::before_file'] %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please mind that this is now an epp template and variables are accessed in a different way:
https://puppet.com/docs/puppet/5.5/lang_template_epp.html

@arjenz arjenz force-pushed the before_file branch 4 times, most recently from 91b4748 to 69fcaea Compare August 1, 2018 19:48
@arjenz
Copy link
Member Author

arjenz commented Aug 1, 2018

@bastelfreak Done, I think :)

arjenz and others added 3 commits August 17, 2018 11:38
"before" File is called 'paths-debian.conf' on osfamily Debian, add this setting to params, init and template.
Don't allow empty string as parameter values.
@arjenz
Copy link
Member Author

arjenz commented Aug 17, 2018

@bastelfreak Anything else I can do? :)

@bastelfreak
Copy link
Member

looks good, thanks!

@bastelfreak bastelfreak merged commit 07247c6 into voxpupuli:master Aug 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants