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 +13 -13
3 changed files with 13 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit bff04c7b7e - Show all commits

6
brain.proto
View File

@ -118,7 +118,7 @@ service BrainDaemonService {
rpc ListVMContracts (ListVMContractsReq) returns (stream VMContract);
rpc GetUpdateVMReq (NodePubkey) returns (stream UpdateVMReq);
rpc SendUpdateVMResp (stream UpdateVMResp) returns (Empty);
// rpc SendMeasurementArgs (NodePubkey) returns (MeasurementArgs);
//rpc GetMeasurementArgs (ListVMContractsReq) returns (stream MeasurementArgs);
}
message NodeFilters {
@ -151,5 +151,5 @@ service BrainCliService {
rpc GetOneNode (NodeFilters) returns (NodeListResp);
rpc DeleteVM (DeleteVMReq) returns (Empty);
rpc UpdateVM (UpdateVMReq) returns (UpdateVMResp);
// rpc GetMeasurementArgs (ListVMContractsReq) returns (MeasurementArgs);
}
//rpc GetMeasurementArgs (ListVMContractsReq) returns (MeasurementArgs);
}

18
src/data.rs
View File

@ -1,5 +1,5 @@
#![allow(dead_code)]
use crate::grpc::brain as grpc;
use crate::grpc::brain::{self as grpc, MeasurementArgs};
use chrono::Utc;
use dashmap::DashMap;
use log::{debug, info, warn};
@ -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 marked this conversation as resolved Outdated
ghe0 commented 2025-01-04 23:00:17 +00:00
Outdated
Review
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
Outdated
Review
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.
}
#[derive(Debug)]
@ -106,6 +107,7 @@ impl BrainData {
daemon_deletevm_tx: DashMap::new(),
daemon_newvm_tx: DashMap::new(),
daemon_updatevm_tx: DashMap::new(),
daemon_get_measurement_tx: DashMap::new(),
}
}
@ -208,7 +210,7 @@ impl BrainData {
}
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 marked this conversation as resolved Outdated
ghe0 commented 2025-01-04 23:03:54 +00:00
Outdated
Review
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.
if let Ok(ipv4_addr) = std::net::Ipv4Addr::from_str(&ip.address) {
if !ipv4_addr.is_private() && !ipv4_addr.is_link_local() {
public_ipv4 = ipv4_addr.to_string();
@ -221,7 +223,7 @@ impl BrainData {
}
let contract = Contract {
uuid: new_vm_resp.uuid,
exposed_ports: new_vm_resp.exposed_ports,
exposed_ports: new_vm_resp.args.unwrap_or(grpc::MeasurementArgs::default()).exposed_ports,
public_ipv4,
public_ipv6,
created_at: Utc::now().to_rfc3339(),
@ -293,9 +295,7 @@ impl BrainData {
self.submit_newvm_resp(grpc::NewVmResp {
error: "Daemon is offline.".to_string(),
uuid: req.uuid,
exposed_ports: Vec::new(),
ovmf_hash: "".to_string(),
ips: Vec::new(),
args: None,
})
.await;
}
@ -319,7 +319,7 @@ impl BrainData {
let _ = tx.send(grpc::UpdateVmResp {
uuid,
error: "Contract does not exist.".to_string(),
ovmf_hash: "".to_string(),
args: None,
});
return;
}
@ -344,7 +344,7 @@ impl BrainData {
self.submit_updatevm_resp(grpc::UpdateVmResp {
uuid,
error: "Daemon is offline.".to_string(),
ovmf_hash: "".to_string(),
args: None,
})
.await;
}
@ -354,7 +354,7 @@ impl BrainData {
self.submit_updatevm_resp(grpc::UpdateVmResp {
uuid,
error: "Daemon is offline.".to_string(),
ovmf_hash: "".to_string(),
args: None,
})
.await;
}

2
src/grpc.rs
View File

@ -256,7 +256,7 @@ impl BrainDaemonService for BrainDaemonMock {
data.submit_updatevm_resp(UpdateVmResp {
error: "Daemon not connected.".to_string(),
uuid: updatevmreq.uuid,
ovmf_hash: "".to_string(),
args: None,
})
.await;
break;