config: add multiple database configuration

config/database.go

@@ -1,12 +1,93 @@
 package config
 
 import (
+	"fmt"
 	"net/url"
 	"os"
 	"strings"
 )
 
-func checkDSN(s string) (w []Warning, err error) {
+// CheckConnString reports a warning for using the "connstring" member instead
+// of the "database" member.
+//
+// This will panic if the type parameter is not [Matcher], [Indexer], or [Notifier].
+func checkConnString[T any](ws *[]Warning, v *T) {
+	var cs *string
+	var d *Database
+	switch v := any(v).(type) {
+	case *Matcher:
+		cs = &v.ConnString
+		d = v.Database
+	case *Indexer:
+		cs = &v.ConnString
+		d = v.Database
+	case *Notifier:
+		cs = &v.ConnString
+		d = v.Database
+	default:
+		panic(fmt.Sprintf("programmer error: passed unexpected type: %T", v))
+	}
+	if *cs != "" {
+		*ws = append(*ws, errConnString)
+	}
+	if d == nil {
+		*ws = append(*ws, Warning{
+			path: ".database",
+			msg:  `missing database configuration`,
+		})
+	}
+}
+
+// ErrConnString is reported by [checkConnString] if the "connstring" member is in use.
+var errConnString = Warning{
+	path:  ".connstring",
+	inner: fmt.Errorf(`using bare-string for database configuration deprecated: %w`, ErrDeprecated),
+}
+
+// SetConnString adjusts the passed variable by porting from the "connstring"
+// member if necessary.
+//
+// This will panic if the type parameter is not [Matcher], [Indexer], or [Notifier].
+func setConnString[T any](ws *[]Warning, v *T) {
+	var cs *string
+	var d *Database
+	var m *bool
+	switch v := any(v).(type) {
+	case *Matcher:
+		cs = &v.ConnString
+		d = v.Database
+		m = v.Migrations
+	case *Indexer:
+		cs = &v.ConnString
+		d = v.Database
+		m = v.Migrations
+	case *Notifier:
+		cs = &v.ConnString
+		d = v.Database
+		m = v.Migrations
+	default:
+		panic(fmt.Sprintf("programmer error: passed unexpected type: %T", v))
+	}
+	switch {
+	case *cs != "" && d != nil:
+		*cs = ""
+	case *cs != "" && d == nil:
+		d = &Database{
+			Name:       `postgresql`,
+			PostgreSQL: &DatabasePostgreSQL{DSN: *cs},
+			Migrations: m,
+		}
+		*cs = ""
+	case *cs == "" && d != nil: // OK, use as-is.
+	case *cs == "" && d == nil: // Will probably explode later.
+	}
+}
+
+// CheckPostgresqlDSN is a (very) light check that the value provided isn't completely bogus.
+//
+// Implementing more rigorous checks would be much more complicated.
+// That's not to say it'd be an unwelcome addition, just that it's very large and probably not needed.
+func checkPostgresqlDSN(s string) (w []Warning) {
 	switch {
 	case s == "":
 		// Nothing specified, make sure something's in the environment.
@@ -27,16 +108,69 @@ func checkDSN(s string) (w []Warning, err error) {
 		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",
 		})
+	case strings.ContainsRune(s, '='):
+		// Looks like a DSN
 	default:
 		w = append(w, Warning{
 			msg: "unable to make sense of connection string",
 		})
 	}
-	return w, nil
+	return w
+}
+
+// Database indicates the database configuration.
+type Database struct {
+	// Name indicates which database backend to use.
+	//
+	// This value must match the json/yaml tag.
+	Name string `json:"name" yaml:"name"`
+	// Migrations indicates if database migrations should run automatically.
+	Migrations *bool `json:"migrations,omitempty" yaml:"migrations,omitempty"`
+	// PostgreSQL is the PostgreSQL configuration.
+	PostgreSQL *DatabasePostgreSQL `json:"postgresql,omitempty" yaml:"postgresql,omitempty"`
+}
+
+func (d *Database) lint() (ws []Warning, err error) {
+	switch n := d.Name; n {
+	case "postgresql": // OK
+	case "postgres":
+		ws = append(ws, Warning{
+			msg:  fmt.Sprintf("unknown database: %q (did you mean %q?)", n, "postgresql"),
+			path: ".name",
+		})
+	default:
+		ws = append(ws, Warning{
+			msg:  fmt.Sprintf("unknown database: %q", n),
+			path: ".name",
+		})
+	}
+	return ws, nil
+}
+func (d *Database) validate(_ Mode) ([]Warning, error) {
+	return d.lint()
+}
+
+// DatabasePostgreSQL is the PostgreSQL-specific database configuration.
+//
+// Validation assumes that if the "DSN" member is empty but any environment variables with a "PG" prefix are present,
+// the configuration is specified in via environment variables.
+// This package implements no checking for the specifics of the DSN/URL/environment variables;
+// providing malformed values will fail at the point of use instead of configuration validation.
+type DatabasePostgreSQL struct {
+	// DSN is a data source name (aka "connection string") as documented for [libpq], with the extensions supported by [pgxpool].
+	//
+	// [libpq]: https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING
+	// [pgxpool]: https://pkg.go.dev/github.com/jackc/pgx/v4/pgxpool#ParseConfig
+	DSN string `json:"dsn" yaml:"dsn"`
+}
+
+func (d *DatabasePostgreSQL) lint() ([]Warning, error) {
+	return checkPostgresqlDSN(d.DSN), nil
+}
+func (d *DatabasePostgreSQL) validate(_ Mode) ([]Warning, error) {
+	return d.lint()
 }

