version: adjust to a pure semver version number, per bradfitz's proposal.

Signed-off-by: David Anderson <danderson@tailscale.com>
(cherry picked from commit c3467fbadb)

control/controlclient/netmap.go

@@ -132,12 +132,18 @@ func printPeerConcise(buf *strings.Builder, p *tailcfg.Node) {
 	if strings.HasPrefix(derp, derpPrefix) {
 		derp = "D" + derp[len(derpPrefix):]
 	}
+	var discoShort string
+	if !p.DiscoKey.IsZero() {
+		discoShort = p.DiscoKey.ShortString() + " "
+	}
 
 	// Most of the time, aip is just one element, so format the
 	// table to look good in that case. This will also make multi-
 	// subnet nodes stand out visually.
-	fmt.Fprintf(buf, " %v %-2v %-15v : %v\n",
-		p.Key.ShortString(), derp,
+	fmt.Fprintf(buf, " %v %s%-2v %-15v : %v\n",
+		p.Key.ShortString(),
+		discoShort,
+		derp,
 		strings.Join(aip, " "),
 		strings.Join(ep, " "))
 }
@@ -146,6 +152,7 @@ func printPeerConcise(buf *strings.Builder, p *tailcfg.Node) {
 func nodeConciseEqual(a, b *tailcfg.Node) bool {
 	return a.Key == b.Key &&
 		a.DERP == b.DERP &&
+		a.DiscoKey == b.DiscoKey &&
 		eqCIDRsIgnoreNil(a.AllowedIPs, b.AllowedIPs) &&
 		eqStringsIgnoreNil(a.Endpoints, b.Endpoints)
 }

control/controlclient/netmap_test.go

@@ -5,8 +5,10 @@
 package controlclient
 
 import (
+	"encoding/hex"
 	"testing"
 
+	"github.com/tailscale/wireguard-go/wgcfg"
 	"tailscale.com/tailcfg"
 )
 
@@ -17,6 +19,15 @@ func testNodeKey(b byte) (ret tailcfg.NodeKey) {
 	return
 }
 
+func testDiscoKey(hexPrefix string) (ret tailcfg.DiscoKey) {
+	b, err := hex.DecodeString(hexPrefix)
+	if err != nil {
+		panic(err)
+	}
+	copy(ret[:], b)
+	return
+}
+
 func TestNetworkMapConcise(t *testing.T) {
 	for _, tt := range []struct {
 		name string
@@ -202,6 +213,62 @@ func TestConciseDiffFrom(t *testing.T) {
 			},
 			want: "- [AQEBA] D1                 :    192.168.0.100:12     192.168.0.100:12354\n- [AwMDA] D3                 :    192.168.0.100:12     192.168.0.100:12354\n",
 		},
+		{
+			name: "peer_port_change",
+			a: &NetworkMap{
+				NodeKey: testNodeKey(1),
+				Peers: []*tailcfg.Node{
+					{
+						ID:        2,
+						Key:       testNodeKey(2),
+						DERP:      "127.3.3.40:2",
+						Endpoints: []string{"192.168.0.100:12", "1.1.1.1:1"},
+					},
+				},
+			},
+			b: &NetworkMap{
+				NodeKey: testNodeKey(1),
+				Peers: []*tailcfg.Node{
+					{
+						ID:        2,
+						Key:       testNodeKey(2),
+						DERP:      "127.3.3.40:2",
+						Endpoints: []string{"192.168.0.100:12", "1.1.1.1:2"},
+					},
+				},
+			},
+			want: "- [AgICA] D2                 :    192.168.0.100:12             1.1.1.1:1  \n+ [AgICA] D2                 :    192.168.0.100:12             1.1.1.1:2  \n",
+		},
+		{
+			name: "disco_key_only_change",
+			a: &NetworkMap{
+				NodeKey: testNodeKey(1),
+				Peers: []*tailcfg.Node{
+					{
+						ID:         2,
+						Key:        testNodeKey(2),
+						DERP:       "127.3.3.40:2",
+						Endpoints:  []string{"192.168.0.100:41641", "1.1.1.1:41641"},
+						DiscoKey:   testDiscoKey("f00f00f00f"),
+						AllowedIPs: []wgcfg.CIDR{{IP: wgcfg.IPv4(100, 102, 103, 104), Mask: 32}},
+					},
+				},
+			},
+			b: &NetworkMap{
+				NodeKey: testNodeKey(1),
+				Peers: []*tailcfg.Node{
+					{
+						ID:         2,
+						Key:        testNodeKey(2),
+						DERP:       "127.3.3.40:2",
+						Endpoints:  []string{"192.168.0.100:41641", "1.1.1.1:41641"},
+						DiscoKey:   testDiscoKey("ba4ba4ba4b"),
+						AllowedIPs: []wgcfg.CIDR{{IP: wgcfg.IPv4(100, 102, 103, 104), Mask: 32}},
+					},
+				},
+			},
+			want: "- [AgICA] d:f00f00f00f000000 D2 100.102.103.104 :   192.168.0.100:41641         1.1.1.1:41641\n+ [AgICA] d:ba4ba4ba4b000000 D2 100.102.103.104 :   192.168.0.100:41641         1.1.1.1:41641\n",
+		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			var got string

version/mkversion.sh

@@ -5,89 +5,107 @@ set -eu
 mode=$1
 describe=$2
 
-long() {
-    ver="${describe#v}"
-    stem="${ver%%-*}"
-    case "$stem" in
-        *.*.*)
-            # Full SemVer, nothing to do.
-            semver="${stem}"
+# Git describe output overall looks like
+# MAJOR.MINOR.PATCH-NUMCOMMITS-GITHASH. Depending on the tag being
+# described and the state of the repo, ver can be missing PATCH,
+# NUMCOMMITS, or both.
+#
+# Valid values look like: 1.2.3-1234-abcdef, 0.98-1234-abcdef,
+# 1.0.0-abcdef, 0.99-abcdef.
+ver="${describe#v}"
+stem="${ver%%-*}" # Just the semver-ish bit e.g. 1.2.3, 0.98
+suffix="${ver#$stem}" # The rest e.g. -23-abcdef, -abcdef
+
+# Normalize the stem into a full major.minor.patch semver. We might
+# not use all those pieces depending on what kind of version we're
+# making, but it's good to have them all on hand.
+case "$stem" in
+    *.*.*)
+        # Full SemVer, nothing to do
+        stem="$stem"
         ;;
-        *.*)
-            # Old style major.minor, add a .0
-            semver="${stem}.0"
-            ;;
-        *)
-            echo "Unparseable version $stem" >&2
-            exit 1
-            ;;
-    esac
-    suffix="${ver#$stem}"
-    case "$suffix" in
-        -*-*)
-            # Has a change count in addition to the commit hash.
+    *.*)
+        # Old style major.minor, add a .0
+        stem="${stem}.0"
+        ;;
+    *)
+        echo "Unparseable version $stem" >&2
+        exit 1
+        ;;
+esac
+major=$(echo "$stem" | cut -f1 -d.)
+minor=$(echo "$stem" | cut -f2 -d.)
+patch=$(echo "$stem" | cut -f3 -d.)
+
+# Extract the change count and git ID from the suffix.
+case "$suffix" in
+    -*-*)
+        # Has both a change count and a commit hash.
+        changecount=$(echo "$suffix" | cut -f2 -d-)
+        githash=$(echo "$suffix" | cut -f3 -d-)
+        ;;
+    -*)
+        # Git hash only, change count is zero.
+        changecount="0"
+        githash=$(echo "$suffix" | cut -f2 -d-)
+        ;;
+    *)
+        echo "Unparseable version suffix $suffix" >&2
+        exit 1
+        ;;
+esac
+
+# Validate that the version data makes sense. Rules:
+#  - Odd number minors are unstable. Patch must be 0, and gets
+#    replaced by changecount.
+#  - Even number minors are stable. Changecount must be 0, and
+#    gets removed.
+#
+# After this section, we only use major/minor/patch, which have been
+# tweaked as needed.
+if expr "$minor" : "[0-9]*[13579]$" >/dev/null; then
+    # Unstable
+    if [ "$patch" != "0" ]; then
+        # This is a fatal error, because a non-zero patch number
+        # indicates that we created an unstable git tag in violation
+        # of our versioning policy, and we want to blow up loudly to
+        # get that fixed.
+        echo "Unstable release $describe has a non-zero patch number, which is not allowed" >&2
+        exit 1
+    fi
+    patch="$changecount"
+else
+    # Stable
+    if [ "$changecount" != "0" ]; then
+        # This is a commit that's sitting between two stable
+        # releases. We never want to release such a commit to the
+        # pbulic, but it's useful to be able to build it for
+        # debugging. Just force the version to 0.0.0, so that we're
+        # forced to rely on the git commit hash.
+        major=0
+        minor=0
+        patch=0
+    fi
+fi
+
+case "$1" in
+    long)
+        echo "${major}.${minor}.${patch}-${githash}"
+        ;;
+    short)
+        echo "${major}.${minor}.${patch}"
+        ;;
+    xcode)
+        # CFBundleShortVersionString: the "short name" used in the App
+        # Store.  eg. 0.92.98
+        echo "VERSION_NAME = ${major}.${minor}.${patch}"
+        # CFBundleVersion: the build number. Needs to be 3 numeric
+        # sections that increment for each release according to SemVer
+        # rules.
+        #
+        # We start counting at 100 because we submitted using raw
+        # build numbers before, and Apple doesn't let you start over.
+        # e.g. 0.98.3 -> 100.98.3
+        echo "VERSION_ID = $((major + 100)).${minor}.${patch}"        
         ;;
