[stable-9] Make apache2_mod_proxy work with Python 3, half-way modern Apache 2 versions, and add basic tests (#9772)

* Make apache2_mod_proxy work with Python 3, half-way modern Apache 2 versions, and add basic tests (#9762) * Move Apache 2 installation to setup role. * Make module work with Python 3. * Add basic tests. * Add changelog fragment. * Simplify change. * Pass referer. * apache2_mod_proxy: follow-up for #9762, forgot one place with find_all/findAll (#9775) Follow-up for #9762, forgot one place. (cherry picked from commit 8e36fd4847)
2025-02-18 21:53:18 +01:00 · 2025-02-18 21:53:18 +01:00 · 1a0a44d179
parent 18e4637042
commit 1a0a44d179
8 changed files with 341 additions and 30 deletions
--- a/changelogs/fragments/9762-apache2_mod_proxy.yml
+++ b/changelogs/fragments/9762-apache2_mod_proxy.yml
@ -0,0 +1,5 @@
+bugfixes:
+  - "apache2_mod_proxy - make compatible with Python 3 (https://github.com/ansible-collections/community.general/pull/9762)."
+  - "apache2_mod_proxy - passing the cluster's page as referer for the member's pages. This makes the module actually work again for halfway modern Apache versions.
+     According to some comments founds on the net the referer was required since at least 2019 for some versions of Apache 2
+     (https://github.com/ansible-collections/community.general/pull/9762)."
--- a/plugins/modules/apache2_mod_proxy.py
+++ b/plugins/modules/apache2_mod_proxy.py
@ -18,10 +18,11 @@ description:
  - Set and/or get members' attributes of an Apache httpd 2.4 mod_proxy balancer
    pool, using HTTP POST and GET requests. The httpd mod_proxy balancer-member
    status page has to be enabled and accessible, as this module relies on parsing
-    this page. This module supports ansible check_mode, and requires BeautifulSoup
-    python module.
+    this page.
 extends_documentation_fragment:
  - community.general.attributes
+requirements:
+  - Python package C(BeautifulSoup) on Python 2, C(beautifulsoup4) on Python 3.
 attributes:
  check_mode:
    support: full
@ -207,12 +208,16 @@ import re
 import traceback

 from ansible.module_utils.basic import AnsibleModule, missing_required_lib
+from ansible.module_utils.common.text.converters import to_text
 from ansible.module_utils.urls import fetch_url
-from ansible.module_utils.six import iteritems
+from ansible.module_utils.six import iteritems, PY2

 BEAUTIFUL_SOUP_IMP_ERR = None
 try:
-    from BeautifulSoup import BeautifulSoup
+    if PY2:
+        from BeautifulSoup import BeautifulSoup
+    else:
+        from bs4 import BeautifulSoup
 except ImportError:
    BEAUTIFUL_SOUP_IMP_ERR = traceback.format_exc()
    HAS_BEAUTIFULSOUP = False
@ -220,9 +225,15 @@ else:
    HAS_BEAUTIFULSOUP = True

 # balancer member attributes extraction regexp:
-EXPRESSION = r"(b=([\w\.\-]+)&w=(https?|ajp|wss?|ftp|[sf]cgi)://([\w\.\-]+):?(\d*)([/\w\.\-]*)&?[\w\-\=]*)"
+EXPRESSION = to_text(r"(b=([\w\.\-]+)&w=(https?|ajp|wss?|ftp|[sf]cgi)://([\w\.\-]+):?(\d*)([/\w\.\-]*)&?[\w\-\=]*)")
 # Apache2 server version extraction regexp:
-APACHE_VERSION_EXPRESSION = r"SERVER VERSION: APACHE/([\d.]+)"
+APACHE_VERSION_EXPRESSION = to_text(r"SERVER VERSION: APACHE/([\d.]+)")
+
+
+def find_all(where, what):
+    if PY2:
+        return where.findAll(what)
+    return where.find_all(what)


 def regexp_extraction(string, _regexp, groups=1):
@ -262,7 +273,7 @@ class BalancerMember(object):
    def get_member_attributes(self):
        """ Returns a dictionary of a balancer member's attributes."""

-        balancer_member_page = fetch_url(self.module, self.management_url)
+        balancer_member_page = fetch_url(self.module, self.management_url, headers={'Referer': self.management_url})

        if balancer_member_page[1]['status'] != 200:
            self.module.fail_json(msg="Could not get balancer_member_page, check for connectivity! " + balancer_member_page[1])
@ -272,11 +283,11 @@ class BalancerMember(object):
            except TypeError as exc:
                self.module.fail_json(msg="Cannot parse balancer_member_page HTML! " + str(exc))
            else:
-                subsoup = soup.findAll('table')[1].findAll('tr')
-                keys = subsoup[0].findAll('th')
+                subsoup = find_all(find_all(soup, 'table')[1], 'tr')
+                keys = find_all(subsoup[0], 'th')
                for valuesset in subsoup[1::1]:
                    if re.search(pattern=self.host, string=str(valuesset)):
-                        values = valuesset.findAll('td')
+                        values = find_all(valuesset, 'td')
                        return {keys[x].string: values[x].string for x in range(0, len(keys))}

    def get_member_status(self):
@ -300,9 +311,9 @@ class BalancerMember(object):
        values_url = "".join("{0}={1}".format(url_param, 1 if values[mode] else 0) for mode, url_param in iteritems(values_mapping))
        request_body = "{0}{1}".format(request_body, values_url)

-        response = fetch_url(self.module, self.management_url, data=request_body)
+        response = fetch_url(self.module, self.management_url, data=request_body, headers={'Referer': self.management_url})
        if response[1]['status'] != 200:
-            self.module.fail_json(msg="Could not set the member status! " + self.host + " " + response[1]['status'])
+            self.module.fail_json(msg="Could not set the member status! {host} {status}".format(host=self.host, status=response[1]['status']))

    attributes = property(get_member_attributes)
    status = property(get_member_status, set_member_status)
@ -329,11 +340,15 @@ class Balancer(object):
        if page[1]['status'] != 200:
            self.module.fail_json(msg="Could not get balancer page! HTTP status response: " + str(page[1]['status']))
        else:
-            content = page[0].read()
+            content = to_text(page[0].read())
            apache_version = regexp_extraction(content.upper(), APACHE_VERSION_EXPRESSION, 1)
            if apache_version:
                if not re.search(pattern=r"2\.4\.[\d]*", string=apache_version):
-                    self.module.fail_json(msg="This module only acts on an Apache2 2.4+ instance, current Apache2 version: " + str(apache_version))
+                    self.module.fail_json(
+                        msg="This module only acts on an Apache2 2.4+ instance, current Apache2 version: {version}".format(
+                            version=apache_version
+                        )
+                    )
                return content
            else:
                self.module.fail_json(msg="Could not get the Apache server version from the balancer-manager")
@ -345,7 +360,8 @@ class Balancer(object):
        except TypeError:
            self.module.fail_json(msg="Cannot parse balancer page HTML! " + str(self.page))
        else:
-            for element in soup.findAll('a')[1::1]:
+            elements = find_all(soup, 'a')
+            for element in elements[1::1]:
                balancer_member_suffix = str(element.get('href'))
                if not balancer_member_suffix:
                    self.module.fail_json(msg="Argument 'balancer_member_suffix' is empty!")
--- a/tests/integration/targets/apache2_mod_proxy/aliases
+++ b/tests/integration/targets/apache2_mod_proxy/aliases
@ -0,0 +1,7 @@
+# Copyright (c) Ansible Project
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+azp/posix/3
+destructive
+skip/aix
--- a/tests/integration/targets/apache2_mod_proxy/meta/main.yml
+++ b/tests/integration/targets/apache2_mod_proxy/meta/main.yml
@ -0,0 +1,8 @@
+---
+# Copyright (c) Ansible Project
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+dependencies:
+  - setup_remote_constraints
+  - setup_apache2
--- a/tests/integration/targets/apache2_mod_proxy/tasks/main.yml
+++ b/tests/integration/targets/apache2_mod_proxy/tasks/main.yml
@ -0,0 +1,253 @@
+---
+####################################################################
+# WARNING: These are designed specifically for Ansible tests       #
+# and should not be used as examples of how to write Ansible roles #
+####################################################################
+
+# Copyright (c) Ansible Project
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+- meta: end_play
+  when: ansible_os_family not in ['Debian', 'Suse']
+
+- name: Enable mod_proxy
+  community.general.apache2_module:
+    state: present
+    name: "{{ item }}"
+  loop:
+    - status
+    - proxy
+    - proxy_http
+    - proxy_balancer
+    - lbmethod_byrequests
+
+- name: Add port 81
+  lineinfile:
+    path: "/etc/apache2/{{ 'ports.conf' if ansible_os_family == 'Debian' else 'listen.conf' }}"
+    line: Listen 81
+
+- name: Set up virtual host
+  copy:
+    dest: "/etc/apache2/{{ 'sites-available' if ansible_os_family == 'Debian' else 'vhosts.d' }}/000-apache2_mod_proxy-test.conf"
+    content: |
+      <VirtualHost *:81>
+        <Proxy balancer://mycluster>
+          BalancerMember http://127.0.0.1:8080
+          BalancerMember http://127.0.0.1:8081
+        </Proxy>
+
+        <IfModule mod_evasive20.c>
+          DOSBlockingPeriod 0
+          DOSWhiteList 127.0.0.1
+          DOSWhiteList ::1
+        </IfModule>
+
+        <Location "/app/">
+          ProxyPreserveHost On
+          ProxyPass balancer://mycluster/
+          ProxyPassReverse balancer://mycluster/
+        </Location>
+
+        <Location "/balancer-manager">
+          SetHandler balancer-manager
+          Require all granted
+        </Location>
+      </VirtualHost>
+
+- name: Enable virtual host
+  file:
+    src: /etc/apache2/sites-available/000-apache2_mod_proxy-test.conf
+    dest: /etc/apache2/sites-enabled/000-apache2_mod_proxy-test.conf
+    owner: root
+    group: root
+    state: link
+  when: ansible_os_family not in ['Suse']
+
+- name: Restart Apache
+  service:
+    name: apache2
+    state: restarted
+
+- name: Install BeautifulSoup
+  pip:
+    name: "{{ 'BeautifulSoup' if ansible_python_version is version('3', '<') else 'BeautifulSoup4' }}"
+    extra_args: "-c {{ remote_constraints }}"
+
+- name: Get all current balancer pool members attributes
+  community.general.apache2_mod_proxy:
+    balancer_vhost: localhost:81
+  register: result
+
+- assert:
+    that:
+      - result is not changed
+      - result.members | length == 2
+      - result.members[0].port in ["8080", "8081"]
+      - result.members[0].balancer_url == "http://localhost:81/balancer-manager/"
+      - result.members[0].host == "127.0.0.1"
+      - result.members[0].path is none
+      - result.members[0].protocol == "http"
+      - result.members[1].port in ["8080", "8081"]
+      - result.members[1].balancer_url == "http://localhost:81/balancer-manager/"
+      - result.members[1].host == "127.0.0.1"
+      - result.members[1].path is none
+      - result.members[1].protocol == "http"
+
+- name: Enable member
+  community.general.apache2_mod_proxy:
+    balancer_vhost: localhost:81
+    member_host: 127.0.0.1
+    state: present
+  register: result
+
+- assert:
+    that:
+      - result is not changed
+
+- name: Get all current balancer pool members attributes
+  community.general.apache2_mod_proxy:
+    balancer_vhost: localhost:81
+  register: result
+
+- assert:
+    that:
+      - result is not changed
+      - result.members | length == 2
+      - result.members[0].port in ["8080", "8081"]
+      - result.members[0].balancer_url == "http://localhost:81/balancer-manager/"
+      - result.members[0].host == "127.0.0.1"
+      - result.members[0].path is none
+      - result.members[0].protocol == "http"
+      - result.members[0].status.disabled == false
+      - result.members[0].status.drained == false
+      - result.members[0].status.hot_standby == false
+      - result.members[0].status.ignore_errors == false
+      - result.members[1].port in ["8080", "8081"]
+      - result.members[1].balancer_url == "http://localhost:81/balancer-manager/"
+      - result.members[1].host == "127.0.0.1"
+      - result.members[1].path is none
+      - result.members[1].protocol == "http"
+      - result.members[1].status.disabled == false
+      - result.members[1].status.drained == false
+      - result.members[1].status.hot_standby == false
+      - result.members[1].status.ignore_errors == false
+
+- name: Drain member
+  community.general.apache2_mod_proxy:
+    balancer_vhost: localhost:81
+    member_host: 127.0.0.1
+    state: drained
+  register: result
+
+- assert:
+    that:
+      - result is changed
+
+# Note that since both members are on the same host, this always affects **both** members!
+
+- name: Get all current balancer pool members attributes
+  community.general.apache2_mod_proxy:
+    balancer_vhost: localhost:81
+  register: result
+
+- assert:
+    that:
+      - result is not changed
+      - result.members | length == 2
+      - result.members[0].port in ["8080", "8081"]
+      - result.members[0].balancer_url == "http://localhost:81/balancer-manager/"
+      - result.members[0].host == "127.0.0.1"
+      - result.members[0].path is none
+      - result.members[0].protocol == "http"
+      - result.members[0].status.disabled == false
+      - result.members[0].status.drained == true
+      - result.members[0].status.hot_standby == false
+      - result.members[0].status.ignore_errors == false
+      - result.members[1].port in ["8080", "8081"]
+      - result.members[1].balancer_url == "http://localhost:81/balancer-manager/"
+      - result.members[1].host == "127.0.0.1"
+      - result.members[1].path is none
+      - result.members[1].protocol == "http"
+      - result.members[1].status.disabled == false
+      - result.members[1].status.drained == true
+      - result.members[1].status.hot_standby == false
+      - result.members[1].status.ignore_errors == false
+
+- name: Disable member
+  community.general.apache2_mod_proxy:
+    balancer_vhost: localhost:81
+    member_host: 127.0.0.1
+    state: absent
+  register: result
+
+- assert:
+    that:
+      - result is changed
+
+- name: Get all current balancer pool members attributes
+  community.general.apache2_mod_proxy:
+    balancer_vhost: localhost:81
+  register: result
+
+- assert:
+    that:
+      - result is not changed
+      - result.members | length == 2
+      - result.members[0].port in ["8080", "8081"]
+      - result.members[0].balancer_url == "http://localhost:81/balancer-manager/"
+      - result.members[0].host == "127.0.0.1"
+      - result.members[0].path is none
+      - result.members[0].protocol == "http"
+      - result.members[0].status.disabled == true
+      - result.members[0].status.drained == false
+      - result.members[0].status.hot_standby == false
+      - result.members[0].status.ignore_errors == false
+      - result.members[1].port in ["8080", "8081"]
+      - result.members[1].balancer_url == "http://localhost:81/balancer-manager/"
+      - result.members[1].host == "127.0.0.1"
+      - result.members[1].path is none
+      - result.members[1].protocol == "http"
+      - result.members[1].status.disabled == true
+      - result.members[1].status.drained == false
+      - result.members[1].status.hot_standby == false
+      - result.members[1].status.ignore_errors == false
+
+- name: Enable member
+  community.general.apache2_mod_proxy:
+    balancer_vhost: localhost:81
+    member_host: 127.0.0.1
+    state: present
+  register: result
+
+- assert:
+    that:
+      - result is changed
+
+- name: Get all current balancer pool members attributes
+  community.general.apache2_mod_proxy:
+    balancer_vhost: localhost:81
+  register: result
+
+- assert:
+    that:
+      - result is not changed
+      - result.members | length == 2
+      - result.members[0].port in ["8080", "8081"]
+      - result.members[0].balancer_url == "http://localhost:81/balancer-manager/"
+      - result.members[0].host == "127.0.0.1"
+      - result.members[0].path is none
+      - result.members[0].protocol == "http"
+      - result.members[0].status.disabled == false
+      - result.members[0].status.drained == false
+      - result.members[0].status.hot_standby == false
+      - result.members[0].status.ignore_errors == false
+      - result.members[1].port in ["8080", "8081"]
+      - result.members[1].balancer_url == "http://localhost:81/balancer-manager/"
+      - result.members[1].host == "127.0.0.1"
+      - result.members[1].path is none
+      - result.members[1].protocol == "http"
+      - result.members[1].status.disabled == false
+      - result.members[1].status.drained == false
+      - result.members[1].status.hot_standby == false
+      - result.members[1].status.ignore_errors == false
--- a/tests/integration/targets/apache2_module/meta/main.yml
+++ b/tests/integration/targets/apache2_module/meta/main.yml
@ -0,0 +1,7 @@
+---
+# Copyright (c) Ansible Project
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+dependencies:
+  - setup_apache2
--- a/tests/integration/targets/apache2_module/tasks/main.yml
+++ b/tests/integration/targets/apache2_module/tasks/main.yml
@ -8,21 +8,6 @@
 # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
 # SPDX-License-Identifier: GPL-3.0-or-later

- name: install apache via apt
-  apt:
-    name: "{{item}}"
-    state: present
-  when: "ansible_os_family == 'Debian'"
-  with_items:
-    - apache2
-    - libapache2-mod-evasive
-
- name: install apache via zypper
-  community.general.zypper:
-    name: apache2
-    state: present
-  when: "ansible_os_family == 'Suse'"
-
 - name: test apache2_module
  block:
    - name: get list of enabled modules
--- a/tests/integration/targets/setup_apache2/tasks/main.yml
+++ b/tests/integration/targets/setup_apache2/tasks/main.yml
@ -0,0 +1,30 @@
+---
+####################################################################
+# WARNING: These are designed specifically for Ansible tests       #
+# and should not be used as examples of how to write Ansible roles #
+####################################################################
+
+# Copyright (c) Ansible Project
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+- name: Install apache via apt
+  apt:
+    name: "{{item}}"
+    state: present
+  when: "ansible_os_family == 'Debian'"
+  with_items:
+    - apache2
+    - libapache2-mod-evasive
+
+- name: Install apache via zypper
+  community.general.zypper:
+    name: apache2
+    state: present
+  when: "ansible_os_family == 'Suse'"
+
+- name: Enable mod_slotmem_shm on SuSE
+  apache2_module:
+    name: slotmem_shm
+    state: present
+  when: "ansible_os_family == 'Suse'"