Skip to content

chore(tests): pin the e2e-proxy probe image, like the one next door (backend#2350) - #813

Merged
LukasWodka merged 3 commits into
developfrom
chore/2350-pin-e2e-proxy-curl
Aug 24, 2026
Merged

chore(tests): pin the e2e-proxy probe image, like the one next door (backend#2350)#813
LukasWodka merged 3 commits into
developfrom
chore/2350-pin-e2e-proxy-curl

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

scripts/tests/e2e-proxy.sh ran its egress-app probe pod on curlimages/curl:latest, while e2e_egress_positive_control in scripts/tests/lib/e2e-common.sh pins curlimages/curl:8.20.0 for the same job. This pins the proxy probe to the same tag, so the two curl probes in this suite cannot drift apart.

This is hygiene, not a regression fix. The unpinned tag was not the cause of the backend#2350 flake — both latest and 8.20.0 were measured under that investigation and behave identically with respect to curl's negative DNS caching. The reason to pin anyway is that E2E auth-proxy (squid) is a candidate required status check: under a floating tag, an external registry's next push would be able to block a merge here, with nothing in the diff to explain why.

Related

Follow-up hygiene from the backend#2350 investigation (the flake itself was fixed in #811). Deliberately no closing keyword — backend#2350 is already closed and this does not change its outcome.

Type of change

  • Feature
  • Bug fix
  • Tech-debt / refactor
  • Docs
  • Security / hardening
  • Breaking change

Test plan

One-line image change plus a comment inside a kubectl apply -f - <<YAML heredoc. Because that heredoc is unquoted, anything in it is shell-expanded — so the comment is deliberately free of backticks and $, and the rendering was checked rather than assumed:

  • bash -n scripts/tests/e2e-proxy.sh — clean
  • shellcheck -S warning -x scripts/tests/e2e-proxy.sh — no findings
  • bash scripts/check-style.shok: style + terminology clean
  • Rendered both YAML heredocs in the script with the runtime vars substituted, asserted every document still parses as YAML, and asserted the egress-app pod's image equals the seal-check's pin parsed out of lib/e2e-common.sh rather than a restated copy of 8.20.0:
2 YAML heredoc(s) found
4 document(s) parse as valid YAML
egress-app image -> curlimages/curl:8.20.0
seal-check pin -> curlimages/curl:8.20.0
PASS: probe image pinned and equal to the seal-check pin
  • Mutation-proved that agreement check: restored :latest, confirmed the anchor applied, and confirmed it reddens —
    AssertionError: REDDENED as expected: curlimages/curl:latest != curlimages/curl:8.20.0 — then restored the pin.

8.20.0 is known-good in this repo's CI: the seal-check has been pulling that exact tag on every green run.

Deployment notes

None. Test-harness only — no chart, no runtime path.

Out of scope (flagged, not changed)

e2e-proxy.sh still has three other floating tags: ubuntu/squid:latest (lines 77 and 206) and nginx:alpine (line 116). Pinning squid is a behavioural call rather than a mechanical one — a squid version bump can change proxy semantics the test asserts — so it wants its own PR and its own run, not a drive-by here. Happy to file it if wanted.

Checklist

  • Tests added / updated and passing locally
  • Docs updated if behavior or config changed (n/a — no behavior or config change)
  • No secrets / credentials in the diff
  • For security-sensitive paths: appropriate reviewer requested
  • Cross-repo issues use Fixes tracebloc/<repo>#N — n/a, no closing keyword by design
  • If this depends on a change in another repo — n/a, self-contained
  • Terminal output follows STYLE.mdbash scripts/check-style.sh passes

🤖 Generated with Claude Code


Note

Low Risk
Test-harness image pin only; no runtime, auth, or probe-behavior change.

Overview
Pins the egress-app probe in e2e-proxy.sh from curlimages/curl:latest to curlimages/curl:8.20.0, matching e2e_egress_positive_control so the suite’s two curl probes cannot drift.

This is supply-chain hygiene for a candidate required check: a floating tag could fail merges on an unrelated registry push. Comments in the script and e2e-proxy-probe.bats note that :latest and 8.20.0 behave the same on curl’s negative DNS cache, so this is not a flake fix.

Reviewed by Cursor Bugbot for commit 2082706. Bugbot is set up for automated code reviews on this repo. Configure here.

…backend#2350)
`scripts/tests/e2e-proxy.sh` ran its egress-app probe on
`curlimages/curl:latest` while `e2e_egress_positive_control` in
`scripts/tests/lib/e2e-common.sh` pinned `curlimages/curl:8.20.0` for the same
job. Same version now, so the two probes cannot drift.
This is hygiene, not a regression fix. The unpinned tag was NOT the cause of the
backend#2350 flake — both `latest` and `8.20.0` were measured there and behave
identically with respect to curl's negative DNS caching.
The reason to pin anyway: `E2E auth-proxy (squid)` is a candidate required
status check. Under a floating tag, an external registry's next push would be
able to block a merge in this repo, with nothing in the diff to explain it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 24, 2026
Comment threadscripts/tests/e2e-proxy.sh Outdated

@saadqbalsaadqbal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good change, no blockers. Both claims verified: lib/e2e-common.sh:78 does pin
curlimages/curl:8.20.0, so the two probes now match exactly, and the <<YAML heredoc at line 245
is indeed unquoted — the added comment contains no $ and no backtick, so it survives expansion
intact. Checking that yourself in the test plan is the right instinct.

One non-blocking note inline: the same file still runs ubuntu/squid:latest at :206 and
nginx:alpine at :116. The rationale you give — a floating tag lets an external registry's push
block a merge on a candidate required check — applies at least as strongly to the squid image, given
the job is named after it. The exposure is narrowed rather than closed, so I'd either soften the
comment to say what remains or reference a follow-up. Not asking you to widen the scope; the unit of
work as titled is clean.

Holding the approval only because CI is still running.

@aptraceblocaptracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pin itself is correct and the unit of work is clean — I confirmed independently: lib/e2e-common.sh:78 does pin curlimages/curl:8.20.0, so the two probes now match exactly, and the heredoc at line 245 is unquoted but the added comment carries no $ or backtick, so it passes through intact.

I concur with @saadqbal's one point, and I'd hold on it: as written the comment reads as though the required-check exposure is now handled, but ubuntu/squid:latest in the same file and the same job (E2E auth-proxy (squid)) still floats — so an external registry push can still block a merge here, just Canonical's rather than curlimages'. Either soften the comment to name what remains, or file the squid-pin follow-up and reference it. No need to widen the scope in this PR.

Happy to approve once the comment reflects that. Nice catch pinning the probe.

— drafted with Claude Code

LukasWodkaand others added 2 commits August 24, 2026 14:42
…mage claim (backend#2350)
Review feedback from @saadqbal and @aptracebloc, both making the same point: the
comment read as though the required-check exposure was handled, while squid and
nginx in the same file and the same job still floated. Neither asked to widen
the scope, so the code is unchanged and only the claim moves.
The comment now says the exposure is a property of the whole job rather than of
one line, and points at client#814 (backend#2446), which digest-pinned squid and
nginx. With those merged and this, no image the job pulls floats.
Also refreshes e2e-proxy-probe.bats, which landed on develop meanwhile and
asserted "curlimages/curl:latest (the image the pod runs)" -- true before this
PR, false after it. It now names the pinned tag and records that the A/B was
measured on both, so the finding still holds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2082706. Configure here.

@saadqbalsaadqbal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Fixing the claim rather than widening the scope was the right call, and the new wording is
better than what I asked for: saying the exposure is a property of the whole job rather than of this
one line, then naming #814 for squid and nginx, makes the pair legible to someone reading either PR
alone.

The cross-reference is also safe to state in the past tense now — #814 merged at 12:38, so "the squid
and nginx images it also pulls were digest-pinned in client#814" is true on develop before this
lands, and "every image the job pulls is pinned; none of them floats" becomes true the moment it does.

You went past my note too: refreshing the A/B comment to say the pod now runs 8.20.0 rather than
:latest, and that the measurement held on both tags, closes a stale reference I had not spotted.

Earlier verification stands: lib/e2e-common.sh:78 pins the same 8.20.0, so the suite's two curl
probes now match exactly, and the <<YAML heredoc is unquoted so nothing here is silently unexpanded.

@LukasWodka
LukasWodka merged commit 1577b6f into developAug 24, 2026
37 checks passed
@LukasWodka
LukasWodka deleted the chore/2350-pin-e2e-proxy-curl branch August 24, 2026 12:58
aptracebloc added a commit that referenced this pull request Aug 27, 2026
…w (backend#2350) (#867)
The auth-proxy E2E shipped RED to staging on client#789 and blocked nothing
because it lived in installer-tests.yaml — an expensive, path-filtered workflow
whose jobs can never be required status checks (a required check behind a paths
filter never reports on a PR outside its paths, so GitHub holds the PR at
"waiting for status to be reported" forever; a guard that cannot block is advice,
not a gate — backend#1729).
Root cause of the red itself was diagnosed and already fixed on develop: the
squid probe's curl --retry was inert against curl's negative-DNS cache (#811),
and the probe/squid/nginx images are pinned (#813). This change does the
remaining enablement so the check can actually gate.
Extract the e2e-proxy job into e2e-auth-proxy.yaml, mirroring the move
Source-of-truth drift made out of installer-tests' old static job:
- pull_request has NO paths filter, so the context always reports and can be
required on staging/main without deadlocking a promotion PR that touches no
scripts/ file (backmerge, hotfix lane, version-bump promotion).
- push keeps a paths filter (pushes aren't gated) and adds staging; the Monday
cron is preserved so extracting the job doesn't drop its weekly canary.
- job name 'E2E auth-proxy (squid)' is unchanged, so the required-context name
and the release train's by-name check reading are preserved.
Arming it as a required check on staging + main is a follow-up branch-protection
setting, to be flipped after a green-watch window (arm while green). See
backend#2350.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@LukasWodka@saadqbal@aptracebloc