Skip to content

ci: restore merge queue GAR auth secret#3713

Merged
joeyorlando merged 3 commits intomainfrom
codex/fix-merge-queue-gar-auth
Apr 11, 2026
Merged

ci: restore merge queue GAR auth secret#3713
joeyorlando merged 3 commits intomainfrom
codex/fix-merge-queue-gar-auth

Conversation

@joeyorlando
Copy link
Copy Markdown
Contributor

@joeyorlando joeyorlando commented Apr 10, 2026

Summary

  • restore the GAR auth secret names used by platform-e2e-tests.yml on trusted runs
  • fix merge queue E2E image builds failing before any tests start

Context

PR #3710 inlined the old reusable platform workflow into first-class workflows. During that refactor, the E2E image-build workflow switched from the repo's working secret names:

  • DEVELOPMENT_OAUTH_PROXY_RELEASER_GCP_SERVICE_ACCOUNT_NAME
  • DEVELOPMENT_OAUTH_PROXY_RELEASER_GCP_WORKLOAD_IDENTITY_PROVIDER_IDENTIFIER

to generic GCP_SERVICE_ACCOUNT_NAME / GCP_WORKLOAD_IDENTITY_PROVIDER_IDENTIFIER.

Those generic secrets are not populated for this workflow in merge queue, so Authenticate to Google Artifact Registry fails with empty inputs and the merge queue never reaches the E2E jobs.

This change restores the original secret names in both GAR auth steps inside .github/workflows/platform-e2e-tests.yml.

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 10, 2026

Claude finished @joeyorlando's task in 2m 19s —— View job


PR Review: ci: restore merge queue GAR auth secrets

  • Read CLAUDE.md for repo-specific guidelines
  • Read changed files (platform-e2e-tests.yml, docker-image-scanning.yml)
  • Compare against origin/main to understand full diff
  • Investigate PR ci: split PR workflow trigger-only jobs #3710 context to verify claims about secret name change
  • Check repository for other workflow files using these secret names
  • Post review feedback

Summary

The PR does two related things (the title/description only mentions the first):

  1. Commit d37e26b — restores GAR auth secret names (the stated fix): .github/workflows/platform-e2e-tests.yml L102-103, L123-124 switch back from secrets.GCP_SERVICE_ACCOUNT_NAME / secrets.GCP_WORKLOAD_IDENTITY_PROVIDER_IDENTIFIER to secrets.DEVELOPMENT_OAUTH_PROXY_RELEASER_GCP_SERVICE_ACCOUNT_NAME / secrets.DEVELOPMENT_OAUTH_PROXY_RELEASER_GCP_WORKLOAD_IDENTITY_PROVIDER_IDENTIFIER.
  2. Commit 6ed8f97 — adds PR pass-through placeholder jobs for merge-queue-only required checks in both platform-e2e-tests.yml and docker-image-scanning.yml.

Verdict: correct fix, ships the merge queue unblock

1) The secret rename is correct and matches repo conventions. The actual repo secrets are DEVELOPMENT_OAUTH_PROXY_RELEASER_GCP_*. Other workflows confirm this:

  • .github/workflows/on-commits-to-main.yml:35-36, 99-100, 119-120 reference secrets.DEVELOPMENT_OAUTH_PROXY_RELEASER_GCP_* directly (same pattern as this PR — first-class workflow).
  • .github/workflows/release-please.yml:84-85, 129-130 read from secrets.DEVELOPMENT_OAUTH_PROXY_RELEASER_GCP_* and forward them as generic GCP_* names to reusable workflows that declare them as secrets: inputs (publish-platform-helm-chart.yml:15-17, build-base-mcp-server-docker-image.yml:15-17).

Because PR #3710 inlined the old reusable platform-linting-and-tests flow and turned platform-e2e-tests.yml into a first-class workflow triggered by pull_request / merge_group / workflow_dispatch, there is no workflow_call: / secrets: mapping layer anymore. A first-class workflow must read repo-level secret names directly — the generic GCP_SERVICE_ACCOUNT_NAME is not populated at the repo level for this workflow, so auth-to-gar received empty inputs and failed. The change restores the correct lookup.

Both updated call sites correctly feed the required inputs declared in .github/actions/auth-to-gar/action.yml:3-9.

2) Pass-through placeholders look sound. The placeholder jobs mirror the real jobs' name: (so they satisfy the same required-check identity in branch protection / ruleset) but run trivially on ordinary PR pushes, reserving the actual expensive work for merge queue or the run-e2e / run-docker-scan labels. The if: condition is correct:

