diff --git a/AGENTS.md b/AGENTS.md index 6e4ef64..079f3a8 100644 --- a/AGENTS.md +++ b/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 diff --git a/README.md b/README.md index fe364aa..b9118fa 100644 --- a/README.md +++ b/README.md @@ -21,8 +21,13 @@ failure path exits non-zero. ``` certmanager [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: 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 `, 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 | |---|---|---|---| diff --git a/cmd/certmanager/main.go b/cmd/certmanager/main.go index 0b1f622..ad0a241 100644 --- a/cmd/certmanager/main.go +++ b/cmd/certmanager/main.go @@ -52,11 +52,20 @@ func rootCmd() *cobra.Command { Use: "certmanager ", 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 } diff --git a/cmd/certmanager/main_test.go b/cmd/certmanager/main_test.go index a02c591..4df7d93 100644 --- a/cmd/certmanager/main_test.go +++ b/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 ") { + 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()) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 3e3b243..2648b83 100644 --- a/internal/config/config.go +++ b/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 { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 4490a16..e4687fe 100644 --- a/internal/config/config_test.go +++ b/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") } }