From 3c7cc7e970236cac7ac55c9e0d4e7ef0c4378e6b Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Wed, 9 Dec 2020 13:49:46 -0500 Subject: [PATCH 01/15] cleanup broken/moved tests + clippy + remove windows code in the upcoming commits, i'll add the code to shell-out to `aziot check`, which will populate the `iothub_hostname` variable (which is currently set to None). --- edgelet/Cargo.lock | 9 - edgelet/Cargo.toml | 1 - edgelet/iotedge/Cargo.toml | 2 - .../src/check/checks/aziot_edged_version.rs | 4 - .../check/checks/certificates_quickstart.rs | 113 ------ .../check/checks/connect_management_uri.rs | 6 - .../src/check/checks/container_engine_dns.rs | 2 +- .../checks/container_engine_installed.rs | 9 - .../src/check/checks/container_engine_ipv6.rs | 6 +- .../check/checks/container_engine_is_moby.rs | 17 +- .../src/check/checks/host_connect_upstream.rs | 18 +- .../src/check/checks/host_local_time.rs | 137 ------- .../checks/identity_certificate_expiry.rs | 158 -------- edgelet/iotedge/src/check/checks/mod.rs | 22 +- .../src/check/checks/parent_hostname.rs | 1 + .../check/checks/storage_mounted_from_host.rs | 11 +- .../src/check/checks/well_formed_config.rs | 7 +- .../checks/well_formed_connection_string.rs | 70 ---- edgelet/iotedge/src/check/mod.rs | 337 ++--------------- edgelet/iotedge/src/check/stdout.rs | 39 +- ...nnect_dps_endpoint.rs => tls_handshake.rs} | 73 ---- edgelet/iotedge/src/main.rs | 47 +-- edgelet/iotedge/src/support_bundle.rs | 2 +- edgelet/mini-sntp/Cargo.toml | 10 - edgelet/mini-sntp/src/error.rs | 126 ------- edgelet/mini-sntp/src/lib.rs | 349 ------------------ 26 files changed, 68 insertions(+), 1508 deletions(-) delete mode 100644 edgelet/iotedge/src/check/checks/certificates_quickstart.rs delete mode 100644 edgelet/iotedge/src/check/checks/host_local_time.rs delete mode 100644 edgelet/iotedge/src/check/checks/identity_certificate_expiry.rs delete mode 100644 edgelet/iotedge/src/check/checks/well_formed_connection_string.rs rename edgelet/iotedge/src/check/{checks/host_connect_dps_endpoint.rs => tls_handshake.rs} (65%) delete mode 100644 edgelet/mini-sntp/Cargo.toml delete mode 100644 edgelet/mini-sntp/src/error.rs delete mode 100644 edgelet/mini-sntp/src/lib.rs diff --git a/edgelet/Cargo.lock b/edgelet/Cargo.lock index ac7bd14d734..975d7ad30ff 100644 --- a/edgelet/Cargo.lock +++ b/edgelet/Cargo.lock @@ -1282,7 +1282,6 @@ dependencies = [ "lazy_static", "libc", "management", - "mini-sntp", "native-tls", "openssl", "regex 0.2.11", @@ -1427,14 +1426,6 @@ version = "0.3.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2a60c7ce501c71e03a9c9c0d35b861413ae925bd979cc7a4e30d060069aaac8d" -[[package]] -name = "mini-sntp" -version = "0.1.0" -dependencies = [ - "chrono", - "failure", -] - [[package]] name = "miniz_oxide" version = "0.3.7" diff --git a/edgelet/Cargo.toml b/edgelet/Cargo.toml index 477f416be52..ac18f01c2dc 100644 --- a/edgelet/Cargo.toml +++ b/edgelet/Cargo.toml @@ -13,7 +13,6 @@ members = [ "identity-client", "iotedge", "management", - "mini-sntp", "systemd", ] diff --git a/edgelet/iotedge/Cargo.toml b/edgelet/iotedge/Cargo.toml index b630abe8a81..9e2f8f49c59 100644 --- a/edgelet/iotedge/Cargo.toml +++ b/edgelet/iotedge/Cargo.toml @@ -37,7 +37,6 @@ edgelet-docker = { path = "../edgelet-docker" } edgelet-http = { path = "../edgelet-http" } edgelet-http-mgmt = { path = "../edgelet-http-mgmt" } management = { path = "../management" } -mini-sntp = { path = "../mini-sntp" } support-bundle = { path = "../support-bundle" } [target.'cfg(unix)'.dependencies] @@ -48,4 +47,3 @@ sysinfo = "0.14.10" [dev-dependencies] edgelet-test-utils = { path = "../edgelet-test-utils" } tempfile = "3.1.0" - diff --git a/edgelet/iotedge/src/check/checks/aziot_edged_version.rs b/edgelet/iotedge/src/check/checks/aziot_edged_version.rs index 01ed6b45690..a2e37eaf12c 100644 --- a/edgelet/iotedge/src/check/checks/aziot_edged_version.rs +++ b/edgelet/iotedge/src/check/checks/aziot_edged_version.rs @@ -41,10 +41,6 @@ impl AziotEdgedVersion { let mut process = Command::new(&check.aziot_edged); process.arg("--version"); - if cfg!(windows) { - process.env("IOTEDGE_RUN_AS_CONSOLE", "true"); - } - let output = process .output() .context("Could not spawn aziot-edged process")?; diff --git a/edgelet/iotedge/src/check/checks/certificates_quickstart.rs b/edgelet/iotedge/src/check/checks/certificates_quickstart.rs deleted file mode 100644 index 35528b1fe62..00000000000 --- a/edgelet/iotedge/src/check/checks/certificates_quickstart.rs +++ /dev/null @@ -1,113 +0,0 @@ -use std::ffi::OsStr; -use std::path::PathBuf; - -use failure::{self, Context, ResultExt}; - -use edgelet_core::RuntimeSettings; - -use super::identity_certificate_expiry::CertificateValidity; -use crate::check::{checker::Checker, Check, CheckResult}; - -#[derive(Default, serde_derive::Serialize)] -pub(crate) struct CertificatesQuickstart { - device_ca_cert_path: Option, - certificate_info: Option, -} - -impl Checker for CertificatesQuickstart { - fn id(&self) -> &'static str { - "certificates-quickstart" - } - fn description(&self) -> &'static str { - "production readiness: certificates" - } - fn execute(&mut self, check: &mut Check, _: &mut tokio::runtime::Runtime) -> CheckResult { - self.inner_execute(check) - .unwrap_or_else(CheckResult::Failed) - } - fn get_json(&self) -> serde_json::Value { - serde_json::to_value(self).unwrap() - } -} - -impl CertificatesQuickstart { - fn inner_execute(&mut self, check: &mut Check) -> Result { - let settings = if let Some(settings) = &check.settings { - settings - } else { - return Ok(CheckResult::Skipped); - }; - - check.device_ca_cert_path = Some({ - if let Some(certificates) = settings.certificates().device_cert() { - certificates.device_ca_cert()? - } else { - let certs_dir = settings.homedir().join("hsm").join("certs"); - - let mut device_ca_cert_path = None; - - let entries = std::fs::read_dir(&certs_dir).with_context(|_| { - format!("Could not enumerate files under {}", certs_dir.display()) - })?; - for entry in entries { - let entry = entry.with_context(|_| { - format!("Could not enumerate files under {}", certs_dir.display()) - })?; - let path = entry.path(); - if let Some(file_name) = path.file_name().and_then(OsStr::to_str) { - if file_name.starts_with("device_ca_alias") - && file_name.ends_with(".cert.pem") - { - device_ca_cert_path = Some(path); - break; - } - } - } - - device_ca_cert_path.ok_or_else(|| { - Context::new(format!( - "Could not find device CA certificate under {}", - certs_dir.display(), - )) - })? - } - }); - self.device_ca_cert_path = check.device_ca_cert_path.clone(); - - if settings.certificates().device_cert().is_none() { - let certificate_info = CertificateValidity::parse( - "Device CA certificate".to_owned(), - check.device_ca_cert_path.clone().unwrap(), - )?; - let not_after = certificate_info.not_after; - self.certificate_info = Some(certificate_info); - - let now = chrono::Utc::now(); - - if not_after < now { - return Ok(CheckResult::Warning( - Context::new(format!( - "The Edge device is using self-signed automatically-generated development certificates.\n\ - The certs expired at {}. Restart the IoT Edge daemon to generate new development certs.\n\ - Please consider using production certificates instead. See https://aka.ms/iotedge-prod-checklist-certs for best practices.", - not_after, - )) - .into(), - )); - } else { - return Ok(CheckResult::Warning( - Context::new(format!( - "The Edge device is using self-signed automatically-generated development certificates.\n\ - They will expire in {} days (at {}) causing module-to-module and downstream device communication to fail on an active deployment.\n\ - After the certs have expired, restarting the IoT Edge daemon will trigger it to generate new development certs.\n\ - Please consider using production certificates instead. See https://aka.ms/iotedge-prod-checklist-certs for best practices.", - (not_after - now).num_days(), not_after, - )) - .into(), - )); - } - } - - Ok(CheckResult::Ok) - } -} diff --git a/edgelet/iotedge/src/check/checks/connect_management_uri.rs b/edgelet/iotedge/src/check/checks/connect_management_uri.rs index 7ef043a4e38..7c78003932b 100644 --- a/edgelet/iotedge/src/check/checks/connect_management_uri.rs +++ b/edgelet/iotedge/src/check/checks/connect_management_uri.rs @@ -81,12 +81,6 @@ impl ConnectManagementUri { connect_management_uri.to_uds_file_path() .context("Could not parse connect.management_uri: does not represent a valid file path")?; - // On Windows we mount the parent folder because we can't mount the socket files directly - #[cfg(windows)] - let socket_path = - socket_path.parent() - .ok_or_else(|| Context::new("Could not parse connect.management_uri: does not have a parent directory"))?; - let socket_path = socket_path.to_str() .ok_or_else(|| Context::new("Could not parse connect.management_uri: file path is not valid utf-8"))?; diff --git a/edgelet/iotedge/src/check/checks/container_engine_dns.rs b/edgelet/iotedge/src/check/checks/container_engine_dns.rs index 07b74a05870..a3f34f99d68 100644 --- a/edgelet/iotedge/src/check/checks/container_engine_dns.rs +++ b/edgelet/iotedge/src/check/checks/container_engine_dns.rs @@ -64,7 +64,7 @@ impl ContainerEngineDns { .context(MESSAGE)?; self.dns = daemon_config.dns.clone(); - if let Some(&[]) | None = daemon_config.dns.as_ref().map(std::ops::Deref::deref) { + if daemon_config.dns.map_or(true, |e| e.is_empty()) { return Ok(CheckResult::Warning(Context::new(MESSAGE).into())); } diff --git a/edgelet/iotedge/src/check/checks/container_engine_installed.rs b/edgelet/iotedge/src/check/checks/container_engine_installed.rs index 1638c9a1d81..e80e3b5f64a 100644 --- a/edgelet/iotedge/src/check/checks/container_engine_installed.rs +++ b/edgelet/iotedge/src/check/checks/container_engine_installed.rs @@ -72,15 +72,6 @@ impl ContainerEngineInstalled { return Ok(CheckResult::Fatal(err.context(error_message).into())); } } - - #[cfg(windows)] - { - if message.contains("Access is denied") { - error_message += - "\nYou might need to run this command as Administrator."; - return Ok(CheckResult::Fatal(err.context(error_message).into())); - } - } } return Err(err.context(error_message).into()); diff --git a/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs b/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs index a65792523ab..8d3bf91839f 100644 --- a/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs +++ b/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs @@ -72,8 +72,10 @@ impl ContainerEngineIPv6 { .context(MESSAGE)?; self.actual_use_ipv6 = daemon_config.ipv6; - match (daemon_config.ipv6.unwrap_or_default(), is_edge_ipv6_configured) { - (true, _) if cfg!(windows) => Err(Context::new("IPv6 container network configuration is not supported for the Windows operating system.").into()), + match ( + daemon_config.ipv6.unwrap_or_default(), + is_edge_ipv6_configured, + ) { (true, _) => Ok(CheckResult::Ok), (false, true) => Err(Context::new(MESSAGE).into()), (false, false) => Ok(CheckResult::Ignored), diff --git a/edgelet/iotedge/src/check/checks/container_engine_is_moby.rs b/edgelet/iotedge/src/check/checks/container_engine_is_moby.rs index 41417628279..893a52da882 100644 --- a/edgelet/iotedge/src/check/checks/container_engine_is_moby.rs +++ b/edgelet/iotedge/src/check/checks/container_engine_is_moby.rs @@ -25,6 +25,7 @@ impl Checker for ContainerEngineIsMoby { } impl ContainerEngineIsMoby { + #[allow(clippy::unnecessary_wraps)] // keeps this inner_execute consistent with all other checks fn inner_execute(&mut self, check: &mut Check) -> Result { const MESSAGE: &str = "Device is not using a production-supported container engine (moby-engine).\n\ @@ -38,22 +39,6 @@ impl ContainerEngineIsMoby { return Ok(CheckResult::Skipped); }; - #[cfg(windows)] - { - let settings = if let Some(settings) = &check.settings { - settings - } else { - return Ok(CheckResult::Skipped); - }; - - let moby_runtime_uri = settings.moby_runtime().uri().to_string(); - self.moby_runtime_uri = Some(moby_runtime_uri.clone()); - - if moby_runtime_uri != "npipe://./pipe/iotedge_moby_engine" { - return Ok(CheckResult::Warning(Context::new(MESSAGE).into())); - } - } - let docker_server_major_version = docker_server_version .split('.') .next() diff --git a/edgelet/iotedge/src/check/checks/host_connect_upstream.rs b/edgelet/iotedge/src/check/checks/host_connect_upstream.rs index f58ac40e781..783014c3b03 100644 --- a/edgelet/iotedge/src/check/checks/host_connect_upstream.rs +++ b/edgelet/iotedge/src/check/checks/host_connect_upstream.rs @@ -1,7 +1,9 @@ use edgelet_core::RuntimeSettings; use crate::check::{ - checker::Checker, upstream_protocol_port::UpstreamProtocolPort, Check, CheckResult, + tls_handshake::{resolve_and_tls_handshake, resolve_and_tls_handshake_proxy}, + upstream_protocol_port::UpstreamProtocolPort, + Check, CheckResult, Checker, }; pub(crate) fn get_host_connect_upstream_tests() -> Vec> { @@ -81,15 +83,13 @@ impl HostConnectUpstream { .and_then(|settings| settings.agent().env().get("https_proxy").cloned()); if let Some(proxy) = &self.proxy { - runtime.block_on( - super::host_connect_dps_endpoint::resolve_and_tls_handshake_proxy( - upstream_hostname.clone(), - Some(self.port_number), - proxy.clone(), - ), - )?; + runtime.block_on(resolve_and_tls_handshake_proxy( + upstream_hostname.clone(), + Some(self.port_number), + proxy.clone(), + ))?; } else { - super::host_connect_dps_endpoint::resolve_and_tls_handshake( + resolve_and_tls_handshake( &(&**upstream_hostname, self.port_number), upstream_hostname, &format!("{}:{}", upstream_hostname, self.port_number), diff --git a/edgelet/iotedge/src/check/checks/host_local_time.rs b/edgelet/iotedge/src/check/checks/host_local_time.rs deleted file mode 100644 index 7352b3b2495..00000000000 --- a/edgelet/iotedge/src/check/checks/host_local_time.rs +++ /dev/null @@ -1,137 +0,0 @@ -use crate::check::{checker::Checker, Check, CheckResult}; -use chrono::{DateTime, Utc}; -use edgelet_core::RuntimeSettings; -use failure::{self, Context, Fail, ResultExt}; -use hyper::Client; -use hyper_tls::HttpsConnector; - -#[derive(Default, serde_derive::Serialize)] -pub(crate) struct HostLocalTime { - offset: Option, -} - -impl Checker for HostLocalTime { - fn id(&self) -> &'static str { - "host-local-time" - } - fn description(&self) -> &'static str { - "host time is close to reference time" - } - fn execute(&mut self, check: &mut Check, runtime: &mut tokio::runtime::Runtime) -> CheckResult { - self.inner_execute(check, runtime) - .unwrap_or_else(CheckResult::Failed) - } - fn get_json(&self) -> serde_json::Value { - serde_json::to_value(self).unwrap() - } -} - -impl HostLocalTime { - fn inner_execute( - &mut self, - check: &mut Check, - runtime: &mut tokio::runtime::Runtime, - ) -> Result { - let settings = if let Some(settings) = &check.settings { - settings - } else { - return Ok(CheckResult::Skipped); - }; - - #[allow(clippy::option_if_let_else)] - if let Some(parent_hostname) = settings.parent_hostname() { - self.check_vs_parent_time(runtime, parent_hostname) - } else { - // This test is not run if parent hostname is not defined - self.check_vs_ntp_time(check) - } - } - - fn check_vs_ntp_time(&mut self, check: &mut Check) -> Result { - fn is_server_unreachable_error(err: &mini_sntp::Error) -> bool { - match err.kind() { - mini_sntp::ErrorKind::ResolveNtpPoolHostname(_) => true, - mini_sntp::ErrorKind::SendClientRequest(err) - | mini_sntp::ErrorKind::ReceiveServerResponse(err) => { - err.kind() == std::io::ErrorKind::TimedOut || // Windows - err.kind() == std::io::ErrorKind::WouldBlock // Unix - } - _ => false, - } - } - - let mini_sntp::SntpTimeQueryResult { - local_clock_offset, .. - } = match mini_sntp::query(&check.ntp_server) { - Ok(result) => result, - Err(err) => { - if is_server_unreachable_error(&err) { - return Ok(CheckResult::Warning( - err.context("Could not query NTP server").into(), - )); - } else { - return Err(err.context("Could not query NTP server").into()); - } - } - }; - - let offset = local_clock_offset.num_seconds().abs(); - self.offset = Some(offset); - if offset >= 10 { - return Ok(CheckResult::Warning(Context::new(format!( - "Time on the device is out of sync with the NTP server. This may cause problems connecting to IoT Hub.\n\ - Please ensure time on device is accurate, for example by {}.", - if cfg!(windows) { - "setting up the Windows Time service to automatically sync with a time server" - } else { - "installing an NTP daemon" - }, - )).into())); - } - - Ok(CheckResult::Ok) - } - - fn check_vs_parent_time( - &mut self, - runtime: &mut tokio::runtime::Runtime, - parent_hostname: &str, - ) -> Result { - let address = format!("https://{}", parent_hostname); - let parent_address = address - .parse::() - .with_context(|_| format!("unable to parse address{}", address))?; - - let https = HttpsConnector::new(1).with_context(|_| "Could not create https connector")?; - - let response = runtime.block_on( - Client::builder() - .build::<_, hyper::Body>(https) - .get(parent_address), - )?; - - let date = response - .headers() - .get("Date") - .ok_or_else(|| Context::new("Could not get date"))? - .to_str() - .with_context(|_| "Could not convert date to string")?; - - let offset = DateTime::parse_from_rfc2822(date) - .with_context(|_| format!("Could not parse date string {}", date))? - .signed_duration_since(Utc::now()) - .num_seconds() - .abs(); - - self.offset = Some(offset); - if offset >= 10 { - return Ok(CheckResult::Warning(Context::new(format!( - "Time on the device is out of sync its parent with a delay of {} seconds. This may cause problems connecting to the parent.\n\ - Please ensure time on device and on the parent is accurate.", - offset, - )).into())); - } - - Ok(CheckResult::Ok) - } -} diff --git a/edgelet/iotedge/src/check/checks/identity_certificate_expiry.rs b/edgelet/iotedge/src/check/checks/identity_certificate_expiry.rs deleted file mode 100644 index 7e8697496b6..00000000000 --- a/edgelet/iotedge/src/check/checks/identity_certificate_expiry.rs +++ /dev/null @@ -1,158 +0,0 @@ -use std::fs::File; -use std::io::Read; -use std::path::PathBuf; - -use failure::{self, Context, ResultExt}; - -use edgelet_core::{self, AttestationMethod, ManualAuthMethod, ProvisioningType, RuntimeSettings}; - -use crate::check::{checker::Checker, Check, CheckResult}; - -#[derive(Default, serde_derive::Serialize)] -pub(crate) struct IdentityCertificateExpiry { - provisioning_mode: Option<&'static str>, - certificate_info: Option, -} - -impl Checker for IdentityCertificateExpiry { - fn id(&self) -> &'static str { - "identity-certificate-expiry" - } - fn description(&self) -> &'static str { - "production readiness: identity certificates expiry" - } - fn execute(&mut self, check: &mut Check, _: &mut tokio::runtime::Runtime) -> CheckResult { - self.inner_execute(check) - .unwrap_or_else(CheckResult::Failed) - } - fn get_json(&self) -> serde_json::Value { - serde_json::to_value(self).unwrap() - } -} - -impl IdentityCertificateExpiry { - fn inner_execute(&mut self, check: &mut Check) -> Result { - let settings = if let Some(settings) = &check.settings { - settings - } else { - return Ok(CheckResult::Skipped); - }; - - match settings.provisioning().provisioning_type() { - ProvisioningType::Dps(dps) => { - if let AttestationMethod::X509(x509_info) = dps.attestation() { - self.provisioning_mode = Some("dps-x509"); - let path = x509_info.identity_cert()?; - - let result = - CertificateValidity::parse("DPS identity certificate".to_owned(), path)?; - self.certificate_info = Some(result.clone()); - return result.to_check_result(); - } - self.provisioning_mode = Some("dps-other"); - } - ProvisioningType::Manual(manual) => { - if let ManualAuthMethod::X509(x509) = manual.authentication_method() { - self.provisioning_mode = Some("manual-x509"); - let path = x509.identity_cert()?; - let result = CertificateValidity::parse( - "Manual authentication identity certificate".to_owned(), - path, - )?; - self.certificate_info = Some(result.clone()); - return result.to_check_result(); - } - self.provisioning_mode = Some("manual-other"); - } - ProvisioningType::External(_) => { - self.provisioning_mode = Some("external"); - } - } - - Ok(CheckResult::Ignored) - } -} - -#[derive(Debug, serde_derive::Serialize, Clone)] -pub(crate) struct CertificateValidity { - cert_name: String, - cert_path: PathBuf, - pub(crate) not_after: chrono::DateTime, - not_before: chrono::DateTime, -} - -impl CertificateValidity { - pub(crate) fn parse(cert_name: String, cert_path: PathBuf) -> Result { - fn parse_openssl_time( - time: &openssl::asn1::Asn1TimeRef, - ) -> chrono::ParseResult> { - // openssl::asn1::Asn1TimeRef does not expose any way to convert the ASN1_TIME to a Rust-friendly type - // - // Its Display impl uses ASN1_TIME_print, so we convert it into a String and parse it back - // into a chrono::DateTime - let time = time.to_string(); - let time = chrono::NaiveDateTime::parse_from_str(&time, "%b %e %H:%M:%S %Y GMT")?; - Ok(chrono::DateTime::::from_utc(time, chrono::Utc)) - } - - let (not_after, not_before) = File::open(&cert_path) - .map_err(failure::Error::from) - .and_then(|mut device_ca_cert_file| { - let mut device_ca_cert = vec![]; - device_ca_cert_file.read_to_end(&mut device_ca_cert)?; - let device_ca_cert = openssl::x509::X509::stack_from_pem(&device_ca_cert)?; - let device_ca_cert = &device_ca_cert[0]; - - let not_after = parse_openssl_time(device_ca_cert.not_after())?; - let not_before = parse_openssl_time(device_ca_cert.not_before())?; - - Ok((not_after, not_before)) - }) - .with_context(|_| { - format!( - "Could not parse {} as a valid certificate file", - &cert_path.display(), - ) - })?; - - Ok(CertificateValidity { - cert_name, - cert_path, - not_after, - not_before, - }) - } - - fn to_check_result(&self) -> Result { - let cert_path_displayable = self.cert_path.display(); - - let now = chrono::Utc::now(); - - if self.not_before > now { - Err(Context::new(format!( - "{} at {} has not-before time {} which is in the future", - self.cert_name, cert_path_displayable, self.not_before, - )) - .into()) - } else if self.not_after < now { - Err(Context::new(format!( - "{} at {} expired at {}", - self.cert_name, cert_path_displayable, self.not_after, - )) - .into()) - } else if self.not_after < now + chrono::Duration::days(7) { - Ok(CheckResult::Warning( - Context::new(format!( - "{} at {} will expire soon ({}, in {} days)", - self.cert_name, - cert_path_displayable, - self.not_after, - (self.not_after - now).num_days(), - )) - .into(), - )) - } else { - Ok(CheckResult::Ok) - } - } -} diff --git a/edgelet/iotedge/src/check/checks/mod.rs b/edgelet/iotedge/src/check/checks/mod.rs index 630f7493181..8d42eaef32e 100644 --- a/edgelet/iotedge/src/check/checks/mod.rs +++ b/edgelet/iotedge/src/check/checks/mod.rs @@ -1,6 +1,6 @@ -// mod certificates_quickstart; +mod aziot_edged_version; mod connect_management_uri; -// mod container_connect_upstream; +mod container_connect_upstream; mod container_engine_dns; mod container_engine_installed; mod container_engine_ipv6; @@ -8,21 +8,16 @@ mod container_engine_is_moby; mod container_engine_logrotate; mod container_local_time; mod container_resolve_parent_hostname; -// mod host_connect_dps_endpoint; -// mod host_connect_upstream; -mod host_local_time; +mod host_connect_upstream; mod hostname; -// mod identity_certificate_expiry; -mod aziot_edged_version; mod parent_hostname; mod pull_agent_from_upstream; mod storage_mounted_from_host; mod well_formed_config; -// mod well_formed_connection_string; -// pub(crate) use self::certificates_quickstart::CertificatesQuickstart; +pub(crate) use self::aziot_edged_version::AziotEdgedVersion; pub(crate) use self::connect_management_uri::ConnectManagementUri; -// pub(crate) use self::container_connect_upstream::get_host_container_upstream_tests; +pub(crate) use self::container_connect_upstream::get_host_container_upstream_tests; pub(crate) use self::container_engine_dns::ContainerEngineDns; pub(crate) use self::container_engine_installed::ContainerEngineInstalled; pub(crate) use self::container_engine_ipv6::ContainerEngineIPv6; @@ -30,17 +25,12 @@ pub(crate) use self::container_engine_is_moby::ContainerEngineIsMoby; pub(crate) use self::container_engine_logrotate::ContainerEngineLogrotate; pub(crate) use self::container_local_time::ContainerLocalTime; pub(crate) use self::container_resolve_parent_hostname::ContainerResolveParentHostname; -// pub(crate) use self::host_connect_dps_endpoint::HostConnectDpsEndpoint; -// pub(crate) use self::host_connect_upstream::get_host_connect_upstream_tests; -pub(crate) use self::host_local_time::HostLocalTime; +pub(crate) use self::host_connect_upstream::get_host_connect_upstream_tests; pub(crate) use self::hostname::Hostname; -// pub(crate) use self::identity_certificate_expiry::IdentityCertificateExpiry; -pub(crate) use self::aziot_edged_version::AziotEdgedVersion; pub(crate) use self::parent_hostname::ParentHostname; pub(crate) use self::pull_agent_from_upstream::PullAgentFromUpstream; pub(crate) use self::storage_mounted_from_host::{EdgeAgentStorageMounted, EdgeHubStorageMounted}; pub(crate) use self::well_formed_config::WellFormedConfig; -// pub(crate) use self::well_formed_connection_string::WellFormedConnectionString; use std::ffi::OsStr; use std::process::Command; diff --git a/edgelet/iotedge/src/check/checks/parent_hostname.rs b/edgelet/iotedge/src/check/checks/parent_hostname.rs index 7f2bac448f5..88957a9b7a3 100644 --- a/edgelet/iotedge/src/check/checks/parent_hostname.rs +++ b/edgelet/iotedge/src/check/checks/parent_hostname.rs @@ -28,6 +28,7 @@ impl Checker for ParentHostname { } impl ParentHostname { + #[allow(clippy::unnecessary_wraps)] // keeps this inner_execute consistent with all other checks fn inner_execute(&mut self, check: &mut Check) -> Result { let settings = if let Some(settings) = &check.settings { settings diff --git a/edgelet/iotedge/src/check/checks/storage_mounted_from_host.rs b/edgelet/iotedge/src/check/checks/storage_mounted_from_host.rs index 618d8b9c628..6c5e3e1ed4d 100644 --- a/edgelet/iotedge/src/check/checks/storage_mounted_from_host.rs +++ b/edgelet/iotedge/src/check/checks/storage_mounted_from_host.rs @@ -95,15 +95,8 @@ fn storage_mounted_from_host( .and_then(|capture| capture.get(1)) .map(|match_| match_.as_str()) }) - .unwrap_or( - // Hard-code the value here rather than using the tempfile crate. It needs to match .Net Core's implementation, - // and needs to be in the context of the container user instead of the host running `iotedge check`. - if cfg!(windows) { - r"C:\Windows\Temp" - } else { - "/tmp" - }, - ); + // Hard-code the value here rather than using the tempfile crate. + .unwrap_or("/tmp"); let storage_directory = Path::new(&*temp_dir).join(storage_directory_name); *storage_directory_out = Some(storage_directory.clone()); diff --git a/edgelet/iotedge/src/check/checks/well_formed_config.rs b/edgelet/iotedge/src/check/checks/well_formed_config.rs index 90b5b430c02..756759b5b32 100644 --- a/edgelet/iotedge/src/check/checks/well_formed_config.rs +++ b/edgelet/iotedge/src/check/checks/well_formed_config.rs @@ -36,13 +36,8 @@ impl WellFormedConfig { if err.kind() == std::io::ErrorKind::PermissionDenied { return Ok(CheckResult::Fatal( err.context(format!( - "Could not open file {}. You might need to run this command as {}.", + "Could not open file {}. You might need to run this command as root.", config_file.display(), - if cfg!(windows) { - "Administrator" - } else { - "root" - }, )) .into(), )); diff --git a/edgelet/iotedge/src/check/checks/well_formed_connection_string.rs b/edgelet/iotedge/src/check/checks/well_formed_connection_string.rs deleted file mode 100644 index 1050ee82469..00000000000 --- a/edgelet/iotedge/src/check/checks/well_formed_connection_string.rs +++ /dev/null @@ -1,70 +0,0 @@ -use failure::{self, Fail, ResultExt}; - -use edgelet_core::{self, ManualAuthMethod, ProvisioningResult, ProvisioningType, RuntimeSettings}; - -use crate::check::{checker::Checker, Check, CheckResult}; - -#[derive(Default, serde_derive::Serialize)] -pub(crate) struct WellFormedConnectionString { - iothub_hostname: Option, - device_id: Option, -} - -impl Checker for WellFormedConnectionString { - fn id(&self) -> &'static str { - "connection-string" - } - fn description(&self) -> &'static str { - "config.yaml has well-formed connection string" - } - fn execute(&mut self, check: &mut Check, _: &mut tokio::runtime::Runtime) -> CheckResult { - self.inner_execute(check) - .unwrap_or_else(CheckResult::Failed) - } - fn get_json(&self) -> serde_json::Value { - serde_json::to_value(self).unwrap() - } -} - -impl WellFormedConnectionString { - fn inner_execute(&mut self, check: &mut Check) -> Result { - let settings = if let Some(settings) = &check.settings { - settings - } else { - return Ok(CheckResult::Skipped); - }; - - if let ProvisioningType::Manual(manual) = settings.provisioning().provisioning_type() { - let hub = match manual.authentication_method() { - ManualAuthMethod::DeviceConnectionString(cs) => { - let (_, device, hub) = cs.parse_device_connection_string().context( - "Invalid connection string format detected.\n\ - Please check the value of the provisioning.device_connection_string parameter.", - )?; - self.device_id = Some(device); - hub - } - ManualAuthMethod::X509(x509) => x509.iothub_hostname().to_owned(), - }; - check.iothub_hostname = Some(hub); - self.iothub_hostname = check.iothub_hostname.clone(); - } else if check.iothub_hostname.is_none() { - let provisioning_file_path = settings - .homedir() - .join("cache") - .join("provisioning_backup.json"); - - let provision_result = provisioning::restore(&provisioning_file_path).map_err(|e| { - let reason = "Could not retrieve iothub_hostname from provisioning file.\n\ - Please specify the backing IoT Hub name using --iothub-hostname switch if you have that information.\n\ - If no hostname is provided, all hub connectivity tests will be skipped."; - e.context(reason) - })?; - - check.iothub_hostname = Some(provision_result.hub_name().to_owned()); - self.iothub_hostname = check.iothub_hostname.clone(); - } - - Ok(CheckResult::Ok) - } -} diff --git a/edgelet/iotedge/src/check/mod.rs b/edgelet/iotedge/src/check/mod.rs index f3ac0968418..4459ec826b9 100644 --- a/edgelet/iotedge/src/check/mod.rs +++ b/edgelet/iotedge/src/check/mod.rs @@ -21,28 +21,20 @@ use self::additional_info::AdditionalInfo; mod stdout; use self::stdout::Stdout; -// mod upstream_protocol_port; - mod hostname_checks_common; +mod tls_handshake; +mod upstream_protocol_port; mod checker; use checker::Checker; mod checks; -// use checks::{ -// get_host_connect_upstream_tests, get_host_container_upstream_tests, AziotEdgedVersion, CertificatesQuickstart, -// ConnectManagementUri, ContainerEngineDns, ContainerEngineIPv6, ContainerEngineInstalled, -// ContainerEngineIsMoby, ContainerEngineLogrotate, ContainerLocalTime, -// ContainerResolveParentHostname, EdgeAgentStorageMounted, EdgeHubStorageMounted, -// HostConnectDpsEndpoint, HostLocalTime, Hostname, IdentityCertificateExpiry, -// ParentHostname, PullAgentFromUpstream, WellFormedConfig, WellFormedConnectionString, -// WindowsHostVersion, -// }; use checks::{ - AziotEdgedVersion, ConnectManagementUri, ContainerEngineDns, ContainerEngineIPv6, - ContainerEngineInstalled, ContainerEngineIsMoby, ContainerEngineLogrotate, ContainerLocalTime, - ContainerResolveParentHostname, EdgeAgentStorageMounted, EdgeHubStorageMounted, HostLocalTime, - Hostname, ParentHostname, PullAgentFromUpstream, WellFormedConfig, + get_host_connect_upstream_tests, get_host_container_upstream_tests, AziotEdgedVersion, + ConnectManagementUri, ContainerEngineDns, ContainerEngineIPv6, ContainerEngineInstalled, + ContainerEngineIsMoby, ContainerEngineLogrotate, ContainerLocalTime, + ContainerResolveParentHostname, EdgeAgentStorageMounted, EdgeHubStorageMounted, Hostname, + ParentHostname, PullAgentFromUpstream, WellFormedConfig, }; pub struct Check { @@ -52,20 +44,17 @@ pub struct Check { dont_run: BTreeSet, aziot_edged: PathBuf, latest_versions: Result>, - ntp_server: String, output_format: OutputFormat, verbose: bool, warnings_as_errors: bool, additional_info: AdditionalInfo, - // iothub_hostname: Option, - // These optional fields are populated by the checks + iothub_hostname: Option, // populated by `aziot check` settings: Option, docker_host_arg: Option, docker_server_version: Option, - // device_ca_cert_path: Option, } #[derive(Clone, Copy, Debug, PartialEq)] @@ -106,8 +95,6 @@ impl Check { dont_run: BTreeSet, expected_aziot_edged_version: Option, aziot_edged: PathBuf, - // iothub_hostname: Option, - ntp_server: String, output_format: OutputFormat, verbose: bool, warnings_as_errors: bool, @@ -157,11 +144,9 @@ impl Check { let uri = response .headers() .get(hyper::header::LOCATION) - .ok_or_else(|| { - ErrorKind::FetchLatestVersions( - FetchLatestVersionsReason::InvalidOrMissingLocationHeader, - ) - })? + .ok_or(ErrorKind::FetchLatestVersions( + FetchLatestVersionsReason::InvalidOrMissingLocationHeader, + ))? .to_str() .context(ErrorKind::FetchLatestVersions( FetchLatestVersionsReason::InvalidOrMissingLocationHeader, @@ -215,42 +200,36 @@ impl Check { dont_run, aziot_edged, latest_versions: latest_versions.map_err(Some), - ntp_server, output_format, verbose, warnings_as_errors, additional_info: AdditionalInfo::new(), + iothub_hostname: None, settings: None, docker_host_arg: None, docker_server_version: None, - // iothub_hostname, - // device_ca_cert_path: None, }) })) } - fn checks() -> [(&'static str, Vec>); 1] { - /* Note: keep ordering consistant. Later tests may depend on earlier tests. */ + fn checks() -> [(&'static str, Vec>); 2] { + /* Note: keep ordering consistent. Later tests may depend on earlier tests. */ [ ( "Configuration checks", vec![ Box::new(WellFormedConfig::default()), - // Box::new(WellFormedConnectionString::default()), Box::new(ContainerEngineInstalled::default()), Box::new(Hostname::default()), Box::new(ParentHostname::default()), Box::new(ContainerResolveParentHostname::default()), Box::new(ConnectManagementUri::default()), Box::new(AziotEdgedVersion::default()), - Box::new(HostLocalTime::default()), Box::new(ContainerLocalTime::default()), Box::new(ContainerEngineDns::default()), Box::new(ContainerEngineIPv6::default()), - // Box::new(IdentityCertificateExpiry::default()), - // Box::new(CertificatesQuickstart::default()), Box::new(ContainerEngineIsMoby::default()), Box::new(ContainerEngineLogrotate::default()), Box::new(EdgeAgentStorageMounted::default()), @@ -258,13 +237,12 @@ impl Check { Box::new(PullAgentFromUpstream::default()), ], ), - // ("Connectivity checks", { - // let mut tests: Vec> = Vec::new(); - // tests.push(Box::new(HostConnectDpsEndpoint::default())); - // tests.extend(get_host_connect_upstream_tests()); - // tests.extend(get_host_container_upstream_tests()); - // tests - // }), + ("Connectivity checks", { + let mut tests: Vec> = Vec::new(); + tests.extend(get_host_connect_upstream_tests()); + tests.extend(get_host_container_upstream_tests()); + tests + }), ] } @@ -633,15 +611,7 @@ struct CheckOutputSerializable { #[cfg(test)] mod tests { - use super::{ - Check, - CheckResult, - Checker, - ContainerEngineIsMoby, - Hostname, - WellFormedConfig, - // WellFormedConnectionString, - }; + use super::{Check, CheckResult, Checker, ContainerEngineIsMoby, Hostname, WellFormedConfig}; #[test] fn config_file_checks_ok() { @@ -661,11 +631,9 @@ mod tests { "daemon.json".into(), // unused for this test "mcr.microsoft.com/azureiotedge-diagnostics:1.0.0".to_owned(), // unused for this test Default::default(), - Some("1.0.0".to_owned()), // unused for this test - "aziot-edged".into(), // unused for this test - // None, // unused for this test - "pool.ntp.org:123".to_owned(), // unused for this test - super::OutputFormat::Text, // unused for this test + Some("1.0.0".to_owned()), // unused for this test + "aziot-edged".into(), // unused for this test + super::OutputFormat::Text, // unused for this test false, false, )) @@ -676,14 +644,6 @@ mod tests { check_result => panic!("parsing {} returned {:?}", filename, check_result), } - // match WellFormedConnectionString::default().execute(&mut check, &mut runtime) { - // CheckResult::Ok => (), - // check_result => panic!( - // "checking connection string in {} returned {:?}", - // filename, check_result - // ), - // } - match Hostname::default().execute(&mut check, &mut runtime) { CheckResult::Failed(err) => { let message = err.to_string(); @@ -732,11 +692,9 @@ mod tests { "daemon.json".into(), // unused for this test "mcr.microsoft.com/azureiotedge-diagnostics:1.0.0".to_owned(), // unused for this test Default::default(), - Some("1.0.0".to_owned()), // unused for this test - "aziot-edged".into(), // unused for this test - // None, // unused for this test - "pool.ntp.org:123".to_owned(), // unused for this test - super::OutputFormat::Text, // unused for this test + Some("1.0.0".to_owned()), // unused for this test + "aziot-edged".into(), // unused for this test + super::OutputFormat::Text, // unused for this test false, false, )) @@ -803,11 +761,9 @@ mod tests { "daemon.json".into(), // unused for this test "mcr.microsoft.com/azureiotedge-diagnostics:1.0.0".to_owned(), // unused for this test Default::default(), - Some("1.0.0".to_owned()), // unused for this test - "aziot-edged".into(), // unused for this test - // None, // unused for this test - "pool.ntp.org:123".to_owned(), // unused for this test - super::OutputFormat::Text, // unused for this test + Some("1.0.0".to_owned()), // unused for this test + "aziot-edged".into(), // unused for this test + super::OutputFormat::Text, // unused for this test false, false, )) @@ -832,231 +788,6 @@ mod tests { } } - // #[test] - // fn settings_connection_string_dps_hostname() { - // let mut runtime = tokio::runtime::Runtime::new().unwrap(); - - // let filename = "sample_settings.dps.sym.yaml"; - // let config_file = format!( - // "{}/../edgelet-docker/test/{}/{}", - // env!("CARGO_MANIFEST_DIR"), - // "linux", - // filename, - // ); - - // let mut check = runtime - // .block_on(Check::new( - // config_file.into(), - // "daemon.json".into(), // unused for this test - // "mcr.microsoft.com/azureiotedge-diagnostics:1.0.0".to_owned(), // unused for this test - // Default::default(), - // Some("1.0.0".to_owned()), // unused for this test - // "aziot-edged".into(), // unused for this test - // // Some("something.something.com".to_owned()), // pretend user specified --iothub-hostname - // "pool.ntp.org:123".to_owned(), // unused for this test - // super::OutputFormat::Text, // unused for this test - // false, - // false, - // )) - // .unwrap(); - - // match WellFormedConfig::default().execute(&mut check, &mut runtime) { - // CheckResult::Ok => (), - // check_result => panic!("parsing {} returned {:?}", filename, check_result), - // } - - // match WellFormedConnectionString::default().execute(&mut check, &mut runtime) { - // CheckResult::Ok => (), - // check_result => panic!("parsing {} returned {:?}", filename, check_result), - // } - // } - - // This test inexplicably fails in the ci pipeline due to file read errors. - // It has been tested on ubuntu 18.04, raspbian buster and windows. - // It is disabled until the test pipeline issue is resolved. - // use std::fs::File; - // use std::io::{BufRead, BufReader, Write}; - - // use tempfile::tempdir; - // #[test] - // fn settings_connection_string_dps_config_file() { - // let mut runtime = tokio::runtime::Runtime::new().unwrap(); - // let hub_name = "hub_1"; - - // let filename = "sample_settings.dps.sym.yaml"; - // let config_file_source = format!( - // "{}/../edgelet-docker/test/{}/{}", - // env!("CARGO_MANIFEST_DIR"), - // "linux", - // filename, - // ); - - // let tmp_dir = tempdir().unwrap(); - // let config_file = tmp_dir.path().join(filename); - // let provision_file = tmp_dir - // .path() - // .join("cache") - // .join("provisioning_backup.json"); - // std::fs::create_dir(tmp_dir.path().join("cache")).unwrap(); - - // // replace homedir with temp directory - // { - // let mut new_config = File::create(&config_file).unwrap(); - // for line in BufReader::new(File::open(config_file_source).unwrap()).lines() { - // if let Ok(line) = line { - // if line.contains("homedir") { - // let new_line = format!( - // r#"homedir: "{}""#, - // tmp_dir.path().to_str().unwrap().replace(r"\", r"\\") - // ); - // new_config.write_all(new_line.as_bytes()).unwrap(); - // } else { - // new_config.write_all(line.as_bytes()).unwrap(); - // } - // new_config.write_all(b"\n").unwrap(); - // } - // } - // } - - // let fake_result = provisioning::ProvisioningResult::new( - // "a", - // hub_name, - // None, - // provisioning::ReprovisioningStatus::default(), - // None, - // ); - // provisioning::backup(&fake_result, &provision_file).unwrap(); - - // let mut check = runtime - // .block_on(Check::new( - // config_file, - // "daemon.json".into(), // unused for this test - // "mcr.microsoft.com/azureiotedge-diagnostics:1.0.0".to_owned(), // unused for this test - // Default::default(), - // Some("1.0.0".to_owned()), // unused for this test - // "aziot-edged".into(), // unused for this test - // None, // pretend user did not specify --iothub-hostname - // "pool.ntp.org:123".to_owned(), // unused for this test - // super::OutputFormat::Text, // unused for this test - // false, - // false, - // )) - // .unwrap(); - - // match WellFormedConfig::default().execute(&mut check, &mut runtime) { - // CheckResult::Ok => (), - // check_result => panic!("parsing config {} returned {:?}", filename, check_result), - // } - - // match WellFormedConnectionString::default().execute(&mut check, &mut runtime) { - // CheckResult::Ok => { - // assert_eq!(check.iothub_hostname, Some(hub_name.to_owned())); - // } - // check_result => panic!( - // "parsing connection string {} returned {:?}", - // filename, check_result - // ), - // } - // } - - // #[test] - // fn settings_connection_string_dps_err() { - // let mut runtime = tokio::runtime::Runtime::new().unwrap(); - - // let filename = "sample_settings.dps.sym.yaml"; - // let config_file = format!( - // "{}/../edgelet-docker/test/{}/{}", - // env!("CARGO_MANIFEST_DIR"), - // "linux", - // filename, - // ); - - // let mut check = runtime - // .block_on(Check::new( - // config_file.into(), - // "daemon.json".into(), // unused for this test - // "mcr.microsoft.com/azureiotedge-diagnostics:1.0.0".to_owned(), // unused for this test - // Default::default(), - // Some("1.0.0".to_owned()), // unused for this test - // "aziot-edged".into(), // unused for this test - // // None, - // "pool.ntp.org:123".to_owned(), // unused for this test - // super::OutputFormat::Text, // unused for this test - // false, - // false, - // )) - // .unwrap(); - - // match WellFormedConfig::default().execute(&mut check, &mut runtime) { - // CheckResult::Ok => (), - // check_result => panic!("parsing {} returned {:?}", filename, check_result), - // } - - // match WellFormedConnectionString::default().execute(&mut check, &mut runtime) { - // CheckResult::Failed(err) => assert!(err - // .to_string() - // .contains("Could not retrieve iothub_hostname from provisioning file.")), - // check_result => panic!( - // "checking connection string in {} returned {:?}", - // filename, check_result - // ), - // } - // } - - #[test] - #[cfg(windows)] - fn moby_runtime_uri_windows_wants_moby_based_on_runtime_uri() { - let mut runtime = tokio::runtime::Runtime::new().unwrap(); - - let filename = "sample_settings_notmoby.yaml"; - let config_file = format!( - "{}/../edgelet-docker/test/{}/{}", - env!("CARGO_MANIFEST_DIR"), - "linux", - filename, - ); - - let mut check = runtime - .block_on(Check::new( - config_file.into(), - "daemon.json".into(), // unused for this test - "mcr.microsoft.com/azureiotedge-diagnostics:1.0.0".to_owned(), // unused for this test - Default::default(), - Some("1.0.0".to_owned()), // unused for this test - "aziot-edged".into(), // unused for this test - None, // unused for this test - "pool.ntp.org:123".to_owned(), // unused for this test - super::OutputFormat::Text, // unused for this test - false, - false, - )) - .unwrap(); - - match WellFormedConfig::default().execute(&mut check, &mut runtime) { - CheckResult::Ok => (), - check_result => panic!("parsing {} returned {:?}", filename, check_result), - } - - // Pretend it's Moby even though named pipe indicates otherwise - check.docker_server_version = Some("19.03.12+azure".to_owned()); - - match ContainerEngineIsMoby::default().execute(&mut check, &mut runtime) { - CheckResult::Warning(warning) => assert!( - warning.to_string().contains( - "Device is not using a production-supported container engine (moby-engine)." - ), - "checking moby_runtime.uri in {} failed with an unexpected warning: {}", - filename, - warning - ), - - check_result => panic!( - "checking moby_runtime.uri in {} returned {:?}", - filename, check_result - ), - } - } - #[test] fn moby_runtime_uri_wants_moby_based_on_server_version() { let mut runtime = tokio::runtime::Runtime::new().unwrap(); @@ -1075,11 +806,9 @@ mod tests { "daemon.json".into(), // unused for this test "mcr.microsoft.com/azureiotedge-diagnostics:1.0.0".to_owned(), // unused for this test Default::default(), - Some("1.0.0".to_owned()), // unused for this test - "aziot-edged".into(), // unused for this test - // None, // unused for this test - "pool.ntp.org:123".to_owned(), // unused for this test - super::OutputFormat::Text, // unused for this test + Some("1.0.0".to_owned()), // unused for this test + "aziot-edged".into(), // unused for this test + super::OutputFormat::Text, // unused for this test false, false, )) diff --git a/edgelet/iotedge/src/check/stdout.rs b/edgelet/iotedge/src/check/stdout.rs index 457775d802b..db7e80e03da 100644 --- a/edgelet/iotedge/src/check/stdout.rs +++ b/edgelet/iotedge/src/check/stdout.rs @@ -21,46 +21,13 @@ impl Stdout { let stdout = termcolor::StandardStream::stdout(termcolor::ColorChoice::Auto); let mut success_color_spec = termcolor::ColorSpec::new(); - if cfg!(windows) { - // `Color::Green` maps to `FG_GREEN` which is too hard to read on the default blue-background profile that PS uses. - // PS uses `FG_GREEN | FG_INTENSITY` == 8 == `[ConsoleColor]::Green` as the foreground color for its error text, - // so mimic that. - success_color_spec.set_fg(Some(termcolor::Color::Rgb(0, 255, 0))); - } else { - success_color_spec.set_fg(Some(termcolor::Color::Green)); - } + success_color_spec.set_fg(Some(termcolor::Color::Green)); let mut warning_color_spec = termcolor::ColorSpec::new(); - if cfg!(windows) { - // `Color::Yellow` maps to `FOREGROUND_GREEN | FOREGROUND_RED` == 6 == `ConsoleColor::DarkYellow`. - // In its default blue-background profile, PS uses `ConsoleColor::DarkYellow` as its default foreground text color - // and maps it to a dark gray. - // - // So use explicit RGB to define yellow for Windows. Also use a black background to mimic PS warnings. - // - // Ref: - // - https://docs.rs/termcolor/0.3.6/src/termcolor/lib.rs.html#1380 defines `termcolor::Color::Yellow` as `wincolor::Color::Yellow` - // - https://docs.rs/wincolor/0.1.6/x86_64-pc-windows-msvc/src/wincolor/win.rs.html#18 - // defines `wincolor::Color::Yellow` as `FG_YELLOW`, which in turn is `FOREGROUND_GREEN | FOREGROUND_RED` - // - https://docs.microsoft.com/en-us/windows/console/char-info-str defines `FOREGROUND_GREEN | FOREGROUND_RED` as `2 | 4 == 6` - // - https://docs.microsoft.com/en-us/dotnet/api/system.consolecolor#fields defines `6` as `[ConsoleColor]::DarkYellow` - // - `$Host.UI.RawUI.ForegroundColor` in the default PS profile is `DarkYellow`, and writing in it prints dark gray text. - warning_color_spec.set_fg(Some(termcolor::Color::Rgb(255, 255, 0))); - warning_color_spec.set_bg(Some(termcolor::Color::Black)); - } else { - warning_color_spec.set_fg(Some(termcolor::Color::Yellow)); - } + warning_color_spec.set_fg(Some(termcolor::Color::Yellow)); let mut error_color_spec = termcolor::ColorSpec::new(); - if cfg!(windows) { - // `Color::Red` maps to `FG_RED` which is too hard to read on the default blue-background profile that PS uses. - // PS uses `FG_RED | FG_INTENSITY` == 12 == `[ConsoleColor]::Red` as the foreground color for its error text, - // with black background, so mimic that. - error_color_spec.set_fg(Some(termcolor::Color::Rgb(255, 0, 0))); - error_color_spec.set_bg(Some(termcolor::Color::Black)); - } else { - error_color_spec.set_fg(Some(termcolor::Color::Red)); - } + error_color_spec.set_fg(Some(termcolor::Color::Red)); Stdout::ColoredText { stdout, diff --git a/edgelet/iotedge/src/check/checks/host_connect_dps_endpoint.rs b/edgelet/iotedge/src/check/tls_handshake.rs similarity index 65% rename from edgelet/iotedge/src/check/checks/host_connect_dps_endpoint.rs rename to edgelet/iotedge/src/check/tls_handshake.rs index da5f00753e3..bee20fce49a 100644 --- a/edgelet/iotedge/src/check/checks/host_connect_dps_endpoint.rs +++ b/edgelet/iotedge/src/check/tls_handshake.rs @@ -10,79 +10,6 @@ use hyper_proxy::{Intercept, Proxy, ProxyConnector}; use hyper_tls::HttpsConnector; use typed_headers::Credentials; -use edgelet_core::{self, ProvisioningType, RuntimeSettings}; - -use crate::check::{checker::Checker, Check, CheckResult}; - -#[derive(serde_derive::Serialize, Default)] -pub(crate) struct HostConnectDpsEndpoint { - dps_endpoint: Option, - dps_hostname: Option, - proxy: Option, -} - -impl Checker for HostConnectDpsEndpoint { - fn id(&self) -> &'static str { - "host-connect-dps-endpoint" - } - fn description(&self) -> &'static str { - "host can connect to and perform TLS handshake with DPS endpoint" - } - fn execute(&mut self, check: &mut Check, runtime: &mut tokio::runtime::Runtime) -> CheckResult { - self.inner_execute(check, runtime) - .unwrap_or_else(CheckResult::Failed) - } - fn get_json(&self) -> serde_json::Value { - serde_json::to_value(self).unwrap() - } -} - -impl HostConnectDpsEndpoint { - fn inner_execute( - &mut self, - check: &mut Check, - runtime: &mut tokio::runtime::Runtime, - ) -> Result { - let settings = if let Some(settings) = &check.settings { - settings - } else { - return Ok(CheckResult::Skipped); - }; - - let dps_endpoint = - if let ProvisioningType::Dps(dps) = settings.provisioning().provisioning_type() { - dps.global_endpoint() - } else { - return Ok(CheckResult::Ignored); - }; - self.dps_endpoint = Some(format!("{}", dps_endpoint)); - - let dps_hostname = dps_endpoint.host_str().ok_or_else(|| { - Context::new("URL specified in provisioning.global_endpoint does not have a host") - })?; - self.dps_hostname = Some(dps_hostname.to_owned()); - - let proxy = settings - .agent() - .env() - .get("https_proxy") - .map(std::string::String::as_str); - self.proxy = proxy.map(std::borrow::ToOwned::to_owned); - - if let Some(proxy) = &self.proxy { - runtime.block_on(resolve_and_tls_handshake_proxy( - dps_endpoint.as_str().to_owned(), - dps_endpoint.port(), - proxy.clone(), - ))?; - } else { - resolve_and_tls_handshake(&dps_endpoint, dps_hostname, dps_hostname)?; - } - - Ok(CheckResult::Ok) - } -} - // Resolves the given `ToSocketAddrs`, then connects to the first address via TCP and completes a TLS handshake. // // `tls_hostname` is used for SNI validation and certificate hostname validation. diff --git a/edgelet/iotedge/src/main.rs b/edgelet/iotedge/src/main.rs index 23a3941fd74..314484d1470 100644 --- a/edgelet/iotedge/src/main.rs +++ b/edgelet/iotedge/src/main.rs @@ -6,7 +6,7 @@ use std::borrow::Cow; use std::io; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process; use clap::{crate_description, crate_name, App, AppSettings, Arg, SubCommand}; @@ -40,42 +40,11 @@ fn main() { #[allow(clippy::too_many_lines)] fn run() -> Result<(), Error> { - let (default_mgmt_uri, default_config_path, default_container_engine_config_path) = - if cfg!(windows) { - let program_data: PathBuf = std::env::var_os("PROGRAMDATA") - .map_or_else(|| r"C:\ProgramData".into(), Into::into); - - let default_mgmt_uri = program_data - .to_str() - .expect("PROGRAMDATA is not a utf-8 path") - .replace('\\', "/"); - let default_mgmt_uri = format!("unix:///{}/iotedge/mgmt/sock", default_mgmt_uri); - let default_mgmt_uri = Cow::Owned(default_mgmt_uri); - - let mut default_config_path = program_data.clone(); - default_config_path.push("iotedge"); - default_config_path.push("config.yaml"); - let default_config_path = Cow::Owned(default_config_path); - - let mut default_container_engine_config_path = program_data; - default_container_engine_config_path.push("iotedge-moby"); - default_container_engine_config_path.push("config"); - default_container_engine_config_path.push("daemon.json"); - let default_container_engine_config_path = - Cow::Owned(default_container_engine_config_path); - - ( - default_mgmt_uri, - default_config_path, - default_container_engine_config_path, - ) - } else { - ( - Cow::Borrowed("unix:///var/lib/aziot/edged/aziot-edged.mgmt.sock"), - Cow::Borrowed(Path::new("/etc/aziot/edged/config.yaml")), - Cow::Borrowed(Path::new("/etc/docker/daemon.json")), - ) - }; + let (default_mgmt_uri, default_config_path, default_container_engine_config_path) = ( + Cow::Borrowed("unix:///run/aziot/edged/aziot-edged.mgmt.sock"), + Cow::Borrowed(Path::new("/etc/aziot/edged/config.yaml")), + Cow::Borrowed(Path::new("/etc/docker/daemon.json")), + ); let default_mgmt_uri = option_env!("IOTEDGE_HOST").unwrap_or(&*default_mgmt_uri); @@ -335,10 +304,6 @@ fn run() -> Result<(), Error> { .expect("arg has a default value") .to_os_string() .into(), - // args.value_of("iothub-hostname").map(ToOwned::to_owned), - args.value_of("ntp-server") - .expect("arg has a default value") - .to_string(), args.value_of("output") .map(|arg| match arg { "json" => OutputFormat::Json, diff --git a/edgelet/iotedge/src/support_bundle.rs b/edgelet/iotedge/src/support_bundle.rs index a492646470a..1c4df6103bd 100644 --- a/edgelet/iotedge/src/support_bundle.rs +++ b/edgelet/iotedge/src/support_bundle.rs @@ -71,7 +71,7 @@ where let path = PathBuf::from(location); println!( "Created support bundle at {}", - path.canonicalize().unwrap_or_else(|_| path).display() + path.canonicalize().unwrap_or(path).display() ); Ok(()) diff --git a/edgelet/mini-sntp/Cargo.toml b/edgelet/mini-sntp/Cargo.toml deleted file mode 100644 index d29c2e07723..00000000000 --- a/edgelet/mini-sntp/Cargo.toml +++ /dev/null @@ -1,10 +0,0 @@ -[package] -name = "mini-sntp" -version = "0.1.0" -authors = ["Azure IoT Edge Devs"] -edition = "2018" -publish = false - -[dependencies] -chrono = "0.4" -failure = "0.1" diff --git a/edgelet/mini-sntp/src/error.rs b/edgelet/mini-sntp/src/error.rs deleted file mode 100644 index 063c453023b..00000000000 --- a/edgelet/mini-sntp/src/error.rs +++ /dev/null @@ -1,126 +0,0 @@ -#[derive(Debug)] -pub struct Error(failure::Context); - -impl Error { - pub fn kind(&self) -> &ErrorKind { - self.0.get_context() - } -} - -impl std::fmt::Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.0.fmt(f) - } -} - -impl failure::Fail for Error { - fn cause(&self) -> Option<&dyn failure::Fail> { - self.0.cause() - } - - fn backtrace(&self) -> Option<&failure::Backtrace> { - self.0.backtrace() - } -} - -impl From for Error { - fn from(kind: ErrorKind) -> Self { - Error(failure::Context::new(kind)) - } -} - -impl From> for Error { - fn from(context: failure::Context) -> Self { - Error(context) - } -} - -#[derive(Debug)] -pub enum ErrorKind { - BadServerResponse(BadServerResponseReason), - BindLocalSocket, - ReceiveServerResponse(std::io::Error), - ResolveNtpPoolHostname(Option), - SendClientRequest(std::io::Error), - SetReadTimeoutOnSocket, - SetWriteTimeoutOnSocket, -} - -impl std::fmt::Display for ErrorKind { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - ErrorKind::BadServerResponse(reason) => { - write!(f, "could not parse NTP server response: {}", reason) - } - ErrorKind::BindLocalSocket => write!(f, "could not bind local UDP socket"), - ErrorKind::ReceiveServerResponse(err) => { - write!(f, "could not receive NTP server response: {}", err) - } - ErrorKind::ResolveNtpPoolHostname(Some(err)) => { - write!(f, "could not resolve NTP pool hostname: {}", err) - } - ErrorKind::ResolveNtpPoolHostname(None) => { - write!(f, "could not resolve NTP pool hostname: no addresses found") - } - ErrorKind::SendClientRequest(err) => { - write!(f, "could not send SNTP client request: {}", err) - } - ErrorKind::SetReadTimeoutOnSocket => { - write!(f, "could not set read timeout on local UDP socket") - } - ErrorKind::SetWriteTimeoutOnSocket => { - write!(f, "could not set write timeout on local UDP socket") - } - } - } -} - -impl failure::Fail for ErrorKind { - fn cause(&self) -> Option<&dyn failure::Fail> { - #[allow(clippy::match_same_arms)] - match self { - ErrorKind::BadServerResponse(_) => None, - ErrorKind::BindLocalSocket => None, - ErrorKind::ReceiveServerResponse(err) => Some(err), - ErrorKind::ResolveNtpPoolHostname(Some(err)) => Some(err), - ErrorKind::ResolveNtpPoolHostname(None) => None, - ErrorKind::SendClientRequest(err) => Some(err), - ErrorKind::SetReadTimeoutOnSocket => None, - ErrorKind::SetWriteTimeoutOnSocket => None, - } - } -} - -#[derive(Clone, Copy, Debug)] -pub enum BadServerResponseReason { - LeapIndicator(u8), - OriginateTimestamp { - expected: chrono::DateTime, - actual: chrono::DateTime, - }, - Mode(u8), - VersionNumber(u8), -} - -impl std::fmt::Display for BadServerResponseReason { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - BadServerResponseReason::LeapIndicator(leap_indicator) => { - write!(f, "invalid value of leap indicator {}", leap_indicator) - } - BadServerResponseReason::OriginateTimestamp { expected, actual } => write!( - f, - "expected originate timestamp to be {} but it was {}", - expected, actual - ), - BadServerResponseReason::Mode(mode) => { - write!(f, "expected mode to be 4 but it was {}", mode) - } - BadServerResponseReason::VersionNumber(version_number) => write!( - f, - "expected version number to be 3 but it was {}", - version_number - ), - } - } -} diff --git a/edgelet/mini-sntp/src/lib.rs b/edgelet/mini-sntp/src/lib.rs deleted file mode 100644 index e5edee42f5f..00000000000 --- a/edgelet/mini-sntp/src/lib.rs +++ /dev/null @@ -1,349 +0,0 @@ -#![deny(rust_2018_idioms, warnings)] -#![deny(clippy::all, clippy::pedantic)] -#![allow( - clippy::doc_markdown, - clippy::missing_errors_doc, - clippy::module_name_repetitions, - clippy::must_use_candidate, - clippy::too_many_lines, - clippy::use_self -)] - -use std::net::{SocketAddr, ToSocketAddrs, UdpSocket}; - -use failure::ResultExt; - -mod error; -pub use error::{BadServerResponseReason, Error, ErrorKind}; - -/// The result of [`query`] -#[derive(Debug)] -pub struct SntpTimeQueryResult { - pub local_clock_offset: chrono::Duration, - pub round_trip_delay: chrono::Duration, -} - -/// Executes an SNTP query against the NTPv3 server at the given address. -/// -/// Ref: -pub fn query(addr: &A) -> Result -where - A: ToSocketAddrs, -{ - let addr = addr - .to_socket_addrs() - .map_err(|err| ErrorKind::ResolveNtpPoolHostname(Some(err)))? - .next() - .ok_or(ErrorKind::ResolveNtpPoolHostname(None))?; - - let socket = UdpSocket::bind("0.0.0.0:0").context(ErrorKind::BindLocalSocket)?; - socket - .set_read_timeout(Some(std::time::Duration::from_secs(10))) - .context(ErrorKind::SetReadTimeoutOnSocket)?; - socket - .set_write_timeout(Some(std::time::Duration::from_secs(10))) - .context(ErrorKind::SetWriteTimeoutOnSocket)?; - - let mut num_retries_remaining = 3; - loop { - match query_inner(&socket, addr) { - Ok(result) => return Ok(result), - Err(err) => { - let is_retriable = match err.kind() { - ErrorKind::SendClientRequest(err) | ErrorKind::ReceiveServerResponse(err) => { - err.kind() == std::io::ErrorKind::TimedOut || // Windows - err.kind() == std::io::ErrorKind::WouldBlock // Unix - } - - _ => false, - }; - if is_retriable { - num_retries_remaining -= 1; - if num_retries_remaining == 0 { - return Err(err); - } - } else { - return Err(err); - } - } - } - } -} - -fn query_inner(socket: &UdpSocket, addr: SocketAddr) -> Result { - let request_transmit_timestamp = { - let (buf, request_transmit_timestamp) = create_client_request(); - - #[cfg(test)] - std::thread::sleep(std::time::Duration::from_secs(5)); // simulate network delay - - let mut buf = &buf[..]; - while !buf.is_empty() { - let sent = socket - .send_to(buf, addr) - .map_err(ErrorKind::SendClientRequest)?; - buf = &buf[sent..]; - } - - request_transmit_timestamp - }; - - let result = { - let mut buf = [0_u8; 48]; - - { - let mut buf = &mut buf[..]; - while !buf.is_empty() { - let (received, received_from) = socket - .recv_from(buf) - .map_err(ErrorKind::ReceiveServerResponse)?; - if received_from == addr { - buf = &mut buf[received..]; - } - } - } - - #[cfg(test)] - std::thread::sleep(std::time::Duration::from_secs(5)); // simulate network delay - - parse_server_response(buf, request_transmit_timestamp)? - }; - - Ok(result) -} - -fn create_client_request() -> ([u8; 48], chrono::DateTime) { - let sntp_epoch = sntp_epoch(); - - let mut buf = [0_u8; 48]; - buf[0] = 0b00_011_011; // version_number: 3, mode: 3 (client) - - let transmit_timestamp = chrono::Utc::now(); - - #[cfg(test)] - let transmit_timestamp = transmit_timestamp - chrono::Duration::seconds(30); // simulate unsynced local clock - - let mut duration_since_sntp_epoch = transmit_timestamp - sntp_epoch; - - let integral_part = duration_since_sntp_epoch.num_seconds(); - duration_since_sntp_epoch = - duration_since_sntp_epoch - chrono::Duration::seconds(integral_part); - - assert!(integral_part >= 0 && integral_part < i64::from(u32::max_value())); - #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] - let integral_part = (integral_part as u32).to_be_bytes(); - buf[40..44].copy_from_slice(&integral_part[..]); - - let fractional_part = duration_since_sntp_epoch - .num_nanoseconds() - .expect("can't overflow nanoseconds"); - let fractional_part = (fractional_part << 32) / 1_000_000_000; - assert!(fractional_part >= 0 && fractional_part < i64::from(u32::max_value())); - #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] - let fractional_part = (fractional_part as u32).to_be_bytes(); - buf[44..48].copy_from_slice(&fractional_part[..]); - - let packet = Packet::parse(buf, sntp_epoch); - #[cfg(test)] - let packet = dbg!(packet); - - // Re-extract transmit timestamp from the packet. This may not be the same as the original `transmit_timestamp` - // that was serialized into the packet due to rounding. Specifically, it's usually off by 1ns. - let transmit_timestamp = packet.transmit_timestamp; - - (buf, transmit_timestamp) -} - -fn parse_server_response( - buf: [u8; 48], - request_transmit_timestamp: chrono::DateTime, -) -> Result { - let sntp_epoch = sntp_epoch(); - - let destination_timestamp = chrono::Utc::now(); - - #[cfg(test)] - let destination_timestamp = destination_timestamp - chrono::Duration::seconds(30); // simulate unsynced local clock - - let packet = Packet::parse(buf, sntp_epoch); - #[cfg(test)] - let packet = dbg!(packet); - - match packet.leap_indicator { - 0..=2 => (), - leap_indicator => { - return Err( - ErrorKind::BadServerResponse(BadServerResponseReason::LeapIndicator( - leap_indicator, - )) - .into(), - ); - } - }; - - // RFC 2030 says: - // - // >Version 4 servers are required to - // >reply in the same version as the request, so the VN field of the - // >request also specifies the version of the reply. - // - // But at least one pool.ntp.org server does not respect this and responds with VN=4 - // even though our client requests have VN=3. - // - // So allow both VN=3 and VN=4 in the server response. The response body format is identical for both anyway. - if packet.version_number != 3 && packet.version_number != 4 { - return Err( - ErrorKind::BadServerResponse(BadServerResponseReason::VersionNumber( - packet.version_number, - )) - .into(), - ); - } - - if packet.mode != 4 { - return Err( - ErrorKind::BadServerResponse(BadServerResponseReason::Mode(packet.mode)).into(), - ); - } - - if packet.originate_timestamp != request_transmit_timestamp { - return Err( - ErrorKind::BadServerResponse(BadServerResponseReason::OriginateTimestamp { - expected: request_transmit_timestamp, - actual: packet.originate_timestamp, - }) - .into(), - ); - } - - Ok(SntpTimeQueryResult { - local_clock_offset: ((packet.receive_timestamp - request_transmit_timestamp) - + (packet.transmit_timestamp - destination_timestamp)) - / 2, - - round_trip_delay: (destination_timestamp - request_transmit_timestamp) - - (packet.receive_timestamp - packet.transmit_timestamp), - }) -} - -fn sntp_epoch() -> chrono::DateTime { - chrono::DateTime::::from_utc( - chrono::NaiveDate::from_ymd(1900, 1, 1).and_time(chrono::NaiveTime::from_hms(0, 0, 0)), - chrono::Utc, - ) -} - -#[derive(Debug)] -struct Packet { - leap_indicator: u8, - version_number: u8, - mode: u8, - stratum: u8, - poll_interval: u8, - precision: u8, - root_delay: u32, - root_dispersion: u32, - reference_identifier: u32, - reference_timestamp: chrono::DateTime, - originate_timestamp: chrono::DateTime, - receive_timestamp: chrono::DateTime, - transmit_timestamp: chrono::DateTime, -} - -impl Packet { - fn parse(buf: [u8; 48], sntp_epoch: chrono::DateTime) -> Self { - let leap_indicator = (buf[0] & 0b11_000_000) >> 6; - let version_number = (buf[0] & 0b00_111_000) >> 3; - let mode = buf[0] & 0b00_000_111; - let stratum = buf[1]; - let poll_interval = buf[2]; - let precision = buf[3]; - let root_delay = u32::from_be_bytes([buf[4], buf[5], buf[6], buf[7]]); - let root_dispersion = u32::from_be_bytes([buf[8], buf[9], buf[10], buf[11]]); - let reference_identifier = u32::from_be_bytes([buf[12], buf[13], buf[14], buf[15]]); - let reference_timestamp = deserialize_timestamp( - [ - buf[16], buf[17], buf[18], buf[19], buf[20], buf[21], buf[22], buf[23], - ], - sntp_epoch, - ); - let originate_timestamp = deserialize_timestamp( - [ - buf[24], buf[25], buf[26], buf[27], buf[28], buf[29], buf[30], buf[31], - ], - sntp_epoch, - ); - let receive_timestamp = deserialize_timestamp( - [ - buf[32], buf[33], buf[34], buf[35], buf[36], buf[37], buf[38], buf[39], - ], - sntp_epoch, - ); - let transmit_timestamp = deserialize_timestamp( - [ - buf[40], buf[41], buf[42], buf[43], buf[44], buf[45], buf[46], buf[47], - ], - sntp_epoch, - ); - - Packet { - leap_indicator, - version_number, - mode, - stratum, - poll_interval, - precision, - root_delay, - root_dispersion, - reference_identifier, - reference_timestamp, - originate_timestamp, - receive_timestamp, - transmit_timestamp, - } - } -} - -fn deserialize_timestamp( - raw: [u8; 8], - sntp_epoch: chrono::DateTime, -) -> chrono::DateTime { - let integral_part = i64::from(u32::from_be_bytes([raw[0], raw[1], raw[2], raw[3]])); - let fractional_part = i64::from(u32::from_be_bytes([raw[4], raw[5], raw[6], raw[7]])); - let duration_since_sntp_epoch = chrono::Duration::nanoseconds( - integral_part * 1_000_000_000 + ((fractional_part * 1_000_000_000) >> 32), - ); - - sntp_epoch + duration_since_sntp_epoch -} - -#[cfg(test)] -mod tests { - use super::{query, Error, SntpTimeQueryResult}; - - #[test] - fn it_works() -> Result<(), Error> { - let SntpTimeQueryResult { - local_clock_offset, - round_trip_delay, - } = query(&("pool.ntp.org", 123))?; - - println!("local clock offset: {}", local_clock_offset); - println!("round-trip delay: {}", round_trip_delay); - - assert!( - (local_clock_offset - chrono::Duration::seconds(30)) - .num_seconds() - .abs() - < 1 - ); - assert!( - (round_trip_delay - chrono::Duration::seconds(10)) - .num_seconds() - .abs() - < 1 - ); - - Ok(()) - } -} From 228c7a8e4a78fa476b482bb8b80a7433a2d1f4d6 Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Mon, 14 Dec 2020 13:32:10 -0500 Subject: [PATCH 02/15] remove host_connect_upstream check --- .../src/check/checks/host_connect_upstream.rs | 115 ------------------ edgelet/iotedge/src/check/checks/mod.rs | 2 - edgelet/iotedge/src/check/mod.rs | 11 +- edgelet/iotedge/src/check/tls_handshake.rs | 115 ------------------ 4 files changed, 4 insertions(+), 239 deletions(-) delete mode 100644 edgelet/iotedge/src/check/checks/host_connect_upstream.rs delete mode 100644 edgelet/iotedge/src/check/tls_handshake.rs diff --git a/edgelet/iotedge/src/check/checks/host_connect_upstream.rs b/edgelet/iotedge/src/check/checks/host_connect_upstream.rs deleted file mode 100644 index 783014c3b03..00000000000 --- a/edgelet/iotedge/src/check/checks/host_connect_upstream.rs +++ /dev/null @@ -1,115 +0,0 @@ -use edgelet_core::RuntimeSettings; - -use crate::check::{ - tls_handshake::{resolve_and_tls_handshake, resolve_and_tls_handshake_proxy}, - upstream_protocol_port::UpstreamProtocolPort, - Check, CheckResult, Checker, -}; - -pub(crate) fn get_host_connect_upstream_tests() -> Vec> { - vec![ - make_check( - "host-connect-upstream-amqp", - "host can connect to and perform TLS handshake with upstream AMQP port", - UpstreamProtocolPort::Amqp, - ), - make_check( - "host-connect-upstream-https", - "host can connect to and perform TLS handshake with upstream HTTPS / WebSockets port", - UpstreamProtocolPort::Https, - ), - make_check( - "host-connect-upstream-mqtt", - "host can connect to and perform TLS handshake with upstream MQTT port", - UpstreamProtocolPort::Mqtt, - ), - ] -} - -#[derive(serde_derive::Serialize)] -pub(crate) struct HostConnectUpstream { - port_number: u16, - upstream_hostname: Option, - proxy: Option, - #[serde(skip)] - id: &'static str, - #[serde(skip)] - description: &'static str, -} - -impl Checker for HostConnectUpstream { - fn id(&self) -> &'static str { - self.id - } - fn description(&self) -> &'static str { - self.description - } - fn execute(&mut self, check: &mut Check, runtime: &mut tokio::runtime::Runtime) -> CheckResult { - self.inner_execute(check, runtime) - .unwrap_or_else(CheckResult::Failed) - } - fn get_json(&self) -> serde_json::Value { - serde_json::to_value(self).unwrap() - } -} - -impl HostConnectUpstream { - fn inner_execute( - &mut self, - check: &mut Check, - runtime: &mut tokio::runtime::Runtime, - ) -> Result { - let settings = if let Some(settings) = &check.settings { - settings - } else { - return Ok(CheckResult::Skipped); - }; - - let parent_hostname: String; - let upstream_hostname = if let Some(upstream_hostname) = settings.parent_hostname() { - parent_hostname = upstream_hostname.to_string(); - &parent_hostname - } else if let Some(iothub_hostname) = &check.iothub_hostname { - iothub_hostname - } else { - return Ok(CheckResult::Skipped); - }; - - self.upstream_hostname = Some(upstream_hostname.clone()); - - self.proxy = check - .settings - .as_ref() - .and_then(|settings| settings.agent().env().get("https_proxy").cloned()); - - if let Some(proxy) = &self.proxy { - runtime.block_on(resolve_and_tls_handshake_proxy( - upstream_hostname.clone(), - Some(self.port_number), - proxy.clone(), - ))?; - } else { - resolve_and_tls_handshake( - &(&**upstream_hostname, self.port_number), - upstream_hostname, - &format!("{}:{}", upstream_hostname, self.port_number), - )?; - } - - Ok(CheckResult::Ok) - } -} - -fn make_check( - id: &'static str, - description: &'static str, - upstream_protocol_port: UpstreamProtocolPort, -) -> Box { - Box::new(HostConnectUpstream { - id, - description, - port_number: upstream_protocol_port.as_port(), - upstream_hostname: None, - proxy: None, - }) -} diff --git a/edgelet/iotedge/src/check/checks/mod.rs b/edgelet/iotedge/src/check/checks/mod.rs index 8d42eaef32e..172c5e5823b 100644 --- a/edgelet/iotedge/src/check/checks/mod.rs +++ b/edgelet/iotedge/src/check/checks/mod.rs @@ -8,7 +8,6 @@ mod container_engine_is_moby; mod container_engine_logrotate; mod container_local_time; mod container_resolve_parent_hostname; -mod host_connect_upstream; mod hostname; mod parent_hostname; mod pull_agent_from_upstream; @@ -25,7 +24,6 @@ pub(crate) use self::container_engine_is_moby::ContainerEngineIsMoby; pub(crate) use self::container_engine_logrotate::ContainerEngineLogrotate; pub(crate) use self::container_local_time::ContainerLocalTime; pub(crate) use self::container_resolve_parent_hostname::ContainerResolveParentHostname; -pub(crate) use self::host_connect_upstream::get_host_connect_upstream_tests; pub(crate) use self::hostname::Hostname; pub(crate) use self::parent_hostname::ParentHostname; pub(crate) use self::pull_agent_from_upstream::PullAgentFromUpstream; diff --git a/edgelet/iotedge/src/check/mod.rs b/edgelet/iotedge/src/check/mod.rs index 4459ec826b9..915c27fe880 100644 --- a/edgelet/iotedge/src/check/mod.rs +++ b/edgelet/iotedge/src/check/mod.rs @@ -22,7 +22,6 @@ mod stdout; use self::stdout::Stdout; mod hostname_checks_common; -mod tls_handshake; mod upstream_protocol_port; mod checker; @@ -30,11 +29,10 @@ use checker::Checker; mod checks; use checks::{ - get_host_connect_upstream_tests, get_host_container_upstream_tests, AziotEdgedVersion, - ConnectManagementUri, ContainerEngineDns, ContainerEngineIPv6, ContainerEngineInstalled, - ContainerEngineIsMoby, ContainerEngineLogrotate, ContainerLocalTime, - ContainerResolveParentHostname, EdgeAgentStorageMounted, EdgeHubStorageMounted, Hostname, - ParentHostname, PullAgentFromUpstream, WellFormedConfig, + get_host_container_upstream_tests, AziotEdgedVersion, ConnectManagementUri, ContainerEngineDns, + ContainerEngineIPv6, ContainerEngineInstalled, ContainerEngineIsMoby, ContainerEngineLogrotate, + ContainerLocalTime, ContainerResolveParentHostname, EdgeAgentStorageMounted, + EdgeHubStorageMounted, Hostname, ParentHostname, PullAgentFromUpstream, WellFormedConfig, }; pub struct Check { @@ -239,7 +237,6 @@ impl Check { ), ("Connectivity checks", { let mut tests: Vec> = Vec::new(); - tests.extend(get_host_connect_upstream_tests()); tests.extend(get_host_container_upstream_tests()); tests }), diff --git a/edgelet/iotedge/src/check/tls_handshake.rs b/edgelet/iotedge/src/check/tls_handshake.rs deleted file mode 100644 index bee20fce49a..00000000000 --- a/edgelet/iotedge/src/check/tls_handshake.rs +++ /dev/null @@ -1,115 +0,0 @@ -use std::convert::TryInto; -use std::net::TcpStream; -use std::str::FromStr; - -use failure::{format_err, Context, Error, ResultExt}; -use futures::Future; -use hyper::client::connect::{Connect, Destination}; -use hyper::Uri; -use hyper_proxy::{Intercept, Proxy, ProxyConnector}; -use hyper_tls::HttpsConnector; -use typed_headers::Credentials; - -// Resolves the given `ToSocketAddrs`, then connects to the first address via TCP and completes a TLS handshake. -// -// `tls_hostname` is used for SNI validation and certificate hostname validation. -// -// `hostname_display` is used for the error messages. -pub fn resolve_and_tls_handshake( - to_socket_addrs: &impl std::net::ToSocketAddrs, - tls_hostname: &str, - hostname_display: &str, -) -> Result<(), Error> { - let host_addr = to_socket_addrs - .to_socket_addrs() - .with_context(|_| { - format!( - "Could not connect to {} : could not resolve hostname", - hostname_display, - ) - })? - .next() - .ok_or_else(|| { - Context::new(format!( - "Could not connect to {} : could not resolve hostname: no addresses found", - hostname_display, - )) - })?; - - let stream = TcpStream::connect_timeout(&host_addr, std::time::Duration::from_secs(10)) - .with_context(|_| format!("Could not connect to {}", hostname_display))?; - - let tls_connector = native_tls::TlsConnector::new().with_context(|_| { - format!( - "Could not connect to {} : could not create TLS connector", - hostname_display, - ) - })?; - let _ = tls_connector - .connect(tls_hostname, stream) - .with_context(|_| { - format!( - "Could not connect to {} : could not complete TLS handshake", - hostname_display, - ) - })?; - - Ok(()) -} - -pub fn resolve_and_tls_handshake_proxy( - tls_hostname: String, - port: Option, - proxy: String, -) -> impl Future { - futures::future::ok(()) - .and_then({ - move |_| -> Result<_, Error> { - let proxy_uri = Uri::from_str(&proxy) - .with_context(|_| format!("Could not make proxi uri from {}", proxy))?; - - let credentials = proxy_uri - .authority_part() - .and_then(|authority| { - if let [userpass, _] = - &authority.as_str().split('@').collect::>()[..] - { - if let [username, password] = - &userpass.split(':').collect::>()[..] - { - return Some(Credentials::basic(username, password)); - } - } - None - }) - .transpose() - .with_context(|e| format!("Could not parse credentuals. Reason: {}", e))?; - - let mut proxy_obj = Proxy::new(Intercept::All, proxy_uri); - if let Some(credentials) = credentials { - proxy_obj.set_authorization(credentials); - } - - let http_connector = HttpsConnector::new(1) - .with_context(|_| "Could not make https connector".to_owned())?; - let proxy_connector = ProxyConnector::from_proxy(http_connector, proxy_obj) - .with_context(|_| "Could not make proxy connector".to_owned())?; - - let port = port.map(|p| format!(":{}", p)).unwrap_or_default(); - let hostname = Uri::from_str(&format!("https://{}{}", tls_hostname, port)) - .with_context(|_| { - format!("Could not make uri from {}{}", tls_hostname, port) - })?; - - let hostname: Result = hostname.try_into(); - let hostname = hostname.with_context(|_| "Could not make hostname uri")?; - Ok((proxy_connector, hostname)) - } - }) - .and_then(move |(proxy_connector, hostname)| { - proxy_connector - .connect(hostname) - .map_err(|e| format_err!("Could not connect: {}", e)) - }) - .map(drop) -} From 8f413ebeac0ee5a11a5750d3ce7f3bfdbbe06bfb Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Tue, 15 Dec 2020 13:33:15 -0500 Subject: [PATCH 03/15] move checks list into checks module --- edgelet/iotedge/src/check/checks/mod.rs | 33 +++++++++++++++++ edgelet/iotedge/src/check/mod.rs | 48 ++++--------------------- 2 files changed, 40 insertions(+), 41 deletions(-) diff --git a/edgelet/iotedge/src/check/checks/mod.rs b/edgelet/iotedge/src/check/checks/mod.rs index 172c5e5823b..30d2f7ec562 100644 --- a/edgelet/iotedge/src/check/checks/mod.rs +++ b/edgelet/iotedge/src/check/checks/mod.rs @@ -35,6 +35,8 @@ use std::process::Command; use failure::{self, Context, Fail}; +use super::Checker; + pub(crate) fn docker( docker_host_arg: &str, args: I, @@ -68,3 +70,34 @@ where Ok(output.stdout) } + +pub(crate) fn all_checks() -> [(&'static str, Vec>); 2] { + /* Note: keep ordering consistent. Later tests may depend on earlier tests. */ + [ + ( + "Configuration checks", + vec![ + Box::new(WellFormedConfig::default()), + Box::new(ContainerEngineInstalled::default()), + Box::new(Hostname::default()), + Box::new(ParentHostname::default()), + Box::new(ContainerResolveParentHostname::default()), + Box::new(ConnectManagementUri::default()), + Box::new(AziotEdgedVersion::default()), + Box::new(ContainerLocalTime::default()), + Box::new(ContainerEngineDns::default()), + Box::new(ContainerEngineIPv6::default()), + Box::new(ContainerEngineIsMoby::default()), + Box::new(ContainerEngineLogrotate::default()), + Box::new(EdgeAgentStorageMounted::default()), + Box::new(EdgeHubStorageMounted::default()), + Box::new(PullAgentFromUpstream::default()), + ], + ), + ("Connectivity checks", { + let mut tests: Vec> = Vec::new(); + tests.extend(get_host_container_upstream_tests()); + tests + }), + ] +} diff --git a/edgelet/iotedge/src/check/mod.rs b/edgelet/iotedge/src/check/mod.rs index 915c27fe880..f40a6ee8788 100644 --- a/edgelet/iotedge/src/check/mod.rs +++ b/edgelet/iotedge/src/check/mod.rs @@ -28,12 +28,6 @@ mod checker; use checker::Checker; mod checks; -use checks::{ - get_host_container_upstream_tests, AziotEdgedVersion, ConnectManagementUri, ContainerEngineDns, - ContainerEngineIPv6, ContainerEngineInstalled, ContainerEngineIsMoby, ContainerEngineLogrotate, - ContainerLocalTime, ContainerResolveParentHostname, EdgeAgentStorageMounted, - EdgeHubStorageMounted, Hostname, ParentHostname, PullAgentFromUpstream, WellFormedConfig, -}; pub struct Check { config_file: PathBuf, @@ -212,39 +206,8 @@ impl Check { })) } - fn checks() -> [(&'static str, Vec>); 2] { - /* Note: keep ordering consistent. Later tests may depend on earlier tests. */ - [ - ( - "Configuration checks", - vec![ - Box::new(WellFormedConfig::default()), - Box::new(ContainerEngineInstalled::default()), - Box::new(Hostname::default()), - Box::new(ParentHostname::default()), - Box::new(ContainerResolveParentHostname::default()), - Box::new(ConnectManagementUri::default()), - Box::new(AziotEdgedVersion::default()), - Box::new(ContainerLocalTime::default()), - Box::new(ContainerEngineDns::default()), - Box::new(ContainerEngineIPv6::default()), - Box::new(ContainerEngineIsMoby::default()), - Box::new(ContainerEngineLogrotate::default()), - Box::new(EdgeAgentStorageMounted::default()), - Box::new(EdgeHubStorageMounted::default()), - Box::new(PullAgentFromUpstream::default()), - ], - ), - ("Connectivity checks", { - let mut tests: Vec> = Vec::new(); - tests.extend(get_host_container_upstream_tests()); - tests - }), - ] - } - pub fn possible_ids() -> impl Iterator { - let result: Vec<&'static str> = Check::checks() + let result: Vec<&'static str> = checks::all_checks() .iter() .flat_map(|(_, section_checks)| section_checks) .map(|check| check.id()) @@ -255,7 +218,7 @@ impl Check { pub fn print_list() -> Result<(), Error> { // All our text is ASCII, so we can measure text width in bytes rather than using unicode-segmentation to count graphemes. - let checks = Check::checks(); + let checks = checks::all_checks(); let widest_section_name_len = checks .iter() .map(|(section_name, _)| section_name.len()) @@ -299,7 +262,7 @@ impl Check { pub fn execute(&mut self, runtime: &mut tokio::runtime::Runtime) -> Result<(), Error> { let mut checks: BTreeMap<&str, CheckOutputSerializable> = Default::default(); - let mut check_data = Check::checks(); + let mut check_data = checks::all_checks(); let mut stdout = Stdout::new(self.output_format); @@ -608,7 +571,10 @@ struct CheckOutputSerializable { #[cfg(test)] mod tests { - use super::{Check, CheckResult, Checker, ContainerEngineIsMoby, Hostname, WellFormedConfig}; + use super::{ + checks::{ContainerEngineIsMoby, Hostname, WellFormedConfig}, + Check, CheckResult, Checker, + }; #[test] fn config_file_checks_ok() { From afe8faa477d6a3684168f0cb3450b912ddc2a0e5 Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Tue, 15 Dec 2020 18:30:15 -0500 Subject: [PATCH 04/15] shell out to 'aziot check-list' --- edgelet/iotedge/src/check/checks/mod.rs | 3 +- edgelet/iotedge/src/check/mod.rs | 88 ++++++++++++++++++++----- edgelet/iotedge/src/error.rs | 3 + edgelet/iotedge/src/main.rs | 20 ++++-- 4 files changed, 90 insertions(+), 24 deletions(-) diff --git a/edgelet/iotedge/src/check/checks/mod.rs b/edgelet/iotedge/src/check/checks/mod.rs index 30d2f7ec562..af549e579d4 100644 --- a/edgelet/iotedge/src/check/checks/mod.rs +++ b/edgelet/iotedge/src/check/checks/mod.rs @@ -71,7 +71,8 @@ where Ok(output.stdout) } -pub(crate) fn all_checks() -> [(&'static str, Vec>); 2] { +// built-in checks, as opposed to those that are deferred to `aziot check` +pub(crate) fn built_in_checks() -> [(&'static str, Vec>); 2] { /* Note: keep ordering consistent. Later tests may depend on earlier tests. */ [ ( diff --git a/edgelet/iotedge/src/check/mod.rs b/edgelet/iotedge/src/check/mod.rs index f40a6ee8788..14c16ad40b1 100644 --- a/edgelet/iotedge/src/check/mod.rs +++ b/edgelet/iotedge/src/check/mod.rs @@ -206,29 +206,81 @@ impl Check { })) } - pub fn possible_ids() -> impl Iterator { - let result: Vec<&'static str> = checks::all_checks() - .iter() - .flat_map(|(_, section_checks)| section_checks) - .map(|check| check.id()) - .collect(); + pub fn print_list(aziot_bin: std::ffi::OsString) -> Result<(), Error> { + // DEVNOTE: keep in sync with aziot CheckerMeta + #[derive(serde::Deserialize)] + struct CheckerMeta { + id: String, + description: String, + } - result.into_iter() - } + let mut all_checks: Vec<(String, Vec)> = Vec::new(); + + // get all the aziot checks by shelling-out to aziot + { + let aziot_check_out = std::process::Command::new(aziot_bin) + .arg("check-list") + .arg("-j") + .output(); + + match aziot_check_out { + Err(_) => { + // not being able to access aziot is bad... but we shouldn't fail here, + // as the user may still want to run other iotedge specific checks. + // + // to make sure the user knows that there should me more checks, we add + // this "dummy" entry instead. + all_checks.push(( + "(aziot)".into(), + vec![CheckerMeta { + id: "(aziot-error)".into(), + description: "(aziot checks unavailable - could not communicate with 'aziot' binary)".into(), + }] + )); + } + Ok(out) => { + let aziot_checks: BTreeMap> = + serde_json::from_slice(&out.stdout).context(ErrorKind::Aziot)?; + + all_checks.extend( + aziot_checks + .into_iter() + .map(|(section_name, checks)| (section_name + " (aziot)", checks)), + ); + } + } + } + + // get all the built-in checks + { + let built_in_checks = checks::built_in_checks(); + let checks = built_in_checks.iter().map(|(section_name, checks)| { + ( + (*section_name).to_string(), + checks + .iter() + .map(|c| CheckerMeta { + id: c.id().into(), + description: c.description().into(), + }) + .collect::>(), + ) + }); + + all_checks.extend(checks); + } - pub fn print_list() -> Result<(), Error> { // All our text is ASCII, so we can measure text width in bytes rather than using unicode-segmentation to count graphemes. - let checks = checks::all_checks(); - let widest_section_name_len = checks + let widest_section_name_len = all_checks .iter() .map(|(section_name, _)| section_name.len()) .max() .expect("Have at least one section"); let section_name_column_width = widest_section_name_len + 1; - let widest_check_id_len = checks + let widest_check_id_len = all_checks .iter() .flat_map(|(_, section_checks)| section_checks) - .map(|check| check.id().len()) + .map(|check_meta| check_meta.id.len()) .max() .expect("Have at least one check"); let check_id_column_width = widest_check_id_len + 1; @@ -242,13 +294,13 @@ impl Check { ); println!(); - for (section_name, section_checks) in &checks { - for check in section_checks { + for (section_name, section_checks) in all_checks { + for check_meta in section_checks { println!( "{:section_name_column_width$}{:check_id_column_width$}{}", section_name, - check.id(), - check.description(), + check_meta.id, + check_meta.description, section_name_column_width = section_name_column_width, check_id_column_width = check_id_column_width, ); @@ -262,7 +314,7 @@ impl Check { pub fn execute(&mut self, runtime: &mut tokio::runtime::Runtime) -> Result<(), Error> { let mut checks: BTreeMap<&str, CheckOutputSerializable> = Default::default(); - let mut check_data = checks::all_checks(); + let mut check_data = checks::built_in_checks(); let mut stdout = Stdout::new(self.output_format); diff --git a/edgelet/iotedge/src/error.rs b/edgelet/iotedge/src/error.rs index 8645fcc4e23..441fe43d58a 100644 --- a/edgelet/iotedge/src/error.rs +++ b/edgelet/iotedge/src/error.rs @@ -53,6 +53,9 @@ pub enum ErrorKind { #[fail(display = "Unable to call docker inspect")] Docker, + + #[fail(display = "Error communicating with 'aziot' binary")] + Aziot, } impl Fail for Error { diff --git a/edgelet/iotedge/src/main.rs b/edgelet/iotedge/src/main.rs index 314484d1470..8e71093d8a9 100644 --- a/edgelet/iotedge/src/main.rs +++ b/edgelet/iotedge/src/main.rs @@ -53,9 +53,6 @@ fn run() -> Result<(), Error> { edgelet_core::version().replace("~", "-") ); - let mut possible_check_id_values: Vec<_> = Check::possible_ids().collect(); - possible_check_id_values.sort_unstable(); - let matches = App::new(crate_name!()) .version(edgelet_core::version_with_source_version()) .about(crate_description!()) @@ -71,6 +68,16 @@ fn run() -> Result<(), Error> { .env("IOTEDGE_HOST") .default_value(default_mgmt_uri), ) + .arg( + Arg::with_name("aziot-bin") + .help("path to 'aziot' binary") + .long("aziot-bin") + .takes_value(true) + .value_name("PATH") + .global(true) + .env("AZIOT_BIN") + .default_value("aziot"), + ) .subcommand( SubCommand::with_name("check") .about("Check for common config and deployment issues") @@ -106,7 +113,6 @@ fn run() -> Result<(), Error> { .help("Space-separated list of check IDs. The checks listed here will not be run. See 'iotedge check-list' for details of all checks.\n") .multiple(true) .takes_value(true) - .possible_values(&possible_check_id_values), ) .arg( Arg::with_name("expected-aziot-edged-version") @@ -317,7 +323,11 @@ fn run() -> Result<(), Error> { tokio_runtime.block_on(check)?.execute(&mut tokio_runtime) } - ("check-list", _) => Check::print_list(), + ("check-list", Some(args)) => Check::print_list( + args.value_of_os("aziot-bin") + .expect("arg has a default value") + .to_os_string(), + ), ("list", _) => tokio_runtime.block_on(List::new(runtime()?, io::stdout()).execute()), ("restart", Some(args)) => tokio_runtime.block_on( Restart::new( From 7bbc8da12c61ccd699cbc746f814b9f601201c79 Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Tue, 22 Dec 2020 16:47:32 -0500 Subject: [PATCH 05/15] first pass at aziot integration it works, but it could really do with some cleanup. notably, it currently duplicates all the wire-protocol types from `aziot`, whereas it really aught to use a shared library to keep things consistent. --- .../src/check/checks/container_engine_ipv6.rs | 6 +- .../src/check/checks/well_formed_config.rs | 12 +- edgelet/iotedge/src/check/mod.rs | 443 ++++++++++++------ edgelet/iotedge/src/main.rs | 3 + 4 files changed, 310 insertions(+), 154 deletions(-) diff --git a/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs b/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs index 8d3bf91839f..93921b5f845 100644 --- a/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs +++ b/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs @@ -55,10 +55,10 @@ impl ContainerEngineIPv6 { let daemon_config_file = match daemon_config_file { Ok(daemon_config_file) => daemon_config_file, Err(err) => { - if is_edge_ipv6_configured { - return Err(err.context(MESSAGE).into()); + return if is_edge_ipv6_configured { + Err(err.context(MESSAGE).into()) } else { - return Ok(CheckResult::Ignored); + Ok(CheckResult::Ignored) } } }; diff --git a/edgelet/iotedge/src/check/checks/well_formed_config.rs b/edgelet/iotedge/src/check/checks/well_formed_config.rs index 756759b5b32..ded21144831 100644 --- a/edgelet/iotedge/src/check/checks/well_formed_config.rs +++ b/edgelet/iotedge/src/check/checks/well_formed_config.rs @@ -33,19 +33,19 @@ impl WellFormedConfig { // // So we first try to open the file for reading ourselves. if let Err(err) = File::open(config_file) { - if err.kind() == std::io::ErrorKind::PermissionDenied { - return Ok(CheckResult::Fatal( + return if err.kind() == std::io::ErrorKind::PermissionDenied { + Ok(CheckResult::Fatal( err.context(format!( "Could not open file {}. You might need to run this command as root.", config_file.display(), )) .into(), - )); + )) } else { - return Err(err + Err(err .context(format!("Could not open file {}", config_file.display())) - .into()); - } + .into()) + }; } let settings = match Settings::new(config_file) { diff --git a/edgelet/iotedge/src/check/mod.rs b/edgelet/iotedge/src/check/mod.rs index 14c16ad40b1..8d27046f0a5 100644 --- a/edgelet/iotedge/src/check/mod.rs +++ b/edgelet/iotedge/src/check/mod.rs @@ -39,6 +39,7 @@ pub struct Check { output_format: OutputFormat, verbose: bool, warnings_as_errors: bool, + aziot_bin: std::ffi::OsString, additional_info: AdditionalInfo, @@ -90,6 +91,7 @@ impl Check { output_format: OutputFormat, verbose: bool, warnings_as_errors: bool, + aziot_bin: std::ffi::OsString, ) -> impl Future + Send { let latest_versions = if let Some(expected_aziot_edged_version) = expected_aziot_edged_version @@ -195,6 +197,7 @@ impl Check { output_format, verbose, warnings_as_errors, + aziot_bin, additional_info: AdditionalInfo::new(), @@ -312,9 +315,26 @@ impl Check { Ok(()) } + fn output_section(&self, section_name: &str) { + if self.output_format == OutputFormat::Text { + println!(); + println!("{}", section_name); + println!("{}", "-".repeat(section_name.len())); + } + } + pub fn execute(&mut self, runtime: &mut tokio::runtime::Runtime) -> Result<(), Error> { - let mut checks: BTreeMap<&str, CheckOutputSerializable> = Default::default(); - let mut check_data = checks::built_in_checks(); + // heterogeneous type representing the output of a check, regardless of + // whether or not it is built-in, or parsed from `aziot check` + #[derive(Debug)] + struct CheckOutput { + id: String, + description: String, + result: CheckResult, + additional_info: serde_json::Value, + }; + + let mut checks: BTreeMap = Default::default(); let mut stdout = Stdout::new(self.output_format); @@ -324,190 +344,319 @@ impl Check { let mut num_fatal = 0_usize; let mut num_errors = 0_usize; - for (section_name, section_checks) in &mut check_data { + let mut output_check = |check: CheckOutput, + verbose: bool, + warnings_as_errors: bool| + -> Result { if num_fatal > 0 { - break; - } - - if self.output_format == OutputFormat::Text { - println!("{}", section_name); - println!("{}", "-".repeat(section_name.len())); + return Ok(true); } - for check in section_checks { - let check_id = check.id(); - let check_name = check.description(); + let CheckOutput { + id: check_id, + description: check_name, + result: check_result, + additional_info, + .. + } = check; + + match check_result { + CheckResult::Ok => { + num_successful += 1; + + checks.insert( + check_id, + CheckOutputSerializable { + result: CheckResultSerializable::Ok, + additional_info, + }, + ); - if num_fatal > 0 { - break; + stdout.write_success(|stdout| { + writeln!(stdout, "\u{221a} {} - OK", check_name)?; + Ok(()) + }); } - let check_result = if self.dont_run.contains(check.id()) { - CheckResult::Ignored - } else { - check.execute(self, runtime) - }; - - match check_result { - CheckResult::Ok => { - num_successful += 1; + CheckResult::Warning(ref warning) if !warnings_as_errors => { + num_warnings += 1; - checks.insert( - check_id, - CheckOutputSerializable { - result: CheckResultSerializable::Ok, - additional_info: check.get_json(), + checks.insert( + check_id, + CheckOutputSerializable { + result: CheckResultSerializable::Warning { + details: warning.iter_chain().map(ToString::to_string).collect(), }, - ); + additional_info, + }, + ); - stdout.write_success(|stdout| { - writeln!(stdout, "\u{221a} {} - OK", check_name)?; - Ok(()) - }); - } + stdout.write_warning(|stdout| { + writeln!(stdout, "\u{203c} {} - Warning", check_name)?; - CheckResult::Warning(ref warning) if !self.warnings_as_errors => { - num_warnings += 1; - - checks.insert( - check_id, - CheckOutputSerializable { - result: CheckResultSerializable::Warning { - details: warning - .iter_chain() - .map(ToString::to_string) - .collect(), - }, - additional_info: check.get_json(), - }, - ); + let message = warning.to_string(); - stdout.write_warning(|stdout| { - writeln!(stdout, "\u{203c} {} - Warning", check_name)?; + write_lines(stdout, " ", " ", message.lines())?; - let message = warning.to_string(); + if verbose { + for cause in warning.iter_causes() { + write_lines( + stdout, + " caused by: ", + " ", + cause.to_string().lines(), + )?; + } + } - write_lines(stdout, " ", " ", message.lines())?; + Ok(()) + }); + } - if self.verbose { - for cause in warning.iter_causes() { - write_lines( - stdout, - " caused by: ", - " ", - cause.to_string().lines(), - )?; - } - } + CheckResult::Ignored => { + checks.insert( + check_id, + CheckOutputSerializable { + result: CheckResultSerializable::Ignored, + additional_info, + }, + ); + } + + CheckResult::Skipped => { + num_skipped += 1; + checks.insert( + check_id, + CheckOutputSerializable { + result: CheckResultSerializable::Skipped, + additional_info, + }, + ); + + if verbose { + stdout.write_warning(|stdout| { + writeln!(stdout, "\u{203c} {} - Warning", check_name)?; + writeln!(stdout, " skipping because of previous failures")?; Ok(()) }); } + } + + CheckResult::Fatal(err) => { + num_fatal += 1; - CheckResult::Ignored => { - checks.insert( - check_id, - CheckOutputSerializable { - result: CheckResultSerializable::Ignored, - additional_info: check.get_json(), + checks.insert( + check_id, + CheckOutputSerializable { + result: CheckResultSerializable::Fatal { + details: err.iter_chain().map(ToString::to_string).collect(), }, - ); - } + additional_info, + }, + ); - CheckResult::Skipped => { - num_skipped += 1; + stdout.write_error(|stdout| { + writeln!(stdout, "\u{00d7} {} - Error", check_name)?; - checks.insert( - check_id, - CheckOutputSerializable { - result: CheckResultSerializable::Skipped, - additional_info: check.get_json(), - }, - ); - - if self.verbose { - stdout.write_warning(|stdout| { - writeln!(stdout, "\u{203c} {} - Warning", check_name)?; - writeln!(stdout, " skipping because of previous failures")?; - Ok(()) - }); + let message = err.to_string(); + + write_lines(stdout, " ", " ", message.lines())?; + + if verbose { + for cause in err.iter_causes() { + write_lines( + stdout, + " caused by: ", + " ", + cause.to_string().lines(), + )?; + } } - } - CheckResult::Fatal(err) => { - num_fatal += 1; + Ok(()) + }); + } + + CheckResult::Warning(err) | CheckResult::Failed(err) => { + num_errors += 1; - checks.insert( - check_id, - CheckOutputSerializable { - result: CheckResultSerializable::Fatal { - details: err.iter_chain().map(ToString::to_string).collect(), - }, - additional_info: check.get_json(), + checks.insert( + check_id, + CheckOutputSerializable { + result: CheckResultSerializable::Error { + details: err.iter_chain().map(ToString::to_string).collect(), }, - ); + additional_info, + }, + ); - stdout.write_error(|stdout| { - writeln!(stdout, "\u{00d7} {} - Error", check_name)?; + stdout.write_error(|stdout| { + writeln!(stdout, "\u{00d7} {} - Error", check_name)?; - let message = err.to_string(); + let message = err.to_string(); - write_lines(stdout, " ", " ", message.lines())?; + write_lines(stdout, " ", " ", message.lines())?; - if self.verbose { - for cause in err.iter_causes() { - write_lines( - stdout, - " caused by: ", - " ", - cause.to_string().lines(), - )?; - } + if verbose { + for cause in err.iter_causes() { + write_lines( + stdout, + " caused by: ", + " ", + cause.to_string().lines(), + )?; } + } - Ok(()) - }); - } + Ok(()) + }); + } + } - CheckResult::Warning(err) | CheckResult::Failed(err) => { - num_errors += 1; + Ok(false) + }; - checks.insert( - check_id, - CheckOutputSerializable { - result: CheckResultSerializable::Error { - details: err.iter_chain().map(ToString::to_string).collect(), - }, - additional_info: check.get_json(), - }, - ); + // run the aziot checks first + { + #[derive(Debug, serde::Deserialize)] + #[serde(tag = "kind")] + #[serde(rename_all = "snake_case")] + enum CheckResultsSerializableStreaming { + AdditionalInfo(serde_json::Value), + Section { + name: String, + }, + Check { + id: String, + description: String, + result: CheckResultSerializable, + additional_info: serde_json::Value, + }, + } + + fn to_check_result(res: CheckResultSerializable) -> CheckResult { + fn vec_to_err(mut v: Vec) -> failure::Error { + let mut err = + failure::err_msg(v.pop().expect("errors always have at least one source")); + while let Some(s) = v.pop() { + err = err.context(s).into(); + } + err + } - stdout.write_error(|stdout| { - writeln!(stdout, "\u{00d7} {} - Error", check_name)?; + match res { + CheckResultSerializable::Ok => CheckResult::Ok, + CheckResultSerializable::Warning { details } => { + CheckResult::Warning(vec_to_err(details)) + } + CheckResultSerializable::Ignored => CheckResult::Ignored, + CheckResultSerializable::Skipped => CheckResult::Skipped, + CheckResultSerializable::Fatal { details } => { + CheckResult::Fatal(vec_to_err(details)) + } + CheckResultSerializable::Error { details } => { + CheckResult::Failed(vec_to_err(details)) + } + } + } - let message = err.to_string(); + let mut aziot_check = std::process::Command::new(&self.aziot_bin); + aziot_check + .arg("check") + .arg("-o=json-stream") + .stdout(std::process::Stdio::piped()); - write_lines(stdout, " ", " ", message.lines())?; + if !self.dont_run.is_empty() { + aziot_check + .arg("--dont-run") + .arg(self.dont_run.iter().cloned().collect::>().join(" ")); + } - if self.verbose { - for cause in err.iter_causes() { - write_lines( - stdout, - " caused by: ", - " ", - cause.to_string().lines(), - )?; + match aziot_check.spawn() { + Err(err) => { + // not being able to access aziot is bad... but we shouldn't fail here, + // as the user may still want to run other iotedge specific checks. + // + // nonetheless, we still need to notify the user that the aziot checks + // could not be run. + self.output_section("(aziot)"); + output_check( + CheckOutput { + id: "(aziot-error)".into(), + description: "aziot checks unavailable - could not communicate with 'aziot' binary.".into(), + result: CheckResult::Failed(err.context(ErrorKind::Aziot).into()), + additional_info: serde_json::Value::Null, + }, + self.verbose, + self.warnings_as_errors, + )?; + } + Ok(child) => { + for val in serde_json::Deserializer::from_reader(child.stdout.unwrap()) + .into_iter::() + { + let val = val.context(ErrorKind::Aziot)?; + match val { + CheckResultsSerializableStreaming::Section { name } => { + self.output_section(&format!("{} (aziot)", name)) + } + CheckResultsSerializableStreaming::Check { + id, + description, + result, + additional_info, + } => { + if output_check( + CheckOutput { + id, + description, + result: to_check_result(result), + additional_info, + }, + self.verbose, + self.warnings_as_errors, + )? { + break; } } - - Ok(()) - }); + CheckResultsSerializableStreaming::AdditionalInfo(info) => { + // try to extract iothub_hostname from additional_info + self.iothub_hostname = info + .as_object() + .and_then(|m| m.get("iothub_hostname")) + .and_then(serde_json::Value::as_str) + .map(Into::into) + } + } } } - } + }; + } + + // run the built-in checks + 'outer: for (section_name, section_checks) in &mut checks::built_in_checks() { + self.output_section(§ion_name); - if self.output_format == OutputFormat::Text { - println!(); + for check in section_checks { + let check_result = if self.dont_run.contains(check.id()) { + CheckResult::Ignored + } else { + check.execute(self, runtime) + }; + + if output_check( + CheckOutput { + id: check.id().into(), + description: check.description().into(), + result: check_result, + additional_info: check.get_json(), + }, + self.verbose, + self.warnings_as_errors, + )? { + break 'outer; + } } } @@ -600,10 +749,10 @@ fn write_lines<'a>( #[derive(Debug, serde_derive::Serialize)] struct CheckResultsSerializable<'a> { additional_info: &'a AdditionalInfo, - checks: BTreeMap<&'static str, CheckOutputSerializable>, + checks: BTreeMap, } -#[derive(Debug, serde_derive::Serialize)] +#[derive(Debug, serde_derive::Serialize, serde_derive::Deserialize)] #[serde(tag = "result")] #[serde(rename_all = "snake_case")] enum CheckResultSerializable { @@ -651,6 +800,7 @@ mod tests { super::OutputFormat::Text, // unused for this test false, false, + "".into(), // unused for this test )) .unwrap(); @@ -712,6 +862,7 @@ mod tests { super::OutputFormat::Text, // unused for this test false, false, + "".into(), // unused for this test )) .unwrap(); @@ -781,6 +932,7 @@ mod tests { super::OutputFormat::Text, // unused for this test false, false, + "".into(), // unused for this test )) .unwrap(); @@ -826,6 +978,7 @@ mod tests { super::OutputFormat::Text, // unused for this test false, false, + "".into(), // unused for this test )) .unwrap(); diff --git a/edgelet/iotedge/src/main.rs b/edgelet/iotedge/src/main.rs index 8e71093d8a9..96b8163729d 100644 --- a/edgelet/iotedge/src/main.rs +++ b/edgelet/iotedge/src/main.rs @@ -319,6 +319,9 @@ fn run() -> Result<(), Error> { .expect("arg has a default value"), args.is_present("verbose"), args.is_present("warnings-as-errors"), + args.value_of_os("aziot-bin") + .expect("arg has a default value") + .to_os_string(), ); tokio_runtime.block_on(check)?.execute(&mut tokio_runtime) From 42d5e6d49a5c91d2cf3290006cca6d6952c2be60 Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Wed, 23 Dec 2020 16:50:50 -0500 Subject: [PATCH 06/15] switch to aziot-check-common types - ONLY WORKS LOCALLY the aziot PR has to land before updating the submodule. --- edgelet/Cargo.lock | 20 ++++++- edgelet/iotedge/Cargo.toml | 2 + edgelet/iotedge/src/check/mod.rs | 96 +++++++++----------------------- 3 files changed, 45 insertions(+), 73 deletions(-) diff --git a/edgelet/Cargo.lock b/edgelet/Cargo.lock index 975d7ad30ff..8dbde412d41 100644 --- a/edgelet/Cargo.lock +++ b/edgelet/Cargo.lock @@ -48,6 +48,12 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "anyhow" +version = "1.0.36" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68803225a7b13e47191bab76f2687382b60d259e8cf37f6e1893658b84bb9479" + [[package]] name = "arc-swap" version = "0.4.7" @@ -96,6 +102,15 @@ dependencies = [ "serde", ] +[[package]] +name = "aziot-check-common" +version = "0.1.0" +dependencies = [ + "anyhow", + "serde", + "serde_json", +] + [[package]] name = "aziot-edged" version = "0.1.0" @@ -1263,6 +1278,7 @@ name = "iotedge" version = "0.1.0" dependencies = [ "atty", + "aziot-check-common", "byte-unit", "bytes 0.4.12", "chrono", @@ -2165,9 +2181,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.57" +version = "1.0.60" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "164eacbdb13512ec2745fb09d51fd5b22b0d65ed294a1dcf7285a360c80a675c" +checksum = "1500e84d27fe482ed1dc791a56eddc2f230046a040fa908c08bda1d9fb615779" dependencies = [ "itoa", "ryu", diff --git a/edgelet/iotedge/Cargo.toml b/edgelet/iotedge/Cargo.toml index 9e2f8f49c59..917f4cec71a 100644 --- a/edgelet/iotedge/Cargo.toml +++ b/edgelet/iotedge/Cargo.toml @@ -39,6 +39,8 @@ edgelet-http-mgmt = { path = "../edgelet-http-mgmt" } management = { path = "../management" } support-bundle = { path = "../support-bundle" } +aziot-check-common = { path = "/home/danielprilik/Work/iot-identity-service/aziot/aziot-check-common" } + [target.'cfg(unix)'.dependencies] byte-unit = "3.0.3" libc = "0.2" diff --git a/edgelet/iotedge/src/check/mod.rs b/edgelet/iotedge/src/check/mod.rs index 8d27046f0a5..71dd8d1fe54 100644 --- a/edgelet/iotedge/src/check/mod.rs +++ b/edgelet/iotedge/src/check/mod.rs @@ -12,6 +12,11 @@ use edgelet_docker::Settings; use edgelet_http::client::ClientImpl; use edgelet_http::MaybeProxyClient; +use aziot_check_common::{ + CheckOuputSerializableStreaming, CheckOutputSerializable, CheckResultSerializable, + CheckResultsSerializable, CheckerMetaSerializable, +}; + use crate::error::{Error, ErrorKind, FetchLatestVersionsReason}; use crate::LatestVersions; @@ -210,14 +215,7 @@ impl Check { } pub fn print_list(aziot_bin: std::ffi::OsString) -> Result<(), Error> { - // DEVNOTE: keep in sync with aziot CheckerMeta - #[derive(serde::Deserialize)] - struct CheckerMeta { - id: String, - description: String, - } - - let mut all_checks: Vec<(String, Vec)> = Vec::new(); + let mut all_checks: Vec<(String, Vec)> = Vec::new(); // get all the aziot checks by shelling-out to aziot { @@ -228,21 +226,21 @@ impl Check { match aziot_check_out { Err(_) => { - // not being able to access aziot is bad... but we shouldn't fail here, - // as the user may still want to run other iotedge specific checks. + // not being able to shell-out to aziot is bad... but we shouldn't fail here, + // as there might be other iotedge specific checks that don't rely on aziot. // // to make sure the user knows that there should me more checks, we add // this "dummy" entry instead. all_checks.push(( "(aziot)".into(), - vec![CheckerMeta { + vec![CheckerMetaSerializable { id: "(aziot-error)".into(), description: "(aziot checks unavailable - could not communicate with 'aziot' binary)".into(), }] )); } Ok(out) => { - let aziot_checks: BTreeMap> = + let aziot_checks: BTreeMap> = serde_json::from_slice(&out.stdout).context(ErrorKind::Aziot)?; all_checks.extend( @@ -262,7 +260,7 @@ impl Check { (*section_name).to_string(), checks .iter() - .map(|c| CheckerMeta { + .map(|c| CheckerMetaSerializable { id: c.id().into(), description: c.description().into(), }) @@ -517,24 +515,9 @@ impl Check { Ok(false) }; - // run the aziot checks first + // run the aziot checks first, as certain bits of `additional_info` from + // aziot are required to run iotedge checks. e.g: the "iothub_hostname". { - #[derive(Debug, serde::Deserialize)] - #[serde(tag = "kind")] - #[serde(rename_all = "snake_case")] - enum CheckResultsSerializableStreaming { - AdditionalInfo(serde_json::Value), - Section { - name: String, - }, - Check { - id: String, - description: String, - result: CheckResultSerializable, - additional_info: serde_json::Value, - }, - } - fn to_check_result(res: CheckResultSerializable) -> CheckResult { fn vec_to_err(mut v: Vec) -> failure::Error { let mut err = @@ -575,8 +558,8 @@ impl Check { match aziot_check.spawn() { Err(err) => { - // not being able to access aziot is bad... but we shouldn't fail here, - // as the user may still want to run other iotedge specific checks. + // not being able to shell-out to aziot is bad... but we shouldn't fail here, + // as there might be other iotedge specific checks that don't rely on aziot. // // nonetheless, we still need to notify the user that the aziot checks // could not be run. @@ -593,26 +576,21 @@ impl Check { )?; } Ok(child) => { - for val in serde_json::Deserializer::from_reader(child.stdout.unwrap()) - .into_iter::() + for val in + serde_json::Deserializer::from_reader(child.stdout.unwrap()).into_iter() { let val = val.context(ErrorKind::Aziot)?; match val { - CheckResultsSerializableStreaming::Section { name } => { + CheckOuputSerializableStreaming::Section { name } => { self.output_section(&format!("{} (aziot)", name)) } - CheckResultsSerializableStreaming::Check { - id, - description, - result, - additional_info, - } => { + CheckOuputSerializableStreaming::Check { meta, output } => { if output_check( CheckOutput { - id, - description, - result: to_check_result(result), - additional_info, + id: meta.id, + description: meta.description, + result: to_check_result(output.result), + additional_info: output.additional_info, }, self.verbose, self.warnings_as_errors, @@ -620,7 +598,7 @@ impl Check { break; } } - CheckResultsSerializableStreaming::AdditionalInfo(info) => { + CheckOuputSerializableStreaming::AdditionalInfo(info) => { // try to extract iothub_hostname from additional_info self.iothub_hostname = info .as_object() @@ -713,7 +691,7 @@ impl Check { if self.output_format == OutputFormat::Json { let check_results = CheckResultsSerializable { - additional_info: &self.additional_info, + additional_info: serde_json::to_value(&self.additional_info).unwrap(), checks, }; @@ -746,30 +724,6 @@ fn write_lines<'a>( Ok(()) } -#[derive(Debug, serde_derive::Serialize)] -struct CheckResultsSerializable<'a> { - additional_info: &'a AdditionalInfo, - checks: BTreeMap, -} - -#[derive(Debug, serde_derive::Serialize, serde_derive::Deserialize)] -#[serde(tag = "result")] -#[serde(rename_all = "snake_case")] -enum CheckResultSerializable { - Ok, - Warning { details: Vec }, - Ignored, - Skipped, - Fatal { details: Vec }, - Error { details: Vec }, -} - -#[derive(Debug, serde_derive::Serialize)] -struct CheckOutputSerializable { - result: CheckResultSerializable, - additional_info: serde_json::Value, -} - #[cfg(test)] mod tests { use super::{ From 6786aa976056c44a6725b772f8d130705b3a30c3 Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Fri, 8 Jan 2021 11:44:59 -0500 Subject: [PATCH 07/15] bump submodule version + use deps from submodule --- edgelet/Cargo.lock | 12 +++++++++--- edgelet/aziot | 2 +- edgelet/iotedge/Cargo.toml | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/edgelet/Cargo.lock b/edgelet/Cargo.lock index 2d959174684..eae3cdc86c2 100644 --- a/edgelet/Cargo.lock +++ b/edgelet/Cargo.lock @@ -100,7 +100,9 @@ dependencies = [ name = "aziot-certd-config" version = "0.1.0" dependencies = [ + "hex 0.4.2", "http-common", + "openssl", "serde", "url 2.1.1", ] @@ -178,7 +180,6 @@ dependencies = [ "aziot-cert-common-http", "aziot-identity-common", "aziot-key-common", - "aziot-key-common-http", "http-common", "serde", "serde_json", @@ -1036,6 +1037,12 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "805026a5d0141ffc30abb3be3173848ad46a1b1664fe632428479619a3644d77" +[[package]] +name = "hex" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "644f9158b2f133fd50f5fb3242878846d9eb792e445c893805ff0e3824006e35" + [[package]] name = "hmac" version = "0.5.0" @@ -1088,7 +1095,6 @@ dependencies = [ "base64 0.12.3", "futures-util", "http 0.2.1", - "lazy_static", "libc", "log", "nix 0.18.0", @@ -1180,7 +1186,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d063d6d5658623c6ef16f452e11437c0e7e23a6d327470573fe78892dafbc4fb" dependencies = [ "futures", - "hex", + "hex 0.3.2", "hyper", "tokio", "tokio-io", diff --git a/edgelet/aziot b/edgelet/aziot index 7fa13f0d852..a344cfa825c 160000 --- a/edgelet/aziot +++ b/edgelet/aziot @@ -1 +1 @@ -Subproject commit 7fa13f0d852f4ed6e7ba94dd518e2231c345300f +Subproject commit a344cfa825c628ddde7192bae838c1dde27fd99f diff --git a/edgelet/iotedge/Cargo.toml b/edgelet/iotedge/Cargo.toml index 1aab8dbbaf2..a981e69f9e9 100644 --- a/edgelet/iotedge/Cargo.toml +++ b/edgelet/iotedge/Cargo.toml @@ -37,6 +37,7 @@ url = "2" zip = "0.5.3" aziot-certd-config = { path = "../aziot/cert/aziot-certd-config" } +aziot-check-common = { path = "../aziot/aziot/aziot-check-common" } aziot-identity-common = { path = "../aziot/identity/aziot-identity-common" } aziot-identityd-config = { path = "../aziot/identity/aziot-identityd-config" } aziot-keyd-config = { path = "../aziot/key/aziot-keyd-config" } @@ -51,7 +52,6 @@ edgelet-utils = { path = "../edgelet-utils" } management = { path = "../management" } support-bundle = { path = "../support-bundle" } -aziot-check-common = { path = "/home/danielprilik/Work/iot-identity-service/aziot/aziot-check-common" } [target.'cfg(unix)'.dependencies] byte-unit = "3.0.3" From 28b3a5b70709c433cdddd699ac8cf28cf1cffdf2 Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Fri, 8 Jan 2021 12:15:45 -0500 Subject: [PATCH 08/15] plumb-though --iothub-hostname flag to aziot check --- edgelet/iotedge/src/check/mod.rs | 22 +++++++++++++++------- edgelet/iotedge/src/main.rs | 1 + 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/edgelet/iotedge/src/check/mod.rs b/edgelet/iotedge/src/check/mod.rs index 98a1eaab2b0..4354c4c1026 100644 --- a/edgelet/iotedge/src/check/mod.rs +++ b/edgelet/iotedge/src/check/mod.rs @@ -97,6 +97,7 @@ impl Check { verbose: bool, warnings_as_errors: bool, aziot_bin: std::ffi::OsString, + iothub_hostname: Option, ) -> impl Future + Send { let latest_versions = if let Some(expected_aziot_edged_version) = expected_aziot_edged_version @@ -206,7 +207,7 @@ impl Check { additional_info: AdditionalInfo::new(), - iothub_hostname: None, + iothub_hostname, settings: None, docker_host_arg: None, docker_server_version: None, @@ -550,6 +551,10 @@ impl Check { .arg("-o=json-stream") .stdout(std::process::Stdio::piped()); + if let Some(iothub_hostname) = &self.iothub_hostname { + aziot_check.arg("--iothub-hostname").arg(iothub_hostname); + } + if !self.dont_run.is_empty() { aziot_check .arg("--dont-run") @@ -599,12 +604,15 @@ impl Check { } } CheckOutputSerializableStreaming::AdditionalInfo(info) => { - // try to extract iothub_hostname from additional_info - self.iothub_hostname = info - .as_object() - .and_then(|m| m.get("iothub_hostname")) - .and_then(serde_json::Value::as_str) - .map(Into::into) + // if it wasn't manually specified via CLI flag, try to + // extract iothub_hostname from additional_info + if self.iothub_hostname.is_none() { + self.iothub_hostname = info + .as_object() + .and_then(|m| m.get("iothub_hostname")) + .and_then(serde_json::Value::as_str) + .map(Into::into) + } } } } diff --git a/edgelet/iotedge/src/main.rs b/edgelet/iotedge/src/main.rs index 17b2483ffa1..817bbd6fdb6 100644 --- a/edgelet/iotedge/src/main.rs +++ b/edgelet/iotedge/src/main.rs @@ -333,6 +333,7 @@ fn run() -> Result<(), Error> { args.value_of_os("aziot-bin") .expect("arg has a default value") .to_os_string(), + args.value_of("iothub-hostname").map(ToOwned::to_owned), ); tokio_runtime.block_on(check)?.execute(&mut tokio_runtime) From 511e9e3ea7fa4d436ff8e26a37757db9fae73c8d Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Fri, 8 Jan 2021 12:25:34 -0500 Subject: [PATCH 09/15] clippy --- edgelet/iotedge/src/check/checks/container_engine_is_moby.rs | 1 - edgelet/iotedge/src/check/checks/parent_hostname.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/edgelet/iotedge/src/check/checks/container_engine_is_moby.rs b/edgelet/iotedge/src/check/checks/container_engine_is_moby.rs index 893a52da882..4dd809c365b 100644 --- a/edgelet/iotedge/src/check/checks/container_engine_is_moby.rs +++ b/edgelet/iotedge/src/check/checks/container_engine_is_moby.rs @@ -25,7 +25,6 @@ impl Checker for ContainerEngineIsMoby { } impl ContainerEngineIsMoby { - #[allow(clippy::unnecessary_wraps)] // keeps this inner_execute consistent with all other checks fn inner_execute(&mut self, check: &mut Check) -> Result { const MESSAGE: &str = "Device is not using a production-supported container engine (moby-engine).\n\ diff --git a/edgelet/iotedge/src/check/checks/parent_hostname.rs b/edgelet/iotedge/src/check/checks/parent_hostname.rs index 88957a9b7a3..7f2bac448f5 100644 --- a/edgelet/iotedge/src/check/checks/parent_hostname.rs +++ b/edgelet/iotedge/src/check/checks/parent_hostname.rs @@ -28,7 +28,6 @@ impl Checker for ParentHostname { } impl ParentHostname { - #[allow(clippy::unnecessary_wraps)] // keeps this inner_execute consistent with all other checks fn inner_execute(&mut self, check: &mut Check) -> Result { let settings = if let Some(settings) = &check.settings { settings From 2f11525cea5c5fee479dd9cf8af6336488d3575e Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Fri, 8 Jan 2021 12:33:27 -0500 Subject: [PATCH 10/15] fix tests --- edgelet/iotedge/src/check/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/edgelet/iotedge/src/check/mod.rs b/edgelet/iotedge/src/check/mod.rs index 4354c4c1026..43eae6a3e86 100644 --- a/edgelet/iotedge/src/check/mod.rs +++ b/edgelet/iotedge/src/check/mod.rs @@ -763,6 +763,7 @@ mod tests { false, false, "".into(), // unused for this test + None, )) .unwrap(); @@ -825,6 +826,7 @@ mod tests { false, false, "".into(), // unused for this test + None, )) .unwrap(); @@ -895,6 +897,7 @@ mod tests { false, false, "".into(), // unused for this test + None, )) .unwrap(); @@ -941,6 +944,7 @@ mod tests { false, false, "".into(), // unused for this test + None, )) .unwrap(); From 3f68400e9fe7368dec50bb0a582eab3ac25541c8 Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Tue, 12 Jan 2021 12:36:25 -0500 Subject: [PATCH 11/15] address PR comments --- .../checks/container_engine_installed.rs | 9 +-- .../src/check/checks/container_engine_ipv6.rs | 13 ++--- .../check/checks/storage_mounted_from_host.rs | 3 +- edgelet/iotedge/src/check/mod.rs | 58 +++++++++---------- 4 files changed, 40 insertions(+), 43 deletions(-) diff --git a/edgelet/iotedge/src/check/checks/container_engine_installed.rs b/edgelet/iotedge/src/check/checks/container_engine_installed.rs index e80e3b5f64a..c229b61cff1 100644 --- a/edgelet/iotedge/src/check/checks/container_engine_installed.rs +++ b/edgelet/iotedge/src/check/checks/container_engine_installed.rs @@ -65,12 +65,9 @@ impl ContainerEngineInstalled { ); if let Some(message) = message { - #[cfg(unix)] - { - if message.contains("Got permission denied") { - error_message += "\nYou might need to run this command as root."; - return Ok(CheckResult::Fatal(err.context(error_message).into())); - } + if message.contains("Got permission denied") { + error_message += "\nYou might need to run this command as root."; + return Ok(CheckResult::Fatal(err.context(error_message).into())); } } diff --git a/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs b/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs index 93921b5f845..f117354033f 100644 --- a/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs +++ b/edgelet/iotedge/src/check/checks/container_engine_ipv6.rs @@ -72,13 +72,12 @@ impl ContainerEngineIPv6 { .context(MESSAGE)?; self.actual_use_ipv6 = daemon_config.ipv6; - match ( - daemon_config.ipv6.unwrap_or_default(), - is_edge_ipv6_configured, - ) { - (true, _) => Ok(CheckResult::Ok), - (false, true) => Err(Context::new(MESSAGE).into()), - (false, false) => Ok(CheckResult::Ignored), + if daemon_config.ipv6.unwrap_or_default() { + Ok(CheckResult::Ok) + } else if is_edge_ipv6_configured { + Err(Context::new(MESSAGE).into()) + } else { + Ok(CheckResult::Ignored) } } } diff --git a/edgelet/iotedge/src/check/checks/storage_mounted_from_host.rs b/edgelet/iotedge/src/check/checks/storage_mounted_from_host.rs index 6c5e3e1ed4d..c8353fbe3c8 100644 --- a/edgelet/iotedge/src/check/checks/storage_mounted_from_host.rs +++ b/edgelet/iotedge/src/check/checks/storage_mounted_from_host.rs @@ -95,7 +95,8 @@ fn storage_mounted_from_host( .and_then(|capture| capture.get(1)) .map(|match_| match_.as_str()) }) - // Hard-code the value here rather than using the tempfile crate. + // Hard-code the value here rather than using the tempfile crate. It needs to match .Net Core's implementation, + // and needs to be in the context of the container user instead of the host running `iotedge check`. .unwrap_or("/tmp"); let storage_directory = Path::new(&*temp_dir).join(storage_directory_name); diff --git a/edgelet/iotedge/src/check/mod.rs b/edgelet/iotedge/src/check/mod.rs index 43eae6a3e86..10bf42b6619 100644 --- a/edgelet/iotedge/src/check/mod.rs +++ b/edgelet/iotedge/src/check/mod.rs @@ -222,10 +222,20 @@ impl Check { { let aziot_check_out = std::process::Command::new(aziot_bin) .arg("check-list") - .arg("-j") + .arg("--json") .output(); match aziot_check_out { + Ok(out) => { + let aziot_checks: BTreeMap> = + serde_json::from_slice(&out.stdout).context(ErrorKind::Aziot)?; + + all_checks.extend( + aziot_checks + .into_iter() + .map(|(section_name, checks)| (section_name + " (aziot)", checks)), + ); + } Err(_) => { // not being able to shell-out to aziot is bad... but we shouldn't fail here, // as there might be other iotedge specific checks that don't rely on aziot. @@ -240,16 +250,6 @@ impl Check { }] )); } - Ok(out) => { - let aziot_checks: BTreeMap> = - serde_json::from_slice(&out.stdout).context(ErrorKind::Aziot)?; - - all_checks.extend( - aziot_checks - .into_iter() - .map(|(section_name, checks)| (section_name + " (aziot)", checks)), - ); - } } } @@ -562,24 +562,6 @@ impl Check { } match aziot_check.spawn() { - Err(err) => { - // not being able to shell-out to aziot is bad... but we shouldn't fail here, - // as there might be other iotedge specific checks that don't rely on aziot. - // - // nonetheless, we still need to notify the user that the aziot checks - // could not be run. - self.output_section("(aziot)"); - output_check( - CheckOutput { - id: "(aziot-error)".into(), - description: "aziot checks unavailable - could not communicate with 'aziot' binary.".into(), - result: CheckResult::Failed(err.context(ErrorKind::Aziot).into()), - additional_info: serde_json::Value::Null, - }, - self.verbose, - self.warnings_as_errors, - )?; - } Ok(child) => { for val in serde_json::Deserializer::from_reader(child.stdout.unwrap()).into_iter() @@ -617,6 +599,24 @@ impl Check { } } } + Err(err) => { + // not being able to shell-out to aziot is bad... but we shouldn't fail here, + // as there might be other iotedge specific checks that don't rely on aziot. + // + // nonetheless, we still need to notify the user that the aziot checks + // could not be run. + self.output_section("(aziot)"); + output_check( + CheckOutput { + id: "(aziot-error)".into(), + description: "aziot checks unavailable - could not communicate with 'aziot' binary.".into(), + result: CheckResult::Failed(err.context(ErrorKind::Aziot).into()), + additional_info: serde_json::Value::Null, + }, + self.verbose, + self.warnings_as_errors, + )?; + } }; } From 437607bd87914a2f70a7f32d6393bcf5d1d7eb74 Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Tue, 12 Jan 2021 12:50:36 -0500 Subject: [PATCH 12/15] specify aziot binary via option_env! --- edgelet/iotedge/src/main.rs | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/edgelet/iotedge/src/main.rs b/edgelet/iotedge/src/main.rs index 817bbd6fdb6..d62dd9d2083 100644 --- a/edgelet/iotedge/src/main.rs +++ b/edgelet/iotedge/src/main.rs @@ -38,6 +38,8 @@ fn main() { #[allow(clippy::too_many_lines)] fn run() -> Result<(), Error> { + let aziot_bin = option_env!("AZIOT_BIN").unwrap_or("aziot"); + let default_mgmt_uri = option_env!("IOTEDGE_HOST").unwrap_or("unix:///var/lib/aziot/edged/aziot-edged.mgmt.sock"); @@ -61,16 +63,6 @@ fn run() -> Result<(), Error> { .env("IOTEDGE_HOST") .default_value(default_mgmt_uri), ) - .arg( - Arg::with_name("aziot-bin") - .help("path to 'aziot' binary") - .long("aziot-bin") - .takes_value(true) - .value_name("PATH") - .global(true) - .env("AZIOT_BIN") - .default_value("aziot"), - ) .subcommand( SubCommand::with_name("check") .about("Check for common config and deployment issues") @@ -330,19 +322,13 @@ fn run() -> Result<(), Error> { .expect("arg has a default value"), args.is_present("verbose"), args.is_present("warnings-as-errors"), - args.value_of_os("aziot-bin") - .expect("arg has a default value") - .to_os_string(), + aziot_bin.into(), args.value_of("iothub-hostname").map(ToOwned::to_owned), ); tokio_runtime.block_on(check)?.execute(&mut tokio_runtime) } - ("check-list", Some(args)) => Check::print_list( - args.value_of_os("aziot-bin") - .expect("arg has a default value") - .to_os_string(), - ), + ("check-list", Some(_)) => Check::print_list(aziot_bin.into()), ("list", _) => tokio_runtime.block_on(List::new(runtime()?, io::stdout()).execute()), ("restart", Some(args)) => tokio_runtime.block_on( Restart::new( From bb2e0a71fe22c3622ebdfd67c3a07ebfe01f4a8a Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Thu, 21 Jan 2021 12:51:44 -0500 Subject: [PATCH 13/15] bump aziot submodule --- edgelet/aziot | 2 +- edgelet/iotedge/src/check/mod.rs | 2 +- edgelet/iotedge/src/init/import/mod.rs | 21 ++++++++----------- .../init/import/ca-certs/identityd.toml | 2 +- .../import/complex-agent-spec/identityd.toml | 2 +- .../import/dps-symmetric-key/identityd.toml | 2 +- .../init/import/dps-tpm/identityd.toml | 2 +- .../init/import/dps-x509/identityd.toml | 2 +- .../manual-connection-string-2/identityd.toml | 2 +- .../manual-connection-string/identityd.toml | 2 +- .../init/import/manual-x509/identityd.toml | 2 +- .../moby-runtime-content-trust/identityd.toml | 2 +- .../moby-runtime-network-2/identityd.toml | 2 +- .../moby-runtime-network/identityd.toml | 2 +- 14 files changed, 22 insertions(+), 25 deletions(-) diff --git a/edgelet/aziot b/edgelet/aziot index 75633f15533..3043cafb91d 160000 --- a/edgelet/aziot +++ b/edgelet/aziot @@ -1 +1 @@ -Subproject commit 75633f155333a96730c33534d5a7fed311e7f501 +Subproject commit 3043cafb91d9343d64b5d548ae27c6c017412dc2 diff --git a/edgelet/iotedge/src/check/mod.rs b/edgelet/iotedge/src/check/mod.rs index 10bf42b6619..1e91d25fe4d 100644 --- a/edgelet/iotedge/src/check/mod.rs +++ b/edgelet/iotedge/src/check/mod.rs @@ -222,7 +222,7 @@ impl Check { { let aziot_check_out = std::process::Command::new(aziot_bin) .arg("check-list") - .arg("--json") + .arg("--output=json") .output(); match aziot_check_out { diff --git a/edgelet/iotedge/src/init/import/mod.rs b/edgelet/iotedge/src/init/import/mod.rs index e958db3f2be..297f5d0160f 100644 --- a/edgelet/iotedge/src/init/import/mod.rs +++ b/edgelet/iotedge/src/init/import/mod.rs @@ -253,7 +253,7 @@ fn execute_inner( ) = { let old_config::Provisioning { provisioning, - dynamic_reprovisioning, + dynamic_reprovisioning: _, } = provisioning; match provisioning { @@ -268,7 +268,7 @@ fn execute_inner( ), }) => ( aziot_identityd_config::Provisioning { - dynamic_reprovisioning: *dynamic_reprovisioning, + always_reprovision_on_startup: true, provisioning: aziot_identityd_config::ProvisioningType::Manual { iothub_hostname: hostname.clone(), device_id: device_id.clone(), @@ -295,7 +295,7 @@ fn execute_inner( }), }) => ( aziot_identityd_config::Provisioning { - dynamic_reprovisioning: *dynamic_reprovisioning, + always_reprovision_on_startup: true, provisioning: aziot_identityd_config::ProvisioningType::Manual { iothub_hostname: iothub_hostname.clone(), device_id: device_id.clone(), @@ -330,11 +330,10 @@ fn execute_inner( symmetric_key, }, ), - // TODO: Start migrating this when IS adds support for this flag - always_reprovision_on_startup: _, + always_reprovision_on_startup, }) => ( aziot_identityd_config::Provisioning { - dynamic_reprovisioning: *dynamic_reprovisioning, + always_reprovision_on_startup: *always_reprovision_on_startup, provisioning: aziot_identityd_config::ProvisioningType::Dps { global_endpoint: global_endpoint.to_string(), scope_id: scope_id.clone(), @@ -360,11 +359,10 @@ fn execute_inner( identity_cert, identity_pk, }), - // TODO: Start migrating this when IS adds support for this flag - always_reprovision_on_startup: _, + always_reprovision_on_startup, }) => ( aziot_identityd_config::Provisioning { - dynamic_reprovisioning: *dynamic_reprovisioning, + always_reprovision_on_startup: *always_reprovision_on_startup, provisioning: aziot_identityd_config::ProvisioningType::Dps { global_endpoint: global_endpoint.to_string(), scope_id: scope_id.clone(), @@ -400,11 +398,10 @@ fn execute_inner( old_config::AttestationMethod::Tpm(old_config::TpmAttestationInfo { registration_id, }), - // TODO: Start migrating this when IS adds support for this flag - always_reprovision_on_startup: _, + always_reprovision_on_startup, }) => ( aziot_identityd_config::Provisioning { - dynamic_reprovisioning: *dynamic_reprovisioning, + always_reprovision_on_startup: *always_reprovision_on_startup, provisioning: aziot_identityd_config::ProvisioningType::Dps { global_endpoint: global_endpoint.to_string(), scope_id: scope_id.clone(), diff --git a/edgelet/iotedge/test-files/init/import/ca-certs/identityd.toml b/edgelet/iotedge/test-files/init/import/ca-certs/identityd.toml index d560462fd8c..a27362f409c 100644 --- a/edgelet/iotedge/test-files/init/import/ca-certs/identityd.toml +++ b/edgelet/iotedge/test-files/init/import/ca-certs/identityd.toml @@ -2,7 +2,7 @@ hostname = "my-device" homedir = "/var/lib/aziot/identityd" [provisioning] -dynamic_reprovisioning = false +always_reprovision_on_startup = true source = "manual" iothub_hostname = "example.azure-devices.net" device_id = "my-device" diff --git a/edgelet/iotedge/test-files/init/import/complex-agent-spec/identityd.toml b/edgelet/iotedge/test-files/init/import/complex-agent-spec/identityd.toml index d560462fd8c..a27362f409c 100644 --- a/edgelet/iotedge/test-files/init/import/complex-agent-spec/identityd.toml +++ b/edgelet/iotedge/test-files/init/import/complex-agent-spec/identityd.toml @@ -2,7 +2,7 @@ hostname = "my-device" homedir = "/var/lib/aziot/identityd" [provisioning] -dynamic_reprovisioning = false +always_reprovision_on_startup = true source = "manual" iothub_hostname = "example.azure-devices.net" device_id = "my-device" diff --git a/edgelet/iotedge/test-files/init/import/dps-symmetric-key/identityd.toml b/edgelet/iotedge/test-files/init/import/dps-symmetric-key/identityd.toml index 6a0b7b4fa78..e1d18b65ce7 100644 --- a/edgelet/iotedge/test-files/init/import/dps-symmetric-key/identityd.toml +++ b/edgelet/iotedge/test-files/init/import/dps-symmetric-key/identityd.toml @@ -2,7 +2,7 @@ hostname = "my-device" homedir = "/var/lib/aziot/identityd" [provisioning] -dynamic_reprovisioning = true +always_reprovision_on_startup = true source = "dps" global_endpoint = "https://global.azure-devices-provisioning.net/" scope_id = "0ab1234C5D6" diff --git a/edgelet/iotedge/test-files/init/import/dps-tpm/identityd.toml b/edgelet/iotedge/test-files/init/import/dps-tpm/identityd.toml index 9b023ec6346..e661a555626 100644 --- a/edgelet/iotedge/test-files/init/import/dps-tpm/identityd.toml +++ b/edgelet/iotedge/test-files/init/import/dps-tpm/identityd.toml @@ -2,7 +2,7 @@ hostname = "my-device" homedir = "/var/lib/aziot/identityd" [provisioning] -dynamic_reprovisioning = true +always_reprovision_on_startup = true source = "dps" global_endpoint = "https://global.azure-devices-provisioning.net/" scope_id = "0ab1234C5D6" diff --git a/edgelet/iotedge/test-files/init/import/dps-x509/identityd.toml b/edgelet/iotedge/test-files/init/import/dps-x509/identityd.toml index fde62a0b0c3..c3a45baf8e1 100644 --- a/edgelet/iotedge/test-files/init/import/dps-x509/identityd.toml +++ b/edgelet/iotedge/test-files/init/import/dps-x509/identityd.toml @@ -2,7 +2,7 @@ hostname = "my-device" homedir = "/var/lib/aziot/identityd" [provisioning] -dynamic_reprovisioning = true +always_reprovision_on_startup = true source = "dps" global_endpoint = "https://global.azure-devices-provisioning.net/" scope_id = "0ab1234C5D6" diff --git a/edgelet/iotedge/test-files/init/import/manual-connection-string-2/identityd.toml b/edgelet/iotedge/test-files/init/import/manual-connection-string-2/identityd.toml index d560462fd8c..a27362f409c 100644 --- a/edgelet/iotedge/test-files/init/import/manual-connection-string-2/identityd.toml +++ b/edgelet/iotedge/test-files/init/import/manual-connection-string-2/identityd.toml @@ -2,7 +2,7 @@ hostname = "my-device" homedir = "/var/lib/aziot/identityd" [provisioning] -dynamic_reprovisioning = false +always_reprovision_on_startup = true source = "manual" iothub_hostname = "example.azure-devices.net" device_id = "my-device" diff --git a/edgelet/iotedge/test-files/init/import/manual-connection-string/identityd.toml b/edgelet/iotedge/test-files/init/import/manual-connection-string/identityd.toml index d560462fd8c..a27362f409c 100644 --- a/edgelet/iotedge/test-files/init/import/manual-connection-string/identityd.toml +++ b/edgelet/iotedge/test-files/init/import/manual-connection-string/identityd.toml @@ -2,7 +2,7 @@ hostname = "my-device" homedir = "/var/lib/aziot/identityd" [provisioning] -dynamic_reprovisioning = false +always_reprovision_on_startup = true source = "manual" iothub_hostname = "example.azure-devices.net" device_id = "my-device" diff --git a/edgelet/iotedge/test-files/init/import/manual-x509/identityd.toml b/edgelet/iotedge/test-files/init/import/manual-x509/identityd.toml index a763df01262..b77aa9c613c 100644 --- a/edgelet/iotedge/test-files/init/import/manual-x509/identityd.toml +++ b/edgelet/iotedge/test-files/init/import/manual-x509/identityd.toml @@ -2,7 +2,7 @@ hostname = "my-device" homedir = "/var/lib/aziot/identityd" [provisioning] -dynamic_reprovisioning = true +always_reprovision_on_startup = true source = "manual" iothub_hostname = "example.azure-devices.net" device_id = "my-device" diff --git a/edgelet/iotedge/test-files/init/import/moby-runtime-content-trust/identityd.toml b/edgelet/iotedge/test-files/init/import/moby-runtime-content-trust/identityd.toml index d560462fd8c..a27362f409c 100644 --- a/edgelet/iotedge/test-files/init/import/moby-runtime-content-trust/identityd.toml +++ b/edgelet/iotedge/test-files/init/import/moby-runtime-content-trust/identityd.toml @@ -2,7 +2,7 @@ hostname = "my-device" homedir = "/var/lib/aziot/identityd" [provisioning] -dynamic_reprovisioning = false +always_reprovision_on_startup = true source = "manual" iothub_hostname = "example.azure-devices.net" device_id = "my-device" diff --git a/edgelet/iotedge/test-files/init/import/moby-runtime-network-2/identityd.toml b/edgelet/iotedge/test-files/init/import/moby-runtime-network-2/identityd.toml index d560462fd8c..a27362f409c 100644 --- a/edgelet/iotedge/test-files/init/import/moby-runtime-network-2/identityd.toml +++ b/edgelet/iotedge/test-files/init/import/moby-runtime-network-2/identityd.toml @@ -2,7 +2,7 @@ hostname = "my-device" homedir = "/var/lib/aziot/identityd" [provisioning] -dynamic_reprovisioning = false +always_reprovision_on_startup = true source = "manual" iothub_hostname = "example.azure-devices.net" device_id = "my-device" diff --git a/edgelet/iotedge/test-files/init/import/moby-runtime-network/identityd.toml b/edgelet/iotedge/test-files/init/import/moby-runtime-network/identityd.toml index d560462fd8c..a27362f409c 100644 --- a/edgelet/iotedge/test-files/init/import/moby-runtime-network/identityd.toml +++ b/edgelet/iotedge/test-files/init/import/moby-runtime-network/identityd.toml @@ -2,7 +2,7 @@ hostname = "my-device" homedir = "/var/lib/aziot/identityd" [provisioning] -dynamic_reprovisioning = false +always_reprovision_on_startup = true source = "manual" iothub_hostname = "example.azure-devices.net" device_id = "my-device" From ba4ec194decd59386416c2500b8cf2eb53cb4532 Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Fri, 22 Jan 2021 12:11:19 -0500 Subject: [PATCH 14/15] add todo --- edgelet/iotedge/src/init/import/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/edgelet/iotedge/src/init/import/mod.rs b/edgelet/iotedge/src/init/import/mod.rs index 297f5d0160f..1f812576d67 100644 --- a/edgelet/iotedge/src/init/import/mod.rs +++ b/edgelet/iotedge/src/init/import/mod.rs @@ -253,6 +253,7 @@ fn execute_inner( ) = { let old_config::Provisioning { provisioning, + // TODO: Migrate this to edged config when support for dynamic reprovisioning is reinstated in edged. dynamic_reprovisioning: _, } = provisioning; From 886843097639bb3178ca2a9a3a35e27d03032a53 Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Fri, 22 Jan 2021 12:09:50 -0500 Subject: [PATCH 15/15] resolve $upstream in diagnostics-image-name (see #4171) --- edgelet/edgelet-docker/src/lib.rs | 4 +++- edgelet/edgelet-docker/src/settings.rs | 2 +- .../iotedge/src/check/checks/well_formed_config.rs | 12 +++++++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/edgelet/edgelet-docker/src/lib.rs b/edgelet/edgelet-docker/src/lib.rs index 2a6f913ab6f..d55d5d31c85 100644 --- a/edgelet/edgelet-docker/src/lib.rs +++ b/edgelet/edgelet-docker/src/lib.rs @@ -22,4 +22,6 @@ pub use crate::config::DockerConfig; pub use error::{Error, ErrorKind}; pub use module::{DockerModule, MODULE_TYPE}; pub use runtime::DockerModuleRuntime; -pub use settings::{ContentTrust, LoadSettingsError, MobyRuntime, Settings, DEFAULTS}; +pub use settings::{ + ContentTrust, LoadSettingsError, MobyRuntime, Settings, DEFAULTS, UPSTREAM_PARENT_KEYWORD, +}; diff --git a/edgelet/edgelet-docker/src/settings.rs b/edgelet/edgelet-docker/src/settings.rs index 2b13eed97bb..047e2eb152f 100644 --- a/edgelet/edgelet-docker/src/settings.rs +++ b/edgelet/edgelet-docker/src/settings.rs @@ -25,7 +25,7 @@ pub const DEFAULTS: &str = include_str!("../config/unix/default.yaml"); const EDGE_NETWORKID_KEY: &str = "NetworkId"; const UNIX_SCHEME: &str = "unix"; -const UPSTREAM_PARENT_KEYWORD: &str = "$upstream"; +pub const UPSTREAM_PARENT_KEYWORD: &str = "$upstream"; #[derive(Clone, Debug, serde_derive::Deserialize, serde_derive::Serialize)] pub struct MobyRuntime { diff --git a/edgelet/iotedge/src/check/checks/well_formed_config.rs b/edgelet/iotedge/src/check/checks/well_formed_config.rs index ded21144831..c42036d520b 100644 --- a/edgelet/iotedge/src/check/checks/well_formed_config.rs +++ b/edgelet/iotedge/src/check/checks/well_formed_config.rs @@ -2,7 +2,8 @@ use std::fs::File; use failure::{self, Fail}; -use edgelet_docker::Settings; +use edgelet_core::RuntimeSettings; +use edgelet_docker::{Settings, UPSTREAM_PARENT_KEYWORD}; use crate::check::{checker::Checker, Check, CheckResult}; @@ -67,6 +68,15 @@ impl WellFormedConfig { } }; + if let Some(parent_hostname) = settings.parent_hostname() { + if let Some(image_tail) = check + .diagnostics_image_name + .strip_prefix(UPSTREAM_PARENT_KEYWORD) + { + check.diagnostics_image_name = format!("{}{}", parent_hostname, image_tail); + } + } + check.settings = Some(settings); Ok(CheckResult::Ok)