-        -*)
-            # Missing change count, add one.
-            suffix="-0${suffix}"
-            ;;
-        *)
-            echo "Unexpected version suffix" >&2
-            exit 1
-    esac
-    echo "${semver}${suffix}"
-}
-
-short() {
-    ver="$(long)"
-    case "$ver" in
-        *-*-*)
-            echo "${ver%-*}"
-            ;;
-        *-*)
-            echo "$ver"
-            ;;
-        *)
-            echo "Long version in invalid format" >&2
-            exit 1
-            ;;
-    esac
-}
-
-xcode() {
-    ver=$(short | sed -e 's/-/./')
-    major=$(echo "$ver" | cut -f1 -d.)
-    minor=$(echo "$ver" | cut -f2 -d.)
-    patch=$(echo "$ver" | cut -f3 -d.)
-    changecount=$(echo "$ver" | cut -f4 -d.)
-
-    # Apple version numbers must be major.minor.patch. We have 4 fields
-    # because we need major.minor.patch for go module compatibility, and
-    # changecount for automatic version numbering of unstable builds. To
-    # resolve this, for Apple builds, we combine changecount into patch:
-    patch=$((patch*10000 + changecount))
-
-    # CFBundleShortVersionString: the "short name" used in the App Store.
-    # eg. 0.92.98
-    echo "VERSION_NAME = $major.$minor.$patch"
-    # CFBundleVersion: the build number. Needs to be 3 numeric sections
-    # that increment for each release according to SemVer rules.
-    #
-    # We start counting at 100 because we submitted using raw build
-    # numbers before, and Apple doesn't let you start over.
-    # e.g. 0.98.3-123 -> 100.98.3123
-    major=$((major + 100))
-    echo "VERSION_ID = $major.$minor.$patch"
-}
-
-case "$mode" in
-    long)
-        long
-    ;;
-    short)
-        short
-    ;;
-    xcode)
-        xcode
-    ;;
 esac

