cmd/tailscale/cli: [serve/funnel] support omitting scheme for TCP (#9856)

The `serve` command for TCP  has always required the scheme of the target to be specified. However, when it's omitted the error message reported is misleading

```
error: failed to apply TCP serve: invalid TCP target "localhost:5900": missing port in address
```

Since we know the target is TCP, we shouldn't require it to be specified. This aligns with the changes for HTTP proxies in https://github.com/tailscale/tailscale/issues/8489

closes #9855

Signed-off-by: Tyler Smalley <tyler@tailscale.com>
pull/9885/head
Tyler Smalley 1 year ago committed by GitHub
parent 35376d52d4
commit 7a3ae39025
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -16,6 +16,7 @@ import (
"os/signal" "os/signal"
"path" "path"
"path/filepath" "path/filepath"
"slices"
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
@ -502,7 +503,7 @@ func (e *serveEnv) applyWebServe(sc *ipn.ServeConfig, dnsName string, srvPort ui
} }
h.Path = target h.Path = target
default: default:
t, err := expandProxyTargetDev(target) t, err := expandProxyTargetDev(target, []string{"http", "https", "https+insecure"}, "http")
if err != nil { if err != nil {
return err return err
} }
@ -552,34 +553,22 @@ func (e *serveEnv) applyTCPServe(sc *ipn.ServeConfig, dnsName string, srcType se
return fmt.Errorf("invalid TCP target %q", target) return fmt.Errorf("invalid TCP target %q", target)
} }
dstURL, err := url.Parse(target) targetURL, err := expandProxyTargetDev(target, []string{"tcp"}, "tcp")
if err != nil { if err != nil {
return fmt.Errorf("invalid TCP target %q: %v", target, err) return fmt.Errorf("unable to expand target: %v", err)
} }
host, dstPortStr, err := net.SplitHostPort(dstURL.Host)
dstURL, err := url.Parse(targetURL)
if err != nil { if err != nil {
return fmt.Errorf("invalid TCP target %q: %v", target, err) return fmt.Errorf("invalid TCP target %q: %v", target, err)
} }
switch host {
case "localhost", "127.0.0.1":
// ok
default:
return fmt.Errorf("invalid TCP target %q, must be one of localhost or 127.0.0.1", target)
}
if p, err := strconv.ParseUint(dstPortStr, 10, 16); p == 0 || err != nil {
return fmt.Errorf("invalid port %q", dstPortStr)
}
fwdAddr := "127.0.0.1:" + dstPortStr
// TODO: needs to account for multiple configs from foreground mode // TODO: needs to account for multiple configs from foreground mode
if sc.IsServingWeb(srcPort) { if sc.IsServingWeb(srcPort) {
return fmt.Errorf("cannot serve TCP; already serving web on %d", srcPort) return fmt.Errorf("cannot serve TCP; already serving web on %d", srcPort)
} }
mak.Set(&sc.TCP, srcPort, &ipn.TCPPortHandler{TCPForward: fwdAddr}) mak.Set(&sc.TCP, srcPort, &ipn.TCPPortHandler{TCPForward: dstURL.Host})
if terminateTLS { if terminateTLS {
sc.TCP[srcPort].TerminateTLS = dnsName sc.TCP[srcPort].TerminateTLS = dnsName
@ -739,24 +728,22 @@ func (e *serveEnv) removeTCPServe(sc *ipn.ServeConfig, src uint16) error {
// examples: // examples:
// - 3000 // - 3000
// - localhost:3000 // - localhost:3000
// - tcp://localhost:3000
// - http://localhost:3000 // - http://localhost:3000
// - https://localhost:3000 // - https://localhost:3000
// - https-insecure://localhost:3000 // - https-insecure://localhost:3000
// - https-insecure://localhost:3000/foo // - https-insecure://localhost:3000/foo
func expandProxyTargetDev(target string) (string, error) { func expandProxyTargetDev(target string, supportedSchemes []string, defaultScheme string) (string, error) {
var ( var host = "127.0.0.1"
scheme = "http"
host = "127.0.0.1"
)
// support target being a port number // support target being a port number
if port, err := strconv.ParseUint(target, 10, 16); err == nil { if port, err := strconv.ParseUint(target, 10, 16); err == nil {
return fmt.Sprintf("%s://%s:%d", scheme, host, port), nil return fmt.Sprintf("%s://%s:%d", defaultScheme, host, port), nil
} }
// prepend scheme if not present // prepend scheme if not present
if !strings.Contains(target, "://") { if !strings.Contains(target, "://") {
target = scheme + "://" + target target = defaultScheme + "://" + target
} }
// make sure we can parse the target // make sure we can parse the target
@ -766,10 +753,8 @@ func expandProxyTargetDev(target string) (string, error) {
} }
// ensure a supported scheme // ensure a supported scheme
switch u.Scheme { if !slices.Contains(supportedSchemes, u.Scheme) {
case "http", "https", "https+insecure": return "", fmt.Errorf("must be a URL starting with one of the supported schemes: %v", supportedSchemes)
default:
return "", errors.New("must be a URL starting with http://, https://, or https+insecure://")
} }
// validate the port // validate the port

@ -367,10 +367,6 @@ func TestServeDevConfigMutations(t *testing.T) {
// // tcp // // tcp
add(step{reset: true}) add(step{reset: true})
add(step{ // must include scheme for tcp
command: cmd("serve --tls-terminated-tcp=443 --bg localhost:5432"),
wantErr: exactErr(errHelp, "errHelp"),
})
add(step{ // !somehost, must be localhost or 127.0.0.1 add(step{ // !somehost, must be localhost or 127.0.0.1
command: cmd("serve --tls-terminated-tcp=443 --bg tcp://somehost:5432"), command: cmd("serve --tls-terminated-tcp=443 --bg tcp://somehost:5432"),
wantErr: exactErr(errHelp, "errHelp"), wantErr: exactErr(errHelp, "errHelp"),
@ -383,6 +379,18 @@ func TestServeDevConfigMutations(t *testing.T) {
command: cmd("serve --tls-terminated-tcp=443 --bg tcp://somehost:65536"), command: cmd("serve --tls-terminated-tcp=443 --bg tcp://somehost:65536"),
wantErr: exactErr(errHelp, "errHelp"), wantErr: exactErr(errHelp, "errHelp"),
}) })
add(step{ // support shorthand
command: cmd("serve --tls-terminated-tcp=443 --bg 5432"),
want: &ipn.ServeConfig{
TCP: map[uint16]*ipn.TCPPortHandler{
443: {
TCPForward: "127.0.0.1:5432",
TerminateTLS: "foo.test.ts.net",
},
},
},
})
add(step{reset: true})
add(step{ add(step{
command: cmd("serve --tls-terminated-tcp=443 --bg tcp://localhost:5432"), command: cmd("serve --tls-terminated-tcp=443 --bg tcp://localhost:5432"),
want: &ipn.ServeConfig{ want: &ipn.ServeConfig{
@ -956,28 +964,43 @@ func TestSrcTypeFromFlags(t *testing.T) {
func TestExpandProxyTargetDev(t *testing.T) { func TestExpandProxyTargetDev(t *testing.T) {
tests := []struct { tests := []struct {
input string name string
expected string input string
wantErr bool defaultScheme string
supportedSchemes []string
expected string
wantErr bool
}{ }{
{input: "8080", expected: "http://127.0.0.1:8080"}, {name: "port-only", input: "8080", expected: "http://127.0.0.1:8080"},
{input: "localhost:8080", expected: "http://127.0.0.1:8080"}, {name: "hostname+port", input: "localhost:8080", expected: "http://127.0.0.1:8080"},
{input: "http://localhost:8080", expected: "http://127.0.0.1:8080"}, {name: "convert-localhost", input: "http://localhost:8080", expected: "http://127.0.0.1:8080"},
{input: "http://127.0.0.1:8080", expected: "http://127.0.0.1:8080"}, {name: "no-change", input: "http://127.0.0.1:8080", expected: "http://127.0.0.1:8080"},
{input: "http://127.0.0.1:8080/foo", expected: "http://127.0.0.1:8080/foo"}, {name: "include-path", input: "http://127.0.0.1:8080/foo", expected: "http://127.0.0.1:8080/foo"},
{input: "https://localhost:8080", expected: "https://127.0.0.1:8080"}, {name: "https-scheme", input: "https://localhost:8080", expected: "https://127.0.0.1:8080"},
{input: "https+insecure://localhost:8080", expected: "https+insecure://127.0.0.1:8080"}, {name: "https+insecure-scheme", input: "https+insecure://localhost:8080", expected: "https+insecure://127.0.0.1:8080"},
{name: "change-default-scheme", input: "localhost:8080", defaultScheme: "https", expected: "https://127.0.0.1:8080"},
{name: "change-supported-schemes", input: "localhost:8080", defaultScheme: "tcp", supportedSchemes: []string{"tcp"}, expected: "tcp://127.0.0.1:8080"},
// errors // errors
{input: "localhost:9999999", wantErr: true}, {name: "invalid-port", input: "localhost:9999999", wantErr: true},
{input: "ftp://localhost:8080", expected: "", wantErr: true}, {name: "unsupported-scheme", input: "ftp://localhost:8080", expected: "", wantErr: true},
{input: "https://tailscale.com:8080", expected: "", wantErr: true}, {name: "not-localhost", input: "https://tailscale.com:8080", expected: "", wantErr: true},
{input: "", expected: "", wantErr: true}, {name: "empty-input", input: "", expected: "", wantErr: true},
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) { defaultScheme := "http"
actual, err := expandProxyTargetDev(tt.input) supportedSchemes := []string{"http", "https", "https+insecure"}
if tt.supportedSchemes != nil {
supportedSchemes = tt.supportedSchemes
}
if tt.defaultScheme != "" {
defaultScheme = tt.defaultScheme
}
t.Run(tt.name, func(t *testing.T) {
actual, err := expandProxyTargetDev(tt.input, supportedSchemes, defaultScheme)
if tt.wantErr == true && err == nil { if tt.wantErr == true && err == nil {
t.Errorf("Expected an error but got none") t.Errorf("Expected an error but got none")

Loading…
Cancel
Save