Skip to content

DSPP-450 Use go-viper/mapstructure and validator packages for config parsing#24

Open
ashley-graham wants to merge 7 commits into
mainfrom
INF-6667
Open

DSPP-450 Use go-viper/mapstructure and validator packages for config parsing#24
ashley-graham wants to merge 7 commits into
mainfrom
INF-6667

Conversation

@ashley-graham

@ashley-graham ashley-graham commented May 4, 2026

Copy link
Copy Markdown
Contributor

DSPP-450

Context
This PR replaces repeated manual config parsing from map[string]interface{} with a with a shared YAML decoding and validation path built on yaml.v3, mapstructure/v2, and validator packages.

The config shape stays the same for users:

  • watcher.type
  • watcher.config
  • executioner.type
  • executioner.config

Key Changes
Internally, each watcher and executioner now:

  • defines a typed config struct
  • uses mapstructure tags for field mapping
  • uses validate tags for required fields and value constraints
  • sets defaults before decoding
  • calls config.Decode

Unknown config fields now fail during load:

  • Invalid watcher/executioner types now fail through validation
  • Required fields, invalid values, and invalid field types are handled consistently
  • Defaults are preserved, including gce_metadata.metadata_error_wait_seconds = 10

Prioritization for Reviewers
I made some small changes to each of the sources of documentation in this repo. Those files aren't as important for review. I'd say, in order of priority, the changed files are:

  1. internal/goverseer/config/decode.go: Core shared decoder/validator logic.
  2. internal/goverseer/config/config.go: Top-level config structs, Viper file loading, and top-level validation tags.
  3. internal/goverseer/config/config_test.go: Coverage for the new stricter parsing behavior and shared decode helper.
  4. Watcher/executioner config structs (shell_executioner.go, file_watcher.go, gce_metadata_watcher.go, gcp_secrets_watcher.go, time_watcher.go, log_executioner.go)
  5. Docs (README.md, docs/watchers/*.md, docs/executioners/*.md)

If it would be better to put this into five stacked PRs instead of 5 commits, let me know.

Caveat on Commit History
Josh reached out and noted that we're not really getting all the benefits of viper here given our use case. So I've removed viper and instead directly required the mapstructure piece I wanted to keep. The decode.go and config.go were updated accordingly here.

@ashley-graham ashley-graham marked this pull request as ready for review May 5, 2026 04:20
@ashley-graham ashley-graham requested a review from a team as a code owner May 5, 2026 04:20
@ashley-graham ashley-graham requested review from cjonesy and twomblyj May 5, 2026 04:20
@ashley-graham ashley-graham changed the title INF-6667 Use viper and validator packages for config parsign INF-6667 Use viper and validator packages for config parsing May 5, 2026
@ipryseski ipryseski self-requested a review May 5, 2026 10:56
@ipryseski

Copy link
Copy Markdown

I don't think it needs to be multiple PRs. Just will take a little bit to look through everything.

@ashley-graham ashley-graham removed the request for review from twomblyj May 5, 2026 13:16
@ashley-graham

Copy link
Copy Markdown
Contributor Author

I don't think it needs to be multiple PRs. Just will take a little bit to look through everything.

Okay, thank you! And absolutely no rush as this was a second seashell thing for me.

@ashley-graham ashley-graham changed the title INF-6667 Use viper and validator packages for config parsing DSPP-450 Use viper and validator packages for config parsing May 29, 2026
@ashley-graham ashley-graham changed the title DSPP-450 Use viper and validator packages for config parsing DSPP-450 Use go-viper/mapstructure and validator packages for config parsing Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants