Fix failing tests and workflow warnings

Author: mesudip

.github/workflows/docker-image.yml

@@ -15,16 +15,16 @@ jobs:
       contents: read
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v6
 
       - name: Set up QEMU
-        uses: docker/setup-qemu-action@v2
+        uses: docker/setup-qemu-action@v4
 
       - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@v2
+        uses: docker/setup-buildx-action@v4
 
       - name: Login to DockerHub
-        uses: docker/login-action@v2
+        uses: docker/login-action@v4
         with:
           username: mesudip
           password: ${{ secrets.DOCKERHUB_TOKEN }}
@@ -51,7 +51,7 @@ jobs:
 
       - name: Build and push for tags
         if: startsWith(github.ref, 'refs/tags/')
-        uses: docker/build-push-action@v5
+        uses: docker/build-push-action@v7
         with:
           file: Dockerfile
           context: .
@@ -63,7 +63,7 @@ jobs:
 
       - name: Build and push for main branch
         if: github.ref == 'refs/heads/master'
-        uses: docker/build-push-action@v5
+        uses: docker/build-push-action@v7
         with:
           file: Dockerfile
           context: .

.github/workflows/publish-python-package.yml

@@ -15,10 +15,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout code
-        uses: actions/checkout@v4
+        uses: actions/checkout@v6
 
       - name: Set up Python
-        uses: actions/setup-python@v5
+        uses: actions/setup-python@v6
         with:
           python-version: "3.12"
 

.github/workflows/run-tests.yml

@@ -22,18 +22,18 @@ jobs:
       contents: read
     steps:
       - name: Checkout code
-        uses: actions/checkout@v3
+        uses: actions/checkout@v6
 
       - name: Set up Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v6
         with:
           python-version: '3.12'
 
       - name: Set up QEMU
-        uses: docker/setup-qemu-action@v2
+        uses: docker/setup-qemu-action@v4
 
       - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@v2
+        uses: docker/setup-buildx-action@v4
 
       - name: Install dependencies
         run: |
@@ -44,16 +44,113 @@ jobs:
 
       - name: Run tests
         run: |
-          pytest --cov --cov-branch --junitxml=junit.xml -o junit_family=legacy
+          pytest --cov --cov-branch --cov-report=xml:coverage.xml --junitxml=junit.xml -o junit_family=legacy
+
+      - name: Write test summary
+        if: ${{ always() }}
+        run: |
+          python - <<'PY'
+          import os
+          import sys
+          import xml.etree.ElementTree as ET
+          from pathlib import Path
+
+          summary_path = os.environ.get("GITHUB_STEP_SUMMARY")
+          if not summary_path:
+              sys.exit(0)
+
+          junit_path = Path("junit.xml")
+
+          def md_escape(value):
+              return str(value).replace("|", "\\|").replace("\n", "<br>")
+
+          lines = ["## Test summary", ""]
+
+          if not junit_path.exists():
+              lines.extend([
+                  "No `junit.xml` test report was generated.",
+                  "",
+              ])
+          else:
+              root = ET.parse(junit_path).getroot()
+              suites = [root] if root.tag == "testsuite" else list(root.findall("testsuite"))
+              if not suites:
+                  suites = [root]
+
+              totals = {"tests": 0, "failures": 0, "errors": 0, "skipped": 0, "time": 0.0}
+              for suite in suites:
+                  totals["tests"] += int(suite.attrib.get("tests", 0))
+                  totals["failures"] += int(suite.attrib.get("failures", 0))
+                  totals["errors"] += int(suite.attrib.get("errors", 0))
+                  totals["skipped"] += int(suite.attrib.get("skipped", 0))
+                  totals["time"] += float(suite.attrib.get("time", 0.0))
+
+              passed = totals["tests"] - totals["failures"] - totals["errors"] - totals["skipped"]
+              lines.extend([
+                  "| Total | Passed | Failed | Errors | Skipped | Duration |",
+                  "| ---: | ---: | ---: | ---: | ---: | ---: |",
+                  f"| {totals['tests']} | {passed} | {totals['failures']} | {totals['errors']} | {totals['skipped']} | {totals['time']:.2f}s |",
+                  "",
+                  "### Metadata",
+                  "",
+                  "| Key | Value |",
+                  "| --- | --- |",
+                  f"| Python | {md_escape(sys.version.split()[0])} |",
+                  f"| Runner OS | {md_escape(os.environ.get('RUNNER_OS', 'unknown'))} |",
+                  f"| Event | {md_escape(os.environ.get('GITHUB_EVENT_NAME', 'unknown'))} |",
+                  f"| Ref | {md_escape(os.environ.get('GITHUB_REF_NAME', 'unknown'))} |",
+                  f"| SHA | `{md_escape(os.environ.get('GITHUB_SHA', 'unknown'))}` |",
+                  f"| Actor | {md_escape(os.environ.get('GITHUB_ACTOR', 'unknown'))} |",
+                  "",
+              ])
+
+              failed_cases = []
+              for case in root.iter("testcase"):
+                  issue = case.find("failure")
+                  if issue is None:
+                      issue = case.find("error")
+                  if issue is not None:
+                      failed_cases.append((case.attrib, issue))
+
+              if failed_cases:
+                  lines.extend([
+                      "### Failed tests",
+                      "",
+                      "| Test | Type | Message |",
+                      "| --- | --- | --- |",
+                  ])
+                  for attrs, issue in failed_cases[:25]:
+                      classname = attrs.get("classname", "")
+                      name = attrs.get("name", "unknown")
+                      test_name = f"{classname}.{name}" if classname else name
+                      message = issue.attrib.get("message", "").strip() or (issue.text or "").strip().splitlines()[0:1]
+                      if isinstance(message, list):
+                          message = message[0] if message else ""
+                      lines.append(f"| `{md_escape(test_name)}` | {md_escape(issue.tag)} | {md_escape(message[:300])} |")
+                  if len(failed_cases) > 25:
+                      lines.append(f"| ... | ... | {len(failed_cases) - 25} more failing tests omitted from summary |")
+                  lines.append("")
+
+          with open(summary_path, "a", encoding="utf-8") as summary:
+              summary.write("\n".join(lines))
+              summary.write("\n")
+          PY
 
       - name: Upload coverage reports to Codecov
