OSAC-1535: Add OSAC connected CI job#528
Conversation
Replace single-value osacProfile (development/vmaas/caas) with osacProfilesList array so multiple profiles can be enabled simultaneously. Aligns with the VMaaS/CaaS/BMaaS experience split. - Add bmaas profile (same controllers as caas) - Drop development profile, default to [caas] - Remove HyperConverged CRD pre-validate check - Update Helm values template to check list membership - Update docs and config example Assisted-by: Claude Code <noreply@anthropic.com>
- Add minItems: 1 and uniqueItems: true to schema - Add vmaas to default profiles list for consistency with previous development profile behavior - Fix VMaaS prerequisites in docs table Assisted-by: Claude Code <noreply@anthropic.com>
WalkthroughThe ChangesOSAC E2E Plugin Chain Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
The inline comment said default was [caas] but defaults.yaml has [caas, vmaas]. Updated both osac.example.yaml and OSAC_DEPLOYMENT.md to show the correct default. Assisted-by: Claude Code <noreply@anthropic.com>
Assisted-by: Claude Code <noreply@anthropic.com>
Make e2e-deployment.yml callable via workflow_call so other workflows can run E2E with a custom plugin chain. Add e2e-osac.yml as a thin caller that deploys the OSAC stack (lvms, trust-manager, rhbk, authorino, aap, osac) in connected mode. Also adds AAP license file handling to deploy_plugin.sh for CI environments where the license is pre-staged on the runner. Assisted-by: Claude Code <noreply@anthropic.com>
Add explicit deploy-plugin steps for each OSAC addon plugin (trust-manager, rhbk, authorino, aap, cnv, osac) to both connected and disconnected E2E jobs. Each step is conditionally run based on the enabled-plugins input. Replaces the hardcoded example plugin step. Assisted-by: Claude Code <noreply@anthropic.com>
1b32ef3 to
5f3247e
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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-deployment.yml:
- Around line 395-423: The `contains()` checks for plugin gating use substring
matching which can unintentionally match partial strings (e.g., 'osac' would
match 'myosac'). Update all the conditional statements in the deploy steps
(deploy_trust_manager, deploy_rhbk, deploy_authorino, deploy_aap, deploy_cnv,
deploy_osac) to use delimiter-based matching instead. Wrap the ENABLED_PLUGINS
environment variable and each plugin name with delimiters (such as commas) to
ensure only explicitly enabled plugins in the CSV list are matched, preventing
unintended deployments from substring collisions.
- Around line 157-165: The `inputs.storage-plugin` value is being directly
interpolated into shell commands and the GITHUB_OUTPUT without validation,
creating a critical injection vulnerability where malicious input could break
quoting, corrupt JSON output, or execute arbitrary shell code. Add validation to
ensure the storage-plugin input only contains allowed plugin names (whitelist
approach) before it is used in the PLUGIN assignment and the echo statement that
writes to storage_plugins in GITHUB_OUTPUT. Alternatively, properly escape or
quote the value to prevent shell injection when constructing the JSON output.
In @.github/workflows/e2e-osac.yml:
- Line 48: In the e2e-osac workflow file, replace the `secrets: inherit`
statement with explicit secret mapping to follow the principle of least
privilege. Instead of inheriting all repository secrets, explicitly define which
specific secrets are required for the called workflow by replacing `secrets:
inherit` with a `secrets:` block that lists only the necessary secrets with
their individual mappings. This reduces the blast radius and prevents accidental
exposure of sensitive information as the workflow evolves.
- Around line 23-28: The GitHub Actions workflow trigger configuration in the
e2e-osac.yml file is missing an entry for the lvms plugin in the paths filter.
Add `plugins/lvms/**` to the paths list in both the push trigger section (around
line 23-28) and the pull_request trigger section (around line 44) to ensure E2E
OSAC tests run whenever changes are made to the lvms plugin, which is part of
the OSAC plugin chain.
In `@scripts/deployment/deploy_plugin.sh`:
- Around line 151-156: The osacProfilesList in the heredoc is hardcoded to only
include the 'caas' profile, which overrides the default plugin configuration
that includes both 'caas' and 'vmaas' profiles. Update the osacProfilesList
section in the SSH command that creates the osac.yaml file to include both
'caas' and 'vmaas' profiles instead of just 'caas' to maintain the expected OSAC
capabilities.
- Around line 140-144: After the scp command that copies the AAP license file to
the Landing Zone (the line copying to ${LZ_SSH}:${LZ_AAP_LICENSE}), add a
follow-up ssh command that applies restrictive file permissions (chmod 600 or
similar least-privileged mode) to the copied license file on the remote Landing
Zone to reduce the risk of unauthorized disclosure of this sensitive file.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 09dfb64e-3ad8-4478-a372-ada16c0809a7
📒 Files selected for processing (3)
.github/workflows/e2e-deployment.yml.github/workflows/e2e-osac.ymlscripts/deployment/deploy_plugin.sh
- Use delimiter-based matching for plugin gating to prevent substring collisions (e.g. 'aap' matching 'osac-aap') - Restrict AAP license file permissions (chmod 600) after scp to LZ - Add plugins/lvms/** to e2e-osac.yml path filter since lvms is part of the OSAC plugin chain - Include vmaas in default osacProfilesList to match plugin defaults Assisted-by: Claude Code <noreply@anthropic.com>
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
…gnostics The CI artifact collection missed CrashLoopBackOff pods because they are technically in Running phase. Add a second pass to capture pods with containers in waiting state or high restart counts, including previous container logs. Also add plugin namespace diagnostics (deployments, pods, events) for addon plugin namespaces when ENABLED_PLUGINS is set. Assisted-by: Claude Code <noreply@anthropic.com>
CNV is not installed on the management cluster, so the osac-operator crashes with "no matches for kind VirtualMachine". Remove vmaas from the CI-generated osac.yaml profiles list until CNV is available. Assisted-by: Claude Code <noreply@anthropic.com>
Summary
e2e-deployment.ymlcallable as a reusable workflow (workflow_call) with inputs for plugin chain, AAP license file, run mode selection, and cleanup/notification controlse2e-osac.yml— thin caller workflow that deploys the OSAC plugin stack (lvms → trust-manager → rhbk → authorino → aap → osac) in connected modeenabled-pluginsdeploy_plugin.shfor CI environments (copies license to LZ, generatesosac.yamlwithosacProfilesList: [caas])The job calls
make deploy-pluginseparately for each addon because addon plugin installation frombootstrap.shwas temporarily removed in #476.Depends on #525.
Changes to
e2e-deployment.ymlworkflow_calltrigger with inputs mirroringworkflow_dispatchplusenabled-pluginsandaap-license-filecheck-e2e-neededresolvesrun_connected/run_disconnectedfrom caller inputs (needed because theinputscontext is unreliable inworkflow_call)inputsdirectlyENABLED_PLUGINSandAAP_LICENSE_FILEenv vars set from inputsAdding more OSAC CI modes
To add disconnected mode, update
e2e-osac.yml:Or create a separate
e2e-osac-disconnected.ymlcaller for independent triggers.Test plan
yamllintpasses on both workflow filesshellcheckpasses ondeploy_plugin.shenabled-pluginsis empty)E2E OSACworkflow deploys the full plugin chain🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Chores