diff --git a/changelog.d/90-domain-upstream-early.md b/changelog.d/90-domain-upstream-early.md new file mode 100644 index 0000000..b0fa1b8 --- /dev/null +++ b/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) diff --git a/engine/README.md b/engine/README.md index c548657..9d8cb9f 100644 --- a/engine/README.md +++ b/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 diff --git a/engine/ansible/playbooks/domains.yml b/engine/ansible/playbooks/domains.yml index 1d72a2b..1bfa21e 100644 --- a/engine/ansible/playbooks/domains.yml +++ b/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 diff --git a/engine/tests/test_domains_one_domain.py b/engine/tests/test_domains_one_domain.py index 182dd93..5276960 100644 --- a/engine/tests/test_domains_one_domain.py +++ b/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"])) diff --git a/engine/tests/test_domains_playbook.py b/engine/tests/test_domains_playbook.py new file mode 100644 index 0000000..4cbe0a4 --- /dev/null +++ b/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