ghe0/brain-mock
1
0
Fork 1
Code Issues Pull Requests 2 Packages Projects Releases Wiki Activity

update functionality for the brain #2

Merged
ghe0 merged 11 commits from new-updater into main 2025-01-09 22:29:56 +00:00
Conversation 1 Commits 11 Files Changed 3 +150 -57
ramrem commented 2025-01-04 16:36:30 +00:00
Collaborator
Copy Link

These changes have been done:
measurement args are a separate argument which get passed from daemon
revamped updatevm and newvm resp functions, specially how args.dtrfs_api_endpoint gets handled
updating now returns error from brain if it cant find the contract
proto has been updated accordingly

These changes have been done: measurement args are a separate argument which get passed from daemon revamped updatevm and newvm resp functions, specially how args.dtrfs_api_endpoint gets handled updating now returns error from brain if it cant find the contract proto has been updated accordingly
ramrem added 7 commits 2025-01-04 16:36:31 +00:00 
I'm not sure if this is the best way to do it, but it works for now.
ghe0 requested changes 2025-01-05 13:01:49 +00:00
src/data.rs Outdated
@ -87,6 +87,7 @@ pub struct BrainData {
daemon_deletevm_tx: DashMap<String, Sender<grpc::DeleteVmReq>>,
daemon_newvm_tx: DashMap<String, Sender<grpc::NewVmReq>>,
daemon_updatevm_tx: DashMap<String, Sender<grpc::UpdateVmReq>>,
daemon_get_measurement_tx: DashMap<String, Sender<MeasurementArgs>>,
ghe0 commented 2025-01-04 23:00:17 +00:00
Owner
Copy Link

The services for MeasurementArgs out in the brain.proto. I see here two options:

  • option 1: we do not commit any code for the measurement args RPC service
  • option 2: we turn the comments from the proto into running code and we test Measurement Args

My suggestion is to go for option 1, since that allows this PR to be isolated to the upgrade. I believe we should add measurement args in the future, if we see it is really needed. It will not be hard to add an extra RPC service when we need it. At this point, it is noise.

The services for MeasurementArgs out in the `brain.proto`. I see here two options: - option 1: we do not commit any code for the measurement args RPC service - option 2: we turn the comments from the proto into running code and we test Measurement Args My suggestion is to go for option 1, since that allows this PR to be isolated to the upgrade. I believe we should add measurement args in the future, if we see it is really needed. It will not be hard to add an extra RPC service when we need it. At this point, it is noise.
ghe0 commented 2025-01-06 17:41:33 +00:00
Owner
Copy Link

He went with option one without dropping a comment. 10 points abstracted from Griffindor.

He went with option one without dropping a comment. 10 points abstracted from Griffindor.
ghe0 marked this conversation as resolved
src/data.rs Outdated
@ -209,3 +211,3 @@
let mut public_ipv4 = String::new();
let mut public_ipv6 = String::new();
for ip in new_vm_resp.ips {
for ip in new_vm_resp.args.clone().unwrap_or(grpc::MeasurementArgs::default()).ips {
ghe0 commented 2025-01-04 23:03:54 +00:00
Owner
Copy Link

If the daemon does not provide Measurement Args, the CLI can't execute the attestation. I believe we should not accept grpc::NewVmResp that does not contain Args. This should be replaced with a NewVmResp that contains an error with the message "SNP Node did not provide measurement args"

Also, this means the contract is broken so we need to discuss the cases.

If the daemon does not provide Measurement Args, the CLI can't execute the attestation. I believe we should not accept `grpc::NewVmResp` that does not contain Args. This should be replaced with a NewVmResp that contains an error with the message "SNP Node did not provide measurement args" Also, this means the contract is broken so we need to discuss the cases.
ghe0 marked this conversation as resolved
ramrem added 1 commit 2025-01-06 11:57:57 +00:00
ghe0 commented 2025-01-06 17:40:55 +00:00
Owner
Copy Link
    pub async fn submit_newvm_resp(&self, new_vm_resp: grpc::NewVmResp) {
        let newvmreq = match self.tmp_newvm_reqs.remove(&new_vm_resp.uuid) {
            Some((_, r)) => r,
            None => {
                log::error!(
                    "Received confirmation for ghost NewVMReq {}",
                    new_vm_resp.uuid
                );
                return;
            }
        };

If the args don't exist here, the brain should overwrite the NewVmResp to an error.

```rust pub async fn submit_newvm_resp(&self, new_vm_resp: grpc::NewVmResp) { let newvmreq = match self.tmp_newvm_reqs.remove(&new_vm_resp.uuid) { Some((_, r)) => r, None => { log::error!( "Received confirmation for ghost NewVMReq {}", new_vm_resp.uuid ); return; } }; ``` If the args don't exist here, the brain should overwrite the NewVmResp to an error.
ramrem added 1 commit 2025-01-06 19:42:33 +00:00
ramrem added 1 commit 2025-01-06 20:11:07 +00:00 
Also removed the unnecessary else clause
ramrem added 1 commit 2025-01-06 23:09:08 +00:00 
I was updating it after sending it, so I fixed that

Removed the unnessary part from the if statement dealing with the send fucntion

Also refactored the code for the submit_newvm_resp and submit_updatevm_resp functions
ramrem changed title from update functionality for the brain is ready? am not sure, there might be need to do a few more checks to update functionality for the brain 2025-01-07 00:16:44 +00:00
ghe0 merged commit 0e85165240 into main 2025-01-09 22:29:56 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
ghe0
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: ghe0/brain-mock#2
No description provided.
Delete Branch "new-updater"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?