Resolve the config path; print help on a bare invocation

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.

AGENTS.md

@@ -46,8 +46,9 @@ dist/                         # build output: binaries, RPM, checksums (not comm
   wires it to the real streams and `os.Exit`. Cobra runs with `SilenceUsage` and
   `SilenceErrors` so nothing reaches stderr unless the run failed.
 - **Flat CLI**: the root command takes `common_name` positionally
-  (`cobra.ExactArgs(1)`). `ca`, `completion` and `help` are the only
-  subcommands; `--config` is persistent so `ca` shares it.
+  (`cobra.MaximumNArgs(1)`); no args prints help, args-less flags fail. `ca`,
+  `completion` and `help` are the only subcommands; `--config` is persistent so
+  `ca` shares it.
 - **`ca` is unauthenticated**: `GET {addr}/v1/{mount}/ca_chain` sends no
   `X-Vault-Token` and loads the config through `config.LoadCA`, which requires
   only `vault.addr`. The body is parsed as PEM, never sniffed — Vault labels it
@@ -59,8 +60,13 @@ dist/                         # build output: binaries, RPM, checksums (not comm
 - **`vault.New`** authenticates once and never falls back between auth methods.
   Every request is bounded by `vault.timeout` (default 30s) — the Python set no
   timeout, so a black-holed Vault hung the whole catalog compile.
-- **`config.defaultPath`** is baked in at build time via
-  `-X ...internal/config.defaultPath=` (`make build CONFIG_PATH=...`).
+- **`config.Resolve`** picks the config: `--config`, then
+  `$XDG_CONFIG_HOME/certmanager/config.yaml`, then
+  `$HOME/.config/certmanager/config.yaml` (both only if the file exists), then
+  `config.FallbackPath()` — baked in at build time via
+  `-X ...internal/config.defaultPath=` (`make build CONFIG_PATH=...`). Under
+  `generate()` the puppet user has no usable home, so the baked path is what
+  resolves.
 
 ## Deliberate deviations from the Python script
 

README.md

@@ -21,8 +21,13 @@ failure path exits non-zero.
 ```
 certmanager <common_name> [flags]   # issue a certificate
 certmanager ca [flags]              # print the CA chain
+certmanager                         # print help
 ```
 
+The common name is positional, with no `pki` subcommand in front of it — that
+is the form Puppet calls, and it is the only issuing form. `ca`, `completion`
+and `help` are the subcommands; every other first argument is a common name.
+
 | Flag | Short | Type | Default | Description |
 |---|---|---|---|---|
 | `--alt-names` | `-a` | comma list | `""` | DNS subject alternative names |
@@ -30,7 +35,7 @@ certmanager ca [flags]              # print the CA chain
 | `--expiry-days` | `-e` | int | `365` | Certificate validity in days (`ttl: <n>d`) |
 | `--compress` | `-c` | bool | `false` | Write a zip instead of a directory |
 | `--json` | | bool | `false` | Print the certificate data as JSON |
-| `--config` | | path | `/opt/certmanager/config.yaml` | Config file location |
+| `--config` | | path | resolved (see below) | Config file location |
 
 `ca` takes `--config` plus:
 
@@ -101,8 +106,21 @@ stdout.
 
 ## Configuration
 
-YAML, everything nested under `vault:`. The path is baked in at build time
-(`make build CONFIG_PATH=...`) and overridable per-invocation with `--config`.
+YAML, everything nested under `vault:`.
+
+### Config resolution order
+
+1. `--config <path>`, which always wins;
+2. `$XDG_CONFIG_HOME/certmanager/config.yaml`, if the file exists;
+3. `$HOME/.config/certmanager/config.yaml`, if the file exists;
+4. `/opt/certmanager/config.yaml`, the path baked in at build time
+   (`make build CONFIG_PATH=...`).
+
+Steps 2 and 3 are skipped unless the file is really there, so the last step is
+what Puppet gets: `generate()` runs the binary as the `puppet` user with no
+usable home directory, and `profiles::helpers::certmanager` writes the config
+to `/opt/certmanager/config.yaml` on managed nodes. The k8s puppetserver
+compilers mount it at the same path.
 
 | Key | Required | Default | Notes |
 |---|---|---|---|

