Make apache2_mod_proxy work with Python 3, half-way modern Apache 2 v…

…ersions, 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.

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)."

plugins/modules/apache2_mod_proxy.py

@@ -19,7 +19,7 @@
 extends_documentation_fragment:
   - community.general.attributes
 requirements:
-  - Python package C(BeautifulSoup).
+  - Python package C(BeautifulSoup) on Python 2, C(beautifulsoup4) on Python 3.
 attributes:
   check_mode:
     support: full
@@ -207,16 +207,27 @@
 
 from ansible_collections.community.general.plugins.module_utils import deps
 from ansible.module_utils.basic import AnsibleModule
+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
 
-with deps.declare("BeautifulSoup"):
-    from BeautifulSoup import BeautifulSoup
+if PY2:
+    with deps.declare("BeautifulSoup"):
+        from BeautifulSoup import BeautifulSoup
+else:
+    with deps.declare("beautifulsoup4"):
+        from bs4 import BeautifulSoup
 
 # balancer member attributes extraction regexp:
-EXPRESSION = re.compile(r"(b=([\w\.\-]+)&w=(https?|ajp|wss?|ftp|[sf]cgi)://([\w\.\-]+):?(\d*)([/\w\.\-]*)&?[\w\-\=]*)")
+EXPRESSION = re.compile(to_text(r"(b=([\w\.\-]+)&w=(https?|ajp|wss?|ftp|[sf]cgi)://([\w\.\-]+):?(\d*)([/\w\.\-]*)&?[\w\-\=]*)"))
 # Apache2 server version extraction regexp:
-APACHE_VERSION_EXPRESSION = re.compile(r"SERVER VERSION: APACHE/([\d.]+)")
+APACHE_VERSION_EXPRESSION = re.compile(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):
@@ -256,7 +267,7 @@ def __init__(self, management_url, balancer_url, module):
     def get_member_attributes(self):
         """ Returns a dictionary of a balancer member's attributes."""
 
-        resp, info = fetch_url(self.module, self.management_url)
+        resp, info = fetch_url(self.module, self.management_url, headers={'Referer': self.management_url})
 
         if info['status'] != 200:
             self.module.fail_json(msg="Could not get balancer_member_page, check for connectivity! {0}".format(info))
@@ -266,11 +277,11 @@ def get_member_attributes(self):
             except TypeError as exc:
                 self.module.fail_json(msg="Cannot parse balancer_member_page HTML! {0}".format(exc))
             else:
-                subsoup = soup.findAll('table')[1].findAll('tr')
-                keys = subsoup[0].findAll('th')
+                subsoup = find_all(soup, 'table')[1].find_all('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):
@@ -294,9 +305,9 @@ def set_member_status(self, values):
         values_url = "".join("{0}={1}".format(url_param, 1 if values[mode] else 0) for mode, url_param in values_mapping.items())
         request_body = "{0}{1}".format(request_body, values_url)
 
-        response, info = fetch_url(self.module, self.management_url, data=request_body)
+        response, info = fetch_url(self.module, self.management_url, data=request_body, headers={'Referer': self.management_url})
         if info['status'] != 200:
-            self.module.fail_json(msg="Could not set the member status! " + self.host + " " + info['status'])
+            self.module.fail_json(msg="Could not set the member status! {host} {status}".format(host=self.host, status=info['status']))
 
     attributes = property(get_member_attributes)
     status = property(get_member_status, set_member_status)
@@ -333,11 +344,15 @@ def fetch_balancer_page(self):
         if info['status'] != 200:
             self.module.fail_json(msg="Could not get balancer page! HTTP status response: {0}".format(info['status']))
         else:
-            content = resp.read()
+            content = to_text(resp.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
 
             self.module.fail_json(msg="Could not get the Apache server version from the balancer-manager")
@@ -349,7 +364,8 @@ def get_balancer_members(self):
         except TypeError:
             self.module.fail_json(msg="Cannot parse balancer page HTML! {0}".format(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!")

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

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

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

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