Update IPTables save method#1003
Conversation
grkvlt
left a comment
grkvlt
left a comment
There was a problem hiding this comment.
Add check that command exists. Also look at IptablesCommandsTest and try to add a useful test for this?
| public static String saveIptablesRules() { | ||
| return alternatives(sudo("service iptables save"), | ||
| chain(installPackage("iptables-persistent"), sudo("/etc/init.d/iptables-persistent save"))); | ||
| return alternatives("if [ ${UID} -eq 0 ] ; then iptables–save > /etc/sysconfig/iptables ; else sudo iptables-save | sudo tee /etc/sysconfig/iptables ; fi", |
There was a problem hiding this comment.
Maybe surround this with BashCommands.ifExecutableElse1("iptables-save", ...) to ensure the command exists
That is, doesn't work with the CentOS 7 AMI we use on EC2, which has a cut-down |
|
Thanks @frogfather LGTM, will merge assuming tests pass |
|
I think Jenkins is failing due to permissions, maybe on the ASF side? |
|
LGTM; happy for this to be merged (once confusion with the identical-looking #1006 is cleared up). |
| return alternatives(sudo("service iptables save"), | ||
| chain(installPackage("iptables-persistent"), sudo("/etc/init.d/iptables-persistent save"))); | ||
| return alternatives( | ||
| ifExecutableElse1("iptables–save", "if [ ${UID} -eq 0 ] ; then iptables–save > /etc/sysconfig/iptables ; else sudo iptables-save | sudo tee /etc/sysconfig/iptables ; fi"), |
There was a problem hiding this comment.
this removes service iptables save altogether, is that intended? a comment to that effect would be useful,
or if it is still useful sometimes (older OS's?) then keep it in the alternatives list
also worth checking whether sudoNew("iptables-save > /etc/sysconfig/iptables") works; the way it does echo COMMAND | sudo bash should support redirect for non-root users; if not maybe refactor to add a sudoRedirect method in BashCommands capturing the conditional tee so that you could write ifExecutableElse1("iptables-save", sudoRedirect("iptable-save", "/etc/sysconfig/iptables"))
|
as @aledsage this is largely included in #1006 . @frogfather your call whether there is value in the recent comments here and if so update this PR, make sure to |
|
@frogfather Can you take a look at this please to see if it is still relevant, and address the comments above if appropriate Thanks |
|
Will do. It looks like #1006 (which was similar) was closed.
…Sent from my iPhone
On 26 Nov 2019, at 11:40, Martin Harris ***@***.***> wrote:
@frogfather Can you take a look at this please to see if it is still relevant, and address the comments above if appropriate
Thanks
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
5 participants
Service iptables save doesn't work with Centos7