diff --git a/cmd/bootstrap/main.go b/cmd/bootstrap/main.go index 065222077..db90f91ac 100644 --- a/cmd/bootstrap/main.go +++ b/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 } diff --git a/cmd/certificates/main.go b/cmd/certificates/main.go index 579699d5b..92bc0b960 100644 --- a/cmd/certificates/main.go +++ b/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() { diff --git a/cmd/identity/main.go b/cmd/identity/main.go index 3eb91c441..7e6371433 100644 --- a/cmd/identity/main.go +++ b/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) } diff --git a/cmd/satellite/main.go b/cmd/satellite/main.go index ab8c00f13..21774477d 100644 --- a/cmd/satellite/main.go +++ b/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 } diff --git a/cmd/storagenode/main.go b/cmd/storagenode/main.go index b4ce76b99..1b797c53d 100644 --- a/cmd/storagenode/main.go +++ b/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 } diff --git a/internal/testplanet/bootstrap.go b/internal/testplanet/bootstrap.go index 7d4e7bd06..c907c8176 100644 --- a/internal/testplanet/bootstrap.go +++ b/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 } diff --git a/internal/testplanet/satellite.go b/internal/testplanet/satellite.go index 7c56a067d..8e22280df 100644 --- a/internal/testplanet/satellite.go +++ b/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) } diff --git a/internal/testplanet/storagenode.go b/internal/testplanet/storagenode.go index 0058ca33a..9556fc3de 100644 --- a/internal/testplanet/storagenode.go +++ b/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()) diff --git a/internal/testplanet/uplink_test.go b/internal/testplanet/uplink_test.go index f3854f609..98645f11a 100644 --- a/internal/testplanet/uplink_test.go +++ b/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 diff --git a/internal/testrevocation/db.go b/internal/testrevocation/db.go new file mode 100644 index 000000000..3d32229f0 --- /dev/null +++ b/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()) + }) +} diff --git a/internal/testrevocation/revocation_db.go b/internal/testrevocation/revocation_db.go deleted file mode 100644 index ceb6a15ac..000000000 --- a/internal/testrevocation/revocation_db.go +++ /dev/null @@ -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) - } - }) -} diff --git a/pkg/certificates/certificates_test.go b/pkg/certificates/certificates_test.go index bff989b92..3c696c893 100644 --- a/pkg/certificates/certificates_test.go +++ b/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) diff --git a/pkg/peertls/extensions/extensions.go b/pkg/peertls/extensions/extensions.go index f8aff6cad..df8cb98eb 100644 --- a/pkg/peertls/extensions/extensions.go +++ b/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 } diff --git a/pkg/peertls/extensions/revocations.go b/pkg/peertls/extensions/revocations.go index c107f43fa..a5657f15a 100644 --- a/pkg/peertls/extensions/revocations.go +++ b/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 diff --git a/pkg/peertls/extensions/revocations_test.go b/pkg/peertls/extensions/revocations_test.go index cb2e5ea31..b4fce338c 100644 --- a/pkg/peertls/extensions/revocations_test.go +++ b/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]) diff --git a/pkg/peertls/tlsopts/options.go b/pkg/peertls/tlsopts/options.go index 1a8a56a7e..78a913786 100644 --- a/pkg/peertls/tlsopts/options.go +++ b/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, } } diff --git a/pkg/peertls/tlsopts/options_test.go b/pkg/peertls/tlsopts/options_test.go index 928c86b91..d47b1f871 100644 --- a/pkg/peertls/tlsopts/options_test.go +++ b/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()) } } diff --git a/pkg/peertls/tlsopts/tls_test.go b/pkg/peertls/tlsopts/tls_test.go index c4ba50a6a..dfe223067 100644 --- a/pkg/peertls/tlsopts/tls_test.go +++ b/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) diff --git a/pkg/revocation/common.go b/pkg/revocation/common.go new file mode 100644 index 000000000..8da406adb --- /dev/null +++ b/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 +} diff --git a/pkg/revocation/revocation_db.go b/pkg/revocation/db.go similarity index 56% rename from pkg/revocation/revocation_db.go rename to pkg/revocation/db.go index 363d33180..1a4bcbef7 100644 --- a/pkg/revocation/revocation_db.go +++ b/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 -} - -// 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 + store storage.KeyValueStore } // 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 -func (db DB) Close() error { - return db.KVStore.Close() +// TestGetStore returns the internal store for testing. +func (db *DB) TestGetStore() storage.KeyValueStore { + return db.store +} + +// Close closes the underlying store +func (db *DB) Close() error { + if db.store == nil { + return nil + } + return db.store.Close() } diff --git a/pkg/revocation/revocation_db_test.go b/pkg/revocation/db_test.go similarity index 91% rename from pkg/revocation/revocation_db_test.go rename to pkg/revocation/db_test.go index d7db38f08..7242d9fde 100644 --- a/pkg/revocation/revocation_db_test.go +++ b/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) diff --git a/pkg/server/config.go b/pkg/server/config.go index ba2c4ee8d..83e6b4ec8 100644 --- a/pkg/server/config.go +++ b/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 { diff --git a/satellite/peer.go b/satellite/peer.go index 572034114..74799a8a3 100644 --- a/satellite/peer.go +++ b/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()) } diff --git a/storagenode/peer.go b/storagenode/peer.go index a57c38302..bf088ffb9 100644 --- a/storagenode/peer.go +++ b/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()) }