Merge remote-tracking branch 'upstream/main'

Author: Mmx233

.golangci.yml

@@ -1,97 +1,110 @@
+version: "2"
+# Configuration for how we run golangci-lint
+# Timeout of 5m was the default in v1.
+run:
+  timeout: 5m
 linters:
   # Don't enable any linters by default; just the ones that we explicitly
   # enable in the list below.
-  disable-all: true
+  default: none
   enable:
     - bidichk
-    - gofmt
-    - goimports
     - govet
     - misspell
     - revive
-
-# Configuration for how we run golangci-lint
-run:
-  timeout: 5m
-
-issues:
-  # Excluding configuration per-path, per-linter, per-text and per-source
-  exclude-rules:
-    # These are forks of an upstream package and thus are exempt from stylistic
-    # changes that would make pulling in upstream changes harder.
-    - path: tempfork/.*\.go
-      text: "File is not `gofmt`-ed with `-s` `-r 'interface{} -> any'`"
-    - path: util/singleflight/.*\.go
-      text: "File is not `gofmt`-ed with `-s` `-r 'interface{} -> any'`"
-
-# Per-linter settings are contained in this top-level key
-linters-settings:
-  gofmt:
-    rewrite-rules:
-      - pattern: 'interface{}'
-        replacement: 'any'
-
-  govet:
+  settings:
     # Matches what we use in corp as of 2023-12-07
-    enable:
-      - asmdecl
-      - assign
-      - atomic
-      - bools
-      - buildtag
-      - cgocall
-      - copylocks
-      - deepequalerrors
-      - errorsas
-      - framepointer
-      - httpresponse
-      - ifaceassert
-      - loopclosure
-      - lostcancel
-      - nilfunc
-      - nilness
-      - printf
-      - reflectvaluecompare
-      - shift
-      - sigchanyzer
-      - sortslice
-      - stdmethods
-      - stringintconv
-      - structtag
-      - testinggoroutine
-      - tests
-      - unmarshal
-      - unreachable
-      - unsafeptr
-      - unusedresult
-    settings:
-      printf:
-        # List of print function names to check (in addition to default)
-        funcs:
-          - github.com/tailscale/tailscale/types/logger.Discard
-          # NOTE(andrew-d): this doesn't currently work because the printf
-          # analyzer doesn't support type declarations
-          #- github.com/tailscale/tailscale/types/logger.Logf
-
-  revive:
-    enable-all-rules: false
-    ignore-generated-header: true
+    govet:
+      enable:
+        - asmdecl
+        - assign
+        - atomic
+        - bools
+        - buildtag
+        - cgocall
+        - copylocks
+        - deepequalerrors
+        - errorsas
+        - framepointer
+        - httpresponse
+        - ifaceassert
+        - loopclosure
+        - lostcancel
+        - nilfunc
+        - nilness
+        - printf
+        - reflectvaluecompare
+        - shift
+        - sigchanyzer
+        - sortslice
+        - stdmethods
+        - stringintconv
+        - structtag
+        - testinggoroutine
+        - tests
+        - unmarshal
+        - unreachable
+        - unsafeptr
+        - unusedresult
+      settings:
+        printf:
+          # List of print function names to check (in addition to default)
+          funcs:
+            - github.com/tailscale/tailscale/types/logger.Discard
+            # NOTE(andrew-d): this doesn't currently work because the printf
+            # analyzer doesn't support type declarations
+            #- github.com/tailscale/tailscale/types/logger.Logf
+    revive:
+      enable-all-rules: false
+      rules:
+        - name: atomic
+        - name: context-keys-type
+        - name: defer
+          arguments: [[
+              # Calling 'recover' at the time a defer is registered (i.e. "defer recover()") has no effect.
+              "immediate-recover",
+              # Calling 'recover' outside of a deferred function has no effect
+              "recover",
+              # Returning values from a deferred function has no effect
+              "return",
+          ]]
+        - name: duplicated-imports
+        - name: errorf
+        - name: string-of-int
+        - name: time-equal
+        - name: unconditional-recursion
+        - name: useless-break
+        - name: waitgroup-by-value
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
     rules:
-      - name: atomic
-      - name: context-keys-type
-      - name: defer
-        arguments: [[
-          # Calling 'recover' at the time a defer is registered (i.e. "defer recover()") has no effect.
-          "immediate-recover",
-          # Calling 'recover' outside of a deferred function has no effect
-          "recover",
-          # Returning values from a deferred function has no effect
-          "return",
-        ]]
-      - name: duplicated-imports
-      - name: errorf
-      - name: string-of-int
-      - name: time-equal
-      - name: unconditional-recursion
-      - name: useless-break
-      - name: waitgroup-by-value
+      # These are forks of an upstream package and thus are exempt from stylistic
+      # changes that would make pulling in upstream changes harder.
+      - path: tempfork/.*\.go
+        text: File is not `gofmt`-ed with `-s` `-r 'interface{} -> any'`
+      - path: util/singleflight/.*\.go
+        text: File is not `gofmt`-ed with `-s` `-r 'interface{} -> any'`
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+formatters:
+  enable:
+    - gofmt
+    - goimports
+  settings:
+    gofmt:
+      rewrite-rules:
+        - pattern: interface{}
+          replacement: any
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

README.md

@@ -71,8 +71,7 @@ We require [Developer Certificate of
 Origin](https://en.wikipedia.org/wiki/Developer_Certificate_of_Origin)
 `Signed-off-by` lines in commits.
 
-See `git log` for our commit message style. It's basically the same as
-[Go's style](https://go.dev/wiki/CommitMessage).
+See [commit-messages.md](docs/commit-messages.md) (or skim `git log`) for our commit message style.
 
 ## About Us
 

build_dist.sh

@@ -28,18 +28,26 @@ EOF
 	exit 0
 fi
 
-tags=""
+tags="${TAGS:-}"
 ldflags="-X tailscale.com/version.longStamp=${VERSION_LONG} -X tailscale.com/version.shortStamp=${VERSION_SHORT}"
 
 # build_dist.sh arguments must precede go build arguments.
 while [ "$#" -gt 1 ]; do
 	case "$1" in
 	--extra-small)
+		if [ ! -z "${TAGS:-}" ]; then
+			echo "set either --extra-small or \$TAGS, but not both"
+			exit 1
+		fi
 		shift
 		ldflags="$ldflags -w -s"
-		tags="${tags:+$tags,}ts_omit_aws,ts_omit_bird,ts_omit_tap,ts_omit_kube,ts_omit_completion,ts_omit_ssh,ts_omit_wakeonlan,ts_omit_capture"
+		tags="${tags:+$tags,}ts_omit_aws,ts_omit_bird,ts_omit_tap,ts_omit_kube,ts_omit_completion,ts_omit_ssh,ts_omit_wakeonlan,ts_omit_capture,ts_omit_relayserver,ts_omit_taildrop"
 		;;
 	--box)
+		if [ ! -z "${TAGS:-}" ]; then
+			echo "set either --box or \$TAGS, but not both"
+			exit 1
+		fi
 		shift
 		tags="${tags:+$tags,}ts_include_cli"
 		;;

client/local/local.go

@@ -1504,7 +1504,7 @@ func (lc *Client) SwitchProfile(ctx context.Context, profile ipn.ProfileID) erro
 // If the profile is the current profile, an empty profile
 // will be selected as if SwitchToEmptyProfile was called.
 func (lc *Client) DeleteProfile(ctx context.Context, profile ipn.ProfileID) error {
-	_, err := lc.send(ctx, "DELETE", "/localapi/v0/profiles"+url.PathEscape(string(profile)), http.StatusNoContent, nil)
+	_, err := lc.send(ctx, "DELETE", "/localapi/v0/profiles/"+url.PathEscape(string(profile)), http.StatusNoContent, nil)
 	return err
 }
 

client/local/local_test.go

@@ -9,10 +9,10 @@ import (
 	"context"
 	"net"
 	"net/http"
-	"net/http/httptest"
 	"testing"
 
 	"tailscale.com/tstest/deptest"
+	"tailscale.com/tstest/nettest"
 	"tailscale.com/types/key"
 )
 
@@ -36,15 +36,15 @@ func TestGetServeConfigFromJSON(t *testing.T) {
 }
 
 func TestWhoIsPeerNotFound(t *testing.T) {
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	nw := nettest.GetNetwork(t)
+	ts := nettest.NewHTTPServer(nw, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(404)
 	}))
 	defer ts.Close()
 
 	lc := &Client{
 		Dial: func(ctx context.Context, network, addr string) (net.Conn, error) {
-			var std net.Dialer
-			return std.DialContext(ctx, network, ts.Listener.Addr().(*net.TCPAddr).String())
+			return nw.Dial(ctx, network, ts.Listener.Addr().String())
 		},
 	}
 	var k key.NodePublic

client/systray/logo.go

@@ -11,10 +11,12 @@ import (
 	"image"
 	"image/color"
 	"image/png"
+	"runtime"
 	"sync"
 	"time"
 
 	"fyne.io/systray"
+	ico "github.com/Kodeworks/golang-image-ico"
 	"github.com/fogleman/gg"
 )
 
@@ -251,7 +253,13 @@ func (logo tsLogo) renderWithBorder(borderUnits int) *bytes.Buffer {
 	}
 
 	b := bytes.NewBuffer(nil)
-	png.Encode(b, dc.Image())
+
+	// Encode as ICO format on Windows, PNG on all other platforms.
+	if runtime.GOOS == "windows" {
+		_ = ico.Encode(b, dc.Image())
+	} else {
+		_ = png.Encode(b, dc.Image())
+	}
 	return b
 }
 

client/systray/systray.go

@@ -7,9 +7,11 @@
 package systray
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
+	"image"
 	"io"
 	"log"
 	"net/http"
@@ -23,6 +25,7 @@ import (
 	"time"
 
 	"fyne.io/systray"
+	ico "github.com/Kodeworks/golang-image-ico"
 	"github.com/atotto/clipboard"
 	dbus "github.com/godbus/dbus/v5"
 	"github.com/toqueteos/webbrowser"
@@ -81,12 +84,13 @@ type Menu struct {
 	bgCancel context.CancelFunc
 
 	// Top-level menu items
-	connect    *systray.MenuItem
-	disconnect *systray.MenuItem
-	self       *systray.MenuItem
-	exitNodes  *systray.MenuItem
-	more       *systray.MenuItem
-	quit       *systray.MenuItem
+	connect     *systray.MenuItem
+	disconnect  *systray.MenuItem
+	self        *systray.MenuItem
+	exitNodes   *systray.MenuItem
+	more        *systray.MenuItem
+	rebuildMenu *systray.MenuItem
+	quit        *systray.MenuItem
 
 	rebuildCh  chan struct{} // triggers a menu rebuild
 	accountsCh chan ipn.ProfileID
@@ -292,6 +296,17 @@ func (menu *Menu) rebuild() {
 		})
 	}
 
+	// TODO(#15528): this menu item shouldn't be necessary at all,
+	// but is at least more discoverable than having users switch profiles or exit nodes.
+	menu.rebuildMenu = systray.AddMenuItem("Rebuild menu", "Fix missing menu items")
+	onClick(ctx, menu.rebuildMenu, func(ctx context.Context) {
+		select {
+		case <-ctx.Done():
+		case menu.rebuildCh <- struct{}{}:
+		}
+	})
+	menu.rebuildMenu.Enable()
+
 	menu.quit = systray.AddMenuItem("Quit", "Quit the app")
 	menu.quit.Enable()
 
@@ -330,6 +345,20 @@ func setRemoteIcon(menu *systray.MenuItem, urlStr string) {
 		resp, err := http.Get(urlStr)
 		if err == nil && resp.StatusCode == http.StatusOK {
 			b, _ = io.ReadAll(resp.Body)
+
+			// Convert image to ICO format on Windows
+			if runtime.GOOS == "windows" {
+				im, _, err := image.Decode(bytes.NewReader(b))
+				if err != nil {
+					return
+				}
+				buf := bytes.NewBuffer(nil)
+				if err := ico.Encode(buf, im); err != nil {
+					return
+				}
+				b = buf.Bytes()
+			}
+
 			httpCache[urlStr] = b
 			resp.Body.Close()
 		}