diff --git a/docs/TRY_IT.md b/docs/TRY_IT.md index 7af75c5..151bc74 100644 --- a/docs/TRY_IT.md +++ b/docs/TRY_IT.md @@ -99,6 +99,20 @@ vpcopilot lab-create --domain vampi.example.com --origin :5000 This creates an origin pool + a clean-slate HTTP LB and prints the **DNS records to add**. Once DNS resolves and the cert issues, the app is reachable through XC at `https://vampi.example.com`. +The lab is built by **cloning** a known-good pool and LB — so every field XC requires is present — +and then stripping the copy back to a clean slate with every security control off. Which objects to +clone is configuration, because they have to exist in *your* namespace: + +```bash +vpcopilot lab-create --domain vampi.example.com --origin :5000 \ + --pool-template --lb-template +``` + +or set `VPCOPILOT_LAB_POOL_TEMPLATE` / `VPCOPILOT_LAB_LB_TEMPLATE` once in `.env`. The templates are +only ever **read** — the copy is what gets created — so it is safe to point them at a load balancer +you would never let the tool modify. If the named object is not there, `lab-create` says so and +names both ways to change it rather than surfacing a bare 404. + ### 2. Scan, then run the flow in the console ```bash diff --git a/src/vpcopilot/cli.py b/src/vpcopilot/cli.py index ac6fad0..4e0f8ad 100644 --- a/src/vpcopilot/cli.py +++ b/src/vpcopilot/cli.py @@ -431,11 +431,19 @@ def lab_create( origin: str = typer.Option(..., "--origin", help="app origin host:port, e.g. 16.59.6.127:5000"), name: str = typer.Option(None, "--name", help="base name (default: first label of the domain)"), origin_tls: bool = typer.Option(False, "--origin-tls", help="origin serves HTTPS (default: HTTP)"), + pool_template: str = typer.Option(None, "--pool-template", help="origin pool to clone for its required XC fields (default: $VPCOPILOT_LAB_POOL_TEMPLATE)"), + lb_template: str = typer.Option(None, "--lb-template", help="HTTP LB to clone, then strip every security control from (default: $VPCOPILOT_LAB_LB_TEMPLATE)"), ): - """Stand up a clean-slate XC test LB for an app origin (pool + LB), then print the DNS to add.""" + """Stand up a clean-slate XC test LB for an app origin (pool + LB), then print the DNS to add. + + The lab is built by CLONING a known-good pool and LB, so every required XC field is present, + and then stripping the copy back to a clean slate. Which objects to clone is configuration: + `--pool-template` / `--lb-template`, or `$VPCOPILOT_LAB_POOL_TEMPLATE` / + `$VPCOPILOT_LAB_LB_TEMPLATE`. The templates are only ever READ.""" from .lab import create_lab res = create_lab(domain, origin, name=name, origin_tls=origin_tls, + pool_template=pool_template, lb_template=lb_template, log=lambda m: rprint(f"[dim]{m}[/dim]")) rprint(Panel.fit( f"[bold]LB[/bold]: {res['lb']}\n[bold]pool[/bold]: {res['pool']} -> {res['origin']}\n" diff --git a/src/vpcopilot/lab.py b/src/vpcopilot/lab.py index d75674f..f0c6767 100644 --- a/src/vpcopilot/lab.py +++ b/src/vpcopilot/lab.py @@ -9,6 +9,7 @@ import copy import ipaddress +import os import time from typing import Callable @@ -18,6 +19,30 @@ _STATUS_FIELDS = ("host_name", "auto_cert_info", "cert_state", "dns_info", "internet_vip_info", "state", "downstream_tls_certificate_expiration_timestamps") +# The objects cloned to build a lab. CONFIG, not constants — they used to be hardcoded defaults +# naming this operator's own tenant (`nimbus-bigip-pool`, `nimbus-www`), which had three costs: +# the command only worked on a tenant that happened to contain objects by those names; the CLI +# exposed no way to override them; and `nimbus-www` was simultaneously the default source template +# here and the default PROTECTED object in `engine.protected_lbs()` — the one load balancer the +# tool refuses to mutate was the one it read to build every lab. +# +# Cloning itself stays: copying a known-good object guarantees every required XC field is present, +# where composing a spec from scratch discovers a missing one at PUT time. Only the names moved to +# config, so the same values keep the same behaviour on this tenant while the tool stops assuming +# them anywhere else. +POOL_TEMPLATE_ENV = "VPCOPILOT_LAB_POOL_TEMPLATE" +LB_TEMPLATE_ENV = "VPCOPILOT_LAB_LB_TEMPLATE" +DEFAULT_POOL_TEMPLATE = "nimbus-bigip-pool" +DEFAULT_LB_TEMPLATE = "nimbus-www" + + +def pool_template_default() -> str: + return os.environ.get(POOL_TEMPLATE_ENV, "").strip() or DEFAULT_POOL_TEMPLATE + + +def lb_template_default() -> str: + return os.environ.get(LB_TEMPLATE_ENV, "").strip() or DEFAULT_LB_TEMPLATE + def _origin_server(host: str) -> dict: try: @@ -44,9 +69,29 @@ def _clean_slate(spec: dict) -> None: spec.pop(k, None) +def _template(fetch: Callable, name: str, kind: str, env: str, flag: str) -> dict: + """Read a template object, and when it is not there say what to set rather than surfacing a 404. + + This is the failure every tenant but the one these defaults were written for will hit first, so + it names the remedy: which object was looked for, and the two ways to point it somewhere else.""" + try: + return fetch(name) + except Exception as e: # noqa: BLE001 — XCError, but the client raises its own type + raise RuntimeError( + f"no {kind} named {name!r} in this namespace to clone as the lab template ({e}). " + f"`lab-create` builds a lab by copying a known-good {kind} so every required XC field " + f"is present. Point it at one of yours with {flag}, or set ${env}." + ) from None + + def create_lab(domain: str, origin: str, *, name: str | None = None, origin_tls: bool = False, - pool_template: str = "nimbus-bigip-pool", lb_template: str = "nimbus-www", + pool_template: str | None = None, lb_template: str | None = None, poll: bool = True, log: Callable = print) -> dict: + """`pool_template`/`lb_template` default to `$VPCOPILOT_LAB_POOL_TEMPLATE` / + `$VPCOPILOT_LAB_LB_TEMPLATE`, falling back to the objects this operator's tenant happens to + carry. Passing them explicitly still wins, so every existing call is unchanged.""" + pool_template = pool_template or pool_template_default() + lb_template = lb_template or lb_template_default() xc = XC() host, _, port_s = origin.partition(":") port = int(port_s) if port_s else (443 if origin_tls else 80) @@ -57,7 +102,8 @@ def create_lab(domain: str, origin: str, *, name: str | None = None, origin_tls: if xc.origin_pool_exists(pool_name): log(f"origin pool {pool_name} already exists") else: - pspec = copy.deepcopy(xc.get_origin_pool(pool_template)["spec"]) + pspec = copy.deepcopy(_template(xc.get_origin_pool, pool_template, "origin pool", + POOL_TEMPLATE_ENV, "--pool-template")["spec"]) pspec["origin_servers"] = [_origin_server(host)] pspec["port"] = port pspec.pop("use_tls", None); pspec.pop("no_tls", None) @@ -73,7 +119,8 @@ def create_lab(domain: str, origin: str, *, name: str | None = None, origin_tls: if xc.lb_exists(lb_name): log(f"LB {lb_name} already exists") else: - spec = copy.deepcopy(xc.get_lb(lb_template)["spec"]) + spec = copy.deepcopy(_template(xc.get_lb, lb_template, "load balancer", + LB_TEMPLATE_ENV, "--lb-template")["spec"]) spec["domains"] = [domain] spec["default_route_pools"] = [{"pool": {"name": pool_name, "namespace": xc.ns}, "weight": 1, "priority": 1}] diff --git a/tests/test_lab.py b/tests/test_lab.py index dc9c16d..8b507b4 100644 --- a/tests/test_lab.py +++ b/tests/test_lab.py @@ -21,3 +21,128 @@ def test_clean_slate_disables_all_controls_and_strips_status(): assert spec["data_guard_rules"] == [] assert "host_name" not in spec and "cert_state" not in spec assert not spec["more_option"].get("request_headers_to_add") + + +# ================= the lab templates are configuration, not constants ================= +class _FakeXC: + """Records which objects were READ as templates, and what was created from them. + + `create_lab` re-reads the LB it just created to pull the DNS records out, so `read` holds the + two TEMPLATE reads first and the readback after — assertions look at `read[:2]`.""" + ns = "test-ns" + + def __init__(self, have=("nimbus-bigip-pool", "nimbus-www", "mine-pool", "mine-lb")): + self.have = set(have) + self.read: list[str] = [] + self.created: list[str] = [] + + def _get(self, name): + self.read.append(name) + if name not in self.have: + raise RuntimeError(f"GET {name} -> 404") + return {"spec": {"origin_servers": [], "port": 80, "domains": ["x"], "loadbalancer_type": {}}} + + get_origin_pool = _get + get_lb = _get + + def origin_pool_exists(self, n): + return False + + def lb_exists(self, n): + return False + + def create_origin_pool(self, obj): + self.created.append(obj["metadata"]["name"]) + + def create_lb(self, obj): + self.created.append(obj["metadata"]["name"]) + self.have.add(obj["metadata"]["name"]) # it exists now, and gets read back + return obj + + +def _run(monkeypatch, fake, **kw): + from vpcopilot import lab + monkeypatch.setattr(lab, "XC", lambda *a, **k: fake) + return lab.create_lab("app.example.com", "10.0.0.1:8080", poll=False, + log=lambda m: None, **kw) + + +def test_the_defaults_are_unchanged_so_this_tenant_behaves_identically(monkeypatch): + """No regression. The two names moved into config; they did not change value, so an existing + `lab-create` on this operator's tenant clones exactly what it always did.""" + from vpcopilot import lab + for var in (lab.POOL_TEMPLATE_ENV, lab.LB_TEMPLATE_ENV): + monkeypatch.delenv(var, raising=False) + assert lab.pool_template_default() == "nimbus-bigip-pool" + assert lab.lb_template_default() == "nimbus-www" + + fake = _FakeXC() + _run(monkeypatch, fake) + assert fake.read[:2] == ["nimbus-bigip-pool", "nimbus-www"] + + +def test_the_environment_can_point_the_lab_at_another_tenants_objects(monkeypatch): + """The actual coupling cost this fixes: `lab-create` only worked on a tenant that happened to + contain objects named `nimbus-bigip-pool` and `nimbus-www`.""" + from vpcopilot import lab + monkeypatch.setenv(lab.POOL_TEMPLATE_ENV, "mine-pool") + monkeypatch.setenv(lab.LB_TEMPLATE_ENV, "mine-lb") + fake = _FakeXC() + _run(monkeypatch, fake) + assert fake.read[:2] == ["mine-pool", "mine-lb"] + + +def test_an_explicit_argument_beats_the_environment(monkeypatch): + from vpcopilot import lab + monkeypatch.setenv(lab.POOL_TEMPLATE_ENV, "env-pool") + monkeypatch.setenv(lab.LB_TEMPLATE_ENV, "env-lb") + fake = _FakeXC(have=("mine-pool", "mine-lb")) + _run(monkeypatch, fake, pool_template="mine-pool", lb_template="mine-lb") + assert fake.read[:2] == ["mine-pool", "mine-lb"] + + +def test_an_empty_environment_value_falls_back_rather_than_asking_for_an_object_named_nothing(monkeypatch): + from vpcopilot import lab + monkeypatch.setenv(lab.POOL_TEMPLATE_ENV, " ") + monkeypatch.setenv(lab.LB_TEMPLATE_ENV, "") + assert lab.pool_template_default() == "nimbus-bigip-pool" + assert lab.lb_template_default() == "nimbus-www" + + +def test_a_missing_template_names_the_remedy_instead_of_surfacing_a_404(monkeypatch): + """This is the first failure any other tenant hits, so it has to say what to set.""" + import pytest + + from vpcopilot import lab + monkeypatch.setenv(lab.POOL_TEMPLATE_ENV, "not-here") + fake = _FakeXC() + with pytest.raises(RuntimeError) as e: + _run(monkeypatch, fake) + msg = str(e.value) + assert "not-here" in msg + assert "--pool-template" in msg and lab.POOL_TEMPLATE_ENV in msg + assert "clone" in msg # says WHY a template is needed at all + + +def test_the_templates_are_only_ever_read_never_written(monkeypatch): + """`nimbus-www` is simultaneously the default template here and the default PROTECTED object in + `engine.protected_lbs()`. That is safe only because the template is read, deep-copied, and the + copy is what gets created.""" + fake = _FakeXC() + _run(monkeypatch, fake) + assert fake.read[:2] == ["nimbus-bigip-pool", "nimbus-www"] + assert "nimbus-www" not in fake.created and "nimbus-bigip-pool" not in fake.created + assert fake.created == ["app-pool", "app-lab"] + + +def test_the_cli_exposes_both_templates(monkeypatch): + """They existed on the module function but no flag reached them, so from the command line they + were effectively hardcoded.""" + import inspect + + from vpcopilot import cli + params = inspect.signature(cli.lab_create).parameters + assert "pool_template" in params and "lb_template" in params + src = inspect.getsource(cli.lab_create) + assert "--pool-template" in src and "--lb-template" in src + assert "pool_template=pool_template" in src and "lb_template=lb_template" in src