Wait for stable private DNS before configuring certificate storage

.github/workflows/build_validation_develop.yml

@@ -93,6 +93,7 @@ jobs:
               - '.github/workflows/pr_comment_bot.yml'
               - 'devops/tests/**'
               - 'templates/shared_services/sonatype-nexus-vm/scripts/**'
+              - 'templates/shared_services/certs/**'
               - 'templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/**'
               - '.github/workflows/build_validation_develop.yml'
               - '.github/workflows/clean_validation_envs.yml'

CHANGELOG.md

@@ -31,6 +31,8 @@ See [Legacy Airlock & migration](docs/azure-tre-overview/airlock.md#legacy-airlo
 * Add weekly and manual builds for Dockerfiles and Porter bundles, with selected pull request builds. Retry a failed target once and report final results. ([#5088](https://github.com/microsoft/AzureTRE/issues/5088))
 
 BUG FIXES:
+
+* Wait for stable private DNS and verified TLS before configuring certificate storage ([#5240](https://github.com/microsoft/AzureTRE/issues/5240)).
 * Keep multi-step uninstall resources visible until the full pipeline finishes. Preserve final-step failures and skip completed callbacks during redelivery. (`API` 0.28.4) ([#5233](https://github.com/microsoft/AzureTRE/issues/5233))
 * Report Porter uninstall lookup errors as failures so a failed child deletion stops parent cleanup. Add regression coverage for missing error output and the API cascade guard. (`resource_processor` 0.13.13, `API` 0.28.3) ([#5228](https://github.com/microsoft/AzureTRE/issues/5228))
 * Run selected bundle tests only after successful smoke tests, including deployment reuse. (`build-scripts` 1.1.3) ([#5226](https://github.com/microsoft/AzureTRE/pull/5226))

devops/tests/test_certificate_storage_readiness.py

@@ -0,0 +1,201 @@
+"""Check private certificate storage readiness without contacting Azure."""
+
+import contextlib
+import importlib.util
+import io
+import json
+from pathlib import Path
+import socket
+import ssl
+import subprocess
+import sys
+import unittest
+from unittest.mock import MagicMock, patch
+
+
+HELPER = Path(__file__).resolve().parents[2] / "templates/shared_services/certs/scripts/wait_for_private_storage.py"
+SPEC = importlib.util.spec_from_file_location("certificate_storage_readiness", HELPER)
+READINESS = importlib.util.module_from_spec(SPEC)
+SPEC.loader.exec_module(READINESS)
+HOSTNAME = "certificate.blob.core.windows.net"
+PRIVATE_IP = "10.0.1.27"
+
+
+class CertificateStorageReadinessTests(unittest.TestCase):
+    def test_private_address_uses_verified_tls_and_storage_hostname(self):
+        connection = MagicMock()
+        context = MagicMock()
+        answers = [(socket.AF_INET, socket.SOCK_STREAM, 6, "", (PRIVATE_IP, 443))]
+        with (
+            patch.object(READINESS.socket, "getaddrinfo", return_value=answers),
+            patch.object(READINESS.socket, "create_connection", return_value=connection) as connect,
+            patch.object(READINESS.ssl, "create_default_context", return_value=context),
+        ):
+            self.assertTrue(READINESS.probe(HOSTNAME, PRIVATE_IP)[0])
+        connect.assert_called_once_with((PRIVATE_IP, 443), timeout=5)
+        context.wrap_socket.assert_called_once_with(connection.__enter__.return_value, server_hostname=HOSTNAME)
+
+    def test_probe_requires_tls12_and_preserves_certificate_and_hostname_checks(self):
+        context = ssl.create_default_context()
+        context.minimum_version = ssl.TLSVersion.MINIMUM_SUPPORTED
+        answers = [(socket.AF_INET, socket.SOCK_STREAM, 6, "", (PRIVATE_IP, 443))]
+        with (
+            patch.object(READINESS.socket, "getaddrinfo", return_value=answers),
+            patch.object(READINESS.socket, "create_connection", return_value=MagicMock()),
+            patch.object(READINESS.ssl, "create_default_context", return_value=context),
+            patch.object(context, "wrap_socket", return_value=MagicMock()),
+        ):
+            self.assertTrue(READINESS.probe(HOSTNAME, PRIVATE_IP)[0])
+        self.assertEqual(context.minimum_version, ssl.TLSVersion.TLSv1_2)
+        self.assertEqual(context.verify_mode, ssl.CERT_REQUIRED)
+        self.assertTrue(context.check_hostname)
+
+    def test_public_mixed_empty_and_wrong_private_answers_never_connect(self):
+        for addresses in ([], ["52.239.251.164"], [PRIVATE_IP, "52.239.251.164"], ["10.0.1.28"]):
+            with self.subTest(addresses=addresses):
+                answers = [(socket.AF_INET, socket.SOCK_STREAM, 6, "", (address, 443)) for address in addresses]
+                with (
+                    patch.object(READINESS.socket, "getaddrinfo", return_value=answers),
+                    patch.object(READINESS.socket, "create_connection") as connect,
+                ):
+                    self.assertFalse(READINESS.probe(HOSTNAME, PRIVATE_IP)[0])
+                    connect.assert_not_called()
+
+    def test_lookup_failure_does_not_connect(self):
+        with (
+            patch.object(READINESS.socket, "getaddrinfo", side_effect=socket.gaierror("diagnostic detail")),
+            patch.object(READINESS.socket, "create_connection") as connect,
+        ):
+            ready, reason = READINESS.probe(HOSTNAME, PRIVATE_IP)
+        self.assertFalse(ready)
+        self.assertIn("gaierror", reason)
+        self.assertNotIn("diagnostic detail", reason)
+        connect.assert_not_called()
+
+    def test_tls_verification_failure_is_not_readiness(self):
+        answers = [(socket.AF_INET, socket.SOCK_STREAM, 6, "", (PRIVATE_IP, 443))]
+        with (
+            patch.object(READINESS.socket, "getaddrinfo", return_value=answers),
+            patch.object(
+                READINESS.socket, "create_connection", side_effect=ssl.SSLCertVerificationError("private diagnostic")
+            ),
+        ):
+            ready, reason = READINESS.probe(HOSTNAME, PRIVATE_IP)
+        self.assertFalse(ready)
+        self.assertIn("SSLCertVerificationError", reason)
+        self.assertNotIn("private diagnostic", reason)
+
+    def wait_with_results(self, results, **kwargs):
+        elapsed = [0.0]
+
+        def sleep(seconds):
+            elapsed[0] += seconds
+
+        with (
+            patch.object(READINESS.time, "monotonic", side_effect=lambda: elapsed[0]),
+            patch.object(READINESS.time, "sleep", side_effect=sleep),
+            patch.object(READINESS, "run_probe", side_effect=results) as probe,
+            contextlib.redirect_stdout(io.StringIO()),
+        ):
+            READINESS.wait_for_private_storage(HOSTNAME, PRIVATE_IP, **kwargs)
+        return probe.call_count, elapsed[0]
+
+    def test_observed_dns_changes_restart_all_consecutive_checks(self):
+        # Private, lookup error, private, public, private, public, then stable.
+        results = [
+            (ready, "private" if ready else "lookup or public DNS")
+            for ready in [True, False, True, False, True, False] + [True] * 7
+        ]
+        count, elapsed = self.wait_with_results(results)
+        self.assertEqual(count, 13)
+        self.assertEqual(elapsed, 60)
+
+    def test_sustained_readiness_spans_at_least_thirty_seconds(self):
+        count, elapsed = self.wait_with_results([(True, "ready")] * 7)
+        self.assertEqual(count, 7)
+        self.assertEqual(elapsed, 30)
+
+    def test_tls_failure_also_restarts_stability(self):
+        results = [(True, "ready")] * 6 + [(False, "TLS check failed")] + [(True, "ready")] * 7
+        count, elapsed = self.wait_with_results(results)
+        self.assertEqual(count, 14)
+        self.assertEqual(elapsed, 65)
+
+    def test_timeout_prevents_unbounded_wait(self):
+        with self.assertRaisesRegex(TimeoutError, "within 12 seconds"):
+            self.wait_with_results([(False, "not ready")] * 3, timeout=12)
+
+    def test_probe_gets_only_remaining_deadline(self):
+        elapsed = [0.0]
+
+        def consume_budget(hostname, expected_ip, timeout):
+            self.assertEqual(timeout, 2)
+            elapsed[0] += timeout
+            return True, "too late"
+
+        with (
+            patch.object(READINESS.time, "monotonic", side_effect=lambda: elapsed[0]),
+            patch.object(READINESS, "run_probe", side_effect=consume_budget),
+            patch.object(READINESS.time, "sleep") as sleep,
+        ):
+            with self.assertRaises(TimeoutError):
+                READINESS.wait_for_private_storage(HOSTNAME, PRIVATE_IP, timeout=2, required_successes=1)
+            sleep.assert_not_called()
+
+    def test_blocked_resolver_child_has_finite_timeout(self):
+        with patch.object(READINESS.subprocess, "run", side_effect=subprocess.TimeoutExpired("probe", 2)) as run:
+            ready, reason = READINESS.run_probe(HOSTNAME, PRIVATE_IP, 2)
+        self.assertFalse(ready)
+        self.assertIn("timed out", reason)
+        self.assertEqual(run.call_args.kwargs["timeout"], 2)
+        self.assertFalse(run.call_args.kwargs.get("shell", False))
+
+    def test_invalid_child_results_cannot_report_readiness(self):
+        for value, code in [
+            ("not json", 0),
+            ("[]", 0),
+            ('{"ready":"yes","reason":"x"}', 0),
+            ('{"ready":true,"reason":"x"}', 1),
+            ("", 2),
+        ]:
+            with self.subTest(value=value, code=code):
+                result = subprocess.CompletedProcess([], code, value, "internal diagnostic")
+                with patch.object(READINESS.subprocess, "run", return_value=result):
+                    ready, reason = READINESS.run_probe(HOSTNAME, PRIVATE_IP, 2)
+                self.assertFalse(ready)
+                self.assertNotIn("internal diagnostic", reason)
+
+    def test_cli_rejects_public_and_special_expected_addresses(self):
+        for address in ("52.239.251.164", "127.0.0.1", "169.254.169.254", "0.0.0.0", "invalid"):
+            with self.subTest(address=address):
+                result = subprocess.run(
+                    [sys.executable, str(HELPER), "--hostname", HOSTNAME, "--expected-ip", address],
+                    capture_output=True,
+                    text=True,
+                    timeout=5,
+                    check=False,
+                )
+                self.assertEqual(result.returncode, 2)
+                self.assertIn("private endpoint IP address", result.stderr)
+
+    def test_cli_failure_returns_nonzero(self):
+        with (
+            patch.object(sys, "argv", [str(HELPER), "--hostname", HOSTNAME, "--expected-ip", PRIVATE_IP]),
+            patch.object(READINESS, "wait_for_private_storage", side_effect=TimeoutError("not ready")),
+            contextlib.redirect_stderr(io.StringIO()),
+        ):
+            self.assertEqual(READINESS.main(), 1)
+
+    def test_probe_mode_reports_json_for_parent(self):
+        output = io.StringIO()
+        with (
+            patch.object(sys, "argv", [str(HELPER), "--probe", "--hostname", HOSTNAME, "--expected-ip", PRIVATE_IP]),
+            patch.object(READINESS, "probe", return_value=(False, "not ready")),
+            contextlib.redirect_stdout(output),
+        ):
+            self.assertEqual(READINESS.main(), 1)
+        self.assertEqual(json.loads(output.getvalue()), {"ready": False, "reason": "not ready"})
+
+
+if __name__ == "__main__":
+    unittest.main()

templates/shared_services/certs/porter.yaml

@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-shared-service-certs
-version: 0.7.13
+version: 0.7.14
 description: "An Azure TRE shared service to generate certificates for a specified internal domain using Letsencrypt"
 registry: azuretre
 dockerfile: Dockerfile.tmpl

templates/shared_services/certs/scripts/wait_for_private_storage.py

@@ -0,0 +1,133 @@
+#!/usr/bin/env python3
+"""Wait for consecutive private DNS and verified TLS checks before storage writes."""
+
+import argparse
+import ipaddress
+import json
+import os
+from pathlib import Path
+import socket
+import ssl
+import subprocess
+import sys
+import time
+
+
+PROBE_TIMEOUT_SECONDS = 10
+TIMEOUT_SECONDS = 300
+REQUIRED_SUCCESSES = 7
+INTERVAL_SECONDS = 5
+
+
+def probe(hostname, expected_ip):
+    """Connect only when every DNS answer matches the private endpoint address."""
+    try:
+        answers = {
+            ipaddress.ip_address(item[4][0]) for item in socket.getaddrinfo(hostname, 443, type=socket.SOCK_STREAM)
+        }
+        if answers != {ipaddress.ip_address(expected_ip)}:
+            return False, "DNS does not exclusively resolve to the expected private endpoint"
+        context = ssl.create_default_context()
+        context.minimum_version = ssl.TLSVersion.TLSv1_2
+        with socket.create_connection((expected_ip, 443), timeout=5) as connection:
+            with context.wrap_socket(connection, server_hostname=hostname):
+                return True, "private DNS and TLS verification passed"
+    except (OSError, ValueError) as error:
+        return False, f"DNS or TLS check failed ({type(error).__name__})"
+
+
+def run_probe(hostname, expected_ip, timeout):
+    # A child process bounds getaddrinfo(), which has no per-call timeout.
+    try:
+        result = subprocess.run(
+            [
+                sys.executable,
+                str(Path(__file__).resolve()),
+                "--probe",
+                "--hostname",
+                hostname,
+                "--expected-ip",
+                expected_ip,
+            ],
+            capture_output=True,
+            text=True,
+            timeout=timeout,
+            check=False,
+        )
+    except subprocess.TimeoutExpired:
+        return False, "DNS or TLS check timed out"
+    if result.returncode not in (0, 1):
+        return False, "DNS or TLS check process failed"
+    try:
+        value = json.loads(result.stdout)
+        if type(value.get("ready")) is not bool or not isinstance(value.get("reason"), str):
+            raise ValueError("Invalid probe result")
+    except (AttributeError, ValueError):
+        return False, "DNS or TLS check returned an invalid result"
+    if value["ready"] != (result.returncode == 0):
+        return False, "DNS or TLS check returned an inconsistent result"
+    return value["ready"], value["reason"]
+
+
+def wait_for_private_storage(
+    hostname, expected_ip, timeout=TIMEOUT_SECONDS, required_successes=REQUIRED_SUCCESSES, interval=INTERVAL_SECONDS
+):
+    deadline = time.monotonic() + timeout
+    successes = 0
+    last_reason = "no completed check"
+    while time.monotonic() < deadline:
+        remaining = deadline - time.monotonic()
+        if remaining <= 0:
+            break
+        ready, last_reason = run_probe(hostname, expected_ip, min(PROBE_TIMEOUT_SECONDS, remaining))
+        if time.monotonic() >= deadline:
+            break
+        successes = successes + 1 if ready else 0
+        print(f"Storage readiness: {last_reason} ({successes}/{required_successes} consecutive checks)", flush=True)
+        if successes >= required_successes:
+            return
+        time.sleep(min(interval, max(0, deadline - time.monotonic())))
+    raise TimeoutError(
+        f"Private storage endpoint was not ready within {timeout} seconds: {last_reason}. Check private DNS and endpoint connectivity before retrying."
+    )
+
+
+def parse_args():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--hostname", default=os.environ.get("STORAGE_BLOB_HOST"))
+    parser.add_argument("--expected-ip", default=os.environ.get("STORAGE_PRIVATE_IP"))
+    parser.add_argument("--probe", action="store_true", help=argparse.SUPPRESS)
+    args = parser.parse_args()
+    if not args.hostname or any(c.isspace() for c in args.hostname) or any(c in args.hostname for c in "/:\\"):
+        parser.error("A storage DNS hostname is required")
+    try:
+        address = ipaddress.ip_address(args.expected_ip)
+    except ValueError:
+        parser.error("A private endpoint IP address is required")
+    if (
+        not address.is_private
+        or address.is_loopback
+        or address.is_link_local
+        or address.is_unspecified
+        or address.is_multicast
+    ):
+        parser.error("The expected address must be a private endpoint IP address")
+    return args
+
+
+def main():
+    args = parse_args()
+    if args.probe:
+        ready, reason = probe(args.hostname, args.expected_ip)
+        print(json.dumps({"ready": ready, "reason": reason}))
+        return 0 if ready else 1
+    try:
+        wait_for_private_storage(args.hostname, args.expected_ip)
+    except (OSError, TimeoutError) as error:
+        print(f"Error: {error}", file=sys.stderr)
+        return 1
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

templates/shared_services/certs/terraform/staticweb.tf

@@ -47,7 +47,30 @@ resource "azurerm_storage_account_static_website" "staticweb_site" {
   index_document     = "index.html"
   error_404_document = "404.html"
 
-  depends_on = [azurerm_private_endpoint.blobpe, azurerm_private_endpoint.webpe]
+  depends_on = [terraform_data.staticweb_private_endpoint_ready]
+}
+
+# Endpoint creation can finish before the DNS answers become stable.
+resource "terraform_data" "staticweb_private_endpoint_ready" {
+  triggers_replace = {
+    endpoint = azurerm_private_endpoint.blobpe.id
+    address  = azurerm_private_endpoint.blobpe.private_service_connection[0].private_ip_address
+    helper   = filesha256("${path.module}/../scripts/wait_for_private_storage.py")
+  }
+
+  lifecycle {
+    replace_triggered_by = [azurerm_private_endpoint.blobpe]
+  }
+
+  depends_on = [azurerm_private_endpoint.webpe]
+
+  provisioner "local-exec" {
+    command = "python3 \"${path.module}/../scripts/wait_for_private_storage.py\""
+    environment = {
+      STORAGE_BLOB_HOST  = azurerm_storage_account.staticweb.primary_blob_host
+      STORAGE_PRIVATE_IP = azurerm_private_endpoint.blobpe.private_service_connection[0].private_ip_address
+    }
+  }
 }
 
 resource "azurerm_role_assignment" "stgwriter" {