-
-
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
Support overriding configuration for default jails #145
base: master
Are you sure you want to change the base?
Conversation
Dear @deric, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
1 similar comment
Dear @deric, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
Dear @deric, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
This.... I was just looking for a way to do this :) Hope it get's merged. |
@bastelfreak There's slight inconsistency in naming. The
while for all other jails it seems to match the jail name. I guess this should be kept for backward compatibility? |
mmh good question. I guess we should stay backwards compatible. There are no other breaking changes at the moment (sometimes they are needed, but we try to always release a bunch of them). |
@deric since you're at it, there's no ssh-ddos on centos 7 either.... (at least none of my installs has it) |
@r3pek Yeah, I've noticed. It looks like Debian 8 it the only anomaly with It will avoid confusion in the future if we will support both names and documentation should probably mention just |
any status update on this? |
hello? sorry the ping ;) |
@deric is this still WIP? |
Yes, sorry. I'll try to finish this soon. |
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.
👀
sorry the ping guys, any news about this? |
Debian 8
Ubuntu 18.04 and 20.04
Ubuntu 16.04
CentOS 6, 7, RedHat 8, 8, OpenSuse 15
CentOS 6, 7
|
The config hash fail2ban::jails_config:
ssh:
port: ssh,2200
dropbear:
port:
- ssh
- 2201
selinux-ssh:
port:
- 'ssh'
- '2202' with little extra effort it might be possible to convert current fail2ban::jails:
- ssh
- ssh-ddos to fail2ban::jails:
ssh:
port: 22
nginx-botsearch: while supporting the old syntax (in order to change ports converting to Hash would be necessary). @bastelfreak , @igalic Let me know, what you think. Making this change later might be complicated. For most jails it should be possible to change the port, the remaining ones might be modified in separate PRs. Please squash the commits before merging. |
e8d6588
to
35299b6
Compare
db5582a
to
c295954
Compare
cb07ac7
to
a94d546
Compare
- `fail2ban::port` function to simplify syntax - Include param tags
@Dan33l All conflicts have been resolved, could you have a look at it? |
The PR addresses the same issue as #48 but with a bit more generic approach.
Using 2 level Hash might be possible to override basically any attribute in the template.
Using
lookup()
in templates might not be the best approach, but I can't think of better alternative. Passing explicitly each variable explicitly to the template would generate loads of code. Another option is to write a custom function for checking key existence in the configuration hash (but it doesn't add much to code readability).Let me know if you're ok with this, so that I'm able to proceed with modifying rest of jails.