From 35cd939d6d9d6fb973ba485511bf3e79e362728c Mon Sep 17 00:00:00 2001
From: Mathias Laurin <mathias.laurin@checkmk.com>
Date: Fri, 17 Nov 2023 14:44:48 +0100
Subject: [PATCH] check-cert: Improve error handling / early exit

CMK-14683

Change-Id: I974cb8e1997c0a749badbcf3135843f65e729920
---
 packages/check-cert/src/checker.rs | 18 ++++++++-----
 packages/check-cert/src/main.rs    | 42 ++++++++++++------------------
 packages/check-cert/src/output.rs  | 26 ++++++++++++++++++
 3 files changed, 54 insertions(+), 32 deletions(-)

diff --git a/packages/check-cert/src/checker.rs b/packages/check-cert/src/checker.rs
index f1e417a6301..85f809bf2a7 100644
--- a/packages/check-cert/src/checker.rs
+++ b/packages/check-cert/src/checker.rs
@@ -16,9 +16,12 @@ impl<T> LowerLevels<T>
 where
     T: PartialOrd,
 {
-    pub fn warn_crit(warn: T, crit: T) -> Self {
-        std::assert!(warn >= crit);
-        Self { warn, crit }
+    pub fn try_new(warn: T, crit: T) -> Result<Self, Box<dyn std::error::Error>> {
+        if warn >= crit {
+            Ok(Self { warn, crit })
+        } else {
+            Err(Box::from("bad values"))
+        }
     }
 
     pub fn evaluate(&self, value: &T) -> State {
@@ -41,9 +44,12 @@ impl<T> UpperLevels<T>
 where
     T: PartialOrd,
 {
-    pub fn warn_crit(warn: T, crit: T) -> Self {
-        std::assert!(crit >= warn);
-        Self { warn, crit }
+    pub fn try_new(warn: T, crit: T) -> Result<Self, Box<dyn std::error::Error>> {
+        if crit >= warn {
+            Ok(Self { warn, crit })
+        } else {
+            Err(Box::from("bad values"))
+        }
     }
 
     pub fn evaluate(&self, value: &T) -> State {
diff --git a/packages/check-cert/src/main.rs b/packages/check-cert/src/main.rs
index e846f8df2f9..cababb335d1 100644
--- a/packages/check-cert/src/main.rs
+++ b/packages/check-cert/src/main.rs
@@ -61,15 +61,19 @@ struct Args {
 fn main() -> Result<(), Box<dyn std::error::Error>> {
     let args = Args::parse();
 
-    if args.not_after_warn < args.not_after_crit {
-        eprintln!("not after crit limit larger than warn limit");
-        std::process::exit(1);
-    }
-
-    if args.response_time_warn > args.response_time_crit {
-        eprintln!("response time crit limit lower than warn limit");
-        std::process::exit(1);
-    }
+    let Ok(not_after_levels) = checker::LowerLevels::try_new(
+        args.not_after_warn * Duration::DAY,
+        args.not_after_crit * Duration::DAY,
+    ) else {
+        output::Output::bail_out("invalid args: not after crit level larger than warn");
+    };
+
+    let Ok(response_time_levels) = checker::UpperLevels::try_new(
+        args.response_time_warn * Duration::MILLISECOND,
+        args.response_time_crit * Duration::MILLISECOND,
+    ) else {
+        output::Output::bail_out("invalid args: response time crit higher than warn");
+    };
 
     let start = Instant::now();
     let der = fetcher::fetch_server_cert(
@@ -86,13 +90,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
 
     let (_rem, cert) = X509Certificate::from_der(&der)?;
     let out = output::Output::from(vec![
-        checker::check_response_time(
-            response_time,
-            checker::UpperLevels::warn_crit(
-                args.response_time_warn * Duration::MILLISECOND,
-                args.response_time_crit * Duration::MILLISECOND,
-            ),
-        ),
+        checker::check_response_time(response_time, response_time_levels),
         checker::check_details_serial(cert.tbs_certificate.raw_serial_as_string(), args.serial)
             .unwrap_or_default(),
         checker::check_details_subject(cert.tbs_certificate.subject(), args.subject)
@@ -101,18 +99,10 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             .unwrap_or_default(),
         checker::check_validity_not_after(
             cert.tbs_certificate.validity().time_to_expiration(),
-            checker::LowerLevels::warn_crit(
-                args.not_after_warn * Duration::DAY,
-                args.not_after_crit * Duration::DAY,
-            ),
+            not_after_levels,
             cert.tbs_certificate.validity().not_after,
         ),
     ]);
     println!("HTTP {}", out);
-    std::process::exit(match out.state {
-        checker::State::Ok => 0,
-        checker::State::Warn => 1,
-        checker::State::Crit => 2,
-        checker::State::Unknown => 3,
-    })
+    out.bye()
 }
diff --git a/packages/check-cert/src/output.rs b/packages/check-cert/src/output.rs
index 07aee9b7b3a..c5af0fcb0bc 100644
--- a/packages/check-cert/src/output.rs
+++ b/packages/check-cert/src/output.rs
@@ -10,6 +10,23 @@ pub struct Output {
     summary: String,
 }
 
+impl Output {
+    pub fn bye(&self) -> ! {
+        std::process::exit(match self.state {
+            State::Ok => 0,
+            State::Warn => 1,
+            State::Crit => 2,
+            State::Unknown => 3,
+        })
+    }
+
+    pub fn bail_out(message: &str) -> ! {
+        let out = Self::from(CheckResult::unknown(String::from(message)));
+        eprintln!("{}", out);
+        out.bye()
+    }
+}
+
 impl Display for Output {
     fn fmt(&self, f: &mut Formatter) -> FormatResult {
         if self.summary.is_empty() {
@@ -21,6 +38,15 @@ impl Display for Output {
     }
 }
 
+impl From<CheckResult> for Output {
+    fn from(check_result: CheckResult) -> Self {
+        Self {
+            state: check_result.state,
+            summary: check_result.summary,
+        }
+    }
+}
+
 impl From<Vec<CheckResult>> for Output {
     fn from(check_results: Vec<CheckResult>) -> Self {
         Self {