From a91a257b1551a91505dcc6e74f7c292d5d5e723f Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen Date: Tue, 18 Apr 2023 15:12:11 +0300 Subject: [PATCH] Pull request 1817: AG-20352-imp-leases-db Merge in DNS/adguard-home from AG-20352-imp-leases-db to master Squashed commit of the following: commit 2235fb4671bb3f80c933847362cd35b5704dd18d Merge: 0c4d76d4f 76a74b271 Author: Stanislav Chzhen Date: Tue Apr 18 15:09:34 2023 +0300 Merge branch 'master' into AG-20352-imp-leases-db commit 0c4d76d4f6222ae06c568864d366df866dc55a54 Merge: e586b82c7 4afd39b22 Author: Stanislav Chzhen Date: Tue Apr 18 11:07:27 2023 +0300 Merge branch 'master' into AG-20352-imp-leases-db commit e586b82c700c4d432e34f36400519eb08b2653ad Author: Stanislav Chzhen Date: Tue Apr 18 11:06:40 2023 +0300 dhcpd: imp docs commit 411d4e6f6e36051bf6a66c709380ed268c161c41 Author: Stanislav Chzhen Date: Mon Apr 17 16:56:56 2023 +0300 dhcpd: imp code commit e457dc2c385ab62b36df7f96c949e6b90ed2034a Author: Stanislav Chzhen Date: Mon Apr 17 14:29:29 2023 +0300 all: imp code more commit c2df20d0125d368d0155af0808af979921763e1f Author: Stanislav Chzhen Date: Fri Apr 14 15:07:53 2023 +0300 all: imp code commit a4e9ffb9ae769c828c22d62ddf231f7bcfea14db Author: Stanislav Chzhen Date: Wed Apr 12 19:19:35 2023 +0300 dhcpd: fix test more commit 138d89414f1a89558b23962acb7174dce28346d9 Author: Stanislav Chzhen Date: Wed Apr 12 18:08:29 2023 +0300 dhcpd: fix test commit e07e7a23e7c913951c8ecb38c12a3345ebe473be Author: Stanislav Chzhen Date: Wed Apr 12 17:22:27 2023 +0300 all: upd chlog commit 1b6a76e79cf4beed9ca980766ce97930b375bfde Author: Stanislav Chzhen Date: Wed Apr 12 13:24:11 2023 +0300 all: migrate leases db --- .gitignore | 1 - CHANGELOG.md | 2 + internal/dhcpd/config.go | 12 +- internal/dhcpd/db.go | 175 ++++++++---------------- internal/dhcpd/dhcpd.go | 22 +-- internal/dhcpd/dhcpd_unix_test.go | 41 ++---- internal/dhcpd/http_unix.go | 6 +- internal/dhcpd/http_unix_test.go | 3 +- internal/dhcpd/migrate.go | 106 ++++++++++++++ internal/dhcpd/migrate_internal_test.go | 73 ++++++++++ internal/dhcpd/v4_unix.go | 3 +- internal/dhcpd/v4_unix_test.go | 5 - internal/dhcpd/v6_unix.go | 16 +-- internal/dhcpd/v6_unix_test.go | 6 +- internal/home/clients_test.go | 9 +- internal/home/home.go | 2 + 16 files changed, 283 insertions(+), 199 deletions(-) create mode 100644 internal/dhcpd/migrate.go create mode 100644 internal/dhcpd/migrate_internal_test.go diff --git a/.gitignore b/.gitignore index 3873fd3d..bdc4c29f 100644 --- a/.gitignore +++ b/.gitignore @@ -21,7 +21,6 @@ /snapcraft_login AdGuardHome* coverage.txt -leases.db node_modules/ !/build/gitkeep diff --git a/CHANGELOG.md b/CHANGELOG.md index 6731038c..54820495 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,8 @@ See also the [v0.107.28 GitHub milestone][ms-v0.107.28]. ### Changed +- Stored DHCP leases moved from `leases.db` to `data/leases.json`. The file + format has also been optimized. - ARPA domain names containing a subnet within private networks now also considered private, behaving closer to [RFC 6761][rfc6761] ([#5567]). diff --git a/internal/dhcpd/config.go b/internal/dhcpd/config.go index c942039a..f7919fdf 100644 --- a/internal/dhcpd/config.go +++ b/internal/dhcpd/config.go @@ -31,8 +31,16 @@ type ServerConfig struct { Conf4 V4ServerConf `yaml:"dhcpv4"` Conf6 V6ServerConf `yaml:"dhcpv6"` - WorkDir string `yaml:"-"` - DBFilePath string `yaml:"-"` + // WorkDir is used to store DHCP leases. + // + // Deprecated: Remove it when migration of DHCP leases will not be needed. + WorkDir string `yaml:"-"` + + // DataDir is used to store DHCP leases. + DataDir string `yaml:"-"` + + // dbFilePath is the path to the file with stored DHCP leases. + dbFilePath string `yaml:"-"` } // DHCPServer - DHCP server interface diff --git a/internal/dhcpd/db.go b/internal/dhcpd/db.go index 453470a1..3a6e98b7 100644 --- a/internal/dhcpd/db.go +++ b/internal/dhcpd/db.go @@ -5,43 +5,34 @@ package dhcpd import ( "encoding/json" "fmt" - "net" - "net/netip" "os" - "time" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/google/renameio/maybe" + "golang.org/x/exp/slices" ) -const dbFilename = "leases.db" +const ( + // dataFilename contains saved leases. + dataFilename = "leases.json" -type leaseJSON struct { - HWAddr []byte `json:"mac"` - IP []byte `json:"ip"` - Hostname string `json:"host"` - Expiry int64 `json:"exp"` + // dataVersion is the current version of the stored DHCP leases structure. + dataVersion = 1 +) + +// dataLeases is the structure of the stored DHCP leases. +type dataLeases struct { + // Version is the current version of the structure. + Version int `json:"version"` + + // Leases is the list containing stored DHCP leases. + Leases []*Lease `json:"leases"` } -func normalizeIP(ip net.IP) net.IP { - ip4 := ip.To4() - if ip4 != nil { - return ip4 - } - return ip -} - -// Load lease table from DB -// -// TODO(s.chzhen): Decrease complexity. +// dbLoad loads stored leases. func (s *server) dbLoad() (err error) { - dynLeases := []*Lease{} - staticLeases := []*Lease{} - v6StaticLeases := []*Lease{} - v6DynLeases := []*Lease{} - - data, err := os.ReadFile(s.conf.DBFilePath) + data, err := os.ReadFile(s.conf.dbFilePath) if err != nil { if !errors.Is(err, os.ErrNotExist) { return fmt.Errorf("reading db: %w", err) @@ -50,52 +41,30 @@ func (s *server) dbLoad() (err error) { return nil } - obj := []leaseJSON{} - err = json.Unmarshal(data, &obj) + dl := &dataLeases{} + err = json.Unmarshal(data, dl) if err != nil { return fmt.Errorf("decoding db: %w", err) } - numLeases := len(obj) - for i := range obj { - obj[i].IP = normalizeIP(obj[i].IP) + leases := dl.Leases - ip, ok := netip.AddrFromSlice(obj[i].IP) - if !ok { - log.Info("dhcp: invalid IP: %s", obj[i].IP) - continue - } + leases4 := []*Lease{} + leases6 := []*Lease{} - lease := Lease{ - HWAddr: obj[i].HWAddr, - IP: ip, - Hostname: obj[i].Hostname, - Expiry: time.Unix(obj[i].Expiry, 0), - IsStatic: obj[i].Expiry == leaseExpireStatic, - } - - if len(obj[i].IP) == 16 { - if lease.IsStatic { - v6StaticLeases = append(v6StaticLeases, &lease) - } else { - v6DynLeases = append(v6DynLeases, &lease) - } + for _, l := range leases { + if l.IP.Is4() { + leases4 = append(leases4, l) } else { - if lease.IsStatic { - staticLeases = append(staticLeases, &lease) - } else { - dynLeases = append(dynLeases, &lease) - } + leases6 = append(leases6, l) } } - leases4 := normalizeLeases(staticLeases, dynLeases) err = s.srv4.ResetLeases(leases4) if err != nil { return fmt.Errorf("resetting dhcpv4 leases: %w", err) } - leases6 := normalizeLeases(v6StaticLeases, v6DynLeases) if s.srv6 != nil { err = s.srv6.ResetLeases(leases6) if err != nil { @@ -104,90 +73,54 @@ func (s *server) dbLoad() (err error) { } log.Info("dhcp: loaded leases v4:%d v6:%d total-read:%d from DB", - len(leases4), len(leases6), numLeases) + len(leases4), len(leases6), len(leases)) return nil } -// Skip duplicate leases -// Static leases have a priority over dynamic leases -func normalizeLeases(staticLeases, dynLeases []*Lease) []*Lease { - leases := []*Lease{} - index := map[string]int{} - - for i, lease := range staticLeases { - _, ok := index[lease.HWAddr.String()] - if ok { - continue // skip the lease with the same HW address - } - index[lease.HWAddr.String()] = i - leases = append(leases, lease) - } - - for i, lease := range dynLeases { - _, ok := index[lease.HWAddr.String()] - if ok { - continue // skip the lease with the same HW address - } - index[lease.HWAddr.String()] = i - leases = append(leases, lease) - } - - return leases -} - -// Store lease table in DB +// dbStore stores DHCP leases. func (s *server) dbStore() (err error) { // Use an empty slice here as opposed to nil so that it doesn't write // "null" into the database file if leases are empty. - leases := []leaseJSON{} + leases := []*Lease{} leases4 := s.srv4.getLeasesRef() - for _, l := range leases4 { - if l.Expiry.Unix() == 0 { - continue - } - - lease := leaseJSON{ - HWAddr: l.HWAddr, - IP: l.IP.AsSlice(), - Hostname: l.Hostname, - Expiry: l.Expiry.Unix(), - } - - leases = append(leases, lease) - } + leases = append(leases, leases4...) if s.srv6 != nil { leases6 := s.srv6.getLeasesRef() - for _, l := range leases6 { - if l.Expiry.Unix() == 0 { - continue - } - - lease := leaseJSON{ - HWAddr: l.HWAddr, - IP: l.IP.AsSlice(), - Hostname: l.Hostname, - Expiry: l.Expiry.Unix(), - } - - leases = append(leases, lease) - } + leases = append(leases, leases6...) } - var data []byte - data, err = json.Marshal(leases) + return writeDB(s.conf.dbFilePath, leases) +} + +// writeDB writes leases to file at path. +func writeDB(path string, leases []*Lease) (err error) { + defer func() { err = errors.Annotate(err, "writing db: %w") }() + + slices.SortFunc(leases, func(a, b *Lease) bool { + return a.Hostname < b.Hostname + }) + + dl := &dataLeases{ + Version: dataVersion, + Leases: leases, + } + + buf, err := json.Marshal(dl) if err != nil { - return fmt.Errorf("encoding db: %w", err) + // Don't wrap the error since it's informative enough as is. + return err } - err = maybe.WriteFile(s.conf.DBFilePath, data, 0o644) + err = maybe.WriteFile(path, buf, 0o644) if err != nil { - return fmt.Errorf("writing db: %w", err) + // Don't wrap the error since it's informative enough as is. + return err } - log.Info("dhcp: stored %d leases in db", len(leases)) + log.Info("dhcp: stored %d leases in %q", len(leases), path) return nil } diff --git a/internal/dhcpd/dhcpd.go b/internal/dhcpd/dhcpd.go index 6ac830d6..69082c0c 100644 --- a/internal/dhcpd/dhcpd.go +++ b/internal/dhcpd/dhcpd.go @@ -15,13 +15,6 @@ import ( ) const ( - // leaseExpireStatic is used to define the Expiry field for static - // leases. - // - // TODO(e.burkov): Remove it when static leases determining mechanism - // will be improved. - leaseExpireStatic = 1 - // DefaultDHCPLeaseTTL is the default time-to-live for leases. DefaultDHCPLeaseTTL = uint32(timeutil.Day / time.Second) @@ -35,10 +28,10 @@ const ( defaultBackoff time.Duration = 500 * time.Millisecond ) -// Lease contains the necessary information about a DHCP lease +// Lease contains the necessary information about a DHCP lease. It's used in +// various places. So don't change it without good reason. type Lease struct { - // Expiry is the expiration time of the lease. The unix timestamp value - // of 1 means that this is a static lease. + // Expiry is the expiration time of the lease. Expiry time.Time `json:"expires"` // Hostname of the client. @@ -238,7 +231,7 @@ func Create(conf *ServerConfig) (s *server, err error) { LocalDomainName: conf.LocalDomainName, - DBFilePath: filepath.Join(conf.WorkDir, dbFilename), + dbFilePath: filepath.Join(conf.DataDir, dataFilename), }, } @@ -279,6 +272,13 @@ func Create(conf *ServerConfig) (s *server, err error) { return nil, fmt.Errorf("neither dhcpv4 nor dhcpv6 srv is configured") } + // Migrate leases db if needed. + err = migrateDB(conf) + if err != nil { + // Don't wrap the error since it's informative enough as is. + return nil, err + } + // Don't delay database loading until the DHCP server is started, // because we need static leases functionality available beforehand. err = s.dbLoad() diff --git a/internal/dhcpd/dhcpd_unix_test.go b/internal/dhcpd/dhcpd_unix_test.go index 40e83697..7eced536 100644 --- a/internal/dhcpd/dhcpd_unix_test.go +++ b/internal/dhcpd/dhcpd_unix_test.go @@ -5,7 +5,7 @@ package dhcpd import ( "net" "net/netip" - "os" + "path/filepath" "testing" "time" @@ -27,7 +27,7 @@ func TestDB(t *testing.T) { var err error s := server{ conf: &ServerConfig{ - DBFilePath: dbFilename, + dbFilePath: filepath.Join(t.TempDir(), dataFilename), }, } @@ -67,8 +67,6 @@ func TestDB(t *testing.T) { err = s.dbStore() require.NoError(t, err) - testutil.CleanupAndRequireSuccess(t, func() (err error) { return os.Remove(dbFilename) }) - err = s.srv4.ResetLeases(nil) require.NoError(t, err) @@ -78,36 +76,13 @@ func TestDB(t *testing.T) { ll := s.srv4.GetLeases(LeasesAll) require.Len(t, ll, len(leases)) - assert.Equal(t, leases[1].HWAddr, ll[0].HWAddr) - assert.Equal(t, leases[1].IP, ll[0].IP) - assert.True(t, ll[0].IsStatic) + assert.Equal(t, leases[0].HWAddr, ll[0].HWAddr) + assert.Equal(t, leases[0].IP, ll[0].IP) + assert.Equal(t, leases[0].Expiry.Unix(), ll[0].Expiry.Unix()) - assert.Equal(t, leases[0].HWAddr, ll[1].HWAddr) - assert.Equal(t, leases[0].IP, ll[1].IP) - assert.Equal(t, leases[0].Expiry.Unix(), ll[1].Expiry.Unix()) -} - -func TestNormalizeLeases(t *testing.T) { - dynLeases := []*Lease{{ - HWAddr: net.HardwareAddr{1, 2, 3, 4}, - }, { - HWAddr: net.HardwareAddr{1, 2, 3, 5}, - }} - - staticLeases := []*Lease{{ - HWAddr: net.HardwareAddr{1, 2, 3, 4}, - IP: netip.MustParseAddr("0.2.3.4"), - }, { - HWAddr: net.HardwareAddr{2, 2, 3, 4}, - }} - - leases := normalizeLeases(staticLeases, dynLeases) - require.Len(t, leases, 3) - - assert.Equal(t, leases[0].HWAddr, dynLeases[0].HWAddr) - assert.Equal(t, leases[0].IP, staticLeases[0].IP) - assert.Equal(t, leases[1].HWAddr, staticLeases[1].HWAddr) - assert.Equal(t, leases[2].HWAddr, dynLeases[1].HWAddr) + assert.Equal(t, leases[1].HWAddr, ll[1].HWAddr) + assert.Equal(t, leases[1].IP, ll[1].IP) + assert.True(t, ll[1].IsStatic) } func TestV4Server_badRange(t *testing.T) { diff --git a/internal/dhcpd/http_unix.go b/internal/dhcpd/http_unix.go index 9eb4eb47..6430afdc 100644 --- a/internal/dhcpd/http_unix.go +++ b/internal/dhcpd/http_unix.go @@ -639,7 +639,7 @@ func (s *server) handleReset(w http.ResponseWriter, r *http.Request) { return } - err = os.Remove(s.conf.DBFilePath) + err = os.Remove(s.conf.dbFilePath) if err != nil && !errors.Is(err, os.ErrNotExist) { log.Error("dhcp: removing db: %s", err) } @@ -651,8 +651,8 @@ func (s *server) handleReset(w http.ResponseWriter, r *http.Request) { LocalDomainName: s.conf.LocalDomainName, - WorkDir: s.conf.WorkDir, - DBFilePath: s.conf.DBFilePath, + DataDir: s.conf.DataDir, + dbFilePath: s.conf.dbFilePath, } v4conf := &V4ServerConf{ diff --git a/internal/dhcpd/http_unix_test.go b/internal/dhcpd/http_unix_test.go index d23614c3..2a569f4e 100644 --- a/internal/dhcpd/http_unix_test.go +++ b/internal/dhcpd/http_unix_test.go @@ -31,8 +31,7 @@ func TestServer_handleDHCPStatus(t *testing.T) { s, err := Create(&ServerConfig{ Enabled: true, Conf4: *defaultV4ServerConf(), - WorkDir: t.TempDir(), - DBFilePath: dbFilename, + DataDir: t.TempDir(), ConfigModified: func() {}, }) require.NoError(t, err) diff --git a/internal/dhcpd/migrate.go b/internal/dhcpd/migrate.go new file mode 100644 index 00000000..aafee9b6 --- /dev/null +++ b/internal/dhcpd/migrate.go @@ -0,0 +1,106 @@ +package dhcpd + +import ( + "encoding/json" + "net" + "net/netip" + "os" + "path/filepath" + "time" + + "github.com/AdguardTeam/golibs/errors" + "github.com/AdguardTeam/golibs/log" +) + +const ( + // leaseExpireStatic is used to define the Expiry field for static + // leases. + // + // Deprecated: Remove it when migration of DHCP leases will be not needed. + leaseExpireStatic = 1 + + // dbFilename contains saved leases. + // + // Deprecated: Use dataFilename. + dbFilename = "leases.db" +) + +// leaseJSON is the structure of stored lease. +// +// Deprecated: Use [Lease]. +type leaseJSON struct { + HWAddr []byte `json:"mac"` + IP []byte `json:"ip"` + Hostname string `json:"host"` + Expiry int64 `json:"exp"` +} + +func normalizeIP(ip net.IP) net.IP { + ip4 := ip.To4() + if ip4 != nil { + return ip4 + } + + return ip +} + +// migrateDB migrates stored leases if necessary. +func migrateDB(conf *ServerConfig) (err error) { + defer func() { err = errors.Annotate(err, "migrating db: %w") }() + + oldLeasesPath := filepath.Join(conf.WorkDir, dbFilename) + dataDirPath := filepath.Join(conf.DataDir, dataFilename) + + file, err := os.Open(oldLeasesPath) + if errors.Is(err, os.ErrNotExist) { + // Nothing to migrate. + return nil + } else if err != nil { + // Don't wrap the error since it's informative enough as is. + return err + } + + ljs := []leaseJSON{} + err = json.NewDecoder(file).Decode(&ljs) + if err != nil { + // Don't wrap the error since it's informative enough as is. + return err + } + + err = file.Close() + if err != nil { + // Don't wrap the error since it's informative enough as is. + return err + } + + leases := []*Lease{} + + for _, lj := range ljs { + lj.IP = normalizeIP(lj.IP) + + ip, ok := netip.AddrFromSlice(lj.IP) + if !ok { + log.Info("dhcp: invalid IP: %s", lj.IP) + + continue + } + + lease := &Lease{ + Expiry: time.Unix(lj.Expiry, 0), + Hostname: lj.Hostname, + HWAddr: lj.HWAddr, + IP: ip, + IsStatic: lj.Expiry == leaseExpireStatic, + } + + leases = append(leases, lease) + } + + err = writeDB(dataDirPath, leases) + if err != nil { + // Don't wrap the error since it's informative enough as is. + return err + } + + return os.Remove(oldLeasesPath) +} diff --git a/internal/dhcpd/migrate_internal_test.go b/internal/dhcpd/migrate_internal_test.go new file mode 100644 index 00000000..2c0e6ecd --- /dev/null +++ b/internal/dhcpd/migrate_internal_test.go @@ -0,0 +1,73 @@ +package dhcpd + +import ( + "encoding/json" + "net" + "net/netip" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const testData = `[ +{"mac":"ESIzRFVm","ip":"AQIDBA==","host":"test1","exp":1}, +{"mac":"ZlVEMyIR","ip":"BAMCAQ==","host":"test2","exp":1231231231} +]` + +func TestMigrateDB(t *testing.T) { + dir := t.TempDir() + + oldLeasesPath := filepath.Join(dir, dbFilename) + dataDirPath := filepath.Join(dir, dataFilename) + + err := os.WriteFile(oldLeasesPath, []byte(testData), 0o644) + require.NoError(t, err) + + wantLeases := []*Lease{{ + Expiry: time.Time{}, + Hostname: "test1", + HWAddr: net.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66}, + IP: netip.MustParseAddr("1.2.3.4"), + IsStatic: true, + }, { + Expiry: time.Unix(1231231231, 0), + Hostname: "test2", + HWAddr: net.HardwareAddr{0x66, 0x55, 0x44, 0x33, 0x22, 0x11}, + IP: netip.MustParseAddr("4.3.2.1"), + IsStatic: false, + }} + + conf := &ServerConfig{ + WorkDir: dir, + DataDir: dir, + } + + err = migrateDB(conf) + require.NoError(t, err) + + _, err = os.Stat(oldLeasesPath) + require.ErrorIs(t, err, os.ErrNotExist) + + var data []byte + data, err = os.ReadFile(dataDirPath) + require.NoError(t, err) + + dl := &dataLeases{} + err = json.Unmarshal(data, dl) + require.NoError(t, err) + + leases := dl.Leases + + for i, wl := range wantLeases { + assert.Equal(t, wl.Hostname, leases[i].Hostname) + assert.Equal(t, wl.HWAddr, leases[i].HWAddr) + assert.Equal(t, wl.IP, leases[i].IP) + assert.Equal(t, wl.IsStatic, leases[i].IsStatic) + + require.True(t, wl.Expiry.Equal(leases[i].Expiry)) + } +} diff --git a/internal/dhcpd/v4_unix.go b/internal/dhcpd/v4_unix.go index bd5aba6e..20b2c96e 100644 --- a/internal/dhcpd/v4_unix.go +++ b/internal/dhcpd/v4_unix.go @@ -256,6 +256,8 @@ func (s *v4Server) rmLeaseByIndex(i int) { // Remove a dynamic lease with the same properties // Return error if a static lease is found +// +// TODO(s.chzhen): Refactor the code. func (s *v4Server) rmDynamicLease(lease *Lease) (err error) { for i, l := range s.leases { isStatic := l.IsStatic @@ -357,7 +359,6 @@ func (s *v4Server) AddStaticLease(l *Lease) (err error) { return fmt.Errorf("can't assign the gateway IP %s to the lease", gwIP) } - l.Expiry = time.Unix(leaseExpireStatic, 0) l.IsStatic = true err = netutil.ValidateMAC(l.HWAddr) diff --git a/internal/dhcpd/v4_unix_test.go b/internal/dhcpd/v4_unix_test.go index 6c51cc5f..a5ce5e0e 100644 --- a/internal/dhcpd/v4_unix_test.go +++ b/internal/dhcpd/v4_unix_test.go @@ -68,7 +68,6 @@ func TestV4Server_leasing(t *testing.T) { t.Run("add_static", func(t *testing.T) { err := s.AddStaticLease(&Lease{ - Expiry: time.Unix(leaseExpireStatic, 0), Hostname: staticName, HWAddr: staticMAC, IP: staticIP, @@ -78,7 +77,6 @@ func TestV4Server_leasing(t *testing.T) { t.Run("same_name", func(t *testing.T) { err = s.AddStaticLease(&Lease{ - Expiry: time.Unix(leaseExpireStatic, 0), Hostname: staticName, HWAddr: anotherMAC, IP: anotherIP, @@ -93,7 +91,6 @@ func TestV4Server_leasing(t *testing.T) { " (" + staticMAC.String() + "): static lease already exists" err = s.AddStaticLease(&Lease{ - Expiry: time.Unix(leaseExpireStatic, 0), Hostname: anotherName, HWAddr: staticMAC, IP: anotherIP, @@ -108,7 +105,6 @@ func TestV4Server_leasing(t *testing.T) { " (" + anotherMAC.String() + "): static lease already exists" err = s.AddStaticLease(&Lease{ - Expiry: time.Unix(leaseExpireStatic, 0), Hostname: anotherName, HWAddr: anotherMAC, IP: staticIP, @@ -784,7 +780,6 @@ func TestV4Server_FindMACbyIP(t *testing.T) { s := &v4Server{ leases: []*Lease{{ - Expiry: time.Unix(leaseExpireStatic, 0), Hostname: staticName, HWAddr: staticMAC, IP: staticIP, diff --git a/internal/dhcpd/v6_unix.go b/internal/dhcpd/v6_unix.go index 2655a343..cbe67eaa 100644 --- a/internal/dhcpd/v6_unix.go +++ b/internal/dhcpd/v6_unix.go @@ -66,8 +66,7 @@ func (s *v6Server) ResetLeases(leases []*Lease) (err error) { s.leases = nil for _, l := range leases { ip := net.IP(l.IP.AsSlice()) - if l.Expiry.Unix() != leaseExpireStatic && - !ip6InRange(s.conf.ipStart, ip) { + if !l.IsStatic && !ip6InRange(s.conf.ipStart, ip) { log.Debug("dhcpv6: skipping a lease with IP %v: not within current IP range", l.IP) @@ -89,7 +88,7 @@ func (s *v6Server) GetLeases(flags GetLeasesFlags) (leases []*Lease) { leases = []*Lease{} s.leasesLock.Lock() for _, l := range s.leases { - if l.Expiry.Unix() == leaseExpireStatic { + if l.IsStatic { if (flags & LeasesStatic) != 0 { leases = append(leases, l.Clone()) } @@ -150,7 +149,7 @@ func (s *v6Server) rmDynamicLease(lease *Lease) (err error) { l := s.leases[i] if bytes.Equal(l.HWAddr, lease.HWAddr) { - if l.Expiry.Unix() == leaseExpireStatic { + if l.IsStatic { return fmt.Errorf("static lease already exists") } @@ -163,7 +162,7 @@ func (s *v6Server) rmDynamicLease(lease *Lease) (err error) { } if l.IP == lease.IP { - if l.Expiry.Unix() == leaseExpireStatic { + if l.IsStatic { return fmt.Errorf("static lease already exists") } @@ -187,7 +186,7 @@ func (s *v6Server) AddStaticLease(l *Lease) (err error) { return fmt.Errorf("validating lease: %w", err) } - l.Expiry = time.Unix(leaseExpireStatic, 0) + l.IsStatic = true s.leasesLock.Lock() err = s.rmDynamicLease(l) @@ -274,8 +273,7 @@ func (s *v6Server) findLease(mac net.HardwareAddr) *Lease { func (s *v6Server) findExpiredLease() int { now := time.Now().Unix() for i, lease := range s.leases { - if lease.Expiry.Unix() != leaseExpireStatic && - lease.Expiry.Unix() <= now { + if !lease.IsStatic && lease.Expiry.Unix() <= now { return i } } @@ -421,7 +419,7 @@ func (s *v6Server) commitLease(msg *dhcpv6.Message, lease *Lease) time.Duration dhcpv6.MessageTypeRenew, dhcpv6.MessageTypeRebind: - if lease.Expiry.Unix() != leaseExpireStatic { + if !lease.IsStatic { s.commitDynamicLease(lease) } } diff --git a/internal/dhcpd/v6_unix_test.go b/internal/dhcpd/v6_unix_test.go index 85c29e3e..c5034e47 100644 --- a/internal/dhcpd/v6_unix_test.go +++ b/internal/dhcpd/v6_unix_test.go @@ -44,7 +44,7 @@ func TestV6_AddRemove_static(t *testing.T) { assert.Equal(t, l.IP, ls[0].IP) assert.Equal(t, l.HWAddr, ls[0].HWAddr) - assert.EqualValues(t, leaseExpireStatic, ls[0].Expiry.Unix()) + assert.True(t, ls[0].IsStatic) // Try to remove non-existent static lease. err = s.RemoveStaticLease(&Lease{ @@ -103,7 +103,7 @@ func TestV6_AddReplace(t *testing.T) { for i, l := range ls { assert.Equal(t, stLeases[i].IP, l.IP) assert.Equal(t, stLeases[i].HWAddr, l.HWAddr) - assert.EqualValues(t, leaseExpireStatic, l.Expiry.Unix()) + assert.True(t, l.IsStatic) } } @@ -327,7 +327,6 @@ func TestV6_FindMACbyIP(t *testing.T) { s := &v6Server{ leases: []*Lease{{ - Expiry: time.Unix(leaseExpireStatic, 0), Hostname: staticName, HWAddr: staticMAC, IP: staticIP, @@ -341,7 +340,6 @@ func TestV6_FindMACbyIP(t *testing.T) { } s.leases = []*Lease{{ - Expiry: time.Unix(leaseExpireStatic, 0), Hostname: staticName, HWAddr: staticMAC, IP: staticIP, diff --git a/internal/home/clients_test.go b/internal/home/clients_test.go index 410ef6d4..ebf879ef 100644 --- a/internal/home/clients_test.go +++ b/internal/home/clients_test.go @@ -3,14 +3,12 @@ package home import ( "net" "net/netip" - "os" "runtime" "testing" "time" "github.com/AdguardTeam/AdGuardHome/internal/dhcpd" "github.com/AdguardTeam/AdGuardHome/internal/filtering" - "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -283,8 +281,8 @@ func TestClientsAddExisting(t *testing.T) { // First, init a DHCP server with a single static lease. config := &dhcpd.ServerConfig{ - Enabled: true, - DBFilePath: "leases.db", + Enabled: true, + DataDir: t.TempDir(), Conf4: dhcpd.V4ServerConf{ Enabled: true, GatewayIP: netip.MustParseAddr("1.2.3.1"), @@ -296,9 +294,6 @@ func TestClientsAddExisting(t *testing.T) { dhcpServer, err := dhcpd.Create(config) require.NoError(t, err) - testutil.CleanupAndRequireSuccess(t, func() (err error) { - return os.Remove("leases.db") - }) clients.dhcpServer = dhcpServer diff --git a/internal/home/home.go b/internal/home/home.go index 7f9762dc..443bdc0f 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -306,7 +306,9 @@ func setupConfig(opts options) (err error) { return fmt.Errorf("initializing safesearch: %w", err) } + //lint:ignore SA1019 Migration is not over. config.DHCP.WorkDir = Context.workDir + config.DHCP.DataDir = Context.getDataDir() config.DHCP.HTTPRegister = httpRegister config.DHCP.ConfigModified = onConfigModified