dhcpsvc: imp code, tests

internal/dhcpsvc/config.go

@@ -66,6 +66,8 @@ func (conf *Config) Validate() (err error) {
 		errs = append(errs, err)
 	}
 
+	// This is a best-effort check for the file accessibility.  The file will be
+	// checked again when it is opened later.
 	if _, err = os.Stat(conf.DBFilePath); err != nil && !errors.Is(err, os.ErrNotExist) {
 		errs = append(errs, fmt.Errorf("db file path %q: %w", conf.DBFilePath, err))
 	}

internal/dhcpsvc/db.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io/fs"
 	"net"
 	"net/netip"
 	"os"
@@ -19,6 +20,9 @@ import (
 // dataVersion is the current version of the stored DHCP leases structure.
 const dataVersion = 1
 
+// databasePerm is the permissions for the database file.
+const databasePerm fs.FileMode = 0o640
+
 // dataLeases is the structure of the stored DHCP leases.
 type dataLeases struct {
 	// Leases is the list containing stored DHCP leases.
@@ -43,8 +47,8 @@ func (dl *dbLease) compareNames(other *dbLease) (res int) {
 	return strings.Compare(dl.Hostname, other.Hostname)
 }
 
-// fromLease converts *Lease to *dbLease.
-func fromLease(l *Lease) (dl *dbLease) {
+// toDBLease converts *Lease to *dbLease.
+func toDBLease(l *Lease) (dl *dbLease) {
 	var expiryStr string
 	if !l.IsStatic {
 		// The front-end is waiting for RFC 3999 format of the time value.  It
@@ -63,8 +67,8 @@ func fromLease(l *Lease) (dl *dbLease) {
 	}
 }
 
-// toLease converts dl to *Lease.
-func (dl *dbLease) toLease() (l *Lease, err error) {
+// toInternal converts dl to *Lease.
+func (dl *dbLease) toInternal() (l *Lease, err error) {
 	mac, err := net.ParseMAC(dl.HWAddr)
 	if err != nil {
 		return nil, fmt.Errorf("parsing hardware address: %w", err)
@@ -117,47 +121,38 @@ func (srv *DHCPServer) dbLoad(ctx context.Context) (err error) {
 
 // addDBLeases adds leases to the server.
 func (srv *DHCPServer) addDBLeases(ctx context.Context, leases []*dbLease) {
-	const logMsg = "loading lease"
-
 	var v4, v6 uint
 	for i, l := range leases {
-		var lease *Lease
-		lease, err := l.toLease()
+		lease, err := l.toInternal()
 		if err != nil {
-			srv.logger.DebugContext(ctx, logMsg, "idx", i, slogutil.KeyError, err)
+			srv.logger.WarnContext(ctx, "converting lease", "idx", i, slogutil.KeyError, err)
 
 			continue
 		}
 
-		addr := l.IP
-		iface, err := srv.ifaceForAddr(addr)
+		iface, err := srv.ifaceForAddr(l.IP)
 		if err != nil {
-			srv.logger.DebugContext(ctx, logMsg, "idx", i, slogutil.KeyError, err)
+			srv.logger.WarnContext(ctx, "searching lease iface", "idx", i, slogutil.KeyError, err)
 
 			continue
 		}
 
 		err = srv.leases.add(lease, iface)
 		if err != nil {
-			srv.logger.DebugContext(ctx, logMsg, "idx", i, slogutil.KeyError, err)
+			srv.logger.WarnContext(ctx, "adding lease", "idx", i, slogutil.KeyError, err)
 
 			continue
 		}
 
-		if lease.IP.Is4() {
+		if l.IP.Is4() {
 			v4++
 		} else {
 			v6++
 		}
 	}
 
-	srv.logger.InfoContext(
-		ctx,
-		"loaded leases",
-		"v4", v4,
-		"v6", v6,
-		"total", len(leases),
-	)
+	// TODO(e.burkov):  Group by interface.
+	srv.logger.InfoContext(ctx, "loaded leases", "v4", v4, "v6", v6, "total", len(leases))
 }
 
 // writeDB writes leases to the database file.  It expects the
@@ -166,13 +161,13 @@ func (srv *DHCPServer) dbStore(ctx context.Context) (err error) {
 	defer func() { err = errors.Annotate(err, "writing db: %w") }()
 
 	dl := &dataLeases{
-		// Avoid writing "null" into the database file if there are no leases.
+		// Avoid writing null into the database file if there are no leases.
 		Leases:  make([]*dbLease, 0, srv.leases.len()),
 		Version: dataVersion,
 	}
 
 	srv.leases.rangeLeases(func(l *Lease) (cont bool) {
-		lease := fromLease(l)
+		lease := toDBLease(l)
 		i, _ := slices.BinarySearchFunc(dl.Leases, lease, (*dbLease).compareNames)
 		dl.Leases = slices.Insert(dl.Leases, i, lease)
 
@@ -185,7 +180,7 @@ func (srv *DHCPServer) dbStore(ctx context.Context) (err error) {
 		return err
 	}
 
-	err = maybe.WriteFile(srv.dbFilePath, buf, 0o644)
+	err = maybe.WriteFile(srv.dbFilePath, buf, databasePerm)
 	if err != nil {
 		// Don't wrap the error since it's informative enough as is.
 		return err

internal/dhcpsvc/db_internal_test.go

@@ -0,0 +1,4 @@
+package dhcpsvc
+
+// DatabasePerm is the permissions for the test database file.
+const DatabasePerm = databasePerm

internal/dhcpsvc/dhcpsvc_test.go

@@ -0,0 +1,66 @@
+package dhcpsvc_test
+
+import (
+	"net"
+	"net/netip"
+	"time"
+
+	"github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
+	"github.com/stretchr/testify/require"
+)
+
+// testLocalTLD is a common local TLD for tests.
+const testLocalTLD = "local"
+
+// testTimeout is a common timeout for tests and contexts.
+const testTimeout time.Duration = 10 * time.Second
+
+// discardLog is a logger to discard test output.
+var discardLog = slogutil.NewDiscardLogger()
+
+// testInterfaceConf is a common set of interface configurations for tests.
+var testInterfaceConf = map[string]*dhcpsvc.InterfaceConfig{
+	"eth0": {
+		IPv4: &dhcpsvc.IPv4Config{
+			Enabled:       true,
+			GatewayIP:     netip.MustParseAddr("192.168.0.1"),
+			SubnetMask:    netip.MustParseAddr("255.255.255.0"),
+			RangeStart:    netip.MustParseAddr("192.168.0.2"),
+			RangeEnd:      netip.MustParseAddr("192.168.0.254"),
+			LeaseDuration: 1 * time.Hour,
+		},
+		IPv6: &dhcpsvc.IPv6Config{
+			Enabled:       true,
+			RangeStart:    netip.MustParseAddr("2001:db8::1"),
+			LeaseDuration: 1 * time.Hour,
+			RAAllowSLAAC:  true,
+			RASLAACOnly:   true,
+		},
+	},
+	"eth1": {
+		IPv4: &dhcpsvc.IPv4Config{
+			Enabled:       true,
+			GatewayIP:     netip.MustParseAddr("172.16.0.1"),
+			SubnetMask:    netip.MustParseAddr("255.255.255.0"),
+			RangeStart:    netip.MustParseAddr("172.16.0.2"),
+			RangeEnd:      netip.MustParseAddr("172.16.0.255"),
+			LeaseDuration: 1 * time.Hour,
+		},
+		IPv6: &dhcpsvc.IPv6Config{
+			Enabled:       true,
+			RangeStart:    netip.MustParseAddr("2001:db9::1"),
+			LeaseDuration: 1 * time.Hour,
+			RAAllowSLAAC:  true,
+			RASLAACOnly:   true,
+		},
+	},
+}
+
+// mustParseMAC parses a hardware address from s and requires no errors.
+func mustParseMAC(t require.TestingT, s string) (mac net.HardwareAddr) {
+	mac, err := net.ParseMAC(s)
+	require.NoError(t, err)
+
+	return mac
+}

internal/dhcpsvc/server.go

@@ -29,8 +29,9 @@ type DHCPServer struct {
 
 	// dbFilePath is the path to the database file containing the DHCP leases.
 	//
-	// TODO(e.burkov):  Perhaps, extract leases and database into a separate
-	// type.
+	// TODO(e.burkov):  Consider extracting the database logic into a separate
+	// interface to prevent packages that only need lease data from depending on
+	// the entire server and to simplify testing.
 	dbFilePath string
 
 	// leasesMu protects the leases index as well as leases in the interfaces.