Revisit #2269 to (#2602)

implement conditional stack traces for HTTP APIs.
The original implementation removes the stack traces from HTTP APIs
errors unconditionally which results in reduced troubleshooting efficacy
when the errors contain important details about a failure from the CLI
client.
In order to counter this and provide useful information when necessary,
the stack traces are only stripped if the request does not contain a
custom marker identifying the gravity client.

Updates #2269.

lib/app/handler/handler.go

@@ -967,7 +967,7 @@ func (h *WebHandler) wrap(fn func(w http.ResponseWriter, r *http.Request, p http
  return func(w http.ResponseWriter, r *http.Request, p httprouter.Params) {
  if err := fn(w, r, p); err != nil {
  log.WithFields(fields.FromRequest(r)).WithError(err).Info("Handler error.")
- trace.WriteError(w, trace.Unwrap(err))
+ httplib.WriteError(w, err, r.Header)
  }
  }
 }
@@ -979,7 +979,7 @@ func (h *WebHandler) needsAuth(fn serviceHandler) httprouter.Handle {
  authResult, err := h.Authenticator.Authenticate(w, r)
  if err != nil {
  logger.WithError(err).Warn("Authentication error.")
- trace.WriteError(w, trace.Unwrap(trace.AccessDenied("bad username or password"))) // Hide the actual error.
+ httplib.WriteError(w, trace.AccessDenied("bad username or password"), r.Header) // Hide the actual error.
  return
  }
 
@@ -995,7 +995,7 @@ func (h *WebHandler) needsAuth(fn serviceHandler) httprouter.Handle {
  } else {
  logger.WithError(err).Debug("Handler error.")
  }
- trace.WriteError(w, trace.Unwrap(err))
+ httplib.WriteError(w, err, r.Header)
  }
  }
 }

lib/blob/handler/blobhandler.go

@@ -93,7 +93,7 @@ func (s *Server) notFound(w http.ResponseWriter, r *http.Request) {
  "method": r.Method,
  "url": r.URL.String(),
  }).Warn("Invalid request.")
- trace.WriteError(w, trace.Unwrap(err))
+ httplib.WriteError(w, err, r.Header)
 }
 
 func (s *Server) getBLOBs(w http.ResponseWriter, r *http.Request, p httprouter.Params, objects blob.Objects) error {
@@ -177,16 +177,15 @@ func (s *Server) needsAuth(fn authHandle, objects blob.Objects) httprouter.Handl
  authCreds, err := httplib.ParseAuthHeaders(r)
  if err != nil {
  log.WithError(err).Warn("Invalid auth headers.")
- trace.WriteError(w, trace.Unwrap(err))
+ httplib.WriteError(w, err, r.Header)
  return
  }
 
  user, checker, err := s.cfg.Users.AuthenticateUser(*authCreds)
  if err != nil {
  log.WithError(err).Info("Authentication error.")
  // we hide the error from the remote user to avoid giving any hints
- trace.WriteError(
- w, trace.Unwrap(trace.AccessDenied("bad username or password")))
+ httplib.WriteError(w, trace.AccessDenied("bad username or password"), r.Header)
  return
  }
 
@@ -195,7 +194,7 @@ func (s *Server) needsAuth(fn authHandle, objects blob.Objects) httprouter.Handl
  if !trace.IsNotFound(err) && !trace.IsAlreadyExists(err) {
  log.Errorf("handler error: %v", trace.DebugReport(err))
  }
- trace.WriteError(w, trace.Unwrap(err))
+ httplib.WriteError(w, err, r.Header)
  }
  }
 }

lib/httplib/client.go

@@ -92,20 +92,20 @@ func GetRemoteClient(remoteSite rt.RemoteSite, remoteURL *url.URL) *http.Client
 }
 
 // ClientOption sets custom HTTP client option
-type ClientOption func(*http.Client)
+type ClientOption func(*httpClient)
 
 // WithLocalResolver sets up client to use local DNS resolver
 func WithLocalResolver(dnsAddr string) ClientOption {
- return func(c *http.Client) {
- c.Transport.(*http.Transport).DialContext = DialFromEnviron(dnsAddr)
+ return func(c *httpClient) {
+ c.transport.DialContext = DialFromEnviron(dnsAddr)
  }
 }
 
 // WithInsecure sets insecure TLS config
 func WithInsecure() ClientOption {
- return func(c *http.Client) {
+ return func(c *httpClient) {
  // Make sure not to override existing TLS configuration.
- tlsConfig := c.Transport.(*http.Transport).TLSClientConfig
+ tlsConfig := c.transport.TLSClientConfig
  if tlsConfig == nil {
  //nolint:gosec // TODO: set MinVersion
  tlsConfig = &tls.Config{}
@@ -116,14 +116,14 @@ func WithInsecure() ClientOption {
 
 // WithTLSClientConfig sets TLS client configuration.
 func WithTLSClientConfig(tlsConfig *tls.Config) ClientOption {
- return func(c *http.Client) {
- c.Transport.(*http.Transport).TLSClientConfig = tlsConfig
+ return func(c *httpClient) {
+ c.transport.TLSClientConfig = tlsConfig
  // Note, GetClientCertificate is required to enforce the client to
  // always send the certificate along, otherwise it may choose not
  // send it in specific cases. Source:
  // https://github.com/golang/go/issues/23924#issuecomment-367472052
  if len(tlsConfig.Certificates) != 0 {
- c.Transport.(*http.Transport).TLSClientConfig.GetClientCertificate = func(_ *tls.CertificateRequestInfo) (*tls.Certificate, error) {
+ c.transport.TLSClientConfig.GetClientCertificate = func(_ *tls.CertificateRequestInfo) (*tls.Certificate, error) {
  return &tlsConfig.Certificates[0], nil
  }
  }
@@ -132,43 +132,40 @@ func WithTLSClientConfig(tlsConfig *tls.Config) ClientOption {
 
 // WithTimeout sets timeout
 func WithTimeout(t time.Duration) ClientOption {
- return func(c *http.Client) {
- c.Timeout = t
+ return func(c *httpClient) {
+ c.timeout = t
  }
 }
 
 // WithDialTimeout sets dial timeout
 func WithDialTimeout(t time.Duration) ClientOption {
- return func(c *http.Client) {
- c.Transport.(*http.Transport).DialContext = (&net.Dialer{Timeout: t}).DialContext
+ return func(c *httpClient) {
+ c.transport.DialContext = (&net.Dialer{Timeout: t}).DialContext
  }
 }
 
 // WithClientCert sets a certificate for mTLS client authentication
 func WithClientCert(cert tls.Certificate) ClientOption {
- return func(c *http.Client) {
- transport := c.Transport.(*http.Transport)
- transport.TLSClientConfig.Certificates = append(transport.TLSClientConfig.Certificates, cert)
+ return func(c *httpClient) {
+ c.transport.TLSClientConfig.Certificates = append(c.transport.TLSClientConfig.Certificates, cert)
  }
 }
 
 // WithCA to use a custom certificate authority for server validation
 func WithCA(cert []byte) ClientOption {
- return func(c *http.Client) {
- transport := c.Transport.(*http.Transport)
- if transport.TLSClientConfig.RootCAs == nil {
- transport.TLSClientConfig.RootCAs = x509.NewCertPool()
+ return func(c *httpClient) {
+ if c.transport.TLSClientConfig.RootCAs == nil {
+ c.transport.TLSClientConfig.RootCAs = x509.NewCertPool()
  }
 
- transport.TLSClientConfig.RootCAs.AppendCertsFromPEM(cert)
+ c.transport.TLSClientConfig.RootCAs.AppendCertsFromPEM(cert)
  }
 }
 
 // WithIdleConnTimeout overrides the transport connection idle timeout
 func WithIdleConnTimeout(timeout time.Duration) ClientOption {
- return func(c *http.Client) {
- transport := c.Transport.(*http.Transport)
- transport.IdleConnTimeout = timeout
+ return func(c *httpClient) {
+ c.transport.IdleConnTimeout = timeout
  }
 }
 
@@ -188,17 +185,39 @@ func NewClient(options ...ClientOption) *http.Client {
  TLSClientConfig: &tls.Config{},
  DialContext: (&net.Dialer{Timeout: defaults.DialTimeout}).DialContext,
  }
- client := &http.Client{Transport: transport}
- for _, o := range options {
- o(client)
- }
  if transport.IdleConnTimeout == 0 {
  transport.IdleConnTimeout = defaults.ConnectionIdleTimeout
  }
  if transport.MaxIdleConnsPerHost == 0 {
  transport.MaxIdleConnsPerHost = defaults.MaxIdleConnsPerHost
  }
- return client
+ client := &httpClient{transport: transport}
+ for _, o := range options {
+ o(client)
+ }
+ return &http.Client{
+ Transport: &roundTripper{tr: client.transport},
+ Timeout: client.timeout,
+ }
+}
+
+// httpClient is a configuration stub to collect external configuration
+// for the HTTP client before the actual value is built
+type httpClient struct {
+ transport *http.Transport
+ // See http.Client.Timeout
+ timeout time.Duration
+}
+
+// RoundTrip sets a custom client header before delegating to the underlying round tripper
+func (r *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+ req.Header.Set(clientIDHeader, gravityCLIClientID)
+ return r.tr.RoundTrip(req)
+}
+
+// roundTripper is an http.RoundTripper to set custom headers
+type roundTripper struct {
+ tr *http.Transport
 }
 
 func GetPlanetClient(options ...ClientOption) (*http.Client, error) {

lib/httplib/http.go

@@ -205,3 +205,19 @@ var Methods = []string{
  http.MethodPatch,
  http.MethodHead,
 }
+
+// WriteError serializes the given error into the provided response writer.
+// The amount of serialized information depends on the HTTP headers.
+func WriteError(w http.ResponseWriter, err error, hdr http.Header) {
+ if hdr.Get(clientIDHeader) != gravityCLIClientID {
+ // Remove stack traces if this is not a gravity client
+ trace.WriteError(w, trace.Unwrap(err))
+ return
+ }
+ trace.WriteError(w, err)
+}
+
+const (
+ clientIDHeader = "X-Gravity-Client-ID"
+ gravityCLIClientID = "gravity"
+)

lib/install/fsmspec.go

@@ -119,10 +119,6 @@ func DefaultFSMSpec(config FSMConfig) fsm.FSMSpecFunc {
  config.Operator,
  config.OperationKey)
 
- case phases.ConfigurePhase:
- return phases.NewConfigure(p,
- config.Operator)
-
  case phases.WaitPhase:
  client, err := getKubeClient(p)
  if err != nil {

lib/ops/opshandler/opshandler.go

@@ -359,7 +359,7 @@ func (h *WebHandler) getSiteInstructions(w http.ResponseWriter, r *http.Request,
  instructions, err := h.cfg.Operator.GetSiteInstructions(token, serverProfile, r.URL.Query())
  if err != nil {
  log.WithError(err).Warn("Failed to query cluster install instructions.")
- trace.WriteError(w, trace.Unwrap(err))
+ httplib.WriteError(w, err, r.Header)
  return
  }
  w.Header().Set("Content-Type", "text/plain")
@@ -2441,7 +2441,7 @@ func NeedsAuth(devmode bool, backend storage.Backend, operator ops.Operator, aut
  if trace.IsAccessDenied(err) {
  log.WithFields(fields.FromRequest(r)).WithError(err).Warn("Access denied.")
  }
- trace.WriteError(w, trace.Unwrap(err))
+ httplib.WriteError(w, err, r.Header)
  }
  }
 }

lib/pack/webpack/webpack.go

@@ -25,6 +25,7 @@ import (
  "strconv"
  "time"
 
+ "github.com/gravitational/gravity/lib/httplib"
  "github.com/gravitational/gravity/lib/loc"
  "github.com/gravitational/gravity/lib/pack"
  "github.com/gravitational/gravity/lib/storage"
@@ -320,7 +321,7 @@ func (s *Server) needsAuth(fn authHandle) httprouter.Handle {
  authResult, err := s.cfg.Authenticator.Authenticate(w, r)
  if err != nil {
  logger.WithError(err).Warn("Authentication error.")
- trace.WriteError(w, trace.Unwrap(trace.AccessDenied("bad username or password"))) // Hide the actual error.
+ httplib.WriteError(w, trace.AccessDenied("bad username or password"), r.Header) // Hide the actual error.
  return
  }
 
@@ -334,7 +335,7 @@ func (s *Server) needsAuth(fn authHandle) httprouter.Handle {
  } else if !trace.IsNotFound(err) && !trace.IsAlreadyExists(err) {
  logger.WithError(err).Error("Handler error.")
  }
- trace.WriteError(w, trace.Unwrap(err))
+ httplib.WriteError(w, err, r.Header)
  }
  }
 }