Merge pull request #8 from zstyblik/add_tests

Add tests for and minor fixes in plugins

.github/workflows/ci.yml

@@ -38,3 +38,6 @@ jobs:
       - name: Lint with black
         run: |
           ./ci/run-black.sh check || ( ./ci/run-black.sh diff; exit 1 )
+      - name: Test with pytest
+        run: |
+          ./ci/run-pytest.sh

action_plugins/apache_ports_generator.py

@@ -77,30 +77,39 @@ def run(self, tmp=None, task_vars=None):
                     vhost.get("servername", "unknown")
                 )
                 raise AnsibleError(msg) from exception
-            except ValueError as exception:
+            except (TypeError, ValueError) as exception:
                 msg = "failed to convert port '{}' of vhost '{}' to int".format(
-                    vhost.get("servername", "unknown"),
                     vhost.get("port", None),
+                    vhost.get("servername", "unknown"),
                 )
                 raise AnsibleError(msg) from exception
 
+            if port < 0 or port > 65535:
+                raise AnsibleError(
+                    "port number '{}' of vhost '{}' is out of 0-65535 "
+                    "range".format(
+                        port,
+                        vhost.get("servername", "unknown"),
+                    )
+                )
+
             if port not in bindings:
                 bindings[port] = {}
 
             ssl = vhost.get("ssl", {})
             # According to documentation, https is default proto for port 443.
             # Therefore there is no need to specify it.
             if ssl and port != 443:
-                ssl = "https"
+                proto = "https"
             else:
-                ssl = ""
+                proto = ""
 
             listen_ip = vhost.get("listen_ip", "")
             if bindings[port]:
                 # We need to check for possible numerous and various conflicts.
                 if (
                     listen_ip in bindings[port]
-                    and bindings[port][listen_ip] != ssl
+                    and bindings[port][listen_ip] != proto
                 ):
                     # Reasoning: 'IP:Port' is the same and protocol is
                     # different -> error
@@ -129,7 +138,7 @@ def run(self, tmp=None, task_vars=None):
                     )
                     raise AnsibleError(msg)
 
