Skip to content

Commit 5b3c9ed

Browse files
hdonnayhdonnay
committed
config: add multiple database configuration
This will give us the ability to add additional database backends if the need arises. Signed-off-by: Hank Donnay <[email protected]>
1 parent 1f98abe commit 5b3c9ed

File tree

7 files changed

+222
-38
lines changed

7 files changed

+222
-38
lines changed

‎config/database.go‎

Lines changed: 138 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,93 @@
11
package config
22

33
import (
4+
"fmt"
45
"net/url"
56
"os"
67
"strings"
78
)
89

9-
func checkDSN(s string) (w []Warning, err error) {
10+
// CheckConnString reports a warning for using the "connstring" member instead
11+
// of the "database" member.
12+
//
13+
// This will panic if the type parameter is not [Matcher], [Indexer], or [Notifier].
14+
func checkConnString[T any](ws *[]Warning, v *T) {
15+
var cs *string
16+
var d *Database
17+
switch v := any(v).(type) {
18+
case *Matcher:
19+
cs = &v.ConnString
20+
d = v.Database
21+
case *Indexer:
22+
cs = &v.ConnString
23+
d = v.Database
24+
case *Notifier:
25+
cs = &v.ConnString
26+
d = v.Database
27+
default:
28+
panic(fmt.Sprintf("programmer error: passed unexpected type: %T", v))
29+
}
30+
if *cs != "" {
31+
*ws = append(*ws, errConnString)
32+
}
33+
if d == nil {
34+
*ws = append(*ws, Warning{
35+
path: ".database",
36+
msg: `missing database configuration`,
37+
})
38+
}
39+
}
40+
41+
// ErrConnString is reported by [checkConnString] if the "connstring" member is in use.
42+
var errConnString = Warning{
43+
path: ".connstring",
44+
inner: fmt.Errorf(`using bare-string for database configuration deprecated: %w`, ErrDeprecated),
45+
}
46+
47+
// SetConnString adjusts the passed variable by porting from the "connstring"
48+
// member if necessary.
49+
//
50+
// This will panic if the type parameter is not [Matcher], [Indexer], or [Notifier].
51+
func setConnString[T any](ws *[]Warning, v *T) {
52+
var cs *string
53+
var d *Database
54+
var m *bool
55+
switch v := any(v).(type) {
56+
case *Matcher:
57+
cs = &v.ConnString
58+
d = v.Database
59+
m = v.Migrations
60+
case *Indexer:
61+
cs = &v.ConnString
62+
d = v.Database
63+
m = v.Migrations
64+
case *Notifier:
65+
cs = &v.ConnString
66+
d = v.Database
67+
m = v.Migrations
68+
default:
69+
panic(fmt.Sprintf("programmer error: passed unexpected type: %T", v))
70+
}
71+
switch {
72+
case *cs != "" && d != nil:
73+
*cs = ""
74+
case *cs != "" && d == nil:
75+
d = &Database{
76+
Name: `postgresql`,
77+
PostgreSQL: &DatabasePostgreSQL{DSN: *cs},
78+
Migrations: m,
79+
}
80+
*cs = ""
81+
case *cs == "" && d != nil: // OK, use as-is.
82+
case *cs == "" && d == nil: // Will probably explode later.
83+
}
84+
}
85+
86+
// CheckPostgresqlDSN is a (very) light check that the value provided isn't completely bogus.
87+
//
88+
// Implementing more rigorous checks would be much more complicated.
89+
// That's not to say it'd be an unwelcome addition, just that it's very large and probably not needed.
90+
func checkPostgresqlDSN(s string) (w []Warning) {
1091
switch {
1192
case s == "":
1293
// Nothing specified, make sure something's in the environment.
@@ -27,16 +108,69 @@ func checkDSN(s string) (w []Warning, err error) {
27108
if _, err := url.Parse(s); err != nil {
28109
w = append(w, Warning{inner: err})
29110
}
30-
case strings.ContainsRune(s, '='):
31-
// Looks like a DSN
32111
case strings.Contains(s, `://`):
33112
w = append(w, Warning{
34113
msg: "connection string looks like a URL but scheme is unrecognized",
35114
})
115+
case strings.ContainsRune(s, '='):
116+
// Looks like a DSN
36117
default:
37118
w = append(w, Warning{
38119
msg: "unable to make sense of connection string",
39120
})
40121
}
41-
return w, nil
122+
return w
123+
}
124+
125+
// Database indicates the database configuration.
126+
type Database struct {
127+
// Name indicates which database backend to use.
128+
//
129+
// This value must match the json/yaml tag.
130+
Name string `json:"name" yaml:"name"`
131+
// Migrations indicates if database migrations should run automatically.
132+
Migrations *bool `json:"migrations,omitempty" yaml:"migrations,omitempty"`
133+
// PostgreSQL is the PostgreSQL configuration.
134+
PostgreSQL *DatabasePostgreSQL `json:"postgresql,omitempty" yaml:"postgresql,omitempty"`
135+
}
136+
137+
func (d *Database) lint() (ws []Warning, err error) {
138+
switch n := d.Name; n {
139+
case "postgresql": // OK
140+
case "postgres":
141+
ws = append(ws, Warning{
142+
msg: fmt.Sprintf("unknown database: %q (did you mean %q?)", n, "postgresql"),
143+
path: ".name",
144+
})
145+
default:
146+
ws = append(ws, Warning{
147+
msg: fmt.Sprintf("unknown database: %q", n),
148+
path: ".name",
149+
})
150+
}
151+
return ws, nil
152+
}
153+
func (d *Database) validate(_ Mode) ([]Warning, error) {
154+
return d.lint()
155+
}
156+
157+
// DatabasePostgreSQL is the PostgreSQL-specific database configuration.
158+
//
159+
// Validation assumes that if the "DSN" member is empty but any environment variables with a "PG" prefix are present,
160+
// the configuration is specified in via environment variables.
161+
// This package implements no checking for the specifics of the DSN/URL/environment variables;
162+
// providing malformed values will fail at the point of use instead of configuration validation.
163+
type DatabasePostgreSQL struct {
164+
// DSN is a data source name (aka "connection string") as documented for [libpq], with the extensions supported by [pgxpool].
165+
//
166+
// [libpq]: https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING
167+
// [pgxpool]: https://pkg.go.dev/github.com/jackc/pgx/v4/pgxpool#ParseConfig
168+
DSN string `json:"dsn" yaml:"dsn"`
169+
}
170+
171+
func (d *DatabasePostgreSQL) lint() ([]Warning, error) {
172+
return checkPostgresqlDSN(d.DSN), nil
173+
}
174+
func (d *DatabasePostgreSQL) validate(_ Mode) ([]Warning, error) {
175+
return d.lint()
42176
}

‎config/database_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package config
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/google/go-cmp/cmp"
8+
)
9+
10+
func TestDatabaseUnmarshal(t *testing.T) {
11+
want := Database{
12+
Name: "postgresql",
13+
PostgreSQL: &DatabasePostgreSQL{
14+
DSN: "host=test",
15+
},
16+
}
17+
input := []string{
18+
`{"name":"postgresql","postgresql":{"dsn":"host=test"}}`,
19+
}
20+
21+
for _, tc := range input {
22+
t.Logf("testing: %#q", tc)
23+
var got Database
24+
if err := json.Unmarshal([]byte(tc), &got); err != nil {
25+
t.Error(err)
26+
continue
27+
}
28+
ws, err := got.lint()
29+
if err != nil {
30+
t.Error(err)
31+
continue
32+
}
33+
for _, w := range ws {
34+
t.Logf("got lint: %v", &w)
35+
}
36+
if !cmp.Equal(&got, &want) {
37+
t.Error(cmp.Diff(&got, &want))
38+
}
39+
}
40+
}

‎config/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
module github.com/quay/clair/config
22

3-
go 1.17
3+
go 1.18
44

55
require github.com/google/go-cmp v0.6.0

‎config/indexer.go‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ type Indexer struct {
1212
// url: "postgres://pqgotest:password@localhost/pqgotest?sslmode=verify-full"
1313
// or
1414
// string: "user=pqgotest dbname=pqgotest sslmode=verify-full"
15-
ConnString string `yaml:"connstring" json:"connstring"`
15+
//
16+
// Deprecated: Use the ".database" member instead.
17+
ConnString string `yaml:"connstring,omitempty" json:"connstring,omitempty"`
18+
// Database is the database configuration.
19+
Database *Database `yaml:"database,omitempty" json:"database,omitempty"`
1620
// A positive value representing seconds.
1721
//
1822
// Concurrent Indexers lock on manifest scans to avoid clobbering.
@@ -34,7 +38,9 @@ type Indexer struct {
3438
// A "true" or "false" value
3539
//
3640
// Whether Indexer nodes handle migrations to their database.
37-
Migrations bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
41+
//
42+
// Deprecated: Use the ".database.migrations" member instead.
43+
Migrations *bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
3844
// Airgap disables HTTP access to the Internet. This affects both indexers and
3945
// the layer fetcher. Database connections are unaffected.
4046
//
@@ -66,18 +72,15 @@ func (i *Indexer) validate(mode Mode) (ws []Warning, err error) {
6672
msg: `automatically sizing number of concurrent requests`,
6773
})
6874
}
75+
6976
lws, err := i.lint()
77+
setConnString(&ws, i)
7078
return append(ws, lws...), err
7179
}
7280

