Skip to content

Fix verify-packages.sh to dynamically read repo filenames#3898

Merged
skitt merged 2 commits intosubmariner-io:develfrom
dfarrell07:fix-verify-packages-dynamic-repo
Apr 29, 2026
Merged

Fix verify-packages.sh to dynamically read repo filenames#3898
skitt merged 2 commits intosubmariner-io:develfrom
dfarrell07:fix-verify-packages-dynamic-repo

Conversation

@dfarrell07
Copy link
Copy Markdown
Member

@dfarrell07 dfarrell07 commented Mar 13, 2026

The script was hardcoded to fetch "submariner-rhel-10.repo" from git, which silently failed (|| true) when verifying older branches that use submariner-rhel-9.repo. This caused the script to verify packages against the wrong RHEL version.

Changes:

  • Extract repo filename from rpms.in.yaml for each component
  • Remove || true to fail fast when repo file is missing
  • Makes the script version-agnostic (works with RHEL 9 and 10)

Verified:

  • release-0.22 (RHEL 9) correctly shows rhel-9 repos
  • release-0.23 (RHEL 10) correctly shows rhel-10 repos

Summary by CodeRabbit

  • Chores
    • Updated package verification to dynamically determine and fetch repository files per component based on configuration rather than a fixed default.
    • Added a preflight check to ensure the required CLI tool is available before running verification, improving reliability and clearer failures.

@submariner-bot
Copy link
Copy Markdown
Contributor

🤖 Created branch: z_pr3898/dfarrell07/fix-verify-packages-dynamic-repo
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 045d0ba2-52d7-4b79-b997-141f96b2db1e

📥 Commits

Reviewing files that changed from the base of the PR and between c727eab and 14a8358.

📒 Files selected for processing (1)
  • .rpm-lockfiles/verify-packages.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • .rpm-lockfiles/verify-packages.sh

Walkthrough

Script .rpm-lockfiles/verify-packages.sh now reads the repository filename from each component's rpms.in.yaml (.contentOrigin.repofiles[0]) via yq, conditionally fetches that repo file from the selected git ref into the lockfile directory, and checks that yq is installed before proceeding.

Changes

Cohort / File(s) Summary
RPM lockfile verification script
.rpm-lockfiles/verify-packages.sh
Replaced hardcoded repo fetch with conditional retrieval: reads .$repo_file from each component's rpms.in.yaml (.contentOrigin.repofiles[0]) using yq, fetches the repo file from the selected git ref into the lockfile dir only if present, and adds a runtime check for yq.

Sequence Diagram(s)

sequenceDiagram
  participant VerifyScript as "verify-packages.sh"
  participant yq as "yq (CLI)"
  participant Git as "Git repo (git show)"
  participant LockDir as "lockfile dir"

  Note over VerifyScript: Start per-component verification
  VerifyScript->>yq: read .rpm-lockfiles/.../rpms.in.yaml -> .contentOrigin.repofiles[0]
  alt repo filename present
    VerifyScript->>Git: git show <GIT_REF>:.<repo_file>
    Git-->>VerifyScript: repo file content
    VerifyScript->>LockDir: write .$repo_file into lockfile dir
  else no repo filename
    VerifyScript-->>VerifyScript: skip repo fetch
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • vthapar
  • sridhargaddam
  • yboaron
  • Oats87
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main change: dynamically reading repo filenames instead of using hardcoded values in verify-packages.sh.
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.
Actionable Comments Resolved ✅ Passed All actionable comments have been resolved; no unresolved TODO, FIXME, or similar markers found in the code.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@dfarrell07 dfarrell07 moved this to In Review in Submariner 0.23 Mar 13, 2026
@dfarrell07 dfarrell07 enabled auto-merge (rebase) March 13, 2026 20:26
@submariner-bot submariner-bot added the ready-to-test When a PR is ready for full E2E testing label Mar 16, 2026
Comment thread .rpm-lockfiles/verify-packages.sh Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further
activity occurs. Thank you for your contributions.

@github-actions
Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further
activity occurs. Thank you for your contributions.

The script was hardcoded to fetch "submariner-rhel-10.repo" from git,
which silently failed (|| true) when verifying older branches that use
submariner-rhel-9.repo. This caused the script to verify packages
against the wrong RHEL version.

Changes:
- Extract repo filename from rpms.in.yaml for each component
- Remove || true to fail fast when repo file is missing
- Makes the script version-agnostic (works with RHEL 9 and 10)

Verified:
- release-0.22 (RHEL 9) correctly shows rhel-9 repos
- release-0.23 (RHEL 10) correctly shows rhel-10 repos

Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
@dfarrell07 dfarrell07 force-pushed the fix-verify-packages-dynamic-repo branch from c727eab to 14a8358 Compare April 28, 2026 16:32
@dfarrell07 dfarrell07 requested a review from skitt April 28, 2026 16:33
@dfarrell07 dfarrell07 disabled auto-merge April 28, 2026 16:33
@skitt skitt merged commit 72398db into submariner-io:devel Apr 29, 2026
53 checks passed
@submariner-bot
Copy link
Copy Markdown
Contributor

🤖 Closed branches: [z_pr3898/dfarrell07/fix-verify-packages-dynamic-repo]

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

Labels

ready-to-test When a PR is ready for full E2E testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants