[BUG] On CentOS 7, enabled modules are reported as changed on every highstate

[iaingeorgeson]

Your setup

Formula commit hash / release tag

apache-formula v1.1.8

Versions reports (master & minion)

Salt 3003.3
Master: Debian 10
Minion: CentOS 7.8.2003

Pillar / config used

In pillar:

apache:
  modules:
    enabled:
      - proxy
      - proxy_ajp
      - proxy_http
      - rewrite
      - autoindex
    disabled:
      - access_compat
      - status

include
  - apache.config.modules
  ...

---

Bug details

Describe the bug

On CentOS 7, the formula does not try to detect whether modules are already enabled. Therefore it tries to use sed to enable the listed modules on every highstate, interprets the command as causing a change, and restarts httpd.

Steps to reproduce the bug

Use apache-formula to manage apache on CentOS 7 -

List modules in pillar under
apache:
modules:
enabled:

and include apache.config.modules.

Expected behaviour

Only report a change, and therefore only restart httpd, when the configuration is actually changed.

Attempts to fix the bug

The quick fix is probably to activate the onlyif test for CentOS 7 as well as Arch in

https://github.com/saltstack-formulas/apache-formula/blob/v1.1.8/apache/config/modules/install.sls#L27

A better fix would probably for this formula to completely rewrite the apache configuration, and manage it in the same way on every OS.

Additional context

[doubletwist13]

Part of the problem is that it's running the sed command even if the module is already loaded or statically compiled in. That seems backwards from what you'd want.

I can't speak to what Arch needs and I don't have any in my environment, but I've split out the RedHat family to do its own thing, and to only run the sed command UNLESS it's already loaded/compiled in.

  {% elif grains.os_family in ('RedHat') %}

  cmd.run:
    - name: find /etc/httpd/ -name '*.conf' -type f -exec sed -i -e 's/\(^#\)\(\s*LoadModule.{{ module }}_module\)/\2/g' {} \;
    - unless: (httpd -M 2> /dev/null |grep "[[:space:]]{{ module }}_module")

  {% elif grains.os_family in ('Arch') %}
 
 cmd.run:
    - name: find /etc/httpd/ -name '*.conf' -type f -exec sed -i -e 's/\(^#\)\(\s*LoadModule.{{ module }}_module\)/\2/g' {} \;
    - onlyif: {{ grains.os_family in ('Arch',) and 'true' }} || (httpd -M 2> /dev/null |grep "[[:space:]]{{ module }}_module")