fix: config validation and flag-override precedence
Problem
Three config bugs let a misconfigured run produce a near-empty dashboard with exit 0, or silently ignore what the operator asked for.
window_days <= 0collected roughly one day, silently. A 0 or missing window made the collector window start at midnight today, so the dashboard showed near-zero adoption while the process exited 0. Nothing flagged it.- Integer parser overflow. The YAML scalar parser accumulated digits as
n = n*10 + digitwith no overflow guard, so a large integer such aswindow_days: 99999999999999999999wrapped to a garbage or negative value. - Flag overrides and config-file
skip_pipelinesdid not take effect.collect.goandrun.godetected "was this flag set" by comparing the value to the default (if *windowDays != 90), so an explicit--window-days 90was treated as unset and could not override a differing config-file value back to the default.LoadFilecould not propagateskip_pipelines: truefrom a config file: the bool zero value (false) was indistinguishable from "unset", so the merge dropped it. A test comment recorded this as a pre-existing gap.
Fix
- Added
Config.Validate, called in bothcollect.goandrun.goafter the config file and flags are merged. It returns an error whencollect.window_days <= 0. Unset still defaults to 90 viaDefaults(), so the default path is unchanged. parseScalarnow usesstrconv.Atoi, so an oversized or invalid integer returns an error instead of wrapping. The error propagates throughyamlToJSONwith a line number.- Precedence:
collect.goandrun.gonow detect explicitly provided flags withflag.FlagSet.Visit(matching the existingpurge.gopattern) and apply only those over the config. An explicit--window-days 90is honored.LoadFileparses into a tri-state struct that uses a*boolforskip_pipelines. A non-nil pointer means the file set the value, soskip_pipelines: true(andfalse) propagates through the merge. The CLI layer applies an explicit flag after the merge, so a flag still wins over the file.
Validation
Toolchain: Go 1.24.2.
make check(go vet plus full test suite): passed.go test -race ./...: passed.
New and converted assertions:
window_days <= 0returns an error; unset defaults to 90 and passes; a valid value passes.- An oversized integer (
99999999999999999999) returns an error at bothparseScalarandyamlToJSONlevel, not a wrapped negative. - An explicit
--window-days 90is honored even when the config sets a different value (Visit-based detection). - A config-file
skip_pipelines: truepropagates; the previously commented "pre-existing gap" inTestLoadFile_YAMLis now a real passing assertion.
Fails-on-old proof for skip_pipelines propagation: with the merge fix temporarily reverted, TestLoadFile_YAML failed with skip_pipelines: want true (from file), got false (and TestLoadFile_SkipPipelinesFalsePropagates failed for the false case). Restoring the tri-state merge made both pass.