Merge pull request #58 from dvdsk/print-local-ip

dvdsk · web-flow · commit 912ea966d023 · 2023-06-11T19:36:07.000+02:00
adds local machine ip as note to some errors
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.3.1] - 2023-06-11
+
+### Added
+ - Note the current machines local IP when the challenge server is not reachable
+
 ## [0.3.0] - 2023-06-11
 
 ### Added
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/main/Cargo.toml b/main/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "renewc"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["David Kleingeld <opensource@davidsk.dev>"]
 edition = "2021"
 rust-version = "1.70"
@@ -43,6 +43,7 @@ async-trait = "0.1"
 data-encoding = "2.4"
 pem = "2"
 strum = { version = "0.24", features = ["derive"] }
+local-ip-address = "0.5.3"
 
 [dev-dependencies]
 libc = "0.2"
diff --git a/main/src/config.rs b/main/src/config.rs
@@ -118,7 +118,7 @@ pub struct Config {
     pub(crate) domains: Vec<String>,
     pub(crate) email: Vec<String>,
     pub production: bool,
-    pub(crate) port: u16,
+    pub port: u16,
     pub output_config: OutputConfig,
     pub reload: Option<String>,
     pub(crate) renew_early: bool,
diff --git a/main/src/diagnostics/reachable.rs b/main/src/diagnostics/reachable.rs
@@ -4,7 +4,7 @@ use std::time::Duration;
 use color_eyre::{eyre, Help};
 use hyper::{body, Body, Response, StatusCode, Uri};
 use tokio::time::timeout;
-use tracing::debug;
+use tracing::{debug, instrument};
 
 use crate::config::Config;
 use crate::renew::server::Http01Challenge;
@@ -21,14 +21,15 @@ async fn check_response(resp: Response<Body>, key_auth: &str, domain: &str) -> e
         StatusCode::SERVICE_UNAVAILABLE | StatusCode::NOT_FOUND => {
             Err(eyre::eyre!(
                 "Could not reach {APP} via {domain}"
-            )
+            ))
             .note("Another server is getting traffic for external port 80")
-            .suggestion(format!("Check if port 80 is forwarded to a port on this machine. If it is configure {APP} to use that port with the `--port` option. If not forward port 80 to this machine")))
+            .suggestion(format!("Check if port 80 is forwarded to a port on this machine. If it is configure {APP} to use that port with the `--port` option. If not forward port 80 to this machine")).with_local_ip_note()
         }
         _ => unreachable!("got incorrect status code: {resp:?}"),
     }
 }
 
+#[instrument(ret, skip(key_auth, path))]
 async fn check(path: &str, domain: &str, key_auth: &str) -> eyre::Result<()> {
     let url = format!("http://{domain}{path}");
     debug!("checking: {url}");
@@ -40,9 +41,11 @@ async fn check(path: &str, domain: &str, key_auth: &str) -> eyre::Result<()> {
         Ok(Err(e)) if e.is_timeout() || e.is_connect() => {
             Err(eyre::eyre!("Could not reach {APP} via {domain}"))
                 .suggestion("Forward port 80 to this machine")
+                .with_local_ip_note()
         }
         Err(_) => Err(eyre::eyre!("Could not reach {APP} via {domain}"))
-            .suggestion("Forward port 80 to this machine"),
+            .suggestion("Forward port 80 to this machine")
+            .with_local_ip_note(),
         Ok(Err(e)) => unreachable!("reqwest error: {e:?}"),
     }
 }
@@ -62,3 +65,18 @@ pub async fn server(config: &Config, challanges: &[Http01Challenge]) -> eyre::Re
 
     Ok(())
 }
+
+trait WithLocalIp {
+    fn with_local_ip_note(self) -> Self;
+}
+
+impl<T> WithLocalIp for eyre::Result<T> {
+    fn with_local_ip_note(self) -> Self {
+        match local_ip_address::local_ip() {
+            Ok(ip) => self.with_note(|| format!("This machines local IP adress: {ip:?}")),
+            Err(e) => self.with_warning(|| {
+                format!("Failed to be helpfull and find this machines local IP error: {e:?}")
+            }),
+        }
+    }
+}
diff --git a/main/src/main.rs b/main/src/main.rs
@@ -38,7 +38,8 @@ async fn main() -> eyre::Result<()> {
     match cli.command {
         Commands::Run(args) => {
             let config = Config::try_from(args)?;
-            let Some(certs): Option<Signed<pem::Pem>> = run(&mut InstantAcme {}, &mut stdout, &config, debug).await? else {
+            let Some(certs): Option<Signed<pem::Pem>> = 
+            run(&mut InstantAcme {}, &mut stdout, &config, debug).await? else {
                 return Ok(());
             };
             cert::store::on_disk(&config, certs, &mut stdout)
diff --git a/main/tests/behaviour.rs b/main/tests/behaviour.rs
@@ -10,7 +10,6 @@ use shared::TestAcme;
 use shared::TestPrinter;
 use tracing::info;
 
-
 #[tokio::test]
 async fn production_does_not_overwrite_valid_production() {
     shared::setup_color_eyre();
diff --git a/main/tests/diagnostics.rs b/main/tests/diagnostics.rs
@@ -54,3 +54,23 @@ async fn insufficent_permissions() {
     assert!(test.contains("You normally need sudo to attach to ports below 1025"));
     assert!(test.contains("port: 42"));
 }
+
+#[tokio::test]
+async fn port_forward_suggestion_includes_ip() {
+    shared::setup_color_eyre();
+    shared::setup_tracing();
+
+    let dir = tempfile::tempdir().unwrap();
+    // port 1119 is assigned to a use by the IANA 
+    // and should not route to the current machine
+    let config = Config::test(1119, &dir.path());
+    let err = run::<Pem>(&mut InstantAcme {}, &mut TestPrinter, &config, true)
+        .await
+        .unwrap_err();
+
+    let test = format!("{err:?}");
+    assert!(
+        test.contains("This machines local IP adress:"),
+        "\n\n***********error was:\n\n {test}\n\n************\n"
+    );
+}
diff --git a/main/tests/shared/gen_cert.rs b/main/tests/shared/gen_cert.rs
@@ -23,7 +23,7 @@ fn ca_cert(is_staging: bool) -> Certificate {
 pub fn client_cert(valid_till: OffsetDateTime) -> Certificate {
     let subject_alt_names = vec!["example.org".to_string()];
     let mut params = CertificateParams::new(subject_alt_names);
-    params.not_after = dbg!(valid_till);
+    params.not_after = valid_till;
     Certificate::from_params(params).unwrap()
 }
 

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ fn ca_cert(is_staging: bool) -> Certificate {`
`23`	`23`	`pub fn client_cert(valid_till: OffsetDateTime) -> Certificate {`
`24`	`24`	`let subject_alt_names = vec!["example.org".to_string()];`
`25`	`25`	`let mut params = CertificateParams::new(subject_alt_names);`
`26`		`- params.not_after = dbg!(valid_till);`
	`26`	`+ params.not_after = valid_till;`
`27`	`27`	`Certificate::from_params(params).unwrap()`
`28`	`28`	`}`
`29`	`29`