From 3fe95babdfc2682bb94b9ae07b4680f9d273d5b1 Mon Sep 17 00:00:00 2001 From: Greg Guthe Date: Fri, 19 Jul 2019 21:24:02 -0400 Subject: [PATCH 1/7] remove incorrect comment from dev config --- autograph.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/autograph.yaml b/autograph.yaml index a90f86943..80db26f7a 100755 --- a/autograph.yaml +++ b/autograph.yaml @@ -513,7 +513,6 @@ signers: - id: testmar type: mar - # label of the private key in the hsm, it isn't stored locally privatekey: | -----BEGIN PRIVATE KEY----- MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQCAUrUDTS86CuqV From 3d1fe8cc4e3ed3085a4f91e1795e6b28b108f6ea Mon Sep 17 00:00:00 2001 From: Greg Guthe Date: Fri, 19 Jul 2019 12:58:14 -0400 Subject: [PATCH 2/7] database: error instead of dying on monitor failure factor out DB init to addDB in main factor out CheckConnection func add connect/monitor tests add MonitorPollInterval to DB config --- autograph.yaml | 1 + database/connect.go | 59 ++++++++++++++++++---------- database/connect_test.go | 44 +++++++++++++++++++++ database/queries_test.go | 9 +++-- docs/configuration.rst | 1 + main.go | 34 ++++++++++------ tools/softhsm/autograph.softhsm.yaml | 1 + 7 files changed, 112 insertions(+), 37 deletions(-) create mode 100644 database/connect_test.go diff --git a/autograph.yaml b/autograph.yaml index 80db26f7a..4e5e53686 100755 --- a/autograph.yaml +++ b/autograph.yaml @@ -29,6 +29,7 @@ statsd: # sslrootcert: /etc/ssl/certs/db-root.crt # maxopenconns: 100 # maxidleconns: 10 +# monitorpollinterval: 10s # The keys below are testing keys that do not grant any power signers: diff --git a/database/connect.go b/database/connect.go index 167c59e3f..af72d8fa5 100755 --- a/database/connect.go +++ b/database/connect.go @@ -34,14 +34,15 @@ type Transaction struct { // Config holds the parameters to connect to a database type Config struct { - Name string - User string - Password string - Host string - SSLMode string - SSLRootCert string - MaxOpenConns int - MaxIdleConns int + Name string + User string + Password string + Host string + SSLMode string + SSLRootCert string + MaxOpenConns int + MaxIdleConns int + MonitorPollInterval time.Duration } // Connect creates a database connection and returns a handler @@ -70,20 +71,36 @@ func Connect(config Config) (*Handler, error) { return &Handler{dbfd}, nil } -// Monitor runs an infinite loop that queries the database every 10 seconds -// and panics if the query fails. It can be used in a goroutine to crash when -// the database becomes unavailable. -func (db *Handler) Monitor() { - // simple DB watchdog, crashes the process if connection dies +// CheckConnection runs a test query against the database and returns +// an error if it fails +func (db *Handler) CheckConnection() error { + var one uint + err := db.QueryRow("SELECT 1").Scan(&one) + if err != nil { + return errors.Wrap(err, "Database connection failed") + } + if one != 1 { + return errors.Errorf("Apparently the database doesn't know the meaning of one anymore") + } + return nil +} + +// Monitor queries the database every pollInterval until it gets a +// quit signal logging an error when the test query fails. It can be +// used in a goroutine to check when the database becomes unavailable. +func (db *Handler) Monitor(pollInterval time.Duration, quit chan bool) { + log.Infof("starting DB monitor polling every %s", pollInterval) for { - var one uint - err := db.QueryRow("SELECT 1").Scan(&one) - if err != nil { - log.Fatal("Database connection failed:", err) - } - if one != 1 { - log.Fatal("Apparently the database doesn't know the meaning of one anymore. Crashing.") + select { + case <-time.After(pollInterval): + err := db.CheckConnection() + if err != nil { + log.Error(err) + break + } + case <-quit: + log.Info("Shutting down DB monitor") + return } - time.Sleep(10 * time.Second) } } diff --git a/database/connect_test.go b/database/connect_test.go new file mode 100644 index 000000000..afb578266 --- /dev/null +++ b/database/connect_test.go @@ -0,0 +1,44 @@ +package database + +import ( + "testing" + "time" +) + +func TestMonitor(t *testing.T) { + t.Parallel() + + t.Run("runs and dies on connection close", func(t *testing.T) { + t.Parallel() + + // connects + db, err := Connect(Config{ + Name: "autograph", + User: "myautographdbuser", + Password: "myautographdbpassword", + Host: "127.0.0.1:5432", + }) + if err != nil { + t.Fatal(err) + } + + quit := make(chan bool) + go db.Monitor(5*time.Millisecond, quit) + + // should not error for initial monitor run + err = db.CheckConnection() + if err != nil { + t.Fatalf("db.CheckConnection failed when it should not have with error: %s", err) + } + time.Sleep(10 * time.Millisecond) + + // error for failing checks + db.Close() + err = db.CheckConnection() + if err == nil { + t.Fatalf("db.CheckConnection did not fail for a closed DB") + } + + quit <- true + }) +} diff --git a/database/queries_test.go b/database/queries_test.go index 454bcb3ea..2bfe10c07 100644 --- a/database/queries_test.go +++ b/database/queries_test.go @@ -10,10 +10,11 @@ import ( func TestConcurrentEndEntityOperations(t *testing.T) { db, err := Connect(Config{ - Name: "autograph", - User: "myautographdbuser", - Password: "myautographdbpassword", - Host: "127.0.0.1:5432", + Name: "autograph", + User: "myautographdbuser", + Password: "myautographdbpassword", + Host: "127.0.0.1:5432", + MonitorPollInterval: 10 * time.Second, }) if err != nil { t.Fatal(err) diff --git a/docs/configuration.rst b/docs/configuration.rst index 425f58829..3432b4e66 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -61,6 +61,7 @@ Make sure to set a user with limited grants in the configuration. sslrootcert: /etc/ssl/certs/db-root.crt maxopenconns: 100 maxidleconns: 10 + monitorpollinterval: 10s Hardware Security Module (HSM) ------------------------------ diff --git a/main.go b/main.go index a7ce2b350..0db0636f5 100755 --- a/main.go +++ b/main.go @@ -129,19 +129,10 @@ func run(conf configuration, listen string, authPrint, debug bool) { // and store them into the autographer handler ag = newAutographer(conf.Server.NonceCacheSize) - // connect to the database if conf.Database.Name != "" { - ag.db, err = database.Connect(conf.Database) - if err != nil { - log.Fatal(err) - } - if ag.db == nil { - log.Fatal("failed to initialize database connection, unknown error") - } - // start a monitoring function that panics if - // the db becomes inaccessible - go ag.db.Monitor() - log.Print("database connection established") + // ignore the monitor close chan since it will stop + // when the app is stopped + _ = ag.addDB(conf.Database) } // initialize the hsm if a configuration is defined @@ -294,6 +285,25 @@ func (a *autographer) startCleanupHandler() { }() } +// addDB connects to the DB and starts a gorountine to monitor DB +// connectivity +func (a *autographer) addDB(dbConf database.Config) chan bool { + var err error + a.db, err = database.Connect(dbConf) + if err != nil { + log.Fatal(err) + } + if a.db == nil { + log.Fatal("failed to initialize database connection, unknown error") + } + // start a monitoring function that errors if the db + // becomes inaccessible + closeDBMonitor := make(chan bool) + go a.db.Monitor(dbConf.MonitorPollInterval, closeDBMonitor) + log.Print("database connection established") + return closeDBMonitor +} + // addSigners initializes each signer specified in the configuration by parsing // and loading their private keys. The signers are then copied over to the // autographer handler. diff --git a/tools/softhsm/autograph.softhsm.yaml b/tools/softhsm/autograph.softhsm.yaml index 846336742..a2e5cb8dd 100755 --- a/tools/softhsm/autograph.softhsm.yaml +++ b/tools/softhsm/autograph.softhsm.yaml @@ -24,6 +24,7 @@ database: sslrootcert: /opt/db-root.crt maxopenconns: 100 maxidleconns: 10 + monitorpollinterval: 10s # The keys below are testing keys that do not grant any power signers: From 1327d82468e33ec5dd84f5e4ff4d940f33f8d200 Mon Sep 17 00:00:00 2001 From: Greg Guthe Date: Fri, 19 Jul 2019 15:37:14 -0400 Subject: [PATCH 3/7] factor initHSM out of main --- main.go | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/main.go b/main.go index 0db0636f5..a547777e8 100755 --- a/main.go +++ b/main.go @@ -137,17 +137,7 @@ func run(conf configuration, listen string, authPrint, debug bool) { // initialize the hsm if a configuration is defined if conf.HSM.Path != "" { - tmpCtx, err := crypto11.Configure(&conf.HSM) - if err != nil { - log.Fatal(err) - } - if tmpCtx != nil { - // if we successfully initialized the crypto11 context, - // tell the signers they can try using the HSM - for i := range conf.Signers { - conf.Signers[i].HsmIsAvailable(tmpCtx) - } - } + ag.initHSM(conf) } if conf.Statsd.Addr != "" { @@ -304,6 +294,21 @@ func (a *autographer) addDB(dbConf database.Config) chan bool { return closeDBMonitor } +// initHSM sets up the HSM and notifies signers it is available +func (a *autographer) initHSM(conf configuration) { + tmpCtx, err := crypto11.Configure(&conf.HSM) + if err != nil { + log.Fatal(err) + } + if tmpCtx != nil { + // if we successfully initialized the crypto11 context, + // tell the signers they can try using the HSM + for i := range conf.Signers { + conf.Signers[i].HsmIsAvailable(tmpCtx) + } + } +} + // addSigners initializes each signer specified in the configuration by parsing // and loading their private keys. The signers are then copied over to the // autographer handler. From edd16cf851a17e768fe69f31a869acc4ac6a7c92 Mon Sep 17 00:00:00 2001 From: Greg Guthe Date: Fri, 19 Jul 2019 15:35:09 -0400 Subject: [PATCH 4/7] rename lb heartbeat handler --- handlers.go | 4 ++-- handlers_test.go | 2 +- main.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/handlers.go b/handlers.go index c2c0a291e..79257d2bb 100644 --- a/handlers.go +++ b/handlers.go @@ -229,8 +229,8 @@ func (a *autographer) handleSignature(w http.ResponseWriter, r *http.Request) { log.WithFields(log.Fields{"rid": rid}).Info("signing request completed successfully") } -// handleHeartbeat returns a simple message indicating that the API is alive and well -func handleHeartbeat(w http.ResponseWriter, r *http.Request) { +// handleLBHeartbeat returns a simple message indicating that the API is alive and well +func handleLBHeartbeat(w http.ResponseWriter, r *http.Request) { if r.Method != "GET" { httpError(w, r, http.StatusMethodNotAllowed, "%s method not allowed; endpoint accepts GET only", r.Method) return diff --git a/handlers_test.go b/handlers_test.go index 3646974b2..e3e3949f8 100644 --- a/handlers_test.go +++ b/handlers_test.go @@ -378,7 +378,7 @@ func TestHeartbeat(t *testing.T) { t.Fatal(err) } w := httptest.NewRecorder() - handleHeartbeat(w, req) + handleLBHeartbeat(w, req) if w.Code != testcase.expect { t.Fatalf("test case %d failed with code %d but %d was expected", i, w.Code, testcase.expect) diff --git a/main.go b/main.go index a547777e8..e376c44bc 100755 --- a/main.go +++ b/main.go @@ -173,7 +173,7 @@ func run(conf configuration, listen string, authPrint, debug bool) { router := mux.NewRouter().StrictSlash(true) router.HandleFunc("/__heartbeat__", handleHeartbeat).Methods("GET") - router.HandleFunc("/__lbheartbeat__", handleHeartbeat).Methods("GET") + router.HandleFunc("/__lbheartbeat__", handleLBHeartbeat).Methods("GET") router.HandleFunc("/__version__", handleVersion).Methods("GET") router.HandleFunc("/__monitor__", ag.handleMonitor).Methods("GET") router.HandleFunc("/sign/file", ag.handleSignature).Methods("POST") From af6447a65c1b1ea589a00314c54866b07f4c8a15 Mon Sep 17 00:00:00 2001 From: Greg Guthe Date: Mon, 22 Jul 2019 15:32:46 -0400 Subject: [PATCH 5/7] rename signer.HsmIsAvailable to InitHSM --- main.go | 2 +- signer/signer.go | 4 ++-- signer/signer_test.go | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/main.go b/main.go index e376c44bc..79a73720a 100755 --- a/main.go +++ b/main.go @@ -304,7 +304,7 @@ func (a *autographer) initHSM(conf configuration) { // if we successfully initialized the crypto11 context, // tell the signers they can try using the HSM for i := range conf.Signers { - conf.Signers[i].HsmIsAvailable(tmpCtx) + conf.Signers[i].InitHSM(tmpCtx) } } } diff --git a/signer/signer.go b/signer/signer.go index fb35f74c4..83d576301 100644 --- a/signer/signer.go +++ b/signer/signer.go @@ -143,8 +143,8 @@ type Configuration struct { hsmCtx *pkcs11.Ctx } -// HsmIsAvailable indicates that an HSM has been initialized -func (cfg *Configuration) HsmIsAvailable(ctx *pkcs11.Ctx) { +// InitHSM indicates that an HSM has been initialized +func (cfg *Configuration) InitHSM(ctx *pkcs11.Ctx) { cfg.isHsmAvailable = true cfg.hsmCtx = ctx } diff --git a/signer/signer_test.go b/signer/signer_test.go index 988c60b97..93b3cb131 100644 --- a/signer/signer_test.go +++ b/signer/signer_test.go @@ -157,9 +157,9 @@ func TestParseEmptyPrivateKey(t *testing.T) { } } -func TestEnableHSM(t *testing.T) { +func TestInitHSM(t *testing.T) { tcfg := new(Configuration) - tcfg.HsmIsAvailable(nil) + tcfg.InitHSM(nil) if !tcfg.isHsmAvailable { t.Fatal("expected isHsmAvailable to be set to true but still false") } @@ -190,7 +190,7 @@ func TestHSMNotAvailable(t *testing.T) { } }() tcfg := new(Configuration) - tcfg.HsmIsAvailable(nil) + tcfg.InitHSM(nil) tcfg.GetPrivateKey() } From 066a7fe7dfec6e3828d8f5a5712e9537de77b97d Mon Sep 17 00:00:00 2001 From: Greg Guthe Date: Fri, 19 Jul 2019 15:37:57 -0400 Subject: [PATCH 6/7] add heartbeat handler --- handlers.go | 41 ++++++++++++++++++++++++++++++++++ handlers_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++++++-- main.go | 14 +++++++++++- signer/signer.go | 46 +++++++++++++++++++++++++++++++++----- 4 files changed, 149 insertions(+), 9 deletions(-) diff --git a/handlers.go b/handlers.go index 79257d2bb..684ca72a9 100644 --- a/handlers.go +++ b/handlers.go @@ -238,6 +238,47 @@ func handleLBHeartbeat(w http.ResponseWriter, r *http.Request) { w.Write([]byte("ohai")) } +// handleHeartbeat checks whether backing services are enabled and +// accessible and returns 200 when they are and 502 when the +// aren't. Currently it only checks whether the HSM is accessible. +func (a *autographer) handleHeartbeat(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + httpError(w, r, http.StatusMethodNotAllowed, "%s method not allowed; endpoint accepts GET only", r.Method) + return + } + var ( + // a map of backing service name to up or down/inaccessible status + result = map[string]bool{} + status = http.StatusOK + ) + + // try to fetch the private key from the HSM for the first + // signer conf with a non-PEM private key that we saved on + // server start + conf := a.hsmHeartbeatSignerConf + if conf != nil { + err := conf.CheckHSMConnection() + if err == nil { + result["hsmAccessible"] = true + status = http.StatusOK + } else { + log.Errorf("error checking HSM connection for signer %s: %s", conf.ID, err) + result["hsmAccessible"] = false + status = http.StatusInternalServerError + } + } + + respdata, err := json.Marshal(result) + if err != nil { + log.Errorf("heartbeat failed to marshal JSON with error: %s", err) + httpError(w, r, http.StatusInternalServerError, "error marshaling response JSON") + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + w.Write(respdata) +} + func handleVersion(w http.ResponseWriter, r *http.Request) { if r.Method != "GET" { httpError(w, r, http.StatusMethodNotAllowed, "%s method not allowed; endpoint accepts GET only", r.Method) diff --git a/handlers_test.go b/handlers_test.go index e3e3949f8..a5a0cecd9 100644 --- a/handlers_test.go +++ b/handlers_test.go @@ -360,7 +360,7 @@ func TestAuthFail(t *testing.T) { } } -func TestHeartbeat(t *testing.T) { +func TestLBHeartbeat(t *testing.T) { t.Parallel() var TESTCASES = []struct { @@ -373,7 +373,7 @@ func TestHeartbeat(t *testing.T) { {http.StatusMethodNotAllowed, `HEAD`}, } for i, testcase := range TESTCASES { - req, err := http.NewRequest(testcase.method, "http://foo.bar/__heartbeat__", nil) + req, err := http.NewRequest(testcase.method, "http://foo.bar/__lbheartbeat__", nil) if err != nil { t.Fatal(err) } @@ -386,6 +386,59 @@ func TestHeartbeat(t *testing.T) { } } +func TestHeartbeat(t *testing.T) { + t.Parallel() + + var TESTCASES = []struct { + expectedHTTPStatus int + method string + }{ + {http.StatusOK, `GET`}, + {http.StatusMethodNotAllowed, `POST`}, + {http.StatusMethodNotAllowed, `PUT`}, + {http.StatusMethodNotAllowed, `HEAD`}, + } + for i, testcase := range TESTCASES { + req, err := http.NewRequest(testcase.method, "http://foo.bar/__heartbeat__", nil) + if err != nil { + t.Fatal(err) + } + w := httptest.NewRecorder() + ag.handleHeartbeat(w, req) + if w.Code != testcase.expectedHTTPStatus { + t.Fatalf("test case %d failed with code %d but %d was expected", + i, w.Code, testcase.expectedHTTPStatus) + } + if bytes.Equal(w.Body.Bytes(), []byte("{}\n")) { + t.Fatalf("test case %d returned unexpected heartbeat body %s expected {}", i, w.Body.Bytes()) + } + } +} + +func TestHeartbeatChecksHSMStatusFails(t *testing.T) { + // NB: do not run in parallel with TestHeartbeat + ag.hsmHeartbeatSignerConf = &ag.signers[0].(*contentsignature.ContentSigner).Configuration + + expectedStatus := http.StatusInternalServerError + expectedBody := []byte("{\"hsmAccessible\":false}") + + req, err := http.NewRequest(`GET`, "http://foo.bar/__heartbeat__", nil) + if err != nil { + t.Fatal(err) + } + w := httptest.NewRecorder() + ag.handleHeartbeat(w, req) + + if w.Code != expectedStatus { + t.Fatalf("failed with code %d but %d was expected", w.Code, expectedStatus) + } + if !bytes.Equal(w.Body.Bytes(), expectedBody) { + t.Fatalf("got unexpected heartbeat body %s expected %s", w.Body.Bytes(), expectedBody) + } + + ag.hsmHeartbeatSignerConf = nil +} + func TestVersion(t *testing.T) { t.Parallel() diff --git a/main.go b/main.go index 79a73720a..c1031d2b9 100755 --- a/main.go +++ b/main.go @@ -78,6 +78,11 @@ type autographer struct { signerIndex map[string]int nonces *lru.Cache debug bool + + // hsmHeartbeatSignerConf is the signer conf to use to check + // HSM connectivity (set to the first signer with an HSM label + // in initHSM) when it is non-nil + hsmHeartbeatSignerConf *signer.Configuration } func main() { @@ -172,7 +177,7 @@ func run(conf configuration, listen string, authPrint, debug bool) { ag.startCleanupHandler() router := mux.NewRouter().StrictSlash(true) - router.HandleFunc("/__heartbeat__", handleHeartbeat).Methods("GET") + router.HandleFunc("/__heartbeat__", ag.handleHeartbeat).Methods("GET") router.HandleFunc("/__lbheartbeat__", handleLBHeartbeat).Methods("GET") router.HandleFunc("/__version__", handleVersion).Methods("GET") router.HandleFunc("/__monitor__", ag.handleMonitor).Methods("GET") @@ -305,6 +310,13 @@ func (a *autographer) initHSM(conf configuration) { // tell the signers they can try using the HSM for i := range conf.Signers { conf.Signers[i].InitHSM(tmpCtx) + signerConf := conf.Signers[i] + + // save the first signer with an HSM label as + // the key to test from the heartbeat handler + if a.hsmHeartbeatSignerConf == nil && !signerConf.PrivateKeyHasPEMPrefix() { + a.hsmHeartbeatSignerConf = &signerConf + } } } } diff --git a/signer/signer.go b/signer/signer.go index 83d576301..3479b17fa 100644 --- a/signer/signer.go +++ b/signer/signer.go @@ -244,12 +244,8 @@ func (cfg *Configuration) GetKeysAndRand() (priv crypto.PrivateKey, pub crypto.P // // Note that we assume the PKCS11 library has been previously initialized func (cfg *Configuration) GetPrivateKey() (crypto.PrivateKey, error) { - // make sure heading newlines are removed - removeNewlines := regexp.MustCompile(`^(\r?\n)`) - cfg.PrivateKey = removeNewlines.ReplaceAllString(cfg.PrivateKey, "") - // if a private key in the config starts with a PEM header, it is - // defined locally and is parsed and returned - if strings.HasPrefix(cfg.PrivateKey, "-----BEGIN") { + cfg.PrivateKey = removePrivateKeyNewlines(cfg.PrivateKey) + if cfg.PrivateKeyHasPEMPrefix() { return ParsePrivateKey([]byte(cfg.PrivateKey)) } // otherwise, we assume the privatekey represents a label in the HSM @@ -353,6 +349,44 @@ func parseDSAPKCS8PrivateKey(der []byte) (*dsa.PrivateKey, error) { }, nil } +func removePrivateKeyNewlines(confPrivateKey string) string { + // make sure heading newlines are removed + removeNewlines := regexp.MustCompile(`^(\r?\n)`) + return removeNewlines.ReplaceAllString(confPrivateKey, "") +} + +// PrivateKeyHasPEMPrefix returns whether the signer configuration +// prefix begins with `-----BEGIN` (indicating a PEM block) after +// stripping newlines +func (cfg *Configuration) PrivateKeyHasPEMPrefix() bool { + // if a private key in the config starts with a PEM header, it is + // defined locally and is parsed and returned + return strings.HasPrefix(removePrivateKeyNewlines(cfg.PrivateKey), "-----BEGIN") +} + +// CheckHSMConnection is the default implementation of +// CheckHSMConnection (exposed via the signer.Configuration +// interface). It tried to fetch the signer private key and errors if +// that fails or the private key is not an HSM key handle. +func (cfg *Configuration) CheckHSMConnection() error { + if cfg.PrivateKeyHasPEMPrefix() { + return errors.Errorf("private key for signer %s has a PEM prefix and is not an HSM key label", cfg.ID) + } + if !cfg.isHsmAvailable { + return errors.Errorf("HSM is not available for signer %s", cfg.ID) + } + + privKey, err := cfg.GetPrivateKey() + if err != nil { + return errors.Wrapf(err, "error fetching private key for signer %s", cfg.ID) + } + // returns 0 if the key is not stored in the hsm + if GetPrivKeyHandle(privKey) != 0 { + return nil + } + return errors.Errorf("Unable to check HSM connection for signer %s private key is not stored in the HSM", cfg.ID) +} + // MakeKey generates a new key of type keyTpl and returns the priv and public interfaces. // If an HSM is available, it is used to generate and store the key, in which case 'priv' // just points to the HSM handler and must be used via the crypto.Signer interface. From 76f64b99049537f3df13e4f6e8dd7bdfb7e0488b Mon Sep 17 00:00:00 2001 From: Greg Guthe Date: Tue, 23 Jul 2019 10:04:33 -0400 Subject: [PATCH 7/7] handlers: check DB access from heartbeat but don't fail when inaccessible --- handlers.go | 13 +++++++++++++ handlers_test.go | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/handlers.go b/handlers.go index 684ca72a9..2f117700d 100644 --- a/handlers.go +++ b/handlers.go @@ -268,6 +268,19 @@ func (a *autographer) handleHeartbeat(w http.ResponseWriter, r *http.Request) { } } + // check the database connection and return its status, but + // don't fail the heartbeat since we only care about DB + // connectivity on server start + if a.db != nil { + err := a.db.CheckConnection() + if err == nil { + result["dbAccessible"] = true + } else { + log.Errorf("error checking DB connection: %s", err) + result["dbAccessible"] = false + } + } + respdata, err := json.Marshal(result) if err != nil { log.Errorf("heartbeat failed to marshal JSON with error: %s", err) diff --git a/handlers_test.go b/handlers_test.go index a5a0cecd9..07bb1640e 100644 --- a/handlers_test.go +++ b/handlers_test.go @@ -24,6 +24,7 @@ import ( "strings" "testing" + "go.mozilla.org/autograph/database" "go.mozilla.org/autograph/signer/apk" "go.mozilla.org/autograph/signer/contentsignature" "go.mozilla.org/autograph/signer/mar" @@ -416,7 +417,7 @@ func TestHeartbeat(t *testing.T) { } func TestHeartbeatChecksHSMStatusFails(t *testing.T) { - // NB: do not run in parallel with TestHeartbeat + // NB: do not run in parallel with TestHeartbeat* ag.hsmHeartbeatSignerConf = &ag.signers[0].(*contentsignature.ContentSigner).Configuration expectedStatus := http.StatusInternalServerError @@ -439,6 +440,40 @@ func TestHeartbeatChecksHSMStatusFails(t *testing.T) { ag.hsmHeartbeatSignerConf = nil } +func TestHeartbeatChecksDBStatusOK(t *testing.T) { + // NB: do not run in parallel with TestHeartbeat* or DB tests + db, err := database.Connect(database.Config{ + Name: "autograph", + User: "myautographdbuser", + Password: "myautographdbpassword", + Host: "127.0.0.1:5432", + }) + if err != nil { + t.Fatal(err) + } + ag.db = db + + expectedStatus := http.StatusOK + expectedBody := []byte("{\"dbAccessible\":true}") + + req, err := http.NewRequest(`GET`, "http://foo.bar/__heartbeat__", nil) + if err != nil { + t.Fatal(err) + } + w := httptest.NewRecorder() + ag.handleHeartbeat(w, req) + + if w.Code != expectedStatus { + t.Fatalf("failed with code %d but %d was expected", w.Code, expectedStatus) + } + if !bytes.Equal(w.Body.Bytes(), expectedBody) { + t.Fatalf("got unexpected heartbeat body %s expected %s", w.Body.Bytes(), expectedBody) + } + + db.Close() + ag.db = nil +} + func TestVersion(t *testing.T) { t.Parallel()