reload and shutdown trickery

* Only shutdown the database once.
* Be careful when reloading the CA. Depending on whether the DB has
already been shutdown, and error may be unrecoverable.
This commit is contained in:
max furman 2019-04-25 12:37:25 -07:00
parent cbeca9383b
commit c242602231
4 changed files with 75 additions and 29 deletions

1
Gopkg.lock generated
View file

@ -671,6 +671,7 @@
"github.com/smallstep/cli/token/provision",
"github.com/smallstep/cli/usage",
"github.com/smallstep/nosql",
"github.com/smallstep/nosql/database",
"github.com/tsenart/deadcode",
"github.com/urfave/cli",
"golang.org/x/crypto/ocsp",

View file

@ -3,6 +3,7 @@ package ca
import (
"crypto/tls"
"crypto/x509"
"log"
"net/http"
"github.com/go-chi/chi"
@ -123,7 +124,7 @@ func (ca *CA) Run() error {
func (ca *CA) Stop() error {
ca.renewer.Stop()
if err := ca.auth.Shutdown(); err != nil {
return err
log.Printf("error stopping ca.Authority: %+v\n", err)
}
return ca.srv.Shutdown()
}
@ -131,8 +132,9 @@ func (ca *CA) Stop() error {
// Reload reloads the configuration of the CA and calls to the server Reload
// method.
func (ca *CA) Reload() error {
if err := ca.auth.Shutdown(); err != nil {
return err
var hasDB bool
if ca.config.DB != nil {
hasDB = true
}
if ca.opts.configFile == "" {
return errors.New("error reloading ca: configuration file is not set")
@ -140,15 +142,54 @@ func (ca *CA) Reload() error {
config, err := authority.LoadConfiguration(ca.opts.configFile)
if err != nil {
return errors.Wrap(err, "error reloading ca")
return errors.Wrap(err, "error reloading ca configuration")
}
logShutDown := func(ss ...string) {
for _, s := range ss {
log.Println(s)
}
log.Println("Continuing to serve requests may result in inconsistent state. Shutting Down ...")
}
logContinue := func(reason string) {
log.Println(reason)
log.Println("Continuing to run with the original configuration.")
log.Println("You can force a restart by sending a SIGTERM signal and then restarting the step-ca.")
}
// Shut down the old authority (shut down the database). If New or Reload
// fails then the CA will continue to run but the database will have been
// shutdown, which will cause errors.
if err := ca.auth.Shutdown(); err != nil {
if hasDB {
logShutDown("Attempt to shut down the ca.Authority has failed.")
return ca.Stop()
}
logContinue("Reload failed because the ca.Authority could not be shut down.")
return err
}
newCA, err := New(config, WithPassword(ca.opts.password), WithConfigFile(ca.opts.configFile))
if err != nil {
if hasDB {
logShutDown("Attempt to initialize a CA with the new configuration has failed.",
"The database has already been shutdown.")
return ca.Stop()
}
logContinue("Reload failed because the CA with new configuration could " +
"not be initialized.")
return errors.Wrap(err, "error reloading ca")
}
return ca.srv.Reload(newCA.srv)
if err = ca.srv.Reload(newCA.srv); err != nil {
if hasDB {
logShutDown("Attempt to replace the old CA server has failed.",
"The database has already been shutdown.")
return ca.Stop()
}
logContinue("Reload failed because server could not be replaced.")
return errors.Wrap(err, "error reloading server")
}
return nil
}
// getTLSConfig returns a TLSConfig for the CA server with a self-renewing

View file

@ -37,6 +37,7 @@ type AuthDB interface {
// DB is a wrapper over the nosql.DB interface.
type DB struct {
nosql.DB
isUp bool
}
// New returns a new database client that implements the AuthDB interface.
@ -59,7 +60,7 @@ func New(c *Config) (AuthDB, error) {
}
}
return &DB{db}, nil
return &DB{db, true}, nil
}
// RevokedCertificateInfo contains information regarding the certificate
@ -127,8 +128,11 @@ func (db *DB) StoreCertificate(crt *x509.Certificate) error {
// Shutdown sends a shutdown message to the database.
func (db *DB) Shutdown() error {
if err := db.Close(); err != nil {
return errors.Wrap(err, "database shutdown error")
if db.isUp {
if err := db.Close(); err != nil {
return errors.Wrap(err, "database shutdown error")
}
db.isUp = false
}
return nil
}

View file

@ -8,7 +8,7 @@ import (
"github.com/smallstep/nosql/database"
)
type MockdatabaseDB struct {
type MockNoSQLDB struct {
err error
ret1, ret2 interface{}
get func(bucket, key []byte) ([]byte, error)
@ -22,7 +22,7 @@ type MockdatabaseDB struct {
update func(tx *database.Tx) error
}
func (m *MockdatabaseDB) Get(bucket, key []byte) ([]byte, error) {
func (m *MockNoSQLDB) Get(bucket, key []byte) ([]byte, error) {
if m.get != nil {
return m.get(bucket, key)
}
@ -32,56 +32,56 @@ func (m *MockdatabaseDB) Get(bucket, key []byte) ([]byte, error) {
return m.ret1.([]byte), m.err
}
func (m *MockdatabaseDB) Set(bucket, key, value []byte) error {
func (m *MockNoSQLDB) Set(bucket, key, value []byte) error {
if m.set != nil {
return m.set(bucket, key, value)
}
return m.err
}
func (m *MockdatabaseDB) Open(dataSourceName string, opt ...database.Option) error {
func (m *MockNoSQLDB) Open(dataSourceName string, opt ...database.Option) error {
if m.open != nil {
return m.open(dataSourceName, opt...)
}
return m.err
}
func (m *MockdatabaseDB) Close() error {
func (m *MockNoSQLDB) Close() error {
if m.close != nil {
return m.close()
}
return m.err
}
func (m *MockdatabaseDB) CreateTable(bucket []byte) error {
func (m *MockNoSQLDB) CreateTable(bucket []byte) error {
if m.createTable != nil {
return m.createTable(bucket)
}
return m.err
}
func (m *MockdatabaseDB) DeleteTable(bucket []byte) error {
func (m *MockNoSQLDB) DeleteTable(bucket []byte) error {
if m.deleteTable != nil {
return m.deleteTable(bucket)
}
return m.err
}
func (m *MockdatabaseDB) Del(bucket, key []byte) error {
func (m *MockNoSQLDB) Del(bucket, key []byte) error {
if m.del != nil {
return m.del(bucket, key)
}
return m.err
}
func (m *MockdatabaseDB) List(bucket []byte) ([]*database.Entry, error) {
func (m *MockNoSQLDB) List(bucket []byte) ([]*database.Entry, error) {
if m.list != nil {
return m.list(bucket)
}
return m.ret1.([]*database.Entry), m.err
}
func (m *MockdatabaseDB) Update(tx *database.Tx) error {
func (m *MockNoSQLDB) Update(tx *database.Tx) error {
if m.update != nil {
return m.update(tx)
}
@ -100,16 +100,16 @@ func TestIsRevoked(t *testing.T) {
},
"false/ErrNotFound": {
key: "sn",
db: &DB{&MockdatabaseDB{err: database.ErrNotFound, ret1: nil}},
db: &DB{&MockNoSQLDB{err: database.ErrNotFound, ret1: nil}, true},
},
"error/checking bucket": {
key: "sn",
db: &DB{&MockdatabaseDB{err: errors.New("force"), ret1: nil}},
db: &DB{&MockNoSQLDB{err: errors.New("force"), ret1: nil}, true},
err: errors.New("error checking revocation bucket: force"),
},
"true": {
key: "sn",
db: &DB{&MockdatabaseDB{ret1: []byte("value")}},
db: &DB{&MockNoSQLDB{ret1: []byte("value")}, true},
isRevoked: true,
},
}
@ -136,44 +136,44 @@ func TestRevoke(t *testing.T) {
}{
"error/force isRevoked": {
rci: &RevokedCertificateInfo{Serial: "sn"},
db: &DB{&MockdatabaseDB{
db: &DB{&MockNoSQLDB{
get: func(bucket []byte, sn []byte) ([]byte, error) {
return nil, errors.New("force IsRevoked")
},
}},
}, true},
err: errors.New("error checking revocation bucket: force IsRevoked"),
},
"error/was already revoked": {
rci: &RevokedCertificateInfo{Serial: "sn"},
db: &DB{&MockdatabaseDB{
db: &DB{&MockNoSQLDB{
get: func(bucket []byte, sn []byte) ([]byte, error) {
return nil, nil
},
}},
}, true},
err: ErrAlreadyExists,
},
"error/database set": {
rci: &RevokedCertificateInfo{Serial: "sn"},
db: &DB{&MockdatabaseDB{
db: &DB{&MockNoSQLDB{
get: func(bucket []byte, sn []byte) ([]byte, error) {
return nil, database.ErrNotFound
},
set: func(bucket []byte, key []byte, value []byte) error {
return errors.New("force")
},
}},
}, true},
err: errors.New("database Set error: force"),
},
"ok": {
rci: &RevokedCertificateInfo{Serial: "sn"},
db: &DB{&MockdatabaseDB{
db: &DB{&MockNoSQLDB{
get: func(bucket []byte, sn []byte) ([]byte, error) {
return nil, database.ErrNotFound
},
set: func(bucket []byte, key []byte, value []byte) error {
return nil
},
}},
}, true},
},
}
for name, tc := range tests {