Skip to content

ROX-32532: Auto-detect registry auth type via /v2/ probe - #22747

Draft
kovayur wants to merge 2 commits into
masterfrom
yury/ROX-32532-authdetect
Draft

ROX-32532: Auto-detect registry auth type via /v2/ probe#22747
kovayur wants to merge 2 commits into
masterfrom
yury/ROX-32532-authdetect

Conversation

@kovayur

@kovayur kovayur commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Description

Update docker-registry-client to consume the update which adds WrapTransportWithDetection (stackrox/docker-registry-client#24). This function probes /v2/ to detect whether a registry requires bearer or basic auth, avoiding spurious Basic auth headers to registries that reject them.

A feature flag ROX_REGISTRY_AUTH_DETECTION (enabled by default) gates the new behavior at both call sites (docker config transport and cosign signature fetcher).

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

Deployed ACS on a local CRC cluster with Prometheus scraping Central's monitoring port. Created a private repo (quay.io/ykovalev/auth-test) and configured an authenticated docker-type integration in ACS.

Triggered 10 unique image scans per scenario and compared rox_central_registry_client_requests_total metrics:

Metric (docker type) Nightly (no fix) Fix, flag ON Fix, flag OFF
401 from scans 10 0 10
401 from startup probes 0 7 0

With the fix enabled, per-request 401s from Basic auth being sent to Bearer-only registries drop to ~0. The startup 401s are one-time /v2/ auth-detection probes (one per integration) that don't grow with usage. Disabling the feature flag restores the old behavior, confirming it works as a kill switch.

Update docker-registry-client to consume the fix/proactive-bearer-token-injection
branch which adds WrapTransportWithDetection. This function probes /v2/ to detect
whether a registry requires bearer or basic auth, avoiding spurious Basic auth
headers to registries that reject them.

A feature flag ROX_REGISTRY_AUTH_DETECTION (enabled by default) gates the new
behavior at both call sites (docker config transport and cosign signature fetcher).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci

openshift-ci Bot commented Sep 9, 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

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: c312ed73-99ab-41a3-94bb-929cb52cdc5e

📥 Commits

Reviewing files that changed from the base of the PR and between 8e2ee6d and 1ba16dd.

📒 Files selected for processing (1)
  • CHANGELOG.md

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Summary

Summary by CodeRabbit

  • New Features
    • Registry connections can automatically detect whether basic or bearer authentication is required by probing the registry’s /v2/ endpoint.
    • Authentication detection is enabled by default and can be controlled through the registry authentication detection setting.
    • Signature retrieval uses the same authentication detection behavior for more consistent registry access.
    • Registry access avoids sending unnecessary basic authentication headers when bearer authentication is appropriate.

Walkthrough

The change adds an enabled-by-default registry authentication detection flag. Docker and Cosign registry transports use /v2/ detection when enabled. Existing transport wrapping remains available when disabled.

Changes

Registry authentication detection

Layer / File(s) Summary
Feature flag and client contract
pkg/features/list.go, go.mod, CHANGELOG.md
Adds RegistryAuthDetection, updates the Docker registry client replacement, and documents the new behavior.
Docker transport integration
pkg/registries/docker/config.go, pkg/registries/docker/registry_without_digest_call_test.go
Selects detection in DefaultTransport when enabled. The metadata fallback test disables the feature.
Cosign transport integration
pkg/signatures/cosign_sig_fetcher.go
Selects detection when creating the Cosign registry transport and retains the previous wrapper when disabled.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 1ba16

This update documents the registry authentication detection feature and introduces no runtime behavior changes in the reviewed increment.

Suggested reviewers: parametalol

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the primary change: automatic registry authentication detection through a /v2/ probe.
Description check ✅ Passed The description explains the implementation, feature flag, user-facing impact, documentation status, CI inspection, and detailed validation results. The automated testing checklist is not marked, desp…
Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch yury/ROX-32532-authdetect

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

🚀 Build Images Ready

Images are ready for commit 1ba16dd. To use with deploy scripts:

export MAIN_IMAGE_TAG=5.0.x-244-g1ba16dd13f

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.74%. Comparing base (1db9fb2) to head (1ba16dd).
⚠️ Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
pkg/signatures/cosign_sig_fetcher.go 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #22747      +/-   ##
==========================================
- Coverage   51.78%   51.74%   -0.05%     
==========================================
  Files        2893     2893              
  Lines      182488   182526      +38     
==========================================
- Hits        94510    94452      -58     
- Misses      79691    79766      +75     
- Partials     8287     8308      +21     
Flag Coverage Δ
go-unit-tests 51.74% <81.81%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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