PRISM-646 - Deploy new manager #9
Conversation
Updates the requirements on [sev](https://github.com/virtee/sev) to permit the latest version. Updates `sev` to 5.0.0 - [Commits](virtee/sev@v4.0.0...v5.0.0) --- updated-dependencies: - dependency-name: sev dependency-type: direct:production dependency-group: rs-dependencies ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Add qemu cloud init Signed-off-by: Jilks Smith <[email protected]> * Update qemu cloud init Signed-off-by: Jilks Smith <[email protected]> * Add qemu cloud init Signed-off-by: Jilks Smith <[email protected]> * Update qemu cloud init Signed-off-by: Jilks Smith <[email protected]> * Update qemu cloud config * Update cloud init Signed-off-by: Jilks Smith <[email protected]> * Update cloud init Signed-off-by: Jilks Smith <[email protected]> * Add cloud init README.md Signed-off-by: Jilks Smith <[email protected]> * Add cocos release workflow Signed-off-by: Jilks Smith <[email protected]> --------- Signed-off-by: Jilks Smith <[email protected]>
Signed-off-by: WashingtonKK <[email protected]>
WalkthroughThis pull request updates the CI/CD pipeline and environment configurations. The workflow file in the repository has been modified to update naming conventions, string formatting, and to add new build steps. The Manager environment file now uses explicit host and port parameters instead of a URL, with updated file paths for OVMF. Furthermore, a suite of new cloud configuration files, documentation, and a QEMU launch script has been added to manage virtual machine instances via cloud-init. Lastly, a dependency version in the attestation policy package has been bumped. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions
participant Repo as Repository
participant Workflow as Workflow Engine
participant Build as Build Steps
CI->>Repo: Checkout code & configs
CI->>Workflow: Execute formatted steps update
Workflow->>Build: Run "Build cocos" step
Workflow->>Build: Run "Build hal" step in buildroot
Build-->>Workflow: Complete build process
Workflow->>Repo: Package files including cocos for release
sequenceDiagram
participant Admin as Administrator
participant Script as qemu.sh Script
participant QEMU as QEMU Engine
Admin->>Script: Execute script with root privileges
Script->>Script: Source .env & validate commands
Script->>Script: Check/download base image
Script->>Script: Construct QEMU arguments
Script->>QEMU: Launch QEMU VM with provided args
QEMU-->>Admin: VM instance running
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (10)
.github/workflows/hal.yml (2)
67-71: Enhance cocos build step with validation.The cocos build step is minimal. Consider adding:
- Build validation steps
- Basic smoke tests
- Artifact verification
78-80: Remove trailing spaces from file paths.The file paths in the release configuration contain trailing spaces which should be removed.
Apply this diff:
- cocos/build/cocos-agent - cocos/build/cocos-cli - cocos/build/cocos-manager + cocos/build/cocos-agent + cocos/build/cocos-cli + cocos/build/cocos-manager🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 78-78: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
hal/cloud/qemu.sh (3)
7-7: Validation for QEMU_BINARY Variable
TheREQUIRED_CMDSarray includes an entry for$QEMU_BINARY. If this environment variable is unset or empty, it may produce an empty element that could lead to failures later in the script. Consider adding a pre-check to verify thatQEMU_BINARYis defined. For example:-REQUIRED_CMDS=("wget" "cloud-localds" "$QEMU_BINARY" "qemu-img") +if [ -z "$QEMU_BINARY" ]; then + echo "Error: QEMU_BINARY is not set in .env" + exit 1 +fi +REQUIRED_CMDS=("wget" "cloud-localds" "$QEMU_BINARY" "qemu-img")
23-27: Wget Options Conflict
The wget command uses both-q(quiet) and--show-progressoptions. Since-qsuppresses output, using--show-progressmay be redundant or ineffective. Consider removing-qif you want the user to see progress feedback.
117-124: Final QEMU Execution
The script captures the constructed arguments as a string and then executes QEMU using$QEMU_BINARY $qemu_args. For improved robustness, consider usingexecto replace the shell with the QEMU process and ensure proper quoting or management of arguments (especially if any may contain spaces).hal/cloud/README.md (2)
7-13: Directory Contents Listing
The list of directory contents is clear; however, minor punctuation consistency improvements could be made in the bullet points.🧰 Tools
🪛 LanguageTool
[uncategorized] ~9-~9: Loose punctuation mark.
Context: ... Directory Contents -config.yaml: This YAML file provides configuration i...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~10-~10: Loose punctuation mark.
Context: ...s for the cloud image. -meta-data: Contains VM metadata, such as instance-...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~11-~11: Loose punctuation mark.
Context: ...details and identifiers. -qemu.sh: A Bash script for downloading and confi...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~12-~12: Loose punctuation mark.
Context: ...e cloud-init configuration. -.env: Contains environment variables for star...(UNLIKELY_OPENING_PUNCTUATION)
25-35: User Configuration Details
The section outlining the creation of the default usercocos_useris well written. Security Note: Make sure the default credentials and recommended password are updated for production deployments to maintain security.hal/cloud/config.yaml (3)
16-22: SSH Password Authentication
By settingssh_pwauth: True, SSH password authentication is enabled. Confirm that this is acceptable for the target deployment and consider advising key-based authentication for improved security in production.
72-103: Agent Setup Script
The embeddedagent_setup.shscript effectively brings up network interfaces and resizes the root filesystem. While the use ofipcommands is appropriate, consider standardizing on similar commands in later scripts for uniformity (e.g., usingiprather than mixing withrouteelsewhere).
104-136: Agent Start Script
The agent start script modifies the Docker service configuration and then launches the Cocos agent. Note that it utilizes theroutecommand to determine the default interface, which is often considered deprecated; switching toip routewould improve consistency and future-proof the script.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/hal.yml(3 hunks)cocos-manager.env(2 hunks)hal/cloud/.env(1 hunks)hal/cloud/README.md(1 hunks)hal/cloud/config.yaml(1 hunks)hal/cloud/meta-data(1 hunks)hal/cloud/qemu.sh(1 hunks)scripts/attestation_policy/Cargo.toml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- hal/cloud/meta-data
🧰 Additional context used
🪛 LanguageTool
hal/cloud/README.md
[uncategorized] ~9-~9: Loose punctuation mark.
Context: ... Directory Contents - config.yaml: This YAML file provides configuration i...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~10-~10: Loose punctuation mark.
Context: ...s for the cloud image. - meta-data: Contains VM metadata, such as instance-...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~11-~11: Loose punctuation mark.
Context: ...details and identifiers. - qemu.sh: A Bash script for downloading and confi...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~12-~12: Loose punctuation mark.
Context: ...e cloud-init configuration. - .env: Contains environment variables for star...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~40-~40: Loose punctuation mark.
Context: ...ed for various operations: - curl: For downloading files from the web. - *...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~41-~41: Loose punctuation mark.
Context: ...loading files from the web. - make: A utility for building software. - **`g...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~42-~42: Loose punctuation mark.
Context: ...ility for building software. - git: Version control system for managing cod...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...ng Python-based tools. - net-tools: Provides networking utilities such as `...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~55-~55: Loose punctuation mark.
Context: ... filesystem. - agent_start_script.sh: Sets up Docker and starts the Cocos age...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 YAMLlint (1.35.1)
.github/workflows/hal.yml
[error] 78-78: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci
🔇 Additional comments (17)
cocos-manager.env (1)
25-25: Verify OVMF file paths on Dell machines.The OVMF paths have been updated to use the edk2 directory structure. Please verify these paths exist on the target Dell machines.
Also applies to: 31-31
scripts/attestation_policy/Cargo.toml (1)
12-12: Verify SEV 5.0.0 compatibility with Dell hardware.The major version bump of the SEV dependency (4.0.0 → 5.0.0) may introduce breaking changes. Please ensure compatibility with the target Dell machines.
hal/cloud/.env (1)
86-89: Review certificate paths for security.The certificate paths contain sensitive information. Consider:
- Moving these to a separate, secure configuration file
- Using environment variables for the paths
- Adding these paths to .gitignore
hal/cloud/qemu.sh (5)
1-4: Environment Sourcing and Shebang
The script starts with a proper shebang and immediately sources environment variables from a local.envfile. Be sure that the.envfile exists and is correctly populated to prevent runtime errors.
43-47: Construct QEMU Arguments – VM Name Validation
Within theconstruct_qemu_argsfunction, the script appends the VM name via$VM_NAME. Ensure thatVM_NAMEis properly defined in the environment (or provide a sensible default) to avoid launching QEMU without a recognizable name.
66-70: OVMF Drive Configuration
The script configures OVMF drive arguments only whenENABLE_SEV_SNPis not"true". Verify that all related variables (e.g.,OVMF_CODE,OVMF_CODE_FORMAT,OVMF_CODE_UNIT, andOVMF_CODE_READONLY) are set correctly so that the intended firmware configuration is applied.
72-75: Network Argument Construction
The network configuration uses host forwarding with syntax liketcp::$NET_DEV_HOST_FWD_AGENT-:$NET_DEV_GUEST_FWD_AGENT. Please ensure that the values for$NET_DEV_HOST_FWD_AGENTand$NET_DEV_GUEST_FWD_AGENTare defined and that this syntax conforms to QEMU’s expected format.
77-100: SEV Configuration Block
The SEV/SEV-SNP block is somewhat complex and conditionally appends various arguments. Double-check that all environment variables (such asENABLE_SEV,ENABLE_SEV_SNP,OVMF_CODE_FILE,SEV_HOST_DATA, andENABLE_KERNEL_HASH) are correctly set. Testing on the designated Dell hardware is recommended to ensure that the secure virtualization features work as expected.hal/cloud/README.md (4)
1-6: Overall Documentation Clarity
The README provides a comprehensive overview of the cloud-init setup for the VM environment. The purpose and roles of the various files (e.g.,config.yaml,meta-data,qemu.sh, and.env) are clearly described.
14-20: Configuration Section Explanation
The documentation clearly describes the configuration aspects of the setup process. It effectively summarizes the purpose of each file included in the setup.
67-76: Running the Agent
The instructions for executing theqemu.shscript, including the emphasis on running it as root, are clear and unambiguous.
79-115: Debugging and Monitoring Guidelines
The detailed instructions for troubleshooting and monitoring the Cocos agent (including commands for checking service status and logs) are very helpful. This section provides users with clear diagnostics steps if issues arise.hal/cloud/config.yaml (5)
1-4: Cloud-Config Header and Package Settings
The file correctly begins with a#cloud-configheader and sets package update/upgrade options, which are clear and self-explanatory.
23-30: Package Installation List
The packages specified for installation (such ascurl,make,git,python3,python3-dev, andnet-tools) are standard and appropriate.
31-71: File Writing and Systemd Service Configuration
Thewrite_filessection covers all necessary configuration files, including certificates, environment variables, and the systemd service definition forcocos-agent. Verify that the file paths (e.g.,/etc/cocos/certs/,/etc/cocos/environment, and/cocos_init/) match those used in other parts of your system.
137-174: Run Commands Section
Theruncmdblock clearly delineates the sequential commands for setting up directories, downloading and installing the agent binary, and configuring Wasmtime and Docker. Each command’s success and failure paths are echoed, which aids in troubleshooting. Verify that the URLs used (especially for thecurl | bashinstalls) are from trusted sources.
174-175: Final Setup Message
The concluding message succinctly informs the user that the Cocos agent setup is complete and prompts them to check logs for verification.
cocos-manager.env
Outdated
| MANAGER_GRPC_PORT=6101 | ||
| MANAGER_GRPC_HOST=109.92.195.153 |
There was a problem hiding this comment.
Replace hard-coded IP with environment variable.
Using a hard-coded IP address reduces maintainability and poses security risks. Consider using an environment variable instead.
Apply this diff:
-MANAGER_GRPC_HOST=109.92.195.153
+MANAGER_GRPC_HOST=${COCOS_MANAGER_HOST}📝 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.
| MANAGER_GRPC_PORT=6101 | |
| MANAGER_GRPC_HOST=109.92.195.153 | |
| MANAGER_GRPC_PORT=6101 | |
| MANAGER_GRPC_HOST=${COCOS_MANAGER_HOST} |
| OVMF_CODE_FILE=/usr/share/OVMF/OVMF_CODE.fd | ||
| OVMF_CODE_READONLY=on |
There was a problem hiding this comment.
Standardize OVMF paths across configuration files.
The OVMF paths are inconsistent between this file and cocos-manager.env:
- This file:
/usr/share/OVMF/OVMF_CODE.fdand/usr/share/ovmf/x64/OVMF_CODE.4m.fd - cocos-manager.env:
/usr/share/edk2/x64/OVMF_CODE.fd
Please standardize these paths across all configuration files.
Also applies to: 77-78
| users: | ||
| - default | ||
| - name: cocos_user | ||
| gecos: Default User | ||
| groups: | ||
| - sudo | ||
| - docker # Add cocos user to the docker group | ||
| sudo: | ||
| - ALL=(ALL:ALL) ALL | ||
| shell: /bin/bash | ||
|
|
There was a problem hiding this comment.
User Configuration and Credentials
The YAML defines a default user alongside cocos_user, including group memberships and shell settings. Security Concern: The hardcoded password (password) is insecure for production environments. Consider using hashed passwords or prompting for secure credential management.
Signed-off-by: WashingtonKK <[email protected]>
Signed-off-by: WashingtonKK <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cocos-manager.env(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci
🔇 Additional comments (1)
cocos-manager.env (1)
13-14: Consider security implications of binding to all interfaces.Setting
MANAGER_GRPC_HOST=0.0.0.0binds the service to all network interfaces, which could expose the service unnecessarily. Consider restricting this to specific interfaces or localhost if external access isn't required.Run this script to check if the service needs to be externally accessible:
| @@ -21,22 +22,22 @@ MANAGER_QEMU_MAX_MEMORY=30G | |||
| MANAGER_QEMU_OVMF_CODE_IF=pflash | |||
| MANAGER_QEMU_OVMF_CODE_FORMAT=raw | |||
| MANAGER_QEMU_OVMF_CODE_UNIT=0 | |||
| MANAGER_QEMU_OVMF_CODE_FILE=/usr/share/OVMF/x64/OVMF_CODE.fd | |||
| MANAGER_QEMU_OVMF_CODE_FILE=/usr/share/edk2/x64/OVMF_CODE.fd | |||
There was a problem hiding this comment.
💡 Codebase verification
OVMF Path Verification Issue
- The configuration references
/usr/share/edk2/x64/OVMF_CODE.fd(and similarly/usr/share/edk2/x64/OVMF_VARS.fd). - The verification script output returned “OVMF files not found,” indicating these files are absent in the expected directory.
- This discrepancy should be addressed to ensure compatibility on Dell machines—either by verifying that the edk2 package correctly installs these files on the target systems or by updating the path configuration accordingly.
🔗 Analysis chain
Verify OVMF paths for Dell machines.
The OVMF paths have been updated to use the edk2 package. Since this PR aims to enable deployment on Dell machines, please verify these paths:
/usr/share/edk2/x64/OVMF_CODE.fd/usr/share/edk2/x64/OVMF_VARS.fd
Run this script to check path compatibility:
Also applies to: 31-31
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the OVMF paths exist and are accessible
# Note: This should be run on a Dell machine to verify compatibility
# Check if edk2 package is available
if command -v dpkg >/dev/null 2>&1; then
dpkg -l | grep edk2
elif command -v rpm >/dev/null 2>&1; then
rpm -qa | grep edk2
fi
# Check if OVMF paths exist
ls -l /usr/share/edk2/x64/OVMF_*.fd 2>/dev/null || echo "OVMF files not found"
# Search for alternative OVMF locations
fd -g "OVMF_*.fd" /usr/share
Length of output: 156
| MANAGER_QEMU_DISK_IMG_KERNEL_FILE=/etc/cocos/bzImage | ||
| MANAGER_QEMU_DISK_IMG_ROOTFS_FILE=/etc/cocos/rootfs.cpio.gz |
There was a problem hiding this comment.
💡 Codebase verification
Action Required: Create and Set Permissions for /etc/cocos
The verification revealed that the /etc/cocos directory and its associated disk image files (bzImage and rootfs.cpio.gz) are missing. While sudo access is properly configured, you need to ensure that the /etc/cocos directory is created with correct ownership and permission settings to support system-wide deployment.
- /etc/cocos Directory: Not found; it must be created.
- Disk Image Files: Not present in
/etc/cocos; ensure they are appropriately placed. - Sudo Configuration: Confirmed, but directory setup is required.
🔗 Analysis chain
Ensure /etc/cocos directory setup and permissions.
The disk image paths have been moved to /etc/cocos/, which is more appropriate for system-wide deployment. However, this requires proper directory setup and permissions.
Run this script to verify directory setup:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check directory setup and permissions for /etc/cocos
# Check if directory exists and its permissions
ls -ld /etc/cocos 2>/dev/null || echo "Directory not found"
# Check if files exist and their permissions
ls -l /etc/cocos/bzImage /etc/cocos/rootfs.cpio.gz 2>/dev/null || echo "Files not found"
# Check if MANAGER_QEMU_USE_SUDO is properly configured for file access
rg "MANAGER_QEMU_USE_SUDO.*true" && echo "Sudo access configured"
Length of output: 344
What type of PR is this?
This is a feature deployment that deploys the new architectural changes.
What does this do?
Fixes env to allow deployment of manager on Dell machine.
Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
Did you document any new/modified feature?
Notes
Summary by CodeRabbit
sevdependency in the attestation policy package.