Dstack-TEE / Dstack-TEE/dstack

certbot: `renew --once` exits 0 when the renewed hook fails

Open
#1,012 0 comments 0 reactions 0 assignees View on GitHub
bug rust
Dominant language
Rust
Stars
544
Forks
96
Avg merge
17h 57m
Merged PRs (30d)
117

Description

Follow-up to #924.

`certbot renew --once` is the cron/one-shot entry point. #924 made it invoke the `renewed_hook` (correct — a one-shot renewal that skips the reload hook leaves the serving process on the old cert), but hook failures are only logged:

```rust
// dstack/certbot/src/bot.rs
match std::process::Command::new("/bin/sh").arg("-c").arg(hook).status() {
Ok(status) if status.success() => {}
Ok(status) => error!("renewed hook failed with status: {status}"),
Err(error) => error!("failed to run renewed hook: {error:?}"),
}
Ok(true)
```

So `renew_and_run_hook` returns `Ok(true)` regardless, and `certbot renew --once` exits 0 even when the hook never ran or exited non-zero. A cron job or systemd `OneShot` unit wrapping this sees success while the certificate on disk is new and the serving process is still holding the old one — exactly the failure that is supposed to be visible.

Swallowing the error is right for the daemon (the next interval retries), wrong for `--once` (there is no next interval).

## Proposal

Split the two semantics. Options, roughly in order of preference:

1. Have `renew_and_run_hook` return the hook outcome (e.g. `Result` carrying `hook_failed`) and let the `--once` path in `cli/src/main.rs` turn a hook failure into a non-zero exit, while `run()` keeps logging and continuing.
2. Add a `fail_on_hook_error: bool` parameter, set from the `once` flag.

Either way the daemon loop must keep its current behaviour: a failing hook should not abort the loop.

Contributor guide

Open the contributing guide

Research direction

Start in dstack/certbot/src/bot.rs, tracing how renew_and_run_hook reports renewed-hook failures, then follow the --once path in cli/src/main.rs. Preserve the daemon loop's current logging-and-continuing behavior, but make --once return a non-zero exit when the hook does not run or exits unsuccessfully. Verify both semantics with the relevant certbot command paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
64/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.