Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit6c99d5e

Browse files
authored
fix: avoid connection logging crashes in agent (#20307)
- Ignore errors when reporting a connection from the server, just logthem instead- Translate connection log IP `localhost` to `127.0.0.1` on both theserver and the agentNote that the temporary fix for converting invalid IPs to localhost isnot required in main since the database no longer forbids NULL for theIP column since#19788Relates to#20194
1 parent09e2daf commit6c99d5e

File tree

4 files changed

+45
-11
lines changed

4 files changed

+45
-11
lines changed

‎agent/agent.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -781,11 +781,15 @@ func (a *agent) reportConnectionsLoop(ctx context.Context, aAPI proto.DRPCAgentC
781781
logger.Debug(ctx,"reporting connection")
782782
_,err:=aAPI.ReportConnection(ctx,payload)
783783
iferr!=nil {
784-
returnxerrors.Errorf("failed to report connection: %w",err)
784+
// Do not fail the loop if we fail to report a connection, just
785+
// log a warning.
786+
// Related to https://github.com/coder/coder/issues/20194
787+
logger.Warn(ctx,"failed to report connection to server",slog.Error(err))
788+
// keep going, we still need to remove it from the slice
789+
}else {
790+
logger.Debug(ctx,"successfully reported connection")
785791
}
786792

787-
logger.Debug(ctx,"successfully reported connection")
788-
789793
// Remove the payload we sent.
790794
a.reportConnectionsMu.Lock()
791795
a.reportConnections[0]=nil// Release the pointer from the underlying array.
@@ -816,6 +820,13 @@ func (a *agent) reportConnection(id uuid.UUID, connectionType proto.Connection_T
816820
ip=host
817821
}
818822

823+
// If the IP is "localhost" (which it can be in some cases), set it to
824+
// 127.0.0.1 instead.
825+
// Related to https://github.com/coder/coder/issues/20194
826+
ifip=="localhost" {
827+
ip="127.0.0.1"
828+
}
829+
819830
a.reportConnectionsMu.Lock()
820831
defera.reportConnectionsMu.Unlock()
821832

‎coderd/agentapi/connectionlog.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,14 @@ func (a *ConnLogAPI) ReportConnection(ctx context.Context, req *agentproto.Repor
6161
returnnil,xerrors.Errorf("get workspace by agent id: %w",err)
6262
}
6363

64+
// Some older clients may incorrectly report "localhost" as the IP address.
65+
// Related to https://github.com/coder/coder/issues/20194
66+
logIPRaw:=req.GetConnection().GetIp()
67+
iflogIPRaw=="localhost" {
68+
logIPRaw="127.0.0.1"
69+
}
70+
logIP:=database.ParseIP(logIPRaw)// will return null if invalid
71+
6472
reason:=req.GetConnection().GetReason()
6573
connLogger:=*a.ConnectionLogger.Load()
6674
err=connLogger.Upsert(ctx, database.UpsertConnectionLogParams{
@@ -73,7 +81,7 @@ func (a *ConnLogAPI) ReportConnection(ctx context.Context, req *agentproto.Repor
7381
AgentName:workspaceAgent.Name,
7482
Type:connectionType,
7583
Code:code,
76-
Ip:database.ParseIP(req.GetConnection().GetIp()),
84+
Ip:logIP,
7785
ConnectionID: uuid.NullUUID{
7886
UUID:connectionID,
7987
Valid:true,

‎coderd/agentapi/connectionlog_test.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,11 @@ package agentapi_test
33
import (
44
"context"
55
"database/sql"
6-
"net"
76
"sync/atomic"
87
"testing"
98
"time"
109

1110
"github.com/google/uuid"
12-
"github.com/sqlc-dev/pqtype"
1311
"github.com/stretchr/testify/require"
1412
"go.uber.org/mock/gomock"
1513
"google.golang.org/protobuf/types/known/timestamppb"
@@ -75,6 +73,9 @@ func TestConnectionLog(t *testing.T) {
7573
action:agentproto.Connection_CONNECT.Enum(),
7674
typ:agentproto.Connection_JETBRAINS.Enum(),
7775
time:dbtime.Now(),
76+
// Sometimes, JetBrains clients report as localhost, see
77+
// https://github.com/coder/coder/issues/20194
78+
ip:"localhost",
7879
},
7980
{
8081
name:"Reconnecting PTY Connect",
@@ -129,6 +130,12 @@ func TestConnectionLog(t *testing.T) {
129130
},
130131
})
131132

133+
expectedIPRaw:=tt.ip
134+
ifexpectedIPRaw=="localhost" {
135+
expectedIPRaw="127.0.0.1"
136+
}
137+
expectedIP:=database.ParseIP(expectedIPRaw)
138+
132139
require.True(t,connLogger.Contains(t, database.UpsertConnectionLogParams{
133140
Time:dbtime.Time(tt.time).In(time.UTC),
134141
OrganizationID:workspace.OrganizationID,
@@ -146,7 +153,7 @@ func TestConnectionLog(t *testing.T) {
146153
Int32:tt.status,
147154
Valid:*tt.action==agentproto.Connection_DISCONNECT,
148155
},
149-
Ip:pqtype.Inet{Valid:true,IPNet: net.IPNet{IP:net.ParseIP(tt.ip),Mask:net.CIDRMask(32,32)}},
156+
Ip:expectedIP,
150157
Type:agentProtoConnectionTypeToConnectionLog(t,*tt.typ),
151158
DisconnectReason: sql.NullString{
152159
String:tt.reason,

‎coderd/connectionlog/connectionlog.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,6 @@ func (m *FakeConnectionLogger) Contains(t testing.TB, expected database.UpsertCo
6262
t.Logf("connection log %d: expected ID %s, got %s",idx+1,expected.ID,cl.ID)
6363
continue
6464
}
65-
if!expected.Time.IsZero()&&expected.Time!=cl.Time {
66-
t.Logf("connection log %d: expected Time %s, got %s",idx+1,expected.Time,cl.Time)
67-
continue
68-
}
6965
ifexpected.OrganizationID!=uuid.Nil&&cl.OrganizationID!=expected.OrganizationID {
7066
t.Logf("connection log %d: expected OrganizationID %s, got %s",idx+1,expected.OrganizationID,cl.OrganizationID)
7167
continue
@@ -114,6 +110,18 @@ func (m *FakeConnectionLogger) Contains(t testing.TB, expected database.UpsertCo
114110
t.Logf("connection log %d: expected ConnectionID %s, got %s",idx+1,expected.ConnectionID.UUID,cl.ConnectionID.UUID)
115111
continue
116112
}
113+
ifexpected.DisconnectReason.Valid&&cl.DisconnectReason.String!=expected.DisconnectReason.String {
114+
t.Logf("connection log %d: expected DisconnectReason %s, got %s",idx+1,expected.DisconnectReason.String,cl.DisconnectReason.String)
115+
continue
116+
}
117+
if!expected.Time.IsZero()&&expected.Time!=cl.Time {
118+
t.Logf("connection log %d: expected Time %s, got %s",idx+1,expected.Time,cl.Time)
119+
continue
120+
}
121+
ifexpected.ConnectionStatus!=""&&expected.ConnectionStatus!=cl.ConnectionStatus {
122+
t.Logf("connection log %d: expected ConnectionStatus %s, got %s",idx+1,expected.ConnectionStatus,cl.ConnectionStatus)
123+
continue
124+
}
117125
returntrue
118126
}
119127

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp