config: add linter

Signed-off-by: Hank Donnay <hdonnay@redhat.com>

config/auth.go

@@ -3,6 +3,7 @@ package config
 import (
 	"encoding"
 	"encoding/base64"
+	"fmt"
 )
 
 // Base64 is a byte slice that encodes to and from base64-encoded strings.
@@ -48,6 +49,15 @@ func (a Auth) Any() bool {
 		a.Keyserver != nil
 }
 
+func (a *Auth) lint() ([]Warning, error) {
+	if a.PSK != nil && a.Keyserver != nil {
+		return []Warning{{
+			msg: `both "PSK" and "Keyserver" authentication methods are defined`,
+		}}, nil
+	}
+	return nil, nil
+}
+
 // AuthKeyserver is the configuration for doing authentication with the Quay
 // keyserver protocol.
 //
@@ -58,10 +68,32 @@ type AuthKeyserver struct {
 	Intraservice Base64 `yaml:"intraservice" json:"intraservice"`
 }
 
+func (a *AuthKeyserver) lint() ([]Warning, error) {
+	return []Warning{{
+		inner: fmt.Errorf(`authentication method deprecated: %w`, ErrDeprecated),
+	}}, nil
+}
+
 // AuthPSK is the configuration for doing pre-shared key based authentication.
 //
 // The "Issuer" key is what the service expects to verify as the "issuer" claim.
 type AuthPSK struct {
 	Key    Base64   `yaml:"key" json:"key"`
 	Issuer []string `yaml:"iss" json:"iss"`
 }
+
+func (a *AuthPSK) lint() (ws []Warning, err error) {
+	if len(a.Key) == 0 {
+		ws = append(ws, Warning{
+			path: ".key",
+			msg:  "key is empty",
+		})
+	}
+	if len(a.Issuer) == 0 {
+		ws = append(ws, Warning{
+			path: ".iss",
+			msg:  "no issuers defined",
+		})
+	}
+	return ws, nil
+}

config/config.go

@@ -20,6 +20,8 @@ const (
 // DefaultAddress is used if an http_listen_addr is not provided in the config.
 const DefaultAddress = ":6060"
 
+// Config is the configuration object for the commands in
+// github.com/quay/clair/v4/cmd/...
 type Config struct {
 	// One of the following strings
 	// Sets which mode the clair instances will run in
@@ -95,3 +97,19 @@ func Validate(conf *Config) error {
 	}
 	return nil
 }
+
+func (c *Config) lint() (ws []Warning, err error) {
+	if c.HTTPListenAddr == "" {
+		ws = append(ws, Warning{
+			path: ".http_listen_addr",
+			msg:  `http listen address not provided, default will be used`,
+		})
+	}
+	if c.IntrospectionAddr == "" {
+		ws = append(ws, Warning{
+			path: ".introspection_addr",
+			msg:  `introspection address not provided, default will be used`,
+		})
+	}
+	return ws, nil
+}

config/database.go

@@ -0,0 +1,42 @@
+package config
+
+import (
+	"net/url"
+	"os"
+	"strings"
+)
+
+func checkDSN(s string) (w []Warning, err error) {
+	switch {
+	case s == "":
+		// Nothing specified, make sure something's in the environment.
+		envSet := false
+		for _, k := range os.Environ() {
+			if strings.HasPrefix(k, `PG`) {
+				envSet = true
+				break
+			}
+		}
+		if !envSet {
+			w = append(w, Warning{
+				msg: "connection string is empty and no relevant environment variables found",
+			})
+		}
+	case strings.HasPrefix(s, "postgresql://"):
+		// Looks like a URL
+		if _, err := url.Parse(s); err != nil {
+			w = append(w, Warning{inner: err})
+		}
+	case strings.ContainsRune(s, '='):
+		// Looks like a DSN
+	case strings.Contains(s, `://`):
+		w = append(w, Warning{
+			msg: "connection string looks like a URL but scheme is unrecognized",
+		})
+	default:
+		w = append(w, Warning{
+			msg: "unable to make sense of connection string",
+		})
+	}
+	return w, nil
+}

config/indexer.go

@@ -21,7 +21,7 @@ type Indexer struct {
 	// This value tunes how often a waiting Indexer will poll for the lock.
 	// TODO: Move to async operating mode
 	ScanLockRetry int `yaml:"scanlock_retry" json:"scanlock_retry"`
-	// A positive values represeting quantity.
+	// A positive values representing quantity.
 	//
 	// Indexers will index a Manifest's layers concurrently.
 	// This value tunes the number of layers an Indexer will scan in parallel.
@@ -53,6 +53,46 @@ func (i *Indexer) Validate(combo bool) error {
 	return nil
 }
 
+func (i *Indexer) lint() (ws []Warning, err error) {
+	ws, err = checkDSN(i.ConnString)
+	if err != nil {
+		return ws, err
+	}
+	for i := range ws {
+		ws[i].path = ".connstring"
+	}
+	if i.ScanLockRetry > 10 { // Guess at what a "large" value is here.
+		ws = append(ws, Warning{
+			path: ".scanlock_retry",
+			msg:  `large values will increase latency`,
+		})
+	}
+	switch {
+	case i.LayerScanConcurrency == 0:
+		// Skip, autosized.
+	case i.LayerScanConcurrency < 4:
+		ws = append(ws, Warning{
+			path: ".layer_scan_concurrency",
+			msg:  `small values will limit resource utilization and increase latency`,
+		})
+	case i.LayerScanConcurrency > 32:
+		ws = append(ws, Warning{
+			path: ".layer_scan_concurrency",
+			msg:  `large values may exceed resource quotas`,
+		})
+	}
+	if i.IndexReportRequestConcurrency < 1 {
+		// Remove this lint if we come up with a heuristic instead of just
+		// "unlimited".
+		ws = append(ws, Warning{
+			path: ".index_report_request_concurrency",
+			msg:  `unlimited concurrent requests may exceed resource quotas`,
+		})
+	}
+
+	return ws, nil
+}
+
 type ScannerConfig struct {
 	Package map[string]interface{} `yaml:"package" json:"package"`
 	Dist    map[string]interface{} `yaml:"dist" json:"dist"`

config/introspection.go

@@ -1,13 +1,28 @@
 package config
 
+import "fmt"
+
 // Configure distributed tracing via OTEL
 type Trace struct {
 	Name        string   `yaml:"name" json:"name"`
 	Probability *float64 `yaml:"probability" json:"probability"`
 	Jaeger      Jaeger   `yaml:"jaeger" json:"jaeger"`
 }
 
-// Jager specific distributed tracing configuration.
+func (t *Trace) lint() ([]Warning, error) {
+	switch t.Name {
+	case "":
+	case "jaeger":
+	default:
+		return []Warning{{
+			path: ".name",
+			msg:  fmt.Sprintf(`unrecognized trace provider: %q`, t.Name),
+		}}, nil
+	}
+	return nil, nil
+}
+
+// Jaeger specific distributed tracing configuration.
 type Jaeger struct {
 	Tags  map[string]string `yaml:"tags" json:"tags"`
 	Agent struct {
@@ -28,6 +43,19 @@ type Metrics struct {
 	Name       string     `yaml:"name" json:"name"`
 }
 
+func (m *Metrics) lint() ([]Warning, error) {
+	switch m.Name {
+	case "":
+	case "prometheus":
+	default:
+		return []Warning{{
+			path: ".name",
+			msg:  fmt.Sprintf(`unrecognized metrics provider: %q`, m.Name),
+		}}, nil
+	}
+	return nil, nil
+}
+
 // Prometheus specific metrics configuration
 type Prometheus struct {
 	// Endpoint is a URL path where

config/lint.go

@@ -0,0 +1,131 @@
+package config
+
+import (
+	"errors"
+	"fmt"
+	"reflect"
+	"strings"
+)
+
+// Lint runs lints on the provided Config.
+//
+// An error is reported only if an error occurred while running the lints. An
+// invalid Config may still report a nil error along with a slice of Warnings.
+func Lint(c *Config) ([]Warning, error) {
+	// The linter does a teeny bit of reflection with an internal interface to
+	// check different values.
+	ws := []Warning{}
+	v := reflect.ValueOf(c)
+	path := "$"
+	var walk func(string, reflect.Value) error
+	walk = func(path string, v reflect.Value) error {
+		t := v.Type()
+		var vi interface{}
+		// Figure out if we should take the address to do the interface
+		// assertion, or if the value is already a pointer.
+		switch {
+		case t.Kind() != reflect.Ptr && v.CanAddr():
+			vi = v.Addr().Interface()
+		case v.CanInterface() && v.IsValid() && !v.IsZero():
+			vi = v.Interface()
+		}
+
+		if l, ok := vi.(linter); ok {
+			w, err := l.lint()
+			if err != nil {
+				return err
+			}
+			for i := range w {
+				// Adjust the path here, so that the lint method doesn't need to
+				// know where it is.
+				w[i].path = path + w[i].path
+			}
+			ws = append(ws, w...)
+		}
+
+		// Dereference the pointer, if this is a pointer.
+		if t.Kind() == reflect.Ptr {
+			t = t.Elem()
+			v = v.Elem()
+		}
+		if !v.IsValid() {
+			return nil
+		}
+		switch t.Kind() {
+		case reflect.Struct:
+			for i, lim := 0, t.NumField(); i < lim; i++ {
+				f := t.Field(i)
+				n := f.Name
+				if t := f.Tag.Get("json"); t != "" && t != "-" {
+					// Handle the comma options.
+					if i := strings.IndexByte(t, ','); i != -1 {
+						t = t[:i]
+					}
+					n = t
+				}
+				p := fmt.Sprintf(`%s.%s`, path, n)
+				if err := walk(p, v.Field(i)); err != nil {
+					return err
+				}
+			}
+		case reflect.Map:
+			i := v.MapRange()
+			for i.Next() {
+				p := fmt.Sprintf(`%s.[%s]`, path, i.Key().String())
+				if err := walk(p, i.Value()); err != nil {
+					return err
+				}
+
+			}
+		case reflect.Slice:
+			for i, lim := 0, v.Len(); i < lim; i++ {
+				p := fmt.Sprintf(`%s.[%d]`, path, i)
+				if err := walk(p, v.Index(i)); err != nil {
+					return err
+				}
+			}
+		default:
+			// everything else, just pass
+		}
+		return nil
+	}
+	return ws, walk(path, v)
+}
+
+// Types in this package can implement this interface to report common issues or
+// deprecation warnings.
+type linter interface {
+	lint() ([]Warning, error)
+}
+
+// Warning is a linter warning.
+//
+// Users can treat them like errors and use the sentinel values exported by this
+// package.
+type Warning struct {
+	inner error
+	path  string // json-schema style path
+	msg   string
+}
+
+// Should have inner xor msg
+
+func (w *Warning) Error() string {
+	var b strings.Builder
+	if w.inner != nil {
+		b.WriteString(w.inner.Error())
+	} else {
+		b.WriteString(w.msg)
+	}
+	b.WriteString(" (at ")
+	b.WriteString(w.path)
+	b.WriteRune(')')
+	return b.String()
+}
+
+func (w *Warning) Unwrap() error { return w.inner }
+
+// These are some common kinds of Warnings.
+var (
+	ErrDeprecated = errors.New("setting will be removed in a future release")
+)

config/lint_test.go

@@ -0,0 +1,30 @@
+package config
+
+import "fmt"
+
+func ExampleLint() {
+	var c Config
+	c.Auth.Keyserver = &AuthKeyserver{}
+	c.Auth.PSK = &AuthPSK{}
+	ws, err := Lint(&c)
+	fmt.Println("error:", err)
+	for _, w := range ws {
+		fmt.Printf("warning: %v\n", &w)
+	}
+	// Output:
+	// error: <nil>
+	// warning: http listen address not provided, default will be used (at $.http_listen_addr)
+	// warning: introspection address not provided, default will be used (at $.introspection_addr)
+	// warning: connection string is empty and no relevant environment variables found (at $.indexer.connstring)
+	// warning: unlimited concurrent requests may exceed resource quotas (at $.indexer.index_report_request_concurrency)
+	// warning: connection string is empty and no relevant environment variables found (at $.matcher.connstring)
+	// warning: updater period is very aggressive: most sources are updated daily (at $.matcher.period)
+	// warning: update garbage collection is off (at $.matcher.update_retention)
+	// warning: connection string is empty and no relevant environment variables found (at $.notifier.connstring)
+	// warning: interval is very fast: may result in increased workload (at $.notifier.poll_interval)
+	// warning: interval is very fast: may result in increased workload (at $.notifier.delivery_interval)
+	// warning: both "PSK" and "Keyserver" authentication methods are defined (at $.auth)
+	// warning: key is empty (at $.auth.psk.key)
+	// warning: no issuers defined (at $.auth.psk.iss)
+	// warning: authentication method deprecated: setting will be removed in a future release (at $.auth.keyserver)
+}