NO_ISSUE: rename fulfillment-cli to osac in Containerfile - #30
Conversation
|
@omer-vishlitzky: This pull request references MGMT-22635 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe Containerfile is updated to replace the CLI binary installation from Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Containerfile`:
- Around line 16-17: The container installs the osac binary but the test
automation expects the command fulfillment-cli (referenced via
fulfillment_cli_path), so add a symlink after installing /usr/local/bin/osac to
provide /usr/local/bin/fulfillment-cli; update the RUN step that installs osac
to create a symbolic link from /usr/local/bin/osac to
/usr/local/bin/fulfillment-cli (and ensure the chmod +x remains) so roles
invoking fulfillment-cli succeed.
- Around line 16-17: The Containerfile currently downloads the osac CLI with an
incorrect OSAC_CLI_VERSION and without integrity verification; change
OSAC_CLI_VERSION to a published tag (e.g., v0.0.46) and update the RUN that
fetches /usr/local/bin/osac accordingly, then add one of the following integrity
mitigations: (a) switch to a release workflow that supports
signed/artifact-verified downloads (e.g., use sigstore/cosign or image
attestation) and verify signatures before installing, (b) pin the exact release
and add a documented manual source/artifact review step in the repo (risk
acceptance), or (c) embed a verified build step that compiles osac from tagged
source and verifies git commit/tag before installing; reference
OSAC_CLI_VERSION, the RUN curl line that writes /usr/local/bin/osac, and the
/usr/local/bin/osac install step when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| RUN curl -Lsfo /usr/local/bin/osac "https://github.com/osac-project/fulfillment-service/releases/download/v${OSAC_CLI_VERSION}/osac_Linux_x86_64" \ | ||
| && chmod +x /usr/local/bin/osac |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether repo still invokes fulfillment-cli by name.
rg -n -C2 '\bfulfillment_cli_path\b|fulfillment-cli\b|cli_binary\b' --type yml --type yamlRepository: osac-project/osac-test-infra
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Verify fulfillment-cli usage with correct file type syntax
rg -n -C2 'fulfillment_cli_path|fulfillment-cli' --type yamlRepository: osac-project/osac-test-infra
Length of output: 8549
🏁 Script executed:
#!/bin/bash
# Also check if there's already a symlink or alias in the Containerfile
cat -n ContainerfileRepository: osac-project/osac-test-infra
Length of output: 1184
🏁 Script executed:
#!/bin/bash
# Check the referenced files to confirm they still use fulfillment-cli
echo "=== inventory/group_vars/all.yml (line 8) ==="
head -20 inventory/group_vars/all.yml
echo -e "\n=== roles/fulfillment_cli_base/tasks/main.yml ==="
cat roles/fulfillment_cli_base/tasks/main.yml 2>/dev/null || echo "File not found"
echo -e "\n=== roles/test_compute_instance_creation/tasks/main.yml ==="
head -20 roles/test_compute_instance_creation/tasks/main.yml 2>/dev/null || echo "File not found"Repository: osac-project/osac-test-infra
Length of output: 3636
🏁 Script executed:
#!/bin/bash
# Check if group_vars/all.yml was modified to use osac instead
git diff HEAD^ HEAD -- inventory/group_vars/all.yml 2>/dev/null || echo "Not a git repo or no changes to group_vars"
# Also check for any role defaults that might override the path
rg -n "fulfillment_cli_path.*osac|cli_binary.*osac" --type yamlRepository: osac-project/osac-test-infra
Length of output: 114
🏁 Script executed:
#!/bin/bash
# Check if there are any other mechanisms to create the fulfillment-cli alias
rg -n "ln -sf|ln -s|alias|fulfillment-cli" Containerfile || echo "No symlink/alias in Containerfile"Repository: osac-project/osac-test-infra
Length of output: 105
Add symlink to provide fulfillment-cli command required by test automation.
At lines 16–17, the container installs only osac but the test suite expects fulfillment-cli. All roles invoking {{ fulfillment_cli_path }} (defined as "fulfillment-cli" in inventory/group_vars/all.yml:8) will fail with command-not-found. This breaks test execution for ComputeInstance and Hub lifecycle operations.
Suggested fix
RUN curl -Lsfo /usr/local/bin/osac "https://github.com/osac-project/fulfillment-service/releases/download/v${OSAC_CLI_VERSION}/osac_Linux_x86_64" \
&& chmod +x /usr/local/bin/osac
+RUN ln -sf /usr/local/bin/osac /usr/local/bin/fulfillment-cli📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RUN curl -Lsfo /usr/local/bin/osac "https://github.com/osac-project/fulfillment-service/releases/download/v${OSAC_CLI_VERSION}/osac_Linux_x86_64" \ | |
| && chmod +x /usr/local/bin/osac | |
| RUN curl -Lsfo /usr/local/bin/osac "https://github.com/osac-project/fulfillment-service/releases/download/v${OSAC_CLI_VERSION}/osac_Linux_x86_64" \ | |
| && chmod +x /usr/local/bin/osac | |
| RUN ln -sf /usr/local/bin/osac /usr/local/bin/fulfillment-cli |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Containerfile` around lines 16 - 17, The container installs the osac binary
but the test automation expects the command fulfillment-cli (referenced via
fulfillment_cli_path), so add a symlink after installing /usr/local/bin/osac to
provide /usr/local/bin/fulfillment-cli; update the RUN step that installs osac
to create a symbolic link from /usr/local/bin/osac to
/usr/local/bin/fulfillment-cli (and ensure the chmod +x remains) so roles
invoking fulfillment-cli succeed.
🧩 Analysis chain
🌐 Web query:
Does the GitHub release for osac-project/fulfillment-service tag v0.0.55 publish checksum assets (e.g., SHA256) for osac_Linux_x86_64, and what is the exact checksum verification method recommended by the project?
💡 Result:
No, the GitHub release for osac-project/fulfillment-service tag v0.0.55 does not publish checksum assets (e.g., SHA256) for osac_Linux_x86_64. No evidence of such a release or assets was found across multiple searches of GitHub pages, releases, and related documentation. The repository has 11 releases with the latest being v0.0.46 as of 2026-03-11, and no mentions of v0.0.55, checksum files like checksums.txt or .sha256, or verification methods in README or configs. No recommended checksum verification method is documented by the project.
Citations:
- 1: https://github.com/osac-project/fulfillment-service
- 2: https://github.com/osac-project/fulfillment-service/activity
- 3: https://pkg.go.dev/github.com/innabox/fulfillment-service
🏁 Script executed:
cd /
find . -name "Containerfile" -o -name "Dockerfile" 2>/dev/null | head -20Repository: osac-project/osac-test-infra
Length of output: 224
🏁 Script executed:
cat -n ./home/jailuser/git/Containerfile | head -30Repository: osac-project/osac-test-infra
Length of output: 137
🏁 Script executed:
cat -n Containerfile | head -30Repository: osac-project/osac-test-infra
Length of output: 1184
Add integrity verification for downloaded CLI artifact; note version mismatch.
The binary is fetched without checksum/signature validation, creating supply-chain risk. However, the referenced version v0.0.55 does not exist in the upstream project (latest release is v0.0.46). Additionally, the osac-project/fulfillment-service does not publish SHA256 checksums or signatures for release artifacts, making the suggested checksum validation approach infeasible.
Resolve this in two steps:
- First, correct
OSAC_CLI_VERSIONto match an available release (v0.0.46 or earlier) - For integrity verification without published checksums, consider: pinning the release and reviewing its source code/artifacts directly, using image attestation mechanisms (e.g., sigstore/cosign if supported), or documenting the risk acceptance
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Containerfile` around lines 16 - 17, The Containerfile currently downloads
the osac CLI with an incorrect OSAC_CLI_VERSION and without integrity
verification; change OSAC_CLI_VERSION to a published tag (e.g., v0.0.46) and
update the RUN that fetches /usr/local/bin/osac accordingly, then add one of the
following integrity mitigations: (a) switch to a release workflow that supports
signed/artifact-verified downloads (e.g., use sigstore/cosign or image
attestation) and verify signatures before installing, (b) pin the exact release
and add a documented manual source/artifact review step in the repo (risk
acceptance), or (c) embed a verified build step that compiles osac from tagged
source and verifies git commit/tag before installing; reference
OSAC_CLI_VERSION, the RUN curl line that writes /usr/local/bin/osac, and the
/usr/local/bin/osac install step when making the change.
omer-vishlitzky
changed the title
|
@omer-vishlitzky: No Jira issue is referenced in the title of this pull request. 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. |
The CLI binary was renamed from fulfillment-cli to osac in fulfillment-service v0.0.54+. The conftest.py default was updated to osac in PR osac-project#29, but the Containerfile still downloaded fulfillment-cli. This caused every CLI-dependent test to fail with FileNotFoundError.
fa851ab to
5483431
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: akshaynadkarni, omer-vishlitzky 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 |
…ice name" This reverts commit d6da1c4. ec2:DescribeImages has now been added to the orchestrator's IAM credential, so the workaround (hardcoding ROOT_DEVICE_NAME to /dev/sda1) is no longer needed. Restore the dynamic describe-images lookup, which correctly handles a future AMI change without needing another manual override -- task #30 tracked this exact follow-up.
3 participants
Summary
fulfillment-clitoosacin the ContainerfileProblem
PR #29 changed the default CLI binary name from
fulfillment-clitoosacin conftest.py, but the Containerfile still downloadedfulfillment-cli. This caused every CLI-dependent test to fail withFileNotFoundError: No such file or directory: 'osac'.CI evidence from openshift/release#77782:
Summary by CodeRabbit
Summary by CodeRabbit