Refuse a Domain without upstream before domains.yml changes any server

changelog.d/90-domain-upstream-early.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- `domains.yml` now fails before it changes any server when a Domain it would render has no `proxy.upstream`, and names every such Domain; before, the site role's assert stopped the run only on that Domain's server, after `serial: 1` had already converged the servers before it. `upstream` stays optional in the schema, for a Domain the fleet routes with its own proxy (#90)

engine/README.md

@@ -211,6 +211,10 @@ task domains:deploy
 task domains:deploy ISSUE_CERTIFICATES=true
 ```
 
+`proxy.upstream` is optional in the schema, for a Domain the fleet routes with
+its own proxy, but the site role needs it. Before it changes any server, the
+playbook fails and names every Domain of the run that has no `upstream`.
+
 The site role installs Nginx and Certbot, serves the ACME challenge webroot,
 and converges in two phases: until a certificate exists the route serves
 plain HTTP so the site stays reachable during cutover; once TLS material is

engine/ansible/playbooks/domains.yml

@@ -1,4 +1,50 @@
 ---
+# `upstream` is optional in the schema, since a fleet may route a Domain with
+# its own proxy, but cloudfall_nginx_site cannot render one without it. The
+# check runs across every host of the run before the serial play below, so a
+# refused run changes no server.
+- name: Refuse Domains the site role cannot render
+  hosts: cloudfall_servers
+  gather_facts: false
+  any_errors_fatal: true
+  tasks:
+    - name: Require an upstream on every Domain this run renders
+      ansible.builtin.assert:
+        that:
+          - cloudfall_domains_without_upstream | length == 0
+        fail_msg: >-
+          {{ 'Domain' if cloudfall_domains_without_upstream | length == 1
+          else 'Domains' }} {{ cloudfall_domains_without_upstream | join(', ') }}
+          {{ 'has' if cloudfall_domains_without_upstream | length == 1
+          else 'have' }} no proxy.upstream, which cloudfall_nginx_site needs to
+          render {{ 'it' if cloudfall_domains_without_upstream | length == 1
+          else 'them' }}; no server was changed
+        quiet: true
+      vars:
+        cloudfall_domains_in_run: >-
+          {{
+            ansible_play_hosts_all
+            | map('extract', hostvars)
+            | map(attribute='cloudfall_domains', default=[])
+            | flatten(levels=1)
+          }}
+        # The same one-Domain filter as the render loop below.
+        cloudfall_domains_without_upstream: >-
+          {{
+            (
+              cloudfall_domains_in_run
+              | selectattr('id', 'equalto', cloudfall_domains_domain_id)
+              | list
+              if cloudfall_domains_domain_id is defined
+              else cloudfall_domains_in_run
+            )
+            | rejectattr('proxy.upstream', 'defined')
+            | map(attribute='id')
+            | unique
+            | sort
+          }}
+      run_once: true
+
 - name: Deploy declared Cloudfall public domain routes
   hosts: cloudfall_servers
   become: true

engine/tests/test_domains_one_domain.py

@@ -29,12 +29,14 @@
     "Write the deployment receipt on the controller",
 )
 RENDERED = "server { listen 80; }\n"
+RENDER_PLAY = "Deploy declared Cloudfall public domain routes"
 
 
 def _loop(variables: dict[str, object]) -> object:
-    play = DataLoader().load_from_file(str(ANSIBLE / "playbooks" / "domains.yml"))
-    assert isinstance(play, list)
-    (task,) = play[0]["tasks"]
+    plays = DataLoader().load_from_file(str(ANSIBLE / "playbooks" / "domains.yml"))
+    assert isinstance(plays, list)
+    (render,) = (play for play in plays if play["name"] == RENDER_PLAY)
+    (task,) = render["tasks"]
     templar = Templar(loader=DataLoader(), variables=variables)
     return templar.template(trust_as_template(task["loop"]))
 

engine/tests/test_domains_playbook.py

@@ -0,0 +1,161 @@
+"""``domains.yml`` refuses a Domain it cannot render before it touches a server.
+
+The hosts sit on TEST-NET-1 (192.0.2.0/24), which never answers, so a run
+that reached any of them would end UNREACHABLE. A refused run never gets that
+far: the check needs no connection.
+"""
+
+from __future__ import annotations
+
+import json
+import os
+import subprocess
+from pathlib import Path
+
+from cloudfall_engine.playbook import PlaybookRun, playbook_command, resolve_playbook
+
+ENGINE = Path(__file__).parents[2] / "engine"
+REFUSAL = "no proxy.upstream"
+
+
+def _domain(domain_id: str, server: str, *, upstream: bool) -> dict[str, object]:
+    proxy: dict[str, object] = {
+        "server": server,
+        "configurationPath": f"/etc/nginx/sites-available/{domain_id}.conf",
+        "service": "nginx.service",
+    }
+    if upstream:
+        proxy["upstream"] = {"address": "127.0.0.1", "port": 8080}
+    return {
+        "id": domain_id,
+        "primaryName": f"{domain_id}.example.test",
+        "aliases": [],
+        "proxy": proxy,
+        "tls": {"mode": "disabled"},
+    }
+
+
+def _host(server: str, address: str, domains: list[dict[str, object]]) -> object:
+    return {
+        "ansible_host": address,
+        "ansible_user": "root",
+        "ansible_timeout": 1,
+        "cloudfall_server_id": server,
+        "cloudfall_domains": domains,
+    }
+
+
+def _run(
+    tmp_path: Path, hosts: dict[str, object], *, domain_id: str | None = None
+) -> str:
+    inventory = tmp_path / "inventory.json"
+    inventory.write_text(
+        json.dumps({"all": {"children": {"cloudfall_servers": {"hosts": hosts}}}}),
+        encoding="utf-8",
+    )
+    one_domain = (
+        ()
+        if domain_id is None
+        else (json.dumps({"cloudfall_domains_domain_id": domain_id}),)
+    )
+    run = PlaybookRun(
+        playbook=resolve_playbook("domains", ENGINE),
+        inventory_file=inventory,
+        extra_vars=one_domain,
+    )
+    command = playbook_command(run, ENGINE)
+    completed = subprocess.run(  # noqa: S603 - argv built by the engine
+        command.argv,
+        env={
+            **os.environ,
+            **command.environment,
+            "ANSIBLE_HOST_KEY_CHECKING": "False",
+            "ANSIBLE_NOCOLOR": "1",
+        },
+        capture_output=True,
+        text=True,
+        check=False,
+        timeout=120,
+    )
+    assert completed.returncode != 0
+    return completed.stdout + completed.stderr
+
+
+def test_a_domain_without_upstream_stops_the_run_before_any_server(
+    tmp_path: Path,
+) -> None:
+    """The offending Domains sit on the second server; the first is not touched."""
+    output = _run(
+        tmp_path,
+        {
+            "edge-a": _host(
+                "edge-a", "192.0.2.10", [_domain("site-a", "edge-a", upstream=True)]
+            ),
+            "edge-b": _host(
+                "edge-b",
+                "192.0.2.11",
+                [
+                    _domain("site-b", "edge-b", upstream=True),
+                    _domain("bare-one", "edge-b", upstream=False),
+                    _domain("bare-two", "edge-b", upstream=False),
+                ],
+            ),
+        },
+    )
+
+    refusal = next(line for line in output.splitlines() if REFUSAL in line)
+    assert "Domains bare-one, bare-two have no proxy.upstream" in refusal
+    assert "render them;" in refusal
+    assert "site-" not in refusal
+    assert "UNREACHABLE" not in output
+    assert "Gathering Facts" not in output
+
+
+def test_domains_that_render_pass_the_check_and_reach_their_servers(
+    tmp_path: Path,
+) -> None:
+    output = _run(
+        tmp_path,
+        {
+            "edge-a": _host(
+                "edge-a", "192.0.2.10", [_domain("site-a", "edge-a", upstream=True)]
+            ),
+            "edge-b": _host("edge-b", "192.0.2.11", []),
+        },
+    )
+
+    assert REFUSAL not in output
+    assert "UNREACHABLE" in output
+
+
+def _shared_server() -> dict[str, object]:
+    """One server proxying a Domain that renders and one that cannot."""
+    return {
+        "edge-b": _host(
+            "edge-b",
+            "192.0.2.11",
+            [
+                _domain("site-b", "edge-b", upstream=True),
+                _domain("bare-one", "edge-b", upstream=False),
+            ],
+        ),
+    }
+
+
+def test_a_one_domain_run_checks_only_that_domain(tmp_path: Path) -> None:
+    """``bare-one`` shares the server but is not rendered by this run."""
+    output = _run(tmp_path, _shared_server(), domain_id="site-b")
+
+    assert REFUSAL not in output
+    assert "UNREACHABLE" in output
+
+
+def test_a_one_domain_run_for_a_domain_without_upstream_is_refused(
+    tmp_path: Path,
+) -> None:
+    output = _run(tmp_path, _shared_server(), domain_id="bare-one")
+
+    refusal = next(line for line in output.splitlines() if REFUSAL in line)
+    assert "Domain bare-one has no proxy.upstream" in refusal
+    assert "render it;" in refusal
+    assert "UNREACHABLE" not in output