fix: avoid connection logging crashes in agent [2.26] (coder#20306)

# For release 2.26

- Ignore errors when reporting a connection from the server, just log
them instead
- Translate connection log IP `localhost` to `127.0.0.1` on both the
server and the agent
- Temporary fix: convert invalid IPs to `127.0.0.1` since the database
forbids NULL

Relates to coder#20194

Author: deansheather

agent/agent.go

@@ -790,11 +790,15 @@ func (a *agent) reportConnectionsLoop(ctx context.Context, aAPI proto.DRPCAgentC
 			logger.Debug(ctx, "reporting connection")
 			_, err := aAPI.ReportConnection(ctx, payload)
 			if err != nil {
-				return xerrors.Errorf("failed to report connection: %w", err)
+				// Do not fail the loop if we fail to report a connection, just
+				// log a warning.
+				// Related to https://github.com/coder/coder/issues/20194
+				logger.Warn(ctx, "failed to report connection to server", slog.Error(err))
+				// no continue here, we still need to remove it from the slice
+			} else {
+				logger.Debug(ctx, "successfully reported connection")
 			}
 
-			logger.Debug(ctx, "successfully reported connection")
-
 			// Remove the payload we sent.
 			a.reportConnectionsMu.Lock()
 			a.reportConnections[0] = nil // Release the pointer from the underlying array.
@@ -825,6 +829,13 @@ func (a *agent) reportConnection(id uuid.UUID, connectionType proto.Connection_T
 		ip = host
 	}
 
+	// If the IP is "localhost" (which it can be in some cases), set it to
+	// 127.0.0.1 instead.
+	// Related to https://github.com/coder/coder/issues/20194
+	if ip == "localhost" {
+		ip = "127.0.0.1"
+	}
+
 	a.reportConnectionsMu.Lock()
 	defer a.reportConnectionsMu.Unlock()
 

coderd/agentapi/connectionlog.go

@@ -3,9 +3,11 @@ package agentapi
 import (
 	"context"
 	"database/sql"
+	"net"
 	"sync/atomic"
 
 	"github.com/google/uuid"
+	"github.com/sqlc-dev/pqtype"
 	"golang.org/x/xerrors"
 	"google.golang.org/protobuf/types/known/emptypb"
 
@@ -61,6 +63,27 @@ func (a *ConnLogAPI) ReportConnection(ctx context.Context, req *agentproto.Repor
 		return nil, xerrors.Errorf("get workspace by agent id: %w", err)
 	}
 
+	// Some older clients may incorrectly report "localhost" as the IP address.
+	// Related to https://github.com/coder/coder/issues/20194
+	logIPRaw := req.GetConnection().GetIp()
+	if logIPRaw == "localhost" {
+		logIPRaw = "127.0.0.1"
+	}
+
+	// TEMPORARY FIX for https://github.com/coder/coder/issues/20194
+	logIP := database.ParseIP(logIPRaw)
+	if !logIP.Valid {
+		// In older versions of Coder, NULL IPs are not permitted in the DB, so
+		// use 127.0.0.1 instead.
+		logIP = pqtype.Inet{
+			IPNet: net.IPNet{
+				IP:   net.IPv4(127, 0, 0, 1),
+				Mask: net.CIDRMask(32, 32),
+			},
+			Valid: true,
+		}
+	}
+
 	reason := req.GetConnection().GetReason()
 	connLogger := *a.ConnectionLogger.Load()
 	err = connLogger.Upsert(ctx, database.UpsertConnectionLogParams{
@@ -73,7 +96,7 @@ func (a *ConnLogAPI) ReportConnection(ctx context.Context, req *agentproto.Repor
 		AgentName:        workspaceAgent.Name,
 		Type:             connectionType,
 		Code:             code,
-		Ip:               database.ParseIP(req.GetConnection().GetIp()),
+		Ip:               logIP,
 		ConnectionID: uuid.NullUUID{
 			UUID:  connectionID,
 			Valid: true,

coderd/agentapi/connectionlog_test.go

@@ -110,6 +110,11 @@ func TestConnectionLog(t *testing.T) {
 			mDB := dbmock.NewMockStore(gomock.NewController(t))
 			mDB.EXPECT().GetWorkspaceByAgentID(gomock.Any(), agent.ID).Return(workspace, nil)
 
+			// TEMPORARY FIX for https://github.com/coder/coder/issues/20194
+			if tt.ip == "" {
+				tt.ip = "127.0.0.1"
+			}
+
 			api := &agentapi.ConnLogAPI{
 				ConnectionLogger: asAtomicPointer[connectionlog.ConnectionLogger](connLogger),
 				Database:         mDB,