Skip to content

Commit 9e10ef2

Browse files
jaxxstormjaxxstorm
committed
generate docs and address feedback
Signed-off-by: Lee Briggs <lee@leebriggs.co.uk>
1 parent 8148193 commit 9e10ef2

7 files changed

Lines changed: 84 additions & 29 deletions

File tree

‎cmd/tscli/set/dns/nameservers/cli.go‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
// cmd/tscli/set/nameservers/cli.go
1+
// cmd/tscli/set/dns/nameservers/cli.go
22
//
3-
// `tscli set nameservers --nameserver 1.1.1.1 --nameserver https://dns.google/dns-query`
3+
// `tscli set dns nameservers --nameserver 1.1.1.1 --nameserver https://dns.google/dns-query`
44
// Replace the tailnet-wide DNS nameserver list with IPs or DoH endpoints.
55
//
6-
// If you pass an empty slice (`--nameserver ""`) the custom list is removed
7-
// and Tailscale falls back to its defaults.
6+
// If you pass `--nameserver ""`, the custom list is removed and Tailscale
7+
// falls back to its defaults.
88
package nameservers
99

1010
import (
@@ -29,7 +29,7 @@ func Command() *cobra.Command {
2929
Aliases: []string{"ns"},
3030
Short: "Set the DNS nameservers for the tailnet",
3131
PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
32-
if len(ns) == 0 {
32+
if !cmd.Flags().Lookup("nameserver").Changed {
3333
return fmt.Errorf("at least one --nameserver is required")
3434
}
3535
for _, nameserver := range ns {
@@ -46,7 +46,12 @@ func Command() *cobra.Command {
4646
return err
4747
}
4848

49-
body := map[string][]string{"dns": ns}
49+
nameservers := ns
50+
if len(nameservers) == 0 {
51+
nameservers = []string{}
52+
}
53+
54+
body := map[string][]string{"dns": nameservers}
5055

5156
var resp json.RawMessage // <- receives the body untouched
5257
if _, err := tscli.Do(
@@ -70,7 +75,7 @@ func Command() *cobra.Command {
7075
&ns,
7176
"nameserver", "N",
7277
nil,
73-
"DNS nameserver IP or DoH endpoint (repeatable). Example: --nameserver 1.1.1.1 --nameserver https://dns.google/dns-query",
78+
"DNS nameserver IP or DoH endpoint (repeatable). Use --nameserver \"\" to clear the custom list.",
7479
)
7580
_ = cmd.MarkFlagRequired("nameserver")
7681

‎cmd/tscli/set/dns/split/cli.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
1-
// cmd/tscli/set/splitdns/cli.go
1+
// cmd/tscli/set/dns/split/cli.go
22
//
33
// Replace *or* patch the split-DNS mapping.
44
//
55
// # add two nameservers for example.com, one DoH endpoint for other.com
6-
// tscli set splitdns \
6+
// tscli set dns split-dns \
77
// --entry example.com=1.1.1.1 \
88
// --entry example.com=8.8.8.8 \
99
// --entry other.com=https://dns.google/dns-query
1010
//
1111
// # clear a single domain (entry with empty RHS)
12-
// tscli set splitdns --entry stale.com=
12+
// tscli set dns split-dns --entry stale.com=
1313
//
1414
// # replace the whole mapping (PUT, drop everything else)
15-
// tscli set splitdns --replace --entry corp.local=10.0.0.53
15+
// tscli set dns split-dns --replace --entry corp.local=10.0.0.53
1616
package split
1717

1818
import (
@@ -36,7 +36,7 @@ var domainRE = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9\.\-]*\.[a-zA-Z]{2,}$`)
3636
// Command registers `tscli set dns split-dns`.
3737
func Command() *cobra.Command {
3838
var (
39-
entries []string // domain=ip (repeatable, one per IP)
39+
entries []string // domain=nameserver (repeatable, one per nameserver)
4040
replace bool
4141
)
4242

@@ -47,7 +47,7 @@ func Command() *cobra.Command {
4747

4848
PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
4949
if !cmd.Flags().Lookup("entry").Changed {
50-
return fmt.Errorf("at least one --entry is required (domain=ip)")
50+
return fmt.Errorf("at least one --entry is required (domain=nameserver)")
5151
}
5252
return nil
5353
},
@@ -61,24 +61,24 @@ func Command() *cobra.Command {
6161
for _, e := range entries {
6262
parts := strings.SplitN(e, "=", 2)
6363
if len(parts) != 2 {
64-
return fmt.Errorf("invalid --entry %q (expect domain=ip)", e)
64+
return fmt.Errorf("invalid --entry %q (expect domain=nameserver)", e)
6565
}
66-
dom, ip := strings.ToLower(parts[0]), parts[1]
66+
dom, nameserver := strings.ToLower(parts[0]), parts[1]
6767

6868
if !domainRE.MatchString(dom) {
6969
return fmt.Errorf("invalid domain: %q", dom)
7070
}
7171

7272
// empty RHS → clear this domain
73-
if ip == "" {
73+
if nameserver == "" {
7474
payload[dom] = nil
7575
continue
7676
}
7777

78-
if err := cldns.ValidateNameserver(ip); err != nil {
79-
return fmt.Errorf("invalid nameserver %q for %s", ip, dom)
78+
if err := cldns.ValidateNameserver(nameserver); err != nil {
79+
return fmt.Errorf("invalid nameserver %q for %q", nameserver, dom)
8080
}
81-
tmp[dom] = append(tmp[dom], ip)
81+
tmp[dom] = append(tmp[dom], nameserver)
8282
}
8383

8484
// merge temp slices unless domain already set to nil
@@ -122,7 +122,7 @@ func Command() *cobra.Command {
122122
&entries,
123123
"entry", "e",
124124
nil,
125-
`Mapping "domain=nameserver". Repeat --entry for multiple IP/DoH values or domains. Set an empty value to clear.`,
125+
`Mapping "domain=nameserver". Repeat --entry for multiple IP or DoH endpoint values. Set an empty value to clear.`,
126126
)
127127
cmd.Flags().BoolVar(&replace, "replace", false,
128128
"Replace the entire mapping (PUT) instead of patching (PATCH).")

‎docs/commands/tscli_set_dns_nameservers.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ tscli set dns nameservers [flags]
1212

1313
```
1414
-h, --help help for nameservers
15-
-N, --nameserver strings DNS nameserver IP (repeatable). Example: --nameserver 1.1.1.1 --nameserver 8.8.8.8
15+
-N, --nameserver strings DNS nameserver IP or DoH endpoint (repeatable). Use --nameserver "" to clear the custom list.
1616
```
1717

1818
### Options inherited from parent commands

‎docs/commands/tscli_set_dns_split-dns.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ tscli set dns split-dns [flags]
1111
### Options
1212

1313
```
14-
-e, --entry stringArray Mapping "domain=ip". Repeat --entry for multiple IPs or domains. Set an empty IP to clear.
14+
-e, --entry stringArray Mapping "domain=nameserver". Repeat --entry for multiple IP or DoH endpoint values. Set an empty value to clear.
1515
-h, --help help for split-dns
1616
--replace Replace the entire mapping (PUT) instead of patching (PATCH).
1717
```

‎pkg/dns/nameserver.go‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,44 @@ import (
44
"fmt"
55
"net"
66
"net/url"
7+
"strconv"
78
"strings"
89
)
910

1011
// ValidateNameserver accepts either a literal IP address or a valid HTTPS DoH endpoint.
1112
func ValidateNameserver(value string) error {
13+
if value == "" {
14+
return nil
15+
}
16+
1217
if net.ParseIP(value) != nil {
1318
return nil
1419
}
1520

1621
parsed, err := url.Parse(value)
1722
if err != nil {
18-
return fmt.Errorf("invalid nameserver: %s", value)
23+
return invalidNameserverError(value)
1924
}
2025
if parsed.Scheme != "https" {
21-
return fmt.Errorf("invalid nameserver: %s", value)
26+
return invalidNameserverError(value)
2227
}
2328
if parsed.Host == "" || parsed.Hostname() == "" {
24-
return fmt.Errorf("invalid nameserver: %s", value)
29+
return invalidNameserverError(value)
2530
}
2631
if strings.Contains(parsed.Host, " ") {
27-
return fmt.Errorf("invalid nameserver: %s", value)
32+
return invalidNameserverError(value)
33+
}
34+
35+
if parsed.Port() != "" {
36+
port, err := strconv.ParseUint(parsed.Port(), 10, 16)
37+
if err != nil || port > 65535 {
38+
return invalidNameserverError(value)
39+
}
2840
}
2941

3042
return nil
3143
}
44+
45+
func invalidNameserverError(value string) error {
46+
return fmt.Errorf("invalid nameserver %q", value)
47+
}

‎pkg/dns/nameserver_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ func TestValidateNameserver(t *testing.T) {
1212
}{
1313
{name: "ipv4", value: "1.1.1.1", valid: true},
1414
{name: "ipv6", value: "2606:4700:4700::1111", valid: true},
15+
{name: "empty clears list", value: "", valid: true},
1516
{name: "doh base", value: "https://dns.google/dns-query", valid: true},
1617
{name: "doh with path and query", value: "https://dns.nextdns.io/abcd12?device=test", valid: true},
1718
{name: "http rejected", value: "http://dns.google/dns-query", valid: false},
19+
{name: "invalid port rejected", value: "https://dns.google:65536/dns-query", valid: false},
1820
{name: "missing host rejected", value: "https:///dns-query", valid: false},
1921
{name: "nonsense rejected", value: "not-a-nameserver", valid: false},
2022
}

‎test/cli/dns_nameserver_validation_test.go‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,24 @@ func TestSetDNSNameserversValidation(t *testing.T) {
2222
name: "accepts doh nameserver",
2323
args: []string{"set", "dns", "nameservers", "--nameserver", "https://dns.google/dns-query"},
2424
},
25+
{
26+
name: "accepts empty nameserver to clear list",
27+
args: []string{"set", "dns", "nameservers", "--nameserver", ""},
28+
},
2529
{
2630
name: "rejects malformed nameserver",
2731
args: []string{"set", "dns", "nameservers", "--nameserver", "dns.google"},
28-
errContains: "invalid nameserver",
32+
errContains: `invalid nameserver "dns.google"`,
2933
},
3034
{
3135
name: "rejects non-https doh nameserver",
3236
args: []string{"set", "dns", "nameservers", "--nameserver", "http://dns.google/dns-query"},
33-
errContains: "invalid nameserver",
37+
errContains: `invalid nameserver "http://dns.google/dns-query"`,
38+
},
39+
{
40+
name: "rejects out of range doh port",
41+
args: []string{"set", "dns", "nameservers", "--nameserver", "https://dns.google:65536/dns-query"},
42+
errContains: `invalid nameserver "https://dns.google:65536/dns-query"`,
3443
},
3544
}
3645

@@ -71,7 +80,12 @@ func TestSetDNSSplitValidation(t *testing.T) {
7180
{
7281
name: "rejects invalid nameserver entry",
7382
args: []string{"set", "dns", "split-dns", "--entry", "corp.example.com=dns.google"},
74-
errContains: "invalid nameserver",
83+
errContains: `invalid nameserver "dns.google"`,
84+
},
85+
{
86+
name: "rejects malformed entry shape",
87+
args: []string{"set", "dns", "split-dns", "--entry", "corp.example.com"},
88+
errContains: `expect domain=nameserver`,
7589
},
7690
}
7791

@@ -120,6 +134,24 @@ func TestSetDNSNameserverDoHValuesReachAPI(t *testing.T) {
120134
}
121135
})
122136

137+
t.Run("nameservers clear list", func(t *testing.T) {
138+
mock := apimock.New(t)
139+
mock.AddJSON(http.MethodPost, "/dns/nameservers", http.StatusOK, map[string]any{"dns": []string{}})
140+
141+
res := executeCLI(t, []string{"set", "dns", "nameservers", "--nameserver", ""}, map[string]string{"TSCLI_BASE_URL": mock.URL()})
142+
if res.err != nil {
143+
t.Fatalf("unexpected error: %v\nstderr:\n%s", res.err, res.stderr)
144+
}
145+
146+
reqs := mock.Requests()
147+
if len(reqs) == 0 {
148+
t.Fatalf("expected request to mock API, got none")
149+
}
150+
if !strings.Contains(reqs[0].Body, `"dns":[]`) {
151+
t.Fatalf("expected request body to send an empty list, got %s", reqs[0].Body)
152+
}
153+
})
154+
123155
t.Run("split dns", func(t *testing.T) {
124156
mock := apimock.New(t)
125157
mock.AddJSON(http.MethodPatch, "/dns/split-dns", http.StatusOK, apimock.DNSSplitConfig())

0 commit comments

Comments
 (0)