From 7c1ed38ab399b82d7b5951e47491784ba0bf97c7 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 11 Sep 2023 21:44:38 -0700 Subject: [PATCH] ipn/ipnlocal: fix missing controlknobs.Knobs plumbing I missed connecting some controlknobs.Knobs pieces in 4e91cf20a854 resulting in that breaking control knobs entirely. Whoops. The fix in ipn/ipnlocal (where it makes a new controlclient) but to atone, I also added integration tests. Those integration tests use a new "tailscale debug control-knobs" which by itself might be useful for future debugging. Updates #9351 Change-Id: Id9c89c8637746d879d5da67b9ac4e0d2367a3f0d Signed-off-by: Brad Fitzpatrick --- client/tailscale/localclient.go | 14 ++++++++++++ cmd/tailscale/cli/debug.go | 19 ++++++++++++++++ control/controlknobs/controlknobs.go | 16 ++++++++++++++ ipn/ipnlocal/local.go | 1 + ipn/localapi/localapi.go | 7 ++++++ tstest/integration/integration_test.go | 30 ++++++++++++++++++++++++++ 6 files changed, 87 insertions(+) diff --git a/client/tailscale/localclient.go b/client/tailscale/localclient.go index af46f896e..1b2579772 100644 --- a/client/tailscale/localclient.go +++ b/client/tailscale/localclient.go @@ -392,6 +392,20 @@ func (lc *LocalClient) DebugAction(ctx context.Context, action string) error { return nil } +// DebugResultJSON invokes a debug action and returns its result as something JSON-able. +// These are development tools and subject to change or removal over time. +func (lc *LocalClient) DebugResultJSON(ctx context.Context, action string) (any, error) { + body, err := lc.send(ctx, "POST", "/localapi/v0/debug?action="+url.QueryEscape(action), 200, nil) + if err != nil { + return nil, fmt.Errorf("error %w: %s", err, body) + } + var x any + if err := json.Unmarshal(body, &x); err != nil { + return nil, err + } + return x, nil +} + // DebugPortmapOpts contains options for the DebugPortmap command. type DebugPortmapOpts struct { // Duration is how long the mapping should be created for. It defaults diff --git a/cmd/tailscale/cli/debug.go b/cmd/tailscale/cli/debug.go index 0cedc0256..1fe3bf00b 100644 --- a/cmd/tailscale/cli/debug.go +++ b/cmd/tailscale/cli/debug.go @@ -138,6 +138,11 @@ var debugCmd = &ffcli.Command{ Exec: localAPIAction("break-derp-conns"), ShortHelp: "break any open DERP connections from the daemon", }, + { + Name: "control-knobs", + Exec: debugControlKnobs, + ShortHelp: "see current control knobs", + }, { Name: "prefs", Exec: runPrefs, @@ -915,3 +920,17 @@ func runPeerEndpointChanges(ctx context.Context, args []string) error { fmt.Printf("%s", dst.String()) return nil } + +func debugControlKnobs(ctx context.Context, args []string) error { + if len(args) > 0 { + return errors.New("unexpected arguments") + } + v, err := localClient.DebugResultJSON(ctx, "control-knobs") + if err != nil { + return err + } + e := json.NewEncoder(os.Stdout) + e.SetIndent("", " ") + e.Encode(v) + return nil +} diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index ace56264c..dc24faac2 100644 --- a/control/controlknobs/controlknobs.go +++ b/control/controlknobs/controlknobs.go @@ -39,3 +39,19 @@ type Knobs struct { // running in magicsock, even when idle. ForceBackgroundSTUN atomic.Bool } + +// AsDebugJSON returns k as something that can be marshalled with json.Marshal +// for debug. +func (k *Knobs) AsDebugJSON() map[string]any { + if k == nil { + return nil + } + return map[string]any{ + "DisableUPnP": k.DisableUPnP.Load(), + "DisableDRPO": k.DisableDRPO.Load(), + "KeepFullWGConfig": k.KeepFullWGConfig.Load(), + "RandomizeClientPort": k.RandomizeClientPort.Load(), + "OneCGNAT": k.OneCGNAT.Load(), + "ForceBackgroundSTUN": k.ForceBackgroundSTUN.Load(), + } +} diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 57d1e66aa..bdf593362 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1462,6 +1462,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error { Observer: b, C2NHandler: http.HandlerFunc(b.handleC2N), DialPlan: &b.dialPlan, // pointer because it can't be copied + ControlKnobs: b.sys.ControlKnobs(), // Don't warn about broken Linux IP forwarding when // netstack is being used. diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index d0e4bc761..c5fec6091 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -557,6 +557,13 @@ func (h *Handler) serveDebug(w http.ResponseWriter, r *http.Request) { err = h.b.DebugBreakTCPConns() case "break-derp-conns": err = h.b.DebugBreakDERPConns() + case "control-knobs": + k := h.b.ControlKnobs() + w.Header().Set("Content-Type", "application/json") + err = json.NewEncoder(w).Encode(k.AsDebugJSON()) + if err == nil { + return + } case "": err = fmt.Errorf("missing parameter 'action'") default: diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index 5fc17302c..286c88845 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -119,6 +119,36 @@ func TestOneNodeExpiredKey(t *testing.T) { d1.MustCleanShutdown(t) } +func TestControlKnobs(t *testing.T) { + t.Parallel() + env := newTestEnv(t) + n1 := newTestNode(t, env) + + d1 := n1.StartDaemon() + defer d1.MustCleanShutdown(t) + n1.AwaitResponding() + n1.MustUp() + + t.Logf("Got IP: %v", n1.AwaitIP()) + n1.AwaitRunning() + + cmd := n1.Tailscale("debug", "control-knobs") + cmd.Stdout = nil // in case --verbose-tailscale was set + cmd.Stderr = nil // in case --verbose-tailscale was set + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatal(err) + } + t.Logf("control-knobs output:\n%s", out) + var m map[string]any + if err := json.Unmarshal(out, &m); err != nil { + t.Fatal(err) + } + if got, want := m["DisableUPnP"], true; got != want { + t.Errorf("control-knobs DisableUPnP = %v; want %v", got, want) + } +} + func TestCollectPanic(t *testing.T) { t.Parallel() env := newTestEnv(t)