version/mkversion_test.go

@@ -15,42 +15,60 @@ func xcode(short, long string) string {
 	return fmt.Sprintf("VERSION_NAME = %s\nVERSION_ID = %s", short, long)
 }
 
-func mkversion(t *testing.T, mode, in string) string {
+func mkversion(t *testing.T, mode, in string) (string, bool) {
 	t.Helper()
 	bs, err := exec.Command("./mkversion.sh", mode, in).CombinedOutput()
 	if err != nil {
 		t.Logf("mkversion.sh output: %s", string(bs))
-		t.Fatalf("mkversion.sh %s %s: %v", mode, in, err)
+		return "", false
 	}
-	return strings.TrimSpace(string(bs))
+	return strings.TrimSpace(string(bs)), true
 }
 
 func TestMkversion(t *testing.T) {
 	tests := []struct {
 		in    string
+		ok    bool
 		long  string
 		short string
 		xcode string
 	}{
-		{"v0.98-abcdef", "0.98.0-0-abcdef", "0.98.0-0", xcode("0.98.0", "100.98.0")},
-		{"v0.98-123-abcdef", "0.98.0-123-abcdef", "0.98.0-123", xcode("0.98.123", "100.98.123")},
-		{"v0.99.5-123-abcdef", "0.99.5-123-abcdef", "0.99.5-123", xcode("0.99.50123", "100.99.50123")},
-		{"v0.99.5-123-abcdef", "0.99.5-123-abcdef", "0.99.5-123", xcode("0.99.50123", "100.99.50123")},
-		{"v2.3-0-abcdef", "2.3.0-0-abcdef", "2.3.0-0", xcode("2.3.0", "102.3.0")},
-		{"1.2.3-4-abcdef", "1.2.3-4-abcdef", "1.2.3-4", xcode("1.2.30004", "101.2.30004")},
+		{"v0.98-abcdef", true, "0.98.0-abcdef", "0.98.0", xcode("0.98.0", "100.98.0")},
+		{"v0.98.1-abcdef", true, "0.98.1-abcdef", "0.98.1", xcode("0.98.1", "100.98.1")},
+		{"v1.1.0-37-abcdef", true, "1.1.37-abcdef", "1.1.37", xcode("1.1.37", "101.1.37")},
+		{"v1.2.9-abcdef", true, "1.2.9-abcdef", "1.2.9", xcode("1.2.9", "101.2.9")},
+		{"v1.2.9-0-abcdef", true, "1.2.9-abcdef", "1.2.9", xcode("1.2.9", "101.2.9")},
+		{"v1.15.0-129-abcdef", true, "1.15.129-abcdef", "1.15.129", xcode("1.15.129", "101.15.129")},
+
+		{"v0.98-123-abcdef", true, "0.0.0-abcdef", "0.0.0", xcode("0.0.0", "100.0.0")},
+		{"v1.0.0-37-abcdef", true, "0.0.0-abcdef", "0.0.0", xcode("0.0.0", "100.0.0")},
+
+		{"v0.99.5-0-abcdef", false, "", "", ""},   // unstable, patch not allowed
+		{"v0.99.5-123-abcdef", false, "", "", ""}, // unstable, patch not allowed
+		{"v1-abcdef", false, "", "", ""},          // bad semver
+		{"v1.0", false, "", "", ""},               // missing suffix
 	}
 
 	for _, test := range tests {
-		gotlong := mkversion(t, "long", test.in)
-		gotshort := mkversion(t, "short", test.in)
-		gotxcode := mkversion(t, "xcode", test.in)
-		if gotlong != test.long {
+		gotlong, longOK := mkversion(t, "long", test.in)
+		if longOK != test.ok {
+			t.Errorf("mkversion.sh long %q ok=%v, want %v", test.in, longOK, test.ok)
+		}
+		gotshort, shortOK := mkversion(t, "short", test.in)
+		if shortOK != test.ok {
+			t.Errorf("mkversion.sh short %q ok=%v, want %v", test.in, shortOK, test.ok)
+		}
+		gotxcode, xcodeOK := mkversion(t, "xcode", test.in)
+		if xcodeOK != test.ok {
+			t.Errorf("mkversion.sh xcode %q ok=%v, want %v", test.in, xcodeOK, test.ok)
+		}
+		if longOK && gotlong != test.long {
 			t.Errorf("mkversion.sh long %q: got %q, want %q", test.in, gotlong, test.long)
 		}
-		if gotshort != test.short {
+		if shortOK && gotshort != test.short {
 			t.Errorf("mkversion.sh short %q: got %q, want %q", test.in, gotshort, test.short)
 		}
-		if gotxcode != test.xcode {
+		if xcodeOK && gotxcode != test.xcode {
 			t.Errorf("mkversion.sh xcode %q: got %q, want %q", test.in, gotxcode, test.xcode)
 		}
 	}

wgengine/filter/filter.go

@@ -349,7 +349,10 @@ func omitDropLogging(p *packet.ParsedPacket, dir direction) bool {
 			if string(dst) == ipv6AllMLDv2CapableRouters {
 				return true
 			}
-			panic(fmt.Sprintf("Got proto=%2x; src=%x dst=%x", int(p.IPProto), src, dst))
+			// Actually, just catch all multicast.
+			if dst[0] == 0xff {
+				return true
+			}
 		}
 	}
 	return false

wgengine/filter/filter_test.go

@@ -343,6 +343,12 @@ func TestOmitDropLogging(t *testing.T) {
 			dir:  out,
 			want: true,
 		},
+		{
+			name: "v6_udp_multicast",
+			pkt:  parseHexPkt(t, "60 00 00 00 00 00 11 00  fe800000000000007dc6bc04499262a3 ff120000000000000000000000008384"),
+			dir:  out,
+			want: true,
+		},
 	}
 
 	for _, tt := range tests {