diff --git a/cmd/tailscale/cli/serve_legacy_test.go b/cmd/tailscale/cli/serve_legacy_test.go index 58139ecc2..5b70e6748 100644 --- a/cmd/tailscale/cli/serve_legacy_test.go +++ b/cmd/tailscale/cli/serve_legacy_test.go @@ -21,6 +21,7 @@ import ( "tailscale.com/ipn" "tailscale.com/ipn/ipnstate" "tailscale.com/tailcfg" + "tailscale.com/tstest" "tailscale.com/types/logger" ) @@ -807,9 +808,11 @@ func TestVerifyFunnelEnabled(t *testing.T) { lc.setQueryFeatureResponse(tt.queryFeatureResponse) if tt.caps != nil { - oldCaps := fakeStatus.Self.Capabilities - defer func() { fakeStatus.Self.Capabilities = oldCaps }() // reset after test - fakeStatus.Self.Capabilities = tt.caps + cm := make(tailcfg.NodeCapMap) + for _, c := range tt.caps { + cm[c] = nil + } + tstest.Replace(t, &fakeStatus.Self.CapMap, cm) } defer func() { @@ -853,8 +856,11 @@ type fakeLocalServeClient struct { var fakeStatus = &ipnstate.Status{ BackendState: ipn.Running.String(), Self: &ipnstate.PeerStatus{ - DNSName: "foo.test.ts.net", - Capabilities: []tailcfg.NodeCapability{tailcfg.NodeAttrFunnel, tailcfg.CapabilityFunnelPorts + "?ports=443,8443"}, + DNSName: "foo.test.ts.net", + CapMap: tailcfg.NodeCapMap{ + tailcfg.NodeAttrFunnel: nil, + tailcfg.CapabilityFunnelPorts + "?ports=443,8443": nil, + }, }, } diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 482779b36..4f19e9753 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -172,7 +172,15 @@ func (ms *mapSession) HandleNonKeepAliveMapResponse(ctx context.Context, resp *t resp.Node.Capabilities = nil resp.Node.CapMap = nil } - ms.controlKnobs.UpdateFromNodeAttributes(resp.Node.Capabilities, resp.Node.CapMap) + // If the server is old and is still sending us Capabilities instead of + // CapMap, convert it to CapMap early so the rest of the client code can + // work only in terms of CapMap. + for _, c := range resp.Node.Capabilities { + if _, ok := resp.Node.CapMap[c]; !ok { + mak.Set(&resp.Node.CapMap, c, nil) + } + } + ms.controlKnobs.UpdateFromNodeAttributes(resp.Node.CapMap) } // Call Node.InitDisplayNames on any changed nodes. @@ -354,7 +362,6 @@ var ( patchOnline = clientmetric.NewCounter("controlclient_patch_online") patchLastSeen = clientmetric.NewCounter("controlclient_patch_lastseen") patchKeyExpiry = clientmetric.NewCounter("controlclient_patch_keyexpiry") - patchCapabilities = clientmetric.NewCounter("controlclient_patch_capabilities") patchCapMap = clientmetric.NewCounter("controlclient_patch_capmap") patchKeySignature = clientmetric.NewCounter("controlclient_patch_keysig") @@ -476,10 +483,6 @@ func (ms *mapSession) updatePeersStateFromResponse(resp *tailcfg.MapResponse) (s mut.KeyExpiry = *v patchKeyExpiry.Add(1) } - if v := pc.Capabilities; v != nil { - mut.Capabilities = *v - patchCapabilities.Add(1) - } if v := pc.KeySignature; v != nil { mut.KeySignature = v patchKeySignature.Add(1) @@ -601,6 +604,9 @@ func peerChangeDiff(was tailcfg.NodeView, n *tailcfg.Node) (_ *tailcfg.PeerChang continue case "DataPlaneAuditLogID": // Not sent for peers. + case "Capabilities": + // Deprecated; see https://github.com/tailscale/tailscale/issues/11508 + // And it was never sent by any known control server. case "ID": if was.ID() != n.ID { return nil, false @@ -722,10 +728,6 @@ func peerChangeDiff(was tailcfg.NodeView, n *tailcfg.Node) (_ *tailcfg.PeerChang if was.MachineAuthorized() != n.MachineAuthorized { return nil, false } - case "Capabilities": - if !views.SliceEqual(was.Capabilities(), views.SliceOf(n.Capabilities)) { - pc().Capabilities = ptr.To(n.Capabilities) - } case "UnsignedPeerAPIOnly": if was.UnsignedPeerAPIOnly() != n.UnsignedPeerAPIOnly { return nil, false diff --git a/control/controlclient/map_test.go b/control/controlclient/map_test.go index c3d05b44a..4a9002e17 100644 --- a/control/controlclient/map_test.go +++ b/control/controlclient/map_test.go @@ -331,23 +331,7 @@ func TestUpdatePeersStateFromResponse(t *testing.T) { }), wantStats: updateStats{changed: 1}, }, - { - name: "change_capabilities", - prev: peers(n(1, "foo")), - mapRes: &tailcfg.MapResponse{ - PeersChangedPatch: []*tailcfg.PeerChange{{ - NodeID: 1, - Capabilities: ptr.To([]tailcfg.NodeCapability{"foo"}), - }}, - }, - want: peers(&tailcfg.Node{ - ID: 1, - Name: "foo", - Capabilities: []tailcfg.NodeCapability{"foo"}, - }), - wantStats: updateStats{changed: 1}, - }} - + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if !tt.curTime.IsZero() { @@ -783,18 +767,6 @@ func TestPeerChangeDiff(t *testing.T) { b: &tailcfg.Node{ID: 1, LastSeen: ptr.To(time.Unix(2, 0))}, want: &tailcfg.PeerChange{NodeID: 1, LastSeen: ptr.To(time.Unix(2, 0))}, }, - { - name: "patch-capabilities-to-nonempty", - a: &tailcfg.Node{ID: 1, Capabilities: []tailcfg.NodeCapability{"foo"}}, - b: &tailcfg.Node{ID: 1, Capabilities: []tailcfg.NodeCapability{"bar"}}, - want: &tailcfg.PeerChange{NodeID: 1, Capabilities: ptr.To([]tailcfg.NodeCapability{"bar"})}, - }, - { - name: "patch-capabilities-to-empty", - a: &tailcfg.Node{ID: 1, Capabilities: []tailcfg.NodeCapability{"foo"}}, - b: &tailcfg.Node{ID: 1}, - want: &tailcfg.PeerChange{NodeID: 1, Capabilities: ptr.To([]tailcfg.NodeCapability(nil))}, - }, { name: "patch-online-to-true", a: &tailcfg.Node{ID: 1, Online: ptr.To(false)}, diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index 6a36c9261..6e3a62967 100644 --- a/control/controlknobs/controlknobs.go +++ b/control/controlknobs/controlknobs.go @@ -6,7 +6,6 @@ package controlknobs import ( - "slices" "sync/atomic" "tailscale.com/syncs" @@ -77,14 +76,11 @@ type Knobs struct { // UpdateFromNodeAttributes updates k (if non-nil) based on the provided self // node attributes (Node.Capabilities). -func (k *Knobs) UpdateFromNodeAttributes(selfNodeAttrs []tailcfg.NodeCapability, capMap tailcfg.NodeCapMap) { +func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { if k == nil { return } - has := func(attr tailcfg.NodeCapability) bool { - _, ok := capMap[attr] - return ok || slices.Contains(selfNodeAttrs, attr) - } + has := capMap.Contains var ( keepFullWG = has(tailcfg.NodeAttrDebugDisableWGTrim) disableDRPO = has(tailcfg.NodeAttrDebugDisableDRPO) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 9f6bcf17d..7dfac9595 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -823,15 +823,16 @@ func (b *LocalBackend) UpdateStatus(sb *ipnstate.StatusBuilder) { ss.UserID = b.netMap.User() if sn := b.netMap.SelfNode; sn.Valid() { peerStatusFromNode(ss, sn) - if c := sn.Capabilities(); c.Len() > 0 { - ss.Capabilities = c.AsSlice() - } if cm := sn.CapMap(); cm.Len() > 0 { + ss.Capabilities = make([]tailcfg.NodeCapability, 1, cm.Len()+1) + ss.Capabilities[0] = "HTTPS://TAILSCALE.COM/s/DEPRECATED-NODE-CAPS#see-https://github.com/tailscale/tailscale/issues/11508" ss.CapMap = make(tailcfg.NodeCapMap, sn.CapMap().Len()) cm.Range(func(k tailcfg.NodeCapability, v views.Slice[tailcfg.RawMessage]) bool { ss.CapMap[k] = v.AsSlice() + ss.Capabilities = append(ss.Capabilities, k) return true }) + slices.Sort(ss.Capabilities[1:]) } } for _, addr := range tailscaleIPs { diff --git a/ipn/ipnlocal/peerapi_test.go b/ipn/ipnlocal/peerapi_test.go index 85f5423e3..435ce5017 100644 --- a/ipn/ipnlocal/peerapi_test.go +++ b/ipn/ipnlocal/peerapi_test.go @@ -522,7 +522,7 @@ func TestHandlePeerAPI(t *testing.T) { }, } if tt.debugCap { - selfNode.Capabilities = append(selfNode.Capabilities, tailcfg.CapabilityDebug) + selfNode.CapMap = tailcfg.NodeCapMap{tailcfg.CapabilityDebug: nil} } var e peerAPITestEnv lb := &LocalBackend{ diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go index f2b3e303c..45bd4bc2c 100644 --- a/ipn/ipnlocal/serve_test.go +++ b/ipn/ipnlocal/serve_test.go @@ -684,8 +684,7 @@ func newTestBackend(t *testing.T) *LocalBackend { b.netMap = &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{ - Name: "example.ts.net", - Capabilities: []tailcfg.NodeCapability{tailcfg.NodeAttrsTailFSAccess}, + Name: "example.ts.net", }).View(), UserProfiles: map[tailcfg.UserID]tailcfg.UserProfile{ tailcfg.UserID(1): { diff --git a/ipn/ipnstate/ipnstate.go b/ipn/ipnstate/ipnstate.go index 94f1c4c79..c9ad0d0da 100644 --- a/ipn/ipnstate/ipnstate.go +++ b/ipn/ipnstate/ipnstate.go @@ -266,6 +266,10 @@ type PeerStatus struct { // "https://tailscale.com/cap/is-admin" // "https://tailscale.com/cap/file-sharing" // "funnel" + // + // Deprecated: use CapMap instead. See https://github.com/tailscale/tailscale/issues/11508 + // Every value is Capabilities is also a key in CapMap, even if it + // has no values in that map. Capabilities []tailcfg.NodeCapability `json:",omitempty"` // CapMap is a map of capabilities to their values. @@ -306,7 +310,7 @@ type PeerStatus struct { // HasCap reports whether ps has the given capability. func (ps *PeerStatus) HasCap(cap tailcfg.NodeCapability) bool { - return ps.CapMap.Contains(cap) || slices.Contains(ps.Capabilities, cap) + return ps.CapMap.Contains(cap) } // IsTagged reports whether ps is tagged. diff --git a/ipn/serve.go b/ipn/serve.go index 89ed6e556..b6e085b0c 100644 --- a/ipn/serve.go +++ b/ipn/serve.go @@ -445,7 +445,7 @@ func CheckFunnelPort(wantedPort uint16, node *ipnstate.PeerStatus) error { break } if portsStr == "" { - for _, attr := range node.Capabilities { + for attr := range node.CapMap { attr := string(attr) if !strings.HasPrefix(attr, string(tailcfg.CapabilityFunnelPorts)) { continue diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 877e0e384..353181748 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -129,7 +129,8 @@ type CapabilityVersion int // - 86: 2024-01-23: Client understands NodeAttrProbeUDPLifetime // - 87: 2024-02-11: UserProfile.Groups removed (added in 66) // - 88: 2024-03-05: Client understands NodeAttrSuggestExitNode -const CurrentCapabilityVersion CapabilityVersion = 88 +// - 89: 2024-03-23: Client no longer respects deleted PeerChange.Capabilities (use CapMap) +const CurrentCapabilityVersion CapabilityVersion = 89 type StableID string @@ -325,7 +326,7 @@ type Node struct { // "https://tailscale.com/cap/is-admin" // "https://tailscale.com/cap/file-sharing" // - // Deprecated: use CapMap instead. + // Deprecated: use CapMap instead. See https://github.com/tailscale/tailscale/issues/11508 Capabilities []NodeCapability `json:",omitempty"` // CapMap is a map of capabilities to their optional argument/data values. @@ -415,7 +416,7 @@ func (v NodeView) HasCap(cap NodeCapability) bool { // HasCap reports whether the node has the given capability. // It is safe to call on a nil Node. func (v *Node) HasCap(cap NodeCapability) bool { - return v != nil && (v.CapMap.Contains(cap) || slices.Contains(v.Capabilities, cap)) + return v != nil && v.CapMap.Contains(cap) } // DisplayName returns the user-facing name for a node which should @@ -2660,11 +2661,6 @@ type PeerChange struct { // KeyExpiry, if non-nil, changes the NodeID's key expiry. KeyExpiry *time.Time `json:",omitempty"` - - // Capabilities, if non-nil, means that the NodeID's capabilities changed. - // It's a pointer to a slice for "omitempty", to allow differentiating - // a change to empty from no change. - Capabilities *[]NodeCapability `json:",omitempty"` } // DerpMagicIP is a fake WireGuard endpoint IP address that means to