chore(deps): update osc-tdx-qgs to edc6055#2352
Conversation
Image created from 'https://github.com/openshift/confidential-compute-artifacts?rev=125c5641d6c66f9a8e5f6c7722dbd499e0404417' Signed-off-by: red-hat-konflux <126015336+red-hat-konflux[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughThe pull request updates the Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Hi @red-hat-konflux[bot]. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
openshift-ci
Bot
added
the
needs-ok-to-test
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
scripts/install-helpers/baremetal-coco/intel-dcap/qgs.yaml (4)
20-20: 🔒 Security & Privacy | 🔴 CriticalRemove
hostNetwork: trueor provide security justification.Exposing the host network namespace is a critical security risk. As per path instructions,
hostNetworkis not permitted in Kubernetes manifests. If host network access is absolutely required for DCAP functionality, this must be documented with a security justification explaining the specific requirement and threat model mitigation.🤖 Prompt for 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. In `@scripts/install-helpers/baremetal-coco/intel-dcap/qgs.yaml` at line 20, The qgs.yaml manifest contains hostNetwork: true which violates security policies by exposing the host network namespace. Remove the hostNetwork: true configuration from the Pod specification. If host network access is genuinely required for DCAP functionality to operate correctly, do not remove it but instead add clear documentation in the manifest (as comments or in accompanying documentation) that explains the specific security justification, the exact DCAP requirement that necessitates host network access, and the threat model mitigation strategies in place.Source: Path instructions
22-46: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd resource limits to the initContainer.
The initContainer
platform-registration(lines 22–46) is missing CPU and memory resource limits. Path instructions require resource limits on every container. Addresources.limits.cpuandresources.limits.memory(and optionallyrequests) to ensure the container doesn't exhaust node resources.🤖 Prompt for 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. In `@scripts/install-helpers/baremetal-coco/intel-dcap/qgs.yaml` around lines 22 - 46, The platform-registration initContainer is missing resource limits configuration. Add a resources section to the platform-registration initContainer specification that includes limits for both cpu and memory fields. You can optionally also add requests for cpu and memory if needed. This section should be added at the same indentation level as other container specification fields like securityContext, volumeMounts, and env.Source: Path instructions
37-38: 🔒 Security & Privacy | 🔴 CriticalRemove
privileged: trueandallowPrivilegeEscalation: truefrom initContainer, or provide written security justification.The initContainer violates two critical path instructions:
- Line 37:
allowPrivilegeEscalation: truemust befalse- Line 38:
privileged: trueis explicitly forbidden unless justifiedIf privileged mode is required for EFI/firmware registration on TDX hardware, this must be clearly documented with a security justification explaining why capabilities are insufficient and what threat model this mitigates. Consider using a minimally privileged approach with only required capabilities (e.g.,
CAP_SYS_RAWIOorCAP_SYS_BOOT) instead of blanket privileged access.🤖 Prompt for 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. In `@scripts/install-helpers/baremetal-coco/intel-dcap/qgs.yaml` around lines 37 - 38, The initContainer in the qgs.yaml file has two security vulnerabilities: allowPrivilegeEscalation is set to true and privileged is set to true. Change allowPrivilegeEscalation to false and remove the privileged: true line entirely, or if privileged mode is genuinely required for EFI/firmware registration on TDX hardware, add explicit written security justification in code comments explaining why this is necessary and what threat model it addresses. As an alternative to blanket privileged access, investigate using a minimally privileged approach with only the specific required Linux capabilities (such as CAP_SYS_RAWIO or CAP_SYS_BOOT) instead of full privileged mode.Source: Path instructions
2-88: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd liveness and readiness probes to DaemonSet containers.
Neither the initContainer nor the main container defines liveness or readiness probes. Path instructions require liveness and readiness probes for Kubernetes manifests. Without probes, Kubernetes cannot detect if the
tdx-qgscontainer has crashed or become unresponsive, and the DaemonSet will not properly report pod health status.Consider adding a simple HTTP or exec-based readiness probe to verify the QGS service is listening on port 4050 (as specified in args at line 51).
🤖 Prompt for 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. In `@scripts/install-helpers/baremetal-coco/intel-dcap/qgs.yaml` around lines 2 - 88, Add liveness and readiness probes to both the platform-registration initContainer and the tdx-qgs main container in the DaemonSet spec. For the tdx-qgs container, add an httpGet readiness probe that checks the service on localhost port 4050 (as specified in the container args with -p=4050) to verify the QGS service is responding. Configure appropriate initialDelaySeconds and periodSeconds values to account for startup time. For the platform-registration initContainer, since it runs once during initialization, you may add a simpler exec-based readiness probe or skip the liveness probe but ensure readiness is properly defined. These probes enable Kubernetes to detect when the containers have crashed or become unresponsive and maintain proper health status reporting for the DaemonSet.Source: Path instructions
🤖 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.
Outside diff comments:
In `@scripts/install-helpers/baremetal-coco/intel-dcap/qgs.yaml`:
- Line 20: The qgs.yaml manifest contains hostNetwork: true which violates
security policies by exposing the host network namespace. Remove the
hostNetwork: true configuration from the Pod specification. If host network
access is genuinely required for DCAP functionality to operate correctly, do not
remove it but instead add clear documentation in the manifest (as comments or in
accompanying documentation) that explains the specific security justification,
the exact DCAP requirement that necessitates host network access, and the threat
model mitigation strategies in place.
- Around line 22-46: The platform-registration initContainer is missing resource
limits configuration. Add a resources section to the platform-registration
initContainer specification that includes limits for both cpu and memory fields.
You can optionally also add requests for cpu and memory if needed. This section
should be added at the same indentation level as other container specification
fields like securityContext, volumeMounts, and env.
- Around line 37-38: The initContainer in the qgs.yaml file has two security
vulnerabilities: allowPrivilegeEscalation is set to true and privileged is set
to true. Change allowPrivilegeEscalation to false and remove the privileged:
true line entirely, or if privileged mode is genuinely required for EFI/firmware
registration on TDX hardware, add explicit written security justification in
code comments explaining why this is necessary and what threat model it
addresses. As an alternative to blanket privileged access, investigate using a
minimally privileged approach with only the specific required Linux capabilities
(such as CAP_SYS_RAWIO or CAP_SYS_BOOT) instead of full privileged mode.
- Around line 2-88: Add liveness and readiness probes to both the
platform-registration initContainer and the tdx-qgs main container in the
DaemonSet spec. For the tdx-qgs container, add an httpGet readiness probe that
checks the service on localhost port 4050 (as specified in the container args
with -p=4050) to verify the QGS service is responding. Configure appropriate
initialDelaySeconds and periodSeconds values to account for startup time. For
the platform-registration initContainer, since it runs once during
initialization, you may add a simpler exec-based readiness probe or skip the
liveness probe but ensure readiness is properly defined. These probes enable
Kubernetes to detect when the containers have crashed or become unresponsive and
maintain proper health status reporting for the DaemonSet.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7641896f-a5f2-4c6d-b030-448d975b5b89
📒 Files selected for processing (3)
bundle/manifests/sandboxed-containers-operator.clusterserviceversion.yamlconfig/manager/manager.yamlscripts/install-helpers/baremetal-coco/intel-dcap/qgs.yaml
Image created from 'https://github.com/openshift/confidential-compute-artifacts?rev=125c5641d6c66f9a8e5f6c7722dbd499e0404417'
This PR contains the following updates:
0251f07->edc6055Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Enabled.
♻ Rebasing: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.
To execute skipped test pipelines write comment
/ok-to-test