saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.18k stars 5.48k forks source link

modules.iptables optimistically tries to read /etc/sysconfig/iptables #54459

Open vladionescu opened 5 years ago

vladionescu commented 5 years ago

https://github.com/saltstack/salt/blob/705e8cca0b9ad6441c3884e76798cac1ff7859d7/salt/modules/iptables.py#L977-L994

If modules.iptables.get_saved_rules() is called without an explicit dump file path (generated by iptables -S or iptables-save), then it assumes the default OS' iptables config path and tries to read it, but does not handle the case where that file doesn't exist.

All this happens at the top of modules.iptables._parse_conf().

Here's the unhandled exception when this happens:

sys-firewall:
  ----------
            ID: QBS-FORWARD-exists
      Function: iptables.chain_present
          Name: QBS-FORWARD
        Result: True
       Comment: iptables QBS-FORWARD chain is already exist in filter table for ipv4
       Started: 12:39:56.244931
      Duration: 6.656 ms
       Changes:   
  ----------
            ID: set-downstream-mtu
      Function: iptables.insert
        Result: False
       Comment: An exception occurred in this state: Traceback (most recent call last):
                  File /var/tmp/.root_62a99a_salt/pyall/salt/state.py, line 1933, in call
                    **cdata['kwargs'])
                  File /var/tmp/.root_62a99a_salt/pyall/salt/loader.py, line 1939, in wrapper
                    return f(*args, **kwargs)
                  File /var/tmp/.root_62a99a_salt/pyall/salt/states/iptables.py, line 573, in insert
                    saved_rules = __salt__['iptables.get_saved_rules'](family=family)
                  File /var/tmp/.root_62a99a_salt/pyall/salt/modules/iptables.py, line 561, in get_saved_rules
                    return _parse_conf(conf_file=conf_file, family=family)
                  File /var/tmp/.root_62a99a_salt/pyall/salt/modules/iptables.py, line 990, in _parse_conf
                    with salt.utils.files.fopen(conf_file, 'r') as ifile:
                  File /var/tmp/.root_62a99a_salt/pyall/salt/utils/files.py, line 399, in fopen
                    f_handle = open(*args, **kwargs)  # pylint: disable=resource-leakage
                IOError: [Errno 2] No such file or directory: u'/etc/sysconfig/iptables'
       Started: 12:39:56.252038
      Duration: 14.878 ms
       Changes:   

And the state that triggers it:

QBS-FORWARD-exists:
  iptables.chain_present:
    - name: QBS-FORWARD

set-downstream-mtu:
  iptables.insert:
    - chain: QBS-FORWARD
    - position: 1
    - protocol: tcp
    - match: tcp
    - tcp-flags: SYN,RST SYN
    - jump: TCPMSS
    - set-mss: 1320
    - save: True
    - require:
      - iptables: QBS-FORWARD-exists

The target minion (sys-firewall) indeed doesn't have anything at /etc/sysconfig/iptables

I have a fix for this, just need to fork and send PR.

xeacott commented 5 years ago

👍 Thanks for throwing this together! I'll check out the PR

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

stale[bot] commented 4 years ago

Thank you for updating this issue. It is no longer marked as stale.

sagetherage commented 4 years ago

Removing the fixed label since the PR was closed. @vladionescu can/will you open a PR against master?

vladionescu commented 4 years ago

I submitted this issue with a fix nearly 10 months ago, and it appears to still be broken.

Does Salt plan on fixing this and properly supporting iptables?

sagetherage commented 4 years ago

@vladionescu yes, I saw that PR was against the develop branch and since time has passed we are now making changes in the master branch and I wondered if you could open your same PR against the master branch, no is an acceptable answer. Since you opened the original PR I wanted to at least involve you in the conversation.