OCPBUGS-92034: fix registry override matching for digest and tag separators#8824
Conversation
|
@muraee: This pull request references Jira Issue OCPBUGS-92034, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/jira refresh |
|
@muraee: This pull request references Jira Issue OCPBUGS-92034, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@support/util/registryoverride/registryoverride.go`:
- Around line 60-61: The boundary check in the registry override matcher is too
permissive because the logic in the override matching function accepts ':' after
every source, which causes host-only entries to match host:port images
unintentionally. Update the matching logic around the source/image boundary
check so ':' is only treated as a valid separator when it truly indicates a tag
or digest boundary, not a registry port, and keep the existing '/' and '@'
behavior intact. Use the registry override matching code path and the symbol
that performs the boundary comparison to localize the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8d284b7f-3998-4c24-8479-0a7b04a7725c
📒 Files selected for processing (2)
support/util/registryoverride/registryoverride.gosupport/util/registryoverride/registryoverride_test.go
5628395 to
aa1bab5
Compare
|
@muraee: This pull request references Jira Issue OCPBUGS-92034, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8824 +/- ##
=======================================
Coverage 42.59% 42.60%
=======================================
Files 768 768
Lines 95359 95371 +12
=======================================
+ Hits 40617 40629 +12
Misses 51934 51934
Partials 2808 2808
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
…arators The registryoverride.Replace function only accepted "/" as a valid separator after the source prefix, causing repository-level --registry-overrides to fail for images with @sha256: digest references. This broke all component image rewrites in disconnected environments using overrides like: quay.io/openshift-release-dev/ocp-v4.0-art-dev=mirror/art-dev Extend matchesPrefix to also accept "@" (digest) and ":" (tag) as valid separators alongside "/". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
aa1bab5 to
e498117
Compare
|
/verified by UT Changes covered well by unit tests |
|
@bryan-cox: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, muraee The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
Test Resultse2e-aws
e2e-aks
|
|
Now I have all the evidence I need. Let me produce the final report for both jobs. Test Failure Analysis CompleteJob Information
e2e-aks —
|
| Evidence | Detail |
|---|---|
| PR #8824 changed files | support/util/registryoverride/registryoverride.go, registryoverride_test.go — registry override matching only |
| e2e-aks pass rate | 404/409 passed (98.8%), 5 failures all cascade from 1 test |
| e2e-aks failing test | TestAdditionalTrustBundlePropagation/AdditionalTrustBundlePropagationTest |
| e2e-aks root cause | Azure OSProvisioningTimedOut for VM 8zvfjz (started 17:53:22, failed 18:13:34) |
| e2e-aks Azure error | OSProvisioningTimedOut: OS Provisioning for VM did not finish in the allotted time |
| e2e-aks AzureMachine status | ready: true but vmState: Failed — known CAPZ inconsistency |
| e2e-aks healthy comparison | Other VM (gnmkh4) in same NodePool succeeded: vmState: Succeeded, phase: Running |
| e2e-aws pass rate | 619/623 passed (99.4%), 4 failures all cascade from 1 test |
| e2e-aws failing test | TestKarpenter/Main/Parallel_provisioning_tests/OpenshiftEC2NodeClass_Kubelet_propagation |
| e2e-aws root cause | tls: internal error on kubelet API at 10.0.142.65:10250 — TLS handshake failure |
| e2e-aws error code | HTTP 500 — Get "https://10.0.142.65:10250/containerLogs/kube-system/kubelet-config-checker/checker": remote error: tls: internal error |
| e2e-aws node provisioning | Node became ready in 5m33s, then immediate log read attempt failed with TLS error |
| Relationship to PR | None — neither test exercises registry override matching code |
|
/test e2e-aks |
|
/test e2e-aws |
|
@muraee: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
…ROSLSRE-1318) The latest HyperShift operator image includes a regression in registryoverride.Replace (openshift/hypershift#8509) that breaks repository-level --registry-overrides for digest-based images. This causes all CAPI and component image rewrites to fail in environments using ACR mirrors, blocking cluster creation. Pin to the known-good build (a101e669, 2026-06-05) until the upstream fix (openshift/hypershift#8824, OCPBUGS-92034) merges and a new image is published.
|
@muraee: Jira Issue Verification Checks: Jira Issue OCPBUGS-92034 Jira Issue OCPBUGS-92034 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Fix included in release 5.0.0-0.nightly-2026-06-25-194049 |
|
/cherrypick release-4.22 release-4.21 release-4.20 |
|
@avollmer-redhat: #8824 failed to apply on top of branch "release-4.22": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
… containers Backports openshift#8509 and openshift#8824 to release-4.22: 1. Add support/util/registryoverride package with strict longest-prefix matching that correctly handles digest (@sha256:) and tag (:) separators, preventing false substring matches (e.g. "quay.io" matching "quay.io.example.com"). 2. Fix RegistryMirrorProviderDecorator.Lookup to use registryoverride.Replace instead of strings.Replace, eliminating the original substring-match bug. 3. Add imageprovider.NewWithRegistryOverrides to apply registry overrides to all component images at provider creation time, ensuring init containers (availability-prober) and other CPO sub-resources use overridden images. 4. Wire NewWithRegistryOverrides into the HCP controller reconcile loop so the control-plane release image provider applies overrides. Without this fix, CPO-managed init containers (e.g. availability-prober) retain original registry references, causing ValidatingAdmissionPolicies in Deny mode to block HCP creation in environments that restrict image sources.
… containers Backports openshift#8509 and openshift#8824 to release-4.21: 1. Add support/util/registryoverride package with strict longest-prefix matching that correctly handles digest (@sha256:) and tag (:) separators, preventing false substring matches (e.g. "quay.io" matching "quay.io.example.com"). 2. Fix RegistryMirrorProviderDecorator.Lookup to use registryoverride.Replace instead of strings.Replace, eliminating the original substring-match bug. 3. Add imageprovider.NewWithRegistryOverrides to apply registry overrides to all component images at provider creation time, ensuring init containers (availability-prober) and other CPO sub-resources use overridden images. 4. Wire NewWithRegistryOverrides into the HCP controller reconcile loop so the control-plane release image provider applies overrides. Without this fix, CPO-managed init containers (e.g. availability-prober) retain original registry references, causing ValidatingAdmissionPolicies in Deny mode to block HCP creation in environments that restrict image sources.
… containers Backports openshift#8509 and openshift#8824 to release-4.20: 1. Add support/util/registryoverride package with strict longest-prefix matching that correctly handles digest (@sha256:) and tag (:) separators, preventing false substring matches (e.g. "quay.io" matching "quay.io.example.com"). 2. Fix RegistryMirrorProviderDecorator.Lookup to use registryoverride.Replace instead of strings.Replace, eliminating the original substring-match bug. 3. Add imageprovider.NewWithRegistryOverrides to apply registry overrides to all component images at provider creation time, ensuring init containers (availability-prober) and other CPO sub-resources use overridden images. 4. Wire NewWithRegistryOverrides into the HCP controller reconcile loop so the control-plane release image provider applies overrides. Without this fix, CPO-managed init containers (e.g. availability-prober) retain original registry references, causing ValidatingAdmissionPolicies in Deny mode to block HCP creation in environments that restrict image sources.
…t containers Combined manual backport of openshift#8509 and openshift#8824 to release-4.21. Introduces strict longest-prefix registry override matching and wires overrides into CPO init container image resolution.
…t containers Combined manual backport of openshift#8509 and openshift#8824 to release-4.20. Introduces strict longest-prefix registry override matching and wires overrides into CPO init container image resolution.
…t containers Combined manual backport of openshift#8509 and openshift#8824 to release-4.22. Introduces strict longest-prefix registry override matching and wires overrides into CPO init container image resolution.
Summary
registryoverride.Replaceto accept@(digest) and:(tag) as valid separators alongside/, so repository-level--registry-overrideswork for digest-based imagesRoot Cause
PR #8509 introduced strict prefix matching in
registryoverride.Replacethat only checked for/as a valid separator after the source key. Release payload images use@sha256:digest references, so repository-level overrides like:failed to match
quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:abc123because the character after the source prefix is@, not/.This broke CAPI and all other component image rewrites in disconnected/air-gapped environments using repository-level overrides.
Which issue(s) this PR fixes
Fixes OCPBUGS-92034
Regression of OCPBUGS-74247 (PR #7575)
Test plan
@sha256:):latest)registryoverride,imageprovider, andregistry_mirror_providertests pass🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
host:portbehavior.Tests
host:portmatching and correct boundary handling.