MGMT-23844: renames fulfillment-cli to osac in osac-installer - #63
Conversation
|
@wgordon17: This pull request references MGMT-23842 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 epic 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. |
|
Skipping CI for Draft Pull Request. |
ac40994 to
2caf38c
Compare
wgordon17
changed the title
|
@wgordon17: This pull request references MGMT-23844 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 story 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. |
f9704f1 to
c4eb118
Compare
|
@wgordon17: This pull request references MGMT-23844 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 story 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. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 56 minutes and 15 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThis pull request updates CLI branding throughout the codebase by replacing "Fulfillment CLI" with "OSAC CLI". Changes include renaming command invocations from Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 3
🧹 Nitpick comments (4)
OSAC-CLI-HOWTO.md (2)
1821-1822:OSAC_ENVis also illustrative, not CLI-consumed.Same note as above — just flagging so readers don't assume this is recognized by the
osacbinary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@OSAC-CLI-HOWTO.md` around lines 1821 - 1822, The document currently shows OSAC_ENV and config_file as if OSAC_ENV is consumed by the osac CLI; clarify that OSAC_ENV is illustrative only and not read by the osac binary by adding a short note next to the OSAC_ENV example (referencing the symbols OSAC_ENV and config_file) stating it is an illustrative environment variable and that the osac binary does not consume OSAC_ENV (mention the osac binary explicitly) so readers won't assume the CLI recognizes it.
239-260:OSAC_CONFIG_DEV/OSAC_CONFIG_PRODare example shell variables, not CLI-recognized env vars.Nit: these are just local shell convenience variables used by the
cpcommands in the examples, not environment variables the CLI itself consumes. A reader skimming might assumeOSAC_CONFIG_*is a supported mechanism for config selection. Consider either (a) clarifying the text to call these out as ad-hoc shell vars, or (b) using lowercase names likedev_configto avoid confusion with genuineOSAC_*env vars the binary may support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@OSAC-CLI-HOWTO.md` around lines 239 - 260, The examples use OSAC_CONFIG_DEV and OSAC_CONFIG_PROD which look like official env vars but are only local shell convenience variables; update the docs to either (a) add a short clarifying sentence above the blocks stating "OSAC_CONFIG_DEV/OSAC_CONFIG_PROD are ad-hoc local shell variables used only in these examples and are not consumed by the CLI", or (b) rename the example variables to non-OSAC names such as dev_config/prod_config and update the subsequent cp commands that reference them as well as the target ~/.config/osac/config.json so readers won't confuse them with real OSAC_* environment variables. Ensure you change every occurrence of OSAC_CONFIG_DEV and OSAC_CONFIG_PROD in the snippet (including the cp lines and any explanatory text).scripts/prepare-fulfillment-service.sh (2)
22-23: Verifyosacis on PATH before invocation.Under
set -o errexit, ifosacis not installed the script fails at line 22 with an opaque "command not found". The README already notes this requirement, but sincesetup.shcalls this unconditionally, consider a preflight check (command -v osac >/dev/null || { echo "ERROR: 'osac' not found on PATH"; exit 1; }) to produce a clearer error — especially given the rename will trip up users who still havefulfillment-cliinstalled instead ofosac.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/prepare-fulfillment-service.sh` around lines 22 - 23, Add a preflight check to verify the osac binary exists before the script calls osac (i.e., before the `osac login` / `osac create hub` invocations) so the script fails with a clear message under `set -o errexit`; use `command -v osac` (or similar) and exit with an explanatory error like "ERROR: 'osac' not found on PATH (you may have 'fulfillment-cli' installed)" if not found. Ensure the check runs early in the script and prevents executing the `osac login` and `osac create hub` commands when the binary is missing.
27-27: Consider renaming this script file for consistency.With
fulfillment-cligone, the filenameprepare-fulfillment-service.shand theFULFILLMENT_API_URLvariable/route name at line 21 are the only remnants of the old naming in this script. Thefulfillment-apiroute is a server-side resource (likely still named that way in manifests, so leave it), but a follow-up renaming this script to e.g.prepare-osac.shorregister-hub.shwould complete the cleanup. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/prepare-fulfillment-service.sh` at line 27, The script filename and internal variable names still reference the old "fulfillment" naming; rename the script (e.g., prepare-osac.sh or register-hub.sh) and update in-script variables and comments for consistency: change the script name and any occurrences of FULFILLMENT_API_URL (and other "FULFILLMENT" identifiers) to the new chosen name (e.g., OSAC_API_URL or HUB_API_URL) while leaving server-side manifest route names untouched, and update any CI/job, README, or invocation references to this script to use the new filename.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@OSAC-CLI-HOWTO.md`:
- Around line 150-154: The HOWTO currently tells users to create
~/.config/osac/config.json but the CLI binary (fulfillment-cli) actually reads
~/.config/fulfillment-cli/config.json; pick one resolution and apply it
consistently: either update all documentation occurrences of
"~/.config/osac/config.json" (and the mkdir example) to
"~/.config/fulfillment-cli/config.json" and search/replace the other mentioned
occurrences, or modify the CLI startup/config-loading code to look for
"~/.config/osac/config.json" (and its mkdir/create logic) so the docs remain
correct; ensure the chosen symbol names (the string paths
"~/.config/osac/config.json" and "~/.config/fulfillment-cli/config.json" and the
binary name "fulfillment-cli") are updated wherever they appear.
In `@prerequisites/keycloak/service/files/realm.json`:
- Line 100: The realm.json change renames the Keycloak client from
fulfillment-cli to osac-cli and is a breaking upgrade because Keycloak's
--import-realm (configMap-mounted realm.json) will not automatically rename
existing clients; update the PR/changelog and release notes to clearly state
this is a breaking change for upgrades, that the provided realm.json only
supports greenfield installs, and include explicit remediation instructions for
operators (manual client migration or data remediation) referencing the old
clientId "fulfillment-cli", the new clientId "osac-cli", and the import
mechanism "--import-realm" so maintainers know to perform a manual migration for
existing Keycloak instances.
In `@README.md`:
- Around line 383-385: Update the curl download URL asset name used in the
README: change the filename token "osac_Linux_x86_64" in the curl command to the
actual release asset name "fulfillment-service_Linux_x86_64" so the URL becomes
https://github.com/osac-project/fulfillment-service/releases/latest/download/fulfillment-service_Linux_x86_64;
keep the local output filename ("-o osac") and the subsequent chmod +x osac
unchanged.
---
Nitpick comments:
In `@OSAC-CLI-HOWTO.md`:
- Around line 1821-1822: The document currently shows OSAC_ENV and config_file
as if OSAC_ENV is consumed by the osac CLI; clarify that OSAC_ENV is
illustrative only and not read by the osac binary by adding a short note next to
the OSAC_ENV example (referencing the symbols OSAC_ENV and config_file) stating
it is an illustrative environment variable and that the osac binary does not
consume OSAC_ENV (mention the osac binary explicitly) so readers won't assume
the CLI recognizes it.
- Around line 239-260: The examples use OSAC_CONFIG_DEV and OSAC_CONFIG_PROD
which look like official env vars but are only local shell convenience
variables; update the docs to either (a) add a short clarifying sentence above
the blocks stating "OSAC_CONFIG_DEV/OSAC_CONFIG_PROD are ad-hoc local shell
variables used only in these examples and are not consumed by the CLI", or (b)
rename the example variables to non-OSAC names such as dev_config/prod_config
and update the subsequent cp commands that reference them as well as the target
~/.config/osac/config.json so readers won't confuse them with real OSAC_*
environment variables. Ensure you change every occurrence of OSAC_CONFIG_DEV and
OSAC_CONFIG_PROD in the snippet (including the cp lines and any explanatory
text).
In `@scripts/prepare-fulfillment-service.sh`:
- Around line 22-23: Add a preflight check to verify the osac binary exists
before the script calls osac (i.e., before the `osac login` / `osac create hub`
invocations) so the script fails with a clear message under `set -o errexit`;
use `command -v osac` (or similar) and exit with an explanatory error like
"ERROR: 'osac' not found on PATH (you may have 'fulfillment-cli' installed)" if
not found. Ensure the check runs early in the script and prevents executing the
`osac login` and `osac create hub` commands when the binary is missing.
- Line 27: The script filename and internal variable names still reference the
old "fulfillment" naming; rename the script (e.g., prepare-osac.sh or
register-hub.sh) and update in-script variables and comments for consistency:
change the script name and any occurrences of FULFILLMENT_API_URL (and other
"FULFILLMENT" identifiers) to the new chosen name (e.g., OSAC_API_URL or
HUB_API_URL) while leaving server-side manifest route names untouched, and
update any CI/job, README, or invocation references to this script to use the
new filename.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ce90b1c1-0377-49f4-abbc-f3ad986a0c82
📒 Files selected for processing (5)
OSAC-CLI-HOWTO.mdREADME.mdprerequisites/keycloak/service/files/realm.jsonscripts/prepare-fulfillment-service.shscripts/setup.sh
| ], | ||
| "client": { | ||
| "fulfillment-cli": [], | ||
| "osac-cli": [], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find keycloak realm import strategy
rg -nP --type=yaml -C3 '(import[_-]?strategy|importStrategy|KEYCLOAK_IMPORT)' prerequisites/keycloak/
fd -a . prerequisites/keycloak/ | head -50Repository: osac-project/osac-installer
Length of output: 1529
🏁 Script executed:
cat -n prerequisites/keycloak/service/deployment.yaml | head -100Repository: osac-project/osac-installer
Length of output: 3784
🏁 Script executed:
# Look for import-related environment variables or volume mounts in deployment
rg -n 'KEYCLOAK|import|realm' prerequisites/keycloak/service/deployment.yaml -C 2Repository: osac-project/osac-installer
Length of output: 1067
🏁 Script executed:
# Check the full realm.json to understand the change scope
wc -l prerequisites/keycloak/service/files/realm.jsonRepository: osac-project/osac-installer
Length of output: 123
🏁 Script executed:
# Get context around the clientId changes mentioned (lines 100, 912-915)
sed -n '95,110p' prerequisites/keycloak/service/files/realm.json
sed -n '907,920p' prerequisites/keycloak/service/files/realm.jsonRepository: osac-project/osac-installer
Length of output: 926
🏁 Script executed:
# Search for all occurrences of both old and new clientId in realm.json
rg -c 'fulfillment-cli|osac-cli' prerequisites/keycloak/service/files/realm.jsonRepository: osac-project/osac-installer
Length of output: 72
🏁 Script executed:
# Check git history of realm.json to see how clientId changed
git log --oneline prerequisites/keycloak/service/files/realm.json | head -20Repository: osac-project/osac-installer
Length of output: 140
🏁 Script executed:
# Search for migration scripts or upgrade documentation
fd -type f -name '*migrat*' -o -name '*upgrade*' -o -name '*CHANGELOG*' -o -name '*MIGRATION*' prerequisites/Repository: osac-project/osac-installer
Length of output: 241
🏁 Script executed:
# Search for any references to fulfillment-cli in the entire codebase
rg -r 'fulfillment-cli' . --type yaml --type json --type mdRepository: osac-project/osac-installer
Length of output: 50385
🏁 Script executed:
# Check for any version or breaking changes documentation
rg -n 'breaking|BREAKING|5\.0|version' prerequisites/keycloak/ --type yaml --type jsonRepository: osac-project/osac-installer
Length of output: 337
Document the breaking change for upgrades.
This is an intentional rename (MGMT-23844) confirmed in git history. The deployment uses Keycloak's --import-realm mechanism with a configMap-mounted realm.json. The current realm.json contains only the new osac-cli clientId and no trace of the old fulfillment-cli.
For greenfield deployments, the import works as expected. However, for existing Keycloak instances with provisioned users/tokens against fulfillment-cli, the realm import will not automatically rename the old client—it will either remain unchanged or coexist with the new one depending on Keycloak's import strategy. Since no migration scripts or upgrade path documentation exists in the codebase, add a clear note in the changelog/PR description clarifying:
- This is a breaking change for upgrades
- Only greenfield installs are supported with this realm definition
- Existing deployments require manual client migration or data remediation
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@prerequisites/keycloak/service/files/realm.json` at line 100, The realm.json
change renames the Keycloak client from fulfillment-cli to osac-cli and is a
breaking upgrade because Keycloak's --import-realm (configMap-mounted
realm.json) will not automatically rename existing clients; update the
PR/changelog and release notes to clearly state this is a breaking change for
upgrades, that the provided realm.json only supports greenfield installs, and
include explicit remediation instructions for operators (manual client migration
or data remediation) referencing the old clientId "fulfillment-cli", the new
clientId "osac-cli", and the import mechanism "--import-realm" so maintainers
know to perform a manual migration for existing Keycloak instances.
| $ curl -L -o osac \ | ||
| https://github.com/osac-project/fulfillment-service/releases/latest/download/osac_Linux_x86_64 | ||
| $ chmod +x osac |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
osac-project/fulfillment-service latest release asset filenames
💡 Result:
The latest release of osac-project/fulfillment-service is v0.0.46, released on 2026-03-11. No specific asset filenames (e.g., binaries, source archives, checksums) are listed in available sources. The repository has 11 releases total, but detailed release page content with assets was not surfaced in search results. Typical GitHub Go project releases include fulfillment-service_v0.0.46_linux_amd64.tar.gz, fulfillment-service_v0.0.46_darwin_amd64.tar.gz, checksums.txt, but confirmation requires direct access to https://github.com/osac-project/fulfillment-service/releases/tag/v0.0.46.
Citations:
- 1: https://github.com/osac-project/fulfillment-service
- 2: https://github.com/osac-project/fulfillment-service/activity
- 3: https://www.movnn.com
🌐 Web query:
site:github.com/osac-project/fulfillment-service/releases/tag/v0.0.46
💡 Result:
osac-project/fulfillment-service — Release v0.0.46 (tag v0.0.46)
- Released by @eranco74 on 11 Mar (11:13); tag points to commit
4a90b67. [1] - What’s changed: [1]
- Bump
goreleaser/goreleaser-action6 → 7 (Dependabot) (#322) - Bump
docker/login-action3 → 4 (Dependabot) (#320) - Bump
actions/upload-artifact6 → 7 (Dependabot) (#321) - Bump Go dependencies group (7 updates) (Dependabot) (
#316) - Release binaries directly instead of archives (
@jhernand) (#323) - MGMT-23194: Add Networking Resources to Private API (
@eranco74) (#314) - Implement Core Networking Infrastructure (
@eranco74) (#324)
- Bump
- Full changelog compares
v0.0.45...v0.0.46. [1]
Source: GitHub release page for v0.0.46. [1]
🌐 Web query:
osac-project fulfillment-service github releases linux asset binary
💡 Result:
The osac-project/fulfillment-service GitHub repository (https://github.com/osac-project/fulfillment-service) is a Go-based cloud-in-a-box fulfillment service for the Open Sovereign AI Cloud (OSAC) project. It has 11 releases, with the latest being v0.0.46 released on 2026-03-11. No evidence of pre-built Linux binary assets in the releases was found across multiple searches targeting releases and assets. The README explicitly instructs users to build the binary themselves using 'go build' after installing Go and other development tools (Buf, Ginkgo, etc.). The project supports running on Linux (e.g., Podman for containers, PostgreSQL setup), but binaries must be compiled from source. To obtain the Linux binary: 1. Clone the repo: git clone https://github.com/osac-project/fulfillment-service 2. cd fulfillment-service 3. go build This produces the ./fulfillment-service executable, which can run the gRPC server or REST gateway as documented (e.g., ./fulfillment-service start grpc-server --grpc-listener-address=localhost:8000 --db-url=postgres://user:pass@localhost:5432/db). Note: A related fulfillment-cli repo mentions pre-built binaries from releases, but not for fulfillment-service itself.
Citations:
- 1: https://github.com/osac-project/fulfillment-service
- 2: https://github.com/osac-project/fulfillment-cli
🏁 Script executed:
# Search for any GitHub workflows that might reveal release asset naming
curl -s https://api.github.com/repos/osac-project/fulfillment-service/releases/latest 2>/dev/null | jq '.assets[].name' || echo "API call may have failed in sandbox"Repository: osac-project/osac-installer
Length of output: 531
🌐 Web query:
"fulfillment-service_Linux_x86_64" v0.0.46 github release osac-project
💡 Result:
The "fulfillment-service_Linux_x86_64" v0.0.46 is the Linux x86_64 binary asset from the latest release of the osac-project/fulfillment-service GitHub repository. This is a Go-based gRPC/REST service for "Cloud-in-a-box fulfillment" in the Open Sovereign AI Cloud (OSAC) project, used for managing cluster orders, templates, and deployments in OpenShift/Kubernetes environments. It requires PostgreSQL for state and supports deployment via Podman/Docker to clusters. Release details: - Repository: https://github.com/osac-project/fulfillment-service - Latest release: v0.0.46, published 2026-03-11 - Total releases: 11 - License: Apache-2.0 To build the binary yourself: Clone the repo and run go build. The specific pre-built Linux x86_64 binary named "fulfillment-service_Linux_x86_64" (likely versioned for v0.0.46) is available for download from the release assets page: https://github.com/osac-project/fulfillment-service/releases/tag/v0.0.46 Usage example (gRPC server): ./fulfillment-service start grpc-server --log-level=debug --grpc-listener-address=localhost:8000 --db-url=postgres://user:pass@localhost:5432/db
Citations:
- 1: https://github.com/osac-project/fulfillment-service
- 2: https://github.com/osac-project/fulfillment-service/activity
🏁 Script executed:
# Try to access the GitHub API for the exact fulfillment-service v0.0.46 release
python3 << 'EOF'
import json
import urllib.request
import urllib.error
try:
# Query GitHub API for v0.0.46 release of fulfillment-service
url = "https://api.github.com/repos/osac-project/fulfillment-service/releases/tags/v0.0.46"
req = urllib.request.Request(url)
req.add_header('User-Agent', 'Mozilla/5.0')
with urllib.request.urlopen(req, timeout=10) as response:
data = json.loads(response.read().decode())
print("Release: ", data.get('tag_name'))
print("Assets:")
for asset in data.get('assets', []):
print(f" - {asset['name']}")
except urllib.error.HTTPError as e:
print(f"HTTP Error: {e.code}")
except Exception as e:
print(f"Error: {e}")
EOFRepository: osac-project/osac-installer
Length of output: 205
Update asset name in download URL to fulfillment-service_Linux_x86_64.
The README specifies osac_Linux_x86_64, but the actual release asset is named fulfillment-service_Linux_x86_64. The curl command will fail with a 404 for users following the docs.
Current code (lines 383-385)
$ curl -L -o osac \
https://github.com/osac-project/fulfillment-service/releases/latest/download/osac_Linux_x86_64
$ chmod +x osac
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 383 - 385, Update the curl download URL asset name
used in the README: change the filename token "osac_Linux_x86_64" in the curl
command to the actual release asset name "fulfillment-service_Linux_x86_64" so
the URL becomes
https://github.com/osac-project/fulfillment-service/releases/latest/download/fulfillment-service_Linux_x86_64;
keep the local output filename ("-o osac") and the subsequent chmod +x osac
unchanged.
Signed-off-by: Will Gordon <wgordon@redhat.com> Generated-By: Claude Code (Anthropic) Signed-off-by: Will Gordon <wgordon@redhat.com>
c4eb118 to
df2ce5c
Compare
|
@achuzhoy: changing LGTM is restricted to collaborators 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: achuzhoy, trewest, wgordon17 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 |
4 participants
Summary
Summary by CodeRabbit
Release Notes
Documentation
Chores