Resolve the config path; print help on a bare invocation #3

2026-09-22T22:35:05+10:00

unkin-agent commented

2026-09-22 22:35:05 +10:00

Puppet calls the binary from generate() as the puppet user, which has no usable HOME, so only the baked-in /opt/certmanager/config.yaml can resolve — but an operator running it by hand wants their own config, and a bare invocation currently errors on the missing positional instead of explaining itself.

resolve --config, then $XDG_CONFIG_HOME, then $HOME/.config, then the baked-in path
take the XDG and home paths only when the file exists
print help and exit zero when called with no arguments
document the resolution order and the positional form in the README

Puppet calls the binary from generate() as the puppet user, which has no usable HOME, so only the baked-in /opt/certmanager/config.yaml can resolve — but an operator running it by hand wants their own config, and a bare invocation currently errors on the missing positional instead of explaining itself. - resolve --config, then $XDG_CONFIG_HOME, then $HOME/.config, then the baked-in path - take the XDG and home paths only when the file exists - print help and exit zero when called with no arguments - document the resolution order and the positional form in the README

unkin-agent added 1 commit 2026-09-22 22:35:05 +10:00

Resolve the config path; print help on a bare invocation

ci/woodpecker/pr/build Pipeline was successful

Details

ci/woodpecker/pr/integration Pipeline was successful

Details

ci/woodpecker/pr/test Pipeline was successful

Details

ci/woodpecker/pr/pre-commit Pipeline was successful

Details

6042499e25

Fall back to $XDG_CONFIG_HOME and $HOME/.config before the baked-in
/opt/certmanager/config.yaml, and stop failing when called with no args.

benvin merged commit f7c8ca5fba into master

2026-09-22 22:41:09 +10:00

benvin deleted branch benvin/legacy-invocation

2026-09-22 22:41:09 +10:00

unkin-agent commented

2026-09-22 22:43:09 +10:00

cmd/certmanager/main.go:63 — a bare invocation now exits 0 with help text on stdout. Under generate(..., combine: true) the exit code is the only failure signal, so any argv that loses the common name turns a loud compile failure into 15 lines of help spliced into the catalog. Gate the help path on stdout being a TTY and exit non-zero otherwise.
internal/config/config.go:42 — the XDG/HOME lookup runs on the Puppet path too; "the puppet user has no usable HOME" is an assumption the code does not enforce. A ~/.config/certmanager/config.yaml under the puppetserver's HOME silently repoints every compile estate-wide, and nothing can report which config was used because stderr is spliced. Consult XDG/HOME only when stdout is a TTY, or behind an explicit opt-in.
cmd/certmanager/main_test.go:779 — run(nil, ...) does not test a bare invocation: SetArgs(nil) makes cobra fall back to os.Args[1:], and it only passes because pflag silently swallows -test.* shorthands. go test ./cmd/certmanager -args foo fails it. Pass []string{}.
test/integration/certmanager_test.go:277 — exec.Command(bin, args...) inherits HOME/XDG_CONFIG_HOME, so the comment's "exactly the argv Puppet uses -- no --config" no longer holds: a developer's ~/.config/certmanager/config.yaml shadows the baked path and the test talks to the wrong Vault. Set cmd.Env with both cleared.
Scope — two independent behaviour changes in one PR. Split into: (1) config resolution (config.go, config_test.go, the two config.Resolve call sites, README Configuration); (2) bare-invocation help (Args, the RunE guard, TestRunNoArgsPrintsHelp, README/AGENTS CLI lines).
nit: internal/config/config.go:43 — a relative XDG_CONFIG_HOME is joined against the cwd instead of being ignored, and under generate() the cwd is puppetserver's. Require filepath.IsAbs(base).
nit: cmd/certmanager/main.go:94 — --config '' used to fail on the empty path and now silently falls through to resolution. Undocumented contract change.
nit: internal/config/config_test.go:215 — Resolve("") == explicit is vacuous (explicit is a tempfile elsewhere); it passes whatever the fallback does. Assert it equals DefaultPath().

- `cmd/certmanager/main.go:63` — a bare invocation now exits 0 with help text on stdout. Under `generate(..., combine: true)` the exit code is the only failure signal, so any argv that loses the common name turns a loud compile failure into 15 lines of help spliced into the catalog. Gate the help path on stdout being a TTY and exit non-zero otherwise. - `internal/config/config.go:42` — the XDG/HOME lookup runs on the Puppet path too; "the puppet user has no usable HOME" is an assumption the code does not enforce. A `~/.config/certmanager/config.yaml` under the puppetserver's HOME silently repoints every compile estate-wide, and nothing can report which config was used because stderr is spliced. Consult XDG/HOME only when stdout is a TTY, or behind an explicit opt-in. - `cmd/certmanager/main_test.go:779` — `run(nil, ...)` does not test a bare invocation: `SetArgs(nil)` makes cobra fall back to `os.Args[1:]`, and it only passes because pflag silently swallows `-test.*` shorthands. `go test ./cmd/certmanager -args foo` fails it. Pass `[]string{}`. - `test/integration/certmanager_test.go:277` — `exec.Command(bin, args...)` inherits `HOME`/`XDG_CONFIG_HOME`, so the comment's "exactly the argv Puppet uses -- no `--config`" no longer holds: a developer's `~/.config/certmanager/config.yaml` shadows the baked path and the test talks to the wrong Vault. Set `cmd.Env` with both cleared. - Scope — two independent behaviour changes in one PR. Split into: (1) config resolution (`config.go`, `config_test.go`, the two `config.Resolve` call sites, README Configuration); (2) bare-invocation help (`Args`, the `RunE` guard, `TestRunNoArgsPrintsHelp`, README/AGENTS CLI lines). - nit: `internal/config/config.go:43` — a relative `XDG_CONFIG_HOME` is joined against the cwd instead of being ignored, and under `generate()` the cwd is puppetserver's. Require `filepath.IsAbs(base)`. - nit: `cmd/certmanager/main.go:94` — `--config ''` used to fail on the empty path and now silently falls through to resolution. Undocumented contract change. - nit: `internal/config/config_test.go:215` — `Resolve("") == explicit` is vacuous (`explicit` is a tempfile elsewhere); it passes whatever the fallback does. Assert it equals `DefaultPath()`.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/certmanager#3