pkg/revocation: ensure we close revocation databases (#2825)

cmd/bootstrap/main.go

@@ -88,12 +88,15 @@ func cmdRun(cmd *cobra.Command, args []string) (err error) {
 		err = errs.Combine(err, db.Close())
 	}()
 
-	revDB, err := revocation.NewDBFromCfg(runCfg.Server.Config)
+	revocationDB, err := revocation.NewDBFromCfg(runCfg.Server.Config)
 	if err != nil {
 		return errs.New("Error creating revocation database: %+v", err)
 	}
+	defer func() {
+		err = errs.Combine(err, revocationDB.Close())
+	}()
 
-	peer, err := bootstrap.New(log, identity, db, revDB, runCfg, version.Build)
+	peer, err := bootstrap.New(log, identity, db, revocationDB, runCfg, version.Build)
 	if err != nil {
 		return err
 	}

cmd/certificates/main.go

@@ -62,12 +62,15 @@ func cmdRun(cmd *cobra.Command, args []string) error {
 		zap.S().Fatal(err)
 	}
 
-	revDB, err := revocation.NewDBFromCfg(config.Server.Config.Config)
+	revocationDB, err := revocation.NewDBFromCfg(config.Server.Config.Config)
 	if err != nil {
 		return errs.New("Error creating revocation database: %+v", err)
 	}
+	defer func() {
+		err = errs.Combine(err, revocationDB.Close())
+	}()
 
-	return config.Server.Run(ctx, zap.L(), identity, revDB, nil, config.Signer)
+	return config.Server.Run(ctx, zap.L(), identity, revocationDB, nil, config.Signer)
 }
 
 func main() {

cmd/identity/main.go

@@ -186,12 +186,15 @@ func cmdAuthorize(cmd *cobra.Command, args []string) error {
 	// Ensure we dont enforce a signed Peer Identity
 	config.Signer.TLS.UsePeerCAWhitelist = false
 
-	revDB, err := revocation.NewDBFromCfg(config.Signer.TLS)
+	revocationDB, err := revocation.NewDBFromCfg(config.Signer.TLS)
 	if err != nil {
 		return errs.New("Error creating revocation database: %+v", err)
 	}
+	defer func() {
+		err = errs.Combine(err, revocationDB.Close())
+	}()
 
-	signedChainBytes, err := config.Signer.Sign(ctx, ident, authToken, revDB)
+	signedChainBytes, err := config.Signer.Sign(ctx, ident, authToken, revocationDB)
 	if err != nil {
 		return errs.New("error occurred while signing certificate: %s\n(identity files were still generated and saved, if you try again existing files will be loaded)", err)
 	}

cmd/satellite/main.go

@@ -131,12 +131,15 @@ func cmdRun(cmd *cobra.Command, args []string) (err error) {
 		err = errs.Combine(err, db.Close())
 	}()
 
-	revDB, err := revocation.NewDBFromCfg(runCfg.Config.Server.Config)
+	revocationDB, err := revocation.NewDBFromCfg(runCfg.Config.Server.Config)
 	if err != nil {
 		return errs.New("Error creating revocation database: %+v", err)
 	}
+	defer func() {
+		err = errs.Combine(err, revocationDB.Close())
+	}()
 
-	peer, err := satellite.New(log, identity, db, revDB, &runCfg.Config, version.Build)
+	peer, err := satellite.New(log, identity, db, revocationDB, &runCfg.Config, version.Build)
 	if err != nil {
 		return err
 	}

cmd/storagenode/main.go

@@ -140,12 +140,15 @@ func cmdRun(cmd *cobra.Command, args []string) (err error) {
 		err = errs.Combine(err, db.Close())
 	}()
 
-	revDB, err := revocation.NewDBFromCfg(runCfg.Server.Config)
+	revocationDB, err := revocation.NewDBFromCfg(runCfg.Server.Config)
 	if err != nil {
 		return errs.New("Error creating revocation database: %+v", err)
 	}
+	defer func() {
+		err = errs.Combine(err, revocationDB.Close())
+	}()
 
-	peer, err := storagenode.New(log, identity, db, revDB, runCfg.Config, version.Build)
+	peer, err := storagenode.New(log, identity, db, revocationDB, runCfg.Config, version.Build)
 	if err != nil {
 		return err
 	}

internal/testplanet/bootstrap.go

@@ -100,12 +100,15 @@ func (planet *Planet) newBootstrap() (peer *bootstrap.Peer, err error) {
 	var verInfo version.Info
 	verInfo = planet.NewVersionInfo()
 
-	revDB, err := revocation.NewDBFromCfg(config.Server.Config)
+	revocationDB, err := revocation.NewDBFromCfg(config.Server.Config)
 	if err != nil {
 		return nil, errs.New("Error creating revocation database: %+v", err)
 	}
+	defer func() {
+		err = errs.Combine(err, revocationDB.Close())
+	}()
 
-	peer, err = bootstrap.New(log, identity, db, revDB, config, verInfo)
+	peer, err = bootstrap.New(log, identity, db, revocationDB, config, verInfo)
 	if err != nil {
 		return nil, err
 	}

internal/testplanet/satellite.go

@@ -70,13 +70,6 @@ func (planet *Planet) newSatellites(count int) ([]*satellite.Peer, error) {
 			return nil, err
 		}
 
-		err = db.CreateTables()
-		if err != nil {
-			return nil, err
-		}
-
-		planet.databases = append(planet.databases, db)
-
 		config := satellite.Config{
 			Server: server.Config{
 				Address:        "127.0.0.1:0",
@@ -216,16 +209,23 @@ func (planet *Planet) newSatellites(count int) ([]*satellite.Peer, error) {
 
 		verInfo := planet.NewVersionInfo()
 
-		revDB, err := revocation.NewDBFromCfg(config.Server.Config)
+		revocationDB, err := revocation.NewDBFromCfg(config.Server.Config)
 		if err != nil {
-			return xs, errs.New("Error creating revocation database: %+v", err)
+			return xs, errs.Wrap(err)
 		}
+		planet.databases = append(planet.databases, revocationDB)
 
-		peer, err := satellite.New(log, identity, db, revDB, &config, verInfo)
+		peer, err := satellite.New(log, identity, db, revocationDB, &config, verInfo)
 		if err != nil {
 			return xs, err
 		}
 
+		err = db.CreateTables()
+		if err != nil {
+			return nil, err
+		}
+		planet.databases = append(planet.databases, db)
+
 		log.Debug("id=" + peer.ID().String() + " addr=" + peer.Addr())
 		xs = append(xs, peer)
 	}

internal/testplanet/storagenode.go

@@ -162,12 +162,13 @@ func (planet *Planet) newStorageNodes(count int, whitelistedSatellites storj.Nod
 			}
 		}
 
-		revDB, err := revocation.NewDBFromCfg(config.Server.Config)
+		revocationDB, err := revocation.NewDBFromCfg(config.Server.Config)
 		if err != nil {
-			return nil, errs.New("Error creating revocation database: %+v", err)
+			return xs, errs.Wrap(err)
 		}
+		planet.databases = append(planet.databases, revocationDB)
 
-		peer, err := storagenode.New(log, identity, db, revDB, config, verInfo)
+		peer, err := storagenode.New(log, identity, db, revocationDB, config, verInfo)
 		if err != nil {
 			return xs, err
 		}
@@ -176,7 +177,6 @@ func (planet *Planet) newStorageNodes(count int, whitelistedSatellites storj.Nod
 		if err != nil {
 			return nil, err
 		}
-
 		planet.databases = append(planet.databases, db)
 
 		log.Debug("id=" + peer.ID().String() + " addr=" + peer.Addr())

internal/testplanet/uplink_test.go

@@ -215,17 +215,23 @@ func TestDownloadFromUnresponsiveNode(t *testing.T) {
 						WhitelistSignedLeaf: false,
 					},
 				}
-				revDB, err := revocation.NewDBFromCfg(tlscfg)
+
+				revocationDB, err := revocation.NewDBFromCfg(tlscfg)
 				require.NoError(t, err)
-				options, err := tlsopts.NewOptions(storageNode.Identity, tlscfg, revDB)
+
+				options, err := tlsopts.NewOptions(storageNode.Identity, tlscfg, revocationDB)
 				require.NoError(t, err)
 
 				server, err := server.New(storageNode.Log.Named("mock-server"), options, storageNode.Addr(), storageNode.PrivateAddr(), nil)
 				require.NoError(t, err)
 				pb.RegisterPiecestoreServer(server.GRPC(), &piecestoreMock{})
 				go func() {
+					// TODO: get goroutine under control
 					err := server.Run(ctx)
 					require.NoError(t, err)
+
+					err = revocationDB.Close()
+					require.NoError(t, err)
 				}()
 				stopped = true
 				break

internal/testrevocation/db.go

@@ -0,0 +1,48 @@
+// Copyright (C) 2019 Storj Labs, Inc.
+// See LICENSE for copying information.
+
+package testrevocation
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"storj.io/storj/internal/testcontext"
+	"storj.io/storj/pkg/peertls/extensions"
+	"storj.io/storj/pkg/revocation"
+	"storj.io/storj/storage"
+	"storj.io/storj/storage/redis/redisserver"
+)
+
+// RunDBs runs the passed test function with each type of revocation database.
+func RunDBs(t *testing.T, test func(*testing.T, extensions.RevocationDB, storage.KeyValueStore)) {
+	t.Run("Redis", func(t *testing.T) {
+		ctx := testcontext.New(t)
+		defer ctx.Cleanup()
+
+		addr, cleanup, err := redisserver.Start()
+		require.NoError(t, err)
+		defer cleanup()
+
+		// Test using redis-backed revocation DB
+		dbURL := "redis://" + addr + "?db=0"
+		db, err := revocation.NewDB(dbURL)
+		require.NoError(t, err)
+		defer ctx.Check(db.Close)
+
+		test(t, db, db.TestGetStore())
+	})
+
+	t.Run("Bolt", func(t *testing.T) {
+		ctx := testcontext.New(t)
+		defer ctx.Cleanup()
+
+		// Test using bolt-backed revocation DB
+		db, err := revocation.NewDB("bolt://" + ctx.File("revocations.db"))
+		require.NoError(t, err)
+		defer ctx.Check(db.Close)
+
+		test(t, db, db.TestGetStore())
+	})
+}

internal/testrevocation/revocation_db.go

@@ -1,57 +0,0 @@
-// Copyright (C) 2019 Storj Labs, Inc.
-// See LICENSE for copying information.
-
-package testrevocation
-
-import (
-	"testing"
-
-	"github.com/alicebob/miniredis"
-	"github.com/stretchr/testify/require"
-
-	"storj.io/storj/internal/testcontext"
-	"storj.io/storj/pkg/peertls/extensions"
-	"storj.io/storj/pkg/revocation"
-	"storj.io/storj/storage"
-)
-
-// RevocationDBsTest runs the passed test function with each type of revocation database.
-func RevocationDBsTest(t *testing.T, test func(*testing.T, extensions.RevocationDB, storage.KeyValueStore)) {
-
-	t.Run("Redis-backed revocation DB", func(t *testing.T) {
-		ctx := testcontext.New(t)
-		defer ctx.Cleanup()
-
-		redisServer, err := miniredis.Run()
-		require.NoError(t, err)
-		defer redisServer.Close()
-
-		{
-			// Test using redis-backed revocation DB
-			dbURL := "redis://" + redisServer.Addr() + "?db=0"
-			redisRevDB, err := revocation.NewDB(dbURL)
-			require.NoError(t, err)
-			defer ctx.Check(redisRevDB.Close)
-
-			test(t, redisRevDB, redisRevDB.KVStore)
-		}
-
-	})
-
-	t.Run("Bolt-backed revocation DB", func(t *testing.T) {
-		{
-			ctx := testcontext.New(t)
-			defer ctx.Cleanup()
-
-			// Test using bolt-backed revocation DB
-			revocationDBPath := ctx.File("revocations.db")
-
-			dbURL := "bolt://" + revocationDBPath
-			boltRevDB, err := revocation.NewDB(dbURL)
-			require.NoError(t, err)
-			defer ctx.Check(boltRevDB.Close)
-
-			test(t, boltRevDB, boltRevDB.KVStore)
-		}
-	})
-}

pkg/certificates/certificates_test.go

@@ -607,10 +607,11 @@ func TestCertificateSigner_Sign_E2E(t *testing.T) {
 					PrivateAddress: "127.0.0.1:0",
 				}
 
-				revDB, err := revocation.NewDBFromCfg(sc.Config)
+				revocationDB, err := revocation.NewDBFromCfg(sc.Config)
 				require.NoError(t, err)
+				defer ctx.Check(revocationDB.Close)
 
-				serverOpts, err := tlsopts.NewOptions(serverIdent, sc.Config, revDB)
+				serverOpts, err := tlsopts.NewOptions(serverIdent, sc.Config, revocationDB)
 				require.NoError(t, err)
 				require.NotNil(t, serverOpts)
 

pkg/peertls/extensions/extensions.go

@@ -73,7 +73,7 @@ type Config struct {
 // Options holds common options for use in handling extensions.
 type Options struct {
 	PeerCAWhitelist []*x509.Certificate
-	RevDB           RevocationDB
+	RevocationDB    RevocationDB
 	PeerIDVersions  string
 }
 

pkg/peertls/extensions/revocations.go

@@ -53,7 +53,6 @@ type RevocationDB interface {
 	Get(ctx context.Context, chain []*x509.Certificate) (*Revocation, error)
 	Put(ctx context.Context, chain []*x509.Certificate, ext pkix.Extension) error
 	List(ctx context.Context) ([]*Revocation, error)
-	Close() error
 }
 
 // NewRevocationExt generates a revocation extension for a certificate.
@@ -89,7 +88,7 @@ func NewRevocationExt(key crypto.PrivateKey, revokedCert *x509.Certificate) (pki
 func revocationChecker(opts *Options) HandlerFunc {
 	return func(_ pkix.Extension, chains [][]*x509.Certificate) error {
 		ca, leaf := chains[0][peertls.CAIndex], chains[0][peertls.LeafIndex]
-		lastRev, lastRevErr := opts.RevDB.Get(context.TODO(), chains[0])
+		lastRev, lastRevErr := opts.RevocationDB.Get(context.TODO(), chains[0])
 		if lastRevErr != nil {
 			return Error.Wrap(lastRevErr)
 		}
@@ -121,7 +120,7 @@ func revocationChecker(opts *Options) HandlerFunc {
 
 func revocationUpdater(opts *Options) HandlerFunc {
 	return func(ext pkix.Extension, chains [][]*x509.Certificate) error {
-		if err := opts.RevDB.Put(context.TODO(), chains[0], ext); err != nil {
+		if err := opts.RevocationDB.Put(context.TODO(), chains[0], ext); err != nil {
 			return err
 		}
 		return nil

pkg/peertls/extensions/revocations_test.go

@@ -25,11 +25,11 @@ import (
 var ctx = context.Background() // test context
 
 func TestRevocationCheckHandler(t *testing.T) {
-	testrevocation.RevocationDBsTest(t, func(t *testing.T, revDB extensions.RevocationDB, _ storage.KeyValueStore) {
+	testrevocation.RunDBs(t, func(t *testing.T, revDB extensions.RevocationDB, _ storage.KeyValueStore) {
 		keys, chain, err := testpeertls.NewCertChain(2, storj.LatestIDVersion().Number)
 		assert.NoError(t, err)
 
-		opts := &extensions.Options{RevDB: revDB}
+		opts := &extensions.Options{RevocationDB: revDB}
 		revocationChecker := extensions.RevocationCheckHandler.NewHandlerFunc(opts)
 
 		revokingChain, leafRevocationExt, err := testpeertls.RevokeLeaf(keys[peertls.CAIndex], chain)
@@ -67,12 +67,12 @@ func TestRevocationCheckHandler(t *testing.T) {
 		}
 	})
 
-	testrevocation.RevocationDBsTest(t, func(t *testing.T, revDB extensions.RevocationDB, _ storage.KeyValueStore) {
+	testrevocation.RunDBs(t, func(t *testing.T, revDB extensions.RevocationDB, _ storage.KeyValueStore) {
 		t.Log("new revocation DB")
 		keys, chain, err := testpeertls.NewCertChain(2, storj.LatestIDVersion().Number)
 		assert.NoError(t, err)
 
-		opts := &extensions.Options{RevDB: revDB}
+		opts := &extensions.Options{RevocationDB: revDB}
 		revocationChecker := extensions.RevocationCheckHandler.NewHandlerFunc(opts)
 		revokingChain, caRevocationExt, err := testpeertls.RevokeCA(keys[peertls.CAIndex], chain)
 		require.NoError(t, err)
@@ -119,7 +119,7 @@ func TestRevocationCheckHandler(t *testing.T) {
 }
 
 func TestRevocationUpdateHandler(t *testing.T) {
-	testrevocation.RevocationDBsTest(t, func(t *testing.T, revDB extensions.RevocationDB, _ storage.KeyValueStore) {
+	testrevocation.RunDBs(t, func(t *testing.T, revDB extensions.RevocationDB, _ storage.KeyValueStore) {
 		keys, chain, err := testpeertls.NewCertChain(2, storj.LatestIDVersion().Number)
 		assert.NoError(t, err)
 
@@ -134,7 +134,7 @@ func TestRevocationUpdateHandler(t *testing.T) {
 		newestRevokedChain, newestRevocation, err := testpeertls.RevokeLeaf(keys[peertls.CAIndex], revokedLeafChain)
 		require.NoError(t, err)
 
-		opts := &extensions.Options{RevDB: revDB}
+		opts := &extensions.Options{RevocationDB: revDB}
 		revocationChecker := extensions.RevocationUpdateHandler.NewHandlerFunc(opts)
 
 		{
@@ -159,7 +159,7 @@ func TestWithOptions_NilRevocationDB(t *testing.T) {
 	_, chain, err := testpeertls.NewCertChain(2, storj.LatestIDVersion().Number)
 	require.NoError(t, err)
 
-	opts := &extensions.Options{RevDB: nil}
+	opts := &extensions.Options{RevocationDB: nil}
 	handlerFuncMap := extensions.DefaultHandlers.WithOptions(opts)
 
 	extMap := tlsopts.NewExtensionsMap(chain[peertls.LeafIndex])

pkg/peertls/tlsopts/options.go

@@ -47,10 +47,10 @@ type ExtensionMap map[string]pkix.Extension
 // NewOptions is a constructor for `tls options` given an identity, config, and
 // revocation DB. A caller may pass a nil revocation DB if the revocation
 // extension is disabled.
-func NewOptions(i *identity.FullIdentity, c Config, revDB extensions.RevocationDB) (*Options, error) {
+func NewOptions(i *identity.FullIdentity, c Config, revocationDB extensions.RevocationDB) (*Options, error) {
 	opts := &Options{
 		Config:            c,
-		RevDB:             revDB,
+		RevDB:             revocationDB,
 		Ident:             i,
 		VerificationFuncs: new(VerificationFuncs),
 	}
@@ -78,7 +78,7 @@ func NewExtensionsMap(chain ...*x509.Certificate) ExtensionMap {
 func (opts *Options) ExtensionOptions() *extensions.Options {
 	return &extensions.Options{
 		PeerCAWhitelist: opts.PeerCAWhitelist,
-		RevDB:           opts.RevDB,
+		RevocationDB:    opts.RevDB,
 		PeerIDVersions:  opts.Config.PeerIDVersions,
 	}
 }

pkg/peertls/tlsopts/options_test.go

@@ -106,14 +106,18 @@ func TestNewOptions(t *testing.T) {
 
 	for _, c := range cases {
 		t.Log(c.testID)
-		revDB, err := revocation.NewDBFromCfg(c.config)
+
+		revocationDB, err := revocation.NewDBFromCfg(c.config)
 		require.NoError(t, err)
-		opts, err := tlsopts.NewOptions(fi, c.config, revDB)
+
+		opts, err := tlsopts.NewOptions(fi, c.config, revocationDB)
 		assert.NoError(t, err)
 		assert.True(t, reflect.DeepEqual(fi, opts.Ident))
 		assert.Equal(t, c.config, opts.Config)
 		assert.Len(t, opts.VerificationFuncs.Client(), c.clientVerificationFuncsLen)
 		assert.Len(t, opts.VerificationFuncs.Server(), c.serverVerificationFuncsLen)
+
+		require.NoError(t, revocationDB.Close())
 	}
 }
 

pkg/peertls/tlsopts/tls_test.go

@@ -95,9 +95,9 @@ func TestExtensionMap_HandleExtensions(t *testing.T) {
 		err = rev.Verify(newRevokedLeafChain[peertls.CAIndex])
 		require.NoError(t, err)
 
-		testrevocation.RevocationDBsTest(t, func(t *testing.T, revDB extensions.RevocationDB, db storage.KeyValueStore) {
+		testrevocation.RunDBs(t, func(t *testing.T, revDB extensions.RevocationDB, db storage.KeyValueStore) {
 			opts := &extensions.Options{
-				RevDB:          revDB,
+				RevocationDB:   revDB,
 				PeerIDVersions: "*",
 			}
 
@@ -128,7 +128,7 @@ func TestExtensionMap_HandleExtensions_error(t *testing.T) {
 	ctx := testcontext.New(t)
 	defer ctx.Cleanup()
 
-	testrevocation.RevocationDBsTest(t, func(t *testing.T, revDB extensions.RevocationDB, db storage.KeyValueStore) {
+	testrevocation.RunDBs(t, func(t *testing.T, revDB extensions.RevocationDB, db storage.KeyValueStore) {
 		keys, chain, oldRevocation, err := testpeertls.NewRevokedLeafChain()
 		assert.NoError(t, err)
 
@@ -143,7 +143,7 @@ func TestExtensionMap_HandleExtensions_error(t *testing.T) {
 		err = revDB.Put(ctx, chain, newRevocation)
 		assert.NoError(t, err)
 
-		opts := &extensions.Options{RevDB: revDB}
+		opts := &extensions.Options{RevocationDB: revDB}
 		handlerFuncMap := extensions.HandlerFactories{
 			extensions.RevocationUpdateHandler,
 		}.WithOptions(opts)

pkg/revocation/common.go

@@ -0,0 +1,70 @@
+// Copyright (C) 2019 Storj Labs, Inc.
+// See LICENSE for copying information.
+
+package revocation
+
+import (
+	"storj.io/storj/internal/dbutil"
+	"storj.io/storj/pkg/peertls/extensions"
+	"storj.io/storj/pkg/peertls/tlsopts"
+	"storj.io/storj/storage/boltdb"
+	"storj.io/storj/storage/redis"
+)
+
+// NewDBFromCfg is a convenience method to create a revocation DB
+// directly from a config. If the revocation extension option is not set, it
+// returns a nil db with no error.
+func NewDBFromCfg(cfg tlsopts.Config) (*DB, error) {
+	if !cfg.Extensions.Revocation {
+		return &DB{}, nil
+	}
+	return NewDB(cfg.RevocationDBURL)
+}
+
+// NewDB returns a new revocation database given the URL
+func NewDB(dbURL string) (*DB, error) {
+	driver, source, err := dbutil.SplitConnstr(dbURL)
+	if err != nil {
+		return nil, extensions.ErrRevocationDB.Wrap(err)
+	}
+
+	var db *DB
+	switch driver {
+	case "bolt":
+		db, err = newDBBolt(source)
+		if err != nil {
+			return nil, extensions.ErrRevocationDB.Wrap(err)
+		}
+	case "redis":
+		db, err = newDBRedis(dbURL)
+		if err != nil {
+			return nil, extensions.ErrRevocationDB.Wrap(err)
+		}
+	default:
+		return nil, extensions.ErrRevocationDB.New("database scheme not supported: %s", driver)
+	}
+
+	return db, nil
+}
+
+// newDBBolt creates a bolt-backed DB
+func newDBBolt(path string) (*DB, error) {
+	client, err := boltdb.New(path, extensions.RevocationBucket)
+	if err != nil {
+		return nil, err
+	}
+	return &DB{
+		store: client,
+	}, nil
+}
+
+// newDBRedis creates a redis-backed DB.
+func newDBRedis(address string) (*DB, error) {
+	client, err := redis.NewClientFrom(address)
+	if err != nil {
+		return nil, err
+	}
+	return &DB{
+		store: client,
+	}, nil
+}

pkg/revocation/db.go

@@ -11,14 +11,10 @@ import (
 	"github.com/zeebo/errs"
 	"gopkg.in/spacemonkeygo/monkit.v2"
 
-	"storj.io/storj/internal/dbutil"
 	"storj.io/storj/pkg/identity"
 	"storj.io/storj/pkg/peertls"
 	"storj.io/storj/pkg/peertls/extensions"
-	"storj.io/storj/pkg/peertls/tlsopts"
 	"storj.io/storj/storage"
-	"storj.io/storj/storage/boltdb"
-	"storj.io/storj/storage/redis"
 )
 
 var (
@@ -32,77 +28,24 @@ var (
 // (i.e. nodeID [CA certificate's public key hash] is the key, values is
 // the most recently seen revocation).
 type DB struct {
-	KVStore storage.KeyValueStore
+	store storage.KeyValueStore
-}
-
-// NewDBFromCfg is a convenience method to create a revocation DB
-// directly from a config. If the revocation extension option is not set, it
-// returns a nil db with no error.
-func NewDBFromCfg(cfg tlsopts.Config) (*DB, error) {
-	if !cfg.Extensions.Revocation {
-		return nil, nil
-	}
-	return NewDB(cfg.RevocationDBURL)
-}
-
-// NewDB returns a new revocation database given the URL
-func NewDB(dbURL string) (*DB, error) {
-	driver, source, err := dbutil.SplitConnstr(dbURL)
-	if err != nil {
-		return nil, extensions.ErrRevocationDB.Wrap(err)
-	}
-
-	var db *DB
-	switch driver {
-	case "bolt":
-		db, err = newDBBolt(source)
-		if err != nil {
-			return nil, extensions.ErrRevocationDB.Wrap(err)
-		}
-	case "redis":
-		db, err = newDBRedis(dbURL)
-		if err != nil {
-			return nil, extensions.ErrRevocationDB.Wrap(err)
-		}
-	default:
-		return nil, extensions.ErrRevocationDB.New("database scheme not supported: %s", driver)
-	}
-
-	return db, nil
-}
-
-// newDBBolt creates a bolt-backed DB
-func newDBBolt(path string) (*DB, error) {
-	client, err := boltdb.New(path, extensions.RevocationBucket)
-	if err != nil {
-		return nil, err
-	}
-	return &DB{
-		KVStore: client,
-	}, nil
-}
-
-// newDBRedis creates a redis-backed DB.
-func newDBRedis(address string) (*DB, error) {
-	client, err := redis.NewClientFrom(address)
-	if err != nil {
-		return nil, err
-	}
-	return &DB{
-		KVStore: client,
-	}, nil
 }
 
 // Get attempts to retrieve the most recent revocation for the given cert chain
 // (the  key used in the underlying database is the nodeID of the certificate chain).
-func (db DB) Get(ctx context.Context, chain []*x509.Certificate) (_ *extensions.Revocation, err error) {
+func (db *DB) Get(ctx context.Context, chain []*x509.Certificate) (_ *extensions.Revocation, err error) {
 	defer mon.Task()(&ctx)(&err)
+
+	if db.store == nil {
+		return nil, nil
+	}
+
 	nodeID, err := identity.NodeIDFromCert(chain[peertls.CAIndex])
 	if err != nil {
 		return nil, extensions.ErrRevocation.Wrap(err)
 	}
 
-	revBytes, err := db.KVStore.Get(ctx, nodeID.Bytes())
+	revBytes, err := db.store.Get(ctx, nodeID.Bytes())
 	if err != nil && !storage.ErrKeyNotFound.Has(err) {
 		return nil, extensions.ErrRevocationDB.Wrap(err)
 	}
@@ -120,8 +63,13 @@ func (db DB) Get(ctx context.Context, chain []*x509.Certificate) (_ *extensions.
 // Put stores the most recent revocation for the given cert chain IF the timestamp
 // is newer than the current value (the  key used in the underlying database is
 // the nodeID of the certificate chain).
-func (db DB) Put(ctx context.Context, chain []*x509.Certificate, revExt pkix.Extension) (err error) {
+func (db *DB) Put(ctx context.Context, chain []*x509.Certificate, revExt pkix.Extension) (err error) {
 	defer mon.Task()(&ctx)(&err)
+
+	if db.store == nil {
+		return extensions.ErrRevocationDB.New("not supported")
+	}
+
 	ca := chain[peertls.CAIndex]
 	var rev extensions.Revocation
 	if err := rev.Unmarshal(revExt.Value); err != nil {
@@ -146,21 +94,26 @@ func (db DB) Put(ctx context.Context, chain []*x509.Certificate, revExt pkix.Ext
 	if err != nil {
 		return extensions.ErrRevocationDB.Wrap(err)
 	}
-	if err := db.KVStore.Put(ctx, nodeID.Bytes(), revExt.Value); err != nil {
+	if err := db.store.Put(ctx, nodeID.Bytes(), revExt.Value); err != nil {
 		return extensions.ErrRevocationDB.Wrap(err)
 	}
 	return nil
 }
 
 // List lists all revocations in the store
-func (db DB) List(ctx context.Context) (revs []*extensions.Revocation, err error) {
+func (db *DB) List(ctx context.Context) (revs []*extensions.Revocation, err error) {
 	defer mon.Task()(&ctx)(&err)
-	keys, err := db.KVStore.List(ctx, []byte{}, 0)
+
+	if db.store == nil {
+		return nil, nil
+	}
+
+	keys, err := db.store.List(ctx, []byte{}, 0)
 	if err != nil {
 		return nil, extensions.ErrRevocationDB.Wrap(err)
 	}
 
-	marshaledRevs, err := db.KVStore.GetAll(ctx, keys)
+	marshaledRevs, err := db.store.GetAll(ctx, keys)
 	if err != nil {
 		return nil, extensions.ErrRevocationDB.Wrap(err)
 	}
@@ -176,7 +129,15 @@ func (db DB) List(ctx context.Context) (revs []*extensions.Revocation, err error
 	return revs, nil
 }
 
-// Close closes the underlying store
+// TestGetStore returns the internal store for testing.
-func (db DB) Close() error {
+func (db *DB) TestGetStore() storage.KeyValueStore {
-	return db.KVStore.Close()
+	return db.store
+}
+
+// Close closes the underlying store
+func (db *DB) Close() error {
+	if db.store == nil {
+		return nil
+	}
+	return db.store.Close()
 }

pkg/revocation/db_test.go

@@ -26,7 +26,7 @@ func TestRevocationDB_Get(t *testing.T) {
 	ctx := testcontext.New(t)
 	defer ctx.Cleanup()
 
-	testrevocation.RevocationDBsTest(t, func(t *testing.T, revDB extensions.RevocationDB, db storage.KeyValueStore) {
+	testrevocation.RunDBs(t, func(t *testing.T, revDB extensions.RevocationDB, db storage.KeyValueStore) {
 		keys, chain, err := testpeertls.NewCertChain(2, storj.LatestIDVersion().Number)
 		require.NoError(t, err)
 
@@ -64,7 +64,7 @@ func TestRevocationDB_Put_success(t *testing.T) {
 	ctx := testcontext.New(t)
 	defer ctx.Cleanup()
 
-	testrevocation.RevocationDBsTest(t, func(t *testing.T, revDB extensions.RevocationDB, db storage.KeyValueStore) {
+	testrevocation.RunDBs(t, func(t *testing.T, revDB extensions.RevocationDB, db storage.KeyValueStore) {
 		keys, chain, err := testpeertls.NewCertChain(2, storj.LatestIDVersion().Number)
 		require.NoError(t, err)
 
@@ -114,7 +114,7 @@ func TestRevocationDB_Put_error(t *testing.T) {
 	ctx := testcontext.New(t)
 	defer ctx.Cleanup()
 
-	testrevocation.RevocationDBsTest(t, func(t *testing.T, revDB extensions.RevocationDB, db storage.KeyValueStore) {
+	testrevocation.RunDBs(t, func(t *testing.T, revDB extensions.RevocationDB, db storage.KeyValueStore) {
 		keys, chain, err := testpeertls.NewCertChain(2, storj.LatestIDVersion().Number)
 		require.NoError(t, err)
 

pkg/server/config.go

@@ -6,7 +6,6 @@ package server
 import (
 	"context"
 
-	"github.com/zeebo/errs"
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
 
@@ -37,7 +36,6 @@ func (sc Config) Run(ctx context.Context, log *zap.Logger, identity *identity.Fu
 	if err != nil {
 		return err
 	}
-	defer func() { err = errs.Combine(err, opts.RevDB.Close()) }()
 
 	server, err := New(log.Named("server"), opts, sc.Address, sc.PrivateAddress, interceptor, services...)
 	if err != nil {

satellite/peer.go

@@ -229,7 +229,7 @@ type Peer struct {
 }
 
 // New creates a new satellite
-func New(log *zap.Logger, full *identity.FullIdentity, db DB, revDB extensions.RevocationDB, config *Config, versionInfo version.Info) (*Peer, error) {
+func New(log *zap.Logger, full *identity.FullIdentity, db DB, revocationDB extensions.RevocationDB, config *Config, versionInfo version.Info) (*Peer, error) {
 	peer := &Peer{
 		Log:      log,
 		Identity: full,
@@ -251,7 +251,7 @@ func New(log *zap.Logger, full *identity.FullIdentity, db DB, revDB extensions.R
 		log.Debug("Starting listener and server")
 		sc := config.Server
 
-		options, err := tlsopts.NewOptions(peer.Identity, sc.Config, revDB)
+		options, err := tlsopts.NewOptions(peer.Identity, sc.Config, revocationDB)
 		if err != nil {
 			return nil, errs.Combine(err, peer.Close())
 		}

storagenode/peer.go

@@ -156,7 +156,7 @@ type Peer struct {
 }
 
 // New creates a new Storage Node.
-func New(log *zap.Logger, full *identity.FullIdentity, db DB, revDB extensions.RevocationDB, config Config, versionInfo version.Info) (*Peer, error) {
+func New(log *zap.Logger, full *identity.FullIdentity, db DB, revocationDB extensions.RevocationDB, config Config, versionInfo version.Info) (*Peer, error) {
 	peer := &Peer{
 		Log:      log,
 		Identity: full,
@@ -177,7 +177,7 @@ func New(log *zap.Logger, full *identity.FullIdentity, db DB, revDB extensions.R
 	{ // setup listener and server
 		sc := config.Server
 
-		options, err := tlsopts.NewOptions(peer.Identity, sc.Config, revDB)
+		options, err := tlsopts.NewOptions(peer.Identity, sc.Config, revocationDB)
 		if err != nil {
 			return nil, errs.Combine(err, peer.Close())
 		}