iptables states fail when state_aggregate option is enabled

[dstoliker]

Description of Issue

State files with iptables states will fail with a runtime error RuntimeError: maximum recursion depth exceeded in cmp when running Salt 2019.2 with state_aggregate enabled in /etc/salt/minion.

Setup

Enable state_aggregate in /etc/salt/minion:

state_aggregate: True

Create a state file like this example:

# cat /srv/salt/iptables-test.sls
append_accept_local_interface:
  iptables.append:
    - name: accept_local_interface
    - table: filter
    - chain: INPUT
    - in-interface: lo
    - jump: ACCEPT
    - save: True

append_accept_loopback_output:
  iptables.append:
    - table: filter
    - chain: OUTPUT
    - out-interface: lo
    - jump: ACCEPT
    - save: True

append_drop_non_loopback:
  iptables.append:
    - table: filter
    - chain: INPUT
    - source: 127.0.0.0/8
    - jump: DROP
    - save: True

Steps to Reproduce Issue

Apply the state file:

# salt-call --local state.sls iptables-test -l debug
[DEBUG   ] Reading configuration from /etc/salt/minion
[DEBUG   ] Using cached minion ID from /etc/salt/minion_id: 10.0.2.15
[DEBUG   ] Configuration file path: /etc/salt/minion
[WARNING ] Insecure logging configuration detected! Sensitive data may be logged.
[DEBUG   ] Grains refresh requested. Refreshing grains.
[DEBUG   ] Reading configuration from /etc/salt/minion
[DEBUG   ] Determining pillar cache
[DEBUG   ] LazyLoaded jinja.render
[DEBUG   ] LazyLoaded yaml.render
[DEBUG   ] LazyLoaded jinja.render
[DEBUG   ] LazyLoaded yaml.render
[DEBUG   ] LazyLoaded state.sls
[DEBUG   ] LazyLoaded direct_call.execute
[DEBUG   ] LazyLoaded saltutil.is_running
[DEBUG   ] LazyLoaded grains.get
[DEBUG   ] LazyLoaded config.get
[DEBUG   ] key: test, ret: _|-
[DEBUG   ] Updating roots fileserver cache
[DEBUG   ] Gathering pillar data for state run
[DEBUG   ] Determining pillar cache
[DEBUG   ] LazyLoaded jinja.render
[DEBUG   ] LazyLoaded yaml.render
[DEBUG   ] Finished gathering pillar data for state run
[INFO    ] Loading fresh modules for state activity
[DEBUG   ] LazyLoaded jinja.render
[DEBUG   ] LazyLoaded yaml.render
[DEBUG   ] In saltenv 'base', looking at rel_path 'iptables-test.sls' to resolve 'salt://iptables-test.sls'
[DEBUG   ] In saltenv 'base', ** considering ** path '/var/cache/salt/minion/files/base/iptables-test.sls' to resolve 'salt://iptables-test.sls'
[DEBUG   ] compile template: /var/cache/salt/minion/files/base/iptables-test.sls
[DEBUG   ] Jinja search path: [u'/var/cache/salt/minion/files/base']
[PROFILE ] Time (in seconds) to render '/var/cache/salt/minion/files/base/iptables-test.sls' using 'jinja' renderer: 0.0200610160828
[DEBUG   ] Rendered data from file: /var/cache/salt/minion/files/base/iptables-test.sls:
append_accept_local_interface:
  iptables.append:
    - name: accept_local_interface
    - table: filter
    - chain: INPUT
    - in-interface: lo
    - jump: ACCEPT
    - save: True

append_accept_loopback_output:
  iptables.append:
    - table: filter
    - chain: OUTPUT
    - out-interface: lo
    - jump: ACCEPT
    - save: True

append_drop_non_loopback:
  iptables.append:
    - table: filter
    - chain: INPUT
    - source: 127.0.0.0/8
    - jump: DROP
    - save: True

[DEBUG   ] Results of YAML rendering:
OrderedDict([(u'append_accept_local_interface', OrderedDict([(u'iptables.append', [OrderedDict([(u'name', u'accept_local_interface')]), OrderedDict([(u'table', u'filter')]), OrderedDict([(u'chain', u'INPUT')]), OrderedDict([(u'in-interface', u'lo')]), OrderedDict([(u'jump', u'ACCEPT')]), OrderedDict([(u'save', True)])])])), (u'append_accept_loopback_output', OrderedDict([(u'iptables.append', [OrderedDict([(u'table', u'filter')]), OrderedDict([(u'chain', u'OUTPUT')]), OrderedDict([(u'out-interface', u'lo')]), OrderedDict([(u'jump', u'ACCEPT')]), OrderedDict([(u'save', True)])])])), (u'append_drop_non_loopback', OrderedDict([(u'iptables.append', [OrderedDict([(u'table', u'filter')]), OrderedDict([(u'chain', u'INPUT')]), OrderedDict([(u'source', u'127.0.0.0/8')]), OrderedDict([(u'jump', u'DROP')]), OrderedDict([(u'save', True)])])]))])
[PROFILE ] Time (in seconds) to render '/var/cache/salt/minion/files/base/iptables-test.sls' using 'yaml' renderer: 0.00145411491394
[DEBUG   ] LazyLoaded config.option
[DEBUG   ] LazyLoaded iptables.version
[DEBUG   ] LazyLoaded iptables.mod_aggregate
[DEBUG   ] LazyLoaded state.gen_tag
[INFO    ] Running state [accept_local_interface] at time 22:02:42.290637
[INFO    ] Executing state iptables.append for [accept_local_interface]
[DEBUG   ] An exception occurred in this state: maximum recursion depth exceeded in cmp
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/salt/state.py", line 1931, in call
    self.format_slots(cdata)
  File "/usr/lib/python2.7/site-packages/salt/state.py", line 2069, in format_slots
    arg = salt.utils.data.decode(arg, keep=True)
  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 112, in decode
    preserve_dict_class, preserve_tuples, to_str)
  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 215, in decode_list
    preserve_dict_class, preserve_tuples, to_str)
  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 166, in decode_dict
    preserve_dict_class, preserve_tuples, to_str)
  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 215, in decode_list
    preserve_dict_class, preserve_tuples, to_str)
  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 166, in decode_dict
    preserve_dict_class, preserve_tuples, to_str)
  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 215, in decode_list
    preserve_dict_class, preserve_tuples, to_str)
  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 166, in decode_dict
    preserve_dict_class, preserve_tuples, to_str)

These go on repeating for quite a while...

  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 166, in decode_dict
    preserve_dict_class, preserve_tuples, to_str)
  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 215, in decode_list
    preserve_dict_class, preserve_tuples, to_str)
  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 166, in decode_dict
    preserve_dict_class, preserve_tuples, to_str)
  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 215, in decode_list
    preserve_dict_class, preserve_tuples, to_str)
  File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 173, in decode_dict
    elif isinstance(value, Mapping):
  File "/usr/lib64/python2.7/abc.py", line 141, in __instancecheck__
    subtype in cls._abc_negative_cache):
  File "/usr/lib64/python2.7/_weakrefset.py", line 73, in __contains__
    return wr in self.data
RuntimeError: maximum recursion depth exceeded in cmp

[INFO    ] Completed state [accept_local_interface] at time 22:02:42.345888 (duration_in_ms=55.251)
[INFO    ] Running state [append_accept_loopback_output] at time 22:02:42.346348
[INFO    ] Executing state iptables.append for [append_accept_loopback_output]
[DEBUG   ] LazyLoaded cmd.run
[INFO    ] Executing command '/sbin/iptables --wait -t filter -A OUTPUT --out-interface lo --__agg__ True --jump ACCEPT' in directory '/root'
[ERROR   ] Command '[u'/sbin/iptables', u'--wait', u'-t', u'filter', u'-A', u'OUTPUT', u'--out-interface', u'lo', u'--__agg__', u'True', u'--jump', u'ACCEPT']' failed with return code: 2
[ERROR   ] stdout: iptables v1.4.21: unknown option "--__agg__"
Try `iptables -h' or 'iptables --help' for more information.
[ERROR   ] retcode: 2
[ERROR   ] Command '/sbin/iptables --wait -t filter -A OUTPUT --out-interface lo --__agg__ True --jump ACCEPT' failed with return code: 2
[ERROR   ] output: iptables v1.4.21: unknown option "--__agg__"
Try `iptables -h' or 'iptables --help' for more information.
[ERROR   ] Failed to set iptables rule for append_accept_loopback_output.
Attempted rule was /sbin/iptables --wait -t filter -A OUTPUT  --out-interface lo --__agg__ True --jump ACCEPT for ipv4
[INFO    ] Completed state [append_accept_loopback_output] at time 22:02:42.387749 (duration_in_ms=41.4)
[INFO    ] Running state [append_drop_non_loopback] at time 22:02:42.388005
[INFO    ] Executing state iptables.append for [append_drop_non_loopback]
[INFO    ] Executing command '/sbin/iptables --wait -t filter -A INPUT --source 127.0.0.0/8 --__agg__ True --jump DROP' in directory '/root'
[ERROR   ] Command '[u'/sbin/iptables', u'--wait', u'-t', u'filter', u'-A', u'INPUT', u'--source', u'127.0.0.0/8', u'--__agg__', u'True', u'--jump', u'DROP']' failed with return code: 2
[ERROR   ] stdout: iptables v1.4.21: unknown option "--__agg__"
Try `iptables -h' or 'iptables --help' for more information.
[ERROR   ] retcode: 2
[ERROR   ] Command '/sbin/iptables --wait -t filter -A INPUT --source 127.0.0.0/8 --__agg__ True --jump DROP' failed with return code: 2
[ERROR   ] output: iptables v1.4.21: unknown option "--__agg__"
Try `iptables -h' or 'iptables --help' for more information.
[ERROR   ] Failed to set iptables rule for append_drop_non_loopback.
Attempted rule was /sbin/iptables --wait -t filter -A INPUT  --source 127.0.0.0/8 --__agg__ True --jump DROP for ipv4
[INFO    ] Completed state [append_drop_non_loopback] at time 22:02:42.428312 (duration_in_ms=40.307)
[DEBUG   ] File /var/cache/salt/minion/accumulator/140230354864912 does not exist, no need to cleanup
[DEBUG   ] LazyLoaded state.check_result
[DEBUG   ] LazyLoaded highstate.output
local:
----------
          ID: append_accept_local_interface
    Function: iptables.append
        Name: accept_local_interface
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python2.7/site-packages/salt/state.py", line 1931, in call
                  self.format_slots(cdata)
                File "/usr/lib/python2.7/site-packages/salt/state.py", line 2069, in format_slots
                  arg = salt.utils.data.decode(arg, keep=True)
                File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 112, in decode
                  preserve_dict_class, preserve_tuples, to_str)
                File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 215, in decode_list
                  preserve_dict_class, preserve_tuples, to_str)
                File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 166, in decode_dict
                  preserve_dict_class, preserve_tuples, to_str)
                File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 215, in decode_list
                  preserve_dict_class, preserve_tuples, to_str)
                File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 166, in decode_dict
                  preserve_dict_class, preserve_tuples, to_str)

... snip ...

                File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 215, in decode_list
                  preserve_dict_class, preserve_tuples, to_str)
                File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 166, in decode_dict
                  preserve_dict_class, preserve_tuples, to_str)
                File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 215, in decode_list
                  preserve_dict_class, preserve_tuples, to_str)
                File "/usr/lib/python2.7/site-packages/salt/utils/data.py", line 173, in decode_dict
                  elif isinstance(value, Mapping):
                File "/usr/lib64/python2.7/abc.py", line 141, in __instancecheck__
                  subtype in cls._abc_negative_cache):
                File "/usr/lib64/python2.7/_weakrefset.py", line 73, in __contains__
                  return wr in self.data
              RuntimeError: maximum recursion depth exceeded in cmp
     Started: 22:02:42.290637
    Duration: 55.251 ms
     Changes:
----------
          ID: append_accept_loopback_output
    Function: iptables.append
      Result: False
     Comment: Failed to set iptables rule for append_accept_loopback_output.
              Attempted rule was /sbin/iptables --wait -t filter -A OUTPUT  --out-interface lo --__agg__ True --jump ACCEPT for ipv4
     Started: 22:02:42.346349
    Duration: 41.4 ms
     Changes:
----------
          ID: append_drop_non_loopback
    Function: iptables.append
      Result: False
     Comment: Failed to set iptables rule for append_drop_non_loopback.
              Attempted rule was /sbin/iptables --wait -t filter -A INPUT  --source 127.0.0.0/8 --__agg__ True --jump DROP for ipv4
     Started: 22:02:42.388005
    Duration: 40.307 ms
     Changes:

Summary for local
------------
Succeeded: 0
Failed:    3
------------
Total states run:     3
Total run time: 136.958 ms

Above state works fine if state_aggregate is False. Same state also works fine if run on Salt 2018.3.x regardless of state_aggregate setting in config.

Versions Report

# salt-call --versions-report
Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Apr  9 2019, 14:30:50)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.6.1810 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-957.12.2.el7.x86_64
         system: Linux
        version: CentOS Linux 7.6.1810 Core

[waynew]

For reference - it doesn't look like salt/utils/data.py is handling any sort of cyclical reference, which is the final source of this problem.

I'm not sure what the real root cause is, that's creating the cyclical reference in the first place...

In either case we should either be a) throwing a proper Salty exception when we detect a cyclical reference b) skipping recursing through the cycle or c) allow both options.

[waynew]

FWIW I wasn't able to reproduce this in a docker container - a proper VM or something may be necessary.

@dstoliker quick question:

append_accept_local_interface:
  iptables.append:
    - name: accept_local_interface

If you remove name from that state, does it still crash with the recursion error? My hypothesis is that it should still crash, but as I'm unable to reproduce, I can't test it myself 😝

[dstoliker]

@waynew, your hypothesis is correct. Removing the name does not resolve the issue.

[stale]

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]

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

[calve]

For future readers, we worked around this bug by patching our
state_aggregates: True to an explicit list of aggregated modules; state_aggregates: [pkg] in our salt-master configuration.

Beware that setting aggregate: False in the iptable.append state might actually inkove iptables --aggregate False, as shown in test mode.

[dstoliker]

There appears to have been a regression at some point. I know it worked on some earlier versions of 2019.2.x and 3000.x, but in with any current versions now available in the repo, the same recursion error detailed above appears.

Using the same configuration and test state above on a CentOS 7 test machine, the recursion error appears, and no changes are applied with these versions:

• 2019.2.5 (py2)
• 2019.2.5 (py3)
• 3000.2 (py2)
• 3000.3 (py2)
• 3000.3 (py3)

Version 3001 behaves a bit differently. It completes successfully, changes are applied, but does report errors to the CLI.

# salt-call --local state.sls iptables-test
[ERROR   ] Caught a circular reference in data structure below.Cleaning and continuing execution.
[{'state': 'iptables', 'name': 'accept_local_interface', '__sls__': 'iptables-test', '__env__': 'base', '__id__': 'append_accept_local_interface', 'table': 'filter', 'chain': 'INPUT', 'in-interface': 'lo', 'jump': 'ACCEPT', 'save': True, 'order': 10000, 'fun': 'append', '__agg__': True, 'rules': [...]}, {'state': 'iptables', 'name': 'append_accept_loopback_output', '__sls__': 'iptables-test', '__env__': 'base', '__id__': 'append_accept_loopback_output', 'table': 'filter', 'chain': 'OUTPUT', 'out-interface': 'lo', 'jump': 'ACCEPT', 'save': True, 'order': 10001, 'fun': 'append', '__agg__': True}, {'state': 'iptables', 'name': 'append_drop_non_loopback', '__sls__': 'iptables-test', '__env__': 'base', '__id__': 'append_drop_non_loopback', 'table': 'filter', 'chain': 'INPUT', 'source': '127.0.0.0/8', 'jump': 'DROP', 'save': True, 'order': 10002, 'fun': 'append', '__agg__': True}]
NoneType: None
[ERROR   ] Caught a circular reference in data structure below.Cleaning and continuing execution.
[{'state': 'iptables', 'name': 'append_accept_loopback_output', '__sls__': 'iptables-test', '__env__': 'base', '__id__': 'append_accept_loopback_output', 'table': 'filter', 'chain': 'OUTPUT', 'out-interface': 'lo', 'jump': 'ACCEPT', 'save': False, 'order': 10001, 'fun': 'append', '__agg__': True, 'rules': [...]}, {'state': 'iptables', 'name': 'append_drop_non_loopback', '__sls__': 'iptables-test', '__env__': 'base', '__id__': 'append_drop_non_loopback', 'table': 'filter', 'chain': 'INPUT', 'source': '127.0.0.0/8', 'jump': 'DROP', 'save': False, 'order': 10002, 'fun': 'append', '__agg__': True}]
NoneType: None
[ERROR   ] Caught a circular reference in data structure below.Cleaning and continuing execution.
[{'state': 'iptables', 'name': 'append_drop_non_loopback', '__sls__': 'iptables-test', '__env__': 'base', '__id__': 'append_drop_non_loopback', 'table': 'filter', 'chain': 'INPUT', 'source': '127.0.0.0/8', 'jump': 'DROP', 'save': False, 'order': 10002, 'fun': 'append', '__agg__': True, 'rules': [...]}]
NoneType: None
local:
----------
          ID: append_accept_local_interface
    Function: iptables.append
        Name: accept_local_interface
      Result: True
     Comment: Set iptables rule for accept_local_interface to: /sbin/iptables --wait -t filter -A INPUT  --in-interface lo --jump ACCEPT for ipv4
              Set iptables rule for append_accept_loopback_output to: /sbin/iptables --wait -t filter -A OUTPUT  --out-interface lo --jump ACCEPT for ipv4
              Set iptables rule for append_drop_non_loopback to: /sbin/iptables --wait -t filter -A INPUT  --source 127.0.0.0/8 --jump DROP for ipv4
     Started: 16:55:05.740523
    Duration: 121.507 ms
     Changes:
              ----------
              locale:
                  - accept_local_interface
                  - append_accept_loopback_output
                  - append_drop_non_loopback
----------
          ID: append_accept_loopback_output
    Function: iptables.append
      Result: True
     Comment: iptables rule for append_accept_loopback_output already set (/sbin/iptables --wait -t filter -A OUTPUT  --out-interface lo --jump ACCEPT) for ipv4
              iptables rule for append_drop_non_loopback already set (/sbin/iptables --wait -t filter -A INPUT  --source 127.0.0.0/8 --jump DROP) for ipv4
     Started: 16:55:05.862263
    Duration: 22.723 ms
     Changes:
----------
          ID: append_drop_non_loopback
    Function: iptables.append
      Result: True
     Comment: iptables rule for append_drop_non_loopback already set (/sbin/iptables --wait -t filter -A INPUT  --source 127.0.0.0/8 --jump DROP) for ipv4
     Started: 16:55:05.885261
    Duration: 15.406 ms
     Changes:

Summary for local
------------
Succeeded: 3 (changed=1)
Failed:    0
------------
Total states run:     3
Total run time: 159.636 ms
[root@localhost ~]# iptables -L
Chain INPUT (policy ACCEPT)
target     prot opt source               destination
ACCEPT     all  --  anywhere             anywhere
DROP       all  --  loopback/8           anywhere

Chain FORWARD (policy ACCEPT)
target     prot opt source               destination

Chain OUTPUT (policy ACCEPT)
target     prot opt source               destination
ACCEPT     all  --  anywhere             anywhere
[root@localhost ~]# salt-call --versions-report
Salt Version:
           Salt: 3001

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.11.1
        libgit2: Not Installed
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: Not Installed
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.8 (default, Apr  2 2020, 13:34:55)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 17.0.0
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.1.4

System Versions:
           dist: centos 7 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-1127.el7.x86_64
         system: Linux
        version: CentOS Linux 7 Core

[sagetherage]

my mistake, the merged PR is from when we thought we fixed this and there is not a new PR to correct the regression, putting this back into Commit for Magnesium

[sagetherage]

@dstoliker we didn't get to this in Magnesium, so I am putting it in Aluminium to get it addressed and assigned Shane to take a look now.

[sagetherage]

Shane was able to get this in AL let me see when he can @twangboy

[sagetherage]

@dstoliker looks like this might be something else and putting this work into Silicon so we can dive into it