Skip to content

Commit 3ea7531

Browse files
myasnikovdaniilclaude
myasnikovdaniil
and
claude
committed
fix(ci): fork e2e — Chainsaw+TIA run and image-push hardening
The fork e2e path in e2e-fork.yaml still ran the pre-#2826 BATS app loop (`hack/e2e-apps/*.bats` + `test-apps-%`), which the merged Chainsaw migration deleted — so the fork run was already broken. Rewrite it to the same Chainsaw + Test-Impact-Analysis path as the in-tree job, and clear the remaining review follow-ups on the publish job. - Run E2E via a single `make test-chainsaw` invocation (no per-app retry — e2e-testing.md §1). Suite selection and the full-e2e override come from `resolve`'s TRUSTED base-repo listFiles/labels (new `changed_b64` / `full_e2e` outputs), never a git diff of the fork-controlled merge ref, so TIA cannot be shaped into skipping the suite (keeps the #3257 property). Collect + upload the Chainsaw JUnit report like the in-tree job. - Restrict fork image pushes to a trusted allowlist derived from the base parent (HEAD^1) Makefiles, so a fork cannot mint an arbitrary repo path under the CI registry by renaming an image. The charset guard stays as defence in depth. - `skopeo copy --all` so a multi-platform build's index digest (the one baked into pr.patch) is preserved. - Pin the flux CLI (FLUX_VERSION) in the privileged publish job instead of install.sh's rolling latest. Docs: e2e-testing.md §10 records that the fork path now runs Chainsaw + TIA and that fork suite selection must stay sourced from trusted data. Assisted-By: Claude <[email protected]> Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 0021309 commit 3ea7531

2 files changed

Lines changed: 144 additions & 25 deletions

File tree

‎.github/workflows/e2e-fork.yaml‎

Lines changed: 140 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ jobs:
6666
head_sha: ${{ steps.r.outputs.head_sha }}
6767
run_id: ${{ steps.r.outputs.run_id }}
6868
check_run_id: ${{ steps.open.outputs.check_run_id }}
69+
# TIA inputs derived from TRUSTED base-repo data (the PR file list and
70+
# labels), NOT the fork-controlled merge-ref diff — so suite selection
71+
# keeps the same anti-forgery property as the docs-only decision (#3257).
72+
changed_b64: ${{ steps.r.outputs.changed_b64 }}
73+
full_e2e: ${{ steps.r.outputs.full_e2e }}
6974
steps:
7075
# Decide the verdict using this step's default (workflow_run) token, which
7176
# already has the actions:read / pull-requests:read scopes these lookups
@@ -147,6 +152,19 @@ jobs:
147152
return;
148153
}
149154
155+
// Test-Impact Analysis inputs, from the SAME trusted sources as the
156+
// docs-only decision above: the base-repo PR file list drives suite
157+
// selection (base64 so newlines survive the job output), and the
158+
// base-repo PR labels drive the full-e2e override. Deriving these
159+
// from `git diff` on the fork-controlled merge ref would let a fork
160+
// shape the diff so TIA selects nothing and e2e is skipped — the
161+
// same class of bypass #3257 closes. select-e2e.sh fails safe to the
162+
// full suite on any unrecognised path, so this can only narrow.
163+
const changed = files.map(f => f.filename).join('\n');
164+
core.setOutput('changed_b64', Buffer.from(changed, 'utf8').toString('base64'));
165+
const fullE2e = (pr.labels || []).some(l => l.name === 'full-e2e');
166+
core.setOutput('full_e2e', fullE2e ? 'true' : 'false');
167+
150168
// Real run: the `open` step opens an in-progress check-run; `report`
151169
// concludes it.
152170
core.setOutput('verdict_status', 'in_progress');
@@ -210,9 +228,21 @@ jobs:
210228
packages_digest: ${{ steps.push.outputs.packages_digest }}
211229
steps:
212230
- name: Set up toolchain (skopeo, flux)
231+
env:
232+
# Pin the flux CLI instead of install.sh's rolling latest. This is the
233+
# one privileged job that runs a remote installer as root right before
234+
# the OCIR token is loaded, so it pins where the rest of CI does not;
235+
# keep it in step with the flux distro cozystack ships
236+
# (packages/system/fluxcd, 2.8.x).
237+
FLUX_VERSION: "2.8.6"
213238
run: |
214239
command -v skopeo >/dev/null || { sudo apt-get update -qq && sudo apt-get install -y -qq skopeo; }
215-
if ! command -v flux >/dev/null; then s="$(mktemp)"; curl -fsSL https://fluxcd.io/install.sh -o "$s"; sudo bash "$s"; rm -f "$s"; fi
240+
if ! command -v flux >/dev/null; then
241+
s="$(mktemp)"
242+
curl -fsSL https://fluxcd.io/install.sh -o "$s"
243+
sudo env "FLUX_VERSION=$FLUX_VERSION" bash "$s"
244+
rm -f "$s"
245+
fi
216246
# The base repo's own merge ref — safe to fetch, not fork-controlled code
217247
# execution. pr.patch is applied as DATA below; nothing from the tree runs.
218248
- name: Checkout PR merge ref
@@ -257,6 +287,33 @@ jobs:
257287
# keychain) each need OCIR auth. Token via stdin, never argv.
258288
echo "$OCIR_TOKEN" | skopeo login "${REGISTRY%%/*}" -u "$OCIR_USER" --password-stdin
259289
echo "$OCIR_TOKEN" | docker login "${REGISTRY%%/*}" -u "$OCIR_USER" --password-stdin
290+
# Trusted allowlist of image repo names. Every OCI archive a fork PR pushes
291+
# under ${REGISTRY}/ must be one the BASE tree can itself export — derived
292+
# from the base parent (HEAD^1), never the fork's Makefiles, so a fork
293+
# cannot mint an arbitrary repo path under the CI registry by renaming an
294+
# image (#3262 non-blocking hardening). `make -n` only expands recipes (no
295+
# build); COZYSTACK_VERSION is set so the version probe in common-envs.mk
296+
# stays dormant. The charset guard in the push loop stays as defence in
297+
# depth; this narrows it to the exact expected set.
298+
- name: Derive trusted image allowlist (base tree)
299+
run: |
300+
set -eu
301+
base="$(git rev-parse HEAD^1)"
302+
# Unique path so a leftover from a crashed/concurrent run on the shared
303+
# self-hosted runner can't fail `git worktree add`.
304+
bt="$(mktemp -d)/basetree"
305+
git worktree add --quiet --detach "$bt" "$base"
306+
git -C "$bt" ls-files 'packages/*/*/Makefile' | while IFS= read -r mk; do
307+
grep -qE '^image:' "$bt/$mk" || continue
308+
make -n -C "$bt/$(dirname "$mk")" image \
309+
OCI_EXPORT_DIR=/oci REGISTRY=r COZYSTACK_VERSION=0 2>/dev/null || true
310+
done \
311+
| grep -oE '(dest=/oci/|oci-archive:/oci/)[a-z0-9._-]+\.oci\.tar' \
312+
| sed -E 's#.*/##; s#\.oci\.tar$##' | sort -u > _allow.txt
313+
git worktree remove --force "$bt"
314+
n="$(wc -l < _allow.txt)"
315+
[ "$n" -ge 1 ] || { echo "::error::image allowlist derivation produced no names"; exit 1; }
316+
echo "Allowed image repos ($n):"; sed 's/^/ /' _allow.txt
260317
- name: Push images by digest + packages artifact
261318
id: push
262319
env:
@@ -281,10 +338,16 @@ jobs:
281338
case "$name" in
282339
*[!a-z0-9._-]*|""|-*) echo "::error::unexpected image archive name: $name"; exit 1;;
283340
esac
341+
# …and against the trusted base-tree allowlist, so a charset-clean but
342+
# unexpected name cannot create a junk repo under the CI registry.
343+
grep -qxF "$name" _allow.txt || { echo "::error::image '$name' is not in the base-tree allowlist"; exit 1; }
284344
echo "Pushing $name → ${REGISTRY}/${name}:${IMAGE_TAG}"
285-
# skopeo preserves the manifest digest bit-for-bit, so the
345+
# --all so a multi-platform build copies the whole manifest list and
346+
# its index digest is preserved (else containerimage.digest — the
347+
# index digest baked into pr.patch — would not resolve). --preserve-
348+
# digests keeps every manifest digest bit-for-bit so the
286349
# ${REGISTRY}/${name}@sha256:… refs in pr.patch resolve.
287-
skopeo copy --preserve-digests "oci-archive:$f" "docker://${REGISTRY}/${name}:${IMAGE_TAG}"
350+
skopeo copy --all --preserve-digests "oci-archive:$f" "docker://${REGISTRY}/${name}:${IMAGE_TAG}"
288351
done
289352
# Push the fully-patched packages tree — the OCI artifact the in-cluster
290353
# operator pulls. This uploads data; it runs no code from the tree.
@@ -299,8 +362,9 @@ jobs:
299362
300363
# Run the e2e suite against the pushed closure. NO OCIR push token here — the
301364
# sandbox pulls images anonymously; the fork's images/scripts run only in this
302-
# ephemeral job. Mirrors pull-requests.yaml's in-tree e2e (full suite; a fork
303-
# PR has no reliable cross-workflow diff to scope TIA against).
365+
# ephemeral job. Mirrors pull-requests.yaml's in-tree e2e, including Chainsaw
366+
# Test-Impact Analysis — scoped from resolve's TRUSTED base-repo file list
367+
# (not a fork-controlled merge-ref diff), so suite selection can only narrow.
304368
e2e:
305369
name: E2E (fork run)
306370
needs: [resolve, publish]
@@ -393,26 +457,79 @@ jobs:
393457
run: |
394458
cd "/tmp/$SANDBOX_NAME"
395459
make -C packages/core/testing SANDBOX_NAME=$SANDBOX_NAME test-openapi
396-
- name: Run E2E tests (full suite)
460+
# Test-Impact Analysis: select only the Chainsaw suites the PR diff
461+
# affects. The changed-file list is resolve's TRUSTED base-repo listFiles
462+
# (base64 through the job output), NOT a git diff on the fork-controlled
463+
# merge ref — so a fork cannot shape the diff to make TIA select nothing.
464+
# select-e2e.sh escalates to the full suite on any unrecognised path, so
465+
# this only narrows. Skipped (→ full suite below) on the full-e2e label.
466+
- name: Select E2E tests
467+
id: select
468+
if: ${{ needs.resolve.outputs.full_e2e != 'true' }}
469+
env:
470+
CHANGED_B64: ${{ needs.resolve.outputs.changed_b64 }}
397471
run: |
398472
cd "/tmp/$SANDBOX_NAME"
399-
apps_list=$(ls hack/e2e-apps/*.bats | xargs -n1 basename | cut -d. -f1)
400-
failed_tests=""
401-
for app in $apps_list; do
402-
echo "::group::Testing $app"
403-
if make -C packages/core/testing SANDBOX_NAME=$SANDBOX_NAME test-apps-$app; then
404-
echo "::endgroup::"; echo "✅ $app passed"
405-
else
406-
echo "::endgroup::"; echo "❌ $app failed"
407-
failed_tests="$failed_tests $app"
408-
echo "::group::Diagnostics for $app"
409-
docker exec "$SANDBOX_NAME" sh -c 'kubectl get hr -A -o wide 2>&1 | tail -50' || true
410-
docker exec "$SANDBOX_NAME" sh -c 'kubectl get events -A --sort-by=.lastTimestamp 2>&1 | tail -30' || true
411-
echo "::endgroup::"
412-
fi
413-
done
414-
if [ -n "$failed_tests" ]; then echo "❌ Failed:$failed_tests"; exit 1; fi
415-
echo "✅ All E2E tests passed"
473+
printf '%s' "$CHANGED_B64" | base64 -d > /tmp/changed.txt
474+
echo "Changed files:"; sed 's/^/ /' /tmp/changed.txt
475+
apps="$(./hack/select-e2e.sh /tmp/changed.txt)"
476+
echo "Selected apps: ${apps:-<none>}"
477+
echo "apps=${apps}" >> "$GITHUB_OUTPUT"
478+
if [ -z "${apps}" ]; then
479+
echo "skip=true" >> "$GITHUB_OUTPUT"
480+
else
481+
echo "skip=false" >> "$GITHUB_OUTPUT"
482+
fi
483+
# Single `chainsaw test` invocation, no per-app retry loop (e2e-testing.md
484+
# §1) — Chainsaw polls each assertion to its own timeout. Full suite when
485+
# the full-e2e label skipped selection or the selection is empty.
486+
- name: Run E2E tests
487+
id: e2e_tests
488+
if: ${{ steps.select.outputs.skip != 'true' }}
489+
env:
490+
SELECTED_APPS: ${{ steps.select.outputs.apps }}
491+
FULL_E2E: ${{ needs.resolve.outputs.full_e2e }}
492+
run: |
493+
cd "/tmp/$SANDBOX_NAME"
494+
if [ "$FULL_E2E" = "true" ] || [ -z "$SELECTED_APPS" ]; then
495+
suites="" # empty => test-chainsaw runs the whole suite
496+
echo "Running full chainsaw suite"
497+
else
498+
suites="$SELECTED_APPS"
499+
echo "Running selected chainsaw suites: $suites"
500+
fi
501+
if ! make -C packages/core/testing SANDBOX_NAME=$SANDBOX_NAME test-chainsaw CHAINSAW_SUITES="$suites"; then
502+
echo "❌ Chainsaw E2E failed (see the assertion diffs above and diagnostics below)"
503+
echo "::group::Diagnostics: HelmRelease status"
504+
docker exec "$SANDBOX_NAME" sh -c 'kubectl get hr -A -o wide 2>&1 | tail -50' || true
505+
echo "::endgroup::"
506+
echo "::group::Diagnostics: recent events"
507+
docker exec "$SANDBOX_NAME" sh -c 'kubectl get events -A --sort-by=.lastTimestamp 2>&1 | tail -30' || true
508+
echo "::endgroup::"
509+
echo "::group::Diagnostics: COSI bucket state"
510+
docker exec "$SANDBOX_NAME" sh -c '
511+
kubectl get bucketclaims.objectstorage.k8s.io -A -o custom-columns="NAMESPACE:.metadata.namespace,NAME:.metadata.name,READY:.status.bucketReady,BUCKET:.status.bucketName" 2>&1
512+
kubectl get buckets.objectstorage.k8s.io -o custom-columns="NAME:.metadata.name,READY:.status.bucketReady,ID:.status.bucketID,CLAIMNS:.spec.bucketClaim.namespace" 2>&1
513+
kubectl get bucketaccesses.objectstorage.k8s.io -A -o custom-columns="NAMESPACE:.metadata.namespace,NAME:.metadata.name,GRANTED:.status.accessGranted,CLAIM:.spec.bucketClaimName" 2>&1
514+
' || true
515+
echo "::endgroup::"
516+
exit 1
517+
fi
518+
echo "✅ All selected E2E tests passed"
519+
- name: Collect chainsaw report
520+
if: always()
521+
run: |
522+
cd "/tmp/$SANDBOX_NAME"
523+
mkdir -p _out
524+
docker cp "$SANDBOX_NAME":/workspace/hack/e2e-chainsaw/chainsaw-report.xml \
525+
_out/chainsaw-report.xml || true
526+
- name: Upload chainsaw report
527+
if: always()
528+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
529+
with:
530+
name: chainsaw-report
531+
path: /tmp/${{ env.SANDBOX_NAME }}/_out/chainsaw-report.xml
532+
if-no-files-found: ignore
416533
- name: Collect report
417534
if: always()
418535
run: |

‎docs/agents/e2e-testing.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,16 @@ Be precise about TIA's scope — it is narrower than "skip E2E for unrelated PRs
102102
A fork's `pull_request` run carries no registry credentials, so it cannot push the images the e2e sandbox pulls. E2E therefore runs across **two** workflows:
103103

104104
- **`.github/workflows/pull-requests.yaml`** runs e2e in-tree for **same-repo** PRs (the `e2e` job, named `E2E (in-tree)`). Fork PRs skip this job.
105-
- **`.github/workflows/e2e-fork.yaml`** runs e2e for **fork** PRs via a privileged `workflow_run` that fires after the fork's Pull Request run completes. It pushes the fork-built images to the registry and runs the same suite in an ephemeral sandbox that itself has no push credentials.
105+
- **`.github/workflows/e2e-fork.yaml`** runs e2e for **fork** PRs via a privileged `workflow_run` that fires after the fork's Pull Request run completes. It pushes the fork-built images to the registry and runs the suite — the same Chainsaw + TIA path as in-tree (§7) — in an ephemeral sandbox that itself has no push credentials.
106106

107-
Two consequences to keep in mind when touching either workflow:
107+
Consequences to keep in mind when touching either workflow:
108108

109109
- **Fork builds export, they do not push (`OCI_EXPORT_DIR`).** The fork's unprivileged build sets `OCI_EXPORT_DIR` (see `hack/common-envs.mk`) so every image is written to a per-image OCI archive artifact instead of being pushed; the privileged `e2e-fork.yaml` later pushes those archives to the registry **by digest**. Any build recipe that shells out to push directly — e.g. a `skopeo copy … docker://…` that bypasses the `image-tags`/`oci-output` macros — must honour `OCI_EXPORT_DIR` too, or a fork build dies with an anonymous-push `denied` (#3257). This is covered by `hack/common-envs_test.bats`.
110110
- **"E2E Tests" is a synthetic check-run, not a job.** The required merge-gate context named `E2E Tests` is an explicit GitHub check-run — concluded by the `e2e-report` job for same-repo PRs and by `e2e-fork.yaml` for forks — **not** the `e2e` job itself. This is deliberate: a required *job* that is skipped (as the `e2e` job is for forks) counts as passed by branch protection, which is exactly how fork PRs used to merge with e2e never run (#3257). If you rename the `e2e` job, do **not** name it `E2E Tests` and do not wire branch protection to a job name — the check-run must stay the gate.
111111
- **The check-run is published by the COZYSTACK_CI GitHub App, not the default `GITHUB_TOKEN`.** A token-created check-run is owned by the app that created it, and GitHub files it under that app's check-suite. Created with the default token (the "GitHub Actions" app) a *floating* check-run — one with no owning job — attaches to an arbitrary GitHub Actions check-suite for the head SHA, in practice the earliest-registered one, which is a tiny labeler workflow; that is why `E2E Tests` used to render under "PR size label". Publishing it as the app (both `e2e-report` in `pull-requests.yaml` and the `open`/`report` steps in `e2e-fork.yaml` pass `github-token: ${{ steps.app-token.outputs.token }}`) gives the check its own dedicated suite shown under the app's name, and is stricter — only the CI app can satisfy the gate, not any workflow holding the default token. The app token is minted with `permission-checks: write` only; in `e2e-fork.yaml`'s `resolve` the PR/artifact lookups stay on the default `workflow_run` token, so the decision step and the check-publishing step use different tokens by design. **Consequence for branch protection:** the required `E2E Tests` context must be required *from the COZYSTACK_CI app's id*, not from GitHub Actions (`app_id 15368`). If it is pinned to GitHub Actions, the app-published check will not satisfy the gate and every PR stays "Expected"; conversely a `checks.create`/`checks.update` must always run as the same app that opened the pending run (the fork `report` step re-mints the token for exactly this reason).
112112

113+
- **Fork TIA is driven from trusted data, not a merge-ref diff.** `e2e-fork.yaml`'s `e2e` job runs the same `select-e2e.sh` TIA as in-tree (§7), but a `workflow_run` job cannot trust a `git diff` of the fork-controlled merge ref — a fork could shape that diff so TIA selects nothing and the suite is skipped. So the changed-file list and the `full-e2e` label both come from `resolve`'s base-repo API lookups (`pulls.listFiles` / the PR's labels), the same trusted source as the docs-only decision, passed to the `e2e` job as outputs (`changed_b64`, `full_e2e`). Keep it that way: never re-derive the fork selection from the checked-out tree.
114+
113115
**Security prerequisite (repo setting).** The split is only safe if fork runs require maintainer approval before they execute. The repository must keep **Settings → Actions → "Require approval for all outside collaborators"** enabled, so a fork's `pull_request` run — which produces the artifacts `e2e-fork.yaml` consumes — cannot run untrusted code (or feed the privileged run) without a maintainer's go-ahead.
114116
## Chainsaw v0.2.15 gotchas
115117

0 commit comments

Comments
 (0)