Make internal errors more useful

api/api.go

@@ -324,7 +324,7 @@ func (h *caHandler) Provisioners(w http.ResponseWriter, r *http.Request) {
 
 	p, next, err := h.Authority.GetProvisioners(cursor, limit)
 	if err != nil {
-		WriteError(w, errs.InternalServerErr(err))
+		WriteError(w, errs.InternalServerErr(err, "error getting provisioners"))
 		return
 	}
 	JSON(w, &ProvisionersResponse{

api/api_test.go

@@ -1090,7 +1090,7 @@ func Test_caHandler_Provisioners(t *testing.T) {
 	expectedError400 := errs.BadRequest("limit 'abc' is not an integer")
 	expectedError400Bytes, err := json.Marshal(expectedError400)
 	assert.FatalError(t, err)
-	expectedError500 := errs.InternalServer("force")
+	expectedError500 := errs.InternalServer("error getting provisioners")
 	expectedError500Bytes, err := json.Marshal(expectedError500)
 	assert.FatalError(t, err)
 	for _, tt := range tests {

api/rekey.go

@@ -44,7 +44,7 @@ func (h *caHandler) Rekey(w http.ResponseWriter, r *http.Request) {
 
 	certChain, err := h.Authority.Rekey(r.TLS.PeerCertificates[0], body.CsrPEM.CertificateRequest.PublicKey)
 	if err != nil {
-		WriteError(w, errs.Wrap(http.StatusInternalServerError, err, "cahandler.Rekey"))
+		WriteError(w, errs.InternalServerErr(err, "error rekeying certificate"))
 		return
 	}
 	certChainPEM := certChainToPEM(certChain)

api/renew.go

@@ -16,7 +16,7 @@ func (h *caHandler) Renew(w http.ResponseWriter, r *http.Request) {
 
 	certChain, err := h.Authority.Renew(r.TLS.PeerCertificates[0])
 	if err != nil {
-		WriteError(w, errs.Wrap(http.StatusInternalServerError, err, "cahandler.Renew"))
+		WriteError(w, errs.InternalServerErr(err, "error renewing certificate"))
 		return
 	}
 	certChainPEM := certChainToPEM(certChain)

api/ssh.go

@@ -344,7 +344,7 @@ func (h *caHandler) SSHSign(w http.ResponseWriter, r *http.Request) {
 func (h *caHandler) SSHRoots(w http.ResponseWriter, r *http.Request) {
 	keys, err := h.Authority.GetSSHRoots(r.Context())
 	if err != nil {
-		WriteError(w, errs.InternalServerErr(err))
+		WriteError(w, errs.InternalServerErr(err, "error getting ssh roots"))
 		return
 	}
 
@@ -369,7 +369,7 @@ func (h *caHandler) SSHRoots(w http.ResponseWriter, r *http.Request) {
 func (h *caHandler) SSHFederation(w http.ResponseWriter, r *http.Request) {
 	keys, err := h.Authority.GetSSHFederation(r.Context())
 	if err != nil {
-		WriteError(w, errs.InternalServerErr(err))
+		WriteError(w, errs.InternalServerErr(err, "error getting federated ssh roots"))
 		return
 	}
 
@@ -404,7 +404,7 @@ func (h *caHandler) SSHConfig(w http.ResponseWriter, r *http.Request) {
 
 	ts, err := h.Authority.GetSSHConfig(r.Context(), body.Type, body.Data)
 	if err != nil {
-		WriteError(w, errs.InternalServerErr(err))
+		WriteError(w, errs.InternalServerErr(err, "error getting ssh config"))
 		return
 	}
 
@@ -415,7 +415,7 @@ func (h *caHandler) SSHConfig(w http.ResponseWriter, r *http.Request) {
 	case provisioner.SSHHostCert:
 		cfg.HostTemplates = ts
 	default:
-		WriteError(w, errs.InternalServer("it should hot get here"))
+		WriteError(w, errs.Internal("it should hot get here"))
 		return
 	}
 
@@ -436,7 +436,7 @@ func (h *caHandler) SSHCheckHost(w http.ResponseWriter, r *http.Request) {
 
 	exists, err := h.Authority.CheckSSHHost(r.Context(), body.Principal, body.Token)
 	if err != nil {
-		WriteError(w, errs.InternalServerErr(err))
+		WriteError(w, errs.InternalServerErr(err, "error checking for host"))
 		return
 	}
 	JSON(w, &SSHCheckPrincipalResponse{
@@ -453,7 +453,7 @@ func (h *caHandler) SSHGetHosts(w http.ResponseWriter, r *http.Request) {
 
 	hosts, err := h.Authority.GetSSHHosts(r.Context(), cert)
 	if err != nil {
-		WriteError(w, errs.InternalServerErr(err))
+		WriteError(w, errs.InternalServerErr(err, "error getting ssh hosts"))
 		return
 	}
 	JSON(w, &SSHGetHostsResponse{
@@ -475,7 +475,7 @@ func (h *caHandler) SSHBastion(w http.ResponseWriter, r *http.Request) {
 
 	bastion, err := h.Authority.GetSSHBastion(r.Context(), body.User, body.Hostname)
 	if err != nil {
-		WriteError(w, errs.InternalServerErr(err))
+		WriteError(w, errs.InternalServerErr(err, "error getting ssh bastion"))
 		return
 	}
 

api/sshRekey.go

@@ -63,7 +63,7 @@ func (h *caHandler) SSHRekey(w http.ResponseWriter, r *http.Request) {
 	}
 	oldCert, _, err := provisioner.ExtractSSHPOPCert(body.OTT)
 	if err != nil {
-		WriteError(w, errs.InternalServerErr(err))
+		WriteError(w, errs.InternalServerErr(err, "error exacting ssh certificate"))
 	}
 
 	newCert, err := h.Authority.RekeySSH(ctx, oldCert, publicKey, signOpts...)

api/sshRenew.go

@@ -55,7 +55,7 @@ func (h *caHandler) SSHRenew(w http.ResponseWriter, r *http.Request) {
 	}
 	oldCert, _, err := provisioner.ExtractSSHPOPCert(body.OTT)
 	if err != nil {
-		WriteError(w, errs.InternalServerErr(err))
+		WriteError(w, errs.InternalServerErr(err, "error extraction ssh certificate"))
 	}
 
 	newCert, err := h.Authority.RenewSSH(ctx, oldCert)

authority/admin/api/provisioner.go

@@ -53,14 +53,13 @@ func (h *Handler) GetProvisioner(w http.ResponseWriter, r *http.Request) {
 func (h *Handler) GetProvisioners(w http.ResponseWriter, r *http.Request) {
 	cursor, limit, err := api.ParseCursor(r)
 	if err != nil {
-		api.WriteError(w, admin.WrapError(admin.ErrorBadRequestType, err,
-			"error parsing cursor & limit query params"))
+		api.WriteError(w, err)
 		return
 	}
 
 	p, next, err := h.auth.GetProvisioners(cursor, limit)
 	if err != nil {
-		api.WriteError(w, errs.InternalServerErr(err))
+		api.WriteError(w, errs.InternalServerErr(err, "error getting provisioners"))
 		return
 	}
 	api.JSON(w, &GetProvisionersResponse{

authority/authorize.go

@@ -227,7 +227,10 @@ func (a *Authority) Authorize(ctx context.Context, token string) ([]provisioner.
 		_, signOpts, err := a.authorizeSSHRekey(ctx, token)
 		return signOpts, errs.Wrap(http.StatusInternalServerError, err, "authority.Authorize", opts...)
 	default:
-		return nil, errs.InternalServer("authority.Authorize; method %d is not supported", append([]interface{}{m}, opts...)...)
+		return nil, errs.ApplyOptions(
+			errs.InternalServer("method %d is not supported", m),
+			opts...,
+		)
 	}
 }
 

authority/authorize_test.go

@@ -722,7 +722,7 @@ func TestAuthority_Authorize(t *testing.T) {
 				auth:  a,
 				token: "foo",
 				ctx:   provisioner.NewContextWithMethod(context.Background(), 15),
-				err:   errors.New("authority.Authorize; method 15 is not supported"),
+				err:   errors.New("method 15 is not supported"),
 				code:  http.StatusInternalServerError,
 			}
 		},

authority/provisioner/aws.go

@@ -619,7 +619,7 @@ func (p *AWS) authorizeToken(token string) (*awsPayload, error) {
 		return nil, errs.Wrapf(http.StatusUnauthorized, err, "aws.authorizeToken; error parsing aws token")
 	}
 	if len(jwt.Headers) == 0 {
-		return nil, errs.InternalServer("aws.authorizeToken; error parsing token, header is missing")
+		return nil, errs.BadRequest("error parsing token, header is missing")
 	}
 
 	var unsafeClaims awsPayload

authority/provisioner/sshpop.go

@@ -126,7 +126,7 @@ func (p *SSHPOP) authorizeToken(token string, audiences []string) (*sshPOPPayloa
 	}
 	sshCryptoPubKey, ok := sshCert.Key.(ssh.CryptoPublicKey)
 	if !ok {
-		return nil, errs.InternalServer("sshpop.authorizeToken; sshpop public key could not be cast to ssh CryptoPublicKey")
+		return nil, errs.Internal("sshpop public key could not be cast to ssh CryptoPublicKey")
 	}
 	pubKey := sshCryptoPubKey.CryptoPublicKey()
 

authority/root.go

@@ -15,7 +15,7 @@ func (a *Authority) Root(sum string) (*x509.Certificate, error) {
 
 	crt, ok := val.(*x509.Certificate)
 	if !ok {
-		return nil, errs.InternalServer("stored value is not a *x509.Certificate")
+		return nil, errs.Internal("stored value is not a *x509.Certificate")
 	}
 	return crt, nil
 }
@@ -49,7 +49,7 @@ func (a *Authority) GetFederation() (federation []*x509.Certificate, err error)
 		crt, ok := v.(*x509.Certificate)
 		if !ok {
 			federation = nil
-			err = errs.InternalServer("stored value is not a *x509.Certificate")
+			err = errs.Internal("stored value is not a *x509.Certificate")
 			return false
 		}
 		federation = append(federation, crt)

authority/root_test.go

@@ -22,7 +22,7 @@ func TestRoot(t *testing.T) {
 		code int
 	}{
 		"not-found":                  {"foo", errors.New("certificate with fingerprint foo was not found"), http.StatusNotFound},
-		"invalid-stored-certificate": {"invaliddata", errors.New("stored value is not a *x509.Certificate"), http.StatusInternalServerError},
+		"invalid-stored-certificate": {"invaliddata", errors.New(errs.InternalServerErrorDefaultMsg), http.StatusInternalServerError},
 		"success":                    {"189f573cfa159251e445530847ef80b1b62a3a380ee670dcb49e33ed34da0616", nil, http.StatusOK},
 	}
 

authority/ssh.go

@@ -177,7 +177,7 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi
 			}
 
 		default:
-			return nil, errs.InternalServer("authority.SignSSH: invalid extra option type %T", o)
+			return nil, errs.Internal("invalid extra SignOption of type %T", o)
 		}
 	}
 
@@ -231,7 +231,7 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi
 		}
 		signer = a.sshCAHostCertSignKey
 	default:
-		return nil, errs.InternalServer("authority.SignSSH: unexpected ssh certificate type: %d", certTpl.CertType)
+		return nil, errs.Internal("invalid ssh certificate of type '%d'", certTpl.CertType)
 	}
 
 	// Sign certificate.
@@ -297,7 +297,7 @@ func (a *Authority) RenewSSH(ctx context.Context, oldCert *ssh.Certificate) (*ss
 		}
 		signer = a.sshCAHostCertSignKey
 	default:
-		return nil, errs.InternalServer("renewSSH: unexpected ssh certificate type: %d", certTpl.CertType)
+		return nil, errs.Internal("invalid ssh certificate of type '%d'", certTpl.CertType)
 	}
 
 	// Sign certificate.
@@ -323,7 +323,7 @@ func (a *Authority) RekeySSH(ctx context.Context, oldCert *ssh.Certificate, pub
 		case provisioner.SSHCertValidator:
 			validators = append(validators, o)
 		default:
-			return nil, errs.InternalServer("rekeySSH; invalid extra option type %T", o)
+			return nil, errs.Internal("invalid extra SignOption of type %T", o)
 		}
 	}
 

authority/ssh_test.go

@@ -901,7 +901,7 @@ func TestAuthority_RekeySSH(t *testing.T) {
 				hostSigner: signer,
 				key:        pub,
 				signOpts:   []provisioner.SignOption{userOptions},
-				err:        errors.New("rekeySSH; invalid extra option type"),
+				err:        errors.New(errs.InternalServerErrorDefaultMsg),
 				code:       http.StatusInternalServerError,
 			}
 		},

authority/tls.go

@@ -113,7 +113,10 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Sign
 			certEnforcers = append(certEnforcers, k)
 
 		default:
-			return nil, errs.InternalServer("authority.Sign; invalid extra option type %T", append([]interface{}{k}, opts...)...)
+			return nil, errs.ApplyOptions(
+				errs.Internal("invalid extra SignOption of type %T", k),
+				opts...,
+			)
 		}
 	}
 

authority/tls_test.go

@@ -268,7 +268,7 @@ func TestAuthority_Sign(t *testing.T) {
 				csr:       csr,
 				extraOpts: append(extraOpts, "42"),
 				signOpts:  signOpts,
-				err:       errors.New("authority.Sign; invalid extra option type string"),
+				err:       errors.New(errs.InternalServerErrorDefaultMsg),
 				code:      http.StatusInternalServerError,
 			}
 		},

ca/client_test.go

@@ -163,7 +163,7 @@ func TestClient_Version(t *testing.T) {
 		expectedErr  error
 	}{
 		{"ok", ok, 200, false, nil},
-		{"500", errs.InternalServer("force"), 500, true, errors.New(errs.InternalServerErrorDefaultMsg)},
+		{"500", errs.InternalServer("force"), 500, true, errors.New(errs.InternalServerPrefix)},
 		{"404", errs.NotFound("force"), 404, true, errors.New(errs.NotFoundDefaultMsg)},
 	}
 
@@ -193,7 +193,7 @@ func TestClient_Version(t *testing.T) {
 				if got != nil {
 					t.Errorf("Client.Version() = %v, want nil", got)
 				}
-				assert.HasPrefix(t, tt.expectedErr.Error(), err.Error())
+				assert.HasPrefix(t, err.Error(), tt.expectedErr.Error())
 			default:
 				if !reflect.DeepEqual(got, tt.response) {
 					t.Errorf("Client.Version() = %v, want %v", got, tt.response)
@@ -214,7 +214,7 @@ func TestClient_Health(t *testing.T) {
 		expectedErr  error
 	}{
 		{"ok", ok, 200, false, nil},
-		{"not ok", errs.InternalServer("force"), 500, true, errors.New(errs.InternalServerErrorDefaultMsg)},
+		{"not ok", errs.InternalServer("force"), 500, true, errors.New(errs.InternalServerPrefix)},
 	}
 
 	srv := httptest.NewServer(nil)
@@ -244,7 +244,7 @@ func TestClient_Health(t *testing.T) {
 				if got != nil {
 					t.Errorf("Client.Health() = %v, want nil", got)
 				}
-				assert.HasPrefix(t, tt.expectedErr.Error(), err.Error())
+				assert.HasPrefix(t, err.Error(), tt.expectedErr.Error())
 			default:
 				if !reflect.DeepEqual(got, tt.response) {
 					t.Errorf("Client.Health() = %v, want %v", got, tt.response)
@@ -648,7 +648,7 @@ func TestClient_Provisioners(t *testing.T) {
 				if got != nil {
 					t.Errorf("Client.Provisioners() = %v, want nil", got)
 				}
-				assert.HasPrefix(t, errs.InternalServerErrorDefaultMsg, err.Error())
+				assert.HasPrefix(t, err.Error(), errs.InternalServerPrefix)
 			default:
 				if !reflect.DeepEqual(got, tt.response) {
 					t.Errorf("Client.Provisioners() = %v, want %v", got, tt.response)

ca/testdata/ca.json

@@ -6,7 +6,7 @@
     "password": "password",
     "address": "127.0.0.1:0",
     "dnsNames": ["127.0.0.1"],
-    "logger": {"format": "text"},
+    "_logger": {"format": "text"},
     "tls": {
         "minVersion": 1.2,
         "maxVersion": 1.3,

ca/testdata/federated-ca.json

@@ -6,7 +6,7 @@
     "password": "asdf",
     "address": "127.0.0.1:0",
     "dnsNames": ["127.0.0.1"],
-    "logger": {"format": "text"},
+    "_logger": {"format": "text"},
     "tls": {
         "minVersion": 1.2,
         "maxVersion": 1.2,

ca/testdata/rotate-ca-0.json

@@ -5,7 +5,7 @@
     "password": "password",
     "address": "127.0.0.1:0",
     "dnsNames": ["127.0.0.1"],
-    "logger": {"format": "text"},
+    "_logger": {"format": "text"},
     "tls": {
         "minVersion": 1.2,
         "maxVersion": 1.2,

ca/testdata/rotate-ca-1.json

@@ -5,7 +5,7 @@
     "password": "password",
     "address": "127.0.0.1:0",
     "dnsNames": ["127.0.0.1"],
-    "logger": {"format": "text"},
+    "_logger": {"format": "text"},
     "tls": {
         "minVersion": 1.2,
         "maxVersion": 1.2,

ca/testdata/rotate-ca-2.json

@@ -5,7 +5,7 @@
     "password": "asdf",
     "address": "127.0.0.1:0",
     "dnsNames": ["127.0.0.1"],
-    "logger": {"format": "text"},
+    "_logger": {"format": "text"},
     "tls": {
         "minVersion": 1.2,
         "maxVersion": 1.2,

ca/testdata/rotate-ca-3.json

@@ -5,7 +5,7 @@
     "password": "asdf",
     "address": "127.0.0.1:0",
     "dnsNames": ["127.0.0.1"],
-    "logger": {"format": "text"},
+    "_logger": {"format": "text"},
     "tls": {
         "minVersion": 1.2,
         "maxVersion": 1.2,