-
-
Notifications
You must be signed in to change notification settings - Fork 106
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
Add unit test for checking sender address #75
Conversation
|
Transient Travis-CI failure 🙄 Trying to be much more lucky by closing/reopening PR… |
| @@ -72,7 +72,7 @@ destemail = <%= scope['::fail2ban::email'] %> | |||
| sendername = Fail2Ban | |||
|
|
|||
| # Email address of the sender | |||
| sender = fail2ban@localhost | |||
| sender = <%= scope['::fail2ban::sender'] %> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please use @fail2ban::sender to access the variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you meant $fail2ban::sender, but this is an erb template, not an epp one 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The desired way to access a variable in an erb template is @class::variable:
https://puppet.com/docs/puppet/5.5/lang_template_erb.html#variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tested to be absolutely certain that it is not working as expected in erb.
@fail2ban is nil, and nil::sender is basically nil.sender, so:
fail2ban
on debian-9-x86_64
fail2ban::config
when content template
should contain File[fail2ban.conf] with ensure => "present", content =~ /THIS FILE IS MANAGED BY PUPPET/, notify => "Service[fail2ban]", require => "Package[fail2ban]" and content =~ /^chain = INPUT$/ (FAILED - 1)
when content template (custom)
should contain File[fail2ban.conf] with ensure => "present", content =~ /THIS FILE IS MANAGED BY PUPPET/, notify => "Service[fail2ban]", require => "Package[fail2ban]" and content =~ /^chain = INPUT$/ (FAILED - 2)
when iptables chain provided
should contain File[fail2ban.conf] with content =~ /^chain = TEST$/ (FAILED - 3)
when custom banaction is provided
should contain File[fail2ban.conf] with content =~ /^banaction = iptables$/ (FAILED - 4)
[...]
Failures:
1) fail2ban on debian-9-x86_64 fail2ban::config when content template should contain File[fail2ban.conf] with ensure => "present", content =~ /THIS FILE IS MANAGED BY PUPPET/, notify => "Service[fail2ban]", require => "Package[fail2ban]" and content =~ /^chain = INPUT$/
Failure/Error: return call_function('template', template_name) unless template_name.end_with?('.epp')
Puppet::PreformattedError:
Evaluation Error: Error while evaluating a Function Call, Failed to parse template fail2ban/stretch/etc/fail2ban/jail.conf.erb:
Filepath: /usr/home/romain/Projects/puppet/modules/fail2ban/spec/fixtures/modules/fail2ban/templates/stretch/etc/fail2ban/jail.conf.erb
Line: 75
Detail: undefined method `sender' for nil:NilClass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That maybe doesn't work because sender isn't a variable within the fail2ban::jail class. let's stick with the scope way.
|
Hi @smortex, thanks for the fix. Can you please add a test that sets the variable and verifies the file content? |
|
#76 this just got merged and should fix your travis issues. Can you please rebase against our latest master? |
3058979
to
ff28921
Compare
c9ad604
to
5234c39
Compare
|
@bastelfreak I added a commit that adds an acceptance test to show the problem on top of master, then moved the commit you previously saw on top if this to fix the issue. |
|
@smortex can you please rebase? Afterwards I can merge this. |
|
@bastelfreak I am working on it ;-) #77 has superseded these changes, but the unit test for checking the sender may still make sense. |
|
@bastelfreak I just rebased the branch on top of master with all that was added today \o/. Thank you for the hard work :-) |
|
Thanks! |
Pull Request (PR) description
The Debian Stretch template use a hardcoded sender address:fail2ban@localhost.This PR use the sender address configured with$fail2ban::senderinstead.The Debian Stetch template used to have a static sender address, ignoring
$fail2ban::sender. This problem was addressed as a side effect of #77 but a unit-test for the correct behavior still makes sense.This Pull Request (PR) fixes the following issues
n/a