Skip to content

Commit

Permalink
Improve error messaging when a connections to nodes fail (#47361)
Browse files Browse the repository at this point in the history
Prevents duplicate information about missing tunnels preventing a
connection when attempting to connect locally and via a proxy peer.
The error from the remote dial is now omitted from the message
provided to end users. The information about how the connection
was attempted was instead updated to reflect that tunnels were
attempted via both the local proxy and a peer and they both
failed. Additionally, if a hostname was provided in the dial
request, it will be included in the error message instead of
the internal @local-node identifier for tunnels or the public
address for direct dial nodes.

Closes #45256.
  • Loading branch information
rosstimothy authored Oct 9, 2024
1 parent a9c6dba commit 3ddcff1
Showing 1 changed file with 19 additions and 5 deletions.
24 changes: 19 additions & 5 deletions lib/reversetunnel/localsite.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,11 @@ with the cluster.`
toAddr = params.To.String()
}

// Prefer providing the hostname over an address.
if params.TargetServer != nil {
toAddr = params.TargetServer.GetHostname()
}

return fmt.Sprintf(errorMessageTemplate, params.ConnType, toAddr, connStr, err)
}

Expand Down Expand Up @@ -613,7 +618,8 @@ func (s *localSite) getConn(params reversetunnelclient.DialParams) (conn net.Con
return newMetricConn(conn, dt, dialStart, s.srv.Clock), true, nil
}

if s.tryProxyPeering(params) {
peeringEnabled := s.tryProxyPeering(params)
if peeringEnabled {
s.log.Info("Dialing over peer proxy")
conn, peerErr = s.peerClient.DialNode(
params.ProxyIDs, params.ServerID, params.From, params.To, params.ConnType,
Expand All @@ -623,20 +629,28 @@ func (s *localSite) getConn(params reversetunnelclient.DialParams) (conn net.Con
}
}

err = trace.NewAggregate(tunnelErr, peerErr)
tunnelMsg := getTunnelErrorMessage(params, "reverse tunnel", err)
// If a connection via tunnel failed directly and via a remote peer,
// then update the tunnel message to indicate that tunnels were not
// found in either place. Avoid aggregating the local and peer errors
// to reduce duplicate data since this message makes its way back to
// users and can be confusing.
msg := "reverse tunnel"
if peeringEnabled {
msg = "local and peer reverse tunnels"
}
tunnelMsg := getTunnelErrorMessage(params, msg, tunnelErr)

// Skip direct dial when the tunnel error is not a not found error. This
// means the agent is tunneling but the connection failed for some reason.
if !trace.IsNotFound(tunnelErr) {
return nil, false, trace.ConnectionProblem(err, tunnelMsg)
return nil, false, trace.ConnectionProblem(tunnelErr, tunnelMsg)
}

skip, err := s.skipDirectDial(params)
if err != nil {
return nil, false, trace.Wrap(err)
} else if skip {
return nil, false, trace.ConnectionProblem(err, tunnelMsg)
return nil, false, trace.ConnectionProblem(tunnelErr, tunnelMsg)
}

// If no tunnel connection was found, dial to the target host.
Expand Down

0 comments on commit 3ddcff1

Please sign in to comment.