Add identifier option to apache2_module (#33748)

* Add identifier option to apache2_module

There is a convention connecting the name passed to a2enmod and the one
appearing in apache2ctl -M. Not all modules follow this convention and
we have added a growing list of implicit conversions.
As a better long-term solution this adds an "identifier" option to be
able to set both strings explicitly.

* Run debian-specific tests only there

* Improve cleanup after apache2 tests

This is a follow-up/extension of https://github.com/ansible/ansible/pull/33630

* Add example for the new identifier option

* Put all debian tests in a block

lib/ansible/modules/web_infrastructure/apache2_module.py

@@ -21,24 +21,31 @@ author:
     - Christian Berendt (@berendt)
     - Ralf Hertel (@n0trax)
     - Robin Roth (@robinro)
-short_description: enables/disables a module of the Apache2 webserver
+short_description: Enables/disables a module of the Apache2 webserver.
 description:
    - Enables or disables a specified module of the Apache2 webserver.
 options:
    name:
      description:
-        - name of the module to enable/disable
+        - Name of the module to enable/disable as given to C(a2enmod/a2dismod).
      required: true
+   identifier:
+     description:
+         - Identifier of the module as listed by C(apache2ctl -M).
+           This is optional and usually determined automatically by the common convention of
+           appending C(_module) to I(name) as well as custom exception for popular modules.
+     required: False
+     version_added: "2.5"
    force:
      description:
-        - force disabling of default modules and override Debian warnings
+        - Force disabling of default modules and override Debian warnings.
      required: false
      choices: ['True', 'False']
      default: False
      version_added: "2.1"
    state:
      description:
-        - indicate the desired state of the resource
+        - Desired state of the module.
      choices: ['present', 'absent']
      default: present
    ignore_configcheck:
@@ -69,6 +76,11 @@ EXAMPLES = '''
     state: absent
     name: mpm_worker
     ignore_configcheck: True
+# enable dump_io module, which is identified as dumpio_module inside apache2
+- apache2_module:
+    state: present
+    name: dump_io
+    identifier: dumpio_module
 '''
 
 RETURN = '''
@@ -119,15 +131,12 @@ def _get_ctl_binary(module):
 
 def _module_is_enabled(module):
     control_binary = _get_ctl_binary(module)
-    name = module.params['name']
-    ignore_configcheck = module.params['ignore_configcheck']
-
     result, stdout, stderr = module.run_command("%s -M" % control_binary)
 
     if result != 0:
         error_msg = "Error executing %s: %s" % (control_binary, stderr)
-        if ignore_configcheck:
-            if 'AH00534' in stderr and 'mpm_' in name:
+        if module.params['ignore_configcheck']:
+            if 'AH00534' in stderr and 'mpm_' in module.params['name']:
                 module.warnings.append(
                     "No MPM module loaded! apache2 reload AND other module actions"
                     " will fail if no MPM module is loaded immediately."
@@ -138,7 +147,7 @@ def _module_is_enabled(module):
         else:
             module.fail_json(msg=error_msg)
 
-    searchstring = ' ' + create_apache_identifier(name)
+    searchstring = ' ' + module.params['identifier']
     return searchstring in stdout
 
 
@@ -205,7 +214,18 @@ def _set_state(module, state):
                              result=success_msg,
                              warnings=module.warnings)
         else:
-            module.fail_json(msg="Failed to set module %s to %s: %s" % (name, state_string, stdout),
+            msg = (
+                'Failed to set module {name} to {state}:\n'
+                '{stdout}\n'
+                'Maybe the module identifier ({identifier}) was guessed incorrectly.'
+                'Consider setting the "identifier" option.'
+            ).format(
+                name=name,
+                state=state_string,
+                stdout=stdout,
+                identifier=module.params['identifier']
+            )
+            module.fail_json(msg=msg,
                              rc=result,
                              stdout=stdout,
                              stderr=stderr)
@@ -219,6 +239,7 @@ def main():
     module = AnsibleModule(
         argument_spec=dict(
             name=dict(required=True),
+            identifier=dict(required=False, type='str'),
             force=dict(required=False, type='bool', default=False),
             state=dict(default='present', choices=['absent', 'present']),
             ignore_configcheck=dict(required=False, type='bool', default=False),
@@ -232,6 +253,9 @@ def main():
     if name == 'cgi' and _run_threaded(module):
         module.fail_json(msg="Your MPM seems to be threaded. No automatic actions on module %s possible." % name)
 
+    if not module.params['identifier']:
+        module.params['identifier'] = create_apache_identifier(module.params['name'])
+
     if module.params['state'] in ['present', 'absent']:
         _set_state(module, module.params['state'])
 

test/integration/targets/apache2_module/tasks/actualtest.yml

@@ -32,6 +32,7 @@
   apache2_module:
     name: userdir
     state: absent
+  register: userdir_first_disable
 
 - name: disable userdir module, second run
   apache2_module:
@@ -42,7 +43,7 @@
 - name: ensure apache2_module is idempotent
   assert:
     that:
-      - 'not disable.changed'
+      - disable is not changed
 
 - name: enable userdir module
   apache2_module:
@@ -53,7 +54,7 @@
 - name: ensure changed on successful enable
   assert:
     that:
-      - 'enable.changed'
+      - enable is changed
 
 - name: enable userdir module, second run
   apache2_module:
@@ -77,25 +78,81 @@
     that:
       - 'disablefinal.changed'
 
+- name: set userdir to original state
+  apache2_module:
+    name: userdir
+    state: present
+  when: userdir_first_disable is changed
+
 - name: ensure autoindex enabled
   apache2_module:
     name: autoindex
     state: present
 
-- name: force disable of autoindex  # bug #2499
+- name: Debian/Ubuntu specific tests
+  when: "ansible_os_family == 'Debian'"
+  block:
+  - name: force disable of autoindex  # bug #2499
     apache2_module:
       name: autoindex
       state: absent
       force: True
-  when: "ansible_os_family == 'Debian'"
 
-- name: enable evasive module, test https://github.com/ansible/ansible/issues/22635
+  - name: reenable autoindex
+    apache2_module:
+      name: autoindex
+      state: present
+
+  - name: enable evasive module, test https://github.com/ansible/ansible/issues/22635
     apache2_module:
       name: evasive
       state: present
-  when: "ansible_os_family == 'Debian'"
 
-- name: disable mpm modules
+  - name: disable evasive module
+    apache2_module:
+      name: evasive
+      state: absent
+
+  - name: use identifier to enable module, fix for https://github.com/ansible/ansible/issues/33669
+    apache2_module:
+      name: dump_io
+      state: present
+    ignore_errors: True
+    register: enable_dumpio_wrong
+
+  - name: disable dump_io
+    apache2_module:
+      name: dump_io
+      identifier: dumpio_module
+      state: absent
+
+  - name: use identifier to enable module, fix for https://github.com/ansible/ansible/issues/33669
+    apache2_module:
+      name: dump_io
+      identifier: dumpio_module
+      state: present
+    register: enable_dumpio_correct_1
+
+  - name: ensure idempotency with identifier
+    apache2_module:
+      name: dump_io
+      identifier: dumpio_module
+      state: present
+    register: enable_dumpio_correct_2
+
+  - name: disable dump_io
+    apache2_module:
+      name: dump_io
+      identifier: dumpio_module
+      state: absent
+
+  - assert:
+      that:
+        - enable_dumpio_wrong is failed
+        - enable_dumpio_correct_1 is changed
+        - enable_dumpio_correct_2 is not changed
+
+  - name: disable mpm modules
     apache2_module:
       name: "{{ item }}"
       state: absent
@@ -104,23 +161,20 @@
       - mpm_worker
       - mpm_event
       - mpm_prefork
-  when: "ansible_os_family == 'Debian'"
 
-- name: enabled mpm_event
+  - name: enabled mpm_event
     apache2_module:
       name: mpm_event
       state: present
       ignore_configcheck: True
     register: enabledmpmevent
-  when: "ansible_os_family == 'Debian'"
 
-- name: ensure changed mpm_event
+  - name: ensure changed mpm_event
     assert:
       that:
         - 'enabledmpmevent.changed'
-  when: "ansible_os_family == 'Debian'"
 
-- name: switch between mpm_event and mpm_worker
+  - name: switch between mpm_event and mpm_worker
     apache2_module:
       name: "{{ item.name }}"
       state: "{{ item.state }}"
@@ -130,22 +184,19 @@
         state: absent
       - name: mpm_worker
         state: present
-  when: "ansible_os_family == 'Debian'"
 
-- name: ensure mpm_worker is already enabled
+  - name: ensure mpm_worker is already enabled
     apache2_module:
       name: mpm_worker
       state: present
     register: enabledmpmworker
-  when: "ansible_os_family == 'Debian'"
 
-- name: ensure mpm_worker unchanged
+  - name: ensure mpm_worker unchanged
     assert:
       that:
         - 'not enabledmpmworker.changed'
-  when: "ansible_os_family == 'Debian'"
 
-- name: try to disable all mpm modules with configcheck
+  - name: try to disable all mpm modules with configcheck
     apache2_module:
       name: "{{item}}"
       state: absent
@@ -155,16 +206,14 @@
       - mpm_prefork
     ignore_errors: yes
     register: remove_with_configcheck
-  when: "ansible_os_family == 'Debian'"
 
-- name: ensure configcheck fails task with when run without mpm modules
+  - name: ensure configcheck fails task with when run without mpm modules
     assert:
       that:
         - "{{ item.failed }}"
     with_items: "{{ remove_with_configcheck.results }}"
-  when: "ansible_os_family == 'Debian'"
 
-- name: try to disable all mpm modules without configcheck
+  - name: try to disable all mpm modules without configcheck
     apache2_module:
       name: "{{item}}"
       state: absent
@@ -173,4 +222,10 @@
       - mpm_worker
       - mpm_event
       - mpm_prefork
-  when: "ansible_os_family == 'Debian'"
+
+  - name: enabled mpm_event to restore previous state
+    apache2_module:
+      name: mpm_event
+      state: present
+      ignore_configcheck: True
+    register: enabledmpmevent

test/integration/targets/apache2_module/tasks/cleanup.yml

@@ -1,20 +0,0 @@
-# This file is part of Ansible
-#
-# Ansible is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# Ansible is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
-
-- name: uninstall libapache2-mod-evasive via apt
-  apt:
-    name: libapache2-mod-evasive
-    state: absent
-  when: "ansible_os_family == 'Debian'"

test/integration/targets/apache2_module/tasks/main.yml

@@ -1,11 +1,21 @@
 ---
 
+
 - name:
   block:
+    - name: get list of enabled modules
+      shell: apache2ctl -M | sort
+      register: modules_before
     - name: include only on supported systems
       include: actualtest.yml
   always:
-     - name: cleanup installed modules
-       include: cleanup.yml
+    - name: get list of enabled modules
+      shell: apache2ctl -M | sort
+      register: modules_after
+    - debug: var=modules_before
+    - debug: var=modules_after
+    - name: ensure that all test modules are disabled again
+      assert:
+        that: modules_before.stdout == modules_after.stdout
   when: ansible_os_family in ['Debian', 'Suse']
   # centos/RHEL does not have a2enmod/a2dismod