Skip to content

refactor: simplify --exit-code from integer to boolean flag #140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jbovet wants to merge 2 commits into
base: main
Choose a base branch
from
Open

refactor: simplify --exit-code from integer to boolean flag #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
421fb59
refactor: simplify --exit-code from integer to boolean flag
jbovet Feb 8, 2026
e49cae6
Merge remote-tracking branch 'origin/main' into fix-exit-code
jbovet Feb 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
//! ```toml
//! hosts = ["example.com", "example.com:8443"]
//! output = "summary"
//! exit_code = 1
//! exit_code = true
//! check_revocation = true
//!
//! [prometheus]
Expand All @@ -37,8 +37,8 @@ pub struct Config {
pub hosts: Option<Vec<String>>,
/// Output format: json, text, summary
pub output: Option<String>,
/// Exit code to use when certificates are expired/revoked
pub exit_code: Option<i32>,
/// Enable non-zero exit code when certificate issues are found
pub exit_code: Option<bool>,
/// Enable certificate revocation checking
pub check_revocation: Option<bool>,
/// Prometheus configuration
Expand Down Expand Up @@ -97,7 +97,7 @@ impl Config {
///
/// - `hosts`: None (must be provided)
/// - `output`: "summary"
/// - `exit_code`: 0 (don't fail on expired certificates)
/// - `exit_code`: false (don't fail on certificate issues)
/// - `check_revocation`: false
/// - `prometheus.enabled`: false
/// - `prometheus.address`: "http://localhost:9091"
Expand All @@ -109,7 +109,7 @@ impl Config {
Config {
hosts: None,
output: Some("summary".to_string()),
exit_code: Some(0),
exit_code: Some(false),
check_revocation: Some(false),
prometheus: Some(PrometheusConfig {
enabled: Some(false),
Expand Down Expand Up @@ -185,7 +185,7 @@ impl Config {
///
/// * `addresses` - List of hosts to check
/// * `output` - Output format (json, text, summary)
/// * `exit_code` - Exit code for failures
/// * `exit_code` - Enable exit code on certificate issues
/// * `prometheus` - Enable Prometheus metrics
/// * `prometheus_address` - Prometheus push gateway address
/// * `check_revocation` - Enable certificate revocation checking
Expand All @@ -199,7 +199,7 @@ impl Config {
pub fn from_cli_args(
addresses: Option<Vec<String>>,
output: Option<String>,
exit_code: Option<i32>,
exit_code: Option<bool>,
prometheus: Option<bool>,
prometheus_address: Option<String>,
check_revocation: Option<bool>,
Expand Down Expand Up @@ -246,7 +246,7 @@ impl Config {
"expired.badssl.com".to_string(),
]),
output: Some("summary".to_string()),
exit_code: Some(1),
exit_code: Some(true),
check_revocation: Some(true),
prometheus: Some(PrometheusConfig {
enabled: Some(true),
Expand Down Expand Up @@ -295,7 +295,7 @@ mod tests {
let toml_content = r#"
hosts = ["jpbd.dev", "google.cl"]
output = "json"
exit_code = 1
exit_code = true
check_revocation = true

[prometheus]
Expand All @@ -313,7 +313,7 @@ mod tests {
Some(vec!["jpbd.dev".to_string(), "google.cl".to_string()])
);
assert_eq!(config.output, Some("json".to_string()));
assert_eq!(config.exit_code, Some(1));
assert_eq!(config.exit_code, Some(true));
assert_eq!(config.check_revocation, Some(true));

let prometheus = config.prometheus.unwrap();
Expand All @@ -329,7 +329,7 @@ mod tests {
let base_config = Config {
hosts: Some(vec!["base.com".to_string()]),
output: Some("text".to_string()),
exit_code: Some(0),
exit_code: Some(false),
check_revocation: Some(false),
prometheus: Some(PrometheusConfig {
enabled: Some(false),
Expand All @@ -342,7 +342,7 @@ mod tests {
let override_config = Config {
hosts: Some(vec!["override.com".to_string()]),
output: None,
exit_code: Some(1),
exit_code: Some(true),
check_revocation: Some(true),
prometheus: Some(PrometheusConfig {
enabled: Some(true),
Expand All @@ -357,7 +357,7 @@ mod tests {
// Override config should take precedence where specified
assert_eq!(merged.hosts, Some(vec!["override.com".to_string()]));
assert_eq!(merged.output, Some("text".to_string())); // From base (not overridden)
assert_eq!(merged.exit_code, Some(1)); // Overridden
assert_eq!(merged.exit_code, Some(true)); // Overridden
assert_eq!(merged.check_revocation, Some(true)); // Overridden

let prometheus = merged.prometheus.unwrap();
Expand All @@ -372,7 +372,7 @@ mod tests {

assert_eq!(config.hosts, None);
assert_eq!(config.output, Some("summary".to_string()));
assert_eq!(config.exit_code, Some(0));
assert_eq!(config.exit_code, Some(false));
assert_eq!(config.check_revocation, Some(false));

let prometheus = config.prometheus.unwrap();
Expand All @@ -388,7 +388,7 @@ mod tests {
let config = Config::from_cli_args(
Some(vec!["cli.com".to_string()]),
Some("json".to_string()),
Some(2),
Some(true),
Some(true),
Some("http://cli:9091".to_string()),
Some(true),
Expand All @@ -398,7 +398,7 @@ mod tests {

assert_eq!(config.hosts, Some(vec!["cli.com".to_string()]));
assert_eq!(config.output, Some("json".to_string()));
assert_eq!(config.exit_code, Some(2));
assert_eq!(config.exit_code, Some(true));
assert_eq!(config.check_revocation, Some(true));
assert_eq!(config.grade, Some(true));
assert_eq!(config.min_validity, Some(45));
Expand Down Expand Up @@ -476,7 +476,7 @@ mod tests {
let base = Config {
hosts: Some(vec!["base.com".to_string()]),
output: Some("json".to_string()),
exit_code: Some(1),
exit_code: Some(true),
check_revocation: Some(true),
prometheus: Some(PrometheusConfig {
enabled: Some(true),
Expand All @@ -497,7 +497,7 @@ mod tests {
let merged = base.merge_with(empty_override);
assert_eq!(merged.hosts, Some(vec!["base.com".to_string()]));
assert_eq!(merged.output, Some("json".to_string()));
assert_eq!(merged.exit_code, Some(1));
assert_eq!(merged.exit_code, Some(true));
assert_eq!(merged.check_revocation, Some(true));
assert_eq!(merged.grade, Some(true));
assert_eq!(merged.min_validity, Some(60));
Expand Down
44 changes: 23 additions & 21 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@ struct Args {
#[arg(short, value_enum)]
output: Option<OutFormat>,

/// Exit with this code when expired or revoked certificates are found.
/// Exit with code 1 when certificate issues are found.
///
/// Defaults to 0 (always succeed). Set to 1 for CI/CD pipelines
/// that should fail on certificate issues.
#[arg(long)]
exit_code: Option<i32>,
/// Issues include: expired certificates, revoked certificates,
/// or certificates below the --min-validity threshold.
/// Disabled by default (always exits 0).
#[arg(long, default_value_t = false)]
exit_code: bool,
Comment on lines +48 to +49
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While changing exit_code to a boolean is a good simplification, making it a simple flag (bool) removes the ability to disable it from the command line if it's enabled in a configuration file. The previous Option<i32> allowed this with --exit-code 0.

To maintain this flexibility and for consistency with other arguments like prometheus, I suggest using Option<bool>. This would allow --exit-code true and --exit-code false, providing clear override capability from the command line.

With this change, you can then simplify the call in load_config.

Suggested change
#[arg(long, default_value_t = false)]
exit_code: bool,
#[arg(long)]
exit_code: Option<bool>,


/// Push certificate metrics to a Prometheus Push Gateway
#[arg(long)]
Expand Down Expand Up @@ -470,7 +471,7 @@ struct HostPort {
struct FinalConfig {
addresses: Vec<String>,
output: OutFormat,
exit_code: i32,
exit_code: bool,
prometheus: bool,
prometheus_address: String,
check_revocation: bool,
Expand Down Expand Up @@ -505,7 +506,7 @@ impl FinalConfig {
Ok(FinalConfig {
addresses,
output,
exit_code: config.exit_code.unwrap_or(0),
exit_code: config.exit_code.unwrap_or(false),
prometheus: prometheus_config.enabled.unwrap_or(false),
prometheus_address: prometheus_config
.address
Expand Down Expand Up @@ -710,7 +711,7 @@ fn load_config(cli: &Args) -> Result<FinalConfig, ConfigError> {
let cli_config = Config::from_cli_args(
cli_addresses,
cli.output.as_ref().map(|o| o.to_string()),
cli.exit_code,
if cli.exit_code { Some(true) } else { None },
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If you change exit_code in Args to Option<bool> as suggested in my other comment, you can simplify this to pass cli.exit_code directly. This avoids the conditional logic and correctly handles None when the argument is not provided, preserving the configuration file precedence.

Suggested change
if cli.exit_code { Some(true) } else { None },
cli.exit_code,

cli.prometheus,
cli.prometheus_address.clone(),
cli.check_revocation,
Expand All @@ -724,24 +725,25 @@ fn load_config(cli: &Args) -> Result<FinalConfig, ConfigError> {
FinalConfig::from_merged_config(config)
}

/// Exits the program with the appropriate exit code.
/// Exits the program with code 1 when certificate issues are detected.
///
/// This function implements conditional exit behavior based on whether
/// any certificates failed validation (expired or revoked).
/// any certificates failed validation (expired, revoked, or below
/// the minimum validity threshold).
///
/// # Arguments
///
/// * `exit_code` - The exit code to use when failures are detected
/// * `exit_code` - Whether to enable non-zero exit codes on failure
/// * `failed_result` - Whether any certificate checks failed
///
/// # Behavior
///
/// - If `failed_result` is `true` and `exit_code` is non-zero, exits with `exit_code`
/// - If `exit_code` is `true` and `failed_result` is `true`, exits with code 1
/// - Otherwise, exits normally with code 0
/// - Useful for CI/CD pipelines where exit code indicates build success/failure
fn exit(exit_code: i32, failed_result: bool) {
if exit_code != 0 && failed_result {
std::process::exit(exit_code)
fn exit(exit_code: bool, failed_result: bool) {
if exit_code && failed_result {
std::process::exit(1)
}
}

Expand Down Expand Up @@ -939,7 +941,7 @@ mod tests {
let config = Config {
hosts: None,
output: Some("summary".to_string()),
exit_code: Some(0),
exit_code: Some(false),
check_revocation: Some(false),
prometheus: None,
grade: Some(false),
Expand All @@ -954,7 +956,7 @@ mod tests {
let config = Config {
hosts: Some(vec![]),
output: Some("summary".to_string()),
exit_code: Some(0),
exit_code: Some(false),
check_revocation: Some(false),
prometheus: None,
grade: Some(false),
Expand All @@ -969,7 +971,7 @@ mod tests {
let config = Config {
hosts: Some(vec!["example.com".to_string()]),
output: Some("xml".to_string()),
exit_code: Some(0),
exit_code: Some(false),
check_revocation: Some(false),
prometheus: None,
grade: Some(false),
Expand All @@ -992,7 +994,7 @@ mod tests {
};
let final_config = FinalConfig::from_merged_config(config).unwrap();
assert_eq!(final_config.output, OutFormat::Summary);
assert_eq!(final_config.exit_code, 0);
assert!(!final_config.exit_code);
assert!(!final_config.check_revocation);
assert!(!final_config.prometheus);
assert!(!final_config.grade);
Expand All @@ -1005,7 +1007,7 @@ mod tests {
let config = Config {
hosts: Some(vec!["a.com".to_string(), "b.com".to_string()]),
output: Some("json".to_string()),
exit_code: Some(2),
exit_code: Some(true),
check_revocation: Some(true),
prometheus: Some(config::PrometheusConfig {
enabled: Some(true),
Expand All @@ -1017,7 +1019,7 @@ mod tests {
let final_config = FinalConfig::from_merged_config(config).unwrap();
assert_eq!(final_config.addresses, vec!["a.com", "b.com"]);
assert_eq!(final_config.output, OutFormat::Json);
assert_eq!(final_config.exit_code, 2);
assert!(final_config.exit_code);
assert!(final_config.check_revocation);
assert!(final_config.prometheus);
assert_eq!(final_config.prometheus_address, "http://prom:9091");
Expand Down