Conversation
openshift-ci
bot
added
the
do-not-merge/hold
WalkthroughUpdated CI test configuration for sandboxed containers operator across multiple deployment scenarios, including network access restrictions, Kata RPM versioning, test execution parameters, and trustee service endpoint settings. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 8 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (8 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 |
|
/pj-rehearse periodic-ci-openshift-sandboxed-containers-operator-devel-downstream-candidate420-aws-ipi-peerpods |
|
@tbuskey: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tbuskey 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 |
openshift-ci
bot
added
the
approved
|
/pj-rehearse periodic-ci-openshift-sandboxed-containers-operator-devel-downstream-candidate420-azure-ipi-kata |
|
@tbuskey: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse list |
|
@tbuskey: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, 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: 1
🧹 Nitpick comments (2)
ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__downstream-candidate420.yaml (2)
45-45: Consider using HTTPS for the trustee service endpoint.The
TRUSTEE_URLuses HTTP instead of HTTPS. Since this appears to be a Key Broker Service (KBS) endpoint for confidential computing, using unencrypted HTTP could expose sensitive attestation data in transit.If HTTPS is available for this service, prefer:
TRUSTEE_URL: https://kbs-service-trustee-operator-system.apps.tpb.azure.sandboxedcontainers.comIf this is intentionally HTTP for testing purposes, it aligns with the debug nature of this PR but should not be merged to production configurations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__downstream-candidate420.yaml` at line 45, Update the TRUSTEE_URL environment value to use HTTPS instead of HTTP for the KBS trustee endpoint: change the value of the TRUSTEE_URL key (TRUSTEE_URL: http://kbs-service-trustee-operator-system.apps.tpb.azure.sandboxedcontainers.com) to use https:// if the service supports TLS, or explicitly document/guard the HTTP use for test-only configs; ensure the updated value is applied wherever TRUSTEE_URL is referenced in the deployment manifests or CI config.
35-35: Consider documenting or validating the INITDATA payload.The
INITDATAfield contains a large base64-encoded gzip blob. For maintainability and review purposes, consider:
- Adding a comment explaining what this configuration contains
- Storing the source file (e.g.,
initdata.toml) in the repository and generating the encoded value during CI- Validating the decoded content in code review
This would improve auditability and make future updates easier to review.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__downstream-candidate420.yaml` at line 35, The INITDATA field currently holds a large base64+gzip blob; replace this opaque inlined blob with a documented workflow: add a short comment next to INITDATA describing that it is a gzipped base64-encoded initdata payload, check the repository in with the plain-text source file named initdata.toml, and update CI to generate the encoded value from initdata.toml (gzip then base64) and patch the manifest during the pipeline; also add a lightweight validation step (decode+gunzip and schema or TOML lint) in the PR/CI job to assert the decoded content is valid so reviewers can inspect the plain initdata.toml instead of the blob.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__downstream-candidate420.yaml`:
- Around line 39-40: The file contains debug settings that must be reverted:
reset SLEEP_DURATION from "6h" back to the production/default value and set
MUST_GATHER_ON_FAILURE_ONLY back to "true" (or the repo's default) in every
configuration where they were changed (look for the SLEEP_DURATION and
MUST_GATHER_ON_FAILURE_ONLY keys across the 7 test configs), and remove the
"[DEBUG] [DO NOT MERGE]" changes so CI runs use standard timing and only gather
must-gather on failures.
---
Nitpick comments:
In
`@ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__downstream-candidate420.yaml`:
- Line 45: Update the TRUSTEE_URL environment value to use HTTPS instead of HTTP
for the KBS trustee endpoint: change the value of the TRUSTEE_URL key
(TRUSTEE_URL:
http://kbs-service-trustee-operator-system.apps.tpb.azure.sandboxedcontainers.com)
to use https:// if the service supports TLS, or explicitly document/guard the
HTTP use for test-only configs; ensure the updated value is applied wherever
TRUSTEE_URL is referenced in the deployment manifests or CI config.
- Line 35: The INITDATA field currently holds a large base64+gzip blob; replace
this opaque inlined blob with a documented workflow: add a short comment next to
INITDATA describing that it is a gzipped base64-encoded initdata payload, check
the repository in with the plain-text source file named initdata.toml, and
update CI to generate the encoded value from initdata.toml (gzip then base64)
and patch the manifest during the pipeline; also add a lightweight validation
step (decode+gunzip and schema or TOML lint) in the PR/CI job to assert the
decoded content is valid so reviewers can inspect the plain initdata.toml
instead of the blob.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b816716-ccad-4f12-91eb-9b5df6f575ba
📒 Files selected for processing (1)
ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__downstream-candidate420.yaml
| MUST_GATHER_ON_FAILURE_ONLY: "false" | ||
| SLEEP_DURATION: 6h |
There was a problem hiding this comment.
Debug configuration detected - do not merge.
SLEEP_DURATION: 6h and MUST_GATHER_ON_FAILURE_ONLY: "false" are debug settings:
- A 6-hour sleep significantly extends test execution time and consumes CI resources unnecessarily.
- Gathering must-gather on success (not just failure) adds overhead without benefit for normal CI runs.
These settings are applied across all 7 test configurations. Given the PR title explicitly states "[DEBUG] [DO NOT MERGE]", ensure these changes are reverted before any merge consideration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__downstream-candidate420.yaml`
around lines 39 - 40, The file contains debug settings that must be reverted:
reset SLEEP_DURATION from "6h" back to the production/default value and set
MUST_GATHER_ON_FAILURE_ONLY back to "true" (or the repo's default) in every
configuration where they were changed (look for the SLEEP_DURATION and
MUST_GATHER_ON_FAILURE_ONLY keys across the 7 test configs), and remove the
"[DEBUG] [DO NOT MERGE]" changes so CI runs use standard timing and only gather
must-gather on failures.
|
@tbuskey: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
1 participant
/hold
Summary by CodeRabbit
Release Notes