From 3a8a174308d857b15b262f5904ae2ec1d8b9c867 Mon Sep 17 00:00:00 2001 From: Jordan Whited Date: Thu, 17 Apr 2025 16:21:32 -0700 Subject: [PATCH] net/udprelay: change ServerEndpoint time.Duration fields to tstime.GoDuration (#15725) tstime.GoDuration JSON serializes with time.Duration.String(), which is more human-friendly than nanoseconds. ServerEndpoint is currently experimental, therefore breaking changes are tolerable. Updates tailscale/corp#27502 Signed-off-by: Jordan Whited --- net/udprelay/server.go | 13 ++--- net/udprelay/server_test.go | 96 +++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 6 deletions(-) diff --git a/net/udprelay/server.go b/net/udprelay/server.go index 30fc08326..373165777 100644 --- a/net/udprelay/server.go +++ b/net/udprelay/server.go @@ -21,6 +21,7 @@ import ( "go4.org/mem" "tailscale.com/disco" "tailscale.com/net/packet" + "tailscale.com/tstime" "tailscale.com/types/key" ) @@ -114,12 +115,12 @@ type ServerEndpoint struct { // BindLifetime is amount of time post-allocation the Server will consider // the endpoint active while it has yet to be bound via 3-way bind handshake // from both client parties. - BindLifetime time.Duration + BindLifetime tstime.GoDuration // SteadyStateLifetime is the amount of time post 3-way bind handshake from // both client parties the Server will consider the endpoint active lacking // bidirectional data flow. - SteadyStateLifetime time.Duration + SteadyStateLifetime tstime.GoDuration } // serverEndpoint contains Server-internal ServerEndpoint state. serverEndpoint @@ -489,8 +490,8 @@ func (s *Server) AllocateEndpoint(discoA, discoB key.DiscoPublic) (ServerEndpoin AddrPorts: s.addrPorts, VNI: e.vni, LamportID: e.lamportID, - BindLifetime: s.bindLifetime, - SteadyStateLifetime: s.steadyStateLifetime, + BindLifetime: tstime.GoDuration{Duration: s.bindLifetime}, + SteadyStateLifetime: tstime.GoDuration{Duration: s.steadyStateLifetime}, }, nil } // If an endpoint exists for the pair of key.DiscoPublic's, and is @@ -526,7 +527,7 @@ func (s *Server) AllocateEndpoint(discoA, discoB key.DiscoPublic) (ServerEndpoin AddrPorts: s.addrPorts, VNI: e.vni, LamportID: e.lamportID, - BindLifetime: s.bindLifetime, - SteadyStateLifetime: s.steadyStateLifetime, + BindLifetime: tstime.GoDuration{Duration: s.bindLifetime}, + SteadyStateLifetime: tstime.GoDuration{Duration: s.steadyStateLifetime}, }, nil } diff --git a/net/udprelay/server_test.go b/net/udprelay/server_test.go index 733e50b77..fad35ec03 100644 --- a/net/udprelay/server_test.go +++ b/net/udprelay/server_test.go @@ -5,6 +5,8 @@ package udprelay import ( "bytes" + "encoding/json" + "math" "net" "net/netip" "testing" @@ -15,6 +17,7 @@ import ( "go4.org/mem" "tailscale.com/disco" "tailscale.com/net/packet" + "tailscale.com/tstime" "tailscale.com/types/key" ) @@ -202,3 +205,96 @@ func TestServer(t *testing.T) { t.Fatal("unexpected msg B->A") } } + +func TestServerEndpointJSONUnmarshal(t *testing.T) { + tests := []struct { + name string + json []byte + wantErr bool + }{ + { + name: "valid", + json: []byte(`{"ServerDisco":"discokey:003cd7453e04a653eb0e7a18f206fc353180efadb2facfd05ebd6982a1392c7f","LamportID":18446744073709551615,"AddrPorts":["127.0.0.1:1","127.0.0.2:2"],"VNI":16777215,"BindLifetime":"30s","SteadyStateLifetime":"5m0s"}`), + wantErr: false, + }, + { + name: "invalid ServerDisco", + json: []byte(`{"ServerDisco":"1","LamportID":18446744073709551615,"AddrPorts":["127.0.0.1:1","127.0.0.2:2"],"VNI":16777215,"BindLifetime":"30s","SteadyStateLifetime":"5m0s"}`), + wantErr: true, + }, + { + name: "invalid LamportID", + json: []byte(`{"ServerDisco":"discokey:003cd7453e04a653eb0e7a18f206fc353180efadb2facfd05ebd6982a1392c7f","LamportID":1.1,"AddrPorts":["127.0.0.1:1","127.0.0.2:2"],"VNI":16777215,"BindLifetime":"30s","SteadyStateLifetime":"5m0s"}`), + wantErr: true, + }, + { + name: "invalid AddrPorts", + json: []byte(`{"ServerDisco":"discokey:003cd7453e04a653eb0e7a18f206fc353180efadb2facfd05ebd6982a1392c7f","LamportID":18446744073709551615,"AddrPorts":["127.0.0.1.1:1","127.0.0.2:2"],"VNI":16777215,"BindLifetime":"30s","SteadyStateLifetime":"5m0s"}`), + wantErr: true, + }, + { + name: "invalid VNI", + json: []byte(`{"ServerDisco":"discokey:003cd7453e04a653eb0e7a18f206fc353180efadb2facfd05ebd6982a1392c7f","LamportID":18446744073709551615,"AddrPorts":["127.0.0.1:1","127.0.0.2:2"],"VNI":18446744073709551615,"BindLifetime":"30s","SteadyStateLifetime":"5m0s"}`), + wantErr: true, + }, + { + name: "invalid BindLifetime", + json: []byte(`{"ServerDisco":"discokey:003cd7453e04a653eb0e7a18f206fc353180efadb2facfd05ebd6982a1392c7f","LamportID":18446744073709551615,"AddrPorts":["127.0.0.1:1","127.0.0.2:2"],"VNI":16777215,"BindLifetime":"5","SteadyStateLifetime":"5m0s"}`), + wantErr: true, + }, + { + name: "invalid SteadyStateLifetime", + json: []byte(`{"ServerDisco":"discokey:003cd7453e04a653eb0e7a18f206fc353180efadb2facfd05ebd6982a1392c7f","LamportID":18446744073709551615,"AddrPorts":["127.0.0.1:1","127.0.0.2:2"],"VNI":16777215,"BindLifetime":"30s","SteadyStateLifetime":"5"}`), + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var out ServerEndpoint + err := json.Unmarshal(tt.json, &out) + if tt.wantErr != (err != nil) { + t.Fatalf("wantErr: %v (err == nil): %v", tt.wantErr, err == nil) + } + if tt.wantErr { + return + } + }) + } +} + +func TestServerEndpointJSONMarshal(t *testing.T) { + tests := []struct { + name string + serverEndpoint ServerEndpoint + }{ + { + name: "valid roundtrip", + serverEndpoint: ServerEndpoint{ + ServerDisco: key.NewDisco().Public(), + LamportID: uint64(math.MaxUint64), + AddrPorts: []netip.AddrPort{netip.MustParseAddrPort("127.0.0.1:1"), netip.MustParseAddrPort("127.0.0.2:2")}, + VNI: 1<<24 - 1, + BindLifetime: tstime.GoDuration{Duration: defaultBindLifetime}, + SteadyStateLifetime: tstime.GoDuration{Duration: defaultSteadyStateLifetime}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b, err := json.Marshal(&tt.serverEndpoint) + if err != nil { + t.Fatal(err) + } + var got ServerEndpoint + err = json.Unmarshal(b, &got) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(got, tt.serverEndpoint, cmpopts.EquateComparable(netip.AddrPort{}, key.DiscoPublic{})); diff != "" { + t.Fatalf("ServerEndpoint unequal (-got +want)\n%s", diff) + } + }) + } +}