diff --git a/README.md b/README.md index a9c3a98..be48033 100644 --- a/README.md +++ b/README.md @@ -26,12 +26,12 @@ environments, not a public hosting platform — [Known limitations](#known-limit The supported source-checkout trial runs on macOS or Linux with hardware virtualization, outbound HTTPS, and enough capacity for three Lima VMs configured with **8 CPUs, 10 GiB RAM, and 70 GiB of sparse disk in total**. Install Git, Lima, Docker, `kubectl`, Helm, `curl`, `uv`, -`lsof`, and Python 3.12+. Docker must be running, not merely installed. +and Python 3.12+. Docker must be running, not merely installed. On macOS with Homebrew, the command-line dependencies are: ```bash -brew install git lima kubectl helm uv python lsof +brew install git lima kubectl helm uv python brew install --cask docker open -a Docker # wait until Docker reports that it is running ``` @@ -175,7 +175,7 @@ git clone https://github.com/hullwork/site.git cd site uv sync --locked --extra dev make test-db # starts a throwaway PostgreSQL on 127.0.0.1:55439 -make test # 1031 tests +make test # 1060 tests make test-db-down ``` @@ -208,6 +208,14 @@ SITES_LOCAL_PATH_PROVISIONER_ENABLED=false \ scripts/cluster.sh up ``` +A public deployment on such a cluster reports **no URL** until you say how the outside +reaches it. The `nodeport` backend puts each site on one node port from 30080–30088; only +the kubeadm trial above also forwards those to host ports 18090–18098, so nothing else can +be assumed. Add `SITES_HOST_PORT_BASE` (and `SITES_PUBLIC_URL_HOST` when the forwards do +not answer on `http://127.0.0.1`) once such a forward exists. Server-side verification is +independent of this: it probes the in-cluster address, so `status.verification` is evidence +the site serves traffic whether or not a public URL is declared. + The bootstrap helper generates credentials into a mode-0700 temporary directory and never writes them into the repository or a values file. **Do not use it for production.** For the lower-level Helm lifecycle and production Secret contract, see @@ -227,7 +235,16 @@ sites capabilities `bodySha256` from a request the control plane made itself. Redirects are not followed and only 2xx counts, because those are the two facts a tenant cannot forge. The evidence is bounded on purpose: it proves something at that address returned 2xx with that body - digest — not that the body is semantically correct, since the tenant produced it. + digest — not that the body is semantically correct, since the tenant produced it. It is + bounded in *place* too: the probe requests the deployment's `healthPath`, and + `verification.url` names it. With the default `/` that is the address a person opens; with + a health path such as `/healthz` it is not, and a site can be verified while its public URL + answers 403. It is + also bounded in time: the evidence names the `revision` it was collected for and is kept + when a later revision fails to roll out, so `ok: true` beside `phase: Failed` is last + time's answer, not this one's. Compare `verification.revision` with the deployment's + `revision` — [docs/AGENT_CONTRACT.md](docs/AGENT_CONTRACT.md#success-criteria) states the + full acceptance rule. - **The caller cannot name the tenancy it writes into.** Identity is a `(merchant, tenant)` pair decided entirely by the credential. `X-Merchant-ID` and `X-User-ID` are **refused with 403**, not ignored, so a misconfigured client fails loudly instead of quietly @@ -291,8 +308,8 @@ so the reference Deployments intentionally run **one replica each**. ### Module map -`api.py` is the composition root and combines eight endpoint mixins (auth, tenants, -merchants, admin, builds, bundles, deployments, sites). +`api.py` is the composition root and combines nine endpoint mixins (auth, tenants, +merchants, admin, builds, bundles, deployments, sites, mcp) over the shared HTTP kit.
Per-module responsibilities under src/sites/ @@ -301,7 +318,8 @@ merchants, admin, builds, bundles, deployments, sites). |---|---| | `api.py` | Composition root: combines mixins, assembles `serve()`, applies exception mappings, owns metric route templates | | `api_errors.py` | Ordered exception-to-HTTP mappings shared by mutation endpoints | -| `api_auth.py`, `api_tenants.py`, `api_merchants.py`, `api_admin.py`, `api_builds.py`, `api_bundles.py`, `api_deployments.py`, `api_sites.py` | The endpoint mixins | +| `api_auth.py`, `api_tenants.py`, `api_merchants.py`, `api_admin.py`, `api_builds.py`, `api_bundles.py`, `api_deployments.py`, `api_sites.py`, `api_mcp.py` | The endpoint mixins; `api_mcp.py` is the MCP tool surface as `POST /mcp` | +| `http_kit.py` | JSON and static-file helpers, bounded body reads, route matching, console SPA fallback; no control-plane logic | | `identity.py` | Pure authentication: `(headers, store, tokens) → Identity or Refusal` | | `admission.py` | Pure admission, quota, and port-allocation logic plus refusal exceptions | | `validation.py` | Input validation (`normalize_*`), identity and quota constants, `DEPLOY_FIELDS`, `STATIC_IMAGE` | @@ -401,6 +419,13 @@ production is worse off than one who reads them here. the reference Chart. Admission uses a process-local lock and reconciliation has no leader election, so scaling past one replica requires distributed admission and leader election first. +- **Two tenant limits disagree, and only one of them refuses at submission time.** Admission + counts deployments against `maxDeployments` (default 10) and answers `429 quota_exceeded`. + The tenant namespace also carries a `ResourceQuota` of `SITES_TENANT_CPU_LIMIT` (default 4) + against a fixed `limits.cpu: 1` per site, so a tenant can run **4** sites at once. Numbers + 5 through 10 are accepted with a 200 and then fail to roll out. The refusal now names the + exhausted quota rather than only reporting a readiness timeout, but the two numbers are + still set independently. - `sync_once()` holds the API's mutation lock across a Kubernetes `GET`. The Kubernetes client timeout (10s) and the mutation-lock acquire timeout (`SITES_MUTATION_LOCK_TIMEOUT`, 10s) are the same order of magnitude, so a slow apiserver can make write paths return @@ -428,14 +453,11 @@ production is worse off than one who reads them here. **Housekeeping** -- **Historical UIDs remain in the Git history.** The working tree is clean, but publishing - with history attached does not clear them; a `git filter-repo` pass is required before the - repository is made public. -- 45 `SITES_*` environment variables are read by `src/sites/` but appear in no document or - chart — mostly activator, gateway, NodePort-pool, KEDA, and operator tuning. They have - working defaults, and the most useful ones are now listed under - [Undocumented tuning variables](docs/CONFIGURATION.md#undocumented-tuning-variables), but - the set is not yet complete or schema-validated. +- `SITES_*` environment variables are still not schema-validated: an invalid value + generally raises at import time rather than falling back. Every one `src/sites/` reads is + named in a document or the chart — the leftovers are under + [Undocumented tuning variables](docs/CONFIGURATION.md#undocumented-tuning-variables) — and + a test keeps that true, but "named" is not "validated". - `api.py`, `operator.py`, and `storage.py` are large, and environment reads are spread across process-owned modules instead of validated once at startup. diff --git a/charts/site/templates/07-build-plane.yaml b/charts/site/templates/07-build-plane.yaml index 72de62f..7ef6bc2 100644 --- a/charts/site/templates/07-build-plane.yaml +++ b/charts/site/templates/07-build-plane.yaml @@ -401,7 +401,8 @@ spec: policyTypes: - Egress egress: - # The target pushed by buildctl is sites-registry.sites-local.svc:5000, and the service name must be resolved first. + # The target pushed by buildctl is the sites-registry Service in + # namespaces.control on port 5000, and the service name must be resolved first. - to: - namespaceSelector: matchLabels: diff --git a/charts/site/templates/08-gateway.yaml b/charts/site/templates/08-gateway.yaml index 4c7af35..2957594 100644 --- a/charts/site/templates/08-gateway.yaml +++ b/charts/site/templates/08-gateway.yaml @@ -24,11 +24,12 @@ # URL of the party. When the three places are allocated separately, the performance of any one place drifting is "the site cannot be accessed" - and three copies # The configuration alone is reasonable. When placed in ConfigMap, there is no such state as "only two changes" physically. # -# Change to your own wildcard domain name: change SITES_DOMAIN_SUFFIX below, **at the same time** change Gateway listener -# hostname (below this file, two places are nailed by GatewayManifestContractTest), and then give -# `*.` Add a DNS record pointing to the gateway. Domain name goes 80/443 when SITES_GATEWAY_SCHEME -# and SITES_GATEWAY_HOST_PORT must also be changed accordingly, otherwise the URL returned to the caller will contain local -# 18090 port. +# Change to your own wildcard domain name with `gateway.domainSuffix`: it renders both +# SITES_DOMAIN_SUFFIX below and the Gateway listener's wildcard, so the two cannot be +# changed apart (they used to be two literals kept together by +# GatewayManifestContractTest). Point `*.` at the gateway in DNS, and set +# `gateway.scheme`/`gateway.hostPort` to how the host really answers -- they default to +# the reference Lima forward, and left alone they hand the caller a URL naming port 18090. apiVersion: v1 kind: ConfigMap metadata: @@ -41,10 +42,22 @@ data: # Only when this value is gateway, the site will use the L7 entrance and scaleToZero will be allowed. SITES_EXPOSURE_BACKEND: "gateway" # Site hostname = -.. - SITES_DOMAIN_SUFFIX: "127.0.0.1.sslip.io" + SITES_DOMAIN_SUFFIX: {{ .Values.gateway.domainSuffix | quote }} # Host-facing URL scheme and port for the local Lima port forwarding. - SITES_GATEWAY_SCHEME: "http" - SITES_GATEWAY_HOST_PORT: "18090" + # Which Gateway the control plane points at. The object below is named after + # namespaces.gateway, while the code defaulted both of these to the literal + # "sites-gateway", so they agreed only when the release happened to be + # installed into a namespace of that name -- and both install scripts set + # namespaces.gateway to the release namespace, which by default is + # sites-local. Everywhere else the operator wrote HTTPRoutes whose parentRef + # named a Gateway that does not exist and a tenant NetworkPolicy that admitted + # a data-plane Pod label nothing carries. Neither shows up as an error: the + # route is created, the deployment is Running, verification passes on the + # in-cluster address, and the public URL answers 404. + SITES_GATEWAY_NAME: {{ .Values.namespaces.gateway | quote }} + SITES_GATEWAY_NAMESPACE: {{ .Values.namespaces.gateway | quote }} + SITES_GATEWAY_SCHEME: {{ .Values.gateway.scheme | quote }} + SITES_GATEWAY_HOST_PORT: {{ .Values.gateway.hostPort | quote }} # The tenant image is in the workload registry, not the CP Registry that hosts the control plane image. # The node containerd receives this stable name from the deployment portal to the Registry pull Service. SITES_REGISTRY_PULL_HOST: {{ .Values.registry.pullHost | quote }} @@ -134,9 +147,10 @@ spec: # This is especially true for real domain names - `*.*.apps.example.com` is a two-level wildcard that most DNS service providers # Not supported. The host_for of sites/exposure.py and this together form the same contract. # - # When changing to self-built wildcard DNS, change SITES_DOMAIN_SUFFIX and the hostname here. The two must be synchronized. - # (GatewayManifestContractTest pinned). - hostname: "*.127.0.0.1.sslip.io" + # Rendered from gateway.domainSuffix, the same value SITES_DOMAIN_SUFFIX above + # takes, so self-built wildcard DNS is one value rather than two edits that + # have to be kept in step (GatewayManifestContractTest still checks the pair). + hostname: {{ printf "*.%s" .Values.gateway.domainSuffix | quote }} allowedRoutes: # 🔴 Must be Selector instead of Same: HTTPRoute is built in the **tenant** Namespace, # And Gateway is in sites-gateway. Leaving it as the default Same will cause every tenant route to be diff --git a/charts/site/templates/10-control-plane.yaml b/charts/site/templates/10-control-plane.yaml index 10b739d..608ebc5 100644 --- a/charts/site/templates/10-control-plane.yaml +++ b/charts/site/templates/10-control-plane.yaml @@ -336,6 +336,15 @@ spec: # It will also be printed out by any env in the same Pod. - name: SITES_REGISTRY_USERNAME value: sites + # The registry Service is created in namespaces.control, so its + # address is not a constant. Leaving these to the code defaults + # pinned the namespace at sites-local: any other install pushed + # builds at, and resolved digests against, a Service that is not + # there, and nothing said so. + - name: SITES_REGISTRY_PUSH_HOST + value: {{ printf "sites-registry.%s.svc:5000" .Values.namespaces.control | quote }} + - name: SITES_REGISTRY_API + value: {{ printf "http://sites-registry.%s.svc:5000" .Values.namespaces.control | quote }} - name: SITES_CONTROL_NAMESPACE value: {{ .Values.namespaces.control | quote }} - name: SITES_HTTP_PORT @@ -592,10 +601,27 @@ spec: # It will also be printed out by any env in the same Pod. - name: SITES_REGISTRY_USERNAME value: sites + # The registry Service is created in namespaces.control, so its + # address is not a constant. Leaving these to the code defaults + # pinned the namespace at sites-local: any other install pushed + # builds at, and resolved digests against, a Service that is not + # there, and nothing said so. + - name: SITES_REGISTRY_PUSH_HOST + value: {{ printf "sites-registry.%s.svc:5000" .Values.namespaces.control | quote }} + - name: SITES_REGISTRY_API + value: {{ printf "http://sites-registry.%s.svc:5000" .Values.namespaces.control | quote }} - name: SITES_CONTROL_NAMESPACE value: {{ .Values.namespaces.control | quote }} - name: SITES_PUBLIC_URL_HOST - value: http://127.0.0.1 + value: {{ .Values.nodePort.publicUrlHost | quote }} + # Declared only where this environment really does forward a host + # port to the NodePort pool. Left out, exposure.host_port_base() + # returns None and a public deployment reports no URL rather than one + # built from a mapping this host may not have. See values.yaml. + {{- with .Values.nodePort.hostPortBase }} + - name: SITES_HOST_PORT_BASE + value: {{ . | quote }} + {{- end }} # The Pod network this release's NetworkPolicies exclude, and this # Pod's own address to check it against. The operator refuses to # start if the address is outside the declaration, or if the diff --git a/charts/site/values.schema.json b/charts/site/values.schema.json index b6a06f8..a14123c 100644 --- a/charts/site/values.schema.json +++ b/charts/site/values.schema.json @@ -13,6 +13,7 @@ "mcpEndpoint", "monitoring", "namespaces", + "nodePort", "postgresql", "service", "storage", @@ -271,7 +272,10 @@ "type": "object", "additionalProperties": false, "required": [ + "domainSuffix", "enabled", + "hostPort", + "scheme", "serviceType" ], "properties": { @@ -300,6 +304,21 @@ ], "minimum": 30000, "maximum": 32767 + }, + "domainSuffix": { + "type": "string", + "minLength": 1 + }, + "scheme": { + "enum": [ + "http", + "https" + ] + }, + "hostPort": { + "type": "integer", + "minimum": 1, + "maximum": 65535 } } }, @@ -362,6 +381,33 @@ "pattern": "^$|^([0-9]{1,3}\\.){3}[0-9]{1,3}/[0-9]{1,2}$" } } + }, + "nodePort": { + "type": "object", + "additionalProperties": false, + "required": [ + "publicUrlHost", + "hostPortBase" + ], + "properties": { + "publicUrlHost": { + "type": "string", + "minLength": 1 + }, + "hostPortBase": { + "oneOf": [ + { + "type": "string", + "pattern": "^$|^[0-9]{1,5}$" + }, + { + "type": "integer", + "minimum": 1, + "maximum": 65535 + } + ] + } + } } }, "$defs": { diff --git a/charts/site/values.yaml b/charts/site/values.yaml index 8ecd435..602cb01 100644 --- a/charts/site/values.yaml +++ b/charts/site/values.yaml @@ -108,6 +108,31 @@ gateway: serviceType: LoadBalancer httpNodePort: null metricsNodePort: null + # The host-facing half of a gateway URL: what sits between a site's own name + # and its path. scheme and hostPort are only what the caller is handed -- + # routing does not read them -- and default to the reference trial's Lima + # forward, so on any other host they are the first two values to set. + # domainSuffix is load-bearing: it is both the listener's wildcard and the + # hostname the operator writes into every HTTPRoute. One value feeds both, so + # changing it cannot leave the two disagreeing; point `*.` at the + # gateway in DNS before using anything but the sslip.io default. + domainSuffix: 127.0.0.1.sslip.io + scheme: http + hostPort: 18090 + +# How a browser outside the cluster reaches a site published by the `nodeport` +# exposure backend. Both keys describe the machine in front of the cluster, not +# the cluster, so hostPortBase has no default for the same reason +# clusterNetwork.podCIDR has none: a wrong one is not visibly wrong. The +# reference kubeadm trial owns a relay that forwards guest 30080..30088 to host +# 18090..18098 and therefore declares hostPortBase: 18090. Where no such mapping +# exists, leaving it empty makes a public deployment report no URL, instead of +# one assembled from a host port that something unrelated may be serving. +# Server-side verification is unaffected either way: it uses the in-cluster +# address and remains the evidence that the site serves traffic. +nodePort: + publicUrlHost: http://127.0.0.1 + hostPortBase: "" monitoring: enabled: false diff --git a/console/src/components/TenantsView.tsx b/console/src/components/TenantsView.tsx index f205952..a827494 100644 --- a/console/src/components/TenantsView.tsx +++ b/console/src/components/TenantsView.tsx @@ -246,7 +246,7 @@ export default function TenantsView({ {t("Tenant ID")} setDraftUser(event.target.value)} /> diff --git a/console/src/i18n.tsx b/console/src/i18n.tsx index 2ef8207..361c1e2 100644 --- a/console/src/i18n.tsx +++ b/console/src/i18n.tsx @@ -128,7 +128,7 @@ const en = { "New tenant": "New tenant", "Search tenants": "Search tenants", "Search for merchant or tenant ID": "Search for merchant or tenant ID", - "The only one in the merchant": "The only one in the merchant", + "Unique within the merchant": "Unique within the merchant", "Create and issue token": "Create and issue token", "No matching tenant": "No matching tenant", "There are no tenants on the platform yet": "There are no tenants on the platform yet", @@ -554,7 +554,7 @@ const zhCN: Record = { "New tenant": "新建租户", "Search tenants": "搜索租户", "Search for merchant or tenant ID": "搜索商户或租户 ID", - "The only one in the merchant": "商户内唯一", + "Unique within the merchant": "商户内唯一", "Create and issue token": "创建并签发 token", "No matching tenant": "没有匹配的租户", "There are no tenants on the platform yet": "平台上还没有租户", diff --git a/deploy-specs/example-single.json b/deploy-specs/example-single.json index 0483318..f0b0405 100644 --- a/deploy-specs/example-single.json +++ b/deploy-specs/example-single.json @@ -1,7 +1,12 @@ { - "name": "example-web", - "image": "nginx:1.29-alpine", - "port": 80, - "healthPath": "/", - "exposure": "public" + "name": "example-single", + "components": [ + { + "name": "single-web", + "image": "nginxinc/nginx-unprivileged:1.29-alpine", + "port": 8080, + "healthPath": "/", + "exposure": "public" + } + ] } diff --git a/deploy-specs/example-stack.json b/deploy-specs/example-stack.json index 861d527..3e8d3a6 100644 --- a/deploy-specs/example-stack.json +++ b/deploy-specs/example-stack.json @@ -2,19 +2,18 @@ "name": "example-stack", "components": [ { - "name": "web", - "image": "nginx:1.29-alpine", - "port": 80, + "name": "stack-web", + "image": "nginxinc/nginx-unprivileged:1.29-alpine", + "port": 8080, "healthPath": "/", "exposure": "public" }, { - "name": "worker", - "image": "busybox:1.37", + "name": "stack-worker", + "image": "nginxinc/nginx-unprivileged:1.29-alpine", "port": 8080, - "healthPath": "/healthz", - "exposure": "internal", - "command": ["sh", "-c", "httpd -f -p 8080"] + "healthPath": "/", + "exposure": "internal" } ] } diff --git a/docs/AGENT_CONTRACT.md b/docs/AGENT_CONTRACT.md index 4ee4379..c18da63 100644 --- a/docs/AGENT_CONTRACT.md +++ b/docs/AGENT_CONTRACT.md @@ -50,6 +50,38 @@ completed real HTTP evidence collection. The result also records `httpStatus` an `bodySha256`. The public or host entry point is a separate network path: request the returned URL from the user side and compare its response digest with `bodySha256`. +**Evidence covers `healthPath`, not the whole site.** The probe requests +`http://..svc:` and writes that exact address into +`verification.url`. So the digest comparison above is only meaningful when `healthPath` is +`/`: at any other value the two are different resources and comparing them must fail. +`sites deploy` and `sites deploy-static` default to `/`, **`sites build submit` defaults to +`/healthz`**, and a site deployed that way can be `Running`, `ready`, `verification.ok`, +and hold a public URL that answers `403` — measured, not hypothetical. Before treating the +evidence as being about the address a person will open, check that the two agree: + +``` +verification.url.endswith(healthPath) and healthPath == "/" +``` + +Otherwise the evidence says only that the health endpoint answered 2xx with that digest, +which is exactly what it was collected from. + +**Evidence belongs to the revision it names.** `verification.revision` is the revision the +probe ran against, and it is deliberately kept when a later revision fails to roll out — +it remains true that *that* revision served traffic. So `ok=true` next to +`phase: Failed` is not a contradiction and is not success: it is last time's answer. A +caller that reads `ok` alone will report a rollout that never came up as a live site. +Accept a deployment only when **all** of these hold: + +``` +phase == "Running" and ready == true + and verification.ok == true + and verification.revision == revision # the top-level revision of the same response +``` + +Both fields are in every `sites status` and `GET /v1/deployments/{name}` response, so the +comparison needs no extra call. + ## Identity **[AUTH.md](AUTH.md) is the contract.** It is written for any client, @@ -165,6 +197,50 @@ X-Acting-Subject: ": current value is "" +``` + +`--namespace` selects where the control plane runs, not how many control planes a cluster +can hold. Uninstall the existing release before installing another, or use another cluster. + The lower-level path installs Site into the Kubernetes context selected by `kubectl`. Review the current context before continuing: diff --git a/scripts/bootstrap-standalone-secrets.sh b/scripts/bootstrap-standalone-secrets.sh index a3ab603..cec63d8 100755 --- a/scripts/bootstrap-standalone-secrets.sh +++ b/scripts/bootstrap-standalone-secrets.sh @@ -5,6 +5,11 @@ namespace=sites-local api_secret=sites-api-token database_secret=sites-postgres-auth registry_secret=sites-registry-auth +# In-cluster address the BuildKit Job pushes to, and therefore the key the +# generated docker config authenticates. Must equal SITES_REGISTRY_PUSH_HOST as +# the Chart renders it, or the push is a 401 nothing else explains. Resolved +# after argument parsing so --namespace decides it. +registry_host=${SITES_REGISTRY_PUSH_HOST:-} context=${SITES_KUBE_CONTEXT:-} kube() { @@ -27,6 +32,9 @@ Options: --api-secret NAME API Secret name (default: sites-api-token) --database-secret NAME PostgreSQL Secret name (default: sites-postgres-auth) --registry-secret NAME Registry Secret name (default: sites-registry-auth) + --registry-host HOST Registry address the build plane pushes to + (default: sites-registry..svc:5000; + also read from SITES_REGISTRY_PUSH_HOST) --help Show this help EOF } @@ -37,6 +45,7 @@ while [[ $# -gt 0 ]]; do --api-secret) api_secret=${2:?missing API Secret name}; shift 2 ;; --database-secret) database_secret=${2:?missing database Secret name}; shift 2 ;; --registry-secret) registry_secret=${2:?missing registry Secret name}; shift 2 ;; + --registry-host) registry_host=${2:?missing registry host}; shift 2 ;; --help|-h) usage; exit 0 ;; *) echo "unknown argument: $1" >&2; usage >&2; exit 2 ;; esac @@ -57,6 +66,8 @@ if [[ ${#namespace} -gt 63 ]]; then exit 2 fi +registry_host=${registry_host:-sites-registry.$namespace.svc:5000} + secret_dir=$(mktemp -d "${TMPDIR:-/tmp}/site-secrets.XXXXXX") chmod 0700 "$secret_dir" trap 'find "$secret_dir" -type f -exec chmod 0600 {} + 2>/dev/null || true; rm -rf -- "$secret_dir"' EXIT @@ -70,6 +81,15 @@ openssl rand -base64 36 | tr -d '\n' >"$secret_dir/database-password" openssl rand -base64 36 | tr -d '\n' >"$secret_dir/registry-password" registry_sha=$(openssl dgst -binary -sha1 <"$secret_dir/registry-password" | openssl base64 -A) printf 'sites:{SHA}%s\n' "$registry_sha" >"$secret_dir/htpasswd" +# The third form of the same credential, for the BuildKit Job. It sets +# DOCKER_CONFIG to the directory this Secret is mounted at, and Docker looks for +# `config.json` there -- the name is Docker's, not a preference. Without this key +# the Job's volume cannot be set up at all, so the Pod never starts and the only +# thing the caller ever sees is the build deadline expiring several minutes +# later, naming nothing. +registry_auth=$(printf 'sites:%s' "$(cat "$secret_dir/registry-password")" | openssl base64 -A) +printf '{"auths":{"%s":{"auth":"%s"}}}\n' "$registry_host" "$registry_auth" \ + >"$secret_dir/registry-config.json" kube create namespace "$namespace" --dry-run=client -o yaml | kube apply -f - >/dev/null @@ -102,11 +122,12 @@ else --from-file=password="$secret_dir/database-password" >/dev/null fi if kube -n "$namespace" get secret "$registry_secret" >/dev/null 2>&1; then - require_existing_keys "$registry_secret" password htpasswd + require_existing_keys "$registry_secret" password htpasswd config.json else kube -n "$namespace" create secret generic "$registry_secret" \ --from-file=password="$secret_dir/registry-password" \ - --from-file=htpasswd="$secret_dir/htpasswd" >/dev/null + --from-file=htpasswd="$secret_dir/htpasswd" \ + --from-file=config.json="$secret_dir/registry-config.json" >/dev/null fi echo "site development Secrets are present in namespace $namespace (existing values preserved)" diff --git a/scripts/cluster.sh b/scripts/cluster.sh index 7ba2825..9f3a921 100755 --- a/scripts/cluster.sh +++ b/scripts/cluster.sh @@ -24,6 +24,11 @@ Standard inputs: SITES_CLUSTER_POD_CIDR Actual cluster Pod CIDR; overrides the values file. SITES_LOCAL_PATH_PROVISIONER_ENABLED true/false; set false when the cluster already has one. + SITES_HOST_PORT_BASE Host port this environment forwards the NodePort + pool's first port to. Unset means no such forward + exists and a public deployment reports no URL. + SITES_PUBLIC_URL_HOST Scheme and host those forwards answer on + (default http://127.0.0.1). SITES_CONTROL_IMAGE_REPOSITORY Optional pre-published control image repository. SITES_CONTROL_IMAGE_TAG Optional control image tag. SITES_CONTROL_IMAGE_DIGEST Optional immutable sha256 digest. @@ -73,6 +78,16 @@ if [[ -n "${SITES_LOCAL_PATH_PROVISIONER_ENABLED:-}" ]]; then esac cluster_args+=(--set "localPathProvisioner.enabled=$SITES_LOCAL_PATH_PROVISIONER_ENABLED") fi +if [[ -n "${SITES_HOST_PORT_BASE:-}" ]]; then + [[ "$SITES_HOST_PORT_BASE" =~ ^[0-9]{1,5}$ ]] || { + printf 'SITES_HOST_PORT_BASE must be a port number\n' >&2 + exit 2 + } + cluster_args+=(--set-string "nodePort.hostPortBase=$SITES_HOST_PORT_BASE") +fi +if [[ -n "${SITES_PUBLIC_URL_HOST:-}" ]]; then + cluster_args+=(--set-string "nodePort.publicUrlHost=$SITES_PUBLIC_URL_HOST") +fi if [[ -n "${SITES_CONTROL_IMAGE_TAG:-}" ]]; then image_args+=(--set "images.control.tag=$SITES_CONTROL_IMAGE_TAG") fi diff --git a/scripts/quickstart-kubeadm.sh b/scripts/quickstart-kubeadm.sh index 93fa24d..ff93f03 100755 --- a/scripts/quickstart-kubeadm.sh +++ b/scripts/quickstart-kubeadm.sh @@ -12,6 +12,10 @@ worker_memory=${SITES_QUICKSTART_WORKER_MEMORY_GIB:-3} worker_disk=${SITES_QUICKSTART_WORKER_DISK_GIB:-20} api_port=${SITES_QUICKSTART_KUBE_API_PORT:-18447} console_port=${SITES_QUICKSTART_CONSOLE_PORT:-18091} +# Host port this trial's relay maps NODE_PORT_MIN to. It is the trial harness +# that owns the mapping -- Lima forwards guest 30080..30088 here -- so the trial +# is what declares it to the Chart. Keep aligned with dev/kubeadm/lima.yaml. +host_port_base=${SITES_QUICKSTART_HOST_PORT_BASE:-18090} pod_cidr=${SITES_QUICKSTART_POD_CIDR:-10.201.0.0/16} service_cidr=${SITES_QUICKSTART_SERVICE_CIDR:-10.202.0.0/16} cilium_version=${SITES_QUICKSTART_CILIUM_VERSION:-1.19.6} @@ -71,7 +75,7 @@ access Forward and open the console at http://127.0.0.1:18091/console/. token Print the disposable trial's local admin token for console login. clean Delete only the site-quickstart VMs, network, and this checkout's local state. -Prerequisites: Lima, a running Docker daemon, kubectl, Helm, curl, uv, lsof, +Prerequisites: Lima, a running Docker daemon, kubectl, Helm, curl, uv, and Python 3.12+. The default three-node topology allocates 8 CPUs, 10 GiB RAM, and 70 GiB of sparse disk across its VMs. No other repository, pre-created Lima network, or published Site image is used. @@ -85,9 +89,37 @@ require_command() { } } +# Of the ports given, echo those the host cannot hand to Lima. +# +# A bind probe, not a listener listing. `lsof` only reports sockets owned by the +# user running it, so a port held by root or by another account reads as free and +# the trial then fails minutes later at the exact forward this check exists to +# catch. Binding answers the question actually being asked -- can a server take +# this address -- for any owner. Lima publishes these forwards on 127.0.0.1, so +# that is the address probed, and SO_REUSEADDR matches what a server would set, +# keeping a lingering TIME_WAIT socket from reading as a conflict. +ports_in_use() { + python3 - "$@" <<'PY' +import socket +import sys + +busy = [] +for port in sys.argv[1:]: + probe = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + probe.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + try: + probe.bind(("127.0.0.1", int(port))) + except OSError: + busy.append(port) + finally: + probe.close() +print(" ".join(busy)) +PY +} + check_prerequisites() { local command_name missing=() - for command_name in limactl docker kubectl helm curl uv python3 lsof; do + for command_name in limactl docker kubectl helm curl uv python3; do command -v "$command_name" >/dev/null 2>&1 || missing+=("$command_name") done if ((${#missing[@]})); then @@ -111,17 +143,16 @@ PY exit 2 fi - # These are the complete fixed-port contract of the reference VM. Checking - # all of them now avoids a Lima forwarding failure several minutes later. + # The complete fixed-port contract of the reference VM: the Kubernetes API + # forward plus the nine contiguous host ports starting at host_port_base -- the + # NodePort pool, with the console occupying the one Lima does not forward. + # Checking all of them now avoids a Lima forwarding failure several minutes + # later. if ! vm_running; then - local port busy=() - for port in "$api_port" "$console_port" 18090 18092 18093 18094 18095 18096 18097 18098; do - if lsof -nP -iTCP:"$port" -sTCP:LISTEN >/dev/null 2>&1; then - busy+=("$port") - fi - done - if ((${#busy[@]})); then - printf 'quickstart host ports are already in use: %s\n' "${busy[*]}" >&2 + local busy + busy=$(ports_in_use "$api_port" $(seq "$host_port_base" $((host_port_base + 8)))) || exit 2 + if [[ -n "$busy" ]]; then + printf 'quickstart host ports are already in use: %s\n' "$busy" >&2 echo 'Stop the process or older Site trial using those ports, then rerun make quickstart.' >&2 exit 2 fi @@ -408,7 +439,7 @@ create_cluster() { fi if ! vm_exists; then - if lsof -nP -iTCP:"$api_port" -sTCP:LISTEN >/dev/null 2>&1; then + if [[ -n "$(ports_in_use "$api_port")" ]]; then printf 'host port %s is already in use; set SITES_QUICKSTART_KUBE_API_PORT and update the Lima template forward\n' "$api_port" >&2 exit 2 fi @@ -608,6 +639,7 @@ prove_site() { --set images.control.repository=site-control \ --set images.control.tag=quickstart \ --set "localPathProvisioner.allowedNodeNames[0]=${vm}-w1" \ + --set "nodePort.hostPortBase=$host_port_base" \ --set-value monitoring.enabled=true # A truly new node may need several minutes to pull the pinned database, # registry, proxy, and Prometheus images. Do not spend the API rollout budget diff --git a/scripts/standalone.sh b/scripts/standalone.sh index 5158937..96e2f03 100755 --- a/scripts/standalone.sh +++ b/scripts/standalone.sh @@ -6,7 +6,13 @@ action=${1:-} if [[ -n "$action" ]]; then shift; fi namespace=sites-local release=site -values_file="$root/charts/site/values-dev.yaml" +# Same two seams cluster.sh already exposes, and for the same reason: the values +# overlay names one specific local topology, and clusterNetwork.podCIDR is a fact +# about the target cluster that the operator refuses to start without. Without +# them this script -- and therefore scripts/cluster_benchmark.py, which installs +# through it -- could only ever run against a cluster whose Pod network happens +# to be the overlay's. +values_file=${SITES_HELM_VALUES:-$root/charts/site/values-dev.yaml} helm_set=() helm_set_value=() context=${SITES_KUBE_CONTEXT:-} @@ -30,13 +36,18 @@ Usage: standalone.sh [options] Options: --namespace NAME Kubernetes namespace (default: sites-local) --release NAME Helm release name (default: site) - --values FILE Helm values file (default: charts/site/values-dev.yaml) + --values FILE Helm values file (default: charts/site/values-dev.yaml, + or $SITES_HELM_VALUES) --set KEY=VALUE Additional Helm string value (repeatable) --set-value KEY=VALUE Additional typed Helm value (repeatable; booleans/numbers) EOF } +if [[ -n "${SITES_CLUSTER_POD_CIDR:-}" ]]; then + helm_set+=(--set-string "clusterNetwork.podCIDR=$SITES_CLUSTER_POD_CIDR") +fi + while [[ $# -gt 0 ]]; do case "$1" in --namespace) namespace=${2:?missing namespace}; shift 2 ;; diff --git a/src/sites/cli.py b/src/sites/cli.py index cf5499e..cd92b8b 100644 --- a/src/sites/cli.py +++ b/src/sites/cli.py @@ -349,8 +349,14 @@ def _run(args: argparse.Namespace) -> dict[str, Any]: def _add_deploy_flags(parser: argparse.ArgumentParser) -> None: parser.add_argument("--name", required=True, help="service name") parser.add_argument("--image", required=True, help="container image") - parser.add_argument("--port", type=int, default=8080) - parser.add_argument("--health-path", default="/") + parser.add_argument( + "--port", type=int, default=8080, + help="container port the workload listens on (default: %(default)s)", + ) + parser.add_argument( + "--health-path", default="/", + help="readiness and liveness path (default: %(default)s)", + ) parser.add_argument( "--liveness-path", default="", @@ -422,7 +428,7 @@ def _add_admin_commands(sub: argparse._SubParsersAction) -> None: merchant_create = merchants_sub.add_parser( "create", help="create a merchant and print its API key once", - description="The apiKey plaintext is only returned this time, and only the summary is stored in the library; if it is lost, you can only rotate-key.", + description="The plaintext API key is returned only this time; the control plane stores only its digest. A lost key can only be replaced with rotate-key.", ) merchant_create.add_argument("merchant_id") merchant_create.add_argument("--display-name", required=True) @@ -463,7 +469,7 @@ def _add_admin_commands(sub: argparse._SubParsersAction) -> None: tenant_create = tenants_sub.add_parser( "create", help="create a tenant and print its token once", - description="The token plaintext is returned only this time, and only the digest is stored in the library. If it is lost, it can only be rotated.", + description="The plaintext token is returned only this time; the control plane stores only its digest. A lost token can only be replaced with rotate.", ) tenant_create.add_argument("name") tenant_create.add_argument("--merchant", required=True) @@ -537,8 +543,14 @@ def build_parser() -> argparse.ArgumentParser: ) static.add_argument("--name", required=True) static.add_argument("--directory", required=True) - static.add_argument("--port", type=int, default=8080) - static.add_argument("--health-path", default="/") + static.add_argument( + "--port", type=int, default=8080, + help="container port of the fixed static runtime (default: %(default)s)", + ) + static.add_argument( + "--health-path", default="/", + help="readiness and liveness path (default: %(default)s)", + ) static.add_argument( "--exposure", choices=("public", "internal"), default="public" ) @@ -563,8 +575,20 @@ def build_parser() -> argparse.ArgumentParser: ) build_submit.add_argument("--name", required=True) build_submit.add_argument("--directory", required=True) - build_submit.add_argument("--port", type=int, default=8080) - build_submit.add_argument("--health-path", default="/healthz") + build_submit.add_argument( + "--port", type=int, default=8080, + help="container port your Dockerfile exposes (default: %(default)s)", + ) + build_submit.add_argument( + # Deliberately not `/` like the other two: a source build is an + # application the caller wrote, so it is expected to answer a real + # health endpoint. Silent, and different from its siblings, that + # difference cost a build the full readiness timeout and reported only + # "Deployment does not have minimum availability". + "--health-path", default="/healthz", + help="readiness and liveness path your image serves " + "(default: %(default)s, unlike deploy/deploy-static which default to /)", + ) build_status = build_sub.add_parser("status") build_status.add_argument("name") build_delete = build_sub.add_parser("delete") diff --git a/src/sites/exposure.py b/src/sites/exposure.py index 7412e01..447b80e 100644 --- a/src/sites/exposure.py +++ b/src/sites/exposure.py @@ -45,13 +45,32 @@ def bounded_public_route_default(configured: int) -> int: """Clamp a default quota to the selected backend's physical capacity.""" capacity = backend().capacity return min(configured, capacity) if capacity is not None else configured -# Public NodePort URLs use host_port = HOST_PORT_BASE + nodePort - NODE_PORT_MIN. +# Public NodePort URLs use host_port = host_port_base() + nodePort - NODE_PORT_MIN. # topology.py and its contract tests pin the same mapping, preventing a deployment # from becoming ready at a host URL that cannot actually be opened. PUBLIC_URL_HOST = ( getenv("SITES_PUBLIC_URL_HOST", "http://127.0.0.1") or "http://127.0.0.1" ) -HOST_PORT_BASE = int(getenv("SITES_HOST_PORT_BASE", "18090") or "18090") + + +def host_port_base() -> int | None: + """Host port this environment maps NODE_PORT_MIN to, or None if none is declared. + + A NodePort Service is reachable at ``:``; the extra hop from + a host port to that node port is a fact about the machine in front of the + cluster, not about the cluster. Only the reference kubeadm trial publishes + one (Lima forwards guest 30080..30088 to host 18090..18098 and the Chart + declares ``nodePort.hostPortBase``), so there is deliberately no default: + assuming 18090 everywhere hands foreign clusters a URL that resolves to + whatever else happens to own that host port, and the caller cannot tell that + from a working one. Undeclared, ``public_url`` reports nothing rather than a + guess -- the same reason ``clusterNetwork.podCIDR`` has no default. + + Read per call rather than frozen at import so the Chart, the trial script and + the tests can each declare their own mapping, exactly like ``backend()``. + """ + declared = (getenv("SITES_HOST_PORT_BASE", "") or "").strip() + return int(declared) if declared else None # --- Gateway backend parameters --- # sslip.io resolves ..sslip.io to the IP, so the local environment can provide each @@ -210,7 +229,10 @@ def public_url(self, spec: dict[str, Any]) -> str | None: return None if port <= 0: return None - return f"{PUBLIC_URL_HOST}:{HOST_PORT_BASE + port - NODE_PORT_MIN}" + base = host_port_base() + if base is None: + return None + return f"{PUBLIC_URL_HOST}:{base + port - NODE_PORT_MIN}" class GatewayExposure(ExposureBackend): diff --git a/src/sites/operator.py b/src/sites/operator.py index 777a79b..7948944 100644 --- a/src/sites/operator.py +++ b/src/sites/operator.py @@ -342,14 +342,31 @@ def _rollout_stall_reason( """ reasons: list[str] = [] try: - for condition in (deployment.get("status") or {}).get("conditions") or []: + conditions = (deployment.get("status") or {}).get("conditions") or [] + # ReplicaFailure first, because the pod loop below cannot see the one + # stall that produces no pod at all. A namespace ResourceQuota rejects + # creation at the ReplicaSet, so every container state is empty and the + # Available condition says only "does not have minimum availability" -- + # the caller was told the workload was slow when it had in fact been + # refused, and the exceeded quota was named nowhere it would look. + # kubelet copies the reason onto this object, so reading it costs nothing. + for condition in conditions: if ( - str(condition.get("type")) == "Available" - and str(condition.get("status")) != "True" + str(condition.get("type")) == "ReplicaFailure" + and str(condition.get("status")) == "True" and (message := str(condition.get("message") or "").strip()) ): reasons.append(message) break + if not reasons: + for condition in conditions: + if ( + str(condition.get("type")) == "Available" + and str(condition.get("status")) != "True" + and (message := str(condition.get("message") or "").strip()) + ): + reasons.append(message) + break selector = urllib.parse.quote( ",".join( f"{key}={value}" diff --git a/src/sites/serializers.py b/src/sites/serializers.py index 0135144..622d3e7 100644 --- a/src/sites/serializers.py +++ b/src/sites/serializers.py @@ -318,6 +318,19 @@ def bundle_response( and observed == generation ), "url": status.get("url"), + # docs/AGENT_CONTRACT.md makes status.verification the only + # success criterion, and bundles are one of its deployment entry + # points. The operator writes this per component exactly as it + # does for a standalone deployment; leaving it out of the + # projection meant a bundle caller saw phase Running and nothing + # else, with no way to reach evidence that already existed. + "verification": status.get("verification"), + # Same omission on the failure side: a component that could not + # roll out reported `Failed` and nothing else, while the sentence + # naming the cause -- an exhausted tenant quota, an image that + # will not pull -- was already on the resource. A standalone + # deployment has carried it all along. + "message": status.get("message", ""), } ) # The expected number of components is the currently found group: the control plane does not preset any bundle composition. diff --git a/src/sites/validation.py b/src/sites/validation.py index e6903bd..4c4238f 100644 --- a/src/sites/validation.py +++ b/src/sites/validation.py @@ -607,8 +607,9 @@ def normalize_deploy_payload( if not exposure_backend.backend().supports_scale_to_zero: raise ValidationError( "scaleToZero requires L7 gateway (SITES_EXPOSURE_BACKEND=gateway): " - "NodePort directly DNATs external traffic to the site Pod. After shrinking to 0, there will be no" - "The link can receive the request and trigger the wake-up" + "NodePort DNATs external traffic straight to the site Pod, so once it " + "scales to 0 there is no hop left to receive the request and trigger " + "the wake-up" ) # Only write spec when true, the same as nodePort: the shape of the existing CR is not affected, # On the reading side, spec.get("scaleToZero") is used to judge. diff --git a/tests/test_architecture.py b/tests/test_architecture.py index 9429724..f81d505 100644 --- a/tests/test_architecture.py +++ b/tests/test_architecture.py @@ -2,6 +2,7 @@ from __future__ import annotations import ast +import re import unittest from pathlib import Path @@ -52,5 +53,96 @@ def visit(module: str) -> None: visit(module) +class RefusalTextTests(unittest.TestCase): + """Every message the caller is left holding has to read as one sentence.""" + + RUN_TOGETHER = re.compile(r"^[a-z]{2,}[A-Z][a-z]{2,}$") + + @staticmethod + def _repository_word_counts() -> dict[str, int]: + """How often each word occurs across the sources a name can live in. + + No hand-written vocabulary, which is what rots after a rename and then + passes everything. A real field name is written down somewhere else too + -- a schema, the Chart, a document -- while a word produced by joining + two message fragments exists in exactly one place: the message. + """ + counts: dict[str, int] = {} + files = [ + *PACKAGE.glob("*.py"), + *(ROOT / "charts").rglob("*.yaml"), + *(ROOT / "docs").glob("*.md"), + ROOT / "README.md", + ] + for path in files: + for word in re.findall(r"[A-Za-z][A-Za-z0-9_]*", path.read_text(encoding="utf-8")): + counts[word] = counts.get(word, 0) + 1 + return counts + + @classmethod + def _run_together(cls, text: str, counts: dict[str, int]) -> str | None: + for match in re.finditer(r"[A-Za-z][A-Za-z0-9_]*", text): + word = match.group(0) + if cls.RUN_TOGETHER.match(word) and counts.get(word, 0) <= 1: + return word + return None + + def test_the_check_can_tell_the_two_apart(self) -> None: + """Prove the judge before trusting the sweep below. + + Its first version stripped every token containing a capital as + "an identifier" -- including the run-together word it was looking for -- + and reported zero across the whole package. A sweep that cannot fail is + not evidence, so both directions are asserted here. + """ + counts = {"scaleToZero": 40, "healthPath": 30, "noThe": 1} + self.assertEqual( + self._run_together("there will be noThe link can receive", counts), + "noThe", + ) + self.assertIsNone(self._run_together("scaleToZero needs a gateway", counts)) + self.assertIsNone(self._run_together("healthPath is /", counts)) + + def test_field_names_used_elsewhere_are_not_flagged(self) -> None: + # Names that appear only inside message text still count as real when + # the Chart, a schema or a document also writes them down. + counts = self._repository_word_counts() + for field in ("clusterNetwork", "fieldRef", "deploymentIntent", "claimValue"): + with self.subTest(field=field): + self.assertGreater(counts.get(field, 0), 1) + + def test_no_refusal_message_runs_two_sentences_together(self) -> None: + """Two translated fragments joined without a space become one bad word. + + `"...there will be no" "The link can receive..."` reached the caller as + "there will be noThe link can receive", and it is the only thing the + caller gets: the request is refused with a 400 whose whole explanation is + that sentence. + """ + counts = self._repository_word_counts() + offenders = [] + scanned = 0 + for source in sorted(PACKAGE.glob("*.py")): + tree = ast.parse(source.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if not isinstance(node, ast.Raise): + continue + for child in ast.walk(node): + if ( + isinstance(child, ast.Constant) + and isinstance(child.value, str) + and len(child.value) > 25 + ): + scanned += 1 + text = " ".join(child.value.split()) + word = self._run_together(text, counts) + if word: + offenders.append(f"{source.name}:{child.lineno} {word!r}") + # A zero with a zero denominator would mean the sweep found nothing to + # look at, which is not the same answer. + self.assertGreater(scanned, 100, "no refusal messages were examined") + self.assertEqual(offenders, []) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_builds.py b/tests/test_builds.py index 19ca5ba..7cc358e 100644 --- a/tests/test_builds.py +++ b/tests/test_builds.py @@ -214,6 +214,54 @@ def test_job_has_no_host_or_kubernetes_credentials(self) -> None: self.assertEqual(job["spec"]["backoffLimit"], 0) self.assertEqual(job["spec"]["activeDeadlineSeconds"], 300) + def test_every_secret_key_the_job_mounts_is_one_the_bootstrap_creates(self) -> None: + """Nothing may mount a Secret key no supported path ever writes. + + The Job selects `config.json` out of sites-registry-auth, and the + bootstrap helper created only `password` and `htpasswd`. A missing + selected key is not a soft failure: kubelet refuses to set the volume + up, so the Pod never starts and the Job simply runs out its deadline. + The caller's only signal was "Job was active longer than specified + deadline" five minutes later, which names neither the Secret nor the + key, and the whole source-build entry point was dead on every + installation that followed this repository's own instructions. + """ + root = Path(__file__).resolve().parent.parent + bootstrap = (root / "scripts" / "bootstrap-standalone-secrets.sh").read_text( + encoding="utf-8" + ) + pod = build_job_resource(_build(), namespace="sites-local")["spec"][ + "template" + ]["spec"] + selected: list[tuple[str, str]] = [] + for volume in pod["volumes"]: + secret = volume.get("secret") or {} + for item in secret.get("items") or []: + selected.append((secret["secretName"], item["key"])) + self.assertIn(("sites-registry-auth", "config.json"), selected) + required = next( + line + for line in bootstrap.splitlines() + if 'require_existing_keys "$registry_secret"' in line + ) + for secret_name, key in selected: + with self.subTest(secret=secret_name, key=key): + self.assertIn( + f"--from-file={key}=", + bootstrap, + f"{secret_name} key {key!r} is mounted but never created", + ) + # An installation from before this key existed keeps its Secret, + # so a fresh-install-only fix would leave it in the same silent + # failure. Listing the key here makes the rerun say which key is + # missing instead of leaving it to the build deadline. + self.assertIn( + key, + required, + f"an existing {secret_name} missing {key!r} must be refused, " + "not left to fail as a build deadline", + ) + def test_job_reports_its_own_digest_onto_the_build_volume(self) -> None: build = _build() container = build_job_resource(build, namespace="sites-local")[ diff --git a/tests/test_exposure.py b/tests/test_exposure.py index 0b396a3..100f520 100644 --- a/tests/test_exposure.py +++ b/tests/test_exposure.py @@ -53,6 +53,33 @@ def using_backend(name: str): os.environ["SITES_EXPOSURE_BACKEND"] = previous +# The one environment that really does forward this host port to NODE_PORT_MIN: +# the repository's own kubeadm trial, via dev/kubeadm/lima.yaml. +REFERENCE_HOST_PORT_BASE = "18090" + + +@contextmanager +def using_host_port_base(base: str | None): + """Temporarily declare, or withdraw, this environment's host port mapping. + + exposure.host_port_base() rereads env per call for the same reason backend() + does: the mapping belongs to whoever set the host up, so a module constant + frozen at import would let the first test decide it for every later one. + """ + previous = os.environ.get("SITES_HOST_PORT_BASE") + if base is None: + os.environ.pop("SITES_HOST_PORT_BASE", None) + else: + os.environ["SITES_HOST_PORT_BASE"] = base + try: + yield + finally: + if previous is None: + os.environ.pop("SITES_HOST_PORT_BASE", None) + else: + os.environ["SITES_HOST_PORT_BASE"] = previous + + def _spec(*, merchant=DEFAULT_MERCHANT_ID, user="local", **overrides): payload = { "name": "demo", @@ -579,6 +606,85 @@ def test_listener_hostname_covers_the_domain_suffix(self) -> None: listener = self._doc("Gateway")["spec"]["listeners"][0] self.assertEqual(listener["hostname"], f"*.{exposure.DOMAIN_SUFFIX}") + def test_the_gateway_identity_survives_a_non_default_namespace(self) -> None: + """The control plane must be told which Gateway the Chart made. + + The Gateway object is named after `namespaces.gateway`, while + `SITES_GATEWAY_NAME` and `SITES_GATEWAY_NAMESPACE` defaulted to the + literal `sites-gateway` and the Chart never set them. They agreed only + when the release happened to live in a namespace of that name -- and + both install scripts set `namespaces.gateway` to the release namespace, + which defaults to `sites-local`. + + Measured on a real cluster before the fix: every HTTPRoute carried + `parentRef: sites-gateway/sites-gateway`, no Gateway claimed it (the + route had no parent status at all), the tenant NetworkPolicy admitted a + data-plane Pod label nothing carried, and the site was `Running` with + `verification.ok` while its public URL answered 404. After: route + Accepted=True, policy label `sites-gw`, URL 200 with the verified digest. + + The existing identity test above renders the defaults, where both sides + read `sites-gateway` and agree no matter what the Chart does; this one + has to use a different namespace or it asserts nothing. + """ + overrides = ("--set-string", "namespaces.gateway=elsewhere-gw") + docs = chart.documents("08-gateway.yaml", *overrides) + gateway = next(doc for doc in docs if doc["kind"] == "Gateway") + config = next( + doc for doc in docs + if doc["kind"] == "ConfigMap" and "SITES_GATEWAY_NAME" in doc["data"] + ) + self.assertEqual(gateway["metadata"]["name"], "elsewhere-gw") + self.assertEqual(config["data"]["SITES_GATEWAY_NAME"], "elsewhere-gw") + self.assertEqual(config["data"]["SITES_GATEWAY_NAMESPACE"], "elsewhere-gw") + self.assertEqual( + config["data"]["SITES_GATEWAY_NAME"], gateway["metadata"]["name"] + ) + self.assertEqual( + config["data"]["SITES_GATEWAY_NAMESPACE"], + gateway["metadata"]["namespace"], + ) + + def test_the_host_facing_url_parts_are_values_not_literals(self) -> None: + """A different host must not require editing the template. + + These three decide the URL the caller is handed, and all three were + literals: `127.0.0.1.sslip.io`, `http`, and `18090` -- the reference + Lima forward, written into every installation's gateway URLs. The + suffix was worse than merely hardcoded: it appeared twice, in the + ConfigMap and in the listener wildcard, and the file told the operator + to change both by hand. One value renders both now, so they cannot be + changed apart. + """ + overrides = ( + "--set-string", "gateway.domainSuffix=apps.example.test", + "--set-string", "gateway.scheme=https", + "--set", "gateway.hostPort=443", + ) + docs = chart.documents("08-gateway.yaml", *overrides) + config = next( + doc for doc in docs + if doc["kind"] == "ConfigMap" and "SITES_DOMAIN_SUFFIX" in doc["data"] + ) + self.assertEqual(config["data"]["SITES_DOMAIN_SUFFIX"], "apps.example.test") + self.assertEqual(config["data"]["SITES_GATEWAY_SCHEME"], "https") + self.assertEqual(config["data"]["SITES_GATEWAY_HOST_PORT"], "443") + listener = next( + doc for doc in docs if doc["kind"] == "Gateway" + )["spec"]["listeners"][0] + self.assertEqual(listener["hostname"], "*.apps.example.test") + # The default render still is what it was, so this is a seam and not a + # behaviour change for anyone already on the reference topology. + default = next( + doc for doc in self._docs() + if doc["kind"] == "ConfigMap" and "SITES_DOMAIN_SUFFIX" in doc["data"] + ) + self.assertEqual(default["data"]["SITES_DOMAIN_SUFFIX"], exposure.DOMAIN_SUFFIX) + self.assertEqual( + default["data"]["SITES_GATEWAY_HOST_PORT"], + str(exposure.GATEWAY_HOST_PORT), + ) + def test_listener_admits_routes_from_tenant_namespaces(self) -> None: """Must be a Selector: HTTPRoute is built in tenant ns, and Gateway is in its own ns. @@ -685,12 +791,28 @@ def test_both_control_plane_components_are_covered(self) -> None: class PublicUrlTest(unittest.TestCase): def test_nodeport_url_is_host_plus_mapped_port(self) -> None: - with using_backend("nodeport"): + with using_backend("nodeport"), using_host_port_base( + REFERENCE_HOST_PORT_BASE + ): url = public_url_for_spec(_spec()) self.assertEqual( - url, f"{exposure.PUBLIC_URL_HOST}:{exposure.HOST_PORT_BASE}" + url, f"{exposure.PUBLIC_URL_HOST}:{REFERENCE_HOST_PORT_BASE}" ) + def test_nodeport_url_is_absent_when_no_host_mapping_is_declared(self) -> None: + """No declared mapping must yield no URL, not the reference one. + + The offset formula describes a host that really does forward 18090.. to + 30080.., which the trial does and an arbitrary cluster does not. While + 18090 was the default, every other installation was handed a URL naming + whatever else owned that host port, and nothing could tell it from a + working one: server-side verification passes either way because it probes + the in-cluster address, so the failure only ever surfaced as a user + clicking Open and getting nothing. + """ + with using_backend("nodeport"), using_host_port_base(None): + self.assertIsNone(public_url_for_spec(_spec())) + def test_gateway_url_is_a_hostname_not_a_port_offset(self) -> None: with using_backend("gateway"): url = public_url_for_spec(_spec()) diff --git a/tests/test_helm_package.py b/tests/test_helm_package.py index 19f71e7..ac4bef7 100644 --- a/tests/test_helm_package.py +++ b/tests/test_helm_package.py @@ -41,6 +41,27 @@ def test_quickstart_is_a_repository_owned_kubeadm_workflow(self) -> None: self.assertNotIn('node-role.kubernetes.io/control-plane-', script) self.assertIn("dev/kubeadm/lima.yaml", script) + def test_the_port_preflight_binds_rather_than_listing_owned_sockets(self) -> None: + """The preflight must detect a busy port whoever owns it. + + `lsof` reports only sockets owned by the user running it, so a quickstart + port held by root or another account read as free and `make quickstart` + then failed several minutes later on the Lima forward this check exists + to catch -- a false pass in exactly its own scenario. Binding the address + Lima binds answers the question for any owner. + """ + script = (ROOT / "scripts" / "quickstart-kubeadm.sh").read_text(encoding="utf-8") + self.assertNotIn("lsof", script.replace("`lsof`", "")) + self.assertIn("ports_in_use()", script) + self.assertIn('probe.bind(("127.0.0.1", int(port)))', script) + self.assertIn("socket.SO_REUSEADDR", script) + # Both preflights go through it: the fixed-port contract and the + # API-server forward checked again just before the VM is created. + self.assertIn( + 'busy=$(ports_in_use "$api_port" $(seq "$host_port_base"', script + ) + self.assertIn('if [[ -n "$(ports_in_use "$api_port")" ]]; then', script) + def test_quickstart_lima_template_needs_no_precreated_network(self) -> None: template = yaml.safe_load( (ROOT / "dev" / "kubeadm" / "lima.yaml").read_text(encoding="utf-8") @@ -389,7 +410,140 @@ def test_standalone_scripts_are_syntax_checked_and_never_embed_secret_values(sel self.assertIn("SITES_CONTROL_IMAGE_DIGEST", adapter) self.assertNotIn("INFRA_", adapter) self.assertNotIn("limactl", adapter) + self.assertIn("SITES_HOST_PORT_BASE", adapter) + self.assertIn("nodePort.hostPortBase", adapter) + + def test_cluster_scoped_crds_make_the_release_exclusive_and_say_so(self) -> None: + """`--namespace` looks like it permits a second install. It does not. + + Both CRDs are cluster-scoped and rendered as ordinary templates, so the + first release owns them and any later install anywhere in the cluster + stops on Helm's ownership check -- an error about + `meta.helm.sh/release-namespace` annotations, which says nothing about + Site to a reader who has not met it before. Nothing said the limit + existed, while `--namespace` on both install scripts implied the + opposite. + + If the CRDs ever move to the Chart's `crds/` directory the exclusivity + goes away, and this test should be deleted rather than adjusted. + """ + rendered = subprocess.run( + ["helm", "template", "site", str(CHART), *POD_CIDR], + check=True, + capture_output=True, + text=True, + ).stdout + crds = [ + item + for item in yaml.safe_load_all(rendered) + if isinstance(item, dict) + and item.get("kind") == "CustomResourceDefinition" + ] + self.assertEqual( + sorted(item["metadata"]["name"] for item in crds), + ["sitebuilds.sites.local", "sitedeployments.sites.local"], + ) + self.assertFalse( + (CHART / "crds").exists(), + "CRDs moved out of templates/; the exclusivity note is now wrong", + ) + standalone_doc = (ROOT / "docs" / "STANDALONE.md").read_text(encoding="utf-8") + self.assertIn("One release per cluster", standalone_doc) + self.assertIn("meta.helm.sh/release-namespace", standalone_doc) + + def test_both_install_paths_take_the_same_cluster_facts(self) -> None: + """standalone.sh must accept what cluster.sh accepts. + + The two are the repository's install seams and cluster_benchmark.py goes + through standalone.sh. That one hardcoded values-dev.yaml, whose + clusterNetwork.podCIDR names the reference kubeadm topology, and the + operator refuses to start when its own address falls outside it. So the + benchmark -- the script whose report invites reproduction -- could only + run on a cluster whose Pod network happened to match, and nothing said + so: the install succeeds and the operator then refuses. + """ + standalone = (ROOT / "scripts" / "standalone.sh").read_text(encoding="utf-8") + adapter = (ROOT / "scripts" / "cluster.sh").read_text(encoding="utf-8") + for seam in ("SITES_HELM_VALUES", "SITES_CLUSTER_POD_CIDR"): + with self.subTest(seam=seam): + self.assertIn(seam, adapter) + self.assertIn(seam, standalone) + self.assertIn( + 'values_file=${SITES_HELM_VALUES:-$root/charts/site/values-dev.yaml}', + standalone, + ) + self.assertIn( + 'helm_set+=(--set-string "clusterNetwork.podCIDR=$SITES_CLUSTER_POD_CIDR")', + standalone, + ) + def test_registry_address_follows_the_namespace_it_is_installed_into(self) -> None: + """The registry Service address must come from namespaces.control. + + The Service is created in that namespace, but the code defaults pinned + it at `sites-registry.sites-local.svc:5000`. Installed anywhere else, + builds pushed to a Service that is not there and the control plane + resolved digests against the same missing address -- with the Chart + rendering cleanly and nothing at any layer naming the namespace. + """ + rendered = subprocess.run( + [ + "helm", + "template", + "site", + str(CHART), + *POD_CIDR, + "--set-string", + "namespaces.control=elsewhere", + ], + check=True, + capture_output=True, + text=True, + ).stdout + self.assertIn('value: "sites-registry.elsewhere.svc:5000"', rendered) + self.assertIn('value: "http://sites-registry.elsewhere.svc:5000"', rendered) + self.assertNotIn('value: "sites-registry.sites-local.svc:5000"', rendered) + # Both processes reach the registry: the API resolves and deletes + # images, the operator runs the build. One of the two carrying it is + # the same defect with a smaller blast radius. + self.assertEqual(rendered.count("SITES_REGISTRY_PUSH_HOST"), 2) + self.assertEqual(rendered.count("SITES_REGISTRY_API"), 2) + + def test_default_install_declares_no_host_port_mapping(self) -> None: + """The Chart must not assume a host forwards anything to the NodePort pool. + + Only the environment around a cluster can know that, and a wrong guess is + not visibly wrong: the deployment is healthy and control-plane + verification passes, because verification probes the in-cluster address. + A default here therefore reaches the user as a public URL naming whatever + else happens to own that host port. Absent the variable, + exposure.host_port_base() returns None and no URL is claimed at all. + """ + rendered = subprocess.run( + ["helm", "template", "site", str(CHART), *POD_CIDR], + check=True, + capture_output=True, + text=True, + ).stdout + self.assertNotIn("SITES_HOST_PORT_BASE", rendered) + self.assertIn("SITES_PUBLIC_URL_HOST", rendered) + + declared = subprocess.run( + [ + "helm", + "template", + "site", + str(CHART), + *POD_CIDR, + "--set-string", + "nodePort.hostPortBase=18090", + ], + check=True, + capture_output=True, + text=True, + ).stdout + self.assertIn("SITES_HOST_PORT_BASE", declared) + self.assertIn('value: "18090"', declared) if __name__ == "__main__": diff --git a/tests/test_interface.py b/tests/test_interface.py index e322375..d8908d6 100644 --- a/tests/test_interface.py +++ b/tests/test_interface.py @@ -9,6 +9,7 @@ import hashlib import io import json +import re import os import tempfile import threading @@ -1796,6 +1797,75 @@ def test_the_static_runtime_image_has_one_home(self) -> None: class DeploymentIntentBoundaryTests(unittest.TestCase): + def test_the_authorization_an_agent_host_must_inject_is_documented(self) -> None: + """A host cannot implement an object whose shape is written nowhere. + + Every MCP write tool refuses without `_agent_deployment_authorization`, + and the argument is deliberately kept out of `tools/list` so the model + never learns it exists. That leaves the document as the only place a + host integrator can read it. Following AGENT_CONTRACT's own client + configuration, the whole write surface answered + `deployment_authorization_required` and the message names nothing to + implement. + + Each field is checked against the validator rather than a copy of it, so + the table cannot drift away from what is actually enforced. + """ + from sites import mcp as sites_mcp + + contract = ( + Path(__file__).resolve().parent.parent / "docs" / "AGENT_CONTRACT.md" + ).read_text(encoding="utf-8") + self.assertIn(sites_mcp.DEPLOYMENT_AUTHORIZATION_ARGUMENT, contract) + for field in ("version", "runId", "nonce", "expiresAt", "allowInternal"): + with self.subTest(field=field): + self.assertIn(f"`{field}`", contract) + + valid = { + "version": 1, + "runId": "run-1", + "nonce": "n" * 24, + "expiresAt": time.time() + 60, + } + base = { + "deploymentIntent": "publish this and give me the URL", + sites_mcp.DEPLOYMENT_AUTHORIZATION_ARGUMENT: valid, + } + self.assertEqual( + sites_mcp._require_deployment_intent(base), + "publish this and give me the URL", + ) + # Each documented rule must be the reason a call is refused when broken. + for field, broken in ( + ("version", 2), + ("runId", ""), + ("nonce", "n" * 23), + ("expiresAt", time.time() - 1), + ): + with self.subTest(broken=field): + arguments = dict(base) + arguments[sites_mcp.DEPLOYMENT_AUTHORIZATION_ARGUMENT] = { + **valid, + field: broken, + } + with self.assertRaises(sites_mcp.ValidationError) as caught: + sites_mcp._require_deployment_intent(arguments) + self.assertIn("deployment_authorization_required", str(caught.exception)) + # allowInternal is what the table says it is: absent means internal is refused. + with self.assertRaises(sites_mcp.ValidationError): + sites_mcp._require_standalone_exposure_authorization( + {**base, "exposure": "internal"} + ) + sites_mcp._require_standalone_exposure_authorization( + { + "exposure": "internal", + sites_mcp.DEPLOYMENT_AUTHORIZATION_ARGUMENT: { + **valid, + "allowInternal": True, + }, + } + ) + def test_preview_only_intent_is_rejected_before_dispatch(self) -> None: from sites import mcp as sites_mcp @@ -1903,5 +1973,80 @@ def test_gateway_unbounded_capacity_is_rendered_unambiguously(self) -> None: self.assertNotIn("None", sentence) +class CliHelpContractTests(unittest.TestCase): + """What `--help` promises, since nothing else ever executes it.""" + + def test_admin_help_only_names_commands_that_exist(self) -> None: + """Help that tells the reader what to run next must name a real command. + + Both `create` descriptions end by saying how to replace a lost + credential, and the two groups spell that differently -- merchants use + `rotate-key`, tenants use `rotate`. Text like this is never executed, so + a wrong name here is only found by someone typing it and getting + `invalid choice`. + """ + parser = build_parser() + admin = next( + action.choices["admin"] + for action in parser._subparsers._group_actions + if "admin" in action.choices + ) + groups = next( + action.choices for action in admin._subparsers._group_actions + ) + for group_name, expected in (("merchants", "rotate-key"), ("tenants", "rotate")): + with self.subTest(group=group_name): + commands = next( + action.choices + for action in groups[group_name]._subparsers._group_actions + ) + self.assertIn(expected, commands) + description = commands["create"].description or "" + # Whole tokens, not a substring search: "rotate" is a prefix of + # "rotate-token", so `in` would accept the name this group does + # not have. + named = set(re.findall(r"rotate[a-z-]*", description)) + self.assertEqual(named, {expected}, description) + + def test_silent_flag_defaults_are_stated_in_help(self) -> None: + """Defaults a caller cannot see must at least be printed. + + `--health-path` is `/` for deploy and deploy-static and `/healthz` for + build submit, and none of the three said so. Taking the sibling default + on faith produced a build that pushed a correct image and then spent the + whole readiness window failing a probe on a path the image does not + serve, reported as "Deployment does not have minimum availability" -- + which points at the workload rather than at the flag. + """ + expected = { + ("deploy",): "/", + ("deploy-static",): "/", + ("build", "submit"): "/healthz", + } + for path, default in expected.items(): + with self.subTest(command=" ".join(path)): + parser = build_parser() + for name in path: + parser = next( + action.choices[name] + for action in parser._subparsers._group_actions + if name in action.choices + ) + for flag in ("--health-path", "--port"): + action = next( + item for item in parser._actions if flag in item.option_strings + ) + self.assertTrue( + action.help, f"{' '.join(path)} {flag} has no help at all" + ) + self.assertIn("%(default)s", action.help, flag) + health = next( + item + for item in parser._actions + if "--health-path" in item.option_strings + ) + self.assertEqual(health.default, default) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_readme_claims.py b/tests/test_readme_claims.py index 62bf3ec..aa7df9c 100644 --- a/tests/test_readme_claims.py +++ b/tests/test_readme_claims.py @@ -94,5 +94,117 @@ def test_readme_states_the_count_make_test_collects(self) -> None: ) +class ModuleMapTests(unittest.TestCase): + """The per-module table has to still describe this package. + + It listed 43 of 45 modules: `api_mcp.py` -- the `POST /mcp` endpoint, which + the README and AGENT_CONTRACT both treat as a headline surface -- and + `http_kit.py` were simply absent, and the composition-root sentence counted + "eight endpoint mixins" while `Handler` combined nine. A table maintained by + hand drifts silently, because nothing about adding a module makes anyone + reopen it. + """ + + TABLE = re.compile( + r"\| File under `src/sites/` \| Responsibility \|(?P.+?)
", + re.S, + ) + + def setUp(self) -> None: + self.text = README.read_text(encoding="utf-8") + self.modules = sorted( + path.name + for path in (REPO_ROOT / "src" / "sites").glob("*.py") + if path.name != "__init__.py" + ) + + def test_the_table_is_where_this_gate_thinks_it_is(self) -> None: + self.assertIsNotNone(self.TABLE.search(self.text)) + self.assertGreater(len(self.modules), 20) + + def test_every_module_appears_in_the_map(self) -> None: + body = self.TABLE.search(self.text).group("body") + named = set(re.findall(r"`([a-z_0-9]+\.py)`", body)) + self.assertEqual( + sorted(set(self.modules) - named), + [], + "modules with no row in the README map", + ) + self.assertEqual( + sorted(named - set(self.modules)), + [], + "rows naming files that no longer exist", + ) + + def test_the_mixin_sentence_names_every_endpoint_mixin(self) -> None: + """Derived from the classes, not from a number someone typed.""" + mixins = sorted( + path.stem.removeprefix("api_") + for path in (REPO_ROOT / "src" / "sites").glob("api_*.py") + if re.search(r"^class \w+Mixin", path.read_text(encoding="utf-8"), re.M) + ) + self.assertIn("mcp", mixins) + spelled = { + 8: "eight", 9: "nine", 10: "ten", 11: "eleven", 12: "twelve", + }.get(len(mixins)) + self.assertIsNotNone(spelled, f"add {len(mixins)} to the spelled-out numbers") + self.assertIn(f"combines {spelled} endpoint mixins", self.text) + for name in mixins: + with self.subTest(mixin=name): + self.assertRegex(self.text, rf"endpoint mixins \([^)]*\b{name}\b") + + +class EnvironmentVariableDiscoverabilityTests(unittest.TestCase): + """Every knob the code reads has to be named somewhere a reader looks. + + README carried "45 `SITES_*` environment variables ... appear in no document + or chart" long after the number stopped being true: measured on the same + definition the sentence uses, it was 8. A count nobody recomputes is a claim, + and this one drifted the safe direction -- overstating a gap that had mostly + been closed -- which is why nothing surfaced it. + """ + + READ = re.compile( + r"(?:getenv|environ\.get|environ)\(\s*[\"'](SITES_[A-Z0-9_]+)[\"']" + ) + + @staticmethod + def _named_in(paths) -> set[str]: + found: set[str] = set() + for path in paths: + if path.is_file(): + found |= set( + re.findall(r"SITES_[A-Z0-9_]+", path.read_text(encoding="utf-8")) + ) + return found + + def _read_by_source(self) -> set[str]: + names: set[str] = set() + for path in (REPO_ROOT / "src" / "sites").glob("*.py"): + names |= set(self.READ.findall(path.read_text(encoding="utf-8"))) + return names + + def test_the_scan_finds_the_variables_it_is_meant_to(self) -> None: + """Guard the denominator: an empty read set would pass silently.""" + read = self._read_by_source() + self.assertGreater(len(read), 50, "the getenv scan matched almost nothing") + for known in ("SITES_HOST_PORT_BASE", "SITES_EXPOSURE_BACKEND"): + self.assertIn(known, read) + + def test_every_variable_the_code_reads_is_named_somewhere(self) -> None: + documented = self._named_in( + list((REPO_ROOT / "docs").glob("*.md")) + [README] + ) + charted = self._named_in((REPO_ROOT / "charts").rglob("*")) + missing = sorted(self._read_by_source() - documented - charted) + self.assertEqual( + missing, + [], + "read by src/sites/ but named in no document and no chart template; " + "add a row to docs/CONFIGURATION.md#undocumented-tuning-variables " + f"or set it in the chart: {missing}", + ) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_scale_to_zero.py b/tests/test_scale_to_zero.py index 4d3ec4f..cd4f403 100644 --- a/tests/test_scale_to_zero.py +++ b/tests/test_scale_to_zero.py @@ -1120,6 +1120,97 @@ def test_genuine_stall_without_scale_event_still_fails(self) -> None: self.assertEqual(status["phase"], "Failed") self.assertIn("not ready within", status["message"]) + def test_a_stall_that_creates_no_pod_still_names_its_cause(self) -> None: + """The one stall with nothing to inspect is the one that must be reported. + + A namespace ResourceQuota refuses the pod at the ReplicaSet, so there is + no container state to read and the Available condition says only + "Deployment does not have minimum availability". Observed live: the fifth + site in a tenant hit `limits.cpu=4` and the caller was told, 120 seconds + after a 200, that the workload was not ready -- while the sentence naming + the exhausted quota sat unread on the Deployment the operator already had. + """ + from sites.operator import _rollout_stall_reason + + class _NoPods: + def get(self, path: str): + return {"items": []} + + quota = ( + 'pods "demo-6cf4c7f4df-gjhfn" is forbidden: exceeded quota: ' + "sites-tenant-quota, requested: limits.cpu=1, used: limits.cpu=4, " + "limited: limits.cpu=4" + ) + deployment = { + "status": { + "conditions": [ + { + "type": "Available", + "status": "False", + "message": "Deployment does not have minimum availability.", + }, + { + "type": "ReplicaFailure", + "status": "True", + "reason": "FailedCreate", + "message": quota, + }, + ] + } + } + reason = _rollout_stall_reason(_NoPods(), deployment, "ns-a", "demo") + self.assertIn("exceeded quota", reason) + self.assertIn("limits.cpu=4", reason) + # The generic sentence adds nothing once the specific one is present. + self.assertNotIn("minimum availability", reason) + + # Without a ReplicaFailure the previous behaviour is unchanged. + deployment["status"]["conditions"] = [ + { + "type": "Available", + "status": "False", + "message": "Deployment does not have minimum availability.", + } + ] + self.assertIn( + "minimum availability", + _rollout_stall_reason(_NoPods(), deployment, "ns-a", "demo"), + ) + + def test_the_two_tenant_limits_are_documented_with_their_real_numbers(self) -> None: + """Whatever the two limits are, the document must state both. + + `maxDeployments` refuses at submission; the namespace ResourceQuota + refuses at rollout. Deriving one from the other would be a product + decision, so this only pins that the smaller one is written down with the + number it actually is -- a reader who sees 10 and no second number plans + for 10 sites and discovers the fourth is the last. + """ + from sites.k8s_resources import TENANT_CPU_LIMIT + from sites.validation import DEFAULT_MAX_DEPLOYMENTS + + spec = normalize_deploy_payload(payload(), DEFAULT_MERCHANT_ID, "local") + containers = deployment_resource(spec, "ns-a")["spec"]["template"]["spec"][ + "containers" + ] + self.assertTrue(containers) + per_site_cpu = { + container["resources"]["limits"]["cpu"] for container in containers + } + self.assertEqual(per_site_cpu, {"1"}, "one CPU per site is what makes the two comparable") + concurrent = int(TENANT_CPU_LIMIT) + self.assertLess( + concurrent, + DEFAULT_MAX_DEPLOYMENTS, + "if these ever agree, drop the limitation instead of updating it", + ) + readme = (pathlib.Path(__file__).resolve().parent.parent / "README.md").read_text( + encoding="utf-8" + ) + self.assertIn("Two tenant limits disagree", readme) + self.assertIn(f"run **{concurrent}** sites at once", readme) + self.assertIn(f"`maxDeployments` (default {DEFAULT_MAX_DEPLOYMENTS})", readme) + def test_new_revision_is_verified_before_keda_is_enabled(self) -> None: with using_backend("gateway"): spec = normalize_deploy_payload( diff --git a/tests/test_sites.py b/tests/test_sites.py index 7db923f..07b67ef 100644 --- a/tests/test_sites.py +++ b/tests/test_sites.py @@ -22,6 +22,7 @@ import yaml from tests import chart +from tests.test_exposure import REFERENCE_HOST_PORT_BASE, using_host_port_base from tests.test_support import postgres_connection, postgres_store from sites import exposure as _exposure @@ -69,6 +70,7 @@ DEFAULT_MAX_DEPLOYMENTS, DEFAULT_MAX_PUBLIC_ROUTES, DEFAULT_MERCHANT_ID, + dns_label, INLINE_ARTIFACT_MAX_FILES, INLINE_ARTIFACT_MAX_TOTAL_BYTES, MAX_ENV_VARS, @@ -1230,6 +1232,206 @@ def test_bundle_response_reports_the_submitted_name(self) -> None: _bundle_response("demo-stack", objects)["phase"], "Deploying" ) + def test_shipped_bundle_examples_are_accepted_by_the_real_validator(self) -> None: + """Every file README points at must survive the endpoint's own checks. + + Nothing submitted these, so both drifted out of the contract they + illustrate and stayed in the tree: one carried a flat single-deployment + shape the bundle endpoint refuses outright, the other a `command` field + that is not a deployment field. A reader following README's "examples in + deploy-specs/" got a 400 from the first command they ran. + + This goes through the same functions /v1/bundles does -- the top-level + field check and bundle_resources -- rather than a copy of their rules, + so an example cannot pass here and fail there. + """ + specs = sorted( + (Path(__file__).resolve().parent.parent / "deploy-specs").glob("*.json") + ) + self.assertTrue(specs, "deploy-specs/ has no examples to check") + for spec_path in specs: + with self.subTest(example=spec_path.name): + payload = json.loads(spec_path.read_text(encoding="utf-8")) + self.assertEqual( + set(payload) - {"name", "components"}, + set(), + "the bundle endpoint refuses unknown top-level fields", + ) + objects = bundle_resources( + dns_label(str(payload["name"])), + payload.get("components"), + DEFAULT_MERCHANT_ID, + "local", + ) + self.assertEqual(len(objects), len(payload["components"])) + + def test_shipped_bundle_examples_do_not_claim_each_other_names(self) -> None: + # Component names are unique per tenant, so two examples sharing one + # would make whichever is submitted second fail with a 409 that reads + # like a bug in the reader's own cluster. + seen: dict[str, str] = {} + for spec_path in sorted( + (Path(__file__).resolve().parent.parent / "deploy-specs").glob("*.json") + ): + payload = json.loads(spec_path.read_text(encoding="utf-8")) + for component in payload["components"]: + name = component["name"] + self.assertNotIn( + name, + seen, + f"{spec_path.name} reuses component {name!r} from {seen.get(name)}", + ) + seen[name] = spec_path.name + + def test_evidence_is_collected_from_health_path_and_says_so(self) -> None: + """The probe address must be the documented one, and documented as bounded. + + The contract tells a caller to compare the public URL's response digest + with `bodySha256`, which only holds while `healthPath` is `/`. Measured: + a source build (whose `--health-path` defaults to `/healthz`) came up + `Running`, `ready`, `verification.ok=true`, `httpStatus=200`, with a + public URL that answered 403 — the probe had asked `/healthz` and the + person opens `/`. + + Both halves are pinned because either alone is useless: the URL builder + without the caveat is an undocumented trap, and the caveat without the + builder is a claim about code that may have moved on. + """ + source = ( + Path(__file__).resolve().parent.parent / "src" / "sites" / "operator.py" + ).read_text(encoding="utf-8") + self.assertIn('f"{spec[\'healthPath\']}"', source) + contract = ( + Path(__file__).resolve().parent.parent / "docs" / "AGENT_CONTRACT.md" + ).read_text(encoding="utf-8") + self.assertIn("Evidence covers `healthPath`, not the whole site", contract) + self.assertIn("`sites build submit` defaults to\n`/healthz`", contract) + + # The trial prints "public URL body: matches verification digest" and + # hard-fails when they differ, which is only reachable because it takes + # the default health path. Passing one would break the trial itself. + quickstart = ( + Path(__file__).resolve().parent.parent + / "scripts" + / "quickstart-kubeadm.sh" + ).read_text(encoding="utf-8") + self.assertIn("public URL body digest", quickstart) + self.assertNotIn("--health-path", quickstart) + + def test_stale_evidence_stays_and_is_distinguishable_by_revision(self) -> None: + """A failed rollout keeps the previous revision's passing evidence. + + That retention is deliberate -- it is still true that the earlier + revision served traffic -- but it means a response can read + `phase: Failed` and `verification.ok: true` at the same time. Observed + live: a dynamic site's version 2 never became ready, and `sites status` + answered Failed while carrying version 1's 200 and body digest. + + The only thing separating the two is the revision each names, so both + must be in the same response and the contract must say to compare them. + Documenting the rule without the fields, or shipping the fields without + the rule, leaves the caller reading `ok` alone. + """ + obj = { + "metadata": {"name": "local-local-demo-0", "generation": 2}, + "spec": { + "merchantID": DEFAULT_MERCHANT_ID, + "userID": "local", + "serviceName": "demo", + "image": "example.invalid/demo@sha256:" + "b" * 64, + "port": 8080, + "revision": "200", + "exposure": "public", + }, + "status": { + "phase": "Failed", + "observedGeneration": 2, + "message": "Deployment was not ready within 120s", + "verification": { + "ok": True, + "httpStatus": 200, + "bodySha256": "c" * 64, + "revision": "100", + }, + }, + } + response = _deployment_response(obj) + self.assertEqual(response["phase"], "Failed") + self.assertTrue(response["verification"]["ok"]) + self.assertEqual(response["revision"], "200") + self.assertEqual(response["verification"]["revision"], "100") + self.assertNotEqual( + response["revision"], response["verification"]["revision"] + ) + + contract = ( + Path(__file__).resolve().parent.parent / "docs" / "AGENT_CONTRACT.md" + ).read_text(encoding="utf-8") + self.assertIn("verification.revision == revision", contract) + self.assertIn("Evidence belongs to the revision it names", contract) + + def test_bundle_components_carry_the_verification_evidence(self) -> None: + """Each component must surface the evidence the operator wrote for it. + + docs/AGENT_CONTRACT.md makes status.verification the only success + criterion, and a bundle is one of its deployment entry points. The + operator collects it per component, so dropping it from this projection + left a bundle caller with phase Running and nothing to check -- the + evidence existed on the resource the whole time and was simply + unreachable through the API. + """ + evidence = { + "ok": True, + "httpStatus": 200, + "bodySha256": "a" * 64, + "revision": "7", + } + objects = bundle_resources( + "demo-stack", + self._sample_components(), + DEFAULT_MERCHANT_ID, + "local", + ) + for resource in objects: + resource["metadata"]["generation"] = 2 + resource["status"] = { + "phase": "Running", + "observedGeneration": 2, + "verification": evidence, + } + response = _bundle_response("demo-stack", objects) + self.assertTrue(response["components"]) + for component in response["components"]: + self.assertEqual(component["verification"], evidence) + + def test_bundle_components_carry_the_reason_they_failed(self) -> None: + # A component that never rolled out reported `Failed` and nothing else, + # so the caller had no way to tell an exhausted tenant quota from an + # image that will not pull. The operator writes the reason onto the + # resource either way; only the projection dropped it. + objects = bundle_resources( + "demo-stack", + self._sample_components(), + DEFAULT_MERCHANT_ID, + "local", + ) + reason = ( + "Deployment was not ready within 120s (exceeded quota: " + "sites-tenant-quota, requested: limits.cpu=1, used: limits.cpu=4)" + ) + for resource in objects: + resource["metadata"]["generation"] = 1 + resource["status"] = { + "phase": "Failed", + "observedGeneration": 1, + "message": reason, + } + response = _bundle_response("demo-stack", objects) + self.assertEqual(response["phase"], "Failed") + self.assertTrue(response["components"]) + for component in response["components"]: + self.assertEqual(component["message"], reason) + def _crd_spec_properties() -> set[str]: """Read the SiteDeployment spec fields the CRD actually declares. @@ -1588,13 +1790,40 @@ def test_pool_ports_have_unique_host_mappings(self) -> None: def test_the_mapping_matches_the_url_formula(self) -> None: # The only source of truth for URL formulas is in sites/exposure.py - just take the public_url from the backend # Reconciling port_forwards, what is nailed is the real invariant of "exposure constant ↔ host mapping". + # The base is declared here rather than defaulted: this pins the trial's + # own topology, and every other environment gets no URL at all until it + # declares the forward it actually publishes. mappings = self._mappings() - for port in NODE_PORT_RANGE: - self.assertEqual( - _exposure.NodePortExposure().public_url({"nodePort": port}), - f"http://127.0.0.1:{mappings[port]}", - f"nodePort {port} The URL does not match the portForward mapping", - ) + with using_host_port_base(REFERENCE_HOST_PORT_BASE): + for port in NODE_PORT_RANGE: + self.assertEqual( + _exposure.NodePortExposure().public_url({"nodePort": port}), + f"http://127.0.0.1:{mappings[port]}", + f"nodePort {port} The URL does not match the portForward mapping", + ) + + def test_the_reference_base_is_what_the_trial_declares_to_the_chart(self) -> None: + """The trial script must hand the Chart the base its Lima forwards use. + + Two spellings of one fact: dev/kubeadm/lima.yaml opens the host ports and + scripts/quickstart-kubeadm.sh tells the control plane which base to build + URLs from. Drift between them is invisible -- the deployment is healthy, + verification passes on the in-cluster address -- until somebody opens the + URL. + """ + script = ( + Path(__file__).resolve().parent.parent + / "scripts" + / "quickstart-kubeadm.sh" + ).read_text(encoding="utf-8") + self.assertIn( + f"host_port_base=${{SITES_QUICKSTART_HOST_PORT_BASE:-{REFERENCE_HOST_PORT_BASE}}}", + script, + ) + self.assertIn('--set "nodePort.hostPortBase=$host_port_base"', script) + self.assertEqual( + int(REFERENCE_HOST_PORT_BASE), self._mappings()[min(NODE_PORT_RANGE)] + ) def test_there_is_exactly_one_source_for_the_pod_cidr(self) -> None: # 🔴 Successor to test_cluster_cidrs_match_the_profile, which asserted