tailcfg: rename map request version to "capability version"

And add a CapabilityVersion type, primarily for documentation.

This makes MapRequest.Version, RegisterRequest.Version, and
SetDNSRequest.Version all use the same version, which will avoid
confusing in the future if Register or SetDNS ever changed their
semantics on Version change. (Currently they're both always 1)

This will requre a control server change to allow a
SetDNSRequest.Version value other than 1 to be deployed first.

Change-Id: I073042a216e0d745f52ee2dbc45cf336b9f84b7c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

control/controlclient/direct.go

@@ -368,7 +368,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 	}
 	now := time.Now().Round(time.Second)
 	request := tailcfg.RegisterRequest{
-		Version:    1,
+		Version:    1, // TODO(bradfitz): use tailcfg.CurrentCapabilityVersion when over Noise
 		OldNodeKey: oldNodeKey,
 		NodeKey:    tryingNewKey.Public(),
 		Hostinfo:   hi,
@@ -614,7 +614,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm
 	}
 
 	request := &tailcfg.MapRequest{
-		Version:       tailcfg.CurrentMapRequestVersion,
+		Version:       tailcfg.CurrentCapabilityVersion,
 		KeepAlive:     c.keepAlive,
 		NodeKey:       persist.PrivateNodeKey.Public(),
 		DiscoKey:      c.discoPubKey,

ipn/ipnlocal/local.go

@@ -2930,7 +2930,7 @@ func (b *LocalBackend) FileTargets() ([]*apitype.FileTarget, error) {
 // friendly options to get HTTPS certs.
 func (b *LocalBackend) SetDNS(ctx context.Context, name, value string) error {
 	req := &tailcfg.SetDNSRequest{
-		Version: 1,
+		Version: 1, // TODO(bradfitz,maisem): use tailcfg.CurrentCapabilityVersion when using the Noise transport
 		Type:    "TXT",
 		Name:    name,
 		Value:   value,

tailcfg/tailcfg.go

@@ -24,7 +24,19 @@ import (
 	"tailscale.com/util/dnsname"
 )
 
-// CurrentMapRequestVersion is the current MapRequest.Version value.
+// CapabilityVersion represents the client's capability level. That
+// is, it can be thought of as the client's simple version number: a
+// single monotonically increasing integer, rather than the relatively
+// complex x.y.z-xxxxx semver+hash(es). Whenever the client gains a
+// capability or wants to negotiate a change in semantics with the
+// server (control plane), bump this number and document what's new.
+//
+// Previously (prior to 2022-03-06), it was known as the "MapRequest
+// version" or "mapVer" or "map cap" and that name and usage persists
+// in places.
+type CapabilityVersion int
+
+// CurrentCapabilityVersion is the current capability version of the codebase.
 //
 // History of versions:
 //     3: implicit compression, keep-alives
@@ -52,7 +64,7 @@ import (
 //    25: 2021-11-01: MapResponse.Debug.Exit
 //    26: 2022-01-12: (nothing, just bumping for 1.20.0)
 //    27: 2022-02-18: start of SSHPolicy being respected
-const CurrentMapRequestVersion = 27
+const CurrentCapabilityVersion CapabilityVersion = 27
 
 type StableID string
 
@@ -847,7 +859,14 @@ func (st SignatureType) String() string {
 //	https://login.tailscale.com/machine/<mkey hex>
 type RegisterRequest struct {
 	_ structs.Incomparable
-	Version    int // currently 1
+
+	// Version is the client's capabilities when using the Noise
+	// transport.
+	//
+	// When using the original nacl crypto_box transport, the
+	// value must be 1.
+	Version CapabilityVersion
+
 	NodeKey    key.NodePublic
 	OldNodeKey key.NodePublic
 	Auth       struct {
@@ -961,8 +980,8 @@ type MapRequest struct {
 	// we want to signal to the control server that we're capable of something
 	// different.
 	//
-	// For current values and history, see CurrentMapRequestVersion above.
+	// For current values and history, see the CapabilityVersion type's docs.
-	Version int
+	Version CapabilityVersion
 
 	Compress    string // "zstd" or "" (no compression)
 	KeepAlive   bool   // whether server should send keep-alives back to us
@@ -1046,7 +1065,7 @@ type FilterRule struct {
 	SrcIPs []string
 
 	// SrcBits is deprecated; it's the old way to specify a CIDR
-	// prior to MapRequest.Version 7. Its values correspond to the
+	// prior to CapabilityVersion 7. Its values correspond to the
 	// SrcIPs above.
 	//
 	// If an entry of SrcBits is present for the same index as a
@@ -1491,10 +1510,12 @@ const (
 // using the local machine key, and sent to:
 //	https://login.tailscale.com/machine/<mkey hex>/set-dns
 type SetDNSRequest struct {
-	// Version indicates what level of SetDNSRequest functionality
+	// Version is the client's capabilities
-	// the client understands. Currently this type only has
+	// (CurrentCapabilityVersion) when using the Noise transport.
-	// one version; this field should always be 1 for now.
+	//
-	Version int
+	// When using the original nacl crypto_box transport, the
+	// value must be 1.
+	Version CapabilityVersion
 
 	// NodeKey is the client's current node key.
 	NodeKey key.NodePublic

tstest/integration/testcontrol/testcontrol.go

@@ -408,19 +408,18 @@ func (s *Server) CompleteAuth(authPathOrURL string) bool {
 
 func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey key.MachinePublic) {
 	msg, err := ioutil.ReadAll(io.LimitReader(r.Body, msgLimit))
-	if err != nil {
 	r.Body.Close()
+	if err != nil {
 		http.Error(w, fmt.Sprintf("bad map request read: %v", err), 400)
 		return
 	}
-	r.Body.Close()
 
 	var req tailcfg.RegisterRequest
 	if err := s.decode(mkey, msg, &req); err != nil {
 		go panic(fmt.Sprintf("serveRegister: decode: %v", err))
 	}
-	if req.Version != 1 {
+	if req.Version == 0 {
-		go panic(fmt.Sprintf("serveRegister: unsupported version: %d", req.Version))
+		panic("serveRegister: zero Version")
 	}
 	if req.NodeKey.IsZero() {
 		go panic("serveRegister: request has zero node key")