From ec80f8ad80f7c4b7a0199b10b7db34f8b9af9d49 Mon Sep 17 00:00:00 2001 From: Robin Roth Date: Tue, 23 Jan 2018 18:56:59 +0100 Subject: [PATCH] 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 --- .../web_infrastructure/apache2_module.py | 46 +++- .../apache2_module/tasks/actualtest.yml | 227 +++++++++++------- .../targets/apache2_module/tasks/cleanup.yml | 20 -- .../targets/apache2_module/tasks/main.yml | 14 +- 4 files changed, 188 insertions(+), 119 deletions(-) delete mode 100644 test/integration/targets/apache2_module/tasks/cleanup.yml diff --git a/lib/ansible/modules/web_infrastructure/apache2_module.py b/lib/ansible/modules/web_infrastructure/apache2_module.py index 5baa26dfe3..3a9ae568a8 100644 --- a/lib/ansible/modules/web_infrastructure/apache2_module.py +++ b/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']) diff --git a/test/integration/targets/apache2_module/tasks/actualtest.yml b/test/integration/targets/apache2_module/tasks/actualtest.yml index 79f75f3376..03d1914efb 100644 --- a/test/integration/targets/apache2_module/tasks/actualtest.yml +++ b/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,100 +78,154 @@ 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 - apache2_module: - name: autoindex - state: absent - force: True +- name: Debian/Ubuntu specific tests when: "ansible_os_family == 'Debian'" - -- 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 - apache2_module: - name: "{{ item }}" - state: absent - ignore_configcheck: True - with_items: - - mpm_worker - - mpm_event - - mpm_prefork - when: "ansible_os_family == 'Debian'" - -- 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 - assert: - that: - - 'enabledmpmevent.changed' - when: "ansible_os_family == 'Debian'" - -- name: switch between mpm_event and mpm_worker - apache2_module: - name: "{{ item.name }}" - state: "{{ item.state }}" - ignore_configcheck: True - with_items: - - name: mpm_event + block: + - name: force disable of autoindex # bug #2499 + apache2_module: + name: autoindex state: absent - - name: mpm_worker + force: True + + - name: reenable autoindex + apache2_module: + name: autoindex state: present - when: "ansible_os_family == 'Debian'" -- name: ensure mpm_worker is already enabled - apache2_module: - name: mpm_worker - state: present - register: enabledmpmworker - when: "ansible_os_family == 'Debian'" + - name: enable evasive module, test https://github.com/ansible/ansible/issues/22635 + apache2_module: + name: evasive + state: present -- name: ensure mpm_worker unchanged - assert: - that: - - 'not enabledmpmworker.changed' - when: "ansible_os_family == 'Debian'" + - name: disable evasive module + apache2_module: + name: evasive + state: absent -- name: try to disable all mpm modules with configcheck - apache2_module: - name: "{{item}}" - state: absent - with_items: - - mpm_worker - - mpm_event - - mpm_prefork - ignore_errors: yes - register: remove_with_configcheck - when: "ansible_os_family == 'Debian'" + - 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: 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: disable dump_io + apache2_module: + name: dump_io + identifier: dumpio_module + state: absent -- name: try to disable all mpm modules without configcheck - apache2_module: - name: "{{item}}" - state: absent - ignore_configcheck: True - with_items: - - mpm_worker - - mpm_event - - mpm_prefork - when: "ansible_os_family == 'Debian'" + - 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 + ignore_configcheck: True + with_items: + - mpm_worker + - mpm_event + - mpm_prefork + + - name: enabled mpm_event + apache2_module: + name: mpm_event + state: present + ignore_configcheck: True + register: enabledmpmevent + + - name: ensure changed mpm_event + assert: + that: + - 'enabledmpmevent.changed' + + - name: switch between mpm_event and mpm_worker + apache2_module: + name: "{{ item.name }}" + state: "{{ item.state }}" + ignore_configcheck: True + with_items: + - name: mpm_event + state: absent + - name: mpm_worker + state: present + + - name: ensure mpm_worker is already enabled + apache2_module: + name: mpm_worker + state: present + register: enabledmpmworker + + - name: ensure mpm_worker unchanged + assert: + that: + - 'not enabledmpmworker.changed' + + - name: try to disable all mpm modules with configcheck + apache2_module: + name: "{{item}}" + state: absent + with_items: + - mpm_worker + - mpm_event + - mpm_prefork + ignore_errors: yes + register: remove_with_configcheck + + - name: ensure configcheck fails task with when run without mpm modules + assert: + that: + - "{{ item.failed }}" + with_items: "{{ remove_with_configcheck.results }}" + + - name: try to disable all mpm modules without configcheck + apache2_module: + name: "{{item}}" + state: absent + ignore_configcheck: True + with_items: + - mpm_worker + - mpm_event + - mpm_prefork + + - name: enabled mpm_event to restore previous state + apache2_module: + name: mpm_event + state: present + ignore_configcheck: True + register: enabledmpmevent diff --git a/test/integration/targets/apache2_module/tasks/cleanup.yml b/test/integration/targets/apache2_module/tasks/cleanup.yml deleted file mode 100644 index 21fbb6002d..0000000000 --- a/test/integration/targets/apache2_module/tasks/cleanup.yml +++ /dev/null @@ -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 . - -- name: uninstall libapache2-mod-evasive via apt - apt: - name: libapache2-mod-evasive - state: absent - when: "ansible_os_family == 'Debian'" diff --git a/test/integration/targets/apache2_module/tasks/main.yml b/test/integration/targets/apache2_module/tasks/main.yml index 5e6512ad80..d95c21243e 100644 --- a/test/integration/targets/apache2_module/tasks/main.yml +++ b/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