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 +16 -10
1 changed files with 16 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 554468e8b6 - Show all commits

26
src/data.rs
View File

@ -1,5 +1,5 @@
#![allow(dead_code)] #![allow(dead_code)]
use crate::grpc::brain::{self as grpc, MeasurementArgs}; use crate::grpc::brain::{self as grpc};
use chrono::Utc; use chrono::Utc;
use dashmap::DashMap; use dashmap::DashMap;
use log::{debug, info, warn}; use log::{debug, info, warn};
@ -87,7 +87,6 @@ pub struct BrainData {
daemon_deletevm_tx: DashMap<String, Sender<grpc::DeleteVmReq>>, daemon_deletevm_tx: DashMap<String, Sender<grpc::DeleteVmReq>>,
daemon_newvm_tx: DashMap<String, Sender<grpc::NewVmReq>>, daemon_newvm_tx: DashMap<String, Sender<grpc::NewVmReq>>,
daemon_updatevm_tx: DashMap<String, Sender<grpc::UpdateVmReq>>, 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)] #[derive(Debug)]
@ -107,7 +106,6 @@ impl BrainData {
daemon_deletevm_tx: DashMap::new(), daemon_deletevm_tx: DashMap::new(),
daemon_newvm_tx: DashMap::new(), daemon_newvm_tx: DashMap::new(),
daemon_updatevm_tx: DashMap::new(), daemon_updatevm_tx: DashMap::new(),
daemon_get_measurement_tx: DashMap::new(),
} }
} }
@ -210,20 +208,28 @@ impl BrainData {
} }
let mut public_ipv4 = String::new(); let mut public_ipv4 = String::new();
let mut public_ipv6 = String::new(); let mut public_ipv6 = String::new();
for ip in new_vm_resp.args.clone().unwrap_or(grpc::MeasurementArgs::default()).ips { let args = match new_vm_resp.args {
Some(args) => args,
None => {
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.
log::error!("NewVmResp does not contain MeasurementArgs for {}", new_vm_resp.uuid);
return;
}
};
for ip in args.ips {
if let Ok(ipv4_addr) = std::net::Ipv4Addr::from_str(&ip.address) { if let Ok(ipv4_addr) = std::net::Ipv4Addr::from_str(&ip.address) {
if !ipv4_addr.is_private() && !ipv4_addr.is_link_local() { if !ipv4_addr.is_private() && !ipv4_addr.is_link_local() {
public_ipv4 = ipv4_addr.to_string(); public_ipv4 = ipv4_addr.to_string();
} }
continue; continue;
} }
if let Ok(ipv6_addr) = std::net::Ipv6Addr::from_str(&ip.address) { if let Ok(ipv6_addr) = std::net::Ipv6Addr::from_str(&ip.address) {
public_ipv6 = ipv6_addr.to_string(); public_ipv6 = ipv6_addr.to_string();
} }
} }
let contract = Contract { let contract = Contract {
uuid: new_vm_resp.uuid, uuid: new_vm_resp.uuid,
exposed_ports: new_vm_resp.args.unwrap_or(grpc::MeasurementArgs::default()).exposed_ports, exposed_ports: args.exposed_ports,
public_ipv4, public_ipv4,
public_ipv6, public_ipv6,
created_at: Utc::now().to_rfc3339(), created_at: Utc::now().to_rfc3339(),