Repository navigation
Conversation
…onents Pass installer-repo/installer-ref through to the e2e reusable workflows so PRs touching fulfillment-service, osac-operator, osac-aap, or bare-metal-fulfillment-operator deploy osac-installer from the same fork/ref, matching the existing components[].repo/.ref pattern. Also removes the two nested e2e workflow files under osac-installer/.github/ that GitHub never discovers there (workflows only run from the repo root's .github/workflows/) and that duplicated the root callers.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eliorerz 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 |
WalkthroughThe root BMaaS, CaaS, and VMaaS workflows now pass installer repository and ref values to the reusable workflow. The nested BMaaS and VMaaS workflows were deleted. ChangesInstaller source routing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/e2e-bmaas-full-install.yml:
- Line 70: Update the reusable workflow reference in the e2e-bmaas-full-install
workflow to use the required 40-character commit SHA instead of the mutable
osac-3523-mono-repo-installer-support branch. Retain that branch name in an
adjacent comment for traceability, and do not alter the workflow invocation
otherwise.
- Around line 79-80: Update installer-ref in both
.github/workflows/e2e-bmaas-full-install.yml lines 79-80 and
.github/workflows/e2e-caas-full-install.yml lines 86-87 to use the pull request
head SHA when available, while preserving the existing non-PR fallback. Ensure
the reusable workflow receives an immutable commit reference for installer-repo
so cloning and validation cannot follow a moved branch.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 63ce51e5-524b-4f94-b30f-7e541b2e7e9b
📒 Files selected for processing (5)
.github/workflows/e2e-bmaas-full-install.yml.github/workflows/e2e-caas-full-install.yml.github/workflows/e2e-vmaas-full-install.ymlosac-installer/.github/workflows/e2e-bmaas-full-install.ymlosac-installer/.github/workflows/e2e-vmaas-full-install.yml
💤 Files with no reviewable changes (2)
- osac-installer/.github/workflows/e2e-bmaas-full-install.yml
- osac-installer/.github/workflows/e2e-vmaas-full-install.yml
| needs: changes | ||
| if: needs.changes.outputs.should-run == 'true' | ||
| uses: osac-project/osac-test-infra/.github/workflows/e2e-bmaas-full-install.yml@main | ||
| uses: eliorerz/osac-test-infra/.github/workflows/e2e-bmaas-full-install.yml@osac-3523-mono-repo-installer-support |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin the reusable workflow to a commit SHA.
Line 70 references the mutable branch osac-3523-mono-repo-installer-support in an external repository. A branch update can change the workflow code between runs. Pin this reusable workflow to the 40-character commit SHA for the verification revision. Keep the branch name in a comment for traceability.
Suggested pin
- uses: eliorerz/osac-test-infra/.github/workflows/e2e-bmaas-full-install.yml@osac-3523-mono-repo-installer-support
+ uses: eliorerz/osac-test-infra/.github/workflows/e2e-bmaas-full-install.yml@<40-character-commit-sha> # osac-3523-mono-repo-installer-supportAs per path instructions, CI/CD actions must be pinned by full SHA, not tag.
🤖 Prompt for 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.
In @.github/workflows/e2e-bmaas-full-install.yml at line 70, Update the reusable
workflow reference in the e2e-bmaas-full-install workflow to use the required
40-character commit SHA instead of the mutable
osac-3523-mono-repo-installer-support branch. Retain that branch name in an
adjacent comment for traceability, and do not alter the workflow invocation
otherwise.
Source: Path instructions
| installer-repo: ${{ github.event.pull_request.head.repo.full_name || github.repository }} | ||
| installer-ref: ${{ github.event.pull_request.head.ref || github.ref_name }} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
check_workflow() {
local repo="$1"
local ref="$2"
local file="$3"
local url="https://raw.githubusercontent.com/${repo}/${ref}/.github/workflows/${file}"
echo "== ${url} =="
curl -fsSL "$url" |
rg -n -C 8 'workflow_call:|installer-(repo|ref)|head\.sha|ls-remote|rev-parse|actions/checkout'
}
check_workflow "eliorerz/osac-test-infra" "osac-3523-mono-repo-installer-support" "e2e-bmaas-full-install.yml"
check_workflow "osac-project/osac-test-infra" "main" "e2e-caas-full-install.yml"Repository: osac-project/osac
Length of output: 9880
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== workflow file sizes =="
wc -l .github/workflows/e2e-bmaas-full-install.yml .github/workflows/e2e-caas-full-install.yml
echo
echo "== reusable workflow installer input install blocks =="
for f in .github/workflows/e2e-bmaas-full-install.yml .github/workflows/e2e-caas-full-install.yml; do
echo "-- $f --"
sed -n '439,530p' "$f"
done
echo
echo "== all reusable workflows with installer inputs =="
rg -n -C 4 'inputs:|installer-(repo|ref):' .github/workflows || true
echo
echo "== workflow_call usages of target workflows =="
rg -n -C 8 'uses: .*e2e-(bmaas|caas)-full-install|e2e-(bmaas|c|caas)-full-install|\.github/workflows/e2e-(bmaas|caas)-full-install' .github/workflows .github/actions 2>/dev/null || trueRepository: osac-project/osac
Length of output: 19480
Resolve installer-ref to the PR head commit before cloning.
.github/workflows/e2e-bmaas-full-install.yml@osac-3523-mono-repo-installer-support declares and uses installer-repo/installer-ref, but it clones --branch "${installer-ref}"; pull request branch names can move between the caller's clone and the installer clone. Resolve ${{ github.event.pull_request.head.sha }} on the source repo before passing it to the reusable workflow, or resolve it inside the installer deploy script, so both BMaaS and CaaS validate against an immutable commit.
📍 Affects 2 files
.github/workflows/e2e-bmaas-full-install.yml#L79-L80(this comment).github/workflows/e2e-caas-full-install.yml#L86-L87
🤖 Prompt for 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.
In @.github/workflows/e2e-bmaas-full-install.yml around lines 79 - 80, Update
installer-ref in both .github/workflows/e2e-bmaas-full-install.yml lines 79-80
and .github/workflows/e2e-caas-full-install.yml lines 86-87 to use the pull
request head SHA when available, while preserving the existing non-PR fallback.
Ensure the reusable workflow receives an immutable commit reference for
installer-repo so cloning and validation cannot follow a moved branch.
|
Superseded — osac-test-infra#291 and osac#67 both merged and were verified fully green end-to-end (all 3 e2e suites passing) after the CI-wide 0.0.0 Helm-dependency-override bug was hotfixed in osac-test-infra#293. Closing without merging, per this PR's own stated purpose. |
…cret-back fix: Add cluster-fulfillment-ig secret back to overlays
This branch was previously deployed
1 participant
Second throwaway verification PR, needed because osac-project/osac-test-infra#291 gained a follow-up fix (resolved-commit-SHA comparison instead of ref-name comparison, addressing a CodeRabbit finding) after the first verification PR (#68) already confirmed the ref-name version. This PR points the bmaas caller's
uses:at eliorerz/osac-test-infra@osac-3523-mono-repo-installer-support (current tip) to confirm the SHA-comparison skip-splice path still works end to end. Will be closed (not merged) once verified. See #67 and osac-project/osac-test-infra#291 for the real PRs.Summary by CodeRabbit
CI Improvements
Maintenance