From 646bd05d77957213254a2a8eda9246bf612696b5 Mon Sep 17 00:00:00 2001 From: Reto Gantenbein Date: Sat, 16 May 2026 16:08:52 +0200 Subject: [PATCH 1/4] openspec: Add 'config-discovery-error-clarity' change spec --- .../.openspec.yaml | 2 + .../config-discovery-error-clarity/design.md | 62 +++++++++++++++++++ .../proposal.md | 25 ++++++++ .../specs/config-discovery/spec.md | 41 ++++++++++++ .../config-discovery-error-clarity/tasks.md | 27 ++++++++ 5 files changed, 157 insertions(+) create mode 100644 openspec/changes/config-discovery-error-clarity/.openspec.yaml create mode 100644 openspec/changes/config-discovery-error-clarity/design.md create mode 100644 openspec/changes/config-discovery-error-clarity/proposal.md create mode 100644 openspec/changes/config-discovery-error-clarity/specs/config-discovery/spec.md create mode 100644 openspec/changes/config-discovery-error-clarity/tasks.md diff --git a/openspec/changes/config-discovery-error-clarity/.openspec.yaml b/openspec/changes/config-discovery-error-clarity/.openspec.yaml new file mode 100644 index 0000000..ab7f13b --- /dev/null +++ b/openspec/changes/config-discovery-error-clarity/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-16 diff --git a/openspec/changes/config-discovery-error-clarity/design.md b/openspec/changes/config-discovery-error-clarity/design.md new file mode 100644 index 0000000..4e53ef6 --- /dev/null +++ b/openspec/changes/config-discovery-error-clarity/design.md @@ -0,0 +1,62 @@ +## Context + +`cmd/root.go`'s `resolveConfigPath` performs an ordered lookup for the repository configuration file when neither `--config` nor `PKGPROXY_CONFIG` is set: + +1. `./pkgproxy.yaml` +2. `$KO_DATA_PATH/pkgproxy.yaml` (only attempted if `KO_DATA_PATH` is non-empty) + +Today the function returns the joined `$KO_DATA_PATH/pkgproxy.yaml` path unconditionally when the local default is missing — it does not stat the ko candidate. `initConfig` then calls `LoadConfig(configPath)`, and any failure surfaces in a wrapped error that names only the single, last-tried path. An operator who unknowingly inherited `KO_DATA_PATH` from a parent shell sees an error pointing at a directory they may not even know about, with no indication that the local default was tried first. + +The existing capability spec (`openspec/specs/config-discovery/spec.md`) explicitly codifies the current behavior in its "KO_DATA_PATH set but no file present" scenario, so this change must update that spec. + +## Goals / Non-Goals + +**Goals:** + +- Operators see every candidate path the resolver attempted when the lookup fails, in the order it tried them. +- The `$KO_DATA_PATH` candidate is verified (stat'd) before being treated as the path to load — silent fall-through to the local default avoids reporting a confusing single-path error. +- No change to success-path behavior, precedence rules, or public surface (CLI flags, env vars, README, config schema). + +**Non-Goals:** + +- Reworking how `--config` / `PKGPROXY_CONFIG` precedence is evaluated. +- Adding new lookup locations (e.g. `$XDG_CONFIG_HOME`, `/etc/pkgproxy/`). +- Changing what `LoadConfig` itself reports when the file exists but is malformed. + +## Decisions + +### Decision 1: Surface all attempted candidates in the failure error, not just the last one + +The wrapped error from `initConfig` becomes: + +``` +unable to load configuration; tried: ./pkgproxy.yaml, /var/run/ko/pkgproxy.yaml: +``` + +`resolveConfigPath` is reshaped to return both the path to load and the list of candidates it considered. `initConfig` includes that list in its wrapped error. + +**Alternative considered:** keep `resolveConfigPath`'s `(string, error)` signature and have `initConfig` re-derive the candidate list. Rejected — duplicating the lookup logic in two places invites them drifting apart, and the resolver already knows the answer. + +**Alternative considered:** include the candidate list only when `len(candidates) > 1`. Rejected — the cost of always listing is one path in single-candidate cases, which is clearer than a conditional format. + +### Decision 2: Stat the `$KO_DATA_PATH` candidate before returning it + +If `$KO_DATA_PATH/pkgproxy.yaml` does not exist as a regular file, `resolveConfigPath` returns `defaultConfigPath` as the path to load (matching what would happen if `KO_DATA_PATH` were unset). The candidate list still records both paths that were checked, so the eventual error message remains informative. + +**Alternative considered:** return the (unverified) ko path and let `LoadConfig` fail. Rejected — pairs poorly with Decision 1, because the resolver should pick the most-likely-to-succeed path when both candidates miss; reporting `./pkgproxy.yaml` is more meaningful to an operator running locally. + +### Decision 3: Update the existing `config-discovery` capability spec, not introduce a new one + +This is a refinement of an already-shipped capability. The spec delta uses `MODIFIED Requirements` for the lookup requirement (the failure-mode clause changes) and revises the `KO_DATA_PATH set but no file present` scenario in place. No new requirement is introduced. + +## Risks / Trade-offs + +- **Risk:** existing test `"KO_DATA_PATH set but no file returns ko path"` in `cmd/config_test.go` pins the old behavior and will fail. + - **Mitigation:** rewrite that case to assert the new return value (`defaultConfigPath`) and add a new test asserting the wrapped error from `initConfig` enumerates both candidates. +- **Risk:** operators parsing the error string in scripts (unlikely, but possible) break. + - **Mitigation:** the change is documented in `CHANGELOG.md` under `[Unreleased]`; the surface is a CLI error message, not a contract. +- **Trade-off:** the resolver now returns a small struct/tuple instead of a single string, marginally complicating the call site. Worth it for the single source of truth on candidate paths. + +## Migration Plan + +No migration needed — the change is a pure error-message and fallback-path refinement. No data, configuration, or persisted state is affected. Rollback is a single `git revert`. diff --git a/openspec/changes/config-discovery-error-clarity/proposal.md b/openspec/changes/config-discovery-error-clarity/proposal.md new file mode 100644 index 0000000..a6be1d4 --- /dev/null +++ b/openspec/changes/config-discovery-error-clarity/proposal.md @@ -0,0 +1,25 @@ +## Why + +When `KO_DATA_PATH` is set in the operator's environment but no `pkgproxy.yaml` exists under it, the config lookup currently fails with an error pointing at the (unverified) ko path — e.g. `unable to load configuration from /var/run/ko/pkgproxy.yaml: no such file or directory`. An operator running pkgproxy outside a container may have inherited `KO_DATA_PATH` from a parent shell and not realize the lookup ever attempted that path, leaving them confused about where the error originated and which file they were expected to provide. + +## What Changes + +- When the ordered config-file lookup exhausts all candidates without finding a readable file, the resulting error SHALL enumerate every candidate path that was tried (in order), not just the last one. +- The `LoadConfig` call site SHALL receive a candidate path that is known to exist where possible — `resolveConfigPath` will stat the `$KO_DATA_PATH/pkgproxy.yaml` candidate and only return it when it exists, otherwise fall through to the local default path for the final attempt. +- No CLI flags, env vars, or precedence rules change. Behavior is unchanged in the success cases (both for local checkouts and ko-built containers). + +## Capabilities + +### New Capabilities + +_None — this change refines the existing `config-discovery` capability._ + +### Modified Capabilities + +- `config-discovery`: the failure scenario where no config file is found is tightened — the error must name all attempted candidates rather than just the last one, and the `KO_DATA_PATH` candidate must be verified before it is returned as the path to load. + +## Impact + +- `cmd/root.go`: `resolveConfigPath` and `initConfig` (error-wrapping site) +- `cmd/config_test.go`: existing test case `"KO_DATA_PATH set but no file returns ko path"` will be revised, and a new case will assert the multi-candidate error message +- No changes to public CLI surface, configuration schema, README, or CHANGELOG beyond a one-line entry under `[Unreleased]` diff --git a/openspec/changes/config-discovery-error-clarity/specs/config-discovery/spec.md b/openspec/changes/config-discovery-error-clarity/specs/config-discovery/spec.md new file mode 100644 index 0000000..e724922 --- /dev/null +++ b/openspec/changes/config-discovery-error-clarity/specs/config-discovery/spec.md @@ -0,0 +1,41 @@ +## MODIFIED Requirements + +### Requirement: Repository configuration is discovered via an ordered lookup +When neither the `--config`/`-c` flag nor the `PKGPROXY_CONFIG` environment variable is set, the CLI SHALL resolve the repository configuration file by trying the following paths in order and using the first one that exists as a regular readable file: + +1. `./pkgproxy.yaml` (relative to the process working directory) +2. `$KO_DATA_PATH/pkgproxy.yaml` — only attempted if the `KO_DATA_PATH` environment variable is set to a non-empty value + +If neither path yields a readable file, the CLI SHALL fail with an error that enumerates every candidate path that was attempted, in the order it tried them, of the form `unable to load configuration; tried: , : `. When the `$KO_DATA_PATH` candidate is attempted but no `pkgproxy.yaml` exists under it, the resolver SHALL fall through to the local default path as the path that is finally passed to the loader, while still recording the `$KO_DATA_PATH` candidate in the enumerated error. + +Explicit user input SHALL always take precedence over the ordered lookup: when `--config`/`-c` is passed with a value other than the built-in default, or when `PKGPROXY_CONFIG` is set, that path is used directly and the ordered lookup is not consulted. + +#### Scenario: Local source checkout uses `./pkgproxy.yaml` +- **WHEN** the binary is started with no `--config` flag, no `PKGPROXY_CONFIG` env var, and `./pkgproxy.yaml` exists in the working directory +- **THEN** the CLI SHALL load configuration from `./pkgproxy.yaml` +- **AND** the value of `KO_DATA_PATH` SHALL NOT affect the outcome + +#### Scenario: Ko-built container uses the bundled config +- **WHEN** the binary is started in a ko-built image (working directory `/ko-app`, `KO_DATA_PATH=/var/run/ko`, `pkgproxy.yaml` present at `/var/run/ko/pkgproxy.yaml` and not at `./pkgproxy.yaml`) with no `--config` flag and no `PKGPROXY_CONFIG` env var +- **THEN** the CLI SHALL load configuration from `/var/run/ko/pkgproxy.yaml` + +#### Scenario: Local file wins over ko-bundled fallback +- **WHEN** the binary is started with both `./pkgproxy.yaml` present and `KO_DATA_PATH` set to a directory containing a different `pkgproxy.yaml`, with no `--config` flag and no `PKGPROXY_CONFIG` env var +- **THEN** the CLI SHALL load configuration from `./pkgproxy.yaml` + +#### Scenario: Explicit `--config` bypasses the lookup +- **WHEN** the binary is started with `--config /custom/path.yaml` +- **THEN** the CLI SHALL load configuration from `/custom/path.yaml` regardless of whether `./pkgproxy.yaml` or `$KO_DATA_PATH/pkgproxy.yaml` exist + +#### Scenario: `PKGPROXY_CONFIG` bypasses the lookup +- **WHEN** the binary is started with `PKGPROXY_CONFIG=/custom/path.yaml` and no `--config` flag +- **THEN** the CLI SHALL load configuration from `/custom/path.yaml` regardless of whether `./pkgproxy.yaml` or `$KO_DATA_PATH/pkgproxy.yaml` exist + +#### Scenario: All default paths missing produces a clear error +- **WHEN** the binary is started with no `--config` flag, no `PKGPROXY_CONFIG` env var, `./pkgproxy.yaml` absent, and `KO_DATA_PATH` unset +- **THEN** the CLI SHALL exit with an error of the form `unable to load configuration; tried: ./pkgproxy.yaml: ` + +#### Scenario: `KO_DATA_PATH` set but no file present +- **WHEN** the binary is started with no `--config` flag, no `PKGPROXY_CONFIG` env var, `./pkgproxy.yaml` absent, and `KO_DATA_PATH` set to a directory that does not contain `pkgproxy.yaml` +- **THEN** the CLI SHALL exit with an error of the form `unable to load configuration; tried: ./pkgproxy.yaml, $KO_DATA_PATH/pkgproxy.yaml: ` (with the expanded `$KO_DATA_PATH` value) +- **AND** the path that was finally passed to the configuration loader SHALL be `./pkgproxy.yaml`, not the joined `$KO_DATA_PATH/pkgproxy.yaml` diff --git a/openspec/changes/config-discovery-error-clarity/tasks.md b/openspec/changes/config-discovery-error-clarity/tasks.md new file mode 100644 index 0000000..08f8415 --- /dev/null +++ b/openspec/changes/config-discovery-error-clarity/tasks.md @@ -0,0 +1,27 @@ +## 1. Resolver reshape + +- [ ] 1.1 Change `resolveConfigPath` in `cmd/root.go` to return both the path to load and the ordered list of candidate paths it considered (e.g. signature `func resolveConfigPath() (path string, candidates []string, err error)`). +- [ ] 1.2 Stat the `$KO_DATA_PATH/pkgproxy.yaml` candidate before returning it as the path to load; on miss, return `defaultConfigPath` as the path to load while keeping both entries in the `candidates` slice. +- [ ] 1.3 Keep the existing fall-through behavior when `./pkgproxy.yaml` is present-but-not-a-regular-file (current `info.Mode().IsRegular()` check). + +## 2. Error-wrapping update + +- [ ] 2.1 Update `initConfig` in `cmd/root.go` to receive the `candidates` slice from `resolveConfigPath` and include it in the wrapped error of the form `unable to load configuration; tried: , : %w` (only emit the multi-candidate form when the ordered lookup ran — explicit `--config` and `PKGPROXY_CONFIG` paths continue to use a single-path error). + +## 3. Tests + +- [ ] 3.1 Rewrite the `"KO_DATA_PATH set but no file returns ko path"` case in `cmd/config_test.go` (rename appropriately) to assert that `resolveConfigPath` returns `defaultConfigPath` as the load path and a two-element candidate slice (`./pkgproxy.yaml`, `$KO_DATA_PATH/pkgproxy.yaml`). +- [ ] 3.2 Update the other `TestResolveConfigPath` cases to assert the new return signature (including `candidates` content) without changing their underlying intent. +- [ ] 3.3 Add a `TestInitConfig` case asserting the wrapped error contains `tried: ./pkgproxy.yaml, /pkgproxy.yaml` when the ordered lookup exhausts both candidates. +- [ ] 3.4 Add a `TestInitConfig` case asserting the wrapped error contains only the single attempted path when `--config` or `PKGPROXY_CONFIG` is used (no `tried:` prefix needed for that path). + +## 4. Verification + +- [ ] 4.1 Run `make ci-check` and confirm lint, govulncheck, and unit tests all pass. +- [ ] 4.2 Run `pre-commit run --all-files` to catch formatting and codespell issues. +- [ ] 4.3 Manually verify the new error format by running the built binary in a tempdir with `KO_DATA_PATH=/tmp/no-such-dir` and confirming the operator-facing message names both candidate paths. + +## 5. Docs and changelog + +- [ ] 5.1 Add a single concise entry under `[Unreleased]` in `CHANGELOG.md` (80–100 chars), framed for pkgproxy users — e.g. that config-file errors now list every default path that was attempted. +- [ ] 5.2 Confirm no README changes are required (no CLI flags, env vars, or config keys changed). From 95aa592ea273665af7113ac5e3bd891b71f0fe1e Mon Sep 17 00:00:00 2001 From: Reto Gantenbein Date: Sat, 16 May 2026 16:33:31 +0200 Subject: [PATCH 2/4] serve: List all attempted config paths in discovery error When the ordered lookup exhausts both candidates without finding a readable file, the error now names every path tried. The ko-data-path candidate is also stat'd before being returned, so a local operator with KO_DATA_PATH inherited from a parent shell sees a meaningful fallback to ./pkgproxy.yaml rather than a confusing single-path error. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 1 + cmd/config_test.go | 94 ++++++++++++++++--- cmd/root.go | 29 ++++-- .../config-discovery-error-clarity/tasks.md | 26 ++--- 4 files changed, 116 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f4b6c6..ed06135 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed +- Config-file errors now list all default paths attempted, not just the last one - Upgraded Echo web framework to v5.1.0 ## [v0.2.0](https://github.com/ganto/pkgproxy/releases/tag/v0.2.0) - 2026-04-06 diff --git a/cmd/config_test.go b/cmd/config_test.go index a0d4e43..4c88b18 100644 --- a/cmd/config_test.go +++ b/cmd/config_test.go @@ -3,6 +3,7 @@ package cmd import ( + "fmt" "os" "path/filepath" "testing" @@ -22,39 +23,48 @@ func writeConfig(t *testing.T, dir, name string) string { func TestResolveConfigPath(t *testing.T) { tests := []struct { - name string - localExists bool - localIsDir bool - koDataSet bool - koFileExists bool - want func(koDir string) string + name string + localExists bool + localIsDir bool + koDataSet bool + koFileExists bool + wantPath func(koDir string) string + wantCandidates func(koDir string) []string }{ { name: "local file wins over ko fallback", localExists: true, koDataSet: true, koFileExists: true, - want: func(_ string) string { return defaultConfigPath }, + wantPath: func(_ string) string { return defaultConfigPath }, + wantCandidates: func(_ string) []string { return []string{defaultConfigPath} }, }, { name: "ko fallback used when local missing", localExists: false, koDataSet: true, koFileExists: true, - want: func(koDir string) string { return filepath.Join(koDir, "pkgproxy.yaml") }, + wantPath: func(koDir string) string { return filepath.Join(koDir, "pkgproxy.yaml") }, + wantCandidates: func(koDir string) []string { + return []string{defaultConfigPath, filepath.Join(koDir, "pkgproxy.yaml")} + }, }, { name: "both missing returns default path", localExists: false, koDataSet: false, - want: func(_ string) string { return defaultConfigPath }, + wantPath: func(_ string) string { return defaultConfigPath }, + wantCandidates: func(_ string) []string { return []string{defaultConfigPath} }, }, { - name: "KO_DATA_PATH set but no file returns ko path", + name: "KO_DATA_PATH set but file missing falls back to default", localExists: false, koDataSet: true, koFileExists: false, - want: func(koDir string) string { return filepath.Join(koDir, "pkgproxy.yaml") }, + wantPath: func(_ string) string { return defaultConfigPath }, + wantCandidates: func(koDir string) []string { + return []string{defaultConfigPath, filepath.Join(koDir, "pkgproxy.yaml")} + }, }, { name: "directory named pkgproxy.yaml falls through to ko", @@ -62,7 +72,10 @@ func TestResolveConfigPath(t *testing.T) { localIsDir: true, koDataSet: true, koFileExists: true, - want: func(koDir string) string { return filepath.Join(koDir, "pkgproxy.yaml") }, + wantPath: func(koDir string) string { return filepath.Join(koDir, "pkgproxy.yaml") }, + wantCandidates: func(koDir string) []string { + return []string{defaultConfigPath, filepath.Join(koDir, "pkgproxy.yaml")} + }, }, } @@ -105,9 +118,10 @@ func TestResolveConfigPath(t *testing.T) { t.Setenv(koDataPathEnvVar, "") } - got, err := resolveConfigPath() + gotPath, gotCandidates, err := resolveConfigPath() assert.NoError(t, err) - assert.Equal(t, tt.want(koDir), got) + assert.Equal(t, tt.wantPath(koDir), gotPath) + assert.Equal(t, tt.wantCandidates(koDir), gotCandidates) }) } } @@ -142,6 +156,58 @@ func TestInitConfig(t *testing.T) { assert.Equal(t, envFile, configPath) }) + t.Run("ordered lookup error names all candidates", func(t *testing.T) { + localDir := t.TempDir() + koDir := t.TempDir() + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(localDir)) + t.Cleanup(func() { _ = os.Chdir(origDir) }) + + // unset PKGPROXY_CONFIG so the ordered lookup runs + if prev, ok := os.LookupEnv(configPathEnvVar); ok { + require.NoError(t, os.Unsetenv(configPathEnvVar)) + t.Cleanup(func() { _ = os.Setenv(configPathEnvVar, prev) }) + } + t.Setenv(koDataPathEnvVar, koDir) + // neither ./pkgproxy.yaml nor $koDir/pkgproxy.yaml exist + + configPath = defaultConfigPath + t.Cleanup(func() { configPath = defaultConfigPath }) + + err = initConfig() + require.Error(t, err) + assert.Contains(t, err.Error(), fmt.Sprintf("tried: %s, %s", defaultConfigPath, filepath.Join(koDir, "pkgproxy.yaml"))) + }) + + t.Run("explicit config path error names only that path", func(t *testing.T) { + dir := t.TempDir() + + configPath = filepath.Join(dir, "nonexistent.yaml") + t.Cleanup(func() { configPath = defaultConfigPath }) + + err := initConfig() + require.Error(t, err) + assert.Contains(t, err.Error(), configPath) + assert.NotContains(t, err.Error(), "tried:") + }) + + t.Run("PKGPROXY_CONFIG error names only that path", func(t *testing.T) { + dir := t.TempDir() + + t.Setenv(configPathEnvVar, filepath.Join(dir, "nonexistent.yaml")) + t.Setenv(koDataPathEnvVar, "") + + configPath = defaultConfigPath + t.Cleanup(func() { configPath = defaultConfigPath }) + + err := initConfig() + require.Error(t, err) + assert.Contains(t, err.Error(), filepath.Join(dir, "nonexistent.yaml")) + assert.NotContains(t, err.Error(), "tried:") + }) + t.Run("ordered lookup used when flag and env var unset", func(t *testing.T) { localDir := t.TempDir() diff --git a/cmd/root.go b/cmd/root.go index 6742d4e..b7dd8c2 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/ganto/pkgproxy/pkg/pkgproxy" "github.com/spf13/cobra" @@ -59,29 +60,40 @@ func injectServeDefault() { } // resolveConfigPath returns the config file path to use when neither --config -// nor $PKGPROXY_CONFIG has been set explicitly. -func resolveConfigPath() (string, error) { +// nor $PKGPROXY_CONFIG has been set explicitly, along with the ordered list of +// candidate paths it considered. +func resolveConfigPath() (string, []string, error) { + candidates := []string{defaultConfigPath} + if info, err := os.Stat(defaultConfigPath); err == nil { if info.Mode().IsRegular() { - return defaultConfigPath, nil + return defaultConfigPath, candidates, nil } // not a regular file — fall through to KO_DATA_PATH } else if !errors.Is(err, os.ErrNotExist) { - return "", err + return "", candidates, err } + if koDataPath, ok := os.LookupEnv(koDataPathEnvVar); ok && koDataPath != "" { - return filepath.Join(koDataPath, "pkgproxy.yaml"), nil + koPath := filepath.Join(koDataPath, "pkgproxy.yaml") + candidates = append(candidates, koPath) + if info, err := os.Stat(koPath); err == nil && info.Mode().IsRegular() { + return koPath, candidates, nil + } } - return defaultConfigPath, nil + + return defaultConfigPath, candidates, nil } func initConfig() error { + var candidates []string + if configPath == defaultConfigPath { if value, found := os.LookupEnv(configPathEnvVar); found { configPath = value } else { var err error - configPath, err = resolveConfigPath() + configPath, candidates, err = resolveConfigPath() if err != nil { return fmt.Errorf("unable to resolve config path: %w", err) } @@ -89,6 +101,9 @@ func initConfig() error { } if err := pkgproxy.LoadConfig(&repoConfig, configPath); err != nil { + if candidates != nil { + return fmt.Errorf("unable to load configuration; tried: %s: %w", strings.Join(candidates, ", "), err) + } return fmt.Errorf("unable to load configuration from %s: %w", configPath, err) } return nil diff --git a/openspec/changes/config-discovery-error-clarity/tasks.md b/openspec/changes/config-discovery-error-clarity/tasks.md index 08f8415..466c361 100644 --- a/openspec/changes/config-discovery-error-clarity/tasks.md +++ b/openspec/changes/config-discovery-error-clarity/tasks.md @@ -1,27 +1,27 @@ ## 1. Resolver reshape -- [ ] 1.1 Change `resolveConfigPath` in `cmd/root.go` to return both the path to load and the ordered list of candidate paths it considered (e.g. signature `func resolveConfigPath() (path string, candidates []string, err error)`). -- [ ] 1.2 Stat the `$KO_DATA_PATH/pkgproxy.yaml` candidate before returning it as the path to load; on miss, return `defaultConfigPath` as the path to load while keeping both entries in the `candidates` slice. -- [ ] 1.3 Keep the existing fall-through behavior when `./pkgproxy.yaml` is present-but-not-a-regular-file (current `info.Mode().IsRegular()` check). +- [x] 1.1 Change `resolveConfigPath` in `cmd/root.go` to return both the path to load and the ordered list of candidate paths it considered (e.g. signature `func resolveConfigPath() (path string, candidates []string, err error)`). +- [x] 1.2 Stat the `$KO_DATA_PATH/pkgproxy.yaml` candidate before returning it as the path to load; on miss, return `defaultConfigPath` as the path to load while keeping both entries in the `candidates` slice. +- [x] 1.3 Keep the existing fall-through behavior when `./pkgproxy.yaml` is present-but-not-a-regular-file (current `info.Mode().IsRegular()` check). ## 2. Error-wrapping update -- [ ] 2.1 Update `initConfig` in `cmd/root.go` to receive the `candidates` slice from `resolveConfigPath` and include it in the wrapped error of the form `unable to load configuration; tried: , : %w` (only emit the multi-candidate form when the ordered lookup ran — explicit `--config` and `PKGPROXY_CONFIG` paths continue to use a single-path error). +- [x] 2.1 Update `initConfig` in `cmd/root.go` to receive the `candidates` slice from `resolveConfigPath` and include it in the wrapped error of the form `unable to load configuration; tried: , : %w` (only emit the multi-candidate form when the ordered lookup ran — explicit `--config` and `PKGPROXY_CONFIG` paths continue to use a single-path error). ## 3. Tests -- [ ] 3.1 Rewrite the `"KO_DATA_PATH set but no file returns ko path"` case in `cmd/config_test.go` (rename appropriately) to assert that `resolveConfigPath` returns `defaultConfigPath` as the load path and a two-element candidate slice (`./pkgproxy.yaml`, `$KO_DATA_PATH/pkgproxy.yaml`). -- [ ] 3.2 Update the other `TestResolveConfigPath` cases to assert the new return signature (including `candidates` content) without changing their underlying intent. -- [ ] 3.3 Add a `TestInitConfig` case asserting the wrapped error contains `tried: ./pkgproxy.yaml, /pkgproxy.yaml` when the ordered lookup exhausts both candidates. -- [ ] 3.4 Add a `TestInitConfig` case asserting the wrapped error contains only the single attempted path when `--config` or `PKGPROXY_CONFIG` is used (no `tried:` prefix needed for that path). +- [x] 3.1 Rewrite the `"KO_DATA_PATH set but no file returns ko path"` case in `cmd/config_test.go` (rename appropriately) to assert that `resolveConfigPath` returns `defaultConfigPath` as the load path and a two-element candidate slice (`./pkgproxy.yaml`, `$KO_DATA_PATH/pkgproxy.yaml`). +- [x] 3.2 Update the other `TestResolveConfigPath` cases to assert the new return signature (including `candidates` content) without changing their underlying intent. +- [x] 3.3 Add a `TestInitConfig` case asserting the wrapped error contains `tried: ./pkgproxy.yaml, /pkgproxy.yaml` when the ordered lookup exhausts both candidates. +- [x] 3.4 Add a `TestInitConfig` case asserting the wrapped error contains only the single attempted path when `--config` or `PKGPROXY_CONFIG` is used (no `tried:` prefix needed for that path). ## 4. Verification -- [ ] 4.1 Run `make ci-check` and confirm lint, govulncheck, and unit tests all pass. -- [ ] 4.2 Run `pre-commit run --all-files` to catch formatting and codespell issues. -- [ ] 4.3 Manually verify the new error format by running the built binary in a tempdir with `KO_DATA_PATH=/tmp/no-such-dir` and confirming the operator-facing message names both candidate paths. +- [x] 4.1 Run `make ci-check` and confirm lint, govulncheck, and unit tests all pass. +- [x] 4.2 Run `pre-commit run --all-files` to catch formatting and codespell issues. +- [x] 4.3 Manually verify the new error format by running the built binary in a tempdir with `KO_DATA_PATH=/tmp/no-such-dir` and confirming the operator-facing message names both candidate paths. ## 5. Docs and changelog -- [ ] 5.1 Add a single concise entry under `[Unreleased]` in `CHANGELOG.md` (80–100 chars), framed for pkgproxy users — e.g. that config-file errors now list every default path that was attempted. -- [ ] 5.2 Confirm no README changes are required (no CLI flags, env vars, or config keys changed). +- [x] 5.1 Add a single concise entry under `[Unreleased]` in `CHANGELOG.md` (80–100 chars), framed for pkgproxy users — e.g. that config-file errors now list every default path that was attempted. +- [x] 5.2 Confirm no README changes are required (no CLI flags, env vars, or config keys changed). From 4324c89cf7d452436303ea310dc65c0ab7e6c693 Mon Sep 17 00:00:00 2001 From: Reto Gantenbein Date: Sat, 16 May 2026 16:41:12 +0200 Subject: [PATCH 3/4] serve: Propagate unexpected stat errors for KO_DATA_PATH config candidate Co-Authored-By: Claude Sonnet 4.6 --- cmd/config_test.go | 22 ++++++++++++++++++++++ cmd/root.go | 8 ++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/cmd/config_test.go b/cmd/config_test.go index 4c88b18..796514d 100644 --- a/cmd/config_test.go +++ b/cmd/config_test.go @@ -3,6 +3,7 @@ package cmd import ( + "errors" "fmt" "os" "path/filepath" @@ -126,6 +127,27 @@ func TestResolveConfigPath(t *testing.T) { } } +func TestResolveConfigPathKoStatError(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("permission test cannot run as root") + } + localDir := t.TempDir() + koDir := t.TempDir() + require.NoError(t, os.Chmod(koDir, 0)) + t.Cleanup(func() { _ = os.Chmod(koDir, 0700) }) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(localDir)) + t.Cleanup(func() { _ = os.Chdir(origDir) }) + + t.Setenv(koDataPathEnvVar, koDir) + + _, _, err = resolveConfigPath() + require.Error(t, err) + assert.False(t, errors.Is(err, os.ErrNotExist)) +} + func TestInitConfig(t *testing.T) { t.Run("explicit --config bypasses lookup and env var", func(t *testing.T) { dir := t.TempDir() diff --git a/cmd/root.go b/cmd/root.go index b7dd8c2..2aab159 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -77,8 +77,12 @@ func resolveConfigPath() (string, []string, error) { if koDataPath, ok := os.LookupEnv(koDataPathEnvVar); ok && koDataPath != "" { koPath := filepath.Join(koDataPath, "pkgproxy.yaml") candidates = append(candidates, koPath) - if info, err := os.Stat(koPath); err == nil && info.Mode().IsRegular() { - return koPath, candidates, nil + if info, err := os.Stat(koPath); err == nil { + if info.Mode().IsRegular() { + return koPath, candidates, nil + } + } else if !errors.Is(err, os.ErrNotExist) { + return "", candidates, err } } From b0ff5fdb66691b239a3527d71ccc97597b0e8fcc Mon Sep 17 00:00:00 2001 From: Reto Gantenbein Date: Sat, 16 May 2026 16:45:44 +0200 Subject: [PATCH 4/4] openspec: Archive 'config-discovery-error-clarity' change spec --- .../.openspec.yaml | 0 .../2026-05-16-config-discovery-error-clarity}/design.md | 0 .../proposal.md | 0 .../specs/config-discovery/spec.md | 0 .../2026-05-16-config-discovery-error-clarity}/tasks.md | 0 openspec/specs/config-discovery/spec.md | 9 +++++---- 6 files changed, 5 insertions(+), 4 deletions(-) rename openspec/changes/{config-discovery-error-clarity => archive/2026-05-16-config-discovery-error-clarity}/.openspec.yaml (100%) rename openspec/changes/{config-discovery-error-clarity => archive/2026-05-16-config-discovery-error-clarity}/design.md (100%) rename openspec/changes/{config-discovery-error-clarity => archive/2026-05-16-config-discovery-error-clarity}/proposal.md (100%) rename openspec/changes/{config-discovery-error-clarity => archive/2026-05-16-config-discovery-error-clarity}/specs/config-discovery/spec.md (100%) rename openspec/changes/{config-discovery-error-clarity => archive/2026-05-16-config-discovery-error-clarity}/tasks.md (100%) diff --git a/openspec/changes/config-discovery-error-clarity/.openspec.yaml b/openspec/changes/archive/2026-05-16-config-discovery-error-clarity/.openspec.yaml similarity index 100% rename from openspec/changes/config-discovery-error-clarity/.openspec.yaml rename to openspec/changes/archive/2026-05-16-config-discovery-error-clarity/.openspec.yaml diff --git a/openspec/changes/config-discovery-error-clarity/design.md b/openspec/changes/archive/2026-05-16-config-discovery-error-clarity/design.md similarity index 100% rename from openspec/changes/config-discovery-error-clarity/design.md rename to openspec/changes/archive/2026-05-16-config-discovery-error-clarity/design.md diff --git a/openspec/changes/config-discovery-error-clarity/proposal.md b/openspec/changes/archive/2026-05-16-config-discovery-error-clarity/proposal.md similarity index 100% rename from openspec/changes/config-discovery-error-clarity/proposal.md rename to openspec/changes/archive/2026-05-16-config-discovery-error-clarity/proposal.md diff --git a/openspec/changes/config-discovery-error-clarity/specs/config-discovery/spec.md b/openspec/changes/archive/2026-05-16-config-discovery-error-clarity/specs/config-discovery/spec.md similarity index 100% rename from openspec/changes/config-discovery-error-clarity/specs/config-discovery/spec.md rename to openspec/changes/archive/2026-05-16-config-discovery-error-clarity/specs/config-discovery/spec.md diff --git a/openspec/changes/config-discovery-error-clarity/tasks.md b/openspec/changes/archive/2026-05-16-config-discovery-error-clarity/tasks.md similarity index 100% rename from openspec/changes/config-discovery-error-clarity/tasks.md rename to openspec/changes/archive/2026-05-16-config-discovery-error-clarity/tasks.md diff --git a/openspec/specs/config-discovery/spec.md b/openspec/specs/config-discovery/spec.md index fa8f51b..e724922 100644 --- a/openspec/specs/config-discovery/spec.md +++ b/openspec/specs/config-discovery/spec.md @@ -1,4 +1,4 @@ -## ADDED Requirements +## MODIFIED Requirements ### Requirement: Repository configuration is discovered via an ordered lookup When neither the `--config`/`-c` flag nor the `PKGPROXY_CONFIG` environment variable is set, the CLI SHALL resolve the repository configuration file by trying the following paths in order and using the first one that exists as a regular readable file: @@ -6,7 +6,7 @@ When neither the `--config`/`-c` flag nor the `PKGPROXY_CONFIG` environment vari 1. `./pkgproxy.yaml` (relative to the process working directory) 2. `$KO_DATA_PATH/pkgproxy.yaml` — only attempted if the `KO_DATA_PATH` environment variable is set to a non-empty value -If neither path yields a readable file, the CLI SHALL fail with the existing "unable to load configuration from ..." error, naming the last path that was attempted. +If neither path yields a readable file, the CLI SHALL fail with an error that enumerates every candidate path that was attempted, in the order it tried them, of the form `unable to load configuration; tried: , : `. When the `$KO_DATA_PATH` candidate is attempted but no `pkgproxy.yaml` exists under it, the resolver SHALL fall through to the local default path as the path that is finally passed to the loader, while still recording the `$KO_DATA_PATH` candidate in the enumerated error. Explicit user input SHALL always take precedence over the ordered lookup: when `--config`/`-c` is passed with a value other than the built-in default, or when `PKGPROXY_CONFIG` is set, that path is used directly and the ordered lookup is not consulted. @@ -33,8 +33,9 @@ Explicit user input SHALL always take precedence over the ordered lookup: when ` #### Scenario: All default paths missing produces a clear error - **WHEN** the binary is started with no `--config` flag, no `PKGPROXY_CONFIG` env var, `./pkgproxy.yaml` absent, and `KO_DATA_PATH` unset -- **THEN** the CLI SHALL exit with an error of the form `unable to load configuration from ./pkgproxy.yaml: ...` +- **THEN** the CLI SHALL exit with an error of the form `unable to load configuration; tried: ./pkgproxy.yaml: ` #### Scenario: `KO_DATA_PATH` set but no file present - **WHEN** the binary is started with no `--config` flag, no `PKGPROXY_CONFIG` env var, `./pkgproxy.yaml` absent, and `KO_DATA_PATH` set to a directory that does not contain `pkgproxy.yaml` -- **THEN** the CLI SHALL exit with an error naming `$KO_DATA_PATH/pkgproxy.yaml` as the path it attempted to load +- **THEN** the CLI SHALL exit with an error of the form `unable to load configuration; tried: ./pkgproxy.yaml, $KO_DATA_PATH/pkgproxy.yaml: ` (with the expanded `$KO_DATA_PATH` value) +- **AND** the path that was finally passed to the configuration loader SHALL be `./pkgproxy.yaml`, not the joined `$KO_DATA_PATH/pkgproxy.yaml`