From 13029dbb332cf9da28751f1dd8b599491dabee6a Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 23 May 2026 17:43:27 +0000 Subject: [PATCH] Centralize effective config loading and path resolution --- internal/cli/run.go | 98 +++-------- internal/cli/run_test.go | 99 +++++------ internal/core/config/effective_config.go | 119 +++++++++++++ internal/core/config/effective_config_test.go | 163 ++++++++++++++++++ 4 files changed, 341 insertions(+), 138 deletions(-) create mode 100644 internal/core/config/effective_config.go create mode 100644 internal/core/config/effective_config_test.go diff --git a/internal/cli/run.go b/internal/cli/run.go index 6d58f33..273a15b 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -375,30 +375,28 @@ func runProcess(args []string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "audita process: invalid CLI configuration: %v\n", err) return 2 } - configPath, configSource, err := resolveConfigPath(configPathOverride, configPathOverrideSet, os.LookupEnv) + effectiveConfig, err := config.LoadEffectiveConfig(configPathOverride, configPathOverrideSet) if err != nil { - fmt.Fprintf(stderr, "audita process: %v\n", err) + var effectiveConfigErr *config.EffectiveConfigError + if errors.As(err, &effectiveConfigErr) { + switch effectiveConfigErr.Kind { + case config.EffectiveConfigErrorLoadFile, config.EffectiveConfigErrorApplyFile: + fmt.Fprintf(stderr, "audita process: invalid config file: %v\n", effectiveConfigErr) + case config.EffectiveConfigErrorApplyEnv: + fmt.Fprintf(stderr, "audita process: invalid environment configuration: %v\n", effectiveConfigErr) + default: + fmt.Fprintf(stderr, "audita process: %v\n", effectiveConfigErr) + } + } else { + fmt.Fprintf(stderr, "audita process: %v\n", err) + } return 2 } - cfg := config.Default() - var configVersion *int - if configPath != "" { - fileCfg, fileErr := config.LoadFileConfig(configPath) - if fileErr != nil { - fmt.Fprintf(stderr, "audita process: invalid config file: %v\n", fileErr) - return 2 - } - if applyErr := cfg.ApplyFileConfig(fileCfg); applyErr != nil { - fmt.Fprintf(stderr, "audita process: invalid config file: %v\n", applyErr) - return 2 - } - configVersion = &fileCfg.Version - } - if err := cfg.ApplyEnvOverrides(); err != nil { - fmt.Fprintf(stderr, "audita process: invalid environment configuration: %v\n", err) - return 2 - } + cfg := effectiveConfig.Config + configPath := effectiveConfig.ConfigPath + configSource := effectiveConfig.ConfigSource + configVersion := effectiveConfig.ConfigVersion fs, pFlags := newProcessFlagSet(cfg, stderr) @@ -679,28 +677,13 @@ func runConfigPrintEffective(args []string, stdout, stderr io.Writer) int { configPathValue := strings.TrimSpace(*configPath) configPathSet := configPathValue != "" - path, _, err := resolveConfigPath(configPathValue, configPathSet, os.LookupEnv) + effectiveConfig, err := config.LoadEffectiveConfig(configPathValue, configPathSet) if err != nil { fmt.Fprintf(stderr, "audita config print-effective: %v\n", err) return 2 } - cfg := config.Default() - if path != "" { - fileCfg, fileErr := config.LoadFileConfig(path) - if fileErr != nil { - fmt.Fprintf(stderr, "audita config print-effective: %v\n", fileErr) - return 2 - } - if applyErr := cfg.ApplyFileConfig(fileCfg); applyErr != nil { - fmt.Fprintf(stderr, "audita config print-effective: %v\n", applyErr) - return 2 - } - } - if err := cfg.ApplyEnvOverrides(); err != nil { - fmt.Fprintf(stderr, "audita config print-effective: %v\n", err) - return 2 - } + cfg := effectiveConfig.Config redacted := cfg.Redacted() out, err := json.MarshalIndent(redacted, "", " ") @@ -975,47 +958,6 @@ func findConfigPathOverride(args []string) (path string, set bool, err error) { return "", false, nil } -var statConfigPath = os.Stat - -func resolveConfigPath(cliConfigPath string, cliConfigPathSet bool, lookup func(string) (string, bool)) (path string, source string, err error) { - if cliConfigPathSet { - path = strings.TrimSpace(cliConfigPath) - if path == "" { - return "", "", fmt.Errorf("--config requires a non-empty path") - } - if _, statErr := statConfigPath(path); statErr != nil { - if os.IsNotExist(statErr) { - return "", "", fmt.Errorf("config file not found: %s", path) - } - return "", "", fmt.Errorf("cannot access config file %s: %w", path, statErr) - } - return path, "flag", nil - } - - if raw, ok := lookup("AUDITA_CONFIG"); ok { - path = strings.TrimSpace(raw) - if path == "" { - return "", "", fmt.Errorf("AUDITA_CONFIG must not be empty") - } - if _, statErr := statConfigPath(path); statErr != nil { - if os.IsNotExist(statErr) { - return "", "", fmt.Errorf("config file not found: %s", path) - } - return "", "", fmt.Errorf("cannot access config file %s: %w", path, statErr) - } - return path, "env", nil - } - - for _, defaultPath := range config.DefaultConfigSearchPaths { - if _, statErr := statConfigPath(defaultPath); statErr == nil { - return defaultPath, "default", nil - } else if !os.IsNotExist(statErr) { - return "", "", fmt.Errorf("cannot access config file %s: %w", defaultPath, statErr) - } - } - return "", "", nil -} - func isHelpCommand(args []string) bool { if len(args) == 0 { return false diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index 6239e6e..fe6ee44 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -98,66 +98,6 @@ func TestRunProcessHelpListsExpectedFlags(t *testing.T) { } } -func TestResolveConfigPathDefaultIgnoredWhenMissing(t *testing.T) { - lookup := func(string) (string, bool) { return "", false } - path, source, err := resolveConfigPath("", false, lookup) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if path != "" || source != "" { - t.Fatalf("expected no config path/source, got path=%q source=%q", path, source) - } -} - -func TestResolveConfigPathDefaultPrefersUsrLocalOverEtc(t *testing.T) { - oldStat := statConfigPath - statConfigPath = func(path string) (os.FileInfo, error) { - if path == config.DefaultConfigPathUsrLocal || path == config.DefaultConfigPath { - return nil, nil - } - return nil, os.ErrNotExist - } - t.Cleanup(func() { statConfigPath = oldStat }) - - lookup := func(string) (string, bool) { return "", false } - path, source, err := resolveConfigPath("", false, lookup) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if source != "default" { - t.Fatalf("expected default source, got %q", source) - } - if path != config.DefaultConfigPathUsrLocal { - t.Fatalf("expected %q, got %q", config.DefaultConfigPathUsrLocal, path) - } -} - -func TestResolveConfigPathDefaultFallsBackToEtc(t *testing.T) { - oldStat := statConfigPath - statConfigPath = func(path string) (os.FileInfo, error) { - if path == config.DefaultConfigPathUsrLocal { - return nil, os.ErrNotExist - } - if path == config.DefaultConfigPath { - return nil, nil - } - return nil, os.ErrNotExist - } - t.Cleanup(func() { statConfigPath = oldStat }) - - lookup := func(string) (string, bool) { return "", false } - path, source, err := resolveConfigPath("", false, lookup) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if source != "default" { - t.Fatalf("expected default source, got %q", source) - } - if path != config.DefaultConfigPath { - t.Fatalf("expected %q, got %q", config.DefaultConfigPath, path) - } -} - func TestRunConfigValidateSuccess(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer @@ -258,6 +198,45 @@ func TestRunConfigPrintEffectiveOutputsRedactedJSON(t *testing.T) { } } +func TestRunConfigPrintEffectiveAppliesFileThenEnvironment(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + cfgPath := writeFile(t, "config.yml", "version: 1\nllm:\n proposal:\n model: file-model\n") + t.Setenv("AUDITA_MODEL", "env-model") + + exitCode := Run([]string{"config", "print-effective", "--config", cfgPath}, &stdout, &stderr) + if exitCode != 0 { + t.Fatalf("expected success, got %d stderr=%q", exitCode, stderr.String()) + } + + var out struct { + PrimaryLLM struct { + Model string `json:"Model"` + } `json:"PrimaryLLM"` + } + if err := json.Unmarshal(stdout.Bytes(), &out); err != nil { + t.Fatalf("expected valid JSON output, got error: %v output=%q", err, stdout.String()) + } + if out.PrimaryLLM.Model != "env-model" { + t.Fatalf("expected env model override in print-effective output, got %q", out.PrimaryLLM.Model) + } +} + +func TestRunConfigValidateIgnoresEnvironmentOverrides(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + cfgPath := writeFile(t, "config.yml", "version: 1\n") + t.Setenv("AUDITA_MODULES", "made_up") + + exitCode := Run([]string{"config", "validate", "--config", cfgPath}, &stdout, &stderr) + if exitCode != 0 { + t.Fatalf("expected success because config validate is file-only, got %d stderr=%q", exitCode, stderr.String()) + } + if !strings.Contains(stdout.String(), "config is valid") { + t.Fatalf("expected success message, got %q", stdout.String()) + } +} + func TestRunConfigCommandDoesNotRequireTranscriptOrGlossary(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer diff --git a/internal/core/config/effective_config.go b/internal/core/config/effective_config.go new file mode 100644 index 0000000..af2606a --- /dev/null +++ b/internal/core/config/effective_config.go @@ -0,0 +1,119 @@ +package config + +import ( + "fmt" + "os" + "strings" +) + +type EffectiveConfigErrorKind string + +const ( + EffectiveConfigErrorResolvePath EffectiveConfigErrorKind = "resolve_path" + EffectiveConfigErrorLoadFile EffectiveConfigErrorKind = "load_file" + EffectiveConfigErrorApplyFile EffectiveConfigErrorKind = "apply_file" + EffectiveConfigErrorApplyEnv EffectiveConfigErrorKind = "apply_env" +) + +type EffectiveConfigError struct { + Kind EffectiveConfigErrorKind + Err error +} + +func (e *EffectiveConfigError) Error() string { + if e == nil || e.Err == nil { + return "" + } + return e.Err.Error() +} + +func (e *EffectiveConfigError) Unwrap() error { + if e == nil { + return nil + } + return e.Err +} + +type EffectiveConfig struct { + Config Config + ConfigPath string + ConfigSource string + ConfigVersion *int +} + +func ResolveConfigPath(cliConfigPath string, cliConfigPathSet bool) (path string, source string, err error) { + return resolveConfigPathWithLookup(cliConfigPath, cliConfigPathSet, os.LookupEnv, os.Stat, DefaultConfigSearchPaths) +} + +func LoadEffectiveConfig(cliConfigPath string, cliConfigPathSet bool) (EffectiveConfig, error) { + return loadEffectiveConfigWithLookup(cliConfigPath, cliConfigPathSet, os.LookupEnv, os.Stat, DefaultConfigSearchPaths) +} + +func loadEffectiveConfigWithLookup(cliConfigPath string, cliConfigPathSet bool, lookup func(string) (string, bool), statPath func(string) (os.FileInfo, error), defaultSearchPaths []string) (EffectiveConfig, error) { + configPath, configSource, err := resolveConfigPathWithLookup(cliConfigPath, cliConfigPathSet, lookup, statPath, defaultSearchPaths) + if err != nil { + return EffectiveConfig{}, &EffectiveConfigError{Kind: EffectiveConfigErrorResolvePath, Err: err} + } + + cfg := Default() + var configVersion *int + if configPath != "" { + fileCfg, fileErr := LoadFileConfig(configPath) + if fileErr != nil { + return EffectiveConfig{}, &EffectiveConfigError{Kind: EffectiveConfigErrorLoadFile, Err: fileErr} + } + if applyErr := cfg.ApplyFileConfig(fileCfg); applyErr != nil { + return EffectiveConfig{}, &EffectiveConfigError{Kind: EffectiveConfigErrorApplyFile, Err: applyErr} + } + configVersion = &fileCfg.Version + } + if applyEnvErr := cfg.applyEnvOverrides(lookup); applyEnvErr != nil { + return EffectiveConfig{}, &EffectiveConfigError{Kind: EffectiveConfigErrorApplyEnv, Err: applyEnvErr} + } + + return EffectiveConfig{ + Config: cfg, + ConfigPath: configPath, + ConfigSource: configSource, + ConfigVersion: configVersion, + }, nil +} + +func resolveConfigPathWithLookup(cliConfigPath string, cliConfigPathSet bool, lookup func(string) (string, bool), statPath func(string) (os.FileInfo, error), defaultSearchPaths []string) (path string, source string, err error) { + if cliConfigPathSet { + path = strings.TrimSpace(cliConfigPath) + if path == "" { + return "", "", fmt.Errorf("--config requires a non-empty path") + } + if _, statErr := statPath(path); statErr != nil { + if os.IsNotExist(statErr) { + return "", "", fmt.Errorf("config file not found: %s", path) + } + return "", "", fmt.Errorf("cannot access config file %s: %w", path, statErr) + } + return path, "flag", nil + } + + if raw, ok := lookup("AUDITA_CONFIG"); ok { + path = strings.TrimSpace(raw) + if path == "" { + return "", "", fmt.Errorf("AUDITA_CONFIG must not be empty") + } + if _, statErr := statPath(path); statErr != nil { + if os.IsNotExist(statErr) { + return "", "", fmt.Errorf("config file not found: %s", path) + } + return "", "", fmt.Errorf("cannot access config file %s: %w", path, statErr) + } + return path, "env", nil + } + + for _, defaultPath := range defaultSearchPaths { + if _, statErr := statPath(defaultPath); statErr == nil { + return defaultPath, "default", nil + } else if !os.IsNotExist(statErr) { + return "", "", fmt.Errorf("cannot access config file %s: %w", defaultPath, statErr) + } + } + return "", "", nil +} diff --git a/internal/core/config/effective_config_test.go b/internal/core/config/effective_config_test.go new file mode 100644 index 0000000..8211ace --- /dev/null +++ b/internal/core/config/effective_config_test.go @@ -0,0 +1,163 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestResolveConfigPathWithLookupMatrix(t *testing.T) { + statFor := func(existing map[string]bool) func(string) (os.FileInfo, error) { + return func(path string) (os.FileInfo, error) { + if existing[path] { + return nil, nil + } + return nil, os.ErrNotExist + } + } + + tests := []struct { + name string + cliPath string + cliPathSet bool + lookup func(string) (string, bool) + stat func(string) (os.FileInfo, error) + defaultSearchPaths []string + wantPath string + wantSource string + wantErrContains string + }{ + { + name: "explicit config path", + cliPath: "/tmp/explicit.yml", + cliPathSet: true, + lookup: func(string) (string, bool) { return "", false }, + stat: statFor(map[string]bool{"/tmp/explicit.yml": true}), + defaultSearchPaths: []string{ + "/usr/local/etc/audita/config.yml", + "/etc/audita/config.yml", + }, + wantPath: "/tmp/explicit.yml", + wantSource: "flag", + }, + { + name: "env config path", + cliPathSet: false, + lookup: func(key string) (string, bool) { + if key == "AUDITA_CONFIG" { + return "/tmp/from-env.yml", true + } + return "", false + }, + stat: statFor(map[string]bool{"/tmp/from-env.yml": true}), + defaultSearchPaths: []string{"/usr/local/etc/audita/config.yml", "/etc/audita/config.yml"}, + wantPath: "/tmp/from-env.yml", + wantSource: "env", + }, + { + name: "default search path", + cliPathSet: false, + lookup: func(string) (string, bool) { return "", false }, + stat: statFor(map[string]bool{ + "/usr/local/etc/audita/config.yml": true, + "/etc/audita/config.yml": true, + }), + defaultSearchPaths: []string{"/usr/local/etc/audita/config.yml", "/etc/audita/config.yml"}, + wantPath: "/usr/local/etc/audita/config.yml", + wantSource: "default", + }, + { + name: "explicit missing path", + cliPath: "/tmp/missing.yml", + cliPathSet: true, + lookup: func(string) (string, bool) { return "", false }, + stat: statFor(map[string]bool{}), + defaultSearchPaths: []string{ + "/usr/local/etc/audita/config.yml", + "/etc/audita/config.yml", + }, + wantErrContains: "config file not found", + }, + { + name: "missing env path", + cliPathSet: false, + lookup: func(key string) (string, bool) { + if key == "AUDITA_CONFIG" { + return "/tmp/missing-from-env.yml", true + } + return "", false + }, + stat: statFor(map[string]bool{}), + defaultSearchPaths: []string{"/usr/local/etc/audita/config.yml", "/etc/audita/config.yml"}, + wantErrContains: "config file not found", + }, + { + name: "missing default paths", + cliPathSet: false, + lookup: func(string) (string, bool) { return "", false }, + stat: statFor(map[string]bool{}), + defaultSearchPaths: []string{ + "/usr/local/etc/audita/config.yml", + "/etc/audita/config.yml", + }, + wantPath: "", + wantSource: "", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + gotPath, gotSource, err := resolveConfigPathWithLookup(tc.cliPath, tc.cliPathSet, tc.lookup, tc.stat, tc.defaultSearchPaths) + if tc.wantErrContains != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErrContains) { + t.Fatalf("expected error containing %q, got %v", tc.wantErrContains, err) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotPath != tc.wantPath || gotSource != tc.wantSource { + t.Fatalf("unexpected result: got path=%q source=%q, want path=%q source=%q", gotPath, gotSource, tc.wantPath, tc.wantSource) + } + }) + } +} + +func TestLoadEffectiveConfigWithLookupAppliesDefaultsFileThenEnv(t *testing.T) { + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yml") + configYAML := "version: 1\nllm:\n proposal:\n model: file-model\n" + if err := os.WriteFile(configPath, []byte(configYAML), 0o644); err != nil { + t.Fatalf("write config file: %v", err) + } + + lookup := func(key string) (string, bool) { + switch key { + case "AUDITA_CONFIG": + return configPath, true + case "AUDITA_MODEL": + return "env-model", true + default: + return "", false + } + } + + result, err := loadEffectiveConfigWithLookup("", false, lookup, os.Stat, DefaultConfigSearchPaths) + if err != nil { + t.Fatalf("loadEffectiveConfigWithLookup error: %v", err) + } + if result.ConfigPath != configPath { + t.Fatalf("unexpected config path: %q", result.ConfigPath) + } + if result.ConfigSource != "env" { + t.Fatalf("unexpected config source: %q", result.ConfigSource) + } + if result.ConfigVersion == nil || *result.ConfigVersion != SupportedFileConfigVersion { + t.Fatalf("unexpected config version: %#v", result.ConfigVersion) + } + if result.Config.PrimaryLLM.Model != "env-model" { + t.Fatalf("expected env override to win over file value, got %q", result.Config.PrimaryLLM.Model) + } +}