OSAC-3365: Point hardcoded fulfillment-service release/CLI refs at osac repo - #286
Conversation
…ac repo
fulfillment-service has cut over into the osac mono-repo (its own repo is
now a frozen mirror pending archive). Update every hardcoded reference
to fulfillment-service's GitHub releases/repo confirmed via direct file
reads:
- Containerfile: osac CLI release download (latest-version lookup,
binary, checksums file).
- scripts/machine-init.sh: same release-download pattern for local dev
bootstrap (install_osac()).
- fleet/roles/machine_base/tasks/osac.yml and fleet/inventory/group_vars/all.yml:
same pattern for fleet-managed CI runner bootstrap.
- infra/netris/inventory/group_vars/all.yml: fulfillment_service_repo
default.
The checksums asset itself is still named
fulfillment-service_<version>_checksums.txt on osac's releases (verified
against actual published osac-project/osac releases) -- only the repo
path changes, not the asset naming.
Investigated but found already fixed by prior work (no change needed):
e2e-{bmaas,vmaas,caas}-full-install.yml's rebuild-CLI-from-source check
is now keyed off imageKey ("service.images.service"), not a
COMPONENT_REPO substring match, so it already works for a monorepo
caller.
Beyond the literal URL swaps, fulfillment_service_repo is also consumed
by roles/osac_refresh and roles/osac_install (not explicitly listed in
the ticket, found via grep) to clone fulfillment-service directly into
osac-installer's base/osac-fulfillment-service submodule directory,
which expects a single-component layout (cmd/osac, charts/service/, at
its root). Pointing that variable at the mono-repo without adjusting for
this breaks both roles, since the code now lives one level deeper under
fulfillment-service/. Added a flatten step (clone, then move
fulfillment-service/'s contents up to the submodule root and drop
everything else) to both roles, mirroring the equivalent per-component
subdirectory scoping already used for GH Actions e2e runs in
.github/scripts/replace-installer-submodule.sh. It's a no-op if
fulfillment_service_repo is ever overridden back to a non-mono-repo,
single-component checkout.
Also fixed roles/lab_setup's own osac CLI build step the same way: its
clone destination directory keeps the name "fulfillment-service" as this
component's logical source location (unaffected), but the build step's
chdir now points one level deeper into that checkout's own
fulfillment-service/ subdirectory.
Part of OSAC-1732 (Repository Consolidation mono-repo epic).
|
@eliorerz: This pull request references OSAC-3365 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. 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. |
|
[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 |
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
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 |
|
Reviewed the full diff plus surrounding context (checked out the branch, verified against live GitHub state, not just reading the description). The flatten logic and the individual URL edits are mechanically correct — but there are real gaps around the release pipeline this all depends on. 1. The PR description says the checksums asset naming was "verified against actual published Practically: before this PR, 2. Open PR #29 ( This PR's 3. Missed hardcoded reference: the credential-leak audit tooling still targets the old repo.
4. Minor: a few stale example strings weren't updated.
Separately verified and confirmed correct: the |
Addresses PR review findings on this branch: - .github/scripts/discover-audit-runs.sh and .github/workflows/audit-workflow-logs.yml: the credential-leak audit's EXTERNAL_CALLERS/KNOWN_TARGET_REPOS lists (a bare repo slug, not a URL, so not caught by the earlier github.com/osac-project/fulfillment-service grep) still only listed the standalone fulfillment-service/osac-operator repos as e2e audit targets. Since those e2e runs also happen under osac-project/osac now, the daily audit sweep was blind to them. Added osac-project/osac (all three e2e workflows) alongside the existing entries -- not in place of them, since the standalone repos are still actively running e2e (confirmed: fulfillment-service had a completed e2e-vmaas-full-install.yml run within the last day). - Fixed four stale ghcr.io/osac-project/fulfillment-service:pr-123 example strings (image-overrides input descriptions in e2e-vmaas.yml, e2e-vmaas-full-install.yml, e2e-bmaas-full-install.yml, and e2e-caas-netris-full-install-caller.yml) to ghcr.io/osac-project/osac -- confirmed against osac-project/osac's actual build-image.yaml/ publish-image.yaml (REGISTRY: ghcr.io, IMAGE_NAME: github.repository). Investigated and left unchanged: infra/netris/README.md's fulfillment_service_image example still says quay.io/osac-project/fulfillment-service:feature-x. This is a generic "pass your own custom test image here" illustration, not a reference to any actual CI-published path -- structurally identical to the untouched, out-of-scope osac_operator_image example two lines above it (quay.io/osac-project/osac-operator:feature-x), which doesn't match its own component's real ghcr.io publish path either. Changing only the fulfillment-service one would break that parallel structure without fixing an actual inaccuracy.
|
Thanks for the thorough review -- verified all four points independently and updated accordingly. 1. Release pipeline (blocker): Confirmed -- 2. Coordination with #29 / OSAC-3467: Agreed these are stacking unverified assumptions on the same untested pipeline. Called this out explicitly in the PR description and tagged the dependency. Haven't pushed a test tag myself for the same reason #29 didn't -- don't want to trigger a real release/image build/chart publish outside a coordinated dry run. Recommend whoever owns the dry run tests both PRs' assumptions together (tag shape from #29, download URLs from this PR) in one pass. 3. Audit target lists: Good catch, missed this since it's a bare repo slug not a URL. Added 4. Stale ghcr.io examples: Fixed the three you flagged, plus found a fourth instance in New commit: 11073ec. Updated PR description with full detail on all of the above. |
|
Confirmed root cause of all three failing e2e jobs (bmaas 30605247632, caas 30605247675, vmaas 30605247714): each fails at the identical `Containerfile` `STEP 10/16` (`Build test image`), with byte-identical error text: ``` Exit 22 = curl `--fail` hitting an HTTP error -- `osac-project/osac` has zero releases, so `releases/latest` 404s and the CLI install (and the whole test-image build) aborts before any test runs. This is empirical confirmation of the merge-sequencing dependency already flagged above, not a new/different bug -- these e2e runs will stay red on this branch until a first `osac-project/osac` release exists. Not touching the code; the URL logic is correct for the intended end state. |
|
/retest |
|
Re-triggered failed runs:
|
|
Confirmed the actual failure cause via job logs (not the originally-flagged zero-releases blocker, which is now resolved -- osac has 4 releases). Real bug: osac-project/osac now has a mix of tag-scoped releases (osac-operator/v0.0.12, osac-aap/v0.0.13, bare-metal-fulfillment-operator/v0.0.11 -- properly component-prefixed per OSAC-3467) alongside fulfillment-service's own CLI release, which published under a bare v0.0.81 tag, NOT fulfillment-service/v0.0.81. Since BMF's release published later, GET /releases/latest now returns BMF's tag_name, and this PR's ltrimstr("v")-based version resolution (which assumes the latest release is always fulfillment-service's bare-tag release) constructs a garbage download URL -> curl exit 22 -> container build fails. Confirmed in job log: https://github.com/osac-project/osac-test-infra/actions/runs/30605247632/job/91173924719 Needs: (1) fix the OSAC_VERSION resolution to find fulfillment-service's own latest release specifically (filter releases for a bare vX.Y.Z tag pattern, not blanket releases/latest), (2) separately worth asking why fulfillment-service's release isn't prefixed fulfillment-service/vX.Y.Z like the other 3 mono-repo components -- may be a gap in OSAC-3467's original rollout worth its own look. |
…s own tag osac-project/osac now hosts releases for multiple components, each tagged <component>/vX.Y.Z per OSAC-3467 (osac-operator/v0.0.12, osac-aap/v0.0.13, bare-metal-fulfillment-operator/v0.0.11), alongside fulfillment-service's own release which still publishes under a bare vX.Y.Z tag. Both the Containerfile's CLI-install step and machine-init.sh's install_osac() blindly trusted GET .../releases/latest (respectively via the JSON API and by following its redirect) to be fulfillment-service's release -- once a component-prefixed release became more recent, releases/latest started returning that instead, producing a garbage download URL and failing every e2e job's test-image build with curl exit 22. Fixed both to list releases explicitly (?per_page=100) and filter for a release whose tag matches a bare vX.Y.Z pattern (i.e. NOT component- prefixed), taking the most recent match -- verified against the live osac-project/osac release list, correctly resolves to fulfillment-service's 0.0.81 while skipping the three prefixed releases. Both now fail loudly with a clear error instead of constructing a bad URL if no such release is found. Verified with shellcheck (clean on both files) and a live dry run of the resolution logic against the real GitHub API and the resulting download URL (confirmed downloadable, HTTP 200); the destination write itself wasn't exercisable outside a real container/root context. Note: this filter is coupled to fulfillment-service's tags staying unprefixed. If a follow-up ever changes fulfillment-service to publish fulfillment-service/vX.Y.Z like the other three components, this bare-tag filter will need updating too -- flagged separately for investigation.
Found while validating the version-resolution fix end-to-end: the checksums asset on osac-project/osac's release is named osac_<version>_checksums.txt, not fulfillment-service_<version>_checksums.txt like the old standalone repo used (confirmed by comparing against fulfillment-service's own v0.0.79 release). The goreleaser project identifier this filename derives from apparently changed to "osac" at some point during the monorepo migration -- the file's actual content is unaffected (still has an osac_Linux_x86_64 line the existing grep expects), only its name changed. Without this, the Containerfile's checksums download would 404 even after the version-resolution fix, since it was still requesting the old filename pattern. Verified end-to-end: resolved version, downloaded binary, downloaded checksums, and confirmed sha256sum -c passes against the real released binary.
Do not merge until
osac-project/osachas cut at least one real release. Verified:osac-project/osaccurrently has zero releases and zero tags (releases→[],releases/latest→ 404,tags→[]). Before this PR,Containerfile/machine-init.sh/the fleet role pointed atfulfillment-service, which has releases (dozens, up to v0.0.79) -- so CLI installs work today. Merging this as-is would point those same install flows at a repo with nothing to download, breaking them until a firstosacrelease exists.Related: osac-project/osac#29 ("Scope fulfillment-service/osac-operator release-trigger tags", OSAC-3467) is changing the tag scheme that triggers that first release, and per its own description no tag has been pushed there either -- so what
tag_namea realosacrelease actually gets (barevX.Y.Z, matching this PR'sltrimstr("v")/v${OSAC_VERSION}assumptions, vs. something else) is inferred from #29'sgoreleaser --current-tagoverride, not confirmed end-to-end. These two PRs are stacking unverified assumptions about the same untested release pipeline -- recommend one real end-to-end dry run (push an actualfulfillment-service/vX.Y.Ztest tag and confirm the resulting release'stag_nameand asset names) before merging either.Summary
fulfillment-service has cut over into the osac mono-repo (its own repo is now a frozen mirror pending archive). This updates every hardcoded reference to fulfillment-service's GitHub releases/repo, confirmed via direct file reads (not assumptions):
Containerfile: osac CLI release download (latest-version lookup, binary, checksums file).scripts/machine-init.sh: same release-download pattern for local dev bootstrap (install_osac()).fleet/roles/machine_base/tasks/osac.ymlandfleet/inventory/group_vars/all.yml: same pattern for fleet-managed CI runner bootstrap.infra/netris/inventory/group_vars/all.yml:fulfillment_service_repodefault.The checksums asset itself is still named
fulfillment-service_<version>_checksums.txton osac's releases (verified againstosac-project/fulfillment-service's actual release asset naming -- see the merge-sequencing note above re:osac-project/osacnot having any releases yet to check directly) -- only the repo path changes, not the asset naming.Investigated but already fixed by prior work (no change needed):
e2e-{bmaas,vmaas,caas}-full-install.yml's rebuild-CLI-from-source check is now keyed offimageKey("service.images.service"), not aCOMPONENT_REPOsubstring match, so it already works correctly for a monorepo caller.Found via investigation, beyond the ticket's literal list:
fulfillment_service_repois also consumed byroles/osac_refreshandroles/osac_installto clone fulfillment-service directly into osac-installer'sbase/osac-fulfillment-servicesubmodule directory, which expects a single-component layout (cmd/osac,charts/service/, at its root). Pointing that variable at the mono-repo without adjusting for this would silently break both roles (both are actively wired into the livee2e-caas-netris.ymlworkflow), since the code now lives one level deeper underfulfillment-service/. Added a flatten step to both roles (clone, then movefulfillment-service/'s contents up to the submodule root and drop everything else), mirroring the equivalent per-component subdirectory scoping already used for GH Actions e2e runs in.github/scripts/replace-installer-submodule.sh. It's a no-op iffulfillment_service_repois ever pointed back at a non-mono-repo, single-component checkout.Also fixed
roles/lab_setup's own osac CLI build step the same way: its clone destination directory keeps the namefulfillment-serviceas this component's logical source location (unaffected), but the build step'schdirnow points one level deeper into that checkout's ownfulfillment-service/subdirectory.Also fixed, from review (additional findings beyond the ticket's literal URL list):
.github/scripts/discover-audit-runs.shand.github/workflows/audit-workflow-logs.yml: the credential-leak audit's target lists used a bareosac-project/fulfillment-servicerepo slug (not a URL, so not caught by my original grep). Since fulfillment-service's e2e runs also happen underosac-project/osacnow, the daily audit sweep was blind to them. Addedosac-project/osac(all three e2e workflows) alongside the existing standalone-repo entries -- not in place of them, sinceosac-project/fulfillment-serviceis confirmed still actively running e2e (a completede2e-vmaas-full-install.ymlrun within the last day).ghcr.io/osac-project/fulfillment-service:pr-123example strings (image-overridesinput descriptions ine2e-vmaas.yml,e2e-vmaas-full-install.yml,e2e-bmaas-full-install.yml,e2e-caas-netris-full-install-caller.yml-- one more than review caught) toghcr.io/osac-project/osac, confirmed againstosac-project/osac's actualbuild-image.yaml/publish-image.yaml(REGISTRY: ghcr.io,IMAGE_NAME: github.repository).infra/netris/README.md'squay.io/osac-project/fulfillment-service:feature-xexample and left it unchanged: it's a generic "pass your own custom test image here" illustration, not a reference to any real CI-published path -- structurally identical to the untouched, out-of-scopeosac_operator_imageexample two lines above it (quay.io/osac-project/osac-operator:feature-x), which doesn't match its own component's realghcr.iopublish path either. Changing only the fulfillment-service one would break that parallel structure without fixing an actual inaccuracy.Part of OSAC-1732 (Repository Consolidation mono-repo epic).
Note for reviewers
The
osac_refresh/osac_installflatten logic was functionally simulated locally against a mock directory tree (monorepo-shaped and non-monorepo-shaped, including hidden dotfiles) and behaves correctly in both cases -- independently reproduced in review -- but neither of us could run it against real netris/CaaS infrastructure. Worth a livee2e-caas-netris.ymldry run withfulfillment-service-branchset, given this touches an actively-used E2E path.Test plan
github.com/osac-project/fulfillment-serviceafter the change -- no remaining hits in the fixed filespre-commit run yamllintandpre-commit run ansible-lintpass on all touched YAML/Ansible filesshellcheckclean on the new flatten logic (extracted and checked standalone) and on the updateddiscover-audit-runs.shactionlintclean on all touched workflow filesosac-project/osacrelease