From 0894cd6ef0a7c1aeccd6a078d5673db02f84afa2 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Thu, 2 Aug 2018 17:02:28 +0200 Subject: [PATCH] cli: enhance errors reported upon conn failures Prior to this patch, the various CLI commands were making a halfhearted attempt at decorating the underlying error with some troubleshooting instructions. Unfortunately, this was mixing up situations where the TCP connection failed outright, those where the security settings were incorrect, and connections dropped later, after the initial handshake was successful. In practice, we've noticed that the troubleshooting steps are rather different for both kinds of situations. So this patch enhances the code to clarify what is going on. It attempts to distinguish: - TCP connection problem ("check --host vs --advertise-host"). - secure conn to insecure server ("use --insecure?") - invalid TLS settings / TLS auth error ("check credentials"). - timeouts. - connection lost (e.g. conn closed by server). Release note (cli change): The various `cockroach` client comments now better attempt to inform the user about why a connection is failing. --- pkg/base/config.go | 30 ++-- pkg/cli/cli_test.go | 56 ------- pkg/cli/error.go | 147 +++++++++++++++-- pkg/cli/interactive_tests/netcat.py | 15 ++ .../interactive_tests/test_error_hints.tcl | 156 ++++++++++++++++++ pkg/cli/sql_util.go | 27 ++- pkg/security/certificate_loader.go | 12 +- pkg/security/certificate_manager.go | 51 ++++-- 8 files changed, 385 insertions(+), 109 deletions(-) create mode 100644 pkg/cli/interactive_tests/netcat.py create mode 100644 pkg/cli/interactive_tests/test_error_hints.tcl diff --git a/pkg/base/config.go b/pkg/base/config.go index 2a34976affc5..878b101b7ea7 100644 --- a/pkg/base/config.go +++ b/pkg/base/config.go @@ -22,8 +22,6 @@ import ( "sync" "time" - "github.com/pkg/errors" - "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/util/envutil" @@ -152,8 +150,14 @@ type Config struct { HistogramWindowInterval time.Duration } -func didYouMeanInsecureError(err error) error { - return errors.Wrap(err, "problem using security settings, did you mean to use --insecure?") +func wrapError(err error) error { + if _, ok := err.(*security.Error); !ok { + return &security.Error{ + Message: "problem using security settings", + Err: err, + } + } + return err } // InitDefaults sets up the default values for a config. @@ -212,7 +216,7 @@ func (cfg *Config) LoadSecurityOptions(options url.Values, username string) erro // Fetch CA cert. This is required. caCertPath, err := cfg.GetCACertPath() if err != nil { - return didYouMeanInsecureError(err) + return wrapError(err) } options.Add("sslmode", "verify-full") options.Add("sslrootcert", caCertPath) @@ -294,12 +298,12 @@ func (cfg *Config) GetClientTLSConfig() (*tls.Config, error) { cm, err := cfg.GetCertificateManager() if err != nil { - return nil, didYouMeanInsecureError(err) + return nil, wrapError(err) } tlsCfg, err := cm.GetClientTLSConfig(cfg.User) if err != nil { - return nil, didYouMeanInsecureError(err) + return nil, wrapError(err) } return tlsCfg, nil } @@ -316,12 +320,12 @@ func (cfg *Config) GetUIClientTLSConfig() (*tls.Config, error) { cm, err := cfg.GetCertificateManager() if err != nil { - return nil, didYouMeanInsecureError(err) + return nil, wrapError(err) } tlsCfg, err := cm.GetUIClientTLSConfig() if err != nil { - return nil, didYouMeanInsecureError(err) + return nil, wrapError(err) } return tlsCfg, nil } @@ -337,12 +341,12 @@ func (cfg *Config) GetServerTLSConfig() (*tls.Config, error) { cm, err := cfg.GetCertificateManager() if err != nil { - return nil, didYouMeanInsecureError(err) + return nil, wrapError(err) } tlsCfg, err := cm.GetServerTLSConfig() if err != nil { - return nil, didYouMeanInsecureError(err) + return nil, wrapError(err) } return tlsCfg, nil } @@ -358,12 +362,12 @@ func (cfg *Config) GetUIServerTLSConfig() (*tls.Config, error) { cm, err := cfg.GetCertificateManager() if err != nil { - return nil, didYouMeanInsecureError(err) + return nil, wrapError(err) } tlsCfg, err := cm.GetUIServerTLSConfig() if err != nil { - return nil, didYouMeanInsecureError(err) + return nil, wrapError(err) } return tlsCfg, nil } diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go index b480d6b67d23..9f17d27a5d7e 100644 --- a/pkg/cli/cli_test.go +++ b/pkg/cli/cli_test.go @@ -339,62 +339,6 @@ func TestQuit(t *testing.T) { c.Run("quit") // Wait until this async command cleanups the server. <-c.Stopper().IsStopped() - - // NB: if this test is ever flaky due to port reuse, we could run against - // :0 (which however changes some of the errors we get). - // One way of getting that is: - // c.Cfg.AdvertiseAddr = "127.0.0.1:0" - - styled := func(s string) string { - const preamble = `unable to connect or connection lost. - -Please check the address and credentials such as certificates \(if attempting to -communicate with a secure cluster\). - -` - return preamble + s - } - - for _, test := range []struct { - cmd, expOutPattern string - }{ - // Error returned from GRPC to internal/client (which has to pass it - // up the stack as a roachpb.NewError(roachpb.NewSendError(.)). - // Error returned directly from GRPC. - {`quit`, styled( - `Failed to connect to the node: initial connection heartbeat failed: rpc ` + - `error: code = Unavailable desc = all SubConns are in TransientFailure, ` + - `latest connection error: connection error: desc = "transport: Error while dialing dial tcp .*: ` + - `connect: connection refused"`), - }, - // Going through the SQL client libraries gives a *net.OpError which - // we also handle. - // - // On *nix, this error is: - // - // dial tcp 127.0.0.1:65054: getsockopt: connection refused - // - // On Windows, this error is: - // - // dial tcp 127.0.0.1:59951: connectex: No connection could be made because the target machine actively refused it. - // - // So we look for the common bit. - {`zone ls`, styled( - `dial tcp .*: .* refused`), - }, - } { - t.Run(test.cmd, func(t *testing.T) { - out, err := c.RunWithCapture(test.cmd) - if err != nil { - t.Fatal(err) - } - exp := test.cmd + "\n" + test.expOutPattern - re := regexp.MustCompile(exp) - if !re.MatchString(out) { - t.Errorf("expected '%s' to match pattern:\n%s\ngot:\n%s", test.cmd, exp, out) - } - }) - } } func Example_logging() { diff --git a/pkg/cli/error.go b/pkg/cli/error.go index 5adaecee71f4..e864473c8d3d 100644 --- a/pkg/cli/error.go +++ b/pkg/cli/error.go @@ -16,21 +16,51 @@ package cli import ( "context" + "crypto/x509" "fmt" "net" + "regexp" "strings" + "github.com/lib/pq" "github.com/pkg/errors" "github.com/spf13/cobra" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/security" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/util/grpcutil" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/netutil" ) +// reConnRefused is a regular expression that can be applied +// to the details of a GRPC connection failure. +// +// On *nix, a connect error looks like: +// dial tcp : : connection refused +// On Windows, it looks like: +// dial tcp : : No connection could be made because the target machine actively refused it. +// So we look for the common bit. +var reGRPCConnRefused = regexp.MustCompile(`Error while dialing dial tcp .*: connection.* refused`) + +// reGRPCNoTLS is a regular expression that can be applied to the +// details of a GRPC auth failure when the server is insecure. +var reGRPCNoTLS = regexp.MustCompile(`authentication handshake failed: tls: first record does not look like a TLS handshake`) + +// reGRPCAuthFailure is a regular expression that can be applied to +// the details of a GRPC auth failure when the SSL handshake fails. +var reGRPCAuthFailure = regexp.MustCompile(`authentication handshake failed: x509`) + +// reGRPCConnFailed is a regular expression that can be applied +// to the details of a GRPC connection failure when, perhaps, +// the server was expecting a TLS handshake but the client didn't +// provide one (i.e. the client was started with --insecure). +// Note however in that case it's not certain what the problem is, +// as the same error could be raised for other reasons. +var reGRPCConnFailed = regexp.MustCompile(`desc = transport is closing`) + // MaybeDecorateGRPCError catches grpc errors and provides a more helpful error // message to the user. func MaybeDecorateGRPCError( @@ -43,34 +73,115 @@ func MaybeDecorateGRPCError( return nil } - connDropped := func() error { - const format = `unable to connect or connection lost. - -Please check the address and credentials such as certificates (if attempting to -communicate with a secure cluster). + extraInsecureHint := func() string { + extra := "" + if baseCfg.Insecure { + extra = "\nIf the node is configured to require secure connections,\n" + + "remove --insecure and configure secure credentials instead.\n" + } + return extra + } -%s` + connFailed := func() error { + const format = "cannot dial server.\n" + + "Is the server running?\n" + + "If the server is running, check --host client-side and --advertise server-side.\n\n%v" return errors.Errorf(format, err) } - opTimeout := func() error { - const format = `operation timed out. -%s` - return errors.Errorf(format, err) + connSecurityHint := func() error { + const format = "SSL authentication error while connecting.\n%s\n%v" + return errors.Errorf(format, extraInsecureHint(), err) + } + + connInsecureHint := func() error { + return errors.Errorf("cannot establish secure connection to insecure server.\n"+ + "Maybe use --insecure?\n\n%v", err) + } + + connRefused := func() error { + extra := extraInsecureHint() + return errors.Errorf("server closed the connection.\n"+ + "Is this a CockroachDB node?\n"+extra+"\n%v", err) } // Is this an "unable to connect" type of error? unwrappedErr := errors.Cause(err) - switch unwrappedErr.(type) { - case *roachpb.SendError: - return connDropped() + + if unwrappedErr == pq.ErrSSLNotSupported { + // SQL command failed after establishing a TCP connection + // successfully, but discovering that it cannot use TLS while it + // expected the server supports TLS. + return connInsecureHint() + } + + switch wErr := unwrappedErr.(type) { + case *security.Error: + return errors.Errorf("cannot load certificates.\n"+ + "Check your certificate settings, set --certs-dir, or use --insecure for insecure clusters.\n\n%v", + unwrappedErr) + + case *x509.UnknownAuthorityError: + // A SQL connection was attempted with an incorrect CA. + return connSecurityHint() + + case *initialSQLConnectionError: + // SQL handshake failed after establishing a TCP connection + // successfully, something else than CockroachDB responded, was + // confused and closed the door on us. + return connRefused() + + case *pq.Error: + // SQL commands will fail with a pq error but only after + // establishing a TCP connection successfully. So if we got + // here, there was a TCP connection already. + + // Did we fail due to security settings? + if wErr.Code == pgerror.CodeProtocolViolationError { + return connSecurityHint() + } + // Otherwise, there was a regular SQL error. Just report that. + return wErr + case *net.OpError: - return connDropped() + // A non-RPC client command was used (e.g. a SQL command) and an + // error occurred early while establishing the TCP connection. + + // Is this a TLS error? + if msg := wErr.Err.Error(); strings.HasPrefix(msg, "tls: ") { + // Error during the SSL handshake: a provided client + // certificate was invalid, but the CA was OK. (If the CA was + // not OK, we'd get a x509 error, see case above.) + return connSecurityHint() + } + return connFailed() + case *netutil.InitialHeartbeatFailedError: - return connDropped() + // A GRPC TCP connection was established but there was an early failure. + // Try to distinguish the cases. + msg := wErr.Error() + if reGRPCConnRefused.MatchString(msg) { + return connFailed() + } + if reGRPCNoTLS.MatchString(msg) { + return connInsecureHint() + } + if reGRPCAuthFailure.MatchString(msg) { + return connSecurityHint() + } + if reGRPCConnFailed.MatchString(msg) { + return connRefused() + } + + // Other cases may be timeouts or other situations, these + // will be handled below. + } + + opTimeout := func() error { + return errors.Errorf("operation timed out.\n\n%v", err) } - // No, it's not. Is it a plain context cancellation (i.e. timeout)? + // Is it a plain context cancellation (i.e. timeout)? switch unwrappedErr { case context.DeadlineExceeded: return opTimeout() @@ -88,7 +199,7 @@ communicate with a secure cluster). return fmt.Errorf( "incompatible client and server versions (likely server version: v1.0, required: >=v1.1)") } else if grpcutil.IsClosedConnection(unwrappedErr) { - return connDropped() + return errors.Errorf("connection lost.\n\n%v", err) } // Nothing we can special case, just return what we have. diff --git a/pkg/cli/interactive_tests/netcat.py b/pkg/cli/interactive_tests/netcat.py new file mode 100644 index 000000000000..9075913bf65f --- /dev/null +++ b/pkg/cli/interactive_tests/netcat.py @@ -0,0 +1,15 @@ +import socket +import sys + +server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) +server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) +server.bind(("0.0.0.0", 26257)) +server.listen(1) +print "ready" +client_socket, addr = server.accept() +print "connected" + +while True: + c = client_socket.recv(1) + sys.stdout.write("%c" % c) + sys.stdout.flush() diff --git a/pkg/cli/interactive_tests/test_error_hints.tcl b/pkg/cli/interactive_tests/test_error_hints.tcl new file mode 100644 index 000000000000..bc88f3467649 --- /dev/null +++ b/pkg/cli/interactive_tests/test_error_hints.tcl @@ -0,0 +1,156 @@ +#! /usr/bin/env expect -f + +source [file join [file dirname $argv0] common.tcl] + +set fakeserv [file join [file dirname $argv0] netcat.py] + +# We'll override security defaults below. +set certs_dir "/certs" +set ::env(COCKROACH_INSECURE) "false" + +spawn /bin/bash +set shell_spawn_id $spawn_id + +send "PS1=':''/# '\r" +eexpect ":/# " + +# Check what happens when attempting to connect and the server does not exist. + +start_test "Connecting a RPC client to a non-started server" +send "$argv quit\r" +eexpect "Error: cannot load certificates.\r\nCheck your certificate settings" +eexpect "or use --insecure" +eexpect ":/# " + +send "$argv quit --certs-dir=$certs_dir\r" +eexpect "Error: cannot dial server.\r\nIs the server running?" +eexpect "connection refused" +eexpect ":/# " +end_test + +start_test "Connecting a SQL client to a non-started server" +send "$argv sql -e 'select 1'\r" +eexpect "Error: cannot load certificates.\r\nCheck your certificate settings" +eexpect "or use --insecure" +eexpect ":/# " + +send "$argv sql -e 'select 1' --certs-dir=$certs_dir\r" +eexpect "Error: cannot dial server.\r\nIs the server running?" +eexpect "connection refused" +eexpect ":/# " +end_test + +# Check what happens when attempting to connect securely to an +# insecure server. + +send "$argv start --insecure\r" +eexpect "initialized new cluster" + +spawn /bin/bash +set client_spawn_id $spawn_id + +send "PS1=':''/# '\r" +eexpect ":/# " + +start_test "Connecting a secure RPC client to an insecure server" +send "$argv quit --certs-dir=$certs_dir\r" +eexpect "Error: cannot establish secure connection to insecure server." +eexpect "Maybe use --insecure?" +eexpect ":/# " +end_test + +start_test "Connecting a secure SQL client to an insecure server" +send "$argv sql -e 'select 1' --certs-dir=$certs_dir\r" +eexpect "Error: cannot establish secure connection to insecure server." +eexpect ":/# " +end_test + +# Check what happens when attempting to connect insecurely to a secure +# server. + +set spawn_id $shell_spawn_id +interrupt +interrupt +eexpect ":/# " + +send "$argv start --certs-dir=$certs_dir\r" +eexpect "restarted pre-existing node" + +set spawn_id $client_spawn_id + +start_test "Connecting an insecure RPC client to a secure server" +send "$argv quit --insecure\r" +eexpect "Error: server closed the connection." +eexpect "remove --insecure" +eexpect ":/# " +end_test + +start_test "Connecting an insecure SQL client to a secure server" +send "$argv sql -e 'select 1' --insecure\r" +eexpect "Error: SSL authentication error while connecting." +eexpect "remove --insecure" +eexpect ":/# " +end_test + +# Check what happens when attempting to connect to something +# that is not a CockroachDB server. +set spawn_id $shell_spawn_id +interrupt +interrupt +eexpect ":/# " + +start_test "Connecting an insecure RPC client to a non-CockroachDB server" +# In one shell, start a bogus server +send "python2 $fakeserv\r" +eexpect "ready" +# In the other shell, try to run cockroach quit +set spawn_id $client_spawn_id +send "$argv quit --insecure\r" +eexpect "insecure\r\n" +# In the first shell, stop the server. +set spawn_id $shell_spawn_id +eexpect "connected" +eexpect ":26257" +interrupt +eexpect ":/# " +# Check that cockroach quit becomes suitably confused. +set spawn_id $client_spawn_id +eexpect "Error: server closed the connection." +eexpect "Is this a CockroachDB node?" +eexpect ":/# " +set spawn_id $shell_spawn_id +end_test + +start_test "Connecting an insecure SQL client to a non-CockroachDB server" +# In one shell, start a bogus server +send "python2 $fakeserv\r" +eexpect "ready" +# In the other shell, try to run cockroach sql +set spawn_id $client_spawn_id +send "$argv sql -e 'select 1' --insecure\r" +eexpect "insecure\r\n" +# In the first shell, wait for netcat to receive garbage, +# then kill it +set spawn_id $shell_spawn_id +eexpect "connected" +eexpect "cockroach sql" +interrupt +eexpect ":/# " +# Check that cockroach sql becomes suitably confused. +set spawn_id $client_spawn_id +eexpect "Error: server closed the connection." +eexpect "Is this a CockroachDB node?" +eexpect "EOF" +eexpect ":/# " +set spawn_id $shell_spawn_id +end_test + + +# clean up the background processes +set spawn_id $shell_spawn_id +send "exit\r" +eexpect eof + +set spawn_id $client_spawn_id +send "exit\r" +eexpect eof diff --git a/pkg/cli/sql_util.go b/pkg/cli/sql_util.go index c83016f3527c..11c795232506 100644 --- a/pkg/cli/sql_util.go +++ b/pkg/cli/sql_util.go @@ -67,6 +67,29 @@ type sqlConn struct { clusterOrganization string } +// initialSQLConnectionError signals to the error decorator in +// error.go that we're failing during the initial connection set-up. +type initialSQLConnectionError struct { + err error +} + +// Error implements the error interface. +func (i *initialSQLConnectionError) Error() string { return i.err.Error() } + +// wrapConnError detects TCP EOF errors during the initial SQL handshake. +// These are translated to a message "perhaps this is not a CockroachDB node" +// at the top level. +// EOF errors later in the SQL session should not be wrapped in that way, +// because by that time we've established that the server is indeed a SQL +// server. +func wrapConnError(err error) error { + errMsg := err.Error() + if errMsg == "EOF" || errMsg == "unexpected EOF" { + return &initialSQLConnectionError{err} + } + return err +} + func (c *sqlConn) ensureConn() error { if c.conn == nil { if c.reconnecting && cliCtx.isInteractive { @@ -75,7 +98,7 @@ func (c *sqlConn) ensureConn() error { } conn, err := pq.Open(c.url) if err != nil { - return err + return wrapConnError(err) } if c.reconnecting && c.dbName != "" { // Attempt to reset the current database. @@ -88,7 +111,7 @@ func (c *sqlConn) ensureConn() error { c.conn = conn.(sqlConnI) if err := c.checkServerMetadata(); err != nil { c.Close() - return err + return wrapConnError(err) } c.reconnecting = false } diff --git a/pkg/security/certificate_loader.go b/pkg/security/certificate_loader.go index 1d501d77aca6..5ab0a9874ea9 100644 --- a/pkg/security/certificate_loader.go +++ b/pkg/security/certificate_loader.go @@ -255,11 +255,11 @@ func (cl *CertificateLoader) MaybeCreateCertsDir() error { } if !os.IsNotExist(err) { - return errors.Wrapf(err, "could not stat certs directory %s", cl.certsDir) + return makeErrorf(err, "could not stat certs directory %s", cl.certsDir) } if err := os.Mkdir(cl.certsDir, defaultCertsDirPerm); err != nil { - return errors.Wrapf(err, "could not create certs directory %s", cl.certsDir) + return makeErrorf(err, "could not create certs directory %s", cl.certsDir) } return nil } @@ -390,7 +390,7 @@ func (cl *CertificateLoader) findKey(ci *CertInfo) error { // The Error field must be nil func parseCertificate(ci *CertInfo) error { if ci.Error != nil { - return errors.Wrapf(ci.Error, "parseCertificate called on bad CertInfo object: %s", ci.Filename) + return makeErrorf(ci.Error, "parseCertificate called on bad CertInfo object: %s", ci.Filename) } if len(ci.FileContents) == 0 { @@ -400,7 +400,7 @@ func parseCertificate(ci *CertInfo) error { // PEM-decode the file. derCerts, err := PEMToCertificates(ci.FileContents) if err != nil { - return errors.Wrapf(err, "failed to parse certificate file %s as PEM", ci.Filename) + return makeErrorf(err, "failed to parse certificate file %s as PEM", ci.Filename) } // Make sure we get at least one certificate. @@ -413,11 +413,11 @@ func parseCertificate(ci *CertInfo) error { for i, c := range derCerts { x509Cert, err := x509.ParseCertificate(c.Bytes) if err != nil { - return errors.Wrapf(err, "failed to parse certificate %d in file %s", i, ci.Filename) + return makeErrorf(err, "failed to parse certificate %d in file %s", i, ci.Filename) } if err := validateCockroachCertificate(ci, x509Cert); err != nil { - return errors.Wrapf(err, "failed to validate certificate %d in file %s", i, ci.Filename) + return makeErrorf(err, "failed to validate certificate %d in file %s", i, ci.Filename) } if x509Cert.NotAfter.After(latest) { latest = x509Cert.NotAfter diff --git a/pkg/security/certificate_manager.go b/pkg/security/certificate_manager.go index f98082154a4a..6fd7fc0eb787 100644 --- a/pkg/security/certificate_manager.go +++ b/pkg/security/certificate_manager.go @@ -17,6 +17,7 @@ package security import ( "context" "crypto/tls" + "fmt" "os" "path/filepath" @@ -294,12 +295,34 @@ func (cm *CertificateManager) ClientCerts() map[string]*CertInfo { return cm.clientCerts } +// Error is the error type for this package. +type Error struct { + Message string + Err error +} + +// Error implements the error interface. +func (e *Error) Error() string { + return fmt.Sprintf("%s: %v", e.Message, e.Err) +} + +// makeErrorf constructs an Error and returns it. +func makeErrorf(err error, format string, args ...interface{}) *Error { + return &Error{ + Message: fmt.Sprintf(format, args...), + Err: err, + } +} + +// makeError constructs an Error with just a string. +func makeError(err error, s string) *Error { return makeErrorf(err, "%s", s) } + // LoadCertificates creates a CertificateLoader to load all certs and keys. // Upon success, it swaps the existing certificates for the new ones. func (cm *CertificateManager) LoadCertificates() error { cl := NewCertificateLoader(cm.certsDir) if err := cl.Load(); err != nil { - return errors.Wrapf(err, "problem loading certs directory %s", cm.certsDir) + return makeErrorf(err, "problem loading certs directory %s", cm.certsDir) } var caCert, clientCACert, uiCACert, nodeCert, uiCert, nodeClientCert *CertInfo @@ -329,22 +352,22 @@ func (cm *CertificateManager) LoadCertificates() error { if cm.initialized { // If we ran before, make sure we don't reload with missing/bad certificates. if err := checkCertIsValid(caCert); checkCertIsValid(cm.caCert) == nil && err != nil { - return errors.Wrap(err, "reload would lose valid CA cert") + return makeError(err, "reload would lose valid CA cert") } if err := checkCertIsValid(nodeCert); checkCertIsValid(cm.nodeCert) == nil && err != nil { - return errors.Wrap(err, "reload would lose valid node cert") + return makeError(err, "reload would lose valid node cert") } if err := checkCertIsValid(nodeClientCert); checkCertIsValid(cm.nodeClientCert) == nil && err != nil { - return errors.Wrapf(err, "reload would lose valid client cert for '%s'", NodeUser) + return makeErrorf(err, "reload would lose valid client cert for '%s'", NodeUser) } if err := checkCertIsValid(clientCACert); checkCertIsValid(cm.clientCACert) == nil && err != nil { - return errors.Wrap(err, "reload would lose valid CA certificate for client verification") + return makeError(err, "reload would lose valid CA certificate for client verification") } if err := checkCertIsValid(uiCACert); checkCertIsValid(cm.uiCACert) == nil && err != nil { - return errors.Wrap(err, "reload would lose valid CA certificate for UI") + return makeError(err, "reload would lose valid CA certificate for UI") } if err := checkCertIsValid(uiCert); checkCertIsValid(cm.uiCert) == nil && err != nil { - return errors.Wrap(err, "reload would lose valid UI certificate") + return makeError(err, "reload would lose valid UI certificate") } } @@ -512,7 +535,7 @@ func (cm *CertificateManager) getEmbeddedUIServerTLSConfig( // cm.mu must be held. func (cm *CertificateManager) getCACertLocked() (*CertInfo, error) { if err := checkCertIsValid(cm.caCert); err != nil { - return nil, errors.Wrap(err, "problem with CA certificate") + return nil, makeError(err, "problem with CA certificate") } return cm.caCert, nil } @@ -527,7 +550,7 @@ func (cm *CertificateManager) getClientCACertLocked() (*CertInfo, error) { } if err := checkCertIsValid(cm.clientCACert); err != nil { - return nil, errors.Wrap(err, "problem with client CA certificate") + return nil, makeError(err, "problem with client CA certificate") } return cm.clientCACert, nil } @@ -542,7 +565,7 @@ func (cm *CertificateManager) getUICACertLocked() (*CertInfo, error) { } if err := checkCertIsValid(cm.uiCACert); err != nil { - return nil, errors.Wrap(err, "problem with UI CA certificate") + return nil, makeError(err, "problem with UI CA certificate") } return cm.uiCACert, nil } @@ -551,7 +574,7 @@ func (cm *CertificateManager) getUICACertLocked() (*CertInfo, error) { // cm.mu must be held. func (cm *CertificateManager) getNodeCertLocked() (*CertInfo, error) { if err := checkCertIsValid(cm.nodeCert); err != nil { - return nil, errors.Wrap(err, "problem with node certificate") + return nil, makeError(err, "problem with node certificate") } return cm.nodeCert, nil } @@ -565,7 +588,7 @@ func (cm *CertificateManager) getUICertLocked() (*CertInfo, error) { return cm.getNodeCertLocked() } if err := checkCertIsValid(cm.uiCert); err != nil { - return nil, errors.Wrap(err, "problem with UI certificate") + return nil, makeError(err, "problem with UI certificate") } return cm.uiCert, nil } @@ -576,7 +599,7 @@ func (cm *CertificateManager) getUICertLocked() (*CertInfo, error) { func (cm *CertificateManager) getClientCertLocked(user string) (*CertInfo, error) { ci := cm.clientCerts[user] if err := checkCertIsValid(ci); err != nil { - return nil, errors.Wrapf(err, "problem with client cert for user %s", user) + return nil, makeErrorf(err, "problem with client cert for user %s", user) } return ci, nil @@ -593,7 +616,7 @@ func (cm *CertificateManager) getNodeClientCertLocked() (*CertInfo, error) { } if err := checkCertIsValid(cm.nodeClientCert); err != nil { - return nil, errors.Wrap(err, "problem with node client certificate") + return nil, makeError(err, "problem with node client certificate") } return cm.nodeClientCert, nil }