Adding UEFI singature support #651
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds UEFI signature support by updating configuration for confidential image handling and enhancing the Azure image creation process. Key changes include:
- Adding an environment variable for the CoCo image URI used in confidential image configuration.
- Implementing new shell functions to create signed image versions based on UEFI signature settings.
- Updating configuration maps and manager settings to include UEFI signature parameters.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| controllers/confidential_handler.go | Patches the ImageConfigMap with the PODVM image URI read from an environment variable. |
| config/peerpods/podvm/azure-podvm-image-handler.sh | Adds functions to create signed and unsigned Azure image versions with UEFI signature support. |
| config/peerpods/podvm/azure-podvm-image-cm.yaml | Adds UEFI signature key configuration. |
| config/manager/manager.yaml | Updates manager configuration with a new RELATED_IMAGE_PODVM_COCO_URI entry. |
controllers/confidential_handler.go
Outdated
| @@ -42,6 +46,12 @@ func (r *KataConfigOpenShiftReconciler) handleFeatureConfidential(state FeatureG | |||
| if err := updateConfigMap(r.Client, r.Log, ig.getImageConfigMapName(), OperatorNamespace, imageConfigMapData); err != nil { | |||
| return err | |||
| } | |||
|
|
|||
| // Patch ImageConfigMap with the image URI. | |||
There was a problem hiding this comment.
[nitpick] Consider merging the two config map update operations into a single update call if updateConfigMap supports patching multiple keys, to reduce multiple API calls.
Copilot uses AI. Check for mistakes.
openshift-merge-robot
added
the
needs-rebase
There was a problem hiding this comment.
Pull Request Overview
This pull request adds UEFI signature support for podvm image creation while updating various image tags and environmental configurations.
- Updated container image versions for Intel DCAP components.
- Added configuration and logic to default UEFI signature parameters in image generation and Azure image handler scripts.
- Modified CI pipeline defaults in the Tekton definitions.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/install-helpers/baremetal-coco/intel-dcap/registration-ds.yaml.in | Updated dcap-registration-flow image version. |
| scripts/install-helpers/baremetal-coco/intel-dcap/qgs.yaml | Updated tdx-qgs image version. |
| scripts/install-helpers/baremetal-coco/intel-dcap/pccs.yaml.in | Replaced init container image and updated the pccs image version. |
| controllers/image_generator.go | Added logic to set default PODVM_IMAGE_URI and UEFI_SIGNATURE_DATABASE_KEY using environment vars. |
| config/peerpods/podvm/azure-podvm-image-handler.sh | Introduced new functions to create signed/unsigned image versions based on UEFI signature availability. |
| config/peerpods/podvm/azure-podvm-image-cm.yaml | Added commented configuration for UEFI signature support. |
| config/manager/manager.yaml | Updated environment variables to support the new OCI image and UEFI signature key. |
| .tekton/osc-operator-bundle-push.yaml & .tekton/osc-operator-bundle-pull-request.yaml | Modified default CI pipeline settings for skip-checks. |
Comments suppressed due to low confidence (3)
scripts/install-helpers/baremetal-coco/intel-dcap/pccs.yaml.in:91
- Confirm that the new base image for 'init-seclabel' provides the necessary functionality for the chcon operation without introducing compatibility issues.
image: registry.access.redhat.com/ubi9/ubi:latest
.tekton/osc-operator-bundle-push.yaml:97
- Ensure that setting the default for skip-checks to true is intentional and aligns with the overall CI workflow and security policies.
- default: "true"
.tekton/osc-operator-bundle-pull-request.yaml:100
- Ensure that setting the default for skip-checks to true in the pull request pipeline is intentional and meets CI expectations.
- default: "true"
|
|
||
| // set to default image | ||
| if cm.Data["PODVM_IMAGE_URI"] == "" && os.Getenv("RELATED_IMAGE_PODVM_OCI") != "" { | ||
| cm.Data["PODVM_IMAGE_URI"] = "oci::" + os.Getenv("RELATED_IMAGE_PODVM_OCI") |
There was a problem hiding this comment.
Consider adding a comment to explain the use and format of the 'oci::' URI prefix to improve clarity and maintainability.
Copilot uses AI. Check for mistakes.
| { | ||
| echo "Creating Azure signed image version" | ||
| # UEFI_SIGNATURE_DATABASE_KEY is required to be set | ||
| [[ -n "${UEFI_SIGNATURE_DATABASE_KEY}" ]] && local UEFI_SDB="${UEFI_SIGNATURE_DATABASE_KEY}" || error_exit "UEFI_SIGNATURE_DATABASE_KEY is empty" |
There was a problem hiding this comment.
Add a comment to clarify the purpose of UEFI_SIGNATURE_DATABASE_KEY and its role in creating the signed image version for future maintainers.
Copilot uses AI. Check for mistakes.
openshift-merge-robot
added
needs-rebase
|
PR needs rebase. 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. |
If podvm configuration isn't modifed, at image creation time this will point to and OCI URI the contains a prebuilt signed qcow2 image, which should be valid both for CoCo and PeerPods, provider agnostic...
To test this it requires to update:
alternately have an bundle & catalog that has it all embedded
** reach me for prebuilt version **
TODOs: