Remove `firewalld` stroolean from $simp_options::firewall before 6.5 release

Description

Some time after the SIMP 6.4 release, the Boolean $simp_options::firewall global catalyst was updated to be an Enum that also accepts firewalld as a value.

  1. $simp_options::firewall should be reverted to a Boolean before the release of SIMP 6.5, because the Stroolean implementation causes a lot of problems (see comments).

  2. The firewalld setting should be moved into a dedicated parameter in the iptables class (which contains the only line in the entire framework that uses it).

  3. (optional, discouraged) If a global catalyst is considered necessary for this special case, it should be a separately parameter from the boolean $simp_options::firewall.

Acceptance Criteria

None

Activity

Show:
Chris Tessmer
June 8, 2020, 7:03 PM
Edited

I noticed this issue while reviewing the missing global catalysts in default parameter logic in a recent simp_firewalld PR.

To summarize some of the problems:

  1. firewalld muddles the purpose $simp_options::firewall

    • The global catalyst was formerly a simple on/off setting that was clear to understand, easy to consume, and relevant to the entire SIMP framework and every OS.

    • Separating firewalld to another parameter would keep the logic in both firewall enablement and mode checks simple and focused.

  2. firewalld isn't an appropriate global catalyst:

    • Global catalysts orchestrate infrastructure configuration across multiple SIMP modules.

    • The stroolean firewalld value is only used in a single line in the iptables module across the entire SIMP framework.

    • The firewalld operational mode of the iptables module is a separate (and much less global) concept from whether or not to manage firewalls.

    • The firewalld value is only relevant to certain OSes―even when set, it is impossible to satisfy on OSes to that don't support firewalld (e.g., EL6, Windows)

  3. It is fragile and expensive:

    • To accommodate the new firewalld value, all code across the SIMP framework (and any users' profiles) that consumes $simp_options::firewall will require additional logic to handle the now-stroolean value safely, and tests to validate the logic.

    • Adding a special handling case to an otherwise boolean global catalyst is likely to introduce edge case bugs in unrelated modules, and the two-jobs-in-one purpose of the setting makes any bugs found more difficult to troubleshoot

    • The catalyst can only describe the firewall is enabled (true) or {{firewalld}}―it cannot describe any other firewall provider (like iptables or Windows).

      • Not that we'd want it to: those are even less likely to be globally applicable, and adding additional values for them in the future would further compound the implementation and maintenance overhead this described above across the whole framework.

Trevor Vaughan
June 9, 2020, 7:35 PM

Yeah, fair enough.

Here's the only question relating to this though:

  • Do we enable firewalld by default on all systems that natively support it?

Personally, I think this is a good idea however we only want to trigger that default if firewalld is actively running. Flipping existing users from iptables to firewalld without any warning would be...really, really, bad.

Alternatively, we can keep it opt-in for OS versions < EL8

Labels

Epic Link

None

Story Points

1

Assignee

Trevor Vaughan

Sprint

None

Affects versions

Priority

High
Configure