home: mv local domain name to dhcp setts

CHANGELOG.md

@@ -19,6 +19,35 @@ and this project adheres to
 
 - `windows/arm64` support ([#3057]).
 
+### Changed
+
+- The setting `local_domain_name` is now in the `dhcp` block in the
+  configuration file to avoid confusion ([#3367]).
+
+#### 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
 
 <!--
@@ -32,6 +61,7 @@ TODO(a.garipov): Remove this deprecation, if v0.108.0 is released before the Go
 - Go 1.16 support.
 
 [#3057]: https://github.com/AdguardTeam/AdGuardHome/issues/3057
+[#3367]: https://github.com/AdguardTeam/AdGuardHome/issues/3367
 
 
 

internal/aghnet/dhcp_unix.go

@@ -110,7 +110,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)

internal/aghnet/hostscontainer_test.go

@@ -133,7 +133,7 @@ func TestHostsContainer_Refresh(t *testing.T) {
 	knownIP := net.IP{127, 0, 0, 1}
 
 	const knownHost = "localhost"
-	const knownAlias = "hocallost"
+	const knownAlias = "localhost-alias"
 
 	const dirname = "dir"
 	const filename1 = "file1"
@@ -421,7 +421,7 @@ func TestHostsContainer(t *testing.T) {
 func TestUniqueRules_ParseLine(t *testing.T) {
 	const (
 		hostname = "localhost"
-		alias    = "hocallost"
+		alias    = "localhost-alias"
 	)
 
 	knownIP := net.IP{127, 0, 0, 1}
@@ -440,7 +440,7 @@ func TestUniqueRules_ParseLine(t *testing.T) {
 		name:      "aliases",
 		line:      strings.Join([]string{knownIP.String(), hostname, alias}, sp),
 		wantIP:    knownIP,
-		wantHosts: []string{"localhost", "hocallost"},
+		wantHosts: []string{"localhost", "localhost-alias"},
 	}, {
 		name:      "invalid_line",
 		line:      knownIP.String(),

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 {
+	// 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:"-"` // 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:"-"`
+	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)
 }

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 {

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{

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,
 	}}
 

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:

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

internal/home/clients_test.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/dhcpd"
+	"github.com/AdguardTeam/golibs/testutil"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -276,7 +277,7 @@ func TestClientsAddExisting(t *testing.T) {
 		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 +291,13 @@ 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},

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
 	}