Skip to content

Commit 704f29f

Browse files
ethanndicksonethanndickson
committed
review
1 parent a1f980f commit 704f29f

File tree

2 files changed

+103
-27
lines changed

2 files changed

+103
-27
lines changed

‎vpn/dns.go

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,54 +2,57 @@ package vpn
22

33
import "tailscale.com/net/dns"
44

5-
func NewVPNManager(t *Tunnel) dns.OSConfigurator {
6-
return &vpnDNSManager{tunnel: t}
5+
func NewDNSConfigurator(t *Tunnel) dns.OSConfigurator {
6+
return &dnsManager{tunnel: t}
77
}
88

9-
type vpnDNSManager struct {
9+
type dnsManager struct {
1010
tunnel *Tunnel
1111
}
1212

13-
func (v *vpnDNSManager) SetDNS(cfg dns.OSConfig) error {
14-
servers := make([]string, 0, len(cfg.Nameservers))
15-
for _, ns := range cfg.Nameservers {
16-
servers = append(servers, ns.String())
17-
}
18-
searchDomains := make([]string, 0, len(cfg.SearchDomains))
19-
for _, domain := range cfg.SearchDomains {
20-
searchDomains = append(searchDomains, domain.WithoutTrailingDot())
21-
}
22-
matchDomains := make([]string, 0, len(cfg.MatchDomains))
23-
for _, domain := range cfg.MatchDomains {
24-
matchDomains = append(matchDomains, domain.WithoutTrailingDot())
25-
}
13+
func (v *dnsManager) SetDNS(cfg dns.OSConfig) error {
14+
settings := convertDNSConfig(cfg)
2615
return v.tunnel.ApplyNetworkSettings(v.tunnel.ctx, &NetworkSettingsRequest{
27-
DnsSettings: &NetworkSettingsRequest_DNSSettings{
28-
Servers: servers,
29-
SearchDomains: searchDomains,
30-
DomainName: "coder",
31-
MatchDomains: matchDomains,
32-
MatchDomainsNoSearch: false,
33-
},
16+
DnsSettings: settings,
3417
})
3518
}
3619

37-
func (vpnDNSManager) GetBaseConfig() (dns.OSConfig, error) {
20+
func (*dnsManager) GetBaseConfig() (dns.OSConfig, error) {
3821
// Tailscale calls this function to blend the OS's DNS configuration with
3922
// it's own, so this is only called if `SupportsSplitDNS` returns false.
4023
return dns.OSConfig{}, dns.ErrGetBaseConfigNotSupported
4124
}
4225

43-
func (*vpnDNSManager) SupportsSplitDNS() bool {
26+
func (*dnsManager) SupportsSplitDNS() bool {
4427
// macOS & Windows 10+ support split DNS, so we'll assume all CoderVPN
4528
// clients do too.
4629
return true
4730
}
4831

4932
// Close implements dns.OSConfigurator.
50-
func (*vpnDNSManager) Close() error {
33+
func (*dnsManager) Close() error {
5134
// There's no cleanup that we need to initiate from within the dylib.
5235
return nil
5336
}
5437

55-
var _ dns.OSConfigurator = (*vpnDNSManager)(nil)
38+
func convertDNSConfig(cfg dns.OSConfig) *NetworkSettingsRequest_DNSSettings {
39+
servers := make([]string, 0, len(cfg.Nameservers))
40+
for _, ns := range cfg.Nameservers {
41+
servers = append(servers, ns.String())
42+
}
43+
searchDomains := make([]string, 0, len(cfg.SearchDomains))
44+
for _, domain := range cfg.SearchDomains {
45+
searchDomains = append(searchDomains, domain.WithoutTrailingDot())
46+
}
47+
matchDomains := make([]string, 0, len(cfg.MatchDomains))
48+
for _, domain := range cfg.MatchDomains {
49+
matchDomains = append(matchDomains, domain.WithoutTrailingDot())
50+
}
51+
return &NetworkSettingsRequest_DNSSettings{
52+
Servers: servers,
53+
SearchDomains: searchDomains,
54+
DomainName: "coder",
55+
MatchDomains: matchDomains,
56+
MatchDomainsNoSearch: false,
57+
}
58+
}

‎vpn/dns_internal_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package vpn
2+
3+
import (
4+
"net/netip"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
"tailscale.com/net/dns"
9+
"tailscale.com/util/dnsname"
10+
)
11+
12+
func TestConvertDNSConfig(t *testing.T) {
13+
t.Parallel()
14+
15+
tests := []struct {
16+
name string
17+
input dns.OSConfig
18+
expected *NetworkSettingsRequest_DNSSettings
19+
}{
20+
{
21+
name: "Basic",
22+
input: dns.OSConfig{
23+
Nameservers: []netip.Addr{
24+
netip.MustParseAddr("1.1.1.1"),
25+
netip.MustParseAddr("8.8.8.8"),
26+
},
27+
SearchDomains: []dnsname.FQDN{
28+
"example.com.",
29+
"test.local.",
30+
},
31+
MatchDomains: []dnsname.FQDN{
32+
"internal.domain.",
33+
},
34+
},
35+
expected: &NetworkSettingsRequest_DNSSettings{
36+
Servers: []string{"1.1.1.1", "8.8.8.8"},
37+
SearchDomains: []string{"example.com", "test.local"},
38+
DomainName: "coder",
39+
MatchDomains: []string{"internal.domain"},
40+
MatchDomainsNoSearch: false,
41+
},
42+
},
43+
{
44+
name: "Empty",
45+
input: dns.OSConfig{
46+
Nameservers: []netip.Addr{},
47+
SearchDomains: []dnsname.FQDN{},
48+
MatchDomains: []dnsname.FQDN{},
49+
},
50+
expected: &NetworkSettingsRequest_DNSSettings{
51+
Servers: []string{},
52+
SearchDomains: []string{},
53+
DomainName: "coder",
54+
MatchDomains: []string{},
55+
MatchDomainsNoSearch: false,
56+
},
57+
},
58+
}
59+
60+
//nolint:paralleltest // outdated rule
61+
for _, tt := range tests {
62+
t.Run(tt.name, func(t *testing.T) {
63+
t.Parallel()
64+
65+
result := convertDNSConfig(tt.input)
66+
require.Equal(t, tt.expected.Servers, result.Servers)
67+
require.Equal(t, tt.expected.SearchDomains, result.SearchDomains)
68+
require.Equal(t, tt.expected.DomainName, result.DomainName)
69+
require.Equal(t, tt.expected.MatchDomains, result.MatchDomains)
70+
require.Equal(t, tt.expected.MatchDomainsNoSearch, result.MatchDomainsNoSearch)
71+
})
72+
}
73+
}

0 commit comments

Comments
 (0)