Skip to content

Commit

Permalink
feat: Extended validation of config files (#781)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?

- Config file validation did not include any values specified on the
command line or in the environment.

## Short description of the changes

- After initial validation of the raw config, we unmarshal it into a
config structure, write it back out to a local YAML buffer, and then
revalidate the now-hydrated config. This also required masking the keys
in the error output, injecting fake keys if they weren't specified at
all, and fixing a test.

Fixes #775.
  • Loading branch information
kentquirk authored Jul 11, 2023
1 parent 83d1f6e commit d068946
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 6 deletions.
63 changes: 60 additions & 3 deletions config/configLoadHelpers.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package config

import (
"bytes"
"crypto/md5"
"encoding/hex"
"encoding/json"
Expand Down Expand Up @@ -106,7 +107,8 @@ func load(r io.Reader, format Format, into any) error {
}
}

func validateConfig(location string) ([]string, error) {
func validateConfig(opts *CmdEnv) ([]string, error) {
location := opts.ConfigLocation
r, format, err := getReaderFor(location)
if err != nil {
return nil, err
Expand All @@ -115,7 +117,7 @@ func validateConfig(location string) ([]string, error) {

var userData map[string]any
if err := load(r, format, &userData); err != nil {
return nil, fmt.Errorf("readConfigInto unable to load config %s: %w", location, err)
return nil, fmt.Errorf("validateConfig unable to load config %s: %w", location, err)
}

metadata, err := LoadConfigMetadata()
Expand All @@ -124,6 +126,61 @@ func validateConfig(location string) ([]string, error) {
}

failures := metadata.Validate(userData)
if len(failures) > 0 {
return failures, nil
}

// Basic validation worked. Now we need to reload it into the struct so that
// we can apply defaults and options, and then validate a second time.

// we need a new reader for the source data
r2, _, err := getReaderFor(location)
if err != nil {
return nil, err
}
defer r2.Close()

var config configContents
if err := load(r2, format, &config); err != nil {
// this should never happen, since we already validated the config
return nil, fmt.Errorf("validateConfig unable to RELOAD config %s: %w", location, err)
}
// apply defaults and options
if err := defaults.Set(&config); err != nil {
return nil, fmt.Errorf("readConfigInto unable to apply defaults: %w", err)
}

// apply command line options
if err := opts.ApplyTags(reflect.ValueOf(&config)); err != nil {
return nil, fmt.Errorf("readConfigInto unable to apply command line options: %w", err)
}

// possibly inject some keys to keep the validator happy
if config.HoneycombLogger.APIKey == "" {
config.HoneycombLogger.APIKey = "InvalidHoneycombAPIKey"
}
if config.LegacyMetrics.APIKey == "" {
config.LegacyMetrics.APIKey = "InvalidHoneycombAPIKey"
}
if config.OTelMetrics.APIKey == "" {
config.OTelMetrics.APIKey = "InvalidHoneycombAPIKey"
}

// write it out to a YAML buffer
buf := new(bytes.Buffer)
encoder := yaml.NewEncoder(buf)
encoder.SetIndent(2)
if err := encoder.Encode(config); err != nil {
return nil, fmt.Errorf("readConfigInto unable to reencode config: %w", err)
}

var rewrittenUserData map[string]any
if err := load(buf, format, &rewrittenUserData); err != nil {
return nil, fmt.Errorf("validateConfig unable to reload hydrated config from buffer: %w", err)
}

// and finally validate the rewritten config
failures = metadata.Validate(rewrittenUserData)
return failures, nil
}

Expand All @@ -136,7 +193,7 @@ func validateRules(location string) ([]string, error) {

var userData map[string]any
if err := load(r, format, &userData); err != nil {
return nil, fmt.Errorf("readConfigInto unable to load config %s: %w", location, err)
return nil, fmt.Errorf("validateRules unable to load config %s: %w", location, err)
}

metadata, err := LoadRulesMetadata()
Expand Down
2 changes: 1 addition & 1 deletion config/file_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func (e *FileConfigError) Error() string {
func newFileConfig(opts *CmdEnv) (*fileConfig, error) {
// If we're not validating, skip this part
if !opts.NoValidate {
cfgFails, err := validateConfig(opts.ConfigLocation)
cfgFails, err := validateConfig(opts)
if err != nil {
return nil, err
}
Expand Down
14 changes: 13 additions & 1 deletion config/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,13 @@ func validateDatatype(k string, v any, typ string) string {
return ""
}

func maskString(s string) string {
if len(s) < 4 {
return "****"
}
return "****" + s[len(s)-4:]
}

// Validate checks that the given data is valid according to the metadata.
// It returns a list of errors, or an empty list if there are no errors.
// The errors are strings that are suitable for showing to the user.
Expand Down Expand Up @@ -249,10 +256,12 @@ func (m *Metadata) Validate(data map[string]any) []string {
case "format":
var pat *regexp.Regexp
var format string
mask := false
switch validation.Arg.(string) {
case "apikey":
pat = regexp.MustCompile(`^[a-f0-9]{32}|[a-zA-Z0-9]{20,23}$`)
format = "field %s (%v) must be a Honeycomb API key"
format = "field %s (%v) must be a valid Honeycomb API key"
mask = true
case "version":
pat = regexp.MustCompile(`^v[0-9]+\.[0-9]+$`)
format = "field %s (%v) must be a valid major.minor version number, like v2.0"
Expand All @@ -263,6 +272,9 @@ func (m *Metadata) Validate(data map[string]any) []string {
panic("unknown pattern type " + validation.Arg.(string))
}
if !(isString(v) && pat.MatchString(v.(string))) {
if mask {
v = maskString(v.(string))
}
errors = append(errors, fmt.Sprintf(format, k, v))
}
case "minimum":
Expand Down
4 changes: 3 additions & 1 deletion config/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,9 @@ func Test_validate(t *testing.T) {
{"bad choice", mm("Traces.AChoice", "Z"),
`field Traces.AChoice (Z) must be one of [A B C]`},
{"bad format apikey", mm("Network.APIKey", "abc"),
`field Network.APIKey (abc) must be a Honeycomb API key`},
`field Network.APIKey (****) must be a valid Honeycomb API key`},
{"bad format apikey long", mm("Network.APIKey", "abc123abc123whee"),
`field Network.APIKey (****whee) must be a valid Honeycomb API key`},
{"good format apikey", mm("Network.APIKey", "abc123abc123abc123abc123abc123ab"), ""},
{"good format apikey", mm("Network.APIKey", "NewStyleKeyWith22chars"), ""},
{"good format version", mm("General.Version", "v2.0"), ""},
Expand Down

0 comments on commit d068946

Please sign in to comment.