diff --git a/CHANGELOG.md b/CHANGELOG.md
index 709e8fe0..02f1daed 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,9 +21,33 @@ and this project adheres to
### Changed
+- The setting `local_domain_name` is now in the `dhcp` block in the
+ configuration file to avoid confusion ([#3367]).
- The `dns.bogus_nxdomain` configuration file parameter now supports CIDR
notation alongside IP addresses ([#1730]).
+#### Configuration Changes
+
+In this release, the schema version has changed from 12 to 13.
+
+- Parameter `local_domain_name`, which in schema versions 12 and earlier used to
+ be a part of the `dns` object, is now a part of the `dhcp` object:
+
+ ```yaml
+ # BEFORE:
+ 'dns':
+ # …
+ 'local_domain_name': 'lan'
+
+ # AFTER:
+ 'dhcp':
+ # …
+ 'local_domain_name': 'lan'
+ ```
+
+ To rollback this change, move the parameter back into `dns` and change the
+ `schema_version` back to `12`.
+
### Deprecated
<!--
@@ -38,6 +62,7 @@ and this project adheres to
[#1730]: https://github.com/AdguardTeam/AdGuardHome/issues/1730
[#3057]: https://github.com/AdguardTeam/AdGuardHome/issues/3057
+[#3367]: https://github.com/AdguardTeam/AdGuardHome/issues/3367
diff --git a/internal/aghnet/dhcp_unix.go b/internal/aghnet/dhcp_unix.go
index 554d68c6..fd2b9b93 100644
--- a/internal/aghnet/dhcp_unix.go
+++ b/internal/aghnet/dhcp_unix.go
@@ -111,7 +111,7 @@ func discover4(iface *net.Interface, dstAddr *net.UDPAddr, hostname string) (ok
// is spoiled.
//
// It's also known that listening on the specified interface's address
- // ignores broadcasted packets when reading.
+ // ignores broadcast packets when reading.
var c net.PacketConn
if c, err = listenPacketReusable(iface.Name, "udp4", ":68"); err != nil {
return false, fmt.Errorf("couldn't listen on :68: %w", err)
diff --git a/internal/dhcpd/dhcpd.go b/internal/dhcpd/dhcpd.go
index 341aa3d6..55c56c18 100644
--- a/internal/dhcpd/dhcpd.go
+++ b/internal/dhcpd/dhcpd.go
@@ -119,23 +119,28 @@ func (l *Lease) UnmarshalJSON(data []byte) (err error) {
return nil
}
-// ServerConfig - DHCP server configuration
-// field ordering is important -- yaml fields will mirror ordering from here
+// ServerConfig is the configuration for the DHCP server. The order of YAML
+// fields is important, since the YAML configuration file follows it.
type ServerConfig struct {
- Enabled bool `yaml:"enabled"`
- InterfaceName string `yaml:"interface_name"`
-
- Conf4 V4ServerConf `yaml:"dhcpv4"`
- Conf6 V6ServerConf `yaml:"dhcpv6"`
-
- WorkDir string `yaml:"-"`
- DBFilePath string `yaml:"-"` // path to DB file
-
// Called when the configuration is changed by HTTP request
ConfigModified func() `yaml:"-"`
// Register an HTTP handler
HTTPRegister func(string, string, func(http.ResponseWriter, *http.Request)) `yaml:"-"`
+
+ Enabled bool `yaml:"enabled"`
+ InterfaceName string `yaml:"interface_name"`
+
+ // LocalDomainName is the domain name used for DHCP hosts. For example,
+ // a DHCP client with the hostname "myhost" can be addressed as "myhost.lan"
+ // when LocalDomainName is "lan".
+ LocalDomainName string `yaml:"local_domain_name"`
+
+ Conf4 V4ServerConf `yaml:"dhcpv4"`
+ Conf6 V6ServerConf `yaml:"dhcpv6"`
+
+ WorkDir string `yaml:"-"`
+ DBFilePath string `yaml:"-"`
}
// OnLeaseChangedT is a callback for lease changes.
@@ -156,7 +161,9 @@ type Server struct {
srv4 DHCPServer
srv6 DHCPServer
- conf ServerConfig
+ // TODO(a.garipov): Either create a separate type for the internal config or
+ // just put the config values into Server.
+ conf *ServerConfig
// Called when the leases DB is modified
onLeaseChanged []OnLeaseChangedT
@@ -181,14 +188,21 @@ type ServerInterface interface {
}
// Create - create object
-func Create(conf ServerConfig) (s *Server, err error) {
- s = &Server{}
+func Create(conf *ServerConfig) (s *Server, err error) {
+ s = &Server{
+ conf: &ServerConfig{
+ ConfigModified: conf.ConfigModified,
- s.conf.Enabled = conf.Enabled
- s.conf.InterfaceName = conf.InterfaceName
- s.conf.HTTPRegister = conf.HTTPRegister
- s.conf.ConfigModified = conf.ConfigModified
- s.conf.DBFilePath = filepath.Join(conf.WorkDir, dbFilename)
+ HTTPRegister: conf.HTTPRegister,
+
+ Enabled: conf.Enabled,
+ InterfaceName: conf.InterfaceName,
+
+ LocalDomainName: conf.LocalDomainName,
+
+ DBFilePath: filepath.Join(conf.WorkDir, dbFilename),
+ },
+ }
if !webHandlersRegistered && s.conf.HTTPRegister != nil {
if runtime.GOOS == "windows" {
@@ -305,6 +319,7 @@ func (s *Server) notify(flags int) {
func (s *Server) WriteDiskConfig(c *ServerConfig) {
c.Enabled = s.conf.Enabled
c.InterfaceName = s.conf.InterfaceName
+ c.LocalDomainName = s.conf.LocalDomainName
s.srv4.WriteDiskConfig4(&c.Conf4)
s.srv6.WriteDiskConfig6(&c.Conf6)
}
diff --git a/internal/dhcpd/dhcpd_test.go b/internal/dhcpd/dhcpd_test.go
index b8fc5fa0..b704cbb4 100644
--- a/internal/dhcpd/dhcpd_test.go
+++ b/internal/dhcpd/dhcpd_test.go
@@ -27,7 +27,7 @@ func testNotify(flags uint32) {
func TestDB(t *testing.T) {
var err error
s := Server{
- conf: ServerConfig{
+ conf: &ServerConfig{
DBFilePath: dbFilename,
},
}
@@ -140,27 +140,27 @@ func TestNormalizeLeases(t *testing.T) {
func TestV4Server_badRange(t *testing.T) {
testCases := []struct {
name string
+ wantErrMsg string
gatewayIP net.IP
subnetMask net.IP
- wantErrMsg string
}{{
- name: "gateway_in_range",
- gatewayIP: net.IP{192, 168, 10, 120},
- subnetMask: net.IP{255, 255, 255, 0},
+ name: "gateway_in_range",
wantErrMsg: "dhcpv4: gateway ip 192.168.10.120 in the ip range: " +
"192.168.10.20-192.168.10.200",
+ gatewayIP: net.IP{192, 168, 10, 120},
+ subnetMask: net.IP{255, 255, 255, 0},
}, {
- name: "outside_range_start",
- gatewayIP: net.IP{192, 168, 10, 1},
- subnetMask: net.IP{255, 255, 255, 240},
+ name: "outside_range_start",
wantErrMsg: "dhcpv4: range start 192.168.10.20 is outside network " +
"192.168.10.1/28",
- }, {
- name: "outside_range_end",
gatewayIP: net.IP{192, 168, 10, 1},
- subnetMask: net.IP{255, 255, 255, 224},
+ subnetMask: net.IP{255, 255, 255, 240},
+ }, {
+ name: "outside_range_end",
wantErrMsg: "dhcpv4: range end 192.168.10.200 is outside network " +
"192.168.10.1/27",
+ gatewayIP: net.IP{192, 168, 10, 1},
+ subnetMask: net.IP{255, 255, 255, 224},
}}
for _, tc := range testCases {
diff --git a/internal/dhcpd/http.go b/internal/dhcpd/http.go
index 78016010..e340addb 100644
--- a/internal/dhcpd/http.go
+++ b/internal/dhcpd/http.go
@@ -575,12 +575,15 @@ func (s *Server) handleReset(w http.ResponseWriter, r *http.Request) {
log.Error("dhcp: removing db: %s", err)
}
- oldconf := s.conf
- s.conf = ServerConfig{
- WorkDir: oldconf.WorkDir,
- HTTPRegister: oldconf.HTTPRegister,
- ConfigModified: oldconf.ConfigModified,
- DBFilePath: oldconf.DBFilePath,
+ s.conf = &ServerConfig{
+ ConfigModified: s.conf.ConfigModified,
+
+ HTTPRegister: s.conf.HTTPRegister,
+
+ LocalDomainName: s.conf.LocalDomainName,
+
+ WorkDir: s.conf.WorkDir,
+ DBFilePath: s.conf.DBFilePath,
}
v4conf := V4ServerConf{
diff --git a/internal/dhcpd/nullbool_test.go b/internal/dhcpd/nullbool_test.go
index 32c9f9f4..549df608 100644
--- a/internal/dhcpd/nullbool_test.go
+++ b/internal/dhcpd/nullbool_test.go
@@ -12,33 +12,33 @@ import (
func TestNullBool_UnmarshalJSON(t *testing.T) {
testCases := []struct {
name string
- data []byte
wantErrMsg string
+ data []byte
want nullBool
}{{
name: "empty",
- data: []byte{},
wantErrMsg: "",
+ data: []byte{},
want: nbNull,
}, {
name: "null",
- data: []byte("null"),
wantErrMsg: "",
+ data: []byte("null"),
want: nbNull,
}, {
name: "true",
- data: []byte("true"),
wantErrMsg: "",
+ data: []byte("true"),
want: nbTrue,
}, {
name: "false",
- data: []byte("false"),
wantErrMsg: "",
+ data: []byte("false"),
want: nbFalse,
}, {
name: "invalid",
- data: []byte("flase"),
- wantErrMsg: `invalid nullBool value "flase"`,
+ wantErrMsg: `invalid nullBool value "invalid"`,
+ data: []byte("invalid"),
want: nbNull,
}}
diff --git a/internal/dhcpd/v4.go b/internal/dhcpd/v4.go
index e7dc0571..4e2a3a25 100644
--- a/internal/dhcpd/v4.go
+++ b/internal/dhcpd/v4.go
@@ -969,11 +969,10 @@ func (s *v4Server) send(peer net.Addr, conn net.PacketConn, req, resp *dhcpv4.DH
Port: dhcpv4.ServerPort,
}
if mtype == dhcpv4.MessageTypeNak {
- // Set the broadcast bit in the DHCPNAK, so that the
- // relay agent broadcasted it to the client, because the
- // client may not have a correct network address or
- // subnet mask, and the client may not be answering ARP
- // requests.
+ // Set the broadcast bit in the DHCPNAK, so that the relay agent
+ // broadcasts it to the client, because the client may not have
+ // a correct network address or subnet mask, and the client may not
+ // be answering ARP requests.
resp.SetBroadcast()
}
case mtype == dhcpv4.MessageTypeNak:
diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go
index d9e04b80..c11171d9 100644
--- a/internal/dnsforward/dns.go
+++ b/internal/dnsforward/dns.go
@@ -215,9 +215,8 @@ func (s *Server) onDHCPLeaseChanged(flags int) {
ipToHost = netutil.NewIPMap(len(ll))
for _, l := range ll {
- // TODO(a.garipov): Remove this after we're finished
- // with the client hostname validations in the DHCP
- // server code.
+ // TODO(a.garipov): Remove this after we're finished with the client
+ // hostname validations in the DHCP server code.
err = netutil.ValidateDomainName(l.Hostname)
if err != nil {
log.Debug(
@@ -301,6 +300,8 @@ func (s *Server) processInternalHosts(dctx *dnsContext) (rc resultCode) {
}
reqHost := strings.ToLower(q.Name)
+ // TODO(a.garipov): Move everything related to DHCP local domain to the DHCP
+ // server.
host := strings.TrimSuffix(reqHost, s.localDomainSuffix)
if host == reqHost {
return resultCodeSuccess
diff --git a/internal/home/clients_test.go b/internal/home/clients_test.go
index 93bf3360..6cc5d46f 100644
--- a/internal/home/clients_test.go
+++ b/internal/home/clients_test.go
@@ -3,10 +3,12 @@ package home
import (
"net"
"os"
+ "runtime"
"testing"
"time"
"github.com/AdguardTeam/AdGuardHome/internal/dhcpd"
+ "github.com/AdguardTeam/golibs/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -271,12 +273,18 @@ func TestClientsAddExisting(t *testing.T) {
})
t.Run("complicated", func(t *testing.T) {
+ // TODO(a.garipov): Properly decouple the DHCP server from the client
+ // storage.
+ if runtime.GOOS == "windows" {
+ t.Skip("skipping dhcp test on windows")
+ }
+
var err error
ip := net.IP{1, 2, 3, 4}
// First, init a DHCP server with a single static lease.
- config := dhcpd.ServerConfig{
+ config := &dhcpd.ServerConfig{
Enabled: true,
DBFilePath: "leases.db",
Conf4: dhcpd.V4ServerConf{
@@ -290,10 +298,9 @@ func TestClientsAddExisting(t *testing.T) {
clients.dhcpServer, err = dhcpd.Create(config)
require.NoError(t, err)
- // TODO(e.burkov): leases.db isn't created on Windows so removing it
- // causes an error. Split the test to make it run properly on different
- // operating systems.
- t.Cleanup(func() { _ = os.Remove("leases.db") })
+ testutil.CleanupAndRequireSuccess(t, func() (err error) {
+ return os.Remove("leases.db")
+ })
err = clients.dhcpServer.AddStaticLease(&dhcpd.Lease{
HWAddr: net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA},
diff --git a/internal/home/config.go b/internal/home/config.go
index a6247164..6ecb4ccb 100644
--- a/internal/home/config.go
+++ b/internal/home/config.go
@@ -83,7 +83,7 @@ type configuration struct {
WhitelistFilters []filter `yaml:"whitelist_filters"`
UserRules []string `yaml:"user_rules"`
- DHCP dhcpd.ServerConfig `yaml:"dhcp"`
+ DHCP *dhcpd.ServerConfig `yaml:"dhcp"`
// Clients contains the YAML representations of the persistent clients.
// This field is only used for reading and writing persistent client data.
@@ -123,11 +123,6 @@ type dnsConfig struct {
// UpstreamTimeout is the timeout for querying upstream servers.
UpstreamTimeout timeutil.Duration `yaml:"upstream_timeout"`
- // LocalDomainName is the domain name used for known internal hosts.
- // For example, a machine called "myhost" can be addressed as
- // "myhost.lan" when LocalDomainName is "lan".
- LocalDomainName string `yaml:"local_domain_name"`
-
// ResolveClients enables and disables resolving clients with RDNS.
ResolveClients bool `yaml:"resolve_clients"`
@@ -199,7 +194,6 @@ var config = &configuration{
FilteringEnabled: true, // whether or not use filter lists
FiltersUpdateIntervalHours: 24,
UpstreamTimeout: timeutil.Duration{Duration: dnsforward.DefaultTimeout},
- LocalDomainName: "lan",
ResolveClients: true,
UsePrivateRDNS: true,
},
@@ -208,6 +202,9 @@ var config = &configuration{
PortDNSOverTLS: defaultPortTLS, // needs to be passed through to dnsproxy
PortDNSOverQUIC: defaultPortQUIC,
},
+ DHCP: &dhcpd.ServerConfig{
+ LocalDomainName: "lan",
+ },
logSettings: logSettings{
LogCompress: false,
LogLocalTime: false,
@@ -389,8 +386,8 @@ func (c *configuration) write() error {
}
if Context.dhcpServer != nil {
- c := dhcpd.ServerConfig{}
- Context.dhcpServer.WriteDiskConfig(&c)
+ c := &dhcpd.ServerConfig{}
+ Context.dhcpServer.WriteDiskConfig(c)
config.DHCP = c
}
diff --git a/internal/home/dns.go b/internal/home/dns.go
index 8775e152..3ac7a9ab 100644
--- a/internal/home/dns.go
+++ b/internal/home/dns.go
@@ -83,7 +83,7 @@ func initDNSServer() (err error) {
QueryLog: Context.queryLog,
SubnetDetector: Context.subnetDetector,
Anonymizer: anonymizer,
- LocalDomain: config.DNS.LocalDomainName,
+ LocalDomain: config.DHCP.LocalDomainName,
}
if Context.dhcpServer != nil {
p.DHCPServer = Context.dhcpServer
diff --git a/internal/home/upgrade.go b/internal/home/upgrade.go
index 89d41556..34297470 100644
--- a/internal/home/upgrade.go
+++ b/internal/home/upgrade.go
@@ -21,7 +21,7 @@ import (
)
// currentSchemaVersion is the current schema version.
-const currentSchemaVersion = 12
+const currentSchemaVersion = 13
// These aliases are provided for convenience.
type (
@@ -85,6 +85,7 @@ func upgradeConfigSchema(oldVersion int, diskConf yobj) (err error) {
upgradeSchema9to10,
upgradeSchema10to11,
upgradeSchema11to12,
+ upgradeSchema12to13,
}
n := 0
@@ -690,6 +691,52 @@ func upgradeSchema11to12(diskConf yobj) (err error) {
return nil
}
+// upgradeSchema12to13 performs the following changes:
+//
+// # BEFORE:
+// 'dns':
+// # …
+// 'local_domain_name': 'lan'
+//
+// # AFTER:
+// 'dhcp':
+// # …
+// 'local_domain_name': 'lan'
+//
+func upgradeSchema12to13(diskConf yobj) (err error) {
+ log.Printf("Upgrade yaml: 12 to 13")
+ diskConf["schema_version"] = 13
+
+ dnsVal, ok := diskConf["dns"]
+ if !ok {
+ return nil
+ }
+
+ var dns yobj
+ dns, ok = dnsVal.(yobj)
+ if !ok {
+ return fmt.Errorf("unexpected type of dns: %T", dnsVal)
+ }
+
+ dhcpVal, ok := diskConf["dhcp"]
+ if !ok {
+ return nil
+ }
+
+ var dhcp yobj
+ dhcp, ok = dhcpVal.(yobj)
+ if !ok {
+ return fmt.Errorf("unexpected type of dhcp: %T", dnsVal)
+ }
+
+ const field = "local_domain_name"
+
+ dhcp[field] = dns[field]
+ delete(dns, field)
+
+ return nil
+}
+
// TODO(a.garipov): Replace with log.Output when we port it to our logging
// package.
func funcName() string {
diff --git a/internal/home/upgrade_test.go b/internal/home/upgrade_test.go
index 171ce3b2..c63bc443 100644
--- a/internal/home/upgrade_test.go
+++ b/internal/home/upgrade_test.go
@@ -55,7 +55,7 @@ func TestUpgradeSchema2to3(t *testing.T) {
require.Len(t, v, 1)
require.Equal(t, "8.8.8.8:53", v[0])
default:
- t.Fatalf("wrong type for bootsrap dns: %T", v)
+ t.Fatalf("wrong type for bootstrap dns: %T", v)
}
excludedEntries := []string{"bootstrap_dns"}
@@ -511,3 +511,48 @@ func TestUpgradeSchema11to12(t *testing.T) {
assert.Equal(t, 90*24*time.Hour, ivlVal.Duration)
})
}
+
+func TestUpgradeSchema12to13(t *testing.T) {
+ t.Run("no_dns", func(t *testing.T) {
+ conf := yobj{}
+
+ err := upgradeSchema12to13(conf)
+ require.NoError(t, err)
+
+ assert.Equal(t, conf["schema_version"], 13)
+ })
+
+ t.Run("no_dhcp", func(t *testing.T) {
+ conf := yobj{
+ "dns": yobj{},
+ }
+
+ err := upgradeSchema12to13(conf)
+ require.NoError(t, err)
+
+ assert.Equal(t, conf["schema_version"], 13)
+ })
+
+ t.Run("good", func(t *testing.T) {
+ conf := yobj{
+ "dns": yobj{
+ "local_domain_name": "lan",
+ },
+ "dhcp": yobj{},
+ "schema_version": 12,
+ }
+
+ wantConf := yobj{
+ "dns": yobj{},
+ "dhcp": yobj{
+ "local_domain_name": "lan",
+ },
+ "schema_version": 13,
+ }
+
+ err := upgradeSchema12to13(conf)
+ require.NoError(t, err)
+
+ assert.Equal(t, wantConf, conf)
+ })
+}