7381
func (i *Indexer) lint() (ws []Warning, err error) {
74-
ws, err = checkDSN(i.ConnString)
75-
if err != nil {
76-
return ws, err
77-
}
78-
for i := range ws {
79-
ws[i].path = ".connstring"
80-
}
82+
checkConnString(&ws, i)
83+
8184
if i.ScanLockRetry > 10 { // Guess at what a "large" value is here.
8285
ws = append(ws, Warning{
8386
path: ".scanlock_retry",

‎config/lint_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ func ExampleLint() {
1414
// error: <nil>
1515
// warning: http listen address not provided, default will be used (at $.http_listen_addr)
1616
// warning: introspection address not provided, default will be used (at $.introspection_addr)
17-
// warning: connection string is empty and no relevant environment variables found (at $.indexer.connstring)
18-
// warning: connection string is empty and no relevant environment variables found (at $.matcher.connstring)
17+
// warning: missing database configuration (at $.indexer.database)
18+
// warning: missing database configuration (at $.matcher.database)
1919
// warning: updater period is very aggressive: most sources are updated daily (at $.matcher.period)
2020
// warning: update garbage collection is off (at $.matcher.update_retention)
21-
// warning: connection string is empty and no relevant environment variables found (at $.notifier.connstring)
21+
// warning: missing database configuration (at $.notifier.database)
2222
// warning: interval is very fast: may result in increased workload (at $.notifier.poll_interval)
2323
// warning: interval is very fast: may result in increased workload (at $.notifier.delivery_interval)
2424
}

‎config/matcher.go‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ type Matcher struct {
1313
// url: "postgres://pqgotest:password@localhost/pqgotest?sslmode=verify-full"
1414
// or
1515
// string: "user=pqgotest dbname=pqgotest sslmode=verify-full"
16+
//
17+
// Deprecated: Use the ".database" member instead.
1618
ConnString string `yaml:"connstring" json:"connstring"`
19+
// Database is the database configuration.
20+
Database *Database `yaml:"database,omitempty" json:"database,omitempty"`
1721
// A string in <host>:<port> format where <host> can be an empty string.
1822
//
1923
// A Matcher contacts an Indexer to create a VulnerabilityReport.
@@ -36,7 +40,7 @@ type Matcher struct {
3640
// Clair allows for a custom connection pool size. This number will
3741
// directly set how many active sql connections are allowed concurrently.
3842
//
39-
// Deprecated: Pool size should be set through the ConnString member.
43+
// Deprecated: Pool size should be set through the database configuration.
4044
// Currently, Clair only uses the "pgxpool" package to connect to the
4145
// database, so see
4246
// https://pkg.go.dev/github.com/jackc/pgx/v4/pgxpool#ParseConfig for more
@@ -51,15 +55,17 @@ type Matcher struct {
5155
// A "true" or "false" value
5256
//
5357
// Whether Matcher nodes handle migrations to their databases.
54-
Migrations bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
58+
//
59+
// Deprecated: Use the ".database.migrations" member instead.
60+
Migrations *bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
5561
// DisableUpdaters disables the updater's running of matchers.
5662
//
5763
// This should be toggled on if vulnerabilities are being provided by
5864
// another mechanism.
5965
DisableUpdaters bool `yaml:"disable_updaters,omitempty" json:"disable_updaters,omitempty"`
6066
}
6167

62-
func (m *Matcher) validate(mode Mode) ([]Warning, error) {
68+
func (m *Matcher) validate(mode Mode) (ws []Warning, err error) {
6369
if mode != ComboMode && mode != MatcherMode {
6470
return nil, nil
6571
}
@@ -90,17 +96,14 @@ func (m *Matcher) validate(mode Mode) ([]Warning, error) {
9096
default:
9197
panic("programmer error")
9298
}
93-
return m.lint()
99+
100+
lws, err := m.lint()
101+
setConnString(&ws, m)
102+
return append(ws, lws...), err
94103
}
95104

96105
func (m *Matcher) lint() (ws []Warning, err error) {
97-
ws, err = checkDSN(m.ConnString)
98-
if err != nil {
99-
return ws, err
100-
}
101-
for i := range ws {
102-
ws[i].path = ".connstring"
103-
}
106+
checkConnString(&ws, m)
104107

105108
if m.Period < Duration(DefaultMatcherPeriod) {
106109
ws = append(ws, Warning{

‎config/notifier.go‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ type Notifier struct {
2626
// url: "postgres://pqgotest:password@localhost/pqgotest?sslmode=verify-full"
2727
// or
2828
// string: "user=pqgotest dbname=pqgotest sslmode=verify-full"
29+
//
30+
// Deprecated: Use the ".database" member instead.
2931
ConnString string `yaml:"connstring" json:"connstring"`
32+
// Database is the database configuration.
33+
Database *Database `yaml:"database,omitempty" json:"database,omitempty"`
3034
// A string in <host>:<port> format where <host> can be an empty string.
3135
//
3236
// A Notifier contacts an Indexer to create obtain manifests affected by vulnerabilities.
@@ -63,10 +67,12 @@ type Notifier struct {
6367
// A "true" or "false" value
6468
//
6569
// Whether Notifier nodes handle migrations to their database.
66-
Migrations bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
70+
//
71+
// Deprecated: Use the ".database.migrations" member instead.
72+
Migrations *bool `yaml:"migrations,omitempty" json:"migrations,omitempty"`
6773
}
6874

69-
func (n *Notifier) validate(mode Mode) ([]Warning, error) {
75+
func (n *Notifier) validate(mode Mode) (ws []Warning, err error) {
7076
if mode != ComboMode && mode != NotifierMode {
7177
return nil, nil
7278
}
@@ -88,17 +94,15 @@ func (n *Notifier) validate(mode Mode) ([]Warning, error) {
8894
default:
8995
panic("programmer error")
9096
}
91-
return n.lint()
97+
98+
lws, err := n.lint()
99+
setConnString(&ws, n)
100+
return append(ws, lws...), err
92101
}
93102

94103
func (n *Notifier) lint() (ws []Warning, err error) {
95-
ws, err = checkDSN(n.ConnString)
96-
if err != nil {
97-
return ws, err
98-
}
99-
for i := range ws {
100-
ws[i].path = ".connstring"
101-
}
104+
checkConnString(&ws, n)
105+
102106
got := 0
103107
if n.AMQP != nil {
104108
got++

0 commit comments

Comments
 (0)