fix(multiservice-discovery): registry sync returns on missing nns_pub…

…lic_key and msd pings nns before adding the definition (#48)

* registry sync returns on missing nns_public_key

* adding pinging of networks nns before adding definition

* renaming function

* adding oci image with debian slim

* fixing image sha

* fixing image

* migrating to debian slim for msd

* using ubuntu 22.04 for bazel

.github/workflows/bazel.yaml

@@ -2,7 +2,7 @@ name: Bazel
 on: [push]
 jobs:
   bazel:
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
     steps:
       - name: Free Disk Space (Ubuntu)
         uses: jlumbroso/free-disk-space@v1.3.1

WORKSPACE.bazel

@@ -190,6 +190,13 @@ oci_pull(
     image = "index.docker.io/bitnami/git",
 )
 
+oci_pull(
+    # tag = bookworm-20231218-slim
+    name = "debian-slim",
+    digest = "sha256:45287d89d96414e57c7705aa30cb8f9836ef30ae8897440dd8f06c4cff801eec",
+    image = "index.docker.io/library/debian",
+)
+
 http_archive(
     name = "rules_pkg",
     sha256 = "8f9ee2dc10c1ae514ee599a8b42ed99fa262b757058f65ad3c384289ff70c4b8",

rs/ic-observability/multiservice-discovery-downloader/BUILD.bazel

@@ -23,6 +23,7 @@ rust_binary(
 rust_binary_oci_image_rules(
     name = "oci_image",
     src = ":multiservice-discovery-downloader",
+    base_image = "@debian-slim"
 )
 
 rust_test(

rs/ic-observability/multiservice-discovery/BUILD.bazel

@@ -23,6 +23,7 @@ rust_binary(
 rust_binary_oci_image_rules(
     name = "oci_image",
     src = ":multiservice-discovery",
+    base_image = "@debian-slim"
 )
 
 rust_test(

rs/ic-observability/multiservice-discovery/src/server_handlers/add_definition_handler.rs

@@ -4,6 +4,7 @@ use std::thread::JoinHandle;
 use std::time::Duration;
 
 use ic_crypto_utils_threshold_sig_der::parse_threshold_sig_key_from_der;
+use service_discovery::registry_sync::nns_reachable;
 use slog::Logger;
 use tokio::sync::Mutex;
 use warp::Reply;
@@ -22,10 +23,7 @@ pub struct AddDefinitionBinding {
     pub handles: Arc<Mutex<Vec<JoinHandle<()>>>>,
 }
 
-pub async fn add_definition(
-    definition: DefinitionDto,
-    binding: AddDefinitionBinding,
-) -> WebResult<impl Reply> {
+pub async fn add_definition(definition: DefinitionDto, binding: AddDefinitionBinding) -> WebResult<impl Reply> {
     let public_key = match definition.public_key {
         Some(pk) => {
             let decoded = base64::decode(pk).unwrap();
@@ -52,6 +50,13 @@ pub async fn add_definition(
         ));
     }
 
+    if !nns_reachable(definition.nns_urls.clone()).await {
+        return Ok(warp::reply::with_status(
+            "Couldn't ping nns of that definition".to_string(),
+            warp::http::StatusCode::BAD_REQUEST,
+        ));
+    }
+
     let (stop_signal_sender, stop_signal_rcv) = crossbeam::channel::bounded::<()>(0);
     let definition = Definition::new(
         definition.nns_urls,

rs/ic-observability/service-discovery/src/registry_sync.rs

@@ -14,9 +14,7 @@ use ic_registry_common_proto::pb::local_store::v1::{
     ChangelogEntry as PbChangelogEntry, KeyMutation as PbKeyMutation, MutationType,
 };
 use ic_registry_keys::{make_crypto_threshold_signing_pubkey_key, ROOT_SUBNET_ID_KEY};
-use ic_registry_local_store::{
-    Changelog, ChangelogEntry, KeyMutation, LocalStoreImpl, LocalStoreWriter,
-};
+use ic_registry_local_store::{Changelog, ChangelogEntry, KeyMutation, LocalStoreImpl, LocalStoreWriter};
 use ic_registry_nns_data_provider::registry::RegistryCanister;
 use ic_types::{PrincipalId, RegistryVersion, SubnetId};
 use registry_canister::mutations::common::decode_registry_value;
@@ -41,10 +39,7 @@ pub async fn sync_local_registry(
         registry_cache.update_to_latest_version();
         registry_cache.get_latest_version()
     };
-    debug!(
-        log,
-        "Syncing registry version from version : {}", latest_version
-    );
+    debug!(log, "Syncing registry version from version : {}", latest_version);
 
     if use_current_version && latest_version != ZERO_REGISTRY_VERSION {
         debug!(log, "Skipping syncing with registry, using local version");
@@ -59,10 +54,19 @@ pub async fn sync_local_registry(
     let mut latest_certified_time = 0;
     let mut updates = vec![];
     let nns_public_key = match public_key {
-        Some(pk) => Ok(pk),
-        _ => get_nns_public_key(&registry_canister)
-            .await
-            .map_err(|e| anyhow::format_err!("Failed to get nns_public_key: {}", e)),
+        Some(pk) => pk,
+        _ => {
+            let maybe_key = get_nns_public_key(&registry_canister)
+                .await
+                .map_err(|e| anyhow::format_err!("Failed to get nns_public_key: {}", e));
+
+            if let Err(e) = maybe_key {
+                let network_name = local_path.file_name().unwrap().to_str().unwrap();
+                debug!(log, "Unable to fetch public key for network {}: {:?}", network_name, e);
+                return;
+            }
+            maybe_key.unwrap()
+        }
     };
 
     loop {
@@ -80,23 +84,21 @@ pub async fn sync_local_registry(
         }
 
         if let Ok((mut initial_records, _, t)) = registry_canister
-            .get_certified_changes_since(latest_version.get(), nns_public_key.as_ref().unwrap())
+            .get_certified_changes_since(latest_version.get(), &nns_public_key)
             .await
         {
             initial_records.sort_by_key(|r| r.version);
-            let changelog = initial_records
-                .iter()
-                .fold(Changelog::default(), |mut cl, r| {
-                    let rel_version = (r.version - latest_version).get();
-                    if cl.len() < rel_version as usize {
-                        cl.push(ChangelogEntry::default());
-                    }
-                    cl.last_mut().unwrap().push(KeyMutation {
-                        key: r.key.clone(),
-                        value: r.value.clone(),
-                    });
-                    cl
+            let changelog = initial_records.iter().fold(Changelog::default(), |mut cl, r| {
+                let rel_version = (r.version - latest_version).get();
+                if cl.len() < rel_version as usize {
+                    cl.push(ChangelogEntry::default());
+                }
+                cl.last_mut().unwrap().push(KeyMutation {
+                    key: r.key.clone(),
+                    value: r.value.clone(),
                 });
+                cl
+            });
 
             let versions_count = changelog.len();
 
@@ -152,25 +154,16 @@ pub async fn sync_local_registry(
     }
 
     futures::future::join_all(updates).await;
-    local_store
-        .update_certified_time(latest_certified_time)
-        .unwrap();
-    info!(
-        log,
-        "Synced all registry versions in : {:?}",
-        start.elapsed()
-    )
+    local_store.update_certified_time(latest_certified_time).unwrap();
+    info!(log, "Synced all registry versions in : {:?}", start.elapsed())
 }
 
-async fn get_nns_public_key(
-    registry_canister: &RegistryCanister,
-) -> anyhow::Result<ThresholdSigPublicKey> {
+async fn get_nns_public_key(registry_canister: &RegistryCanister) -> anyhow::Result<ThresholdSigPublicKey> {
     let (nns_subnet_id_vec, _) = registry_canister
         .get_value(ROOT_SUBNET_ID_KEY.as_bytes().to_vec(), None)
         .await
         .map_err(|e| anyhow::format_err!("failed to get root subnet: {}", e))?;
-    let nns_subnet_id =
-        decode_registry_value::<ic_protobuf::types::v1::SubnetId>(nns_subnet_id_vec);
+    let nns_subnet_id = decode_registry_value::<ic_protobuf::types::v1::SubnetId>(nns_subnet_id_vec);
     let (nns_pub_key_vec, _) = registry_canister
         .get_value(
             make_crypto_threshold_signing_pubkey_key(SubnetId::new(
@@ -182,8 +175,14 @@ async fn get_nns_public_key(
         )
         .await
         .map_err(|e| anyhow::format_err!("failed to get public key: {}", e))?;
-    Ok(ThresholdSigPublicKey::try_from(
-        PublicKey::decode(nns_pub_key_vec.as_slice()).expect("invalid public key"),
+    Ok(
+        ThresholdSigPublicKey::try_from(PublicKey::decode(nns_pub_key_vec.as_slice()).expect("invalid public key"))
+            .expect("failed to create thresholdsig public key"),
     )
-    .expect("failed to create thresholdsig public key"))
+}
+
+pub async fn nns_reachable(nns_urls: Vec<Url>) -> bool {
+    let registry_canister = RegistryCanister::new(nns_urls);
+
+    get_nns_public_key(&registry_canister).await.is_ok()
 }