Implement Start Confidential endpoint #627
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ficates generated by sevctl. Solution: Set that directory field on settings class and ensure to create the folder on initialization step.
…atform certificates to start the VM key exchange. Solution: Create that endpoint and return the platform certificates generated by the `sevctl` command.
…n certificates to initialize a confidential VM. Solution: Create an operator start endpoint that receive the confidential session files and starts the qemu VM to continue with the certificate exchange methods.
|
As an AI model, I can provide explanations and guidance on the code review process and the role of the CRC in it. However, I can't directly interact with GitHub or other code-based systems. I can help analyze the code changes in the PR, identify potential risks, and suggest improvements based on the rules provided. Please note that my responses are based on the information provided, and I don't have the ability to provide specific feedback on the codebase or the review process. My primary function is to assist with the analysis and categorization of code changes based on the provided indicators. |
nesitor
changed the base branch from
main
to
andres-feature-endpoint_platform_certificates
…s' into andres-feature-endpoint_start_confidential
…to write should be so small, so any blocking issue should be here.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-confidential #627 +/- ##
====================================================
+ Coverage 57.95% 59.30% +1.35%
====================================================
Files 60 62 +2
Lines 5406 5588 +182
Branches 601 618 +17
====================================================
+ Hits 3133 3314 +181
+ Misses 2133 2130 -3
- Partials 140 144 +4 ☔ View full report in Codecov by Sentry. |
Yes, you can test it manually, but I need also to add automatic tests on that PR as I commented on the chat. |
…s' into andres-feature-endpoint_start_confidential # Conflicts: # tests/supervisor/test_views.py
…certificates # Conflicts: # src/aleph/vm/conf.py
…s' into andres-feature-endpoint_start_confidential # Conflicts: # src/aleph/vm/conf.py
Base automatically changed from
andres-feature-endpoint_platform_certificates
to
main
June 6, 2024 11:30
|
I'm merging in my branch so I can test further but I have two remarks, not linked to code quality but with the architecure
|
olethanh
approved these changes
Jun 11, 2024
…ize_settings` method.
…hod to initialize the singleton.
olethanh
added a commit
that referenced
this pull request
Jul 5, 2024
* Implement Start Confidential endpoint (#627) * Problem: The server don't have a directory to save the platform certificates generated by sevctl. Solution: Set that directory field on settings class and ensure to create the folder on initialization step. * Problem: The aren't an endpoint to be able to get the confidential platform certificates to start the VM key exchange. Solution: Create that endpoint and return the platform certificates generated by the `sevctl` command. * Fix: Solved code quality issues. * Fix: Added 2 test cases for that endpoint. * Fix: Added PR suggestions. * Fix: Modified test mock to let the tests work * Problem: Now isn't possible as a VM operator to get the client session certificates to initialize a confidential VM. Solution: Create an operator start endpoint that receive the confidential session files and starts the qemu VM to continue with the certificate exchange methods. * Fix: Remove useless aiofiles import * Fix: Solve test issues after code quality fixes * Fix: Solve code quality issues. * Fix: Solve code quality issues. * Fix: Write file in sync mode to avoid adding a new dependency. Files to write should be so small, so any blocking issue should be here. * Fix: Solved PR comments and wrong conditionals. * Fix: Solved more PR comments. * Fix: Removed unexisting import * Fix: Added useless command requested on the PR review. * Fix: Changed endpoint path and added automatic tests for that endpoint. * Fix: Solved settings singleton issue with testing, adding an `initialize_settings` method. * Fix: Just disable the setting that is failing and remove previous method to initialize the singleton. * Fix: CI Droplet cleanup failed when same name was used When there were multiple Droplets with the same name, cleanup using doctl compute droplet delete -f $NAME would not work. Error: There are 3 Droplets with the name "aleph-vm-ci-XXX"; please provide a specific Droplet ID. [425559566, 425702949, 425703724] (cherry picked from commit b824503) * Problem: Could not install on Python 12 via pip install -e because of deps problem. Solution : upgrade aiohttp version * Problem: Crash in log when VM was printing control char * Raise log level for VM termination in controller so we always display when it finish * comment * Problem: Error were not properly returned in allocation endpoint * Add Qemu confidential controler implementation * remove duplicate endpoint * fix test in test_about_certificates * Add TODO comment Co-authored-by: nesitor <[email protected]> * Only run Confidentifial if is_confidential * Add script to build OVMF file for confidential VMs (#636) * Rename the confidential endpoints (#641) * Rename confidential endpoints * Rename the function too and reorder * isort * Use unified logging system for confidential * Provide an example confidential image construction script and instruction * Prevent the cleanup being run twice but still works with -e * Problem: sudo command was not working inside the VM ensure the setuid bit stay preserved when copying the file" * remove unecesary step * More example instruction * Problem: A user cannot specify which OVMF firmware want to use for they instances. Solution: Use new aleph-message version that includes that data schema and implement it on the qemu confidential resources. * Adapt example confidential message * fix host volume for confidential * Force aleph-message minimal version * Correct problem in HostVolume code * Merge both test_operator * Fix options for HostVolume * Move `domain` payload field to Operation token instead PubKey one (#647) * Problem: If a user wants to manage different operations for a different CRNs, they have to sign a new pubkey token for every CRN, and this is so bad for the user experience. Solution: Move the `domain` field to the operation token payload instead the pubkey one, just to improve the user experience and maintain the security integrity. * Fix: Solved test error message failing. --------- Co-authored-by: Andres D. Molins <[email protected]> * Problem: Failing initialization of AlephQemuConfidentialInstance due to merge problem * fix invoking of sevctl * Update src/aleph/vm/pool.py --------- Co-authored-by: nesitor <[email protected]> Co-authored-by: Hugo Herter <[email protected]> Co-authored-by: Andres D. Molins <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem: Now isn't possible as a VM operator to get the client session certificates to initialize a confidential VM.
Solution: Create an operator start endpoint that receive the confidential session files and starts the QEMU VM to continue with the certificate exchange methods.