+        if: ${{ !cancelled() }}
         uses: codecov/codecov-action@v5
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
+          files: coverage.xml
+          disable_search: true
           fail_ci_if_error: false
 
       - name: Upload test results to Codecov
         if: ${{ !cancelled() }}
-        uses: codecov/test-results-action@v1
+        uses: codecov/codecov-action@v5
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
+          files: junit.xml
+          disable_search: true
+          report_type: test_results
+          fail_ci_if_error: false

nginx_proxy/pre_processors/redirect_processor.py

@@ -12,6 +12,10 @@ def _is_certificate_redirect_target(target: Url):
     return "https" in target.scheme or "wss" in target.scheme or int(target.port or 80) == 443
 
 
+def _hostname_exceeds_certificate_limit(hostname: str) -> bool:
+    return bool(hostname) and len(hostname.rstrip(".")) > 64
+
+
 def process_redirection(backend: BackendTarget, environments: map, vhost_map: Dict[str, Dict[int, Host]]):
     redirect_env = [e[1] for e in environments.items() if e[0].startswith("PROXY_FULL_REDIRECT")]
     hosts = []
@@ -46,9 +50,7 @@ def process_redirection(backend: BackendTarget, environments: map, vhost_map: Di
                 if not Url.is_valid_hostname(target.hostname, allow_wildcard=True):
                     print("Invalid PROXY_FULL_REDIRECT target hostname: " + target.hostname)
                     continue
-                if _is_certificate_redirect_target(target) and not Url.is_valid_hostname(
-                    target.hostname, allow_wildcard=True, max_length=64
-                ):
+                if _is_certificate_redirect_target(target) and _hostname_exceeds_certificate_limit(target.hostname):
                     print("Invalid PROXY_FULL_REDIRECT target certificate hostname: " + target.hostname)
                     continue
                 for source in sources:

nginx_proxy/pre_processors/virtual_host_processor.py

@@ -25,10 +25,14 @@ def _requires_certificate(host: Host) -> bool:
     return "https" in host.scheme or "wss" in host.scheme or int(host.port or 80) == 443
 
 
+def _hostname_exceeds_certificate_limit(hostname: str) -> bool:
+    return bool(hostname) and len(hostname.rstrip(".")) > 64
+
+
 def _validate_external_host(host: Host):
     if not Url.is_valid_hostname(host.hostname, allow_wildcard=True):
         raise InvalidHostConfiguration(host.hostname, "invalid hostname")
-    if _requires_certificate(host) and not Url.is_valid_hostname(host.hostname, allow_wildcard=True, max_length=64):
+    if _requires_certificate(host) and _hostname_exceeds_certificate_limit(host.hostname):
         raise InvalidHostConfiguration(host.hostname, "certificate hostnames must be 64 characters or fewer")
 
 

tests/integration/test_prefer_local_swarm.py

@@ -8,9 +8,9 @@
 from tests.helpers.integration_helpers import get_nginx_config_from_container
 
 
-@pytest.fixture(scope="session")
-def swarm_mode():
-    return "prefer-local"
+@pytest.fixture(scope="session", params=["prefer-local"], ids=["swarm_prefer_local"])
+def swarm_mode(request):
+    return request.param
 
 
 def test_prefer_local_uses_local_swarm_task_primary_and_service_vip_backup(
@@ -25,7 +25,8 @@ def test_prefer_local_uses_local_swarm_task_primary_and_service_vip_backup(
     try:
         upstream = None
         config_str = ""
-        for _ in range(40):
+        deadline = time.monotonic() + 60
+        while time.monotonic() < deadline:
             config_str = get_nginx_config_from_container(nginx_proxy_container[0])
             config = HttpBlock.parse(config_str)
             upstream = next((u for u in config.upstreams if virtual_host in u.parameters), None)

tests/unit/test_backend_target.py

@@ -258,6 +258,18 @@ def test_http_virtual_host_allows_dns_valid_hostname_longer_than_64_chars(self):
         assert len(hosts) == 1
         assert hosts[0].hostname == long_hostname
 
+    def test_virtual_host_rejects_invalid_hostname(self):
+        bt = BackendTarget(
+            id="invalid-host-id",
+            name="invalid-host-test",
+            env={"VIRTUAL_HOST": "bad_host.example.com"},
+            network_settings={"my-net": {"NetworkID": "my-net-id", "IPAddress": "10.0.0.12"}},
+        )
+
+        config_data = process_virtual_hosts(bt, {"my-net-id"})
+
+        assert len(list(config_data.host_list())) == 0
+
     def test_https_virtual_host_allows_certificate_hostname_at_64_chars(self):
         hostname = f"{'a' * 52}.example.com"
         assert len(hostname) == 64
@@ -274,18 +286,6 @@ def test_https_virtual_host_allows_certificate_hostname_at_64_chars(self):
         assert len(hosts) == 1
         assert hosts[0].hostname == hostname
 
-    def test_virtual_host_rejects_invalid_hostname(self):
-        bt = BackendTarget(
-            id="invalid-host-id",
-            name="invalid-host-test",
-            env={"VIRTUAL_HOST": "bad_host.example.com"},
-            network_settings={"my-net": {"NetworkID": "my-net-id", "IPAddress": "10.0.0.12"}},
-        )
-
-        config_data = process_virtual_hosts(bt, {"my-net-id"})
-
-        assert len(list(config_data.host_list())) == 0
-
     def test_virtual_host_allows_wildcard_hostname(self):
         bt = BackendTarget(
             id="wildcard-host-id",

tests/unit/test_nginx_config.py

@@ -5,7 +5,7 @@
 
 @pytest.fixture
 def loaded_config():
-    CONFIG = """
+    CONFIG = r"""
 user  www www;
 
 worker_processes  2;
@@ -239,7 +239,7 @@ def test_location_blocks(loaded_config):
     loc3 = server.locations[3]
     assert loc3.path == "/download/"
     assert loc3.valid_referers == ["none", "blocked", "server_names", "*.example.com"]
-    assert loc3.rewrite == "^/(download/.*)/mp3/(.*)\..*$ /$1/mp3/$2.mp3 break"
+    assert loc3.rewrite == r"^/(download/.*)/mp3/(.*)\..*$ /$1/mp3/$2.mp3 break"
     assert loc3.root == "/spool/www"
     assert loc3.access_log == "/var/log/nginx-download.access_log download"
 
@@ -251,7 +251,7 @@ def test_location_blocks(loaded_config):
 
     # Location ~* \.(jpg|jpeg|gif)$
     loc4 = server.locations[4]
-    assert loc4.path == "~* \.(jpg|jpeg|gif)$"
+    assert loc4.path == r"~* \.(jpg|jpeg|gif)$"
     assert loc4.root == "/spool/www"
     assert loc4.access_log == "off"
     assert loc4.expires == "30d"

tests/unit/test_redirect_processor.py

@@ -0,0 +1,31 @@
+from nginx_proxy.BackendTarget import BackendTarget
+from nginx_proxy.Host import Host
+from nginx_proxy.pre_processors.redirect_processor import process_redirection
+
+
+def test_proxy_full_redirect_rejects_invalid_target_hostname():
+    vhost_map = {"target.example.com": {80: Host("target.example.com", 80)}}
+    backend = BackendTarget(id="redirect-id", name="redirect-test")
+
+    process_redirection(
+        backend,
+        {"PROXY_FULL_REDIRECT": "source.example.com -> bad_target.example.com"},
+        vhost_map,
+    )
+
+    assert "source.example.com" not in vhost_map
+    assert "bad_target.example.com" not in vhost_map
+
+
+def test_proxy_full_redirect_skips_invalid_source_hostname():
+    vhost_map = {"target.example.com": {80: Host("target.example.com", 80)}}
+    backend = BackendTarget(id="redirect-id", name="redirect-test")
+
+    process_redirection(
+        backend,
+        {"PROXY_FULL_REDIRECT": "bad_source.example.com,valid-source.example.com -> target.example.com"},
+        vhost_map,
+    )
+
+    assert "bad_source.example.com" not in vhost_map
+    assert "valid-source.example.com" in vhost_map