From 3b391a7edf0a4e4b2b2a6f84f9edc290fdf9159e Mon Sep 17 00:00:00 2001 From: mashkovd Date: Sun, 4 Oct 2026 18:20:14 +0200 Subject: [PATCH 1/9] feat(workflows): reserve image names; build only names a team owns Tenant builds push, and tenant services run, ghcr.io/mctlhq/, so the component name is the image name, and nothing tied it to the team asking. check-image-name.sh decides from a reviewed list (config/image-names.txt: reserved platform names, a mctl- prefix, and explicit grants for the services that use them today) plus the services/ tree: a name another team already has is not available. It runs in deploy-service validation, in a new image-name step that gates preview-deploy's build and deploy, and in build-image.yaml before any credential is fetched, where a direct dispatch must also push to exactly ghcr.io/mctlhq/. Direct dispatches no longer move :latest, which nothing pulls; release builds keep it. "platform", the team the platform's own image grants use, is reserved as a tenant name. Claude-Session: https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2 --- .github/workflows/build-image.yaml | 64 +++++- .github/workflows/validate-manifests.yml | 11 + .../tpl-validate-tenant.yaml | 47 +++++ .../cluster-templates/wft-create-tenant.yaml | 4 +- .../cluster-templates/wft-preview-deploy.yaml | 17 +- .../argo-workflows/config/image-names.txt | 73 +++++++ .../scripts/check-image-name.sh | 90 ++++++++ tests/test_check_image_name.py | 196 ++++++++++++++++++ 8 files changed, 498 insertions(+), 4 deletions(-) create mode 100644 platform-gitops/argo-workflows/config/image-names.txt create mode 100755 platform-gitops/argo-workflows/scripts/check-image-name.sh create mode 100644 tests/test_check_image_name.py diff --git a/.github/workflows/build-image.yaml b/.github/workflows/build-image.yaml index 1f43a9454..11db0f476 100644 --- a/.github/workflows/build-image.yaml +++ b/.github/workflows/build-image.yaml @@ -13,7 +13,7 @@ on: required: true type: string image_name: - description: "Full image name (e.g. ghcr.io/mctlhq/openclaw)" + description: "Full image name; for a direct dispatch it must be ghcr.io/mctlhq/" required: true type: string image_tag: @@ -100,6 +100,63 @@ jobs: # OIDC JWT for Vault JWT auth (ROADMAP 3.3). No long-lived VAULT_TOKEN. id-token: write steps: + # --- Image name: build only names owned by the requesting team --- + # A direct dispatch is how tenant builds arrive (Argo's deploy-service + # and preview-deploy) and how platform images under platform-gitops/ + # images/ are built by hand. For those the pushed name must be exactly + # ghcr.io/mctlhq/, and that name must be one the team + # may use (platform-gitops/argo-workflows/config/image-names.txt, read + # from main, not from anything the caller chose). Argo's validation + # checks the same rule first; this is the second layer, at the push. + # + # A workflow_call (release-deploy.yaml) is not checked here: release + # images are named by their release and may differ from the component + # name, and release-deploy is dispatched by the release bots and + # operators, never by a tenant workflow. GITHUB_WORKFLOW_REF names the + # caller's workflow in a called run, so it tells the two apart. + - name: Classify caller + id: caller + run: | + case "$GITHUB_WORKFLOW_REF" in + "${GITHUB_REPOSITORY}/.github/workflows/build-image.yaml@"*) direct=true ;; + *) direct=false ;; + esac + echo "direct=${direct}" >> "$GITHUB_OUTPUT" + echo "direct dispatch: ${direct} (${GITHUB_WORKFLOW_REF})" + + - name: Check out image-name policy + if: steps.caller.outputs.direct == 'true' + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + repository: ${{ github.repository }} + ref: main + path: .image-name-policy + persist-credentials: false + sparse-checkout: | + platform-gitops/services + platform-gitops/argo-workflows/config + platform-gitops/argo-workflows/scripts + + - name: Check image name + if: steps.caller.outputs.direct == 'true' + env: + INPUT_IMAGE_NAME: ${{ inputs.image_name }} + INPUT_TEAM_NAME: ${{ inputs.team_name }} + INPUT_COMPONENT_NAME: ${{ inputs.component_name }} + run: | + set -euo pipefail + expected="${REGISTRY}/${GITHUB_REPOSITORY_OWNER}/${INPUT_COMPONENT_NAME}" + if [ "$INPUT_IMAGE_NAME" != "$expected" ]; then + echo "::error::image_name must be ${expected} for a direct dispatch, got: ${INPUT_IMAGE_NAME}" + exit 1 + fi + if ! sh .image-name-policy/platform-gitops/argo-workflows/scripts/check-image-name.sh \ + .image-name-policy "$INPUT_TEAM_NAME" "$INPUT_COMPONENT_NAME"; then + echo "::error::component_name ${INPUT_COMPONENT_NAME} is not available to team ${INPUT_TEAM_NAME}" + exit 1 + fi + rm -rf .image-name-policy + # --- Tier 1: Vault PAT via GitHub OIDC -> Vault JWT --- - name: Fetch Vault PAT id: vault_pat @@ -302,9 +359,12 @@ jobs: # the PAT. secrets: | github_token=${{ contains(fromJSON('["mctlhq/mctl-web","mctlhq/mctl-docs","mctlhq/mctl-academy"]'), inputs.repo) && secrets.GH_PACKAGES_TOKEN || '' }} + # :latest only for release builds. Nothing in this repository or the + # cluster pulls ghcr.io/mctlhq/*:latest, and a direct dispatch moving + # it would repoint the tag at whatever was built last. tags: | ${{ inputs.image_name }}:${{ inputs.image_tag }} - ${{ inputs.image_name }}:latest + ${{ steps.caller.outputs.direct != 'true' && format('{0}:latest', inputs.image_name) || '' }} - name: Summary # Same reason as the Vault step: these land in a shell command, so diff --git a/.github/workflows/validate-manifests.yml b/.github/workflows/validate-manifests.yml index c7c264b03..21a42a0aa 100644 --- a/.github/workflows/validate-manifests.yml +++ b/.github/workflows/validate-manifests.yml @@ -418,6 +418,17 @@ jobs: # nobody watches, so it is tested here instead. run: python3 tests/test_rotate_github_token_scope.py + - name: Unit-test the image-name ownership check + # Tenant builds push, and tenant services run, ghcr.io/mctlhq/. check-image-name.sh decides which team may use a name; this + # runs it against fixtures and this tree, keeps image-names.txt + # complete (every platform image reserved, every existing service + # passing), and checks that validate, preview-deploy and build-image + # call it before anything is built or pushed. + run: | + python3 -m pip install --quiet pyyaml + python3 tests/test_check_image_name.py + - name: Unit-test tpl-git-commit's yq handling # The static check above proves no yq expression interpolates a shell # variable. This proves the shape that replaced it actually resists diff --git a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml index 2c617cbbc..0beac2e35 100644 --- a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml @@ -6,6 +6,9 @@ metadata: workflows.argoproj.io/description: | Reusable template: validates tenant and service naming conventions, checks team exists in Git tenants/ directory. + `image-name` checks only that a team may use a component name as its + image name (config/image-names.txt), for workflows that build without + running `validate`. spec: templates: - name: validate @@ -111,6 +114,16 @@ spec: git clone --depth=1 "git@github.com:${GITOPS_ORG}/${GITOPS_REPO}.git" /tmp/mctl-gitops cd /tmp/mctl-gitops + # ── 3b. Image name: the component name is the image name ─────────── + # Services run, and builds push, ghcr.io//, so + # a team may only use a name it owns. Rule and reviewed grants: + # config/image-names.txt. Any non-zero exit (refused, or could not + # decide) stops here, for every action. + if ! sh platform-gitops/argo-workflows/scripts/check-image-name.sh . "$TEAM" "$SERVICE"; then + echo "❌ component_name '${SERVICE}' is not available to team '${TEAM}'" + exit 1 + fi + # ── 4. Verify service exists for deploy/update-config ──────────────── if [ "$ACTION" = "deploy" ] || [ "$ACTION" = "update-config" ]; then if [ ! -d "platform-gitops/services/${TEAM}/${SERVICE}" ]; then @@ -163,3 +176,37 @@ spec: - name: deploy-key secret: secretName: mctl-gitops-deploy-key + + # Image-name check alone, for preview-deploy (which builds without + # running `validate`). mctl-gitops is public, so an anonymous HTTPS clone + # of main is enough: no deploy key in this pod. Parameters arrive as env, + # never interpolated into the script text. + - name: image-name + inputs: + parameters: + - name: team_name + - name: component_name + script: + image: alpine/git:2.43.0 + command: [sh] + envFrom: + - configMapRef: + name: mctl-platform-config + env: + - name: PARAM_TEAM_NAME + value: "{{inputs.parameters.team_name}}" + - name: PARAM_COMPONENT_NAME + value: "{{inputs.parameters.component_name}}" + source: | + set -e + git clone --depth=1 "https://github.com/${GITOPS_ORG}/${GITOPS_REPO}.git" /tmp/mctl-gitops + if ! sh /tmp/mctl-gitops/platform-gitops/argo-workflows/scripts/check-image-name.sh \ + /tmp/mctl-gitops "$PARAM_TEAM_NAME" "$PARAM_COMPONENT_NAME"; then + echo "❌ component_name '${PARAM_COMPONENT_NAME}' is not available to team '${PARAM_TEAM_NAME}'" + exit 1 + fi + echo "✅ image name ${PARAM_COMPONENT_NAME} is available to ${PARAM_TEAM_NAME}" + resources: + requests: + memory: 32Mi + cpu: 10m diff --git a/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml b/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml index e189fcc44..90d6227bd 100644 --- a/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml @@ -145,7 +145,9 @@ spec: fi # Reserved names - for reserved in argocd argo-workflows argo-events kube-system kube-public kube-node-lease default cert-manager traefik vault external-secrets monitoring; do + # platform: the team name image-names.txt grants the platform's own + # images to; a tenant of that name would inherit those grants. + for reserved in argocd argo-workflows argo-events kube-system kube-public kube-node-lease default cert-manager traefik vault external-secrets monitoring platform; do if [ "$TENANT" = "$reserved" ]; then echo "ERROR: '$TENANT' is a reserved namespace name." exit 1 diff --git a/platform-gitops/argo-workflows/cluster-templates/wft-preview-deploy.yaml b/platform-gitops/argo-workflows/cluster-templates/wft-preview-deploy.yaml index 3c03e1e77..35584aed5 100644 --- a/platform-gitops/argo-workflows/cluster-templates/wft-preview-deploy.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/wft-preview-deploy.yaml @@ -56,8 +56,23 @@ spec: - name: preview-pipeline dag: tasks: + # The component name is the image name: the build pushes + # ghcr.io//. Checked before anything is built. + - name: image-name + templateRef: + name: tpl-validate-tenant + clusterScope: true + template: image-name + arguments: + parameters: + - name: team_name + value: "{{workflow.parameters.team_name}}" + - name: component_name + value: "{{workflow.parameters.component_name}}" + - name: build-image template: trigger-github-build + dependencies: [image-name] when: "\"{{workflow.parameters.git_ref}}\" != \"\"" arguments: parameters: @@ -76,7 +91,7 @@ spec: - name: deploy-preview template: deploy - dependencies: [build-image] + dependencies: [image-name, build-image] arguments: parameters: - name: team_name diff --git a/platform-gitops/argo-workflows/config/image-names.txt b/platform-gitops/argo-workflows/config/image-names.txt new file mode 100644 index 000000000..0560e1f6e --- /dev/null +++ b/platform-gitops/argo-workflows/config/image-names.txt @@ -0,0 +1,73 @@ +# Image names under ghcr.io/mctlhq that a team may build into and run. +# +# Tenant builds push ghcr.io/mctlhq/, and tenant services run +# ghcr.io/mctlhq/, so the component name IS the image name. +# platform-gitops/argo-workflows/scripts/check-image-name.sh decides, for a +# team T and a name N, in this order: +# +# 1. `grant T N` or `grant * N` below -> allowed +# 2. `reserved N`, or N starts with a -> refused +# `reserved-prefix` +# 3. another team already has services//N, or a values.yaml under +# services// runs ghcr.io/mctlhq/N -> refused +# 4. otherwise -> allowed +# +# Read by the deploy-service and preview-deploy validation (Argo) and by +# .github/workflows/build-image.yaml before it pushes. A line this script does +# not understand makes every check refuse, so keep to the three forms below. +# tests/test_check_image_name.py keeps this file complete: every image the +# platform references outside platform-gitops/services/ must be reserved here, +# and every service that exists today must pass. + +# --- Platform images: built by release-deploy or by hand, never by a tenant. +reserved-prefix mctl- +reserved grafana-iac +reserved mc +reserved newton-mcp-gateway +reserved openclaw-whisper-builder +reserved pfeifenpatenschaft-backend +reserved portfolio +reserved projects-mcp +reserved seerrsense +reserved vault-iac +reserved zitadel-iac + +# --- Reviewed grants. Each one is an existing service whose name is reserved +# --- above or shared between teams; add a line here only in review. + +# Platform images built from platform-gitops/images/ and dispatched by hand +# with team "platform" (not a tenant; wft-create-tenant refuses the name). +grant platform grafana-iac +grant platform mc +grant platform mctl-agent +grant platform vault-iac +grant platform zitadel-iac + +# admins: the platform's own services. +grant admins mctl-agents-worker +grant admins mctl-agents-worker-exec +grant admins mctl-agents-worker-implement +grant admins mctl-design +grant admins mctl-docs +grant admins mctl-web + +# labs: platform-owned products deployed as labs services. +grant labs mctl-academy +grant labs mctl-alice +grant labs mctl-loyalty +grant labs mctl-pairdesk +grant labs mctl-telegram +grant labs mctl-telegram-preview +grant labs pfeifenpatenschaft-backend +grant labs portfolio +grant labs projects-mcp +grant labs seerrsense + +# ovk: runs mctl-academy from the same source repository as labs. +grant ovk mctl-academy + +# openclaw services run the fixed image mctl-openclaw (service-templates/ +# openclaw), never an image named after the component, and their build input is pinned +# to mctlhq/mctl-openclaw by mctl-api. The name is shared by every team that +# deploys one. +grant * openclaw diff --git a/platform-gitops/argo-workflows/scripts/check-image-name.sh b/platform-gitops/argo-workflows/scripts/check-image-name.sh new file mode 100755 index 000000000..07c8c66da --- /dev/null +++ b/platform-gitops/argo-workflows/scripts/check-image-name.sh @@ -0,0 +1,90 @@ +#!/bin/sh +# Usage: check-image-name.sh +# +# Decides whether may build into and run ghcr.io/mctlhq/. +# The rule and the data live in platform-gitops/argo-workflows/config/ +# image-names.txt; this script only applies them to the checkout it is given. +# +# Exit 0: allowed. Exit 1: refused. Exit 2: could not decide (bad input, no +# readable list, or a list line it does not understand). Callers treat any +# non-zero exit as a refusal. +# +# POSIX sh plus grep/awk only: it runs in alpine/git (busybox) inside Argo and +# in bash on GitHub runners. +set -u + +if [ "$#" -ne 3 ]; then + echo "usage: $0 " >&2 + exit 2 +fi +ROOT=$1 +TEAM=$2 +NAME=$3 +LIST="$ROOT/platform-gitops/argo-workflows/config/image-names.txt" +SERVICES="$ROOT/platform-gitops/services" + +for pair in "team:$TEAM" "component_name:$NAME"; do + value=${pair#*:} + # A newline would let grep match one good line of a multi-line value. + if [ "$(printf '%s' "$value" | wc -l)" -ne 0 ] \ + || ! printf '%s' "$value" | grep -qE '^[a-z0-9]([a-z0-9-]*[a-z0-9])?$'; then + echo "invalid ${pair%%:*}: ${value}" >&2 + exit 2 + fi +done + +if [ ! -r "$LIST" ] || [ ! -d "$SERVICES" ]; then + echo "cannot read ${LIST} or ${SERVICES}" >&2 + exit 2 +fi + +# Every non-comment line must be one of the three known forms. +if ! awk ' + /^[[:space:]]*(#|$)/ { next } + $1 == "grant" && NF == 3 { next } + ($1 == "reserved" || $1 == "reserved-prefix") && NF == 2 { next } + { bad = 1; print "unrecognised line " NR ": " $0 > "/dev/stderr" } + END { exit bad } +' "$LIST"; then + exit 2 +fi + +if awk -v t="$TEAM" -v n="$NAME" ' + $1 == "grant" && ($2 == t || $2 == "*") && $3 == n { found = 1 } + END { exit !found } +' "$LIST"; then + exit 0 +fi + +if awk -v n="$NAME" ' + $1 == "reserved" && $2 == n { found = 1 } + $1 == "reserved-prefix" && index(n, $2) == 1 { found = 1 } + END { exit !found } +' "$LIST"; then + echo "image name '${NAME}' is reserved for the platform; team '${TEAM}' may not use it" >&2 + exit 1 +fi + +for dir in "$SERVICES"/*/"$NAME"; do + [ -d "$dir" ] || continue + owner=$(basename "$(dirname "$dir")") + if [ "$owner" != "$TEAM" ]; then + echo "image name '${NAME}' belongs to team '${owner}'; team '${TEAM}' may not use it" >&2 + exit 1 + fi +done + +# A service may run an image under a name other than its own directory +# (e.g. a preview of another service): those names are taken as well. +for values in "$SERVICES"/*/*/values.yaml; do + [ -f "$values" ] || continue + if grep -qE "^[[:space:]]*repository:[[:space:]]*[\"']?ghcr\.io/mctlhq/${NAME}[\"']?[[:space:]]*$" "$values"; then + owner=$(basename "$(dirname "$(dirname "$values")")") + if [ "$owner" != "$TEAM" ]; then + echo "image name '${NAME}' is run by team '${owner}'; team '${TEAM}' may not use it" >&2 + exit 1 + fi + fi +done + +exit 0 diff --git a/tests/test_check_image_name.py b/tests/test_check_image_name.py new file mode 100644 index 000000000..d1e7a9d31 --- /dev/null +++ b/tests/test_check_image_name.py @@ -0,0 +1,196 @@ +"""Image names: a team builds into and runs only names it owns. + +Tenant builds push ghcr.io/mctlhq/ and tenant services run the +same name, so the component name is the image name. check-image-name.sh +decides whether a team may use one, from config/image-names.txt and the +services/ tree. This runs that script: + + * against fixtures, for each branch of the rule and for the fail-closed + paths (bad input, missing or malformed list); + * against this repository, so that every service that exists today still + passes and every image the platform references outside services/ is + reserved; + +and checks that the three callers (deploy-service validation, preview-deploy, +build-image.yaml) actually run it before anything is built or pushed. +""" +import pathlib +import re +import subprocess +import sys +import tempfile + +import yaml + +REPO = pathlib.Path(__file__).resolve().parent.parent +SCRIPT = REPO / "platform-gitops/argo-workflows/scripts/check-image-name.sh" +LIST = REPO / "platform-gitops/argo-workflows/config/image-names.txt" + +failures = [] + + +def check(cond, msg): + if not cond: + failures.append(msg) + + +def run(root, team, name): + return subprocess.run(["sh", str(SCRIPT), str(root), team, name], + capture_output=True, text=True).returncode + + +def fixture(list_text, services): + """services: {"team/name": values.yaml text or None}""" + d = pathlib.Path(tempfile.mkdtemp()) + cfg = d / "platform-gitops/argo-workflows/config" + cfg.mkdir(parents=True) + if list_text is not None: + (cfg / "image-names.txt").write_text(list_text) + svc = d / "platform-gitops/services" + svc.mkdir(parents=True) + for path, values in services.items(): + p = svc / path + p.mkdir(parents=True) + if values is not None: + (p / "values.yaml").write_text(values) + return d + + +LIST_TEXT = """# comment +reserved-prefix mctl- +reserved grafana-iac +grant platform grafana-iac +grant labs mctl-academy +grant ovk mctl-academy +grant * openclaw +""" +SERVICES = { + "labs/kuptsi-app": "image:\n repository: ghcr.io/mctlhq/kuptsi-app\n", + "labs/telegram-preview": "image:\n repository: \"ghcr.io/mctlhq/bot-image\"\n", + "labs/mctl-academy": None, + # onboarded, values not written yet: the directory alone claims the name + "labs/fresh-svc": None, + "ovk/mctl-academy": None, + "admins/openclaw": "image:\n repository: ghcr.io/mctlhq/mctl-openclaw\n", +} +root = fixture(LIST_TEXT, SERVICES) + +CASES = [ + # (team, name, expected exit, why) + ("karabu", "brand-new", 0, "an unused name is free"), + ("labs", "kuptsi-app", 0, "a team keeps its own service name"), + ("karabu", "kuptsi-app", 1, "another team's service name"), + ("karabu", "fresh-svc", 1, "another team's service directory, no values yet"), + ("labs", "fresh-svc", 0, "the owning team, no values yet"), + ("karabu", "bot-image", 1, "an image another team's service runs"), + ("labs", "bot-image", 0, "the team running that image may build it"), + ("karabu", "mctl-api", 1, "reserved prefix"), + ("karabu", "grafana-iac", 1, "reserved name"), + ("platform", "grafana-iac", 0, "explicit grant"), + ("labs", "mctl-academy", 0, "explicit grant"), + ("ovk", "mctl-academy", 0, "explicit grant for a second team"), + ("karabu", "mctl-academy", 1, "a grant is for its team only"), + ("karabu", "openclaw", 0, "wildcard grant"), + ("Karabu", "x", 2, "team outside the name grammar"), + ("karabu", "x/../y", 2, "name outside the name grammar"), + ("karabu", "ok\nmctl-api", 2, "multi-line name"), + ("karabu", "", 2, "empty name"), +] +for team, name, want, why in CASES: + got = run(root, team, name) + check(got == want, f"fixture {team}/{name!r}: exit {got}, want {want} ({why})") + +# Fail closed: no list, a list line the script does not understand. +check(run(fixture(None, SERVICES), "karabu", "brand-new") == 2, + "missing image-names.txt must refuse (exit 2)") +check(run(fixture(LIST_TEXT + "allow karabu mctl-api\n", SERVICES), + "karabu", "brand-new") == 2, + "an unrecognised list line must refuse every check (exit 2)") +check(run(fixture(LIST_TEXT + "grant karabu\n", SERVICES), + "karabu", "brand-new") == 2, + "a grant with a missing field must refuse every check (exit 2)") + +# This repository: every existing service passes. +for d in sorted((REPO / "platform-gitops/services").glob("*/*/")): + team, name = d.parent.name, d.name + got = run(REPO, team, name) + check(got == 0, f"existing service {team}/{name} is refused (exit {got}); " + "add a reviewed grant to image-names.txt") + +# This repository: every image the platform references outside services/ is +# reserved, so no tenant can take its name. +entries = [ln.split() for ln in LIST.read_text().splitlines() + if ln.strip() and not ln.lstrip().startswith("#")] +reserved = {e[1] for e in entries if e[0] == "reserved"} +prefixes = [e[1] for e in entries if e[0] == "reserved-prefix"] +SKIP = ("platform-gitops/services/", "platform-gitops/agents-state/", + "platform-gitops/platform-skills/") +files = subprocess.check_output( + ["git", "ls-files", "platform-gitops", ".github/workflows"], + cwd=REPO, text=True).split() +referenced = {} +for f in files: + if f.startswith(SKIP) or f.endswith(".md"): + continue + try: + text = (REPO / f).read_text() + except (UnicodeDecodeError, IsADirectoryError): + continue + for m in re.finditer(r"ghcr\.io/mctlhq/([a-z0-9][a-z0-9._-]*[a-z0-9])", text): + referenced.setdefault(m.group(1), f) + for m in re.finditer(r"registry:\s*[\"']?ghcr\.io[\"']?\s*\n\s*repository:\s*" + r"[\"']?mctlhq/([a-z0-9][a-z0-9._-]*[a-z0-9])", text): + referenced.setdefault(m.group(1), f) +check("mctl-portal" in referenced and "mctl-api" in referenced, + "the reference scan no longer finds mctl-api/mctl-portal; fix the scan") +for name, f in sorted(referenced.items()): + check(name in reserved or any(name.startswith(p) for p in prefixes), + f"ghcr.io/mctlhq/{name} (referenced in {f}) is not reserved in image-names.txt") + +# The callers run the check before anything is built or pushed. +tpl = yaml.safe_load((REPO / "platform-gitops/argo-workflows/cluster-templates/" + "tpl-validate-tenant.yaml").read_text()) +by_name = {t["name"]: t for t in tpl["spec"]["templates"]} +src = by_name["validate"]["script"]["source"] +call = 'check-image-name.sh . "$TEAM" "$SERVICE"' +check(call in src, "validate does not run check-image-name.sh") +check(call in src and src.index(call) < src.index("# ── 4."), + "validate must check the image name before the action-specific checks") +check("check-image-name.sh" in by_name.get("image-name", {}).get("script", {}).get("source", ""), + "tpl-validate-tenant has no image-name template running the script") + +pv = yaml.safe_load((REPO / "platform-gitops/argo-workflows/cluster-templates/" + "wft-preview-deploy.yaml").read_text()) +tasks = {t["name"]: t for t in + [t for t in pv["spec"]["templates"] if t["name"] == "preview-pipeline"][0]["dag"]["tasks"]} +check(tasks.get("image-name", {}).get("templateRef", {}).get("template") == "image-name", + "preview-deploy does not run the image-name check") +check("image-name" in tasks["build-image"].get("dependencies", []), + "preview-deploy's build must depend on the image-name check") +check("image-name" in tasks["deploy-preview"].get("dependencies", []), + "preview-deploy's deploy must depend on the image-name check") + +bi = yaml.safe_load((REPO / ".github/workflows/build-image.yaml").read_text()) +steps = bi["jobs"]["build"]["steps"] +names = [s.get("name", "") for s in steps] +check("Check image name" in names, "build-image.yaml has no image-name check") +if "Check image name" in names: + idx = names.index("Check image name") + for later in ("Fetch Vault PAT", "Log in to GHCR", "Build and push"): + check(later in names and idx < names.index(later), + f"build-image.yaml must check the image name before '{later}'") + run_src = steps[idx]["run"] + check("check-image-name.sh" in run_src, "build-image's check does not run the script") + check('"$INPUT_IMAGE_NAME" != "$expected"' in run_src, + "build-image must pin image_name to ghcr.io//") +push = [s for s in steps if s.get("name") == "Build and push"][0]["with"]["tags"] +latest = [ln for ln in push.splitlines() if "latest" in ln] +check(len(latest) == 1 and "steps.caller.outputs.direct != 'true'" in latest[0], + "build-image must push :latest only for release (workflow_call) builds") + +if failures: + print("\n".join(f"FAIL: {f}" for f in failures)) + sys.exit(1) +print(f"OK: {len(CASES) + 3} fixture cases, " + f"{len(list((REPO / 'platform-gitops/services').glob('*/*/')))} existing services, " + f"{len(referenced)} referenced platform images") From be70817e4fcaec039a81cbd2eda87d38f4542bba Mon Sep 17 00:00:00 2001 From: mashkovd Date: Sun, 4 Oct 2026 18:47:28 +0200 Subject: [PATCH 2/9] fix(workflows): bind image-name checks to real tenants and registry Tenant workflows now run the check with --tenant: the team must be an existing tenant (tenants//) and never "platform", so the grants for hand-dispatched platform images are unreachable from deploy-service and preview-deploy. "admins" is reserved in create-tenant as well. The registry is an argument and must equal the list's registry line, so a caller deriving another organisation is refused instead of being checked against names it does not use. The wildcard grant is gone: openclaw is a shared service name whose image only labs may build, and deploy-service no longer builds for the openclaw template, whose services run the fixed mctl-openclaw image. Build-time callers pass --build, so a shared name can be run but not pushed to by any team. Also: limits on the check pods, the push-time step names its known gaps, and the onboard race is noted where the check runs. Claude-Session: https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2 --- .github/workflows/build-image.yaml | 37 ++++-- .../tpl-validate-tenant.yaml | 53 +++++++-- .../cluster-templates/wft-create-tenant.yaml | 9 +- .../cluster-templates/wft-deploy-service.yaml | 12 +- .../cluster-templates/wft-preview-deploy.yaml | 5 + .../argo-workflows/config/image-names.txt | 47 +++++--- .../scripts/check-image-name.sh | 98 ++++++++++++--- tests/test_check_image_name.py | 112 +++++++++++++++--- 8 files changed, 296 insertions(+), 77 deletions(-) diff --git a/.github/workflows/build-image.yaml b/.github/workflows/build-image.yaml index 11db0f476..91cfb417f 100644 --- a/.github/workflows/build-image.yaml +++ b/.github/workflows/build-image.yaml @@ -102,18 +102,28 @@ jobs: steps: # --- Image name: build only names owned by the requesting team --- # A direct dispatch is how tenant builds arrive (Argo's deploy-service - # and preview-deploy) and how platform images under platform-gitops/ - # images/ are built by hand. For those the pushed name must be exactly - # ghcr.io/mctlhq/, and that name must be one the team - # may use (platform-gitops/argo-workflows/config/image-names.txt, read - # from main, not from anything the caller chose). Argo's validation - # checks the same rule first; this is the second layer, at the push. + # and preview-deploy), how mctl-openclaw's upstream sync builds the + # openclaw image (team labs), and how platform images under + # platform-gitops/images/ are built by hand (team platform). Every + # other dispatcher in the organisation calls release-deploy.yaml. For a + # direct dispatch the pushed name must be exactly + # /, and that name must be one the team may + # build (platform-gitops/argo-workflows/config/image-names.txt, read + # from main, not from anything the caller chose). # - # A workflow_call (release-deploy.yaml) is not checked here: release - # images are named by their release and may differ from the component - # name, and release-deploy is dispatched by the release bots and - # operators, never by a tenant workflow. GITHUB_WORKFLOW_REF names the - # caller's workflow in a called run, so it tells the two apart. + # Argo's validation (tpl-validate-tenant) is the boundary for tenants: + # it also binds team_name to an existing tenant, which this workflow + # cannot, because it never sees who asked. This step is a backstop at + # the push, for whoever holds actions:write on this repository, and it + # has two known gaps that only such a holder can use: + # * release-deploy.yaml takes a free-text image_name and calls this + # workflow (workflow_call), which is not checked here: release + # images are named by their release and may differ from the + # component name (mctl-telegram-preview pushes mctl-telegram). + # * a dispatch at a ref other than main runs that ref's copy of this + # file. The policy data is read from main either way. + # GITHUB_WORKFLOW_REF names the caller's workflow in a called run, so + # it tells a direct dispatch from a workflow_call. - name: Classify caller id: caller run: | @@ -150,8 +160,9 @@ jobs: echo "::error::image_name must be ${expected} for a direct dispatch, got: ${INPUT_IMAGE_NAME}" exit 1 fi - if ! sh .image-name-policy/platform-gitops/argo-workflows/scripts/check-image-name.sh \ - .image-name-policy "$INPUT_TEAM_NAME" "$INPUT_COMPONENT_NAME"; then + if ! sh .image-name-policy/platform-gitops/argo-workflows/scripts/check-image-name.sh --build \ + .image-name-policy "${REGISTRY}/${GITHUB_REPOSITORY_OWNER}" \ + "$INPUT_TEAM_NAME" "$INPUT_COMPONENT_NAME"; then echo "::error::component_name ${INPUT_COMPONENT_NAME} is not available to team ${INPUT_TEAM_NAME}" exit 1 fi diff --git a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml index 0beac2e35..2cfc9951f 100644 --- a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml @@ -34,6 +34,10 @@ spec: - name: service_template default: "default" # when not "default", dockerfile_repo is not required + - name: container_registry + default: "ghcr.io/mctlhq" + # where deploy-service pushes and services pull; must be the + # registry config/image-names.txt governs outputs: parameters: - name: service_dir @@ -51,6 +55,9 @@ spec: envFrom: - configMapRef: name: mctl-platform-config + env: + - name: PARAM_CONTAINER_REGISTRY + value: "{{inputs.parameters.container_registry}}" source: | set -e @@ -115,11 +122,25 @@ spec: cd /tmp/mctl-gitops # ── 3b. Image name: the component name is the image name ─────────── - # Services run, and builds push, ghcr.io//, so - # a team may only use a name it owns. Rule and reviewed grants: - # config/image-names.txt. Any non-zero exit (refused, or could not - # decide) stops here, for every action. - if ! sh platform-gitops/argo-workflows/scripts/check-image-name.sh . "$TEAM" "$SERVICE"; then + # Services run, and builds push, /, so a + # team may only use a name it owns. Rule and reviewed grants: + # config/image-names.txt. --tenant: TEAM is requester-supplied here, + # so it must be an existing tenant (never "platform"). --build when + # deploy-service will build (its build-image `when`, kept in step + # by tests/test_check_image_name.py). Any non-zero exit (refused, + # or could not decide) stops here, for every action. + # + # This is check-then-act: two teams onboarding the same free name + # at the same moment can both pass. The build-image.yaml check at + # the push sees the first team's services/ commit only once it has + # landed; closing the window fully needs the claim and the check + # in one commit. + BUILD_FLAG="" + if [ "$ACTION" != "update-config" ] && [ -n "$REPO" ] && [ "$SERVICE_TEMPLATE" != "openclaw" ]; then + BUILD_FLAG="--build" + fi + if ! sh platform-gitops/argo-workflows/scripts/check-image-name.sh --tenant $BUILD_FLAG \ + . "$PARAM_CONTAINER_REGISTRY" "$TEAM" "$SERVICE"; then echo "❌ component_name '${SERVICE}' is not available to team '${TEAM}'" exit 1 fi @@ -169,6 +190,13 @@ spec: echo "latest" > /tmp/image_tag fi echo "🏷 Image tag: $(cat /tmp/image_tag)" + resources: + requests: + memory: 32Mi + cpu: 10m + limits: + memory: 128Mi + cpu: 200m volumeMounts: - name: deploy-key mountPath: /secrets/deploy-key @@ -186,6 +214,9 @@ spec: parameters: - name: team_name - name: component_name + - name: git_ref + default: "" + # non-empty: preview-deploy will build, so the check is --build script: image: alpine/git:2.43.0 command: [sh] @@ -197,11 +228,16 @@ spec: value: "{{inputs.parameters.team_name}}" - name: PARAM_COMPONENT_NAME value: "{{inputs.parameters.component_name}}" + - name: PARAM_GIT_REF + value: "{{inputs.parameters.git_ref}}" source: | set -e git clone --depth=1 "https://github.com/${GITOPS_ORG}/${GITOPS_REPO}.git" /tmp/mctl-gitops - if ! sh /tmp/mctl-gitops/platform-gitops/argo-workflows/scripts/check-image-name.sh \ - /tmp/mctl-gitops "$PARAM_TEAM_NAME" "$PARAM_COMPONENT_NAME"; then + BUILD_FLAG="" + [ -n "$PARAM_GIT_REF" ] && BUILD_FLAG="--build" + # preview-deploy builds ghcr.io//. + if ! sh /tmp/mctl-gitops/platform-gitops/argo-workflows/scripts/check-image-name.sh --tenant $BUILD_FLAG \ + /tmp/mctl-gitops "ghcr.io/${GITOPS_ORG}" "$PARAM_TEAM_NAME" "$PARAM_COMPONENT_NAME"; then echo "❌ component_name '${PARAM_COMPONENT_NAME}' is not available to team '${PARAM_TEAM_NAME}'" exit 1 fi @@ -210,3 +246,6 @@ spec: requests: memory: 32Mi cpu: 10m + limits: + memory: 128Mi + cpu: 200m diff --git a/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml b/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml index 90d6227bd..6e0459962 100644 --- a/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml @@ -145,9 +145,12 @@ spec: fi # Reserved names - # platform: the team name image-names.txt grants the platform's own - # images to; a tenant of that name would inherit those grants. - for reserved in argocd argo-workflows argo-events kube-system kube-public kube-node-lease default cert-manager traefik vault external-secrets monitoring platform; do + # platform, admins: the teams image-names.txt grants the platform's + # own images to. "platform" is never a tenant; "admins" exists and + # must not be created again by someone else (this workflow continues + # on an existing tenant directory); wft-delete-tenant-safe likewise + # refuses admins. + for reserved in argocd argo-workflows argo-events kube-system kube-public kube-node-lease default cert-manager traefik vault external-secrets monitoring platform admins; do if [ "$TENANT" = "$reserved" ]; then echo "ERROR: '$TENANT' is a reserved namespace name." exit 1 diff --git a/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml b/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml index a3d9772e5..40901dd62 100644 --- a/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml @@ -128,11 +128,17 @@ spec: value: "{{workflow.parameters.image_tag}}" - name: service_template value: "{{workflow.parameters.service_template}}" - - # Step 1b: build Docker image via GitHub Actions (skip for update-config) + - name: container_registry + value: "{{workflow.parameters.container_registry}}" + + # Step 1b: build Docker image via GitHub Actions (skip for + # update-config, and for the openclaw template: its services run + # the fixed image mctl-openclaw, so a build of + # / would push an image nothing runs). + # tpl-validate-tenant's --build decision mirrors this condition. - name: build-image dependencies: [validate] - when: "\"{{workflow.parameters.action}}\" != \"update-config\" && \"{{workflow.parameters.dockerfile_repo}}\" != \"\"" + when: "\"{{workflow.parameters.action}}\" != \"update-config\" && \"{{workflow.parameters.dockerfile_repo}}\" != \"\" && \"{{workflow.parameters.service_template}}\" != \"openclaw\"" template: trigger-github-build arguments: parameters: diff --git a/platform-gitops/argo-workflows/cluster-templates/wft-preview-deploy.yaml b/platform-gitops/argo-workflows/cluster-templates/wft-preview-deploy.yaml index 35584aed5..824282aa9 100644 --- a/platform-gitops/argo-workflows/cluster-templates/wft-preview-deploy.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/wft-preview-deploy.yaml @@ -69,6 +69,8 @@ spec: value: "{{workflow.parameters.team_name}}" - name: component_name value: "{{workflow.parameters.component_name}}" + - name: git_ref + value: "{{workflow.parameters.git_ref}}" - name: build-image template: trigger-github-build @@ -91,6 +93,9 @@ spec: - name: deploy-preview template: deploy + # `dependencies` (not `depends`) treats a Skipped build — no + # git_ref, deploying an existing image_tag — as satisfied, as it + # did before this task existed; a Failed image-name stops it. dependencies: [image-name, build-image] arguments: parameters: diff --git a/platform-gitops/argo-workflows/config/image-names.txt b/platform-gitops/argo-workflows/config/image-names.txt index 0560e1f6e..f0eed170b 100644 --- a/platform-gitops/argo-workflows/config/image-names.txt +++ b/platform-gitops/argo-workflows/config/image-names.txt @@ -1,24 +1,30 @@ -# Image names under ghcr.io/mctlhq that a team may build into and run. +# Image names under the registry below that a team may build into and run. # -# Tenant builds push ghcr.io/mctlhq/, and tenant services run -# ghcr.io/mctlhq/, so the component name IS the image name. +# Tenant builds push /, and tenant services run +# /, so the component name IS the image name. # platform-gitops/argo-workflows/scripts/check-image-name.sh decides, for a # team T and a name N, in this order: # -# 1. `grant T N` or `grant * N` below -> allowed -# 2. `reserved N`, or N starts with a -> refused -# `reserved-prefix` -# 3. another team already has services//N, or a values.yaml under -# services// runs ghcr.io/mctlhq/N -> refused -# 4. otherwise -> allowed +# 0. tenant callers (Argo) only: T must be an existing tenant +# (platform-gitops/tenants/T/) and not "platform" else refused +# 1. `grant T N` -> allowed +# 2. `reserved N`, or N starts with a `reserved-prefix` -> refused +# 3. `shared N`: allowed to run; refused to build (only a granted team +# may push to a shared name) +# 4. another team already has services//N, or a values.yaml under +# services// runs /N -> refused +# 5. otherwise -> allowed # # Read by the deploy-service and preview-deploy validation (Argo) and by -# .github/workflows/build-image.yaml before it pushes. A line this script does -# not understand makes every check refuse, so keep to the three forms below. +# .github/workflows/build-image.yaml before it pushes. A line the script does +# not understand, a wildcard grant, or a registry other than the caller's +# makes every check refuse, so keep to the forms below. # tests/test_check_image_name.py keeps this file complete: every image the # platform references outside platform-gitops/services/ must be reserved here, # and every service that exists today must pass. +registry ghcr.io/mctlhq + # --- Platform images: built by release-deploy or by hand, never by a tenant. reserved-prefix mctl- reserved grafana-iac @@ -36,7 +42,10 @@ reserved zitadel-iac # --- above or shared between teams; add a line here only in review. # Platform images built from platform-gitops/images/ and dispatched by hand -# with team "platform" (not a tenant; wft-create-tenant refuses the name). +# to build-image.yaml with team "platform". Unreachable from tenant workflows: +# their check runs with --tenant, which refuses "platform" and any team +# without platform-gitops/tenants// (and wft-create-tenant refuses to +# create a tenant of that name). grant platform grafana-iac grant platform mc grant platform mctl-agent @@ -66,8 +75,12 @@ grant labs seerrsense # ovk: runs mctl-academy from the same source repository as labs. grant ovk mctl-academy -# openclaw services run the fixed image mctl-openclaw (service-templates/ -# openclaw), never an image named after the component, and their build input is pinned -# to mctlhq/mctl-openclaw by mctl-api. The name is shared by every team that -# deploys one. -grant * openclaw +# openclaw: several teams hold a service of that name (labs, ovk, admins); +# each runs the fixed image mctl-openclaw (service-templates/openclaw), which +# the mctl- prefix reserves, and deploy-service does not build for that +# template. The image named openclaw itself is built only by mctl-openclaw's +# upstream-sync workflow, dispatched as team labs; nothing in this repository +# runs it. Shared, so the service name stays available to every team, with +# builds limited to the grant. +shared openclaw +grant labs openclaw diff --git a/platform-gitops/argo-workflows/scripts/check-image-name.sh b/platform-gitops/argo-workflows/scripts/check-image-name.sh index 07c8c66da..e32a526d3 100755 --- a/platform-gitops/argo-workflows/scripts/check-image-name.sh +++ b/platform-gitops/argo-workflows/scripts/check-image-name.sh @@ -1,9 +1,19 @@ #!/bin/sh -# Usage: check-image-name.sh +# Usage: check-image-name.sh [--tenant] [--build] # -# Decides whether may build into and run ghcr.io/mctlhq/. -# The rule and the data live in platform-gitops/argo-workflows/config/ -# image-names.txt; this script only applies them to the checkout it is given. +# Decides whether may run (and, with --build, build into) +# /. The rule and the data live in +# platform-gitops/argo-workflows/config/image-names.txt; this script only +# applies them to the checkout it is given. +# +# --tenant the caller is a tenant workflow (Argo), where is a value +# the requester supplied: it must be an existing tenant +# (platform-gitops/tenants//), and never "platform". +# --build the caller is about to push to the name, not only run it. +# the registry prefix the caller pushes to or pulls from; it +# must equal the list's `registry` line, so a caller deriving a +# different organisation is refused rather than checked against +# names it does not use. # # Exit 0: allowed. Exit 1: refused. Exit 2: could not decide (bad input, no # readable list, or a list line it does not understand). Callers treat any @@ -13,15 +23,27 @@ # in bash on GitHub runners. set -u -if [ "$#" -ne 3 ]; then - echo "usage: $0 " >&2 +TENANT=false +BUILD=false +while [ "$#" -gt 0 ]; do + case "$1" in + --tenant) TENANT=true; shift ;; + --build) BUILD=true; shift ;; + --*) echo "unknown option: $1" >&2; exit 2 ;; + *) break ;; + esac +done +if [ "$#" -ne 4 ]; then + echo "usage: $0 [--tenant] [--build] " >&2 exit 2 fi ROOT=$1 -TEAM=$2 -NAME=$3 +REGISTRY=$2 +TEAM=$3 +NAME=$4 LIST="$ROOT/platform-gitops/argo-workflows/config/image-names.txt" SERVICES="$ROOT/platform-gitops/services" +TENANTS="$ROOT/platform-gitops/tenants" for pair in "team:$TEAM" "component_name:$NAME"; do value=${pair#*:} @@ -38,26 +60,52 @@ if [ ! -r "$LIST" ] || [ ! -d "$SERVICES" ]; then exit 2 fi -# Every non-comment line must be one of the three known forms. +# Every non-comment line must be one of the known forms, and there must be +# exactly one registry line. if ! awk ' /^[[:space:]]*(#|$)/ { next } - $1 == "grant" && NF == 3 { next } - ($1 == "reserved" || $1 == "reserved-prefix") && NF == 2 { next } + $1 == "grant" && NF == 3 && $2 != "*" { next } + ($1 == "reserved" || $1 == "reserved-prefix" || $1 == "shared") && NF == 2 { next } + $1 == "registry" && NF == 2 { registries++; next } { bad = 1; print "unrecognised line " NR ": " $0 > "/dev/stderr" } - END { exit bad } + END { + if (registries != 1) { bad = 1; print "need exactly one registry line" > "/dev/stderr" } + exit bad + } ' "$LIST"; then exit 2 fi -if awk -v t="$TEAM" -v n="$NAME" ' - $1 == "grant" && ($2 == t || $2 == "*") && $3 == n { found = 1 } - END { exit !found } -' "$LIST"; then +LIST_REGISTRY=$(awk '$1 == "registry" { print $2 }' "$LIST") +if [ "$REGISTRY" != "$LIST_REGISTRY" ]; then + echo "registry '${REGISTRY}' is not the one image-names.txt governs ('${LIST_REGISTRY}')" >&2 + exit 2 +fi + +if [ "$TENANT" = true ]; then + # The grants to team "platform" are for builds dispatched by hand; a tenant + # workflow can never act as that team, nor as one that does not exist. + if [ "$TEAM" = platform ] || [ ! -d "$TENANTS/$TEAM" ]; then + echo "team '${TEAM}' is not an existing tenant" >&2 + exit 1 + fi +fi + +has() { # has [] -- is there a line " " (or " ") + awk -v k="$1" -v t="${2-}" -v n="$NAME" ' + $1 == k && NF == 2 && t == "" && $2 == n { found = 1 } + $1 == k && NF == 3 && t != "" && $2 == t && $3 == n { found = 1 } + END { exit !found } + ' "$LIST" +} + +# 1. An explicit grant to this team. +if has grant "$TEAM"; then exit 0 fi -if awk -v n="$NAME" ' - $1 == "reserved" && $2 == n { found = 1 } +# 2. Reserved for the platform. +if has reserved || awk -v n="$NAME" ' $1 == "reserved-prefix" && index(n, $2) == 1 { found = 1 } END { exit !found } ' "$LIST"; then @@ -65,6 +113,17 @@ if awk -v n="$NAME" ' exit 1 fi +# 3. A shared name: several teams may hold a service by that name, but only a +# granted team may push to the image. +if has shared; then + if [ "$BUILD" = true ]; then + echo "image name '${NAME}' is shared; only a granted team may build it, not '${TEAM}'" >&2 + exit 1 + fi + exit 0 +fi + +# 4. A name another team already has. for dir in "$SERVICES"/*/"$NAME"; do [ -d "$dir" ] || continue owner=$(basename "$(dirname "$dir")") @@ -76,9 +135,10 @@ done # A service may run an image under a name other than its own directory # (e.g. a preview of another service): those names are taken as well. +REGISTRY_RE=$(printf '%s' "$REGISTRY" | sed 's/[.]/\\./g') for values in "$SERVICES"/*/*/values.yaml; do [ -f "$values" ] || continue - if grep -qE "^[[:space:]]*repository:[[:space:]]*[\"']?ghcr\.io/mctlhq/${NAME}[\"']?[[:space:]]*$" "$values"; then + if grep -qE "^[[:space:]]*repository:[[:space:]]*[\"']?${REGISTRY_RE}/${NAME}[\"']?[[:space:]]*$" "$values"; then owner=$(basename "$(dirname "$(dirname "$values")")") if [ "$owner" != "$TEAM" ]; then echo "image name '${NAME}' is run by team '${owner}'; team '${TEAM}' may not use it" >&2 diff --git a/tests/test_check_image_name.py b/tests/test_check_image_name.py index d1e7a9d31..1a03a5ad5 100644 --- a/tests/test_check_image_name.py +++ b/tests/test_check_image_name.py @@ -34,14 +34,19 @@ def check(cond, msg): failures.append(msg) -def run(root, team, name): - return subprocess.run(["sh", str(SCRIPT), str(root), team, name], +REG = "ghcr.io/mctlhq" + + +def run(root, team, name, *flags, registry=REG): + return subprocess.run(["sh", str(SCRIPT), *flags, str(root), registry, team, name], capture_output=True, text=True).returncode -def fixture(list_text, services): +def fixture(list_text, services, tenants=("labs", "ovk", "karabu", "admins")): """services: {"team/name": values.yaml text or None}""" d = pathlib.Path(tempfile.mkdtemp()) + for t in tenants: + (d / "platform-gitops/tenants" / t).mkdir(parents=True) cfg = d / "platform-gitops/argo-workflows/config" cfg.mkdir(parents=True) if list_text is not None: @@ -57,12 +62,14 @@ def fixture(list_text, services): LIST_TEXT = """# comment +registry ghcr.io/mctlhq reserved-prefix mctl- reserved grafana-iac grant platform grafana-iac grant labs mctl-academy grant ovk mctl-academy -grant * openclaw +shared openclaw +grant labs openclaw """ SERVICES = { "labs/kuptsi-app": "image:\n repository: ghcr.io/mctlhq/kuptsi-app\n", @@ -72,11 +79,12 @@ def fixture(list_text, services): "labs/fresh-svc": None, "ovk/mctl-academy": None, "admins/openclaw": "image:\n repository: ghcr.io/mctlhq/mctl-openclaw\n", + "labs/openclaw": None, } root = fixture(LIST_TEXT, SERVICES) CASES = [ - # (team, name, expected exit, why) + # (team, name, expected exit, why[, flags]) ("karabu", "brand-new", 0, "an unused name is free"), ("labs", "kuptsi-app", 0, "a team keeps its own service name"), ("karabu", "kuptsi-app", 1, "another team's service name"), @@ -90,15 +98,39 @@ def fixture(list_text, services): ("labs", "mctl-academy", 0, "explicit grant"), ("ovk", "mctl-academy", 0, "explicit grant for a second team"), ("karabu", "mctl-academy", 1, "a grant is for its team only"), - ("karabu", "openclaw", 0, "wildcard grant"), + ("karabu", "openclaw", 0, "a shared name may be run by any team"), + ("karabu", "openclaw", 1, "only a granted team may build a shared name", "--build"), + ("labs", "openclaw", 0, "the granted team may build a shared name", "--build"), + ("karabu", "brand-new", 0, "build mode does not change a free name", "--build"), + ("karabu", "brand-new", 0, "an existing tenant", "--tenant"), + ("platform", "grafana-iac", 1, "a tenant workflow can never act as team platform", "--tenant"), + ("platform", "brand-new", 1, "nor use team platform for anything", "--tenant"), + ("nosuch", "brand-new", 1, "a tenant workflow's team must exist", "--tenant"), + ("labs", "mctl-academy", 0, "a grant still applies to a real tenant", "--tenant", "--build"), ("Karabu", "x", 2, "team outside the name grammar"), ("karabu", "x/../y", 2, "name outside the name grammar"), ("karabu", "ok\nmctl-api", 2, "multi-line name"), ("karabu", "", 2, "empty name"), ] -for team, name, want, why in CASES: - got = run(root, team, name) - check(got == want, f"fixture {team}/{name!r}: exit {got}, want {want} ({why})") +for team, name, want, why, *flags in CASES: + got = run(root, team, name, *flags) + check(got == want, f"fixture {flags} {team}/{name!r}: exit {got}, want {want} ({why})") + +# The registry is part of the decision: a caller deriving another one is +# refused, not checked against names it does not use. +check(run(root, "karabu", "kuptsi-app", registry="ghcr.io/otherorg") == 2, + "a registry other than the list's must refuse (exit 2)") +check(run(fixture(LIST_TEXT.replace("registry ghcr.io/mctlhq\n", ""), SERVICES), + "karabu", "brand-new") == 2, + "a list without a registry line must refuse (exit 2)") +check(run(fixture(LIST_TEXT + "grant * mctl-api\n", SERVICES), "karabu", "brand-new") == 2, + "a wildcard grant must make the list unusable (exit 2)") +check(run(root, "karabu", "brand-new", "--bogus") == 2, "unknown option must refuse (exit 2)") +# Even if a tenants/platform directory appeared, a tenant workflow may not act +# as team platform. +check(run(fixture(LIST_TEXT, SERVICES, tenants=("platform",)), "platform", "grafana-iac", + "--tenant") == 1, + "--tenant must refuse team platform even with a tenants/platform directory") # Fail closed: no list, a list line the script does not understand. check(run(fixture(None, SERVICES), "karabu", "brand-new") == 2, @@ -113,7 +145,8 @@ def fixture(list_text, services): # This repository: every existing service passes. for d in sorted((REPO / "platform-gitops/services").glob("*/*/")): team, name = d.parent.name, d.name - got = run(REPO, team, name) + flags = ["--tenant"] if (REPO / "platform-gitops/tenants" / team).is_dir() else [] + got = run(REPO, team, name, *flags) check(got == 0, f"existing service {team}/{name} is refused (exit {got}); " "add a reviewed grant to image-names.txt") @@ -152,12 +185,58 @@ def fixture(list_text, services): "tpl-validate-tenant.yaml").read_text()) by_name = {t["name"]: t for t in tpl["spec"]["templates"]} src = by_name["validate"]["script"]["source"] -call = 'check-image-name.sh . "$TEAM" "$SERVICE"' -check(call in src, "validate does not run check-image-name.sh") +call = ('\nif ! sh platform-gitops/argo-workflows/scripts/check-image-name.sh --tenant $BUILD_FLAG \\\n' + ' . "$PARAM_CONTAINER_REGISTRY" "$TEAM" "$SERVICE"') +check(call in src, "validate does not run check-image-name.sh --tenant with its registry") check(call in src and src.index(call) < src.index("# ── 4."), "validate must check the image name before the action-specific checks") -check("check-image-name.sh" in by_name.get("image-name", {}).get("script", {}).get("source", ""), - "tpl-validate-tenant has no image-name template running the script") +build_cond = ('if [ "$ACTION" != "update-config" ] && [ -n "$REPO" ] && ' + '[ "$SERVICE_TEMPLATE" != "openclaw" ]; then\n BUILD_FLAG="--build"') +check(build_cond in src, "validate must pass --build exactly when deploy-service builds") +env = {e["name"]: e["value"] for e in by_name["validate"]["script"].get("env", [])} +check(env.get("PARAM_CONTAINER_REGISTRY") == "{{inputs.parameters.container_registry}}", + "validate must take the registry as a bound env value") +img = by_name.get("image-name", {}).get("script", {}).get("source", "") +check('\nif ! sh /tmp/mctl-gitops/platform-gitops/argo-workflows/scripts/check-image-name.sh --tenant $BUILD_FLAG' in img and '"ghcr.io/${GITOPS_ORG}"' in img, + "image-name template must run the script with --tenant and the org's registry") +check('[ -n "$PARAM_GIT_REF" ] && BUILD_FLAG="--build"' in img, + "image-name template must check in build mode when preview-deploy builds") +for t in ("validate", "image-name"): + lim = by_name.get(t, {}).get("script", {}).get("resources", {}).get("limits", {}) + check(lim.get("memory") and lim.get("cpu"), f"{t} must declare resource limits") + +ds = yaml.safe_load((REPO / "platform-gitops/argo-workflows/cluster-templates/" + "wft-deploy-service.yaml").read_text()) +dtasks = {t["name"]: t for t in + [t for t in ds["spec"]["templates"] if t["name"] == "deploy-pipeline"][0]["dag"]["tasks"]} +vparams = {p["name"]: p["value"] for p in dtasks["validate"]["arguments"]["parameters"]} +check(vparams.get("container_registry") == "{{workflow.parameters.container_registry}}", + "deploy-service must hand validate the registry it builds into") +when = dtasks["build-image"]["when"] +for part in ('"{{workflow.parameters.action}}\" != \"update-config\"', + '"{{workflow.parameters.dockerfile_repo}}\" != \"\"', + '"{{workflow.parameters.service_template}}\" != \"openclaw\"'): + check(part.replace('\\"', '"') in when.replace('\\"', '"'), + f"deploy-service build condition changed; keep validate's --build in step ({part})") + +ct = (REPO / "platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml").read_text() +reserved_line = [ln for ln in ct.splitlines() if "for reserved in" in ln][0] +reserved_names = reserved_line.split("for reserved in", 1)[1].split(";")[0].split() +for t in ("platform", "admins"): + check(t in reserved_names, + f"wft-create-tenant must reserve the tenant name '{t}' (image-names.txt grants to it)") + +# Direct dispatchers of build-image.yaml outside Argo, as found on each +# repository's default branch (all other organisation workflows call +# release-deploy.yaml): each must keep passing the push-time check. +for team, name in (("labs", "openclaw"), # mctl-openclaw upstream-sync-release.yml + ("platform", "grafana-iac"), # platform-gitops/images/*, by hand + ("platform", "mc"), + ("platform", "vault-iac"), + ("platform", "zitadel-iac"), + ("platform", "mctl-agent")): + check(run(REPO, team, name, "--build") == 0, + f"direct build-image dispatcher {team}/{name} would be refused") pv = yaml.safe_load((REPO / "platform-gitops/argo-workflows/cluster-templates/" "wft-preview-deploy.yaml").read_text()) @@ -183,6 +262,9 @@ def fixture(list_text, services): check("check-image-name.sh" in run_src, "build-image's check does not run the script") check('"$INPUT_IMAGE_NAME" != "$expected"' in run_src, "build-image must pin image_name to ghcr.io//") + check('\nif ! sh .image-name-policy/platform-gitops/argo-workflows/scripts/check-image-name.sh --build' in run_src + and '"${REGISTRY}/${GITHUB_REPOSITORY_OWNER}"' in run_src, + "build-image must check in build mode against its own registry") push = [s for s in steps if s.get("name") == "Build and push"][0]["with"]["tags"] latest = [ln for ln in push.splitlines() if "latest" in ln] check(len(latest) == 1 and "steps.caller.outputs.direct != 'true'" in latest[0], @@ -191,6 +273,6 @@ def fixture(list_text, services): if failures: print("\n".join(f"FAIL: {f}" for f in failures)) sys.exit(1) -print(f"OK: {len(CASES) + 3} fixture cases, " +print(f"OK: {len(CASES) + 7} fixture cases, " f"{len(list((REPO / 'platform-gitops/services').glob('*/*/')))} existing services, " f"{len(referenced)} referenced platform images") From 30feeab490e5792e36ee763f96149f3ac865f290 Mon Sep 17 00:00:00 2001 From: mashkovd Date: Sun, 4 Oct 2026 19:32:08 +0200 Subject: [PATCH 3/9] fix(workflows): decide deploy-service builds from the existing service The build-or-not decision keyed off service_template, an onboard-time parameter, so a deploy of an existing openclaw service that let it default was checked as a build and refused. validate now decides once: never for update-config or without a dockerfile_repo; on onboard by the template; otherwise by the service's own values.yaml, which builds only when it runs / (or names no image). The result is both the --build flag and a `builds` output, and deploy-service's build step runs on that output alone, so the check and the build cannot disagree. Also: - drop `admins` from create-tenant's reserved names: tenants/admins already exists, so the entry blocked nothing but the re-run of the platform's own tenant; `platform` stays. - check-image-name.sh --tenant without platform-gitops/tenants/ is exit 2 (could not decide), not "no such tenant". - tests pin preview-deploy's git_ref handover to image-name, and team_name/component_name from both Argo callers' build tasks into the build-image dispatch payload; the decision is run from the template against fixtures. Claude-Session: https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2 --- .../tpl-validate-tenant.yaml | 37 ++++++- .../cluster-templates/wft-create-tenant.yaml | 10 +- .../cluster-templates/wft-deploy-service.yaml | 12 +-- .../scripts/check-image-name.sh | 5 + tests/test_check_image_name.py | 101 +++++++++++++++--- 5 files changed, 136 insertions(+), 29 deletions(-) diff --git a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml index 2cfc9951f..b2d7aa241 100644 --- a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml @@ -49,6 +49,12 @@ spec: - name: image_tag valueFrom: path: /tmp/image_tag + - name: builds + valueFrom: + path: /tmp/builds + # "true" when deploy-service will build and push + # /; its build-image step runs on this + # alone, so the check below and the build cannot disagree. script: image: alpine/git:2.43.0 command: [sh] @@ -126,8 +132,8 @@ spec: # team may only use a name it owns. Rule and reviewed grants: # config/image-names.txt. --tenant: TEAM is requester-supplied here, # so it must be an existing tenant (never "platform"). --build when - # deploy-service will build (its build-image `when`, kept in step - # by tests/test_check_image_name.py). Any non-zero exit (refused, + # deploy-service will build (the `builds` output, which is the only + # thing its build-image step runs on). Any non-zero exit (refused, # or could not decide) stops here, for every action. # # This is check-then-act: two teams onboarding the same free name @@ -135,10 +141,31 @@ spec: # the push sees the first team's services/ commit only once it has # landed; closing the window fully needs the claim and the check # in one commit. - BUILD_FLAG="" - if [ "$ACTION" != "update-config" ] && [ -n "$REPO" ] && [ "$SERVICE_TEMPLATE" != "openclaw" ]; then - BUILD_FLAG="--build" + # + # Whether it builds: never for update-config or without a + # dockerfile_repo. On onboard the new service's template decides + # (openclaw services run the fixed image mctl-openclaw). On any + # other action the existing service decides: if its values.yaml + # names an image and none of them is /, + # it runs another image, and building this name would push one + # nothing runs. A missing values.yaml or one naming no image + # builds, as before (and deploy then fails at step 6). + BUILDS=false + if [ "$ACTION" != "update-config" ] && [ -n "$REPO" ]; then + VALUES="platform-gitops/services/${TEAM}/${SERVICE}/values.yaml" + REGISTRY_RE=$(printf '%s' "$PARAM_CONTAINER_REGISTRY" | sed 's/[.]/\\./g') + if [ "$ACTION" = "onboard" ]; then + if [ "$SERVICE_TEMPLATE" != "openclaw" ]; then BUILDS=true; fi + elif [ -f "$VALUES" ] && grep -qE '^[[:space:]]*repository:' "$VALUES" \ + && ! grep -qE "^[[:space:]]*repository:[[:space:]]*[\"']?${REGISTRY_RE}/${SERVICE}[\"']?[[:space:]]*\$" "$VALUES"; then + echo "ℹ️ ${TEAM}/${SERVICE} runs another image; not building ${PARAM_CONTAINER_REGISTRY}/${SERVICE}" + else + BUILDS=true + fi fi + echo "$BUILDS" > /tmp/builds + BUILD_FLAG="" + [ "$BUILDS" = true ] && BUILD_FLAG="--build" if ! sh platform-gitops/argo-workflows/scripts/check-image-name.sh --tenant $BUILD_FLAG \ . "$PARAM_CONTAINER_REGISTRY" "$TEAM" "$SERVICE"; then echo "❌ component_name '${SERVICE}' is not available to team '${TEAM}'" diff --git a/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml b/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml index 6e0459962..55e6b4210 100644 --- a/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml @@ -145,12 +145,10 @@ spec: fi # Reserved names - # platform, admins: the teams image-names.txt grants the platform's - # own images to. "platform" is never a tenant; "admins" exists and - # must not be created again by someone else (this workflow continues - # on an existing tenant directory); wft-delete-tenant-safe likewise - # refuses admins. - for reserved in argocd argo-workflows argo-events kube-system kube-public kube-node-lease default cert-manager traefik vault external-secrets monitoring platform admins; do + # platform: the team image-names.txt grants the platform's own + # images to. It is never a tenant, which is what keeps those grants + # unreachable from tenant workflows (check-image-name.sh --tenant). + for reserved in argocd argo-workflows argo-events kube-system kube-public kube-node-lease default cert-manager traefik vault external-secrets monitoring platform; do if [ "$TENANT" = "$reserved" ]; then echo "ERROR: '$TENANT' is a reserved namespace name." exit 1 diff --git a/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml b/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml index 40901dd62..1f4ae7dc7 100644 --- a/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml @@ -131,14 +131,14 @@ spec: - name: container_registry value: "{{workflow.parameters.container_registry}}" - # Step 1b: build Docker image via GitHub Actions (skip for - # update-config, and for the openclaw template: its services run - # the fixed image mctl-openclaw, so a build of - # / would push an image nothing runs). - # tpl-validate-tenant's --build decision mirrors this condition. + # Step 1b: build Docker image via GitHub Actions, when validate + # decided this run builds (not for update-config, not without a + # dockerfile_repo, and not for a service that runs another image, + # such as the openclaw template's mctl-openclaw). validate checked + # the image name with --build on exactly this decision. - name: build-image dependencies: [validate] - when: "\"{{workflow.parameters.action}}\" != \"update-config\" && \"{{workflow.parameters.dockerfile_repo}}\" != \"\" && \"{{workflow.parameters.service_template}}\" != \"openclaw\"" + when: "\"{{tasks.validate.outputs.parameters.builds}}\" == \"true\"" template: trigger-github-build arguments: parameters: diff --git a/platform-gitops/argo-workflows/scripts/check-image-name.sh b/platform-gitops/argo-workflows/scripts/check-image-name.sh index e32a526d3..6b8ff3ee7 100755 --- a/platform-gitops/argo-workflows/scripts/check-image-name.sh +++ b/platform-gitops/argo-workflows/scripts/check-image-name.sh @@ -59,6 +59,11 @@ if [ ! -r "$LIST" ] || [ ! -d "$SERVICES" ]; then echo "cannot read ${LIST} or ${SERVICES}" >&2 exit 2 fi +# Without the tenants tree every team would read as "not a tenant". +if [ "$TENANT" = true ] && [ ! -d "$TENANTS" ]; then + echo "cannot read ${TENANTS}" >&2 + exit 2 +fi # Every non-comment line must be one of the known forms, and there must be # exactly one registry line. diff --git a/tests/test_check_image_name.py b/tests/test_check_image_name.py index 1a03a5ad5..db93cf4cc 100644 --- a/tests/test_check_image_name.py +++ b/tests/test_check_image_name.py @@ -132,6 +132,13 @@ def fixture(list_text, services, tenants=("labs", "ovk", "karabu", "admins")): "--tenant") == 1, "--tenant must refuse team platform even with a tenants/platform directory") +# A --tenant check without the tenants tree cannot decide; it is not "no +# such tenant". Without --tenant the tree is not read. +check(run(fixture(LIST_TEXT, SERVICES, tenants=()), "karabu", "brand-new", "--tenant") == 2, + "--tenant without platform-gitops/tenants/ must be exit 2 (could not decide)") +check(run(fixture(LIST_TEXT, SERVICES, tenants=()), "karabu", "brand-new") == 0, + "without --tenant the tenants tree is not needed") + # Fail closed: no list, a list line the script does not understand. check(run(fixture(None, SERVICES), "karabu", "brand-new") == 2, "missing image-names.txt must refuse (exit 2)") @@ -190,9 +197,54 @@ def fixture(list_text, services, tenants=("labs", "ovk", "karabu", "admins")): check(call in src, "validate does not run check-image-name.sh --tenant with its registry") check(call in src and src.index(call) < src.index("# ── 4."), "validate must check the image name before the action-specific checks") -build_cond = ('if [ "$ACTION" != "update-config" ] && [ -n "$REPO" ] && ' - '[ "$SERVICE_TEMPLATE" != "openclaw" ]; then\n BUILD_FLAG="--build"') -check(build_cond in src, "validate must pass --build exactly when deploy-service builds") +# The build decision: extracted from validate and run against a fixture, so +# it is the template's own block that is tested. Its result is both the +# --build flag and the `builds` output deploy-service's build runs on. +m = re.search(r'^(BUILDS=false\n.*?\necho "\$BUILDS" > /tmp/builds)$', src, re.S | re.M) +check(m is not None, "could not extract validate's build decision") +check('\n[ "$BUILDS" = true ] && BUILD_FLAG="--build"' in src, + "validate must pass --build exactly when it decided to build") +outs = {o["name"]: o for o in by_name["validate"]["outputs"]["parameters"]} +check(outs.get("builds", {}).get("valueFrom", {}).get("path") == "/tmp/builds", + "validate must output its build decision") +if m: + decide_root = fixture(LIST_TEXT, { + "labs/own": "image:\n repository: ghcr.io/mctlhq/own\n tag: x\n", + "labs/quoted": 'image:\n repository: "ghcr.io/mctlhq/quoted"\n', + "ovk/openclaw": "image:\n repository: ghcr.io/mctlhq/mctl-openclaw\n", + "labs/runs-other": "image:\n repository: ghcr.io/mctlhq/own\n", + "labs/no-image": "replicas: 1\n", + "labs/no-values": None, + }) + + def builds(action, team, service, repo="org/repo", template="default"): + with tempfile.TemporaryDirectory() as out: + block = m.group(1).replace("/tmp/builds", f"{out}/builds") + env = {"PATH": "/usr/bin:/bin", "ACTION": action, "TEAM": team, + "SERVICE": service, "REPO": repo, "SERVICE_TEMPLATE": template, + "PARAM_CONTAINER_REGISTRY": REG} + r = subprocess.run(["sh", "-c", "set -e\n" + block], cwd=decide_root, + env=env, capture_output=True, text=True) + if r.returncode != 0: + return f"error: {r.stderr}" + return (pathlib.Path(out) / "builds").read_text().strip() + + for args, want, why in ( + (("update-config", "labs", "own"), "false", "update-config never builds"), + (("deploy", "labs", "own", ""), "false", "no dockerfile_repo, nothing to build"), + (("onboard", "labs", "brand-new"), "true", "onboard builds the new service"), + (("onboard", "ovk", "claw", "org/repo", "openclaw"), "false", + "the openclaw template runs mctl-openclaw"), + (("deploy", "labs", "own"), "true", "the service runs /"), + (("deploy", "labs", "quoted"), "true", "a quoted repository still matches"), + (("deploy", "ovk", "openclaw"), "false", + "an existing openclaw service runs mctl-openclaw, whatever service_template says"), + (("deploy", "labs", "runs-other"), "false", "the service runs another image"), + (("deploy", "labs", "no-image"), "true", "no image named: builds as before"), + (("deploy", "labs", "no-values"), "true", "no values.yaml: builds (step 6 refuses deploy)"), + ): + got = builds(*args) + check(got == want, f"validate build decision {args}: {got!r}, want {want!r} ({why})") env = {e["name"]: e["value"] for e in by_name["validate"]["script"].get("env", [])} check(env.get("PARAM_CONTAINER_REGISTRY") == "{{inputs.parameters.container_registry}}", "validate must take the registry as a bound env value") @@ -212,19 +264,14 @@ def fixture(list_text, services, tenants=("labs", "ovk", "karabu", "admins")): vparams = {p["name"]: p["value"] for p in dtasks["validate"]["arguments"]["parameters"]} check(vparams.get("container_registry") == "{{workflow.parameters.container_registry}}", "deploy-service must hand validate the registry it builds into") -when = dtasks["build-image"]["when"] -for part in ('"{{workflow.parameters.action}}\" != \"update-config\"', - '"{{workflow.parameters.dockerfile_repo}}\" != \"\"', - '"{{workflow.parameters.service_template}}\" != \"openclaw\"'): - check(part.replace('\\"', '"') in when.replace('\\"', '"'), - f"deploy-service build condition changed; keep validate's --build in step ({part})") +check(dtasks["build-image"]["when"] == '"{{tasks.validate.outputs.parameters.builds}}" == "true"', + "deploy-service must build exactly when validate decided to (and checked with --build)") ct = (REPO / "platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml").read_text() reserved_line = [ln for ln in ct.splitlines() if "for reserved in" in ln][0] reserved_names = reserved_line.split("for reserved in", 1)[1].split(";")[0].split() -for t in ("platform", "admins"): - check(t in reserved_names, - f"wft-create-tenant must reserve the tenant name '{t}' (image-names.txt grants to it)") +check("platform" in reserved_names, + "wft-create-tenant must reserve the tenant name 'platform' (image-names.txt grants to it)") # Direct dispatchers of build-image.yaml outside Argo, as found on each # repository's default branch (all other organisation workflows call @@ -248,6 +295,36 @@ def fixture(list_text, services, tenants=("labs", "ovk", "karabu", "admins")): "preview-deploy's build must depend on the image-name check") check("image-name" in tasks["deploy-preview"].get("dependencies", []), "preview-deploy's deploy must depend on the image-name check") +iparams = {p["name"]: p["value"] for p in tasks["image-name"]["arguments"]["parameters"]} +check(iparams.get("git_ref") == "{{workflow.parameters.git_ref}}", + "preview-deploy must tell image-name whether it will build (git_ref)") + +# build-image.yaml's push-time check pins image_name to +# / and checks team_name, so both Argo callers must +# send team_name and component_name in the dispatch: an empty one would +# refuse every tenant build. Pinned from the task arguments through the +# trigger template's env into the dispatch payload. +for label, doc, pipeline in (("deploy-service", ds, "deploy-pipeline"), + ("preview-deploy", pv, "preview-pipeline")): + ptasks = {t["name"]: t for t in + [t for t in doc["spec"]["templates"] if t["name"] == pipeline][0]["dag"]["tasks"]} + bargs = {p["name"]: p["value"] for p in ptasks["build-image"]["arguments"]["parameters"]} + trig = [t for t in doc["spec"]["templates"] + if t["name"] == ptasks["build-image"]["template"]][0] + tenv = {e["name"]: e.get("value") for e in trig["script"].get("env", [])} + tsrc = trig["script"]["source"] + for p in ("team_name", "component_name"): + check(bargs.get(p) == "{{workflow.parameters.%s}}" % p, + f"{label}'s build-image must pass {p}") + check(tenv.get("PARAM_" + p.upper()) == "{{inputs.parameters.%s}}" % p, + f"{label}'s trigger template must bind {p} from its inputs") + check(re.search(r'\b%s\s*=\s*os\.environ\["PARAM_%s"\]' % (p, p.upper()), tsrc) is not None + and re.search(r'"%s":\s*%s,' % (p, p), tsrc) is not None, + f"{label}'s dispatch payload must carry {p}") +dbargs = {p["name"]: p["value"] for p in dtasks["build-image"]["arguments"]["parameters"]} +check(dbargs.get("image_name") + == "{{workflow.parameters.container_registry}}/{{workflow.parameters.component_name}}", + "deploy-service must build /") bi = yaml.safe_load((REPO / ".github/workflows/build-image.yaml").read_text()) steps = bi["jobs"]["build"]["steps"] From 204c08460298914974f57768913624275c9b0ddc Mon Sep 17 00:00:00 2001 From: mashkovd Date: Sun, 4 Oct 2026 20:43:55 +0200 Subject: [PATCH 4/9] fix(workflows): read values images with one fail-closed reader The build decision and the ownership scan each grepped values.yaml for `repository:` with a regex anchored on the value ending the line, so a trailing comment read as "runs another image": deploy-service would skip the build and still bump image.tag to a tag nothing pushed. Both now use scripts/values-images.sh. It reads the block form (quoted or not, optional trailing comment) and exits 2 on any other form of the key: flow style, a quoted key, block scalar values, aliases, and a key that would sit inside another key's block scalar. Validate fails the step and check-image-name.sh exits 2 rather than guessing. Onboard now decides from the template tpl-git-commit will render (same selection, including the worker and unknown-name fallbacks) instead of the openclaw template name, so a new fixed-image template cannot drift. deploy-service documents that its build `when` couples it to tpl-validate-tenant's new output. build-image.yaml binds github.workflow_ref explicitly into the step that classifies the caller. --- .github/workflows/build-image.yaml | 14 ++- .../tpl-validate-tenant.yaml | 46 +++++--- .../cluster-templates/wft-deploy-service.yaml | 7 ++ .../scripts/check-image-name.sh | 15 ++- .../argo-workflows/scripts/values-images.sh | 80 ++++++++++++++ tests/test_check_image_name.py | 100 +++++++++++++++++- 6 files changed, 238 insertions(+), 24 deletions(-) create mode 100644 platform-gitops/argo-workflows/scripts/values-images.sh diff --git a/.github/workflows/build-image.yaml b/.github/workflows/build-image.yaml index 91cfb417f..2830493a3 100644 --- a/.github/workflows/build-image.yaml +++ b/.github/workflows/build-image.yaml @@ -122,17 +122,23 @@ jobs: # component name (mctl-telegram-preview pushes mctl-telegram). # * a dispatch at a ref other than main runs that ref's copy of this # file. The policy data is read from main either way. - # GITHUB_WORKFLOW_REF names the caller's workflow in a called run, so - # it tells a direct dispatch from a workflow_call. + # github.workflow_ref names the top-level (caller's) workflow in a + # called run, so it tells a direct dispatch from a workflow_call. It is + # bound explicitly rather than read from the implicit + # GITHUB_WORKFLOW_REF. github.event_name cannot make this distinction: + # in a called run it is the caller's event, so a dispatched + # release-deploy.yaml would also read workflow_dispatch. - name: Classify caller id: caller + env: + CALLER_WORKFLOW_REF: ${{ github.workflow_ref }} run: | - case "$GITHUB_WORKFLOW_REF" in + case "$CALLER_WORKFLOW_REF" in "${GITHUB_REPOSITORY}/.github/workflows/build-image.yaml@"*) direct=true ;; *) direct=false ;; esac echo "direct=${direct}" >> "$GITHUB_OUTPUT" - echo "direct dispatch: ${direct} (${GITHUB_WORKFLOW_REF})" + echo "direct dispatch: ${direct} (${CALLER_WORKFLOW_REF})" - name: Check out image-name policy if: steps.caller.outputs.direct == 'true' diff --git a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml index b2d7aa241..fb566d961 100644 --- a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml @@ -143,24 +143,44 @@ spec: # in one commit. # # Whether it builds: never for update-config or without a - # dockerfile_repo. On onboard the new service's template decides - # (openclaw services run the fixed image mctl-openclaw). On any - # other action the existing service decides: if its values.yaml - # names an image and none of them is /, - # it runs another image, and building this name would push one - # nothing runs. A missing values.yaml or one naming no image - # builds, as before (and deploy then fails at step 6). + # dockerfile_repo. Otherwise the values the service runs decide: + # on onboard, the service template tpl-git-commit will render + # (selected the same way, __SERVICE_NAME__ substituted); on any + # other action, the existing values.yaml. If they name an image and + # none of them is /, the service runs + # another image (e.g. the openclaw template's mctl-openclaw), and + # building this name would push one nothing runs. Values naming no + # image, or a deploy with no values.yaml, build as before (deploy + # then fails at step 6). The images are read by values-images.sh, + # the reader check-image-name.sh uses too; a repository line it + # cannot read fails here rather than guessing "no build", which + # would bump image.tag to a tag nothing pushed. BUILDS=false if [ "$ACTION" != "update-config" ] && [ -n "$REPO" ]; then - VALUES="platform-gitops/services/${TEAM}/${SERVICE}/values.yaml" - REGISTRY_RE=$(printf '%s' "$PARAM_CONTAINER_REGISTRY" | sed 's/[.]/\\./g') + TPLS="platform-gitops/argo-workflows/service-templates" if [ "$ACTION" = "onboard" ]; then - if [ "$SERVICE_TEMPLATE" != "openclaw" ]; then BUILDS=true; fi - elif [ -f "$VALUES" ] && grep -qE '^[[:space:]]*repository:' "$VALUES" \ - && ! grep -qE "^[[:space:]]*repository:[[:space:]]*[\"']?${REGISTRY_RE}/${SERVICE}[\"']?[[:space:]]*\$" "$VALUES"; then - echo "ℹ️ ${TEAM}/${SERVICE} runs another image; not building ${PARAM_CONTAINER_REGISTRY}/${SERVICE}" + # tpl-git-commit.yaml's template selection, step 4. + if [ "$SERVICE_TEMPLATE" != "default" ] && [ -d "${TPLS}/${SERVICE_TEMPLATE}" ]; then + TPL="${TPLS}/${SERVICE_TEMPLATE}/values.yaml.tpl" + elif [ "$TYPE" = "worker-service" ]; then + TPL="${TPLS}/worker/values.yaml.tpl" + else + TPL="${TPLS}/default/values.yaml.tpl" + fi + VALUES=/tmp/onboard-values.yaml + sed -e "s|__SERVICE_NAME__|${SERVICE}|g" "$TPL" > "$VALUES" else + VALUES="platform-gitops/services/${TEAM}/${SERVICE}/values.yaml" + fi + if [ ! -f "$VALUES" ]; then + BUILDS=true + elif ! IMAGES=$(sh platform-gitops/argo-workflows/scripts/values-images.sh "$VALUES"); then + echo "❌ cannot read the images ${VALUES} runs; not deciding whether to build" + exit 1 + elif [ -z "$IMAGES" ] || printf '%s\n' "$IMAGES" | grep -qxF "${PARAM_CONTAINER_REGISTRY}/${SERVICE}"; then BUILDS=true + else + echo "ℹ️ ${TEAM}/${SERVICE} runs another image; not building ${PARAM_CONTAINER_REGISTRY}/${SERVICE}" fi fi echo "$BUILDS" > /tmp/builds diff --git a/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml b/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml index 1f4ae7dc7..61d5242a1 100644 --- a/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/wft-deploy-service.yaml @@ -136,6 +136,13 @@ spec: # dockerfile_repo, and not for a service that runs another image, # such as the openclaw template's mctl-openclaw). validate checked # the image name with --build on exactly this decision. + # + # This `when` reads tpl-validate-tenant's `builds` output, so the + # two ClusterWorkflowTemplates must be on the cluster together: a + # run submitted against an older tpl-validate-tenant has no such + # output and errors at parameter resolution (nothing is built or + # committed). Both sync in the same Argo CD Application; wait for + # it before submitting, as for any template change. - name: build-image dependencies: [validate] when: "\"{{tasks.validate.outputs.parameters.builds}}\" == \"true\"" diff --git a/platform-gitops/argo-workflows/scripts/check-image-name.sh b/platform-gitops/argo-workflows/scripts/check-image-name.sh index 6b8ff3ee7..14e59daeb 100755 --- a/platform-gitops/argo-workflows/scripts/check-image-name.sh +++ b/platform-gitops/argo-workflows/scripts/check-image-name.sh @@ -139,11 +139,20 @@ for dir in "$SERVICES"/*/"$NAME"; do done # A service may run an image under a name other than its own directory -# (e.g. a preview of another service): those names are taken as well. -REGISTRY_RE=$(printf '%s' "$REGISTRY" | sed 's/[.]/\\./g') +# (e.g. a preview of another service): those names are taken as well. The +# images are read by values-images.sh, the same reader tpl-validate-tenant +# uses; a values.yaml it cannot read means this cannot decide. +VALUES_IMAGES="$ROOT/platform-gitops/argo-workflows/scripts/values-images.sh" +if [ ! -r "$VALUES_IMAGES" ]; then + echo "cannot read ${VALUES_IMAGES}" >&2 + exit 2 +fi for values in "$SERVICES"/*/*/values.yaml; do [ -f "$values" ] || continue - if grep -qE "^[[:space:]]*repository:[[:space:]]*[\"']?${REGISTRY_RE}/${NAME}[\"']?[[:space:]]*$" "$values"; then + if ! images=$(sh "$VALUES_IMAGES" "$values"); then + exit 2 + fi + if printf '%s\n' "$images" | grep -qxF "${REGISTRY}/${NAME}"; then owner=$(basename "$(dirname "$(dirname "$values")")") if [ "$owner" != "$TEAM" ]; then echo "image name '${NAME}' is run by team '${owner}'; team '${TEAM}' may not use it" >&2 diff --git a/platform-gitops/argo-workflows/scripts/values-images.sh b/platform-gitops/argo-workflows/scripts/values-images.sh new file mode 100644 index 000000000..b6288d290 --- /dev/null +++ b/platform-gitops/argo-workflows/scripts/values-images.sh @@ -0,0 +1,80 @@ +#!/bin/sh +# Usage: values-images.sh +# +# Prints the value of every `repository:` key in a service values.yaml (or a +# rendered service template), one per line: the images it runs. Used by +# check-image-name.sh (which names another team runs) and tpl-validate-tenant +# (whether deploy-service builds), so both read the field the same way. +# +# A line is read only in the block form every service and template uses, +# repository: optionally "quoted" or 'quoted', +# optionally followed by a ` # comment` +# where is a plain registry/name reference. Any other line naming a +# repository key (flow style `{repository: x}`, a quoted key, a block scalar +# value, an anchor or alias, an empty value, a comment inside quotes) is not +# guessed at: exit 2. So is a line that looks like one but may sit inside +# another key's block scalar (`config: |`), where it is text, not a key: +# reading it as an image would decide ownership and builds from a string. +# +# Exit 0: read (possibly no images). Exit 2: unreadable file, or a +# repository line in a form this does not read. Callers treat 2 as "could +# not decide". +# +# POSIX sh plus awk only: alpine/git (busybox) inside Argo, bash on runners. +# yq would parse structurally but is not in alpine/git. +set -u + +if [ "$#" -ne 1 ] || [ ! -r "$1" ]; then + echo "usage: $0 (readable)" >&2 + exit 2 +fi + +awk -v scalar=-1 ' + { + # Inside a block scalar: every line indented deeper than the key that + # opened it, and blank lines. That is text, skipped, except a line that + # would be a repository key were it not text: the region is measured + # counting a leading "- " too, so it can only be too wide, and a key + # it swallowed by mistake must refuse rather than vanish. Text merely + # mentioning repository (JSON in a config block) is skipped. + if (scalar >= 0) { + if ($0 ~ /^[[:space:]]*$/) next + match($0, /^[[:space:]-]*/) + if (RLENGTH > scalar) { + if ($0 ~ /^[[:space:]-]*[\047"]?repository[\047"]?[[:space:]]*:/) { + print FILENAME ":" NR ": a repository key inside a block scalar: " $0 > "/dev/stderr" + bad = 1 + } + next + } + scalar = -1 + } + } + /^[[:space:]]*#/ { next } + { + line = $0 + # A YAML comment starts at a # preceded by whitespace. + sub(/[[:space:]]+#.*$/, "", line) + if (line ~ /:[[:space:]]+[|>][-+0-9]*[[:space:]]*$/) { + match(line, /^[[:space:]]*/) + scalar = RLENGTH + } + } + line !~ /(^|[^A-Za-z0-9_-])repository[\047"]?[[:space:]]*:/ { next } + { + if (match(line, /^[[:space:]]*repository:[[:space:]]+/)) { + v = substr(line, RSTART + RLENGTH) + sub(/[[:space:]]+$/, "", v) + if (v ~ /^"[^"]*"$/ || v ~ /^\047[^\047]*\047$/) { + v = substr(v, 2, length(v) - 2) + } + if (v ~ /^[A-Za-z0-9][A-Za-z0-9._\/:@-]*$/) { + print v + next + } + } + print FILENAME ":" NR ": a repository key in a form this does not read: " $0 > "/dev/stderr" + bad = 1 + } + END { exit bad ? 2 : 0 } +' "$1" diff --git a/tests/test_check_image_name.py b/tests/test_check_image_name.py index db93cf4cc..6ae533f98 100644 --- a/tests/test_check_image_name.py +++ b/tests/test_check_image_name.py @@ -16,6 +16,7 @@ """ import pathlib import re +import shutil import subprocess import sys import tempfile @@ -24,6 +25,7 @@ REPO = pathlib.Path(__file__).resolve().parent.parent SCRIPT = REPO / "platform-gitops/argo-workflows/scripts/check-image-name.sh" +READER = REPO / "platform-gitops/argo-workflows/scripts/values-images.sh" LIST = REPO / "platform-gitops/argo-workflows/config/image-names.txt" failures = [] @@ -42,9 +44,15 @@ def run(root, team, name, *flags, registry=REG): capture_output=True, text=True).returncode -def fixture(list_text, services, tenants=("labs", "ovk", "karabu", "admins")): +def fixture(list_text, services, tenants=("labs", "ovk", "karabu", "admins"), reader=True): """services: {"team/name": values.yaml text or None}""" d = pathlib.Path(tempfile.mkdtemp()) + scripts = d / "platform-gitops/argo-workflows/scripts" + scripts.mkdir(parents=True) + if reader: + shutil.copy(READER, scripts / READER.name) + shutil.copytree(REPO / "platform-gitops/argo-workflows/service-templates", + d / "platform-gitops/argo-workflows/service-templates") for t in tenants: (d / "platform-gitops/tenants" / t).mkdir(parents=True) cfg = d / "platform-gitops/argo-workflows/config" @@ -139,6 +147,20 @@ def fixture(list_text, services, tenants=("labs", "ovk", "karabu", "admins")): check(run(fixture(LIST_TEXT, SERVICES, tenants=()), "karabu", "brand-new") == 0, "without --tenant the tenants tree is not needed") +# Ownership reads values through the same reader: a commented line still +# claims its image; a values.yaml it cannot read makes the values scan +# undecidable (exit 2), while names decided before the scan are unaffected. +check(run(fixture(LIST_TEXT, {"labs/x": "image:\n repository: ghcr.io/mctlhq/claimed # c\n"}), + "ovk", "claimed") == 1, + "a commented repository line must still claim its image for its team") +bad_values = {"labs/x": "image: {repository: ghcr.io/mctlhq/x}\n"} +check(run(fixture(LIST_TEXT, bad_values), "ovk", "brand-new") == 2, + "an unreadable values.yaml must make the values scan undecidable (exit 2)") +check(run(fixture(LIST_TEXT, bad_values), "labs", "mctl-academy") == 0, + "a granted name is decided before the values scan") +check(run(fixture(LIST_TEXT, SERVICES, reader=False), "ovk", "brand-new") == 2, + "a missing values-images.sh must be exit 2") + # Fail closed: no list, a list line the script does not understand. check(run(fixture(None, SERVICES), "karabu", "brand-new") == 2, "missing image-names.txt must refuse (exit 2)") @@ -215,14 +237,20 @@ def fixture(list_text, services, tenants=("labs", "ovk", "karabu", "admins")): "labs/runs-other": "image:\n repository: ghcr.io/mctlhq/own\n", "labs/no-image": "replicas: 1\n", "labs/no-values": None, + "labs/commented": "image:\n repository: ghcr.io/mctlhq/commented # built here\n", + "labs/single": "image:\n repository: 'ghcr.io/mctlhq/single'\n", + "labs/flow": "image: {repository: ghcr.io/mctlhq/flow, tag: x}\n", + "labs/block": "image:\n repository: >-\n ghcr.io/mctlhq/block\n", }) - def builds(action, team, service, repo="org/repo", template="default"): + def builds(action, team, service, repo="org/repo", template="default", + ctype="base-service"): with tempfile.TemporaryDirectory() as out: - block = m.group(1).replace("/tmp/builds", f"{out}/builds") + block = (m.group(1).replace("/tmp/builds", f"{out}/builds") + .replace("/tmp/onboard-values.yaml", f"{out}/onboard-values.yaml")) env = {"PATH": "/usr/bin:/bin", "ACTION": action, "TEAM": team, "SERVICE": service, "REPO": repo, "SERVICE_TEMPLATE": template, - "PARAM_CONTAINER_REGISTRY": REG} + "TYPE": ctype, "PARAM_CONTAINER_REGISTRY": REG} r = subprocess.run(["sh", "-c", "set -e\n" + block], cwd=decide_root, env=env, capture_output=True, text=True) if r.returncode != 0: @@ -242,9 +270,64 @@ def builds(action, team, service, repo="org/repo", template="default"): (("deploy", "labs", "runs-other"), "false", "the service runs another image"), (("deploy", "labs", "no-image"), "true", "no image named: builds as before"), (("deploy", "labs", "no-values"), "true", "no values.yaml: builds (step 6 refuses deploy)"), + (("deploy", "labs", "commented"), "true", "a trailing comment is not another image"), + (("deploy", "labs", "single"), "true", "single quotes still match"), + (("onboard", "labs", "brand-new", "org/repo", "no-such-template"), "true", + "an unknown template falls back to default, as tpl-git-commit does"), + (("onboard", "labs", "brand-new", "org/repo", "default", "worker-service"), "true", + "worker-service onboard renders the worker template"), ): got = builds(*args) check(got == want, f"validate build decision {args}: {got!r}, want {want!r} ({why})") + # A repository line the reader cannot read fails the step: never a + # silent "no build" that would bump image.tag to a tag nothing pushed. + for svc in ("flow", "block"): + got = builds("deploy", "labs", svc) + check(got.startswith("error:"), f"validate must fail on labs/{svc}'s values, got {got!r}") + +# values-images.sh: the one reader of `repository:` both callers use. +def read_images(text): + with tempfile.NamedTemporaryFile("w", suffix=".yaml", delete=False) as f: + f.write(text) + r = subprocess.run(["sh", str(READER), f.name], capture_output=True, text=True) + pathlib.Path(f.name).unlink() + return r.returncode, r.stdout.split() + +for text, want in ( + ("image:\n repository: ghcr.io/mctlhq/a\n", (0, ["ghcr.io/mctlhq/a"])), + ('image:\n repository: "ghcr.io/mctlhq/a"\n', (0, ["ghcr.io/mctlhq/a"])), + ("image:\n repository: 'ghcr.io/mctlhq/a' # c\n", (0, ["ghcr.io/mctlhq/a"])), + ("image:\n repository: ghcr.io/mctlhq/a # c\n tag: x\n", (0, ["ghcr.io/mctlhq/a"])), + ("image:\n\trepository:\tghcr.io/mctlhq/a\n", (0, ["ghcr.io/mctlhq/a"])), + ("a:\n repository: x/a\nb:\n repository: x/b\n", (0, ["x/a", "x/b"])), + ("# repository: ghcr.io/mctlhq/a\nreplicas: 1\n", (0, [])), + ("image_repository: something odd here\n", (0, [])), + ("replicas: 1\n", (0, [])), + ("image: {repository: ghcr.io/mctlhq/a}\n", (2, [])), + ("image:\n repository: >-\n ghcr.io/mctlhq/a\n", (2, [])), + ("image:\n repository:\n", (2, [])), + ('image:\n repository: ""\n', (2, [])), + ("image:\n repository: *img\n", (2, [])), + ('image:\n repository: "ghcr.io/mctlhq/a # b"\n', (2, [])), + ("image:\n repository: ghcr.io/mctlhq/a extra\n", (2, [])), + ("image:\n repository:ghcr.io/mctlhq/a\n", (2, [])), + ("image:\n repository : ghcr.io/mctlhq/a\n", (2, [])), + ('image:\n "repository": ghcr.io/mctlhq/a\n', (2, [])), + ("image:\n 'repository': ghcr.io/mctlhq/a\n", (2, [])), + ("- repository: ghcr.io/mctlhq/a\n", (2, [])), + ("image:\n repository: ghcr.io/mctlhq/a\r\n", (0, ["ghcr.io/mctlhq/a"])), + # Block scalars are text: JSON mentioning repository is skipped, a line + # that would be a repository key refuses, and the scalar ends at the + # next line no deeper than its key. + ('config: |\n {"subject": {"repository": ["x"]}}\nimage:\n repository: x/a\n', + (0, ["x/a"])), + ("config: |\n repository: x/b\nimage:\n repository: x/a\n", (2, [])), + ("env:\n - name: X\n value: >-\n repository: y\n", (2, [])), + ("note: | # c\n text\n\n more\nimage:\n repository: x/a\n", (0, ["x/a"])), +): + rc, out = read_images(text) + check((rc, out if rc == 0 else []) == want, + f"values-images.sh on {text!r}: {(rc, out)}, want {want}") env = {e["name"]: e["value"] for e in by_name["validate"]["script"].get("env", [])} check(env.get("PARAM_CONTAINER_REGISTRY") == "{{inputs.parameters.container_registry}}", "validate must take the registry as a bound env value") @@ -342,6 +425,15 @@ def builds(action, team, service, repo="org/repo", template="default"): check('\nif ! sh .image-name-policy/platform-gitops/argo-workflows/scripts/check-image-name.sh --build' in run_src and '"${REGISTRY}/${GITHUB_REPOSITORY_OWNER}"' in run_src, "build-image must check in build mode against its own registry") +# The caller is classified from github.workflow_ref bound explicitly into the +# step (in a called run it names the caller's workflow), not from the +# implicit GITHUB_WORKFLOW_REF or the event name. +caller = [s for s in steps if s.get("id") == "caller"] +check(len(caller) == 1 + and caller[0].get("env", {}).get("CALLER_WORKFLOW_REF") == "${{ github.workflow_ref }}" + and 'case "$CALLER_WORKFLOW_REF" in' in caller[0]["run"] + and "GITHUB_WORKFLOW_REF" not in caller[0]["run"], + "build-image must classify its caller from an explicitly bound github.workflow_ref") push = [s for s in steps if s.get("name") == "Build and push"][0]["with"]["tags"] latest = [ln for ln in push.splitlines() if "latest" in ln] check(len(latest) == 1 and "steps.caller.outputs.direct != 'true'" in latest[0], From 644e29d6d4794d6500ff6a1d157c94d33000f045 Mon Sep 17 00:00:00 2001 From: mashkovd Date: Sun, 4 Oct 2026 21:03:21 +0200 Subject: [PATCH 5/9] fix(workflows): skip block scalars opened by sequence items values-images.sh tracked only block scalars opened by a key, so the `- |` scripts in the openclaw values and claude-remote were read as YAML: a line in one that mentioned `repository:` made every check undecidable, and one starting with it claimed an image. Scalars opened by a key or a sequence item are now skipped with YAML's own scoping (deeper than the key or dash that opened them, anchors and tags allowed), so text neither claims nor blocks. The output matches a structural parse on the real tree and on generated documents. The onboard render also substitutes __TEAM_NAME__, and validate fails on an image that still holds a placeholder instead of reading it as another image. The reader is committed executable like its siblings, and its header states that one unreadable values file stops every check. --- .../tpl-validate-tenant.yaml | 12 +++-- .../argo-workflows/scripts/values-images.sh | 50 +++++++++++-------- tests/test_check_image_name.py | 48 ++++++++++++++++-- 3 files changed, 81 insertions(+), 29 deletions(-) mode change 100644 => 100755 platform-gitops/argo-workflows/scripts/values-images.sh diff --git a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml index fb566d961..8692ca975 100644 --- a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml @@ -145,7 +145,8 @@ spec: # Whether it builds: never for update-config or without a # dockerfile_repo. Otherwise the values the service runs decide: # on onboard, the service template tpl-git-commit will render - # (selected the same way, __SERVICE_NAME__ substituted); on any + # (selected the same way, __SERVICE_NAME__ and __TEAM_NAME__ + # substituted, the placeholders an image name can use); on any # other action, the existing values.yaml. If they name an image and # none of them is /, the service runs # another image (e.g. the openclaw template's mctl-openclaw), and @@ -154,7 +155,9 @@ spec: # then fails at step 6). The images are read by values-images.sh, # the reader check-image-name.sh uses too; a repository line it # cannot read fails here rather than guessing "no build", which - # would bump image.tag to a tag nothing pushed. + # would bump image.tag to a tag nothing pushed. So does an image + # still holding a __PLACEHOLDER__: a template using one this step + # does not substitute would otherwise read as "another image". BUILDS=false if [ "$ACTION" != "update-config" ] && [ -n "$REPO" ]; then TPLS="platform-gitops/argo-workflows/service-templates" @@ -168,7 +171,7 @@ spec: TPL="${TPLS}/default/values.yaml.tpl" fi VALUES=/tmp/onboard-values.yaml - sed -e "s|__SERVICE_NAME__|${SERVICE}|g" "$TPL" > "$VALUES" + sed -e "s|__SERVICE_NAME__|${SERVICE}|g" -e "s|__TEAM_NAME__|${TEAM}|g" "$TPL" > "$VALUES" else VALUES="platform-gitops/services/${TEAM}/${SERVICE}/values.yaml" fi @@ -177,6 +180,9 @@ spec: elif ! IMAGES=$(sh platform-gitops/argo-workflows/scripts/values-images.sh "$VALUES"); then echo "❌ cannot read the images ${VALUES} runs; not deciding whether to build" exit 1 + elif printf '%s\n' "$IMAGES" | grep -q '__[A-Z][A-Z_]*__'; then + echo "❌ ${VALUES} names an image with an unsubstituted placeholder; not deciding whether to build" + exit 1 elif [ -z "$IMAGES" ] || printf '%s\n' "$IMAGES" | grep -qxF "${PARAM_CONTAINER_REGISTRY}/${SERVICE}"; then BUILDS=true else diff --git a/platform-gitops/argo-workflows/scripts/values-images.sh b/platform-gitops/argo-workflows/scripts/values-images.sh old mode 100644 new mode 100755 index b6288d290..d42599279 --- a/platform-gitops/argo-workflows/scripts/values-images.sh +++ b/platform-gitops/argo-workflows/scripts/values-images.sh @@ -12,13 +12,21 @@ # where is a plain registry/name reference. Any other line naming a # repository key (flow style `{repository: x}`, a quoted key, a block scalar # value, an anchor or alias, an empty value, a comment inside quotes) is not -# guessed at: exit 2. So is a line that looks like one but may sit inside -# another key's block scalar (`config: |`), where it is text, not a key: -# reading it as an image would decide ownership and builds from a string. +# guessed at: exit 2. +# +# Block scalars are text and are skipped, whichever way they are opened: +# by a key (`config: |`, `- run: >-`) or by a sequence item (`- |`, the +# form the openclaw values use for shell scripts). As in YAML, a scalar +# holds the lines indented deeper than the key or dash that opened it, and +# blank lines; the first line at that column or shallower ends it. So a +# script or JSON blob mentioning `repository:` neither claims an image nor +# makes the file unreadable. # # Exit 0: read (possibly no images). Exit 2: unreadable file, or a # repository line in a form this does not read. Callers treat 2 as "could -# not decide". +# not decide". check-image-name.sh reads every service's values.yaml, so one +# such line in any service makes every check undecidable until it is fixed; +# CI runs that scan on every pull request and push to main. # # POSIX sh plus awk only: alpine/git (busybox) inside Argo, bash on runners. # yq would parse structurally but is not in alpine/git. @@ -31,22 +39,11 @@ fi awk -v scalar=-1 ' { - # Inside a block scalar: every line indented deeper than the key that - # opened it, and blank lines. That is text, skipped, except a line that - # would be a repository key were it not text: the region is measured - # counting a leading "- " too, so it can only be too wide, and a key - # it swallowed by mistake must refuse rather than vanish. Text merely - # mentioning repository (JSON in a config block) is skipped. + # Inside a block scalar: text, skipped. if (scalar >= 0) { if ($0 ~ /^[[:space:]]*$/) next - match($0, /^[[:space:]-]*/) - if (RLENGTH > scalar) { - if ($0 ~ /^[[:space:]-]*[\047"]?repository[\047"]?[[:space:]]*:/) { - print FILENAME ":" NR ": a repository key inside a block scalar: " $0 > "/dev/stderr" - bad = 1 - } - next - } + match($0, /^[[:space:]]*/) + if (RLENGTH > scalar) next scalar = -1 } } @@ -55,9 +52,20 @@ awk -v scalar=-1 ' line = $0 # A YAML comment starts at a # preceded by whitespace. sub(/[[:space:]]+#.*$/, "", line) - if (line ~ /:[[:space:]]+[|>][-+0-9]*[[:space:]]*$/) { - match(line, /^[[:space:]]*/) - scalar = RLENGTH + # A block scalar opens at a line ending in | or > (with optional + # indicators, and an optional anchor or tag before it), after a key or a + # sequence dash. It belongs to that key, or to the last dash, and its + # text is what is indented deeper than that column. + if (line ~ /(:|(^|[[:space:]])-)[[:space:]]+([&!][^[:space:]]*[[:space:]]+)*[|>][-+0-9]*[[:space:]]*$/) { + if (line ~ /:[[:space:]]+([&!][^[:space:]]*[[:space:]]+)*[|>][-+0-9]*[[:space:]]*$/) { + match(line, /^[[:space:]]*(-[[:space:]]+)*/) + scalar = RLENGTH + } else { + match(line, /^[[:space:]]*(-[[:space:]]+)*/) + prefix = substr(line, 1, RLENGTH) + sub(/-[[:space:]]+$/, "", prefix) + scalar = length(prefix) + } } } line !~ /(^|[^A-Za-z0-9_-])repository[\047"]?[[:space:]]*:/ { next } diff --git a/tests/test_check_image_name.py b/tests/test_check_image_name.py index 6ae533f98..9db95a026 100644 --- a/tests/test_check_image_name.py +++ b/tests/test_check_image_name.py @@ -158,6 +158,14 @@ def fixture(list_text, services, tenants=("labs", "ovk", "karabu", "admins"), re "an unreadable values.yaml must make the values scan undecidable (exit 2)") check(run(fixture(LIST_TEXT, bad_values), "labs", "mctl-academy") == 0, "a granted name is decided before the values scan") +# Text in a block scalar claims nothing and is not unreadable, whether a +# key or a sequence item opens it (the openclaw values' `- |` scripts). +script_values = {"labs/x": "command:\n - |\n repository: ghcr.io/mctlhq/in-script\n" + " echo \"repository: $X\"\nimage:\n repository: ghcr.io/mctlhq/x\n"} +check(run(fixture(LIST_TEXT, script_values), "ovk", "in-script") == 0, + "a repository line inside a `- |` script must not claim its image") +check(run(fixture(LIST_TEXT, script_values), "ovk", "x") == 1, + "the real repository key after the script still claims its image") check(run(fixture(LIST_TEXT, SERVICES, reader=False), "ovk", "brand-new") == 2, "a missing values-images.sh must be exit 2") @@ -241,7 +249,18 @@ def fixture(list_text, services, tenants=("labs", "ovk", "karabu", "admins"), re "labs/single": "image:\n repository: 'ghcr.io/mctlhq/single'\n", "labs/flow": "image: {repository: ghcr.io/mctlhq/flow, tag: x}\n", "labs/block": "image:\n repository: >-\n ghcr.io/mctlhq/block\n", + "labs/script": "command:\n - |\n repository: ghcr.io/mctlhq/other\n" + "image:\n repository: ghcr.io/mctlhq/script\n", }) + # Templates using the other substituted placeholder, and one this step + # does not substitute. + tdir = decide_root / "platform-gitops/argo-workflows/service-templates" + (tdir / "team-named").mkdir() + (tdir / "team-named/values.yaml.tpl").write_text( + "image:\n repository: ghcr.io/mctlhq/__TEAM_NAME__-__SERVICE_NAME__\n") + (tdir / "odd").mkdir() + (tdir / "odd/values.yaml.tpl").write_text( + "image:\n repository: ghcr.io/mctlhq/__OTHER__\n") def builds(action, team, service, repo="org/repo", template="default", ctype="base-service"): @@ -276,6 +295,9 @@ def builds(action, team, service, repo="org/repo", template="default", "an unknown template falls back to default, as tpl-git-commit does"), (("onboard", "labs", "brand-new", "org/repo", "default", "worker-service"), "true", "worker-service onboard renders the worker template"), + (("deploy", "labs", "script"), "true", "text in a `- |` script is not another image"), + (("onboard", "labs", "x", "org/repo", "team-named"), "false", + "__TEAM_NAME__ is substituted: the template runs labs-x, not x"), ): got = builds(*args) check(got == want, f"validate build decision {args}: {got!r}, want {want!r} ({why})") @@ -284,8 +306,13 @@ def builds(action, team, service, repo="org/repo", template="default", for svc in ("flow", "block"): got = builds("deploy", "labs", svc) check(got.startswith("error:"), f"validate must fail on labs/{svc}'s values, got {got!r}") + got = builds("onboard", "labs", "x", "org/repo", "odd") + check(got.startswith("error:"), f"validate must fail on an unsubstituted placeholder, got {got!r}") # values-images.sh: the one reader of `repository:` both callers use. +check(subprocess.run(["git", "ls-files", "-s", str(READER)], cwd=REPO, capture_output=True, + text=True).stdout.startswith("100755"), + "values-images.sh must be committed executable, like its siblings") def read_images(text): with tempfile.NamedTemporaryFile("w", suffix=".yaml", delete=False) as f: f.write(text) @@ -316,14 +343,25 @@ def read_images(text): ("image:\n 'repository': ghcr.io/mctlhq/a\n", (2, [])), ("- repository: ghcr.io/mctlhq/a\n", (2, [])), ("image:\n repository: ghcr.io/mctlhq/a\r\n", (0, ["ghcr.io/mctlhq/a"])), - # Block scalars are text: JSON mentioning repository is skipped, a line - # that would be a repository key refuses, and the scalar ends at the - # next line no deeper than its key. + # Block scalars are text, skipped: opened by a key or by a sequence + # item, they hold the lines deeper than that key or dash, and end at the + # next line no deeper. ('config: |\n {"subject": {"repository": ["x"]}}\nimage:\n repository: x/a\n', (0, ["x/a"])), - ("config: |\n repository: x/b\nimage:\n repository: x/a\n", (2, [])), - ("env:\n - name: X\n value: >-\n repository: y\n", (2, [])), + ("config: |\n repository: x/b\nimage:\n repository: x/a\n", (0, ["x/a"])), + ("env:\n - name: X\n value: >-\n repository: y\n", (0, [])), ("note: | # c\n text\n\n more\nimage:\n repository: x/a\n", (0, ["x/a"])), + ("command:\n - |\n repository: x/y\n", (0, [])), + ('command:\n - |\n {"repository": ["x"]}\n echo "repository: $X"\n', (0, [])), + # A sibling `- |` at the dash's column closes the previous scalar. + ("command:\n - |\n echo\n - |\n repository: x/y\n - image:\n repository: x/a\n", + (0, ["x/a"])), + ("command:\n- |\n repository: x/y\nimage:\n repository: x/a\n", (0, ["x/a"])), + # A key in a sequence item opens at the key's column, not the dash's. + ("jobs:\n - run: |\n repository: x/b\n image:\n repository: x/a\n", + (0, ["x/a"])), + ("c: &a |\n repository: x/b\nd: !!str >-\n repository: x/c\nimage:\n repository: x/a\n", + (0, ["x/a"])), ): rc, out = read_images(text) check((rc, out if rc == 0 else []) == want, From 7e200fcffc99e77ec5482092b1c35c6fec71cbcb Mon Sep 17 00:00:00 2001 From: mashkovd Date: Sun, 4 Oct 2026 21:06:25 +0200 Subject: [PATCH 6/9] test(workflows): pin how release dispatchers reach build-image Every release dispatcher in the organisation runs release-deploy.yaml, which calls build-image.yaml as a reusable workflow. The test pins that call and runs the caller classification for a release-deploy ref (not direct, so the image-name pin does not apply) and for build-image's own ref (direct). --- tests/test_check_image_name.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/test_check_image_name.py b/tests/test_check_image_name.py index 9db95a026..a2ed3909c 100644 --- a/tests/test_check_image_name.py +++ b/tests/test_check_image_name.py @@ -397,7 +397,7 @@ def read_images(text): # Direct dispatchers of build-image.yaml outside Argo, as found on each # repository's default branch (all other organisation workflows call # release-deploy.yaml): each must keep passing the push-time check. -for team, name in (("labs", "openclaw"), # mctl-openclaw upstream-sync-release.yml +for team, name in (("labs", "openclaw"), # mctl-openclaw upstream-sync-release.yml (repo archived) ("platform", "grafana-iac"), # platform-gitops/images/*, by hand ("platform", "mc"), ("platform", "vault-iac"), @@ -472,6 +472,27 @@ def read_images(text): and 'case "$CALLER_WORKFLOW_REF" in' in caller[0]["run"] and "GITHUB_WORKFLOW_REF" not in caller[0]["run"], "build-image must classify its caller from an explicitly bound github.workflow_ref") +# Every release dispatcher in the organisation (release-please.yml in +# mctl-academy, -agent, -agents, -api, -docs, -portal, -telegram, -web, +# portfolio, seerrsense; mctl-alice release-deploy.yml; mctl-design +# deploy.yml; mctl-telegram preview-deploy.yml) runs release-deploy.yaml, +# which reaches build-image.yaml as a workflow_call. There the caller is +# classified as not direct, so the image-name pin does not apply and +# release names that differ from the component (mctl-telegram-preview -> +# mctl-telegram) keep building. Run the classification for both refs. +rd = yaml.safe_load((REPO / ".github/workflows/release-deploy.yaml").read_text()) +check(any(j.get("uses") == "./.github/workflows/build-image.yaml" for j in rd["jobs"].values()), + "release-deploy.yaml must call build-image.yaml as a reusable workflow") +if len(caller) == 1: + for ref, want in (("mctlhq/mctl-gitops/.github/workflows/release-deploy.yaml@refs/heads/main", "false"), + ("mctlhq/mctl-gitops/.github/workflows/build-image.yaml@refs/heads/main", "true")): + with tempfile.TemporaryDirectory() as out: + r = subprocess.run(["bash", "-c", caller[0]["run"]], capture_output=True, text=True, + env={"PATH": "/usr/bin:/bin", "CALLER_WORKFLOW_REF": ref, + "GITHUB_REPOSITORY": "mctlhq/mctl-gitops", + "GITHUB_OUTPUT": f"{out}/o"}) + got = (pathlib.Path(out) / "o").read_text().strip() if r.returncode == 0 else r.stderr + check(got == f"direct={want}", f"caller classification for {ref}: {got!r}") push = [s for s in steps if s.get("name") == "Build and push"][0]["with"]["tags"] latest = [ln for ln in push.splitlines() if "latest" in ln] check(len(latest) == 1 and "steps.caller.outputs.direct != 'true'" in latest[0], From f54c24e16b9518a6e323647ff032ae351e7c3d46 Mon Sep 17 00:00:00 2001 From: mashkovd Date: Sun, 4 Oct 2026 21:24:57 +0200 Subject: [PATCH 7/9] test(workflows): read every real values file and pin release gating The reader had fixtures but nothing ran it over the files it actually decides on. The suite now reads every service values.yaml and service template, and requires each one to read cleanly and to yield exactly the repository keys a structural YAML parse finds, so a mis-scoped block scalar fails the suite instead of dropping a claim. A block scalar opened by a sequence item now needs dashes to make up the whole line before the indicator; a plain value that merely ends in " - |" no longer opens one. The suite also pins that the push-time image-name steps run only for a direct dispatch, and that build-image declares its team and component inputs for both triggers. The openclaw build grant is dropped: its only builder is archived, so no team needs it, and a successor would need a reviewed grant. Claude-Session: https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2 --- .../argo-workflows/config/image-names.txt | 10 ++-- .../argo-workflows/scripts/values-images.sh | 26 ++++----- tests/test_check_image_name.py | 56 ++++++++++++++++++- 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/platform-gitops/argo-workflows/config/image-names.txt b/platform-gitops/argo-workflows/config/image-names.txt index f0eed170b..406a8b5f6 100644 --- a/platform-gitops/argo-workflows/config/image-names.txt +++ b/platform-gitops/argo-workflows/config/image-names.txt @@ -78,9 +78,9 @@ grant ovk mctl-academy # openclaw: several teams hold a service of that name (labs, ovk, admins); # each runs the fixed image mctl-openclaw (service-templates/openclaw), which # the mctl- prefix reserves, and deploy-service does not build for that -# template. The image named openclaw itself is built only by mctl-openclaw's -# upstream-sync workflow, dispatched as team labs; nothing in this repository -# runs it. Shared, so the service name stays available to every team, with -# builds limited to the grant. +# template. The image named openclaw itself was built only by mctl-openclaw's +# upstream-sync workflow; that repository is archived and nothing in this +# repository runs the image. Shared, so the service name stays available to +# every team, and with no grant no team may build it. A successor sync that +# builds it again needs a reviewed grant line here. shared openclaw -grant labs openclaw diff --git a/platform-gitops/argo-workflows/scripts/values-images.sh b/platform-gitops/argo-workflows/scripts/values-images.sh index d42599279..11893e368 100755 --- a/platform-gitops/argo-workflows/scripts/values-images.sh +++ b/platform-gitops/argo-workflows/scripts/values-images.sh @@ -53,19 +53,19 @@ awk -v scalar=-1 ' # A YAML comment starts at a # preceded by whitespace. sub(/[[:space:]]+#.*$/, "", line) # A block scalar opens at a line ending in | or > (with optional - # indicators, and an optional anchor or tag before it), after a key or a - # sequence dash. It belongs to that key, or to the last dash, and its - # text is what is indented deeper than that column. - if (line ~ /(:|(^|[[:space:]])-)[[:space:]]+([&!][^[:space:]]*[[:space:]]+)*[|>][-+0-9]*[[:space:]]*$/) { - if (line ~ /:[[:space:]]+([&!][^[:space:]]*[[:space:]]+)*[|>][-+0-9]*[[:space:]]*$/) { - match(line, /^[[:space:]]*(-[[:space:]]+)*/) - scalar = RLENGTH - } else { - match(line, /^[[:space:]]*(-[[:space:]]+)*/) - prefix = substr(line, 1, RLENGTH) - sub(/-[[:space:]]+$/, "", prefix) - scalar = length(prefix) - } + # indicators, and an optional anchor or tag before it), as the value of a key + # or as a sequence item: the dashes then make up the whole line before + # it, so a plain value merely ending in " - |" opens nothing. It belongs + # to that key, or to the last dash, and its text is what is indented + # deeper than that column. + if (line ~ /:[[:space:]]+([&!][^[:space:]]*[[:space:]]+)*[|>][-+0-9]*[[:space:]]*$/) { + match(line, /^[[:space:]]*(-[[:space:]]+)*/) + scalar = RLENGTH + } else if (line ~ /^[[:space:]]*(-[[:space:]]+)*-[[:space:]]+([&!][^[:space:]]*[[:space:]]+)*[|>][-+0-9]*[[:space:]]*$/) { + match(line, /^[[:space:]]*(-[[:space:]]+)*/) + prefix = substr(line, 1, RLENGTH) + sub(/-[[:space:]]+$/, "", prefix) + scalar = length(prefix) } } line !~ /(^|[^A-Za-z0-9_-])repository[\047"]?[[:space:]]*:/ { next } diff --git a/tests/test_check_image_name.py b/tests/test_check_image_name.py index a2ed3909c..81169c5a5 100644 --- a/tests/test_check_image_name.py +++ b/tests/test_check_image_name.py @@ -362,10 +362,43 @@ def read_images(text): (0, ["x/a"])), ("c: &a |\n repository: x/b\nd: !!str >-\n repository: x/c\nimage:\n repository: x/a\n", (0, ["x/a"])), + # A plain value that merely ends in " - |" opens no scalar. + (" d: a - |\n image:\n repository: x/a\n", (0, ["x/a"])), + ("d: pipe - >\nimage:\n repository: x/a\n", (0, ["x/a"])), + ("- d: a - |\n repository: x/a\n", (0, ["x/a"])), ): rc, out = read_images(text) check((rc, out if rc == 0 else []) == want, f"values-images.sh on {text!r}: {(rc, out)}, want {want}") +# The reader against the real files: every service values.yaml and every +# template (with the placeholders an image can use substituted) reads, and +# yields exactly the repository keys a structural parse finds. A +# mis-measured block scalar that swallows a key, or a form the reader reads +# differently from YAML, fails here rather than losing a claim silently. +def yaml_repos(text): + found = [] + def walk(n): + if isinstance(n, dict): + for k, v in n.items(): + if k == "repository" and isinstance(v, str): + found.append(v) + walk(v) + elif isinstance(n, list): + for v in n: + walk(v) + walk(yaml.safe_load(text)) + return found + +real_values = (sorted((REPO / "platform-gitops/services").glob("*/*/values.yaml")) + + sorted((REPO / "platform-gitops/argo-workflows/service-templates").glob("*/values.yaml.tpl"))) +check(len(real_values) > 4, "no real values files found") +for f in real_values: + text = f.read_text().replace("__SERVICE_NAME__", "svc").replace("__TEAM_NAME__", "team") + rc, out = read_images(text) + want = yaml_repos(text) + check(rc == 0 and out == want, + f"values-images.sh on {f.relative_to(REPO)}: {(rc, out)}, want {want}") + env = {e["name"]: e["value"] for e in by_name["validate"]["script"].get("env", [])} check(env.get("PARAM_CONTAINER_REGISTRY") == "{{inputs.parameters.container_registry}}", "validate must take the registry as a bound env value") @@ -397,8 +430,11 @@ def read_images(text): # Direct dispatchers of build-image.yaml outside Argo, as found on each # repository's default branch (all other organisation workflows call # release-deploy.yaml): each must keep passing the push-time check. -for team, name in (("labs", "openclaw"), # mctl-openclaw upstream-sync-release.yml (repo archived) - ("platform", "grafana-iac"), # platform-gitops/images/*, by hand +# mctl-openclaw's upstream sync (team labs, openclaw) is gone with that +# repository's archiving, and so is its grant: nobody may build openclaw. +check(run(REPO, "labs", "openclaw", "--build") == 1, + "no team may build openclaw now that its only builder is archived") +for team, name in (("platform", "grafana-iac"), # platform-gitops/images/*, by hand ("platform", "mc"), ("platform", "vault-iac"), ("platform", "zitadel-iac"), @@ -493,6 +529,19 @@ def read_images(text): "GITHUB_OUTPUT": f"{out}/o"}) got = (pathlib.Path(out) / "o").read_text().strip() if r.returncode == 0 else r.stderr check(got == f"direct={want}", f"caller classification for {ref}: {got!r}") +# ...and the steps that enforce the pin run only for a direct dispatch, so a +# release workflow_call skips them. +for step in ("Check out image-name policy", "Check image name"): + st = [x for x in steps if x.get("name") == step] + check(len(st) == 1 and st[0].get("if") == "steps.caller.outputs.direct == 'true'", + f"'{step}' must run only for a direct dispatch, not for release workflow_calls") +# The check reads team_name and component_name, so both trigger kinds must +# declare them: an undeclared dispatch input is refused by GitHub. +on = bi.get("on", bi.get(True, {})) +for trigger in ("workflow_dispatch", "workflow_call"): + declared = (on.get(trigger) or {}).get("inputs", {}) + for inp in ("image_name", "team_name", "component_name"): + check(inp in declared, f"build-image.yaml must declare {inp} under {trigger}") push = [s for s in steps if s.get("name") == "Build and push"][0]["with"]["tags"] latest = [ln for ln in push.splitlines() if "latest" in ln] check(len(latest) == 1 and "steps.caller.outputs.direct != 'true'" in latest[0], @@ -503,4 +552,5 @@ def read_images(text): sys.exit(1) print(f"OK: {len(CASES) + 7} fixture cases, " f"{len(list((REPO / 'platform-gitops/services').glob('*/*/')))} existing services, " - f"{len(referenced)} referenced platform images") + f"{len(referenced)} referenced platform images, " + f"{len(real_values)} real values files read") From 0c8ce302ff1a7d7d050da2dc068d013057ccd2d2 Mon Sep 17 00:00:00 2001 From: mashkovd Date: Sun, 4 Oct 2026 21:46:24 +0200 Subject: [PATCH 8/9] docs(workflows): drop the archived openclaw sync from dispatchers The build-image comment still listed mctl-openclaw's upstream sync as a direct dispatcher after its grant was removed, and image-names.txt did not say that a preview building the openclaw image is now refused for every team. Comments only. Claude-Session: https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2 --- .github/workflows/build-image.yaml | 3 +-- platform-gitops/argo-workflows/config/image-names.txt | 7 +++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-image.yaml b/.github/workflows/build-image.yaml index 2830493a3..90de63e01 100644 --- a/.github/workflows/build-image.yaml +++ b/.github/workflows/build-image.yaml @@ -102,8 +102,7 @@ jobs: steps: # --- Image name: build only names owned by the requesting team --- # A direct dispatch is how tenant builds arrive (Argo's deploy-service - # and preview-deploy), how mctl-openclaw's upstream sync builds the - # openclaw image (team labs), and how platform images under + # and preview-deploy) and how platform images under # platform-gitops/images/ are built by hand (team platform). Every # other dispatcher in the organisation calls release-deploy.yaml. For a # direct dispatch the pushed name must be exactly diff --git a/platform-gitops/argo-workflows/config/image-names.txt b/platform-gitops/argo-workflows/config/image-names.txt index 406a8b5f6..be0e2a270 100644 --- a/platform-gitops/argo-workflows/config/image-names.txt +++ b/platform-gitops/argo-workflows/config/image-names.txt @@ -81,6 +81,9 @@ grant ovk mctl-academy # template. The image named openclaw itself was built only by mctl-openclaw's # upstream-sync workflow; that repository is archived and nothing in this # repository runs the image. Shared, so the service name stays available to -# every team, and with no grant no team may build it. A successor sync that -# builds it again needs a reviewed grant line here. +# every team, and with no grant no team may build it. That includes an +# onboard or a preview with a git_ref that would push openclaw itself: +# preview-deploy checks in build mode whenever it builds, so such a preview +# is refused for every team. A successor sync that builds the image again +# needs a reviewed grant line here. shared openclaw From ceaf89fb03cf6a3944ac41480b5d588643731a3a Mon Sep 17 00:00:00 2001 From: mashkovd Date: Sun, 11 Oct 2026 20:10:45 +0200 Subject: [PATCH 9/9] fix(workflows): merge the duplicate env key in validate The merge with main left validate with two env keys: main's PARAM_* bindings from #1812 and the branch's PARAM_CONTAINER_REGISTRY. YAML keeps only the last one, so every PARAM_* read by the script was unset. One env list now holds all ten bindings (yamllint key-duplicates and validate-shell-param-interpolation both pass). Claude-Session: https://claude.ai/code/session_012TfVcukEh7ry8MiLNN89N4 --- .../cluster-templates/tpl-validate-tenant.yaml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml index 17669b7d7..5486575f5 100644 --- a/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml +++ b/platform-gitops/argo-workflows/cluster-templates/tpl-validate-tenant.yaml @@ -77,12 +77,11 @@ spec: value: "{{inputs.parameters.git_tag}}" - name: PARAM_SERVICE_TEMPLATE value: "{{inputs.parameters.service_template}}" + - name: PARAM_CONTAINER_REGISTRY + value: "{{inputs.parameters.container_registry}}" envFrom: - configMapRef: name: mctl-platform-config - env: - - name: PARAM_CONTAINER_REGISTRY - value: "{{inputs.parameters.container_registry}}" source: | set -e