diff --git a/cmd/tailscale/cli/funnel.go b/cmd/tailscale/cli/funnel.go index 6086646fe..17d362993 100644 --- a/cmd/tailscale/cli/funnel.go +++ b/cmd/tailscale/cli/funnel.go @@ -14,7 +14,6 @@ import ( "github.com/peterbourgon/ff/v3/ffcli" "tailscale.com/ipn" - "tailscale.com/ipn/ipnstate" "tailscale.com/tailcfg" "tailscale.com/util/mak" ) @@ -88,10 +87,6 @@ func (e *serveEnv) runFunnel(ctx context.Context, args []string) error { if sc == nil { sc = new(ipn.ServeConfig) } - st, err := e.getLocalClientStatusWithoutPeers(ctx) - if err != nil { - return fmt.Errorf("getting client status: %w", err) - } port64, err := strconv.ParseUint(args[0], 10, 16) if err != nil { @@ -103,11 +98,15 @@ func (e *serveEnv) runFunnel(ctx context.Context, args []string) error { // Don't block from turning off existing Funnel if // network configuration/capabilities have changed. // Only block from starting new Funnels. - if err := e.verifyFunnelEnabled(ctx, st, port); err != nil { + if err := e.verifyFunnelEnabled(ctx, port); err != nil { return err } } + st, err := e.getLocalClientStatusWithoutPeers(ctx) + if err != nil { + return fmt.Errorf("getting client status: %w", err) + } dnsName := strings.TrimSuffix(st.Self.DNSName, ".") hp := ipn.HostPort(dnsName + ":" + strconv.Itoa(int(port))) if on == sc.AllowFunnel[hp] { @@ -141,13 +140,7 @@ func (e *serveEnv) runFunnel(ctx context.Context, args []string) error { // If an error is reported, the CLI should stop execution and return the error. // // verifyFunnelEnabled may refresh the local state and modify the st input. -func (e *serveEnv) verifyFunnelEnabled(ctx context.Context, st *ipnstate.Status, port uint16) error { - hasFunnelAttrs := func(selfNode *ipnstate.PeerStatus) bool { - return selfNode.HasCap(tailcfg.CapabilityHTTPS) && selfNode.HasCap(tailcfg.NodeAttrFunnel) - } - if hasFunnelAttrs(st.Self) { - return nil // already enabled - } +func (e *serveEnv) verifyFunnelEnabled(ctx context.Context, port uint16) error { enableErr := e.enableFeatureInteractive(ctx, "funnel", tailcfg.CapabilityHTTPS, tailcfg.NodeAttrFunnel) st, statusErr := e.getLocalClientStatusWithoutPeers(ctx) // get updated status; interactive flow may block switch { diff --git a/cmd/tailscale/cli/serve_legacy.go b/cmd/tailscale/cli/serve_legacy.go index a1803c6dd..e6e18669d 100644 --- a/cmd/tailscale/cli/serve_legacy.go +++ b/cmd/tailscale/cli/serve_legacy.go @@ -818,6 +818,24 @@ func parseServePort(s string) (uint16, error) { // 2023-08-09: The only valid feature values are "serve" and "funnel". // This can be moved to some CLI lib when expanded past serve/funnel. func (e *serveEnv) enableFeatureInteractive(ctx context.Context, feature string, caps ...tailcfg.NodeCapability) (err error) { + st, err := e.getLocalClientStatusWithoutPeers(ctx) + if err != nil { + return fmt.Errorf("getting client status: %w", err) + } + if st.Self == nil { + return errors.New("no self node") + } + hasCaps := func() bool { + for _, c := range caps { + if !st.Self.HasCap(c) { + return false + } + } + return true + } + if hasCaps() { + return nil // already enabled + } info, err := e.lc.QueryFeature(ctx, feature) if err != nil { return err diff --git a/cmd/tailscale/cli/serve_legacy_test.go b/cmd/tailscale/cli/serve_legacy_test.go index f77106ddd..58139ecc2 100644 --- a/cmd/tailscale/cli/serve_legacy_test.go +++ b/cmd/tailscale/cli/serve_legacy_test.go @@ -786,7 +786,7 @@ func TestVerifyFunnelEnabled(t *testing.T) { { name: "fallback-flow-enabled", queryFeatureResponse: mockQueryFeatureResponse{resp: nil, err: errors.New("not-allowed")}, - caps: []tailcfg.NodeCapability{tailcfg.CapabilityHTTPS, tailcfg.NodeAttrFunnel}, + caps: []tailcfg.NodeCapability{tailcfg.CapabilityHTTPS, tailcfg.NodeAttrFunnel, "https://tailscale.com/cap/funnel-ports?ports=80,443,8080-8090"}, wantErr: "", // no error, success }, { @@ -811,10 +811,6 @@ func TestVerifyFunnelEnabled(t *testing.T) { defer func() { fakeStatus.Self.Capabilities = oldCaps }() // reset after test fakeStatus.Self.Capabilities = tt.caps } - st, err := e.getLocalClientStatusWithoutPeers(ctx) - if err != nil { - t.Fatal(err) - } defer func() { r := recover() @@ -826,7 +822,7 @@ func TestVerifyFunnelEnabled(t *testing.T) { t.Errorf("wrong panic; got=%s, want=%s", gotPanic, tt.wantPanic) } }() - gotErr := e.verifyFunnelEnabled(ctx, st, 443) + gotErr := e.verifyFunnelEnabled(ctx, 443) var got string if gotErr != nil { got = gotErr.Error() diff --git a/cmd/tailscale/cli/serve_v2.go b/cmd/tailscale/cli/serve_v2.go index 76677a299..0e0c37d84 100644 --- a/cmd/tailscale/cli/serve_v2.go +++ b/cmd/tailscale/cli/serve_v2.go @@ -213,15 +213,10 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc { ctx, cancel := signal.NotifyContext(ctx, os.Interrupt) defer cancel() - st, err := e.getLocalClientStatusWithoutPeers(ctx) - if err != nil { - return fmt.Errorf("getting client status: %w", err) - } - funnel := subcmd == funnel if funnel { // verify node has funnel capabilities - if err := e.verifyFunnelEnabled(ctx, st, 443); err != nil { + if err := e.verifyFunnelEnabled(ctx, 443); err != nil { return err } } @@ -246,6 +241,10 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc { if sc == nil { sc = new(ipn.ServeConfig) } + st, err := e.getLocalClientStatusWithoutPeers(ctx) + if err != nil { + return fmt.Errorf("getting client status: %w", err) + } dnsName := strings.TrimSuffix(st.Self.DNSName, ".") // set parent serve config to always be persisted