-            bindings[port][listen_ip] = ssl
+            bindings[port][listen_ip] = proto
 
         result["data"] = {
             "{}:{}:{}".format(listen_ip, port, binding[listen_ip]): {

action_plugins/tests/test_apache_ports_generator.py

@@ -0,0 +1,308 @@
+#!/usr/bin/env python3
+"""Unit tests for apache_ports_generator."""
+from unittest.mock import Mock
+
+import pytest
+from ansible.errors import AnsibleError
+
+from action_plugins.apache_ports_generator import ActionModule  # noqa
+
+
+@pytest.mark.parametrize(
+    "listen_ip,port,proto,expected",
+    [
+        ("1.2.3.4", 80, None, "1.2.3.4:80"),
+        ("1.2.3.4", 80, "https", "1.2.3.4:80 https"),
+        (None, 80, None, "80"),
+        (None, 80, "https", "80 https"),
+    ],
+)
+def test_apg_format_binding(listen_ip, port, proto, expected):
+    """Check that ActionModule._format_binding() works as expected."""
+    action = ActionModule(
+        "task",
+        "connection",
+        "play_context",
+        "loader",
+        "templar",
+        "shared_loader_obj",
+    )
+    result = action._format_binding(listen_ip, port, proto)
+    assert result == expected
+
+
+@pytest.mark.parametrize(
+    "vhosts,expected",
+    [
+        # No vhosts defined.
+        (
+            [],
+            {
+                "data": {
+                    ":443:": {
+                        "formatted": "443",
+                        "listen_ip": "",
+                        "port": 443,
+                        "proto": "",
+                    },
+                    ":80:": {
+                        "formatted": "80",
+                        "listen_ip": "",
+                        "port": 80,
+                        "proto": "",
+                    },
+                }
+            },
+        ),
+        # Just HTTP/80
+        (
+            [
+                {
+                    "port": 80,
+                }
+            ],
+            {
+                "data": {
+                    ":80:": {
+                        "formatted": "80",
+                        "listen_ip": "",
+                        "port": 80,
+                        "proto": "",
+                    }
+                }
+            },
+        ),
+        # Just HTTPS/443
+        (
+            [
+                {
+                    "port": 443,
+                }
+            ],
+            {
+                "data": {
+                    ":443:": {
+                        "formatted": "443",
+                        "listen_ip": "",
+                        "port": 443,
+                        "proto": "",
+                    }
+                }
+            },
+        ),
+        # Mix
+        (
+            [
+                {
+                    "port": 80,
+                },
+                {
+                    "port": 80,
+                },
+                {
+                    "port": 443,
+                },
+                {
+                    "port": 443,
+                },
+                {
+                    "port": 8080,
+                },
+                {
+                    "port": 8081,
+                    "ssl": {"attr": "is_irrelevant"},
+                },
+                {
+                    "listen_ip": "1.2.3.4",
+                    "port": 8082,
+                },
+                {
+                    "listen_ip": "1.2.3.4",
+                    "port": 8083,
+                    "ssl": {"attr": "is_irrelevant"},
+                },
+            ],
+            {
+                "data": {
+                    ":80:": {
+                        "formatted": "80",
+                        "listen_ip": "",
+                        "port": 80,
+                        "proto": "",
+                    },
+                    ":443:": {
+                        "formatted": "443",
+                        "listen_ip": "",
+                        "port": 443,
+                        "proto": "",
+                    },
+                    ":8080:": {
+                        "formatted": "8080",
+                        "listen_ip": "",
+                        "port": 8080,
+                        "proto": "",
+                    },
+                    ":8081:https": {
+                        "formatted": "8081 https",
+                        "listen_ip": "",
+                        "port": 8081,
+                        "proto": "https",
+                    },
+                    "1.2.3.4:8082:": {
+                        "formatted": "1.2.3.4:8082",
+                        "listen_ip": "1.2.3.4",
+                        "port": 8082,
+                        "proto": "",
+                    },
+                    "1.2.3.4:8083:https": {
+                        "formatted": "1.2.3.4:8083 https",
+                        "listen_ip": "1.2.3.4",
+                        "port": 8083,
+                        "proto": "https",
+                    },
+                },
+            },
+        ),
+    ],
+)
+def test_apg_run_happy_path(vhosts, expected):
+    """Test happy path in ActionModule.run()."""
+    # NOTE(zstyblik): mocked just enough to make it work.
+    mock_task = Mock()
+    mock_task.async_val = False
+    mock_task.args = {"vhosts": vhosts}
+    mock_conn = Mock()
+    mock_conn._shell.tmpdir = "/path/does/not/exist"
+    action = ActionModule(
+        mock_task,
+        mock_conn,
+        "play_context",
+        "loader",
+        "templar",
+        "shared_loader_obj",
+    )
+    result = action.run(None, None)
+    assert result == expected
+
+
+@pytest.mark.parametrize(
+    "vhosts,expected_exc,expected_exc_msg",
+    [
+        # Port undefined
+        (
+            [
+                {
+                    "servername": "pytest",
+                },
+            ],
+            AnsibleError,
+            "vhost 'pytest' is missing port attribute",
+        ),
+        # Port out-of-range
+        (
+            [
+                {
+                    "port": -1,
+                },
+            ],
+            AnsibleError,
+            "port number '-1' of vhost 'unknown' is out of 0-65535 range",
+        ),
+        (
+            [
+                {
+                    "port": 72329,
+                },
+            ],
+            AnsibleError,
+            "port number '72329' of vhost 'unknown' is out of 0-65535 range",
+        ),
+        # Invalid port
+        (
+            [
+                {
+                    "port": "abcefg",
+                },
+            ],
+            AnsibleError,
+            "failed to convert port 'abcefg' of vhost 'unknown' to int",
+        ),
+        (
+            [
+                {
+                    "port": None,
+                },
+            ],
+            AnsibleError,
+            "failed to convert port 'None' of vhost 'unknown' to int",
+        ),
+        # IP/port/protocol collisions
+        (
+            [
+                {
+                    "port": 8080,
+                },
+                {
+                    "port": 8080,
+                    "ssl": {"attr": "is_irrelevant"},
+                },
+            ],
+            AnsibleError,
+            "HTTP/HTTPS collision for IP '' and port '8080' in vhost 'unknown'",
+        ),
+        (
+            [
+                {
+                    "listen_ip": "1.2.3.4",
+                    "port": 8080,
+                },
+                {
+                    "listen_ip": "1.2.3.4",
+                    "port": 8080,
+                    "ssl": {"attr": "is_irrelevant"},
+                },
+            ],
+            AnsibleError,
+            (
+                "HTTP/HTTPS collision for IP '1.2.3.4' and port '8080' "
+                "in vhost 'unknown'"
+            ),
+        ),
+        (
+            [
+                {
+                    "port": 8080,
+                },
+                {
+                    "listen_ip": "1.2.3.4",
+                    "port": 8080,
+                },
+            ],
+            AnsibleError,
+            (
+                "bind collision any Vs. IP for IP '1.2.3.4' and port '8080' "
+                "in vhost 'unknown'"
+            ),
+        ),
+    ],
+)
+def test_apg_run_unhappy_path(vhosts, expected_exc, expected_exc_msg):
+    """Test unhappy path resp. exceptions in ActionModule.run()."""
+    # NOTE(zstyblik): mocked just enough to make it work.
+    mock_task = Mock()
+    mock_task.async_val = False
+    mock_task.args = {"vhosts": vhosts}
+    mock_conn = Mock()
+    mock_conn._shell.tmpdir = "/path/does/not/exist"
+    action = ActionModule(
+        mock_task,
+        mock_conn,
+        "play_context",
+        "loader",
+        "templar",
+        "shared_loader_obj",
+    )
+    with pytest.raises(expected_exc) as exc:
+        _ = action.run(None, None)
+
+    assert str(exc.value) == expected_exc_msg

ci/run-ansible-lint.sh

@@ -2,4 +2,6 @@
 set -e
 set -u
 
+cd "$(dirname "${0}")/.."
+
 ansible-lint .

ci/run-black.sh

@@ -16,6 +16,8 @@ else
     exit 1
 fi
 
+cd "$(dirname "${0}")/.."
+
 # shellcheck disable=SC2086
 find . ! -path '*/\.*' -name '*.py' -print0 | \
     xargs -0 -- python3 -m black ${black_arg} -l 80

ci/run-flake8.sh

@@ -2,6 +2,8 @@
 set -e
 set -u
 
+cd "$(dirname "${0}")/.."
+
 python3 -m flake8 \
     . \
     --ignore=W503 \