if: ${{ github.event_name == 'pull_request' && (github.event.action != 'labeled' || github.event.label.name != 'run-e2e') }}

This evaluates to true for opened/synchronize/reopened (any non-labeled action) and for any labeled event where the label is not run-e2e. Combined with the real job's existing if: ... || github.event.label.name == 'run-e2e', exactly one of (placeholder, real job) runs per PR event — no duplication, no gaps.


Observations and minor nits

a) Matrix names must exactly match, and they do. For required checks to pass through, the placeholder matrix name: values must produce the same expanded job names as the real jobs. Verified:

  • platform-e2e-tests.yml:38-39 placeholder [Platform, MCP Server Base] matches platform-e2e-build matrix at L58-74 (Platform, MCP Server Base).
  • platform-e2e-tests.yml:166-175 placeholder matrix [API, UI 1/2, UI 2/2, UI Identity Providers, UI Firefox, UI Webkit, Vault] matches the real platform-e2e-tests matrix at L194-247.
  • platform-e2e-tests.yml:333, 529, 648 single-job placeholders match their real counterparts (Platform E2E Tests (Quickstart), Platform E2E Tests (Vault K8s), Merge E2E Test Reports).
  • docker-image-scanning.yml:30-31 placeholder [Platform, MCP Server Base] matches the real job's matrix at docker-image-scanning.yml:44-57.

If you ever add/rename a matrix entry in a real job, don't forget to mirror it in the placeholder — otherwise required checks will silently diverge. Consider leaving a comment pointing to the mirror list, or collapse duplication by reading from a shared matrix include once GitHub Actions supports job-level matrix reuse.

b) Nit: the docker-image-scanning placeholder's if: is slightly more permissive than the corresponding real job. The real job guards on both the label and github.event.pull_request.head.repo.fork != true (docker-image-scanning.yml:38), but the placeholder only checks the label. This is fine — the placeholder is meant to cover "all non-scan PR events", including fork PRs with the run-docker-scan label (where the real scan job is skipped because of the fork filter). Without the placeholder running there, a fork PR labeled run-docker-scan would have no job at all for the required check. The current if: handles this correctly, so no change needed — but it's worth noting explicitly so a future reviewer doesn't try to "tighten" it.

c) Consistency nit. In platform-e2e-tests.yml the placeholders are immediately above their real counterparts, except platform-e2e-tests-placeholder (L159-178) is inserted before the comment for the real platform-e2e-tests job, which leaves the descriptive comment (# E2E tests - runs all e2e test suites in parallel at L157-158) visually attached to the placeholder rather than the real job. Not a functional issue — feel free to move the comment down under the placeholder, or leave it if you prefer to keep this PR tight.

d) PR description scope. The title and body cover only the secret rename, but the PR also lands ~80 lines of new placeholder jobs. If you care about release notes / git archeology, consider either:

  • Splitting commit 6ed8f97 into its own PR, or
  • Expanding the PR description to mention the second change (Add required-check placeholder jobs so merge-queue-only workflows can be required on PRs).

Either is purely a hygiene call; both changes are independently sound.

e) No hardcoded secrets, no shell-injection risk, no permission escalation introduced by either commit. The permissions: {} at workflow level remains, and the real jobs still opt in to contents: read + id-token: write as before.


Bottom line

Ship it. The secret restore is the correct and minimal fix for the merge queue Authenticate to Google Artifact Registry failure, and the placeholder jobs are a reasonable way to keep merge-queue-only required checks from blocking ordinary PR pushes.
codex/fix-merge-queue-gar-auth

@joeyorlando joeyorlando added this pull request to the merge queue Apr 10, 2026
@joeyorlando joeyorlando removed this pull request from the merge queue due to a manual request Apr 10, 2026
@joeyorlando joeyorlando enabled auto-merge April 10, 2026 23:22
@joeyorlando joeyorlando disabled auto-merge April 10, 2026 23:53
@joeyorlando joeyorlando changed the title ci: restore merge queue GAR auth secrets ci: restore merge queue GAR auth secret Apr 10, 2026
@joeyorlando joeyorlando added this pull request to the merge queue Apr 10, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 10, 2026
@joeyorlando joeyorlando merged commit 0d36d45 into main Apr 11, 2026
40 checks passed
@joeyorlando joeyorlando deleted the codex/fix-merge-queue-gar-auth branch April 11, 2026 00:06
Sign up for free to 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.

1 participant