[release-4.22] OCPBUGS-90149: fix(helm): tolerate v-prefix version mismatch in disk-to-mirror#1448
Conversation
During the disk-to-mirror phase oc-mirror reconstructed chart tarball
paths as {name}-{version}.tgz, but the Helm downloader saves files
using the URL basename from the repository index.yaml, which may embed
a "v" prefix (e.g. csi-driver-nfs-v4.9.0.tgz). This caused d2m to
fail with "file not found" even though the m2d download succeeded.
Introduce resolveChartPath which tries the primary path first and falls
back to toggling the "v" prefix on the version component:
- Path traversal is prevented by confining both candidates to the
charts directory via filepath.Rel.
- Non-ENOENT os.Stat errors (permission/I/O) are surfaced immediately
rather than being silently treated as "file missing".
- A descriptive error naming both attempted paths is returned when
neither candidate exists.
Add two new test functions:
- TestResolveChartPath: unit tests for the helper covering all four
cases (exact match, config omits v / disk has v, config has v / disk
has no v, both missing). The not-found case asserts both candidate
path fragments appear in the error message.
- TestHelmImageCollectorVPrefixDiskToMirror: end-to-end d2m collector
test that places a v-prefixed chart on disk while the config specifies
the bare version, and the reverse.
Co-authored-by: Cursor <cursoragent@cursor.com>
Extract chartPathExists helper to handle os.Stat + error classification logic, dropping resolveChartPath's cyclomatic complexity from 11 to 6 to satisfy the cyclop linter's max-complexity limit of 10. Also promote buildChartCandidatePath and toggleVersionPrefix from inline closures to package-level functions for clarity. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace manual v-prefix toggling with semver.NewVersion() which already
exists in the codebase. semver.String() always returns the canonical
form without a "v" prefix, so we can deterministically probe both
"{name}-{canonical}.tgz" and "{name}-v{canonical}.tgz" in a loop,
removing the need for the toggleVersionPrefix() helper.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
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: openshift/coderabbit/.coderabbit.yaml 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-88461 has been cloned as Jira Issue OCPBUGS-90149. 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-ci
Bot
changed the title
openshift-ci-robot
added
jira/valid-reference
|
@openshift-cherrypick-robot: This pull request references Jira Issue OCPBUGS-90149, 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. |
4 participants
This is an automated cherry-pick of #1430
/assign aguidirh