config/database_test.go

@@ -0,0 +1,40 @@
+package config
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestDatabaseUnmarshal(t *testing.T) {
+	want := Database{
+		Name: "postgresql",
+		PostgreSQL: &DatabasePostgreSQL{
+			DSN: "host=test",
+		},
+	}
+	input := []string{
+		`{"name":"postgresql","postgresql":{"dsn":"host=test"}}`,
+	}
+
+	for _, tc := range input {
+		t.Logf("testing: %#q", tc)
+		var got Database
+		if err := json.Unmarshal([]byte(tc), &got); err != nil {
+			t.Error(err)
+			continue
+		}
+		ws, err := got.lint()
+		if err != nil {
+			t.Error(err)
+			continue
+		}
+		for _, w := range ws {
+			t.Logf("got lint: %v", &w)
+		}
+		if !cmp.Equal(&got, &want) {
+			t.Error(cmp.Diff(&got, &want))
+		}
+	}
+}

config/go.mod

@@ -1,5 +1,5 @@
 module github.com/quay/clair/config
 
-go 1.17
+go 1.18
 
 require github.com/google/go-cmp v0.6.0

config/indexer.go

@@ -12,7 +12,11 @@ type Indexer struct {
 	// url: "postgres://pqgotest:password@localhost/pqgotest?sslmode=verify-full"
 	// or
 	// string: "user=pqgotest dbname=pqgotest sslmode=verify-full"
-	ConnString string `yaml:"connstring" json:"connstring"`
+	//
+	// Deprecated: Use the ".database" member instead.
+	ConnString string `yaml:"connstring,omitempty" json:"connstring,omitempty"`
+	// Database is the database configuration.
+	Database *Database `yaml:"database,omitempty" json:"database,omitempty"`
 	// A positive value representing seconds.
 	//
 	// Concurrent Indexers lock on manifest scans to avoid clobbering.
@@ -34,7 +38,9 @@ type Indexer struct {
 	// A "true" or "false" value
 	//
 	// Whether Indexer nodes handle migrations to their database.
-	Migrations bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
+	//
+	// Deprecated: Use the ".database.migrations" member instead.
+	Migrations *bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
 	// Airgap disables HTTP access to the Internet. This affects both indexers and
 	// the layer fetcher. Database connections are unaffected.
 	//
@@ -66,18 +72,15 @@ func (i *Indexer) validate(mode Mode) (ws []Warning, err error) {
 			msg:  `automatically sizing number of concurrent requests`,
 		})
 	}
+
 	lws, err := i.lint()
+	setConnString(&ws, i)
 	return append(ws, lws...), err
 }
 
 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"
-	}
+	checkConnString(&ws, i)
+
 	if i.ScanLockRetry > 10 { // Guess at what a "large" value is here.
 		ws = append(ws, Warning{
 			path: ".scanlock_retry",

config/lint_test.go

@@ -14,11 +14,11 @@ func ExampleLint() {
 	// 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: connection string is empty and no relevant environment variables found (at $.matcher.connstring)
+	// warning: missing database configuration (at $.indexer.database)
+	// warning: missing database configuration (at $.matcher.database)
 	// 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: missing database configuration (at $.notifier.database)
 	// 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)
 }

config/matcher.go

@@ -13,7 +13,11 @@ type Matcher struct {
 	// url: "postgres://pqgotest:password@localhost/pqgotest?sslmode=verify-full"
 	// or
 	// string: "user=pqgotest dbname=pqgotest sslmode=verify-full"
+	//
+	// Deprecated: Use the ".database" member instead.
 	ConnString string `yaml:"connstring" json:"connstring"`
+	// Database is the database configuration.
+	Database *Database `yaml:"database,omitempty" json:"database,omitempty"`
 	// A string in <host>:<port> format where <host> can be an empty string.
 	//
 	// A Matcher contacts an Indexer to create a VulnerabilityReport.
@@ -36,7 +40,7 @@ type Matcher struct {
 	// Clair allows for a custom connection pool size.  This number will
 	// directly set how many active sql connections are allowed concurrently.
 	//
-	// Deprecated: Pool size should be set through the ConnString member.
+	// Deprecated: Pool size should be set through the database configuration.
 	// Currently, Clair only uses the "pgxpool" package to connect to the
 	// database, so see
 	// https://pkg.go.dev/github.com/jackc/pgx/v4/pgxpool#ParseConfig for more
@@ -51,15 +55,17 @@ type Matcher struct {
 	// A "true" or "false" value
 	//
 	// Whether Matcher nodes handle migrations to their databases.
-	Migrations bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
+	//
+	// Deprecated: Use the ".database.migrations" member instead.
+	Migrations *bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
 	// DisableUpdaters disables the updater's running of matchers.
 	//
 	// This should be toggled on if vulnerabilities are being provided by
 	// another mechanism.
 	DisableUpdaters bool `yaml:"disable_updaters,omitempty" json:"disable_updaters,omitempty"`
 }
 
-func (m *Matcher) validate(mode Mode) ([]Warning, error) {
+func (m *Matcher) validate(mode Mode) (ws []Warning, err error) {
 	if mode != ComboMode && mode != MatcherMode {
 		return nil, nil
 	}
@@ -90,17 +96,14 @@ func (m *Matcher) validate(mode Mode) ([]Warning, error) {
 	default:
 		panic("programmer error")
 	}
-	return m.lint()
+
+	lws, err := m.lint()
+	setConnString(&ws, m)
+	return append(ws, lws...), err
 }
 
 func (m *Matcher) lint() (ws []Warning, err error) {
-	ws, err = checkDSN(m.ConnString)
-	if err != nil {
-		return ws, err
-	}
-	for i := range ws {
-		ws[i].path = ".connstring"
-	}
+	checkConnString(&ws, m)
 
 	if m.Period < Duration(DefaultMatcherPeriod) {
 		ws = append(ws, Warning{

config/notifier.go

@@ -26,7 +26,11 @@ type Notifier struct {
 	// url: "postgres://pqgotest:password@localhost/pqgotest?sslmode=verify-full"
 	// or
 	// string: "user=pqgotest dbname=pqgotest sslmode=verify-full"
+	//
+	// Deprecated: Use the ".database" member instead.
 	ConnString string `yaml:"connstring" json:"connstring"`
+	// Database is the database configuration.
+	Database *Database `yaml:"database,omitempty" json:"database,omitempty"`
 	// A string in <host>:<port> format where <host> can be an empty string.
 	//
 	// A Notifier contacts an Indexer to create obtain manifests affected by vulnerabilities.
@@ -63,10 +67,12 @@ type Notifier struct {
 	// A "true" or "false" value
 	//
 	// Whether Notifier nodes handle migrations to their database.
-	Migrations bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
+	//
+	// Deprecated: Use the ".database.migrations" member instead.
+	Migrations *bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
 }
 
-func (n *Notifier) validate(mode Mode) ([]Warning, error) {
+func (n *Notifier) validate(mode Mode) (ws []Warning, err error) {
 	if mode != ComboMode && mode != NotifierMode {
 		return nil, nil
 	}
@@ -88,17 +94,15 @@ func (n *Notifier) validate(mode Mode) ([]Warning, error) {
 	default:
 		panic("programmer error")
 	}
-	return n.lint()
+
+	lws, err := n.lint()
+	setConnString(&ws, n)
+	return append(ws, lws...), err
 }
 
 func (n *Notifier) lint() (ws []Warning, err error) {
-	ws, err = checkDSN(n.ConnString)
-	if err != nil {
-		return ws, err
-	}
-	for i := range ws {
-		ws[i].path = ".connstring"
-	}
+	checkConnString(&ws, n)
+
 	got := 0
 	if n.AMQP != nil {
 		got++