Skip to content

feat(oc-mirror): authenticate to ACR with a Go binary - #6668

Draft
Wenqi He (weherdh) wants to merge 1 commit into
Azure:mainfrom
weherdh:oc-mirror-acr-auth
Draft

feat(oc-mirror): authenticate to ACR with a Go binary#6668
Wenqi He (weherdh) wants to merge 1 commit into
Azure:mainfrom
weherdh:oc-mirror-acr-auth

Conversation

@weherdh

Copy link
Copy Markdown
Collaborator

Warning

Draft — blocked on Azure/ARO-Tools#321.
tooling/acrauth/go.mod carries a temporary replace pointing at a local
ARO-Tools checkout. That must be swapped for a published pseudo-version
before this can merge.

Why

mirror.sh authenticated to ACR like this:

DOCKER_COMMAND=/usr/local/bin/docker-login.sh az acr login -n "${REGISTRY}"

az acr login always persists credentials by exec'ing docker login, so we pointed
DOCKER_COMMAND at a shim that intercepted that call and scraped the credential out of
its arguments.

That coupling broke when azure-cli 2.88.0 (2026-07-07) switched to --password-stdin.
The shim's positional parsing shifted, it wrote a garbage credential, and the job kept
reporting success while mirroring nothing
. The image is built from a single digest
shared by all environments, so every environment was affected simultaneously. It went
unnoticed until STG's registries were emptied for an unrelated reason and there was no
longer a stale-but-present catalog masking it.

Separately, the shim's jq rewrite replaced auth.json wholesale, discarding the
registry.redhat.io credentials the decodesecrets init container had already written
to that same file.

The root problem is that our authentication depended on the internal argv layout of a CLI
we deliberately do not pin — tdnf update -y is required by the
secure supply chain policy.
That is not a contract we can rely on.

What

Do the exchange directly. acrauth login trades the managed identity's Entra token for an
ACR refresh token and upserts a single registry key into the auth file, leaving other
registries and unmodelled fields (email, identitytoken) intact.

/usr/local/bin/acrauth login \
    --registry   "${REGISTRY_URL}" \
    --auth-file  "${XDG_RUNTIME_DIR}/containers/auth.json" \
    --client-id  "${AZURE_CLIENT_ID:-}"

az is no longer on the authentication path, so a CLI upgrade cannot change our behaviour.

Also in here:

  • AZURE_CLIENT_ID is now optional. The old az login --identity --client-id "${AZURE_CLIENT_ID}" under set -u meant the local *-dry-run targets could never run. Empty now falls back to the ambient credential, which the Makefile already mounts via -v ${HOME}/.azure:/root/.azure.
  • The image build context moves to the repo root (matching frontend/Dockerfile and friends) so the Go module can be compiled in.
  • docker-login.sh is deleted.

Testing

Verified against the live arohcpocpdev registry:

Check Result
Token exchange exit 0
oras repo ls --registry-config <our auth.json> authenticated, listed repos
Seeded registry.redhat.io entry (incl. email) preserved yes
File mode 0600
Full container run, real mirror (no --dry-run) ✓ 1 / 1 additional images mirrored successfully

The last row is end-to-end: oc-mirror authenticated to the ACR and pushed an image using
only the credential this binary wrote.

Two bugs were caught by that testing and are already fixed:

  • CGO_ENABLED=0 compiles on stock Go but fails on the Microsoft FIPS toolchain this repo uses — now CGO_ENABLED=1 GOFIPS140=latest, matching frontend/Dockerfile.
  • rename(2) over a bind-mounted auth.json returns EBUSY, which the dry-run targets hit; handled in ARO-Tools#321.

Notes for reviewers

  • Depends on ARO-Tools#321; keep in draft until that merges.
  • The oc-mirror image digest in config/config.yaml still needs bumping once this builds.

mirror.sh authenticated by running 'az acr login' with DOCKER_COMMAND pointed at a shim script, so az's internal 'docker login' call was intercepted and the credential scraped out of its arguments. That broke silently when azure-cli 2.88.0 switched to --password-stdin: the job kept reporting success while mirroring nothing. The shim's jq rewrite also overwrote auth.json, discarding the registry.redhat.io credentials the init container had written there.

Replace it with the acrauth binary, which exchanges the managed identity's Entra token for an ACR refresh token and upserts a single registry key into the auth file. az is no longer on the authentication path, so a CLI upgrade cannot change our behaviour.

AZURE_CLIENT_ID is now optional, which lets the local dry-run targets fall back to the ambient az credential; previously 'set -u' made them fail outright.
@openshift-ci

openshift-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: weherdh
Once this PR has been reviewed and has the lgtm label, please assign deads2k for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci

openshift-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant