From d8167f710b4d973b543747291378d854d86b680c Mon Sep 17 00:00:00 2001 From: Lucas Koontz Date: Sun, 1 Nov 2026 01:36:21 -0700 Subject: [PATCH 1/2] fix(argocd-pr-env-deploy): turn the staff console off when no Deploys line selects it (ENG-3872) --- README.md | 6 ++ argocd-pr-env-deploy/action.yml | 39 ++++++++++ tests/test_argocd_pr_env_deploy.py | 119 ++++++++++++++++++++++++++++- 3 files changed, 163 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index cea57bb..ca48892 100644 --- a/README.md +++ b/README.md @@ -421,6 +421,12 @@ What actually goes wrong with a GitHub secret is different and has a cheaper fix `argocd-pr-env-deploy` applies a PR's image tags to its ArgoCD environment and blocks until the environment is Synced and Healthy. +The staff console is opt-in. A `mindshub_stafftools` anchor PR, or a +`Deploys: mindshub_stafftools#` line, selects it. `argocd app +set` only adds parameters, so when nothing selects the console any more the +action unsets its two parameters, and removing the line turns the console off +on the next deploy. + It talks to ArgoCD over the **in-cluster Service** by default (`argo-cd-argocd-server.argocd.svc.cluster.local`), not over the public hostname. Every caller runs on `mdb-dev`, a runner pod in the same cluster, so diff --git a/argocd-pr-env-deploy/action.yml b/argocd-pr-env-deploy/action.yml index da55749..7d05410 100644 --- a/argocd-pr-env-deploy/action.yml +++ b/argocd-pr-env-deploy/action.yml @@ -15,6 +15,13 @@ # That repo ships a SAM stack, not an image: its pr-env-stacks workflow # reads the revision off the parent Application and deploys it, so there # is no ECR tag to check and no `tags.` to set. +# - The staff console is opt-in. A mindshub_stafftools anchor PR, or any +# `Deploys: mindshub_stafftools#...` line, sets its tag and revision, and +# the parent renders it only while both are set. `app set` only ever adds +# parameters, so when nothing selects the console any more the action +# unsets those two, and removing the line turns the console off on the +# next deploy. Values pinned in an argocd-envs branch are not parameters +# and stay. # - `Deploys: argocd-envs#N` is special-cased. argocd-envs holds the PR # env's own parent chart, so it has no image and no child Application. # The link moves the PARENT Application's source.targetRevision onto @@ -278,6 +285,22 @@ runs: esac } + # The staff console is opt-in: the parent renders it only while both + # tags.mindshub_stafftools and revisions.mindshub_stafftools are set. + # A mindshub_stafftools anchor or a `Deploys: mindshub_stafftools#...` + # line selects it. Args: anchor repo short name, PR body. + staff_console_selected() { + if [ "$1" = mindshub_stafftools ]; then + return 0 + fi + grep -qE 'Deploys:[[:space:]]+([^#[:space:]]+/)?mindshub_stafftools#([0-9]+|staging|main)' <<<"$2" + } + # The staff console parameters a parent Application carries, one per + # line. Arg: the parent's `argocd app get -o json`. + staff_console_params() { + jq -r '.spec.source.helm.parameters[]? | select(.name == "tags.mindshub_stafftools" or .name == "revisions.mindshub_stafftools") | .name' <<<"$1" + } + if is_revision_only_repo "$OWN_SHORT"; then set_args=(--helm-set "revisions.${OWN_SHORT}=${OWN_HEAD_SHA}") else @@ -576,6 +599,22 @@ runs: fi fi + # Turn the staff console off when nothing selects it any more. Only the + # two staff console parameters go: a parent can carry other parameters + # set by hand, which this must keep. A parent that does not exist yet + # has none to unset. + if [ -n "$PARENT_JSON" ] && ! staff_console_selected "$OWN_SHORT" "$PR_BODY"; then + unset_args=() + while IFS= read -r param; do + unset_args+=(-p "$param") + done < <(staff_console_params "$PARENT_JSON") + if [ "${#unset_args[@]}" -gt 0 ]; then + echo "argocd app unset ${PARENT_APP} ${unset_args[*]}" + "$ARGOCD_BIN" app unset "$PARENT_APP" "${ARGOCD_FLAGS[@]}" "${unset_args[@]}" + echo "Staff console turned off: no mindshub_stafftools anchor or Deploys line selects it." + fi + fi + # Retry `app set` directly until the parent App is set-able. Three # transient states are tolerated; anything else is fatal: # 1. App doesn't exist yet — ApplicationSet hasn't run its 60s diff --git a/tests/test_argocd_pr_env_deploy.py b/tests/test_argocd_pr_env_deploy.py index 6a1bc4d..605c89c 100644 --- a/tests/test_argocd_pr_env_deploy.py +++ b/tests/test_argocd_pr_env_deploy.py @@ -1,15 +1,22 @@ -"""Contract tests for the blocked-deploy comment in `argocd-pr-env-deploy/action.yml`. +"""Contract tests for `argocd-pr-env-deploy/action.yml`. When a `Deploys:` link cannot be resolved, the action refuses the deploy and comments on the PR. That comment is where the author learns how to retry, so it has to name a retry that keeps the env: removing the `deploy` label deletes the env and its database. The comment is rendered with the action's own `printf` line, run through bash, so the test reads the text authors see. + +The staff console is opt-in, and `app set` only ever adds parameters, so the +action unsets the two staff console parameters when nothing selects the +console any more. Those tests run the action's own shell against a fake +`argocd` that records its arguments. """ +import os import subprocess from pathlib import Path +import pytest import yaml ACTION_PATH = Path(__file__).resolve().parents[1] / "argocd-pr-env-deploy" / "action.yml" @@ -45,3 +52,113 @@ def test_warns_against_retrying_with_the_deploy_label(self): comment = unresolved_links_comment("mindsdb/api#5") assert "toggling the `deploy` label" not in comment assert "Do not remove the `deploy` label to retry. Removing it deletes this env" in comment + + +STAFF_FUNCTIONS_START = "staff_console_selected() {" +STAFF_FUNCTIONS_END = "\n\nif is_revision_only_repo" +STAFF_OFF_START = "# Turn the staff console off when nothing selects it any more." +STAFF_OFF_END = "\n\n# Retry `app set` directly" +APP_SET_CALL = '"$ARGOCD_BIN" app set "$PARENT_APP"' +PARENT_GET_CALL = '"$ARGOCD_BIN" app get "$PARENT_APP"' + + +def run_slice(start: str, end: str) -> str: + """The action's shell from the line starting with `start` up to `end`.""" + begin = RUN.index(start) + return RUN[begin : RUN.index(end, begin)] + + +def staff_console_selected(own_short: str, body: str) -> bool: + script = f'set -euo pipefail\n{run_slice(STAFF_FUNCTIONS_START, STAFF_FUNCTIONS_END)}\nstaff_console_selected "$1" "$2"' + return subprocess.run(["bash", "-c", script, "_", own_short, body], check=False).returncode == 0 + + +def argocd_calls(tmp_path: Path, own_short: str, body: str, parent_json: str) -> list[str]: + """Run the switch-off block and return each `argocd` call it made.""" + calls = tmp_path / "calls" + fake = tmp_path / "argocd" + fake.write_text('#!/usr/bin/env bash\nprintf "%s\\n" "$*" >>"$CALLS"\n', encoding="utf-8") + fake.chmod(0o755) + script = "\n".join( + [ + "set -euo pipefail", + "ARGOCD_FLAGS=(--grpc-web --plaintext)", + run_slice(STAFF_FUNCTIONS_START, STAFF_FUNCTIONS_END), + run_slice(STAFF_OFF_START, STAFF_OFF_END), + ] + ) + env = { + **os.environ, + "CALLS": str(calls), + "ARGOCD_BIN": str(fake), + "PARENT_APP": "pr-mindshub-frontend-7", + "OWN_SHORT": own_short, + "PR_BODY": body, + "PARENT_JSON": parent_json, + } + subprocess.run(["bash", "-c", script], env=env, check=True) + return calls.read_text(encoding="utf-8").splitlines() if calls.exists() else [] + + +def parent_with(*names: str) -> str: + params = ",".join(f'{{"name": "{name}", "value": "x"}}' for name in names) + return f'{{"spec": {{"source": {{"helm": {{"parameters": [{params}]}}}}}}}}' + + +class TestStaffConsoleSelection: + def test_a_stafftools_anchor_selects_its_own_console(self): + assert staff_console_selected("mindshub_stafftools", "") + + @pytest.mark.parametrize( + "line", + [ + "Deploys: mindshub_stafftools#staging", + "Deploys: mindshub_stafftools#main", + "Deploys: mindshub_stafftools#12", + "Deploys: mindsdb/mindshub_stafftools#12", + ], + ) + def test_a_deploys_line_selects_it(self, line): + assert staff_console_selected("mindshub_frontend", f"Some text\n{line}\nDeploys: auth#5") + + @pytest.mark.parametrize( + "body", + [ + "", + "Deploys: auth#5", + "Follow-up to mindshub_stafftools#12", + "Deploys: other_mindshub_stafftools#1", + "Deploys: mindshub_stafftools_v2#1", + ], + ) + def test_nothing_else_selects_it(self, body): + assert not staff_console_selected("mindshub_frontend", body) + + +class TestStaffConsoleSwitchOff: + def test_unsets_only_the_two_staff_console_parameters(self, tmp_path): + parent = parent_with( + "tags.auth", "tags.mindshub_stafftools", "revisions.mindshub_stafftools", "revisions.mindshub_services" + ) + assert argocd_calls(tmp_path, "mindshub_frontend", "Deploys: auth#5", parent) == [ + "app unset pr-mindshub-frontend-7 --grpc-web --plaintext" + " -p tags.mindshub_stafftools -p revisions.mindshub_stafftools" + ] + + def test_leaves_a_parent_without_staff_console_parameters_alone(self, tmp_path): + # A customer env pins stafftools in its argocd-envs branch's values, + # not as parameters, so its console stays. + parent = parent_with("tags.mindshub_frontend", "revisions.mindshub_services") + assert argocd_calls(tmp_path, "mindshub_frontend", "Deploys: argocd-envs#34", parent) == [] + + def test_keeps_a_selected_console(self, tmp_path): + parent = parent_with("tags.mindshub_stafftools", "revisions.mindshub_stafftools") + body = "Deploys: mindshub_stafftools#staging" + assert argocd_calls(tmp_path, "mindshub_frontend", body, parent) == [] + assert argocd_calls(tmp_path, "mindshub_stafftools", "", parent) == [] + + def test_does_nothing_before_the_parent_exists(self, tmp_path): + assert argocd_calls(tmp_path, "mindshub_frontend", "", "") == [] + + def test_runs_after_reading_the_parent_and_before_app_set(self): + assert RUN.index(PARENT_GET_CALL) < RUN.index(STAFF_OFF_START) < RUN.index(APP_SET_CALL) From f1389c34dba33f25bea04e18ed047bc91d7f2be0 Mon Sep 17 00:00:00 2001 From: Lucas Koontz Date: Sun, 1 Nov 2026 01:58:20 -0800 Subject: [PATCH 2/2] refactor(argocd-pr-env-deploy): select the staff console from the resolved Deploys links (ENG-3872) --- argocd-pr-env-deploy/action.yml | 19 ++++--- tests/test_argocd_pr_env_deploy.py | 84 +++++++++++++++++++++++------- 2 files changed, 77 insertions(+), 26 deletions(-) diff --git a/argocd-pr-env-deploy/action.yml b/argocd-pr-env-deploy/action.yml index 7d05410..feeedbf 100644 --- a/argocd-pr-env-deploy/action.yml +++ b/argocd-pr-env-deploy/action.yml @@ -287,13 +287,18 @@ runs: # The staff console is opt-in: the parent renders it only while both # tags.mindshub_stafftools and revisions.mindshub_stafftools are set. - # A mindshub_stafftools anchor or a `Deploys: mindshub_stafftools#...` - # line selects it. Args: anchor repo short name, PR body. + # It is selected when this run sets its revision: a mindshub_stafftools + # anchor, or a `Deploys: mindshub_stafftools#...` line the link loop + # below resolved. Reading set_args keeps that loop the one parser of + # `Deploys:` lines. staff_console_selected() { - if [ "$1" = mindshub_stafftools ]; then - return 0 - fi - grep -qE 'Deploys:[[:space:]]+([^#[:space:]]+/)?mindshub_stafftools#([0-9]+|staging|main)' <<<"$2" + local arg + for arg in "${set_args[@]}"; do + case "$arg" in + revisions.mindshub_stafftools=*) return 0 ;; + esac + done + return 1 } # The staff console parameters a parent Application carries, one per # line. Arg: the parent's `argocd app get -o json`. @@ -603,7 +608,7 @@ runs: # two staff console parameters go: a parent can carry other parameters # set by hand, which this must keep. A parent that does not exist yet # has none to unset. - if [ -n "$PARENT_JSON" ] && ! staff_console_selected "$OWN_SHORT" "$PR_BODY"; then + if [ -n "$PARENT_JSON" ] && ! staff_console_selected; then unset_args=() while IFS= read -r param; do unset_args+=(-p "$param") diff --git a/tests/test_argocd_pr_env_deploy.py b/tests/test_argocd_pr_env_deploy.py index 605c89c..4b8fcde 100644 --- a/tests/test_argocd_pr_env_deploy.py +++ b/tests/test_argocd_pr_env_deploy.py @@ -8,7 +8,8 @@ The staff console is opt-in, and `app set` only ever adds parameters, so the action unsets the two staff console parameters when nothing selects the -console any more. Those tests run the action's own shell against a fake +console any more. Those tests run the action's own shell, from the anchor's +parameters through the `Deploys:` link loop, with GitHub stubbed and a fake `argocd` that records its arguments. """ @@ -54,13 +55,29 @@ def test_warns_against_retrying_with_the_deploy_label(self): assert "Do not remove the `deploy` label to retry. Removing it deletes this env" in comment -STAFF_FUNCTIONS_START = "staff_console_selected() {" -STAFF_FUNCTIONS_END = "\n\nif is_revision_only_repo" +# The action runs under the runner's bash 5. macOS's /bin/bash 3.2 misparses the +# `case` patterns inside the link loop's process substitution, so the tests that +# run that loop need bash 4 or later. +BASH_MAJOR = int( + subprocess.run(["bash", "-c", "echo ${BASH_VERSINFO[0]}"], capture_output=True, text=True, check=True).stdout +) +NEEDS_RUNNER_BASH = pytest.mark.skipif(BASH_MAJOR < 4, reason="the action's link loop needs bash 4+, as on the runner") + +STAFF_FUNCTIONS_START = "is_revision_only_repo() {" +LINKS_START = 'if is_revision_only_repo "$OWN_SHORT"; then' +LINKS_END = "\n\n# Post a comment on the anchor PR" STAFF_OFF_START = "# Turn the staff console off when nothing selects it any more." STAFF_OFF_END = "\n\n# Retry `app set` directly" APP_SET_CALL = '"$ARGOCD_BIN" app set "$PARENT_APP"' PARENT_GET_CALL = '"$ARGOCD_BIN" app get "$PARENT_APP"' +# Every linked PR resolves, and an argocd-envs link resolves to a branch on +# argocd-envs itself, so the link loop runs without GitHub. +GH_API_STUB = ( + "gh_api() { printf '%s' '{\"merge_commit_sha\": \"m\", \"head\": {\"sha\": \"h\", \"ref\": \"b\", " + "\"repo\": {\"full_name\": \"mindsdb/argocd-envs\"}}}'; }" +) + def run_slice(start: str, end: str) -> str: """The action's shell from the line starting with `start` up to `end`.""" @@ -68,9 +85,43 @@ def run_slice(start: str, end: str) -> str: return RUN[begin : RUN.index(end, begin)] +def deploy_script(*tail: str) -> str: + """The action's own parameters and `Deploys:` link loop, then `tail`. + + GitHub is stubbed and the ECR checks are off, so `set_args` comes out of + the action's own parser exactly as a deploy would build it. + """ + return "\n".join( + [ + "set -euo pipefail", + GH_API_STUB, + "ecr_tag_exists() { return 0; }", + "ECR_CHECKS=off", + run_slice(STAFF_FUNCTIONS_START, LINKS_START), + run_slice(LINKS_START, LINKS_END), + *tail, + ] + ) + + +def deploy_env(own_short: str, body: str) -> dict[str, str]: + return { + **os.environ, + "OWN_REPO": f"mindsdb/{own_short}", + "OWN_SHORT": own_short, + "OWN_MERGE_SHA": "merge", + "OWN_HEAD_SHA": "head", + "PR_BODY": body, + } + + def staff_console_selected(own_short: str, body: str) -> bool: - script = f'set -euo pipefail\n{run_slice(STAFF_FUNCTIONS_START, STAFF_FUNCTIONS_END)}\nstaff_console_selected "$1" "$2"' - return subprocess.run(["bash", "-c", script, "_", own_short, body], check=False).returncode == 0 + # check=True, so a harness error fails the test rather than reading as "not selected". + script = deploy_script("if staff_console_selected; then echo selected; else echo off; fi") + run = subprocess.run( + ["bash", "-c", script], env=deploy_env(own_short, body), capture_output=True, text=True, check=True + ) + return run.stdout.splitlines()[-1] == "selected" def argocd_calls(tmp_path: Path, own_short: str, body: str, parent_json: str) -> list[str]: @@ -79,21 +130,12 @@ def argocd_calls(tmp_path: Path, own_short: str, body: str, parent_json: str) -> fake = tmp_path / "argocd" fake.write_text('#!/usr/bin/env bash\nprintf "%s\\n" "$*" >>"$CALLS"\n', encoding="utf-8") fake.chmod(0o755) - script = "\n".join( - [ - "set -euo pipefail", - "ARGOCD_FLAGS=(--grpc-web --plaintext)", - run_slice(STAFF_FUNCTIONS_START, STAFF_FUNCTIONS_END), - run_slice(STAFF_OFF_START, STAFF_OFF_END), - ] - ) + script = deploy_script("ARGOCD_FLAGS=(--grpc-web --plaintext)", run_slice(STAFF_OFF_START, STAFF_OFF_END)) env = { - **os.environ, + **deploy_env(own_short, body), "CALLS": str(calls), "ARGOCD_BIN": str(fake), "PARENT_APP": "pr-mindshub-frontend-7", - "OWN_SHORT": own_short, - "PR_BODY": body, "PARENT_JSON": parent_json, } subprocess.run(["bash", "-c", script], env=env, check=True) @@ -105,6 +147,7 @@ def parent_with(*names: str) -> str: return f'{{"spec": {{"source": {{"helm": {{"parameters": [{params}]}}}}}}}}' +@NEEDS_RUNNER_BASH class TestStaffConsoleSelection: def test_a_stafftools_anchor_selects_its_own_console(self): assert staff_console_selected("mindshub_stafftools", "") @@ -126,6 +169,7 @@ def test_a_deploys_line_selects_it(self, line): [ "", "Deploys: auth#5", + "Deploys: argocd-envs#5", "Follow-up to mindshub_stafftools#12", "Deploys: other_mindshub_stafftools#1", "Deploys: mindshub_stafftools_v2#1", @@ -135,6 +179,7 @@ def test_nothing_else_selects_it(self, body): assert not staff_console_selected("mindshub_frontend", body) +@NEEDS_RUNNER_BASH class TestStaffConsoleSwitchOff: def test_unsets_only_the_two_staff_console_parameters(self, tmp_path): parent = parent_with( @@ -149,7 +194,7 @@ def test_leaves_a_parent_without_staff_console_parameters_alone(self, tmp_path): # A customer env pins stafftools in its argocd-envs branch's values, # not as parameters, so its console stays. parent = parent_with("tags.mindshub_frontend", "revisions.mindshub_services") - assert argocd_calls(tmp_path, "mindshub_frontend", "Deploys: argocd-envs#34", parent) == [] + assert argocd_calls(tmp_path, "mindshub_frontend", "Deploys: argocd-envs#5", parent) == [] def test_keeps_a_selected_console(self, tmp_path): parent = parent_with("tags.mindshub_stafftools", "revisions.mindshub_stafftools") @@ -160,5 +205,6 @@ def test_keeps_a_selected_console(self, tmp_path): def test_does_nothing_before_the_parent_exists(self, tmp_path): assert argocd_calls(tmp_path, "mindshub_frontend", "", "") == [] - def test_runs_after_reading_the_parent_and_before_app_set(self): - assert RUN.index(PARENT_GET_CALL) < RUN.index(STAFF_OFF_START) < RUN.index(APP_SET_CALL) + +def test_the_switch_off_runs_after_reading_the_parent_and_before_app_set(): + assert RUN.index(PARENT_GET_CALL) < RUN.index(STAFF_OFF_START) < RUN.index(APP_SET_CALL)