cmd/certmanager/main.go

@@ -52,11 +52,20 @@ func rootCmd() *cobra.Command {
 		Use:           "certmanager <common_name>",
 		Short:         "Issue a PKI certificate from HashiCorp Vault",
 		Version:       version,
-		Args:          cobra.ExactArgs(1),
+		Args:          cobra.MaximumNArgs(1),
 		SilenceUsage:  true,
 		SilenceErrors: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			cfg, err := config.Load(cfgFile)
+			if len(args) == 0 {
+				// A bare invocation is someone asking what this is; flags
+				// without a common name is a malformed call and must fail.
+				if cmd.Flags().NFlag() == 0 {
+					return cmd.Help()
+				}
+				return fmt.Errorf("a common name is required")
+			}
+
+			cfg, err := config.Load(config.Resolve(cfgFile))
 			if err != nil {
 				return err
 			}
@@ -79,9 +88,10 @@ func rootCmd() *cobra.Command {
 		},
 	}
 
-	// Persistent so the ca subcommand shares it; the positional form keeps
-	// parsing --config exactly as before.
-	cmd.PersistentFlags().StringVar(&cfgFile, "config", config.DefaultPath(), "path to config.yaml")
+	// Persistent so the ca subcommand shares it; empty means "resolve", which
+	// keeps the resolution order out of the usage text and out of --help.
+	cmd.PersistentFlags().StringVar(&cfgFile, "config", "",
+		"path to config.yaml (default: $XDG_CONFIG_HOME or ~/.config, else "+config.FallbackPath()+")")
 	cmd.Flags().StringVarP(&altNames, "alt-names", "a", "", "comma-separated DNS subject alternative names")
 	cmd.Flags().StringVarP(&ipSans, "ip-sans", "i", "", "comma-separated IP subject alternative names")
 	cmd.Flags().IntVarP(&expiryDays, "expiry-days", "e", 365, "certificate validity in days")
@@ -106,7 +116,7 @@ func caCmd(cfgFile *string) *cobra.Command {
 		SilenceUsage:  true,
 		SilenceErrors: true,
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			cfg, err := config.LoadCA(*cfgFile)
+			cfg, err := config.LoadCA(config.Resolve(*cfgFile))
 			if err != nil {
 				return err
 			}

cmd/certmanager/main_test.go

@@ -21,6 +21,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"git.unkin.net/unkin/certmanager/internal/config"
 )
 
 const (
@@ -769,3 +771,89 @@ func TestRunCAFailures(t *testing.T) {
 		})
 	}
 }
+
+// A bare invocation is a human at a terminal, not Puppet: print help and exit
+// zero rather than failing on the missing positional.
+func TestRunNoArgsPrintsHelp(t *testing.T) {
+	var stdout, stderr bytes.Buffer
+	if code := run(nil, &stdout, &stderr); code != 0 {
+		t.Fatalf("exit code = %d, stderr = %q", code, stderr.String())
+	}
+	if !strings.Contains(stdout.String(), "Usage:") {
+		t.Errorf("stdout = %q, want the help text", stdout.String())
+	}
+	if !strings.Contains(stdout.String(), "certmanager <common_name>") {
+		t.Errorf("help does not document the positional form: %q", stdout.String())
+	}
+	if stderr.Len() != 0 {
+		t.Errorf("stderr = %q, want empty", stderr.String())
+	}
+}
+
+// seedXDGConfig writes a config where DefaultPath() looks first.
+func seedXDGConfig(t *testing.T, addr string) {
+	t.Helper()
+	xdg := t.TempDir()
+	dir := filepath.Join(xdg, "certmanager")
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	body := "vault:\n  addr: " + addr + "\n  auth_method: approle\n" +
+		"  approle_path: approle\n  role_id: test-role-id\n" +
+		"  mount_point: pki_int\n  role_name: servers_default\n"
+	if err := os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(body), 0o600); err != nil {
+		t.Fatalf("write config: %v", err)
+	}
+	t.Setenv("XDG_CONFIG_HOME", xdg)
+	t.Setenv("HOME", t.TempDir())
+}
+
+// Puppet passes no --config, so the resolved path has to be used.
+func TestRunResolvesConfigWithoutFlag(t *testing.T) {
+	srv, got := newVaultStub(t, stubOptions{})
+	seedXDGConfig(t, srv.URL)
+
+	var stdout, stderr bytes.Buffer
+	code := run([]string{"host.example.com", "--alt-names", "host", "--expiry-days", "90", "--json"}, &stdout, &stderr)
+	if code != 0 {
+		t.Fatalf("exit code = %d, stderr = %q", code, stderr.String())
+	}
+	if got.issueBody["common_name"] != "host.example.com" {
+		t.Errorf("common_name = %v", got.issueBody["common_name"])
+	}
+	if stderr.Len() != 0 {
+		t.Errorf("stderr = %q, want empty", stderr.String())
+	}
+}
+
+// The ca subcommand shares the same resolution.
+func TestRunCAResolvesConfigWithoutFlag(t *testing.T) {
+	srv, got := newVaultStub(t, stubOptions{})
+	seedXDGConfig(t, srv.URL)
+
+	var stdout, stderr bytes.Buffer
+	if code := run([]string{"ca"}, &stdout, &stderr); code != 0 {
+		t.Fatalf("exit code = %d, stderr = %q", code, stderr.String())
+	}
+	if !got.caRequested {
+		t.Error("the CA chain endpoint was never called")
+	}
+}
+
+// With no config anywhere the run fails naming the baked-in path, not a
+// home-relative one that could never exist under generate().
+func TestRunWithoutAnyConfigNamesTheBakedPath(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	t.Setenv("HOME", t.TempDir())
+
+	var stdout, stderr bytes.Buffer
+	if code := run([]string{"host.example.com", "--json"}, &stdout, &stderr); code == 0 {
+		t.Fatal("exit code = 0, want non-zero")
+	}
+	if stdout.Len() != 0 {
+		t.Errorf("stdout = %q, want empty", stdout.String())
+	}
+	if !strings.Contains(stderr.String(), config.FallbackPath()) {
+		t.Errorf("stderr = %q, want it to name %q", stderr.String(), config.FallbackPath())
+	}
+}

internal/config/config.go

@@ -3,6 +3,7 @@ package config
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"time"
 
 	"gopkg.in/yaml.v3"
@@ -28,8 +29,42 @@ const DefaultTimeout = 30 * time.Second
 // -X git.unkin.net/unkin/certmanager/internal/config.defaultPath=/some/path
 var defaultPath = "/opt/certmanager/config.yaml"
 
-// DefaultPath returns the config path baked in at build time.
-func DefaultPath() string { return defaultPath }
+// FallbackPath returns the config path baked in at build time. Puppet writes
+// the config there, and it is the only path that resolves under generate(),
+// which runs as the puppet user with no usable home directory.
+func FallbackPath() string { return defaultPath }
+
+// DefaultPath resolves the config to use when --config was not given:
+// $XDG_CONFIG_HOME/certmanager/config.yaml, then
+// $HOME/.config/certmanager/config.yaml, then FallbackPath(). The first two
+// are only taken when the file is actually there, so an unset or unusable
+// HOME lands on the Puppet-managed path instead of a path that cannot exist.
+func DefaultPath() string {
+	if base := os.Getenv("XDG_CONFIG_HOME"); base != "" {
+		if p := filepath.Join(base, "certmanager", "config.yaml"); isFile(p) {
+			return p
+		}
+	}
+	if home, err := os.UserHomeDir(); err == nil && home != "" {
+		if p := filepath.Join(home, ".config", "certmanager", "config.yaml"); isFile(p) {
+			return p
+		}
+	}
+	return defaultPath
+}
+
+// Resolve picks the config path for a run: an explicit --config always wins.
+func Resolve(explicit string) string {
+	if explicit != "" {
+		return explicit
+	}
+	return DefaultPath()
+}
+
+func isFile(path string) bool {
+	info, err := os.Stat(path)
+	return err == nil && info.Mode().IsRegular()
+}
 
 // Config is the top-level configuration structure.
 type Config struct {

internal/config/config_test.go

@@ -120,9 +120,100 @@ func TestLoadMissingFile(t *testing.T) {
 	}
 }
 
-func TestDefaultPath(t *testing.T) {
-	if got := DefaultPath(); got != "/opt/certmanager/config.yaml" {
-		t.Errorf("DefaultPath() = %q, want the Puppet-managed path", got)
+func TestFallbackPath(t *testing.T) {
+	if got := FallbackPath(); got != "/opt/certmanager/config.yaml" {
+		t.Errorf("FallbackPath() = %q, want the Puppet-managed path", got)
+	}
+}
+
+// seedConfig drops a config file at dir/certmanager/config.yaml and returns it.
+func seedConfig(t *testing.T, dir string) string {
+	t.Helper()
+	path := filepath.Join(dir, "certmanager", "config.yaml")
+	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	if err := os.WriteFile(path, []byte(approleBody), 0o600); err != nil {
+		t.Fatalf("write: %v", err)
+	}
+	return path
+}
+
+func TestDefaultPathPrefersXDG(t *testing.T) {
+	xdg := t.TempDir()
+	home := t.TempDir()
+	want := seedConfig(t, xdg)
+	seedConfig(t, filepath.Join(home, ".config"))
+
+	t.Setenv("XDG_CONFIG_HOME", xdg)
+	t.Setenv("HOME", home)
+
+	if got := DefaultPath(); got != want {
+		t.Errorf("DefaultPath() = %q, want the XDG config %q", got, want)
+	}
+}
+
+func TestDefaultPathFallsBackToHome(t *testing.T) {
+	home := t.TempDir()
+	want := seedConfig(t, filepath.Join(home, ".config"))
+
+	t.Setenv("XDG_CONFIG_HOME", "")
+	t.Setenv("HOME", home)
+
+	if got := DefaultPath(); got != want {
+		t.Errorf("DefaultPath() = %q, want the home config %q", got, want)
+	}
+}
+
+// XDG pointing somewhere without a config must not shadow a usable home one.
+func TestDefaultPathSkipsXDGWithoutAConfig(t *testing.T) {
+	home := t.TempDir()
+	want := seedConfig(t, filepath.Join(home, ".config"))
+
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	t.Setenv("HOME", home)
+
+	if got := DefaultPath(); got != want {
+		t.Errorf("DefaultPath() = %q, want the home config %q", got, want)
+	}
+}
+
+// What generate() sees: the puppet user has no usable home, so neither lookup
+// yields a file and the baked-in path has to win.
+func TestDefaultPathFallsBackToBakedPath(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", "")
+	t.Setenv("HOME", "")
+
+	if got := DefaultPath(); got != FallbackPath() {
+		t.Errorf("DefaultPath() = %q, want %q", got, FallbackPath())
+	}
+}
+
+func TestDefaultPathIgnoresADirectory(t *testing.T) {
+	xdg := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(xdg, "certmanager", "config.yaml"), 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	t.Setenv("XDG_CONFIG_HOME", xdg)
+	t.Setenv("HOME", t.TempDir())
+
+	if got := DefaultPath(); got != FallbackPath() {
+		t.Errorf("DefaultPath() = %q, want %q", got, FallbackPath())
+	}
+}
+
+func TestResolvePrefersExplicitPath(t *testing.T) {
+	xdg := t.TempDir()
+	seedConfig(t, xdg)
+	t.Setenv("XDG_CONFIG_HOME", xdg)
+	t.Setenv("HOME", t.TempDir())
+
+	explicit := writeConfig(t, approleBody)
+	if got := Resolve(explicit); got != explicit {
+		t.Errorf("Resolve(%q) = %q, want the explicit path", explicit, got)
+	}
+	if got := Resolve(""); got == explicit {
+		t.Error("Resolve(\"\") returned the explicit path")
 	}
 }