State firewalld.present disables masquerade when making unrelated changes

[craigafinch]

Description of Issue/Question

Setup

I have a pair of states like this, which add a service a server's firewalld configuration:

icinga2-firewall-service:
  firewalld.service:
    - name: icinga2
    - ports:
      - 5665/tcp

icinga2-firewall:
  firewalld.present:
    - name: public
    - services:
      - icinga2
    - require:
      - icinga2-firewall-service
    - prune_services: False

Steps to Reproduce Issue

1. Start with a server that's running firewalld and has masquerade enabled
2. Apply the states shown above

What actually happens: masquerade will be disabled

What should happen: No change in masquerade status, unless I have specifically requested a change.

When I run this state on a server that is running firewalld and has masquerade enabled, this state disables masquerade, even though I didn't request that change. This unexpected behavior is a major problem, since it disables masquerade on a server whose primary function is to serve as a NAT gateway. Fortunately, I have another state that explicitly enables masquerade on that server, so the result is a temporary service interruption instead of a major outage.

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: 3.6.1
         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.10.1.el7.x86_64
         system: Linux
        version: CentOS Linux 7.6.1810 Core

Minion:

salt-minion 2019.2.0 (Fluorine)

References

1. https://github.com/saltstack/salt/issues/43912 (the present bug may have been introduced when this bug was fixed)
2. https://github.com/saltstack/salt/issues/41075 (my previous issue related to modularity of states that manage firewalld)

[twangboy]

@saltstack/team-core Any ideas?

[DmitryKuzmenko]

firewalld.present state has a bool argument masqueradethat is False by default. @craigafinch could you please try to set masquerade: True?

[stale[bot]]

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.

[stobbsm]

I have an issue that this applies to. I have many modular states to manage software and configuration for multiple servers, instead of a set of states for each server.

My problem is that when I install something like say postgres, the firewalld state turns off masquerade, which is after my libvirtd state turns masquerade on.

I can't set masquerade to on for every state that has some firewall ports or services to add, as not all my machines are libvirtd hosts.

Please advise how I can set masquerade once, and then other states that don't reference masquerade don't overwrite it.

ps. I can't guarantee when the firewalld state runs for libvirtd, so moving it around in the load order is not an options.

[DmitryKuzmenko]

Sounds reasonable @stobbsm . So the desired behavior of the firewalld.present state is if masquerade is not set in the state then don't change the state of it.

[ptitdoc]

Hello,

I have the same problem here. Ideally the default for masquerade should be None instead of False or something like this.

Especially because masquerade apply to the whole zone and not to a specific service or rule. It should not be changed when we add a service.