-
-
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 ignoreip parameter to jail class and template #60
Add ignoreip parameter to jail class and template #60
Conversation
|
Hi @leonkoens, thanks for the PR. Can you please document the new parameter in the README.md? |
manifests/jail.pp
Outdated
| @@ -14,6 +14,7 @@ | |||
| Integer $bantime = $fail2ban::bantime, | |||
| Optional[String] $port = undef, | |||
| Optional[String] $backend = undef, | |||
| Optional[Array] $ignoreip = undef, | |||
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.
Please take a look at our review guidelines:
Are there new params with datatype Hash or Array? If possible, they should default to empty Hash/Array instead of undef. You can also enforce the datastructure like Array[String]
Can you please migration the Datatype to something like Array[String[1]]? What kind of ip addresses are valid? Stdlib has quite some datatypes. One of them is probably useful in this case so it could even be Array[Stdlib::IP::Address]
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.
We probably need to update the lowest supported stdlib version in the metadata.json as well, since the new types got introduces in stdlib 4.25.0.
manifests/jail.pp
Outdated
| @@ -14,6 +14,7 @@ | |||
| Integer $bantime = $fail2ban::bantime, | |||
| Optional[String] $port = undef, | |||
| Optional[String] $backend = undef, | |||
| Optional[Array[Stdlib::IP::Address]] $ignoreip = [], | |||
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.
you can remove the Optional[]
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.
Heh, I was wondering if this was still necessary. Removing it.
…arameter Add ignoreip parameter to jail class and template
Pull Request (PR) description
This pull request adds support for defining ignoreip's in the fail2ban jails.