[release-4.22] OCPBUGS-94170: tighten registry override matching to strict longest-prefix across release-image consumers#8873
Conversation
When CPO creates sub-resources (e.g. capi-provider deployments), it injects init containers like availability-prober using image references from the release image. These references were not being remapped by --registry-overrides, causing the init containers to reference the original registry (e.g. quay.io) instead of the override target. Add NewWithRegistryOverrides to the imageprovider package that clones the component images map and applies registry overrides using safe prefix matching (source+"/") to prevent subdomain false matches. Use it when creating the releaseImageProvider in the HostedControlPlane reconciler. Bug: https://redhat.atlassian.net/browse/OCPBUGS-85585
…refix matching
Introduce a shared helper that applies a map of registry-prefix overrides
to an image reference using strict matching semantics:
* An override matches only on exact equality with the source key or on a
"/"-boundary prefix. Bare substring matches (e.g. an override for
"quay.io" against "quay.io.example.com/...") are no longer possible.
* When several override keys match the same image, the longest key wins.
This makes the result deterministic regardless of map iteration order
and lets callers express both broad ("quay.io") and narrow
("quay.io/openshift-release-dev") overrides simultaneously.
* Empty source keys are skipped defensively.
* On no match, the image is returned unchanged.
The helper lives in a leaf subpackage of support/util so it can be consumed
from support/releaseinfo (which support/util itself imports). Follow-up
commits will wire it into RegistryMirrorProviderDecorator and
SimpleReleaseImageProvider, replacing two slightly different ad-hoc
implementations of the same logic.
Related: OCPBUGS-85585
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tryMirrorProviderDecorator
Replace the bare strings.Replace(image, source, dest, 1) loop, which was
the root cause of OCPBUGS-85585, with the shared registryoverride.Replace
helper.
The previous implementation had three latent issues:
1. Substring matching: an override for "quay.io" would also replace the
prefix of "quay.io.example.com/..." or "quay.io-mirror/..." -- any
string containing "quay.io" anywhere in its image path.
2. No slash-boundary check: the same false-positive class affected
narrower keys like "quay.io/openshift" against
"quay.io/openshift-release-dev/...".
3. Non-deterministic iteration: every key in RegistryOverrides was
applied in arbitrary map iteration order, so with overlapping keys
two runs of CPO could end up with different image references.
The shared helper applies strict matching (exact or "/"-boundary prefix),
picks the longest matching key deterministically, and skips empty source
keys defensively. Behaviour is unchanged for the common case of a single,
non-overlapping override registered against a clean image reference.
Related: OCPBUGS-85585
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… shared helper Three small refactors that align this provider with the decorator change in the previous commit and address review feedback on openshift#8509: * New() now delegates to NewWithRegistryOverrides(releaseImage, nil) instead of duplicating the field initialisation. registryoverride.Replace is a no-op for a nil override map, so callers of New() see exactly the same component images as before. * NewWithRegistryOverrides() uses maps.Clone (Go 1.25+, already required by go.mod) to copy ComponentImages() and registryoverride.Replace to remap each image. The inline duplicate-detection loop is gone. * The matching semantics now match RegistryMirrorProviderDecorator: strict slash-boundary prefix matching with longest-prefix-wins, instead of first-match-in-map-iteration-order. As a small defensive bonus, New() now also returns a SimpleReleaseImageProvider that owns a private copy of ComponentImages(), rather than aliasing the map embedded in the ReleaseImage. Related: OCPBUGS-85585 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…gistry overrides Cover the two properties highlighted in review feedback on openshift#8509: * NewWithRegistryOverrides must not mutate either its overrides argument or the source release image's ComponentImages map. This is the contract that lets multiple callers share the same release image and overrides map without surprises. * Applying the same overrides twice must be a no-op (idempotency). Regressions here would manifest as compounding rewrites, e.g. mirror.example.com/quay-cache/mirror.example.com/quay-cache/... Also adds a focused longest-prefix-wins assertion at this layer (the shared helper already has full coverage for the matching algorithm, but exercising it through the imageprovider keeps the layers honest). Related: OCPBUGS-85585 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ntroller tests
Flip the two registry-override mocks added by the previous commits from
map[string]string{} to a non-empty override so the full Reconcile path
actually walks the override-application logic at
hostedcontrolplane_controller.go:1106 (and the related propagation through
configoperatorv2.NewComponent and the ignitionserver --registry-overrides
flag).
The release image fixture has no images matching the override key, so the
existing semantic assertions of these tests are unchanged: this is purely
added coverage of the override code path through reconcile, matching the
convention already used at lines 176 and 1040 of the same file. Addresses
review feedback on openshift#8509.
Related: OCPBUGS-85585
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: openshift-cherrypick-robot The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@openshift-cherrypick-robot: Jira Issue OCPBUGS-85585 has been cloned as Jira Issue OCPBUGS-94170. Will retitle bug to link to clone. 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. |
|
@openshift-cherrypick-robot: This pull request references Jira Issue OCPBUGS-94170, 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. |
|
@openshift-cherrypick-robot: 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. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-4.22 #8873 +/- ##
================================================
+ Coverage 35.78% 35.80% +0.02%
================================================
Files 774 775 +1
Lines 94734 94756 +22
================================================
+ Hits 33904 33931 +27
+ Misses 58065 58060 -5
Partials 2765 2765
🚀 New features to boost your workflow:
|
|
/close |
|
@openshift-cherrypick-robot: This pull request references Jira Issue OCPBUGS-94170. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state. 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. |
|
@muraee: Closed this PR. 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. |
|
/jira refresh |
|
@avollmer-redhat: This pull request references Jira Issue OCPBUGS-94170, which is valid. The bug has been moved to the POST state. 7 validation(s) were run on this bug
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. |
|
@muraee Done — here are the combined manual backports of #8509 + #8824 for all three release branches:
All three include the strict longest-prefix matching fix (#8509) and the digest/tag separator fix (#8824), plus the init container override propagation in CPO. CI is running; reviews welcome when you get a chance. |
This is an automated cherry-pick of #8509
/assign avollmer-redhat
/cherrypick release-4.21 release-4.20