eric/notarius
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix config validation edge cases

Browse Source
This commit is contained in:
Eric Rakestraw
2026-07-03 18:56:35 +00:00
parent 4477b13203
commit fd835b582c
6 changed files with 222 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
internal/core/config/file_config.go
View File

@@ -5,6 +5,7 @@ import (
"fmt"
"os"
"regexp"
"sort"
"strings"
"time"
@@ -198,11 +199,23 @@ func (c *Config) applyFileConfigWithLookup(fileCfg FileConfig, lookup func(strin
c.Pipelines = map[string]pipeline.PipelineProfile{}
}
for rawID, fileProfile := range fileCfg.LLMProfiles {
profileID := strings.TrimSpace(rawID)
if profileID == "" {
return fmt.Errorf("llm profile id must not be empty")
profileIDs, rawLLMProfileIDs, err := normalizedMapKeys(fileCfg.LLMProfiles, "llm profile id")
if err != nil {
return err
}
pipelineIDs, rawPipelineIDs, err := normalizedMapKeys(fileCfg.Pipelines, "pipeline id")
if err != nil {
return err
}
for _, pipelineID := range pipelineIDs {
filePipeline := fileCfg.Pipelines[rawPipelineIDs[pipelineID]]
if _, _, err := normalizedMapKeys(filePipeline.Artifacts, fmt.Sprintf("pipeline %q artifact lane id", pipelineID)); err != nil {
return err
}
}
for _, profileID := range profileIDs {
fileProfile := fileCfg.LLMProfiles[rawLLMProfileIDs[profileID]]
profile := c.LLMProfiles[profileID]
if fileProfile.Provider != nil {
profile.Provider = strings.TrimSpace(*fileProfile.Provider)
@@ -233,10 +246,11 @@ func (c *Config) applyFileConfigWithLookup(fileCfg FileConfig, lookup func(strin
c.LLMProfiles[profileID] = profile
}
for rawID, filePipeline := range fileCfg.Pipelines {
pipelineID := strings.TrimSpace(rawID)
if pipelineID == "" {
return fmt.Errorf("pipeline id must not be empty")
for _, pipelineID := range pipelineIDs {
filePipeline := fileCfg.Pipelines[rawPipelineIDs[pipelineID]]
laneIDs, rawLaneIDs, err := normalizedMapKeys(filePipeline.Artifacts, fmt.Sprintf("pipeline %q artifact lane id", pipelineID))
if err != nil {
return err
}
profile := pipeline.PipelineProfile{
ID: pipelineID,
@@ -249,11 +263,8 @@ func (c *Config) applyFileConfigWithLookup(fileCfg FileConfig, lookup func(strin
if filePipeline.Output != nil {
profile.Output = filePipeline.Output.toPipelineBinding()
}
for rawLaneID, fileLane := range filePipeline.Artifacts {
laneID := strings.TrimSpace(rawLaneID)
if laneID == "" {
return fmt.Errorf("pipeline %q artifact lane id must not be empty", pipelineID)
}
for _, laneID := range laneIDs {
fileLane := filePipeline.Artifacts[rawLaneIDs[laneID]]
lane := pipeline.ArtifactLaneProfile{
Extract: fileLane.Extract.toPipelineBinding(),
}
@@ -289,6 +300,24 @@ func (c *Config) applyFileConfigWithLookup(fileCfg FileConfig, lookup func(strin
return nil
}
func normalizedMapKeys[T any](values map[string]T, keyName string) ([]string, map[string]string, error) {
keys := make([]string, 0, len(values))
rawByNormalized := make(map[string]string, len(values))
for rawID := range values {
id := strings.TrimSpace(rawID)
if id == "" {
return nil, nil, fmt.Errorf("%s must not be empty", keyName)
}
if _, ok := rawByNormalized[id]; ok {
return nil, nil, fmt.Errorf("%s %q is duplicated after trimming", keyName, id)
}
rawByNormalized[id] = rawID
keys = append(keys, id)
}
sort.Strings(keys)
return keys, rawByNormalized, nil
}
func resolveAPIKeyEnv(envName string, lookup func(string) (string, bool)) (string, error) {
name := strings.TrimSpace(envName)
if name == "" {

83
internal/core/config/file_config_test.go
View File

@@ -234,6 +234,89 @@ llm_profiles:
}
}
func TestApplyFileConfigRejectsDuplicateTrimmedLLMProfileIDs(t *testing.T) {
fileCfg, err := ParseFileConfigYAML([]byte(`
version: 1
llm_profiles:
default:
model: first
" default ":
model: second
`))
if err != nil {
t.Fatalf("ParseFileConfigYAML: %v", err)
}
cfg := Default()
err = cfg.applyFileConfigWithLookup(fileCfg, emptyLookup)
if err == nil || !strings.Contains(err.Error(), "llm profile id") || !strings.Contains(err.Error(), "duplicated") {
t.Fatalf("expected duplicate LLM profile ID error, got %v", err)
}
}
func TestApplyFileConfigRejectsDuplicateTrimmedPipelineIDs(t *testing.T) {
fileCfg, err := ParseFileConfigYAML([]byte(`
version: 1
pipelines:
example:
input: fake/input
" example ":
input: fake/other-input
`))
if err != nil {
t.Fatalf("ParseFileConfigYAML: %v", err)
}
cfg := Default()
err = cfg.applyFileConfigWithLookup(fileCfg, emptyLookup)
if err == nil || !strings.Contains(err.Error(), "pipeline id") || !strings.Contains(err.Error(), "duplicated") {
t.Fatalf("expected duplicate pipeline ID error, got %v", err)
}
}
func TestApplyFileConfigRejectsDuplicateTrimmedArtifactLaneIDs(t *testing.T) {
fileCfg, err := ParseFileConfigYAML([]byte(`
version: 1
pipelines:
example:
input: fake/input
artifacts:
events:
extract: fake/extract
" events ":
extract: fake/other-extract
`))
if err != nil {
t.Fatalf("ParseFileConfigYAML: %v", err)
}
cfg := Default()
err = cfg.applyFileConfigWithLookup(fileCfg, emptyLookup)
if err == nil || !strings.Contains(err.Error(), `pipeline "example" artifact lane id`) || !strings.Contains(err.Error(), "duplicated") {
t.Fatalf("expected duplicate artifact lane ID error, got %v", err)
}
}
func TestApplyFileConfigAllowsRetryOnlyLLMProfile(t *testing.T) {
cfg := parseAndApplyConfig(t, `
version: 1
llm_profiles:
retry-only:
max_retries: 3
`)
profile := cfg.LLMProfiles["retry-only"]
if profile.MaxRetries != 3 {
t.Fatalf("unexpected max retries: %d", profile.MaxRetries)
}
if profile.TimeoutSeconds != 0 {
t.Fatalf("expected unset timeout, got %d", profile.TimeoutSeconds)
}
if profile.MaxConcurrency != 0 {
t.Fatalf("expected unset max concurrency, got %d", profile.MaxConcurrency)
}
}
func TestApplyFileConfigRejectsInvalidAPIKeyEnv(t *testing.T) {
tests := []struct {
name string

9
internal/core/config/validation.go
View File

@@ -58,15 +58,6 @@ func validateLLMProfiles(profiles map[string]LLMProfile) error {
if profile.MaxConcurrency < 0 {
return fmt.Errorf("LLM profile %q max concurrency must not be negative", id)
}
if profile.TimeoutSeconds == 0 && profile.MaxRetries == 0 && profile.MaxConcurrency == 0 {
continue
}
if profile.TimeoutSeconds == 0 {
return fmt.Errorf("LLM profile %q timeout seconds must be greater than zero", id)
}
if profile.MaxConcurrency == 0 {
return fmt.Errorf("LLM profile %q max concurrency must be greater than zero", id)
}
}
return nil
}

11
internal/core/config/validation_test.go
View File

@@ -79,7 +79,7 @@ func TestValidateRejectsInvalidNumericFields(t *testing.T) {
name: "max concurrency",
mutate: func(cfg Config) Config {
profile := cfg.LLMProfiles["default"]
profile.MaxConcurrency = 0
profile.MaxConcurrency = -1
cfg.LLMProfiles["default"] = profile
return cfg
},
@@ -97,6 +97,15 @@ func TestValidateRejectsInvalidNumericFields(t *testing.T) {
}
}
func TestValidateAllowsPartialLLMProfileNumericConfig(t *testing.T) {
cfg := validConfig()
cfg.LLMProfiles["retry-only"] = LLMProfile{MaxRetries: 3}
if err := cfg.Validate(); err != nil {
t.Fatalf("Validate: %v", err)
}
}
func TestValidateRejectsInvalidDiagnosticsRetention(t *testing.T) {
cfg := validConfig()
cfg.Diagnostics.Retention = diagnostics.RetentionMode("sometimes")

38
internal/core/diagnostics/run_dir.go
View File

@@ -12,7 +12,14 @@ import (
"gitea.maximumdirect.net/eric/notarius/internal/framework/contracts"
)
const defaultWorkDir = "/tmp/notarius"
const (
defaultWorkDir = "/tmp/notarius"
maxRunDirectoryCreateAttempts = 16
)
var utcNow = func() time.Time {
return time.Now().UTC()
}
// RunDirectory represents a per-run diagnostics directory.
type RunDirectory struct {
@@ -77,18 +84,27 @@ func NewRunDirectory(workDir string, retention RetentionMode) (*RunDirectory, er
return nil, fmt.Errorf("create diagnostics work directory %q: %w", workDir, err)
}
createdAt := time.Now().UTC()
runID := fmt.Sprintf("run-%d", createdAt.UnixNano())
runPath := filepath.Join(workDir, runID)
if err := os.Mkdir(runPath, 0o755); err != nil {
return nil, fmt.Errorf("create diagnostics run directory %q: %w", runPath, err)
var lastRunPath string
for attempt := 0; attempt < maxRunDirectoryCreateAttempts; attempt++ {
createdAt := utcNow()
runID := fmt.Sprintf("run-%d", createdAt.UnixNano())
runPath := filepath.Join(workDir, runID)
lastRunPath = runPath
if err := os.Mkdir(runPath, 0o755); err != nil {
if os.IsExist(err) {
continue
}
return nil, fmt.Errorf("create diagnostics run directory %q: %w", runPath, err)
}
return &RunDirectory{
path: runPath,
retention: retention,
createdAt: createdAt,
}, nil
}
return &RunDirectory{
path: runPath,
retention: retention,
createdAt: createdAt,
}, nil
return nil, fmt.Errorf("create diagnostics run directory %q: exhausted unique run ID attempts", lastRunPath)
}
func (r *RunDirectory) Path() string {

60
internal/core/diagnostics/run_dir_test.go
View File

@@ -2,6 +2,7 @@ package diagnostics
import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"regexp"
@@ -35,6 +36,57 @@ func TestNewRunDirectoryCreatesRunDirectoryAndRunID(t *testing.T) {
}
}
func TestNewRunDirectoryRetriesOnRunIDCollision(t *testing.T) {
workDir := t.TempDir()
first := time.Unix(0, 100).UTC()
second := first.Add(time.Nanosecond)
if err := os.Mkdir(filepath.Join(workDir, fmt.Sprintf("run-%d", first.UnixNano())), 0o755); err != nil {
t.Fatalf("create existing run directory: %v", err)
}
restoreUTCNow := replaceUTCNow(func() func() time.Time {
calls := 0
return func() time.Time {
calls++
if calls == 1 {
return first
}
return second
}
}())
t.Cleanup(restoreUTCNow)
runDir, err := NewRunDirectory(workDir, RetentionAuto)
if err != nil {
t.Fatalf("NewRunDirectory: %v", err)
}
wantRunID := fmt.Sprintf("run-%d", second.UnixNano())
if runDir.RunID() != wantRunID {
t.Fatalf("RunID = %q, want %q", runDir.RunID(), wantRunID)
}
if _, err := os.Stat(runDir.Path()); err != nil {
t.Fatalf("stat run directory: %v", err)
}
}
func TestNewRunDirectoryReturnsErrorAfterRunIDCollisionsExhausted(t *testing.T) {
workDir := t.TempDir()
collisionTime := time.Unix(0, 200).UTC()
collisionPath := filepath.Join(workDir, fmt.Sprintf("run-%d", collisionTime.UnixNano()))
if err := os.Mkdir(collisionPath, 0o755); err != nil {
t.Fatalf("create existing run directory: %v", err)
}
restoreUTCNow := replaceUTCNow(func() time.Time {
return collisionTime
})
t.Cleanup(restoreUTCNow)
_, err := NewRunDirectory(workDir, RetentionAuto)
if err == nil || !strings.Contains(err.Error(), "exhausted unique run ID attempts") {
t.Fatalf("expected exhausted collision error, got %v", err)
}
}
func TestNewRunDirectoryUsesDefaultWorkDirectory(t *testing.T) {
runDir, err := NewRunDirectory("", RetentionAuto)
if err != nil {
@@ -252,6 +304,14 @@ func newTestRunDirectory(t *testing.T) *RunDirectory {
return runDir
}
func replaceUTCNow(replacement func() time.Time) func() {
original := utcNow
utcNow = replacement
return func() {
utcNow = original
}
}
func readArtifact(t *testing.T, runDir *RunDirectory, name string) []byte {
t.Helper()
data, err := os.ReadFile(filepath.Join(runDir.Path(), name))