chore(main): release 0.71.0 - #1
Open
github-actions[bot] wants to merge 1 commit into
Open
Conversation
github-actions
Bot
force-pushed
the
release-please--branches--main
branch
4 times, most recently
from
March 29, 2026 12:41
17e9c8d to
411220f
Compare
github-actions
Bot
force-pushed
the
release-please--branches--main
branch
from
April 1, 2026 20:49
411220f to
7c81562
Compare
github-actions
Bot
force-pushed
the
release-please--branches--main
branch
from
April 2, 2026 21:52
7c81562 to
472599e
Compare
github-actions
Bot
force-pushed
the
release-please--branches--main
branch
from
April 11, 2026 10:08
472599e to
ee2b94f
Compare
github-actions
Bot
force-pushed
the
release-please--branches--main
branch
2 times, most recently
from
April 23, 2026 18:36
cb0b86b to
3026ff3
Compare
rlei-odes
pushed a commit
that referenced
this pull request
May 1, 2026
…r sync; fix flow-sync targetNamespace Live SSH probing of the deployed stack (commit 4a012b1, run 24951698827) surfaced THREE distinct architectural defects in the Kestra-bootstrap block that all stem from PR stefanko-ch#486 making wrong assumptions about Kestra OSS v1.0: 1. `/api/v1/secrets/system/<NAME>` PUT → **HTTP 404**. Kestra OSS does not have a writable runtime secret API. The `/api/v1/namespaces/<ns>/secrets` GET endpoint exists but reports `"readOnly": true`. The only secret-feed in OSS is the `EnvVarSecretProvider` which reads `SECRET_<NAME>=<base64-value>` environment variables at container start. Both the standalone GITEA_TOKEN PUT block AND the bulk Infisical→Kestra PUT loop in PR stefanko-ch#486 have therefore NEVER worked — they returned 404 silently while the FAILED counter stayed at 0 because the prerequisite folder-discovery step also no-op'd (see #3). 2. `system.flow-sync` registration → **HTTP 422**. Direct API probe response: "Invalid entity: sync.targetNamespace: must not be null" Kestra v1.0's SyncFlows plugin requires `targetNamespace` explicitly. The PR stefanko-ch#486 design assumed the namespace would be derived from the subdir layout, which is not how the plugin works. 3. The Infisical→Kestra bulk sync loop **silent no-op**. Even if #1 were fixed, the loop body never ran because `jq` is not installed on the Hetzner VM. The bash-s heredoc executed `jq -r '.folders[]?.name' "$FOLDERS_BODY" 2>/dev/null` on the remote; jq is missing → "command not found" → `2>/dev/null` swallows the error → `|| echo ""` makes FOLDER_LIST empty → zero loop iterations → Pushed=0 / Failed=0 / FetchFailed=0 (which our existing assertions read as "all good"). This commit replaces the broken architecture wholesale: - **scripts/deploy.sh** — the standalone GITEA_TOKEN PUT block and the bulk Infisical→Kestra API-PUT block (the entire `bash -s` heredoc that wrote to /api/v1/secrets/...) are removed. New block: build SECRET_<NAME>=<base64-value> lines on the runner (jq is available there) by reading every Infisical folder + root path, plus a special-case SECRET_GITEA_TOKEN entry (the Gitea token is generated post-Gitea-start and isn't in Infisical at build_folder() time). Lines are written to a delimited block in /opt/docker-server/stacks/kestra/.env on the server, then Kestra is force-recreated so the env-var-based secret provider picks them up. Re-wait for Kestra ready (auth-aware loop) before registering flows. - **scripts/deploy.sh** SyncFlows YAML — adds `targetNamespace: tutorials` (required) and `includeChildNamespaces: true` so subdirs under `kestra/flows/` extend the namespace (`kestra/flows/sub1/x.yaml` → `tutorials.sub1`). - **examples/workspace-seeds/kestra/flows/tutorials/r2-taxi-pipeline.yaml** → flat to **examples/workspace-seeds/kestra/flows/r2-taxi-pipeline.yaml** to match the SyncFlows targetNamespace + includeChildNamespaces semantics. Internal description-block path updated. - **CLAUDE.md / examples/README.md** — diagram and table updated for the flat layout. Diagram now shows `kestra/flows/r2-taxi-pipeline.yaml` with an explanatory paragraph on the SyncFlows config that maps it into namespace `tutorials`. Operator-visible changes: - One Kestra cold-restart added per spin-up (~2–4 min). The restart is unavoidable because Kestra reads SECRET_* only at process startup, not on signal-reload. - `Configuring Kestra Git sync...` now shows secondary progress lines: `Building Kestra secret env from Infisical...`, `Wrote N Kestra SECRET_* env-vars to .env (skipped M invalid keys)`, `Restarting Kestra to load secrets...`, `Waiting for Kestra to come back up...` (with the 10-iteration liveness output and HTTP-status snapshots). - Final outcome line: `✓ Seeded flow tutorials.r2-taxi-pipeline registered in Kestra` (or a yellow warning naming the failure mode if it's not).
github-actions
Bot
force-pushed
the
release-please--branches--main
branch
from
May 1, 2026 14:32
3026ff3 to
c665456
Compare
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
Three Copilot findings, all addressed. One was a real R4-violation that
my round-1 docstring had handwaved away.
1. R4 violation — base64 in argv (real bug):
The push script piped `printf '%s' '<b64>' | base64 -d | docker
exec -i ...`. The b64 string was a positional arg to printf, so
`ps -ef` on the nexus host during the brief exec window would
expose it — and b64 of S3 secret keys is trivially decodable to
the secrets themselves. My round-1 reasoning ("argv to printf is
ok because the *decoded* secret never reaches docker argv") was
wrong: argv is argv, regardless of which command in the pipeline
carries it.
Fix: switched to heredoc form. The b64 travels via `cat <<'NEXUS_
FS_PUSH_EOF' | base64 -d | docker exec -i ...` — bash writes the
heredoc body directly to cat's stdin; no fork in the pipeline
carries the secret in argv visible to remote `ps -ef`. Single-quoted
delimiter disables variable expansion. Two-tier validation: the
existing base64-alphabet regex stays the primary guard; an
additional "delimiter not in payload" check is defence-in-depth
against a future alphabet-rule widening that could allow
collision (currently unreachable since the b64 alphabet excludes
underscores; marked with pragma: no cover).
2. pipefail not enabled (correctness drift):
Without `set -o pipefail`, a base64 -d failure (corrupt input,
missing binary) would be masked by docker exec's exit status —
silent empty-write to config.json while reporting WRITE_OK=true.
Now `set -o pipefail` is the second line of the rendered script,
and the pipeline's exit status is captured into $WRITE_RC for the
subsequent if-check.
3. Test docstring drift (cosmetic, fixed by #1):
`test_render_filestash_push_script_uses_stdin_for_b64` claimed
"via stdin to base64 -d" but tested the printf-argv form. Renamed
to `test_render_filestash_push_script_uses_heredoc_not_argv_for_b64`
and rewrote the assertion to scan ALL lines: any line containing
the b64 must equal exactly the b64 (i.e. the heredoc-body line),
not a printf/echo line that would leak it to argv.
Two new tests:
- `test_render_filestash_push_script_pipefail_enabled` — pins #2
- The renamed `_uses_heredoc_not_argv_for_b64` — pins #1 with a
much stricter assertion (per-line scan, not just substring).
Coverage: 99.25%, 401 tests passing.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
Round 2 Copilot findings on PR stefanko-ch#530 (6 comments): - R2 #1: Wetty ssh-agent start now validates the agent is responsive (socket present + 'ssh-add -l' rc != 2) before setting AGENT_STARTED=1. A silent ssh-agent failure no longer falsely reports success. - R2 #2a: Wetty ssh-add wrapped in 'if ... ; then KEY_ADDED=1' (no unconditional set) so a real ssh-add failure leaves KEY_ADDED=0 in RESULT_WETTY for operator visibility. - R2 #2b: Wetty .env append wrapped in 'if printf ... ; then' with fail-fast emitting auth_sock_written=0 on write failure (legacy unconditional append + AUTH_SOCK_WROTE=1 lied about success). - R2 #3: Wetty inline step comments renumbered 2-6 to align with the docstring framing (1 = mkdir/chmod precondition; 2-6 produce flags). - R2 #4: _infisical_provision_admin returns rc=1 when ProvisionResult has no usable credentials (token / project_id dropped, e.g. invalid base64). Avoids deploy.sh printing 'OK' while eval'ing empty INFISICAL_TOKEN= / PROJECT_ID= lines. - R2 #5: render_pg_ducklake_hook docstring now says '30s wall-clock' to match the SECONDS-bounded loop (was '15 iterations of 2s'). - R2 #6: _setup_wetty_ssh_agent returns rc=1 when auth_sock_written is False — surfaces the fail-fast paths to deploy.sh as a soft failure even when the script returns a parseable RESULT line. Regression tests added: - test_render_wetty_agent_script_validates_agent_responsiveness - test_render_wetty_agent_script_fail_fast_on_env_append_failure - test_render_wetty_agent_script_ssh_add_failure_leaves_key_added_zero - test_render_wetty_agent_script_step_numbering_aligns_with_docstring - test_cli_setup_wetty_ssh_agent_returns_1_when_auth_sock_not_written - test_cli_setup_wetty_ssh_agent_happy_path_returns_0 - test_cli_infisical_provision_admin_returns_1_when_creds_dropped - test_cli_infisical_provision_admin_returns_0_with_full_creds Suite: 1015 passed (was 1007), mypy --strict clean, ruff clean.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
Round 3 Copilot findings on PR stefanko-ch#530 (4 comments): - R3 #1 (init-r2-state.sh bearer header in arglist): dismissed — out of scope. The whole script (12+ instances pre-dating this PR) uses 'curl -H Authorization: Bearer ...'. Migrating the file-wide pattern to mode-600 'curl --config' tempfile is a separate refactor; this PR's changes (pre-cleanup loop + per_page=100 fix) follow the established convention. To be tracked as a follow-up issue. - R3 #2 (deploy.sh:452 stderr message): correctness drift. Reword rc=1 path from 'produced no parseable result' → 'soft-failed'. After R2 #6 the rc=1 path also covers auth_sock_written=0. - R3 #3 (r2-tokens CLI docstring): correctness drift. Docstring said both 'rc=0 even when per-token failures' AND 'rc=1 with at least one delete failure'. Implementation does the latter ('return 0 if result.is_success else 1'); docstring rewritten to match. - R3 #4 (Wetty .pub guard): real bug edge case. The keygen-skip gate only checked $KEY_PATH; if .pub was missing/corrupted (manual cleanup, partial write, fs corruption), keygen would skip but later 'cat $KEY_PATH.pub' would yield empty PUBKEY, silently no-oping the authorized_keys append. Now the gate is OR-of-missing AND we 'rm -f' both files first if half-present (ssh-keygen refuses to overwrite an existing private key). Regression test added: - test_render_wetty_agent_script_regenerates_on_half_present_keypair Suite: 1016 passed (was 1015), mypy --strict clean, ruff clean, bash -n scripts/deploy.sh OK.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
Round 4 Copilot findings on PR stefanko-ch#530 (4 comments): - R4 #1 (setup_wetty_ssh_agent check=False masks transport failure): real bug. Wrapper ran ssh.run_script with check=False, so an SSH transport failure (rc=255, connection drop) would fall through to parse_wetty_agent_result → None → CLI rc=1 (soft fail), violating the documented rc=2 'transport failure' contract. Switched to check=True so CalledProcessError propagates to the CLI handler's rc=2 branch. Safe because the rendered script ALWAYS terminates with exit 0 (fail-fast paths emit a parseable RESULT line + exit 0 before bailing). - R4 #2 (deploy.sh:452 wording 'produced no parseable result'): already fixed in R3 commit 9952233 → 'soft-failed — see stderr above'. Copilot reviewed against the prior head; reply points to the existing fix. - R4 #3 + R4 #4 (init-r2-state.sh new pre-cleanup curl Authorization headers in argv): same theme as R3 #1 — file-wide refactor pattern. The whole script (12+ pre-existing instances) uses the same form; applying mode-600 'curl --config' to ONLY my 2 new additions while leaving 11 existing ones unchanged would be inconsistent. Will be tracked as a dedicated security-hardening follow-up. Regression test added: - test_setup_wetty_ssh_agent_propagates_transport_failure Suite: 1017 passed (was 1016), mypy --strict clean, ruff clean.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
Round 5 Copilot findings on PR stefanko-ch#530 (3 comments): - R5 #1 (test_secret_sync.py:784 grep assertion mismatch): dismissed — Copilot misread the assertion. The trailing " in the Python single- quoted string literal is the bash double-quote closer, not part of the regex. Renderer at secret_sync.py:406 emits 'grep -qE "^${KEY_PREFIX}GITEA_TOKEN="' — exactly what the test asserts; tests pass. - R5 #2 (Wetty authorized_keys substring vs full-line match): real bug. Comment claimed 'full-line match' but 'grep -qF "$PUBKEY"' is a substring grep — a longer existing line containing $PUBKEY as substring would false-positive (skip the append). Switched to 'grep -qFx' (-x = whole-line match) to match the documented invariant. Without this, an attacker's authorized_keys entry that happens to share a prefix with the wetty pubkey could prevent the legitimate pubkey from being appended on re-deploy. Existing R-pubkey-dedup test updated + asserts the -F-only form is gone. - R5 #3 (r2_tokens.py:141 'client_factory' comment vs 'client' parameter): correctness drift. Comment referenced a non-existent 'client_factory' arg; the actual injection seam is the 'client' parameter. Reworded the comment. Suite: 1017 passed, mypy --strict clean, ruff clean.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
Round 6 Copilot finding on PR stefanko-ch#530 (1 comment): - R6 #1 (parse_provision_result docstring): correctness drift. Docstring claimed unparseable RESULT is 'treated as transport- failure by the CLI dispatcher', but the actual chain is parse_provision_result returns None → provision_admin substitutes ProvisionResult(status='not-ready') → CLI maps to rc=1 (soft-fail, warn-and-continue), NOT rc=2. Reworded to spell out the full call chain + clarify that real transport failures (CalledProcessError / OSError) are caught BEFORE this parser and route to rc=2. Suite: 1017 passed, mypy --strict clean, ruff clean.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
Round 7 Copilot finding on PR stefanko-ch#530 (1 comment): - R7 #1 (setup_wetty_ssh_agent stderr-forwarding comment): correctness drift. Comment said 'Forward non-RESULT stderr lines' but the code iterates completed.stdout (SSHClient.run_script defaults to merge_stderr=True, so stderr is folded into stdout). Reworded to spell out the merge behaviour explicitly: the script's stderr ⚠ warnings are folded into stdout, the parseable RESULT_WETTY line is skipped, everything else is forwarded. Suite: 1017 passed, ruff/mypy clean.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
Round 8 Copilot findings on PR stefanko-ch#530 (3 comments): - R8 #1 (__main__.py:2511 'Available:' help text): correctness drift. Help text printed for an unknown command was missing the new subcommands added across this PR series — 'infisical provision-admin', 'r2-tokens list/cleanup', 'setup wetty-ssh-agent', and the kestra variant of 'secret-sync --stack'. Operators mistyping a command couldn't discover them. All four added with their env/arg shapes. - R8 #2 (services.py:1486 pg_ducklake failure hint): correctness drift. Hint said 'check container logs' but the failing 'docker exec ... psql ...' call has stdout/stderr → /dev/null, so the actual psql error never reaches docker logs OR the deploy log. Replaced the hint with the exact ssh + docker exec command the operator can re-run manually to surface the error (kept the /dev/null redirection in the production path so a credential leak in the SQL doesn't end up in CI logs). - R8 #3 (secret_sync.py:85 StackTarget docstring): correctness drift. Said jupyter/marimo write 'plaintext KEY=value lines' but the renderer emits dotenv-style 'KEY="<escaped-value>"' (sed-escaped + double-quoted). Reworded to spell out the actual on-disk format + clarify why kestra's base64 form skips quoting. Suite: 1017 passed, mypy --strict clean, ruff clean.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 1) Round 1 Copilot findings on PR stefanko-ch#531 (5 actionable + 1 dismissed): - R1 #1 (firewall.py stale-cleanup MISSING): real bug. When firewall_rules is empty (zero_entry mode) OR an operator removes a single service's rule from Tofu, the next deploy did NOT clean up the stale stacks/<svc>/docker-compose.firewall.yml files. stack-sync would still rsync them to the server and compose_runner would still '-f'-layer them on every 'docker compose up' — host port mappings persisted even though the operator had already removed them from Tofu. Legacy bash had the same hole; this PR plugs it. write_overrides() now walks stacks/*/docker-compose.firewall.yml and deletes any file NOT in the just-written set. RedPanda's rendered config/redpanda-firewall.yaml is also removed when RedPanda has no firewall ports. configure() always calls write_overrides() (even in zero_entry mode) so the cleanup pass runs unconditionally. New 'remove_stale=False' opt-out exists for the rare back-compat caller. 5 new regression tests covering remove-on-zero-entry, remove-on-rule-removed, redpanda-config-removal, no-stacks-dir short-circuit, OSError aggregation on unlink. - R1 #2 (Prefect manifest paths wrong): real bug. The seeded prefect.yaml referenced 'prefect/requirements.txt' and 'prefect/flows/...' but workspace seeds land under nexus_seeds/ prefix in the user's Gitea repo (per stefanko-ch#501). After git_clone the cwd is the repo root, so the actual paths are 'nexus_seeds/prefect/requirements.txt' and 'nexus_seeds/prefect/flows/nyc_green_taxi_pipeline.py'. Without this fix the Prefect deploy fails on first attempt because the paths don't resolve. Updated both the requirements_file and the entrypoint, plus the manifest's leading comment to spell out the seeded location and the 'cd nexus_seeds/prefect && prefect deploy' invocation pattern. - R1 #3 (Prefect R2 env vars unplumbed): real bug. The Green-Taxi flow does os.environ['R2_ENDPOINT'] / 'R2_ACCESS_KEY' / 'R2_SECRET_KEY' / 'R2_BUCKET' which raises KeyError on a stock install — _render_prefect previously only emitted PREFECT_DB_PASSWORD + PREFECT_UI_API_URL, and deploy.sh's secret-sync only targets Jupyter/Marimo. Extended _render_prefect to also write the four R2_* vars from c.r2_data_*; same pattern Jupyter uses for HETZNER_S3_*. Empty values are kept as empty strings so the flow's 'configure R2 first' check (operator-side) can detect the case and emit a clear error. - R1 #4 (http-fetch-to-r2 description inaccurate): correctness drift. Description claimed re-runs land in different keys but the hour-stamp granularity means same-hour reruns DO overwrite. Reworded to spell out 'idempotent within an hour, sliced into separate folders across hours' + pointer to the parallel-fetch seed's index-suffix pattern for true append behavior. - R1 #5 (parallel-http-fetch zero-padding claim wrong): correctness drift. Inline comment claimed taskrun.iteration was zero-padded for sort stability, but the actual key uses the raw value. Reworded to acknowledge it's NOT padded (acceptable for the default 3-URL teaching case), with a Pebble 'string.format' recipe showing how to switch to '%03d' if the operator extends the URL list past 9. - R1 #0 (CodeQL test_firewall.py URL substring sanitization): dismissed — false positive. The flagged line is a Python substring assertion in test code on hardcoded test data ('redpanda-kafka.example.com'), not URL sanitization in a request-handling code path. The string never participates in any URL parsing or routing decision; the assertion just verifies the template substitution wrote the expected output. Suite: 1061 passed (was 1053, +8 new firewall cleanup tests), firewall.py 100% coverage, mypy --strict clean, ruff clean.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
Round 2 Copilot findings on PR stefanko-ch#531 (6 actionable): - R2 #1 (prefect.yaml description references wrong path): real bug. Deployment description still said 'prefect/flows/...' but the seeded path is 'nexus_seeds/prefect/flows/...'. Updated to point users at the correct location with explicit nexus_seeds/ prefix. - R2 #2 (Marimo DuckDB seed assumes AWS_*): correctness drift. The 'where to go from here' section claimed DuckDB picks up AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY automatically — true in general, but Marimo's secret-sync uses HETZNER_S3_* naming. Replaced with explicit DuckDB SET statements (s3_endpoint, s3_access_key_id, s3_secret_access_key, s3_url_style) reading from os.environ['HETZNER_S3_*'], same pattern NYC_Taxi_Pipeline.py uses for the Spark side. - R2 #3 (parallel-fetch URL collision): real bug. Kestra's each_parallel keys per-iteration outputs by taskrun.value (the loop value), so duplicate URLs in 'inputs.urls' collide on outputs.download[<dup>]. Both download tasks write to the same key; second silently overwrites first. The R2 keys themselves stay distinct (idx<N>-<hash>), but both objects carry the second download's payload. Documented the uniqueness requirement explicitly in the flow description with a recovery suggestion (dedupe upstream OR add a leading dedup task). - R2 #4 (examples/README normative table missing prefect/): correctness drift. Added prefect/ to the diagram in R1 but didn't add a row to the 'Stick to these names' normative table below. Added entry documenting: prefect.yaml manifest location, requirements.txt run-time install, flows/ entrypoint files, the 'cd nexus_seeds/prefect && prefect deploy' invocation pattern. - R2 #5 (deploy.sh local cleanup doesn't propagate): real bug, and this one supersedes my R1 #1 fix. The Python firewall.write_overrides pass added in R1 only deletes LOCAL stale files. stack-sync earlier in deploy.sh uses 'rsync_to_remote' WITHOUT '--delete=True', so the equivalent stale files on the SERVER persist after a Tofu rule removal. compose_runner keeps '-f'-layering them on every 'docker compose up' — the host port mapping stays exposed even though Tofu was supposed to close it. Added an explicit ssh-side cleanup pass in deploy.sh AFTER firewall configure: build a sorted newline-separated list of expected '<svc>/docker-compose.firewall.yml' rel-paths locally + remotely, comm -23 to find orphans, ssh-rm each one. Also covers the rendered redpanda-firewall.yaml. - R2 #6 (prefect.yaml cd path wrong inside container): correctness drift. Manifest comment said 'cd nexus_seeds/prefect && prefect deploy' but inside the prefect-worker container the workspace repo is cloned at /flows/$REPO_NAME/ (not the user's local checkout layout). Reworded to spell out both invocation contexts: from a local checkout vs from inside the worker container. Suite: 1061 passed (no test changes for documentation fixes), bash -n scripts/deploy.sh OK, ruff/mypy clean.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
Round 3 Copilot findings on PR stefanko-ch#531 (5 actionable). Theme: my R1+R2 fixes had silent-failure paths that made the cleanup work look 'green' when it had partially failed — Copilot dug deeper into the exact code I changed and surfaced 3 real failure-mode bugs. - R3 #1 (firewall configure CLI zero-entry path swallowed write.failed): real bug. The CLI's zero-entry branch returned rc=0 BEFORE checking write.failed, so a failed stale-cleanup unlink (OSError on remote- rm or local-unlink) was silently ignored. The whole point of the R1 cleanup is to surface failures so the workflow doesn't finish green with stale firewall state. Now write.failed is logged AND triggers rc=1 even in zero-entry mode. Regression test added. - R3 #2 (deploy.sh FW_RC=1 was warn-and-continue): real bug. rc=1 from the Python firewall step means at least one write OR cleanup failed, which means the local + remote firewall state is now INCONSISTENT with what Tofu requested. Continuing into the copy/start phases would either leave a removed port still exposed (failed cleanup) or skip a newly-requested port (failed render), and the workflow would still finish green. Changed rc=1 from yellow-warn to RED-abort with explicit 'state is inconsistent with Tofu' message. - R3 #3 (deploy.sh orphan-removal failure was warn-and-continue): real bug, same theme as #2. The new orphan-cleanup ssh-rm pass in deploy.sh just printed a yellow warning on rm failure. But compose_runner picks up any docker-compose.firewall.yml on the remote, so a failed orphan rm leaves the host port exposed contrary to Tofu. Changed to: track per-orphan failures in a tempfile, abort the whole deploy with red error if any fail. - R3 #4 (Prefect flow consumes empty R2_* without upfront guard): correctness drift / latent bug. After R1 _render_prefect writes empty R2_* values when r2_data_* aren't configured (optional datalake). Without an upfront check, the first run crashes deep inside boto with a confusing 'invalid endpoint URL' / SSL error. Added a precondition check at flow entry that raises a clear RuntimeError with the exact Tofu fields to set + the workflow invocation to re-pick up the values. - R3 #5 (DuckDB seed cross-reference to Spark-side wrong): correctness drift. My R2 fix said 'see NYC_Taxi_Pipeline.py for the Spark-side pattern' but those SET s3_* statements there are for ITS OWN DuckDB bootstrap (parquet upload), not for the Spark read which uses s3a:// + hadoop-aws + Spark-stack settings. Fixed to clarify: NYC_Taxi_Pipeline uses the same DuckDB-side SET pattern for its bootstrap, then SWITCHES to s3a:// for Spark reads. Suite: 1062 passed (was 1061, +1 for the new zero-entry-cleanup- failure regression test), bash -n scripts/deploy.sh OK, mypy strict clean, ruff clean.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
Round 4 Copilot findings on PR stefanko-ch#531 (9 unique findings): - R4 #1 (firewall configure CLI zero-entry write.failed): already fixed in commit afd21de (R3). Copilot reviewed against the prior HEAD; the fix is already on the branch with regression test test_cli_firewall_configure_zero_entry_with_stale_cleanup_failure_returns_1. - R4 #2 (deploy.sh FW_RC=1 non-blocking): already fixed in commit afd21de (R3). The case branch now exits 1 with red 'state is inconsistent with Tofu' message instead of warn-and-continue. - R4 #3 (deploy.sh SSH listing failure silently produces empty REMOTE_FW_LIST): real bug. The original `|| true` form would swallow ssh failures (network blip, expired Cloudflare-Access token) and produce an empty list, making comm -23 produce no orphans, silently leaving stale firewall files on the server. Replaced with explicit rc capture: `REMOTE_FW_RC=0; ... || REMOTE_FW_RC=$?` + abort with red error if rc != 0. SSH 'connected, no files' remains rc=0 (legitimate empty list); SSH transport failure is now distinguished from it and surfaced. - R4 #4 (deploy.sh redpanda-firewall.yaml rm uses `|| true`): real bug, same theme. `rm -f` on the remote is idempotent for missing files, so a non-zero rc means SSH transport failed — not 'file already gone'. Replaced `|| true` with explicit conditional + abort. Without this, a transient SSH failure during the redpanda cleanup would leave the external-listener config in place and setup_redpanda_hook would keep advertising redpanda-kafka.<domain> while the firewall is closed. - R4 #5 (D1 schema seeds old defaults): correctness drift. control-plane/schema.sql:85-86 still seeded server_type='cax31' and server_location='fsn1' as initial config-row values. control-plane/functions/api/info.js:92-102 reads from D1 first with the Tofu values as a secondary fallback, so fresh control- plane deployments would display the OLD ARM defaults until a later spin-up overwrites the row. Updated to seed cx43/hel1. - R4 #6 (docs/stacks/overview.md says 'larger sizes available in the Control Plane configuration'): correctness drift. The Control Plane web UI displays SERVER_TYPE/SERVER_LOCATION but has no settings API to edit them. Updated the resource-sizing text to point at the correct override path: GitHub repository variables (Settings → Secrets and variables → Actions → Variables), with an explicit link to setup-guide.md's 'Optional Repository Variables' section. The Control Plane displays the value; it doesn't let users edit it. - R4 #7 (docs/stacks/marimo.md missing DuckDB seed entry): correctness drift. Added the new Getting_Started_DuckDB.py to the canonical Marimo docs page's quickstart section, alongside the PySpark + NYC_Taxi seeds. Each notebook gets a one-sentence summary explaining when to start there. - R4 stefanko-ch#8 (Prefect download_month docstring 'retries the task once' vs decorator retries=2): correctness drift. retries=2 produces TWO retries after the initial attempt = 3 total attempts. Reworded docstring to match: 'up to TWO times on transient HTTP errors — three total attempts including the initial one'. - R4 stefanko-ch#9 (no explicit R2_* test for _render_prefect): valid test-gap request. Added two tests: - test_render_prefect_emits_r2_credentials — populated R2 config → all four R2_* keys appear in stacks/prefect/.env with the expected values. - test_render_prefect_emits_empty_r2_when_unconfigured — empty r2_data_* fields → keys still present (so the seed flow's upfront precondition check sees them) but values empty. Suite: 1064 passed (was 1062, +2 prefect R2 tests), bash -n scripts/deploy.sh OK, mypy strict clean, ruff clean.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 2) R2 #1 — Rename misnamed orchestrator field stacks_dir → project_root and env var STACKS_DIR → PROJECT_ROOT. Phases derive project_root/stacks internally; the prior STACKS_DIR name collided with deploy.sh's existing STACKS_DIR=$PROJECT_ROOT/stacks convention and would have produced .../stacks/stacks/... paths when wired up in Phase 4b. R2 #2 — Plumb host through compose_runner.run_compose_up and infisical.provision_admin so a non-default SSH_HOST_ALIAS reaches every pre-bootstrap phase. Previously _phase_stack_sync honored ssh_host but _phase_compose_up + _phase_infisical_provision called helpers that hard-coded 'nexus' via _remote.ssh_run_script's default — different phases would talk to different hosts. R2 #3 — Clear self.infisical_token / self.project_id alongside the state mirrors at the top of run_pre_bootstrap. State guards the rc=1 stdout emission; self.fields guard the post-bootstrap phase gating (infisical-bootstrap, secret-sync) when the same instance is later passed to run_all. Regression test extended. R2 #4 — Update stale section comment that still referenced the dropped ssh-parameter pattern (R1 #2 removed it). R2 #5 — Update OrchestratorState comment to match current behavior: post-bootstrap phases gate on self.fields, not state. State stays as the canonical record for stdout emission; _phase_infisical_provision writes both.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 3) R3 #1 — _phase_service_env now uses self.gitea_repo_owner consistently (the required Orchestrator constructor field) instead of self.bootstrap_env.gitea_repo_owner. Eliminates the dual-source of truth: callers populating only the constructor field would previously have hit the workspace_coords_complete guard against a None bootstrap_env mirror and silently skipped the Gitea block append (or built a malformed http://gitea:3000/None/... URL). Now the orchestrator field is canonical.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 4) R4 #1 — Real bug. Plumb host through stack_sync.cleanup_disabled_stacks so rsync + cleanup target the same SSH alias. Previously the cleanup ran via _remote.ssh_run_script(s) without a host= override → with non-default SSH_HOST_ALIAS, rsync went to host A but the destructive cleanup loop ran on host B (default 'nexus'). Same class as R2 #2; cleanup path was missed. run_stack_sync now forwards its host kwarg to cleanup_disabled_stacks, and the orchestrator's _phase_stack_sync already passes host=self.ssh_host. R4 #2 — Correctness drift. Comment claimed the gitea workspace guard checks 'ALL six coords (repo_url, username, password, author_name, author_email, repo_name)', but the actual guard gates on 5 inputs (repo_owner, repo_name, username, password, email) and derives repo_url + author_name from those. Updated the comment to match the code.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 5) R5 #1 — Real bug. Reordered run_pre_bootstrap phases so firewall-configure runs BEFORE stack-sync (was: service-env → stack-sync → firewall-configure → compose-up; now: service-env → firewall-configure → stack-sync → compose-up). Previously the per-stack docker-compose.firewall.yml overrides were generated AFTER rsync had already pushed stacks/<svc>/ to the server, so compose-up would start containers without the new firewall exposure overrides on the server. Tests updated to assert the new order. Server-side orphan-cleanup of stale firewall overrides remains in deploy.sh's bash for Phase 4b. R5 #2 — Real bug. FIREWALL_RULES_JSON is now REQUIRED env (no '{}' default). The firewall module treats '{}' as intentional zero-entry mode and triggers destructive cleanup of existing override files. An accidentally empty value (forgot to set, tofu output failure) would have silently wiped overrides. Operators must now pass '{}' explicitly to opt into zero-entry mode. Added test_cli_run_pre_bootstrap_missing_firewall_rules_returns_2.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 6) R6 #1-3 — Correctness drift. After R5 phase reordering (firewall before stack-sync), three docstrings/help-text strings still listed the old order: - Orchestrator.run_pre_bootstrap docstring - _run_pre_bootstrap CLI handler docstring - main() help/usage string All three updated to match the actual order (service-env → firewall-configure → stack-sync → compose-up → infisical-provision). R6 #4 — Correctness drift. cleanup_disabled_stacks docstring claimed 'Returns None on transport failure or unparseable RESULT line', but the default runner (_remote.ssh_run_script with check=True) raises CalledProcessError/TimeoutExpired on transport failure rather than returning None. Updated docstring to clarify: Returns None only when RESULT is missing/unparseable; transport failures propagate as exceptions for the caller (orchestrator wraps with try/except → status='failed' PhaseResult).
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 7) R7 #1 — Correctness drift. _phase_compose_up docstring still said 'ssh nexus 'bash -s'' even after R2 #2 plumbed host through run_compose_up. Updated to 'ssh <host> 'bash -s'' with explicit note that <host> = self.ssh_host (default 'nexus', override via SSH_HOST_ALIAS). R7 #2 — Correctness drift. _phase_infisical_provision docstring claimed 'Read the constructor's project_id / infisical_token fields as fallback (so a post-bootstrap-only test can still bypass this phase)' — but the implementation has NO such read-fallback. The phase only WRITES to those fields on success. run_pre_bootstrap also clears them at start (R1 #4 + R2 #3). Updated docstring to describe the actual flow: writes to BOTH state mirrors AND self.fields on success; callers wanting to bypass the phase should use run_all directly with constructor-provided creds. R7 #3 — Correctness drift. Test comment for test_phase_service_env_skips_gitea_block_on_incomplete_coords described 'the original lax check' (a prior pre-R3 #1 implementation) which is no longer accurate now that self.gitea_repo_owner is the canonical source of truth. Rewrote the comment to reflect the current workspace_coords_complete check (5 inputs: repo_owner, repo_name, username, password, email) and which one the test deliberately leaves missing.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 8) R8 #1 — Correctness drift. test_phase_service_env_happy_path comment claimed the gitea-block append is skipped because bootstrap_env.gitea_repo_owner=None makes workspace_coords_complete fail. After R3 #1, the check uses self.gitea_repo_owner (set to 'admin' by the orchestrator fixture), so the bootstrap_env clear alone wouldn't trip the guard. Actual skip reason: orchestrator fixture leaves gitea_user_username/_password/_email at None, which fails the all() check. Comment rewritten to reflect this.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 1) R1 #1 — ❌ Wrong (dismissed). Docker Hub login uses double-quoted outer ssh argument, so $DOCKERHUB_TOKEN and $DOCKERHUB_USER DO expand locally. The single-quotes inside are literal chars (verified: 'echo "echo '$X' | docker login -u '$Y' …"' → 'echo "echo 'value' | docker login -u 'value' …"' which is what the remote sees and works correctly). Same pattern as legacy bash that's been working since Phase 1. R1 #2 — 🔴 Real bug (linked to R1 #3 fix). GITEA_USER_USERNAME wasn't passed to run-pre-bootstrap CLI in deploy.sh, but workspace-coords derives it. The downstream issue (R1 #3) was that the derived value didn't reach _phase_service_env's gate — fixed in R1 #3. Reply to R1 #2 explains the linked fix. R1 #3 — 🔴 Real bug. _phase_workspace_coords now also dual-writes self.gitea_user_username / _password / _email from the derived coords, not just state mirrors. In admin-fallback mode (no GITEA_USER_EMAIL / _PASS env), the legacy bash filled these from admin coords; the orchestrator must replicate that behavior so _phase_service_env's workspace-block-append guard passes. Otherwise git-integrated stacks silently miss GITEA_REPO_URL / GITEA_USERNAME etc. in their .env. Existing constructor values still win — tests can pre-seed alternative identities. Regression test added. R1 #4 — 🔴 Real bug. _phase_firewall_sync now explicitly checks local stacks/ dir is_dir() BEFORE computing the orphan list. Without the guard, a missing local stacks/ directory yields Path.glob → empty list → every remote *.firewall.yml treated as orphan and rm'd. That's catastrophic if project_root is mis-set or the checkout is incomplete. Now fails fast with explicit 'would rm every remote firewall override' message. Regression test added. R1 #5 — 🟠 Correctness drift. compose_restart's rendered remote script now suppresses docker compose's normal output (>/dev/null), captures stderr to a tempfile, and only emits it on failure (with 6-space indent for log readability). Matches legacy bash's '>/dev/null 2>&1 || true' semantics — restart loops produce minimal log output by default. Snapshot regenerated. R1 #6 — 🟠 Correctness drift. _parse_owner_repo docstring claimed 'Returns empty string on a URL that doesn't fit the pattern' but actually returns the input passed-through (with prefix/ query/fragment strips). Updated to match reality + cite the caller's regex check as the actual gate.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 2) R2 #1 — 🔴 Real bug. _phase_firewall_sync's orphan-rm script now prefixed with 'set -e' so the first failed rm propagates as the script's exit code. Without this, a permission-denied (or any non-zero) rm in the middle would be MASKED by a later success — script exits 0, phase reports ok, but stale firewall overrides remain on the server. That's a security-relevant inconsistency: the orphan-cleanup is what closes a host port after Tofu removes a firewall rule. set -e + check=True on the ssh wrapper now ensures any failure → CalledProcessError → status='failed' on the phase. R2 #2 — 🔴 Real bug. _phase_global_env now writes env_content via ssh-stdin streaming (subprocess.run with input=env_content) instead of an inline heredoc. The old approach (heredoc with a fixed 'NEXUS_GLOBAL_ENV_EOF' delimiter) was a heredoc- injection risk: any image-version value or USER_EMAIL whose content contained that exact delimiter on a line by itself would terminate the heredoc early, and the remaining content bytes would be interpreted as shell commands on the server. Streaming via stdin removes the delimiter entirely — the ssh command is just 'cat > <path>', env_content goes through stdin where bash treats it as opaque bytes (cat doesn't parse its stdin). New regression test: test_phase_global_env_no_heredoc_injection_with_malicious_image_value constructs an adversarial image value containing the old delimiter + 'rm -rf /' and asserts the content reaches stdin verbatim (NOT argv) and ssh_args contain no '<<' heredoc.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 3) R3 #1 — 🔴 Real bug. _phase_woodpecker_apply now uses 'docker compose --env-file <path> up -d' instead of 'source <path> && docker compose up -d'. Compose's --env-file directive parses KEY=VALUE without shell interpretation, so a malicious image-version value containing $() / backticks / ; / newlines cannot trigger remote command execution at source-time. (compose_runner.py still uses the legacy 'set -a; source ...' pattern; tracked for Phase 4c follow-up — out of scope for this PR.) R3 #2 — 🔴 Real bug. _phase_global_env now validates every value before write: the file is consumed by TWO mechanisms (compose_runner's 'set -a; source' + compose's 'env_file:' directive). The safe intersection is 'values without shell metacharacters'. The validation rejects values containing whitespace, $, backticks, ;, &, |, <, >, !, ?, *, [], {}, single/double quotes, or newlines/CRs. Image versions legitimately need alphanum + .-_:/@+ only — real values like 'treeverse/lakefs:1.73.0' or 'v1.2.3' pass; adversarial values like 'v1.0\nrm -rf /' or 'v1.$(rm -rf /)' fail with a clear status='failed' detail BEFORE any write. 3 new regression tests cover shell-unsafe rejection in image values, dollar-sign in values, and unsafe ADMIN_EMAIL. R3 #3 — 🟠 Correctness drift. compose_restart.run_restart's default runner now passes check=False to _remote.ssh_run_script. On ssh transport failure (network blip, expired CF Access token), the runner now returns a CompletedProcess with non-zero rc instead of raising CalledProcessError. The existing parse_result fallback (None → failed=len(services)) kicks in, the orchestrator converts that to status='partial', and the deploy continues — matching legacy bash's best-effort '|| true' restart-loop semantics. With check=True, an ssh blip would have aborted the entire post-bootstrap pipeline. R3 #4 — 🟠 Correctness drift. The R2 #2 test asserted safety that didn't actually hold — the .env is later sourced (R3 #1 + #2 above), so 'no heredoc injection' didn't mean 'no injection'. Replaced with 4 stronger tests that assert the new validation gate REJECTS unsafe values BEFORE writing the file (the actual safety property, not a partial one).
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 4) R4 #1 — 🟠 Correctness drift. _phase_kestra_secret_sync docstring claimed 'secret-sync still attempts on wait_ready timeout but may not succeed'. Actual behavior: returns partial immediately and skips run_sync_for_stack entirely (which is the correct semantic — pushing secrets to a Kestra whose basic-auth layer isn't ready would 401). Updated docstring to match: 'we then skip run_sync_for_stack entirely'. R4 #2 — 🔴 Real bug. _phase_firewall_sync's remote-listing command `cd … 2>/dev/null && ls … | sort || true` had `|| true` masking failure of BOTH cd AND ls. If the remote /opt/docker-server/stacks dir was missing (server FS issue or wrong project_root expectation), cd would fail, the inner ls would never run, but `|| true` swallowed the non-zero exit → remote_overrides looked empty → no orphans detected → phase reported ok despite stale firewall overrides remaining on the server (host port stays exposed). Now prefixed with set -e and bare cd so any failure propagates to ssh's exit code → check=True → CalledProcessError → status='failed'. The `|| true` is now scoped to ls's 'no matches' exit-1 only. R4 #3 — 🟠 Correctness drift. Comment above the redpanda chown_script said the chmod fallback was for 'sudo isn't available', but BOTH primary and fallback commands use sudo. Corrected the comment to reflect the actual intent: chown to 101:101 may fail on minimal hosts where uid 101 doesn't exist as a real user (some FS layouts disallow chown to a numeric uid that's not in /etc/passwd); chmod 777 is the fallback. Both paths use sudo because the nexus user can't chown/chmod files owned by 101:101 from a previous firewall-on deploy.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 5) R5 #1 — 🔴 Real bug. _phase_global_env now validates KEYS (not just values) for image_versions. A malicious / invalid image-versions key containing ';', whitespace, newlines, or other shell metacharacters would survive the dash→underscore normalization and produce an env-file line whose left-hand side breaks shell parsing OR (worse) becomes a command injection vector when the file is later sourced by compose_runner. The post-normalization name must now match ^[A-Z_][A-Z0-9_]*$ (POSIX shell variable name); image-version keys from image-versions.tfvars normalize cleanly under normal contributor edits but malicious / typo'd values fail with a clear status='failed' detail BEFORE write. 2 new regression tests cover the rejection. R5 #2+#3 — 🟡 Project-rule (consistency). Switched the two remaining `echo "$SECRETS_JSON" | …` invocations in deploy.sh (lines 335 + 405 — pre-bootstrap + run-all CLI calls) to `printf '%s' "$SECRETS_JSON" | …`. Matches the existing config dump-shell pattern at line 139 and the broader project convention (Phase 1-3 had migrated to printf for the echo-with-arbitrary-content edge cases). Removes the small chance of echo's flag/escape interpretation mangling JSON with backslash-escapes.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 6) R6 #1 — 🔴 Real bug. ENABLED_SERVICES_CSV used echo (always trailing newline) → empty service list → tr converts the lone newline to ',' → CSV becomes a single comma → downstream CLI's comma-split parser sees one empty service in the list. Switched to printf '%s' (no trailing newline). Empty list now stays empty. R6 #2 — 🟠 Correctness drift. _phase_mirror_finalize comment said 'only ALL-failed → partial' but the actual logic marks partial when EITHER (a) kestra-enabled-but-flow-not-triggered OR (b) any git-restart failure. Updated comment to describe the actual two-condition logic + clarify that flow_triggered=False is benign when kestra isn't enabled (the gate excludes that case).
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 7) R7 #1 — 🔴 Real bug. _phase_kestra_secret_sync (and existing _phase_secret_sync for jupyter/marimo) called _secret_sync.run_sync_for_stack() without forwarding self.ssh_host. The default ssh_run_script + ssh_run paths used host='nexus' regardless of SSH_HOST_ALIAS — secrets and the post-sync 'docker compose up -d' would target the wrong host on non-default deployments. Same class of bug as PR stefanko-ch#532 R2 #2 + R4 #1; the secret_sync module was the last unmigrated host=plumbing point. Fix: extended secret_sync.run_sync_for_stack with a host: str = 'nexus' kwarg that flows to the default ssh_run_script + ssh_run lambdas. Both orchestrator phases now pass host=self.ssh_host. Test fakes that monkeypatched _remote.ssh_run_script directly were updated to accept **kwargs (the new host= kwarg). R7 #2 — 🟠 Correctness drift. _phase_woodpecker_apply's catch-all for CalledProcessError + TimeoutExpired labeled both as 'transport (...)', but CalledProcessError from the docker compose ssh-run is a service-level failure (image pull failed, container crashed at startup, port collision, …) not a transport issue. Split into three distinct paths: (a) rsync transport ('rsync transport (CalledProcessError)') (b) docker compose up -d failure ('docker compose up -d failed (rc=N): <last stdout line>') — operator gets a captured tail from the compose error output for direct triage (c) genuine ssh transport timeout ('ssh transport timeout (TimeoutExpired)') Each maps to a distinct phase status='partial' detail so operators know whether to investigate ssh, rsync, or container logs.
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 8) R8 #1 — 🔴 Real bug, third iteration on the firewall-sync orphan listing (R1 #4 added the local guard, R4 #2 fixed the cd-vs-ls scoping, R8 #1 fixes the pipeline-error masking). Without 'set -o pipefail', the 'ls | sort' pipeline only reflects sort's exit code — a real ls error (permission denied, unexpected fs error) returns non-zero but sort still completes successfully, so the pipeline exits 0 and '|| true' is redundant. Real ls errors got swallowed → remote_overrides looked empty → phase reported ok despite stale overrides remaining. Switched to 'find -mindepth 2 -maxdepth 2 -name … -printf'. find returns 0 on the legitimate 'no matches' case (empty stdout) AND propagates non-zero on real errors (unreadable subdir, etc.). With 'set -euo pipefail', any failure in the 'find | sort' pipeline propagates to the script's exit code → check=True → CalledProcessError → status='failed'. No '|| true' anywhere — every error path now surfaces. cd is bare so a missing /opt/…/stacks still fails fast (R4 #2 guard preserved).
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…round 1) R1 #1 — 🔴 Real bug. _docker_hub_login interpolated dockerhub_user directly into the remote shell command via f-string. A DOCKERHUB_USER containing whitespace or shell metacharacters would have been parsed by the remote shell, opening a command-injection vector. Now wrapped with shlex.quote. R1 #2 — 🟠 Correctness drift. run_pipeline() docstring claimed rc=1 on partial; the CLI returns rc=0 (PR stefanko-ch#535 R0 fix). Updated the docstring to describe the actual contract: rc=0 covers both clean and partial; rc=2 only on hard failure. Added cross-reference to the spin-up.yml 'shell: bash -e' reason for the rc=0-on-partial choice. R1 #3 — 🟠 Correctness drift. tfvars.py module docstring claimed malformed/unquoted lines 'fail fast with clear error'; in fact parse() silently doesn't match those lines and returns the dataclass empty-string default. Tests pin the soft-skip behavior. Updated docstring to describe what the code actually does + cite where the empty-after-parse case is caught downstream. R1 #4 — 🔵 Pedantic (trivial fix). troubleshooting.md referenced the orchestrator's private '_phase_firewall_configure' symbol. Replaced with the user-visible 'firewall-configure phase' wording so the doc doesn't stale on internal renames. R1 #5 — 🟠 Correctness drift (same root as R1 #2). Updated _run_pipeline CLI docstring's exit-code section to match the new rc=0-on-partial contract. R1 #6 — 🟠 Correctness drift. Error handler in _run_pipeline only printed the exception class name, not the rc / stderr tail / message. CI failures were undiagnosable without a re-run with --debug. Now: subprocess.CalledProcessError prints rc + last 500 chars of stderr (or stdout), TimeoutExpired prints the timeout duration, SetupError prints its message directly, OSError + Exception print message. Still avoids printing exc.cmd (can carry secrets via env-var-prefixed argv forms).
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
…gate Codecov flagged 88.6% patch coverage; new tests bring pipeline.py to 96% and tfvars.py to 100% (overall 97%). No production-code changes — covers branches that were exercised only end-to-end before: - diagnose_state reason surfaced through PipelineError (R2 #2) - ConfigError → PipelineError wrap (dead-code branch today, but pinned via monkeypatched from_secrets_json so a future schema tightening doesn't silently break the wrap) - enabled_services not-a-list defensive guard - _b64_encode_ssh_key empty + non-empty (legacy bash newline-append) - _ssh_keygen_cleanup empty-target skip + TimeoutExpired suppress (R2 #1) - tfvars OSError-on-read wrapped in TfvarsError
rlei-odes
pushed a commit
that referenced
this pull request
May 10, 2026
- Required tofu outputs are now actually required (R4 #1). Previous safe-looking defaults were destructive in the partial-apply case: enabled_services=[] drives stack-sync to remove ALL remote stacks, firewall_rules={} puts firewall-configure into zero-entry mode (wiping existing per-stack overrides). state_list_ok() doesn't catch partial-apply (state file exists, specific outputs never populated). enabled_services / firewall_rules / ssh_service_token now lack default → TofuError on missing → wrapped to PipelineError with an actionable message. image_versions stays in the same required-list (defensive symmetry — no harm if it can't be empty). server_ip / persistent_volume_id stay optional (their empty values are non-destructive). - TofuError from load_r2_credentials now wrapped to PipelineError (R4 #2). Without the wrap a malformed .r2-credentials file showed as 'unexpected error (TofuError)' which is wrong — it's an obvious operator-actionable preflight failure. - TfvarsError from tfvars.parse same treatment (R4 #3). - mount_persistent_volume result now captured + emits a stderr warning when volume_id is non-zero but mounted=False (R4 #4). Mount failure stays non-fatal (downstream stacks that don't need /opt/data can still come up healthy), so we warn but don't raise. The warn-vs-silent path is gated on volume_id != '0' so an operator who hasn't configured a volume doesn't see noise. - 5 new unit tests pin: TfvarsError wrap, TofuError-on-creds wrap, required-output missing, volume-mount warning emitted, volume-mount warning suppressed when volume_id='0'. 1252 tests green.
github-actions
Bot
force-pushed
the
release-please--branches--main
branch
from
May 10, 2026 14:05
c665456 to
c7d93d5
Compare
rlei-odes
pushed a commit
that referenced
this pull request
May 22, 2026
…udflare R2 User-directed change: R2 makes more sense than Hetzner Object Storage for this use case because (a) the project already uses R2 for the Tofu state backend, the cloudflare/cloudflare provider is already wired up, and the R2 token already exists; (b) R2 is region-agnostic with zero egress fees, eliminating the EU-only- compute caveat that the previous Hetzner-OS revision needed; (c) Cloudflare's cloudflare_r2_bucket resource is a first-class Tofu resource — no parallel storage system to operate. Changes: - Title + tl;dr + motivation: storage provider is now Cloudflare R2. Historical context (Hetzner-OS was previously proposed) retained for reviewers tracking the revision. - Goal #1: removed the EU-only-compute default caveat. R2 has zero egress so non-EU compute (ash, hil, sin) is just as cheap to serve. Operators can keep `SERVER_PREFERENCES` EU-only for latency reasons but the architecture doesn't force it. - "What this is NOT": dropped the EU-only and "we stay on Hetzner for data-residency" framing. Added clarification on R2's `EU` jurisdiction hint for callers who do want a stricter geography pin. - Storage layout diagram: Cloudflare R2 (EU jurisdiction) header. - Tofu code-changes row: cloudflare_r2_bucket (not minio_s3_bucket). No new provider plumbing — cloudflare/ cloudflare is already configured. - Decision Points #1, #3, #5: revised. #1 names R2 explicitly. #3 names cloudflare_r2_bucket. #5 names R2's built-in versioning + lifecycle. - Cost analysis: rewrote for R2 pricing. Free tier (10 GB storage + 1M Class A + 10M Class B ops) covers small classes entirely. Above-free-tier is $0.015/GB storage + free egress. Honest comparison vs Hetzner-OS — R2 is more expensive per GB but the zero-egress advantage dominates the moment any non-EU compute pull happens. - Risks table: removed the "R2 might be better" self-reflection row (resolved by this revision). Added "R2 ops free-tier exhaustion" as the new structural risk to monitor. - Open Question #5: resolved with cloudflare_r2_bucket pattern. - Appendix B (rclone config): updated to Cloudflare R2 form (provider = Cloudflare, region = auto, endpoint = https://<account_id>.r2.cloudflarestorage.com).
github-actions
Bot
force-pushed
the
release-please--branches--main
branch
2 times, most recently
from
May 23, 2026 07:47
c6514c4 to
0ba66cf
Compare
github-actions
Bot
force-pushed
the
release-please--branches--main
branch
4 times, most recently
from
May 27, 2026 16:45
a738fd2 to
e0f6dfe
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This release is too large to preview in the pull request body. View the full release notes here: https://github.com/rlei-odes/Nexus-Stack/blob/release-please--branches--main--release-notes/release-notes.md