Extending rules template to support ORIGINAL DEST #19
Comments
|
I’ve added the comment by changing the affected in in the roles.j2 template:
{{ rule.action }} {{ rule.source }} {{ rule.dest }} {{ rule.proto }} {{ rule.dest_ports|join (',') }} {% if rule.comments is defined %}# {{ rule.comments }} {% endif %}
When comments field is added tot the dictionary, it will be added to the rules file.
From: Andreas Piening <notifications@github.com>
Sent: woensdag 6 mei 2020 17:11
To: mrlesmithjr/ansible-shorewall <ansible-shorewall@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Subject: [mrlesmithjr/ansible-shorewall] Extending rules template to support ORIGINAL DEST (#19)
I have the requirement to set the field ORIGINAL DEST in /etc/shorewall/rules in order to be able to match specific target IP's on a server.
This field is the seventh parameter in a rule definition, right after SOURCE PORT(S) which is also not filled by the current template.
As far as I see, there is just the line nr. 6 in templates/etc/shorewall/rules.j2 to be modified in order to achieve this:
{{ rule.action }} {{ rule.source }} {{ rule.dest }} {{ rule.proto }} {{ rule.dest_ports|join (',') }} {{ rule.source_ports|join (',') }} {{ rule.original_dest }}
Another idea (that has also been addressed in the pull request #18<#18>) is to add support for comments.
This could be done by adding these three lines right before the rule line inside the loop:
{% if rule.comment|length %}
#{{ rule.comment }}
{% endif %}
Of course this is not a issue, it is more a suggestion. I think it would be easy to apply and should not break something because if the variables are empty the template expands to the same rule line.
What do you think?
Btw. just want to say thank you for your ansible role. It is the most often used role in my playbooks.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#19>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIUOCEMKNSBYXM6ZRGTD66DRQF4YFANCNFSM4M2Q5XQQ>.
|
Hi Roger, I would prefer the optional comment on top of the rule line instead at the end, especially when a lot of options are set (like the suggested What about merging these changes into master? |
|
Hi Andreas,
I had to make some more changes specifically for our environment and didn’t cherry pick any parts as potential pull requests. I’ve committed them in our on fork: MaastrichtUniversity/ansible-shorewall@a46898a
Feel free to add these changes to your own fork or mrlesmithjr if you like.
From: Andreas Piening <notifications@github.com>
Sent: donderdag 7 mei 2020 16:25
To: mrlesmithjr/ansible-shorewall <ansible-shorewall@noreply.github.com>
Cc: Niesten, Roger (FACBURFHML) <r.niesten@maastrichtuniversity.nl>; Comment <comment@noreply.github.com>
Subject: Re: [mrlesmithjr/ansible-shorewall] Extending rules template to support ORIGINAL DEST (#19)
I’ve added the comment by changing the affected in in the roles.j2 template: {{ rule.action }} {{ rule.source }} {{ rule.dest }} {{ rule.proto }} {{ rule.dest_ports|join (',') }} {% if rule.comments is defined %}# {{ rule.comments }} {% endif %} When comments field is added tot the dictionary, it will be added to the rules file.
Hi Roger,
which rules.j2 template did you update? Can't see the change in the master branch and this seems to be the only branch.
I would prefer the optional comment on top of the rule line instead at the end, especially when a lot of options are set (like the suggested ORIGINAL DEST field) the rules get pretty long which makes them harder to read with a comment at the end that would potentially be wrapped to a new line by the editor. But that's just cosmetics.
What about merging these changes into master?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub<#19 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIUOCEISMK6X7HL62UM45N3RQLAE5ANCNFSM4M2Q5XQQ>.
|
|
Hi Roger, ok I see. If the extension to support @mrlesmithjr What do you think? Is it possible to update the With kind regards Andreas |
|
@apiening @rogerniesten Glad both of you are getting some use out of this role. And I'd absolutely be open to any pull requests to add additional functionality, etc. |
|
@mrlesmithjr Thank you. I've just committed a pull request (#20). I have commented the change in the PR and only one file is affected (rules.j2). I would be happy if you would accept the PR or discuss if anything is unclear or need to be changed. |
|
Since the PR has been merged into master, I'm closing this issue. |
I have the requirement to set the field ORIGINAL DEST in /etc/shorewall/rules in order to be able to match specific target IP's on a server.
This field is the seventh parameter in a rule definition, right after SOURCE PORT(S) which is also not filled by the current template.
As far as I see, there is just the line nr. 6 in templates/etc/shorewall/rules.j2 to be modified in order to achieve this:
Another idea (that has also been addressed in the pull request #18) is to add support for comments.
This could be done by adding these three lines right before the rule line inside the loop:
Of course this is not a issue, it is more a suggestion. I think it would be easy to apply and should not break something because if the variables are empty the template expands to the same rule line.
What do you think?
Btw. just want to say thank you for your ansible role. It is the most often used role in my playbooks.
The text was updated successfully, but these errors were encountered: