caas-team / sparrow

A monitoring tool to gather infrastructure network information
Apache License 2.0
6 stars 4 forks source link

refactor: startup config validation #120

Closed lvlcn-t closed 3 months ago

lvlcn-t commented 3 months ago

Motivation

Currently the target manager config has a Validate method that is never consumed.

This PR is part of #66 which will implement another target manager type and therefore needs more target manager config validation. That's also when I noticed that the target manager config is never validated. 😄

Changes

I've refactored the startup config validation so every startup config component has its own Validate() method.

For additional information look at the commits.

Tests done

I've adjusted the existing unit tests.

TODO

lvlcn-t commented 3 months ago

The actions are failing because the PR comes from my fork.

All checks are succeeding here, where I've implemented the whole issue of #66 (also with the changes of this PR): https://github.com/caas-team/sparrow/tree/feat/git-tarman

lvlcn-t commented 3 months ago

@puffitos I agree with you, maybe we should change that in the future to returning to a structured error like this:

// ErrInvalidConfig is an error for invalid configuration
type ErrInvalidConfig struct {
    Field string
    Err   error
}

// Error returns the error message
func (e ErrInvalidConfig) Error() string {
    return fmt.Sprintf("invalid config for %q: %v", e.Field, e.Err)
}

But since it makes no difference for the user, I'll implement this in another PR in the near future. (I want to merge this to continue opening PR's for #66 😄 )

puffitos commented 3 months ago

@lvlcn-t the struct seems appropriate. The #111 will take priority in this iteration, just FIY.