Pull request 1800: AG-20352-dhcpd-lease-is-static

Merge in DNS/adguard-home from AG-20352-dhcpd-lease-is-static to master

Squashed commit of the following:

commit e4f4278a7ffa0f084ed41472dd3e7de4466c9f50
Merge: b6e3b62e 15bba281
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Mon Apr 10 11:13:05 2023 +0300

    Merge branch 'master' into AG-20352-dhcpd-lease-is-static

commit b6e3b62ed4
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Thu Apr 6 15:20:42 2023 +0300

    dhcpd: imp naming

commit e2d9ed0832
Merge: ecd244a6 10bffd89
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Apr 5 16:53:55 2023 +0300

    Merge branch 'master' into AG-20352-dhcpd-lease-is-static

commit ecd244a608
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Apr 5 16:53:15 2023 +0300

    dhcpd: add lease json form

commit 9ebc246ed4
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Apr 5 13:11:25 2023 +0300

    dhcpd: imp code

commit fc3d0cdaeb
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Apr 5 13:00:59 2023 +0300

    all: fix json

commit d722578543
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Apr 5 12:23:34 2023 +0300

    dhcpd: add lease is static

internal/dhcpd/db.go

@@ -71,16 +71,17 @@ func (s *server) dbLoad() (err error) {
 			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 obj[i].Expiry == leaseExpireStatic {
+			if lease.IsStatic {
 				v6StaticLeases = append(v6StaticLeases, &lease)
 			} else {
 				v6DynLeases = append(v6DynLeases, &lease)
 			}
 		} else {
-			if obj[i].Expiry == leaseExpireStatic {
+			if lease.IsStatic {
 				staticLeases = append(staticLeases, &lease)
 			} else {
 				dynLeases = append(dynLeases, &lease)

internal/dhcpd/dhcpd.go

@@ -51,6 +51,9 @@ type Lease struct {
 	//
 	// TODO(a.garipov): Migrate leases.db.
 	IP netip.Addr `json:"ip"`
+
+	// IsStatic defines if the lease is static.
+	IsStatic bool `json:"static"`
 }
 
 // Clone returns a deep copy of l.
@@ -64,6 +67,7 @@ func (l *Lease) Clone() (clone *Lease) {
 		Hostname: l.Hostname,
 		HWAddr:   slices.Clone(l.HWAddr),
 		IP:       l.IP,
+		IsStatic: l.IsStatic,
 	}
 }
 
@@ -84,17 +88,10 @@ func (l *Lease) IsBlocklisted() (ok bool) {
 	return true
 }
 
-// IsStatic returns true if the lease is static.
-//
-// TODO(a.garipov): Just make it a boolean field.
-func (l *Lease) IsStatic() (ok bool) {
-	return l != nil && l.Expiry.Unix() == leaseExpireStatic
-}
-
 // MarshalJSON implements the json.Marshaler interface for Lease.
 func (l Lease) MarshalJSON() ([]byte, error) {
 	var expiryStr string
-	if !l.IsStatic() {
+	if !l.IsStatic {
 		// The front-end is waiting for RFC 3999 format of the time
 		// value.  It also shouldn't got an Expiry field for static
 		// leases.

internal/dhcpd/dhcpd_unix_test.go

@@ -80,7 +80,7 @@ func TestDB(t *testing.T) {
 
 	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.True(t, ll[0].IsStatic)
 
 	assert.Equal(t, leases[0].HWAddr, ll[1].HWAddr)
 	assert.Equal(t, leases[0].IP, ll[1].IP)

internal/dhcpd/http_unix.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"net/netip"
 	"os"
+	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
@@ -60,11 +61,76 @@ type dhcpStatusResponse struct {
 	IfaceName    string          `json:"interface_name"`
 	V4           V4ServerConf    `json:"v4"`
 	V6           V6ServerConf    `json:"v6"`
-	Leases       []*Lease     `json:"leases"`
+	Leases       []*leaseDynamic `json:"leases"`
-	StaticLeases []*Lease     `json:"static_leases"`
+	StaticLeases []*leaseStatic  `json:"static_leases"`
 	Enabled      bool            `json:"enabled"`
 }
 
+// leaseStatic is the JSON form of static DHCP lease.
+type leaseStatic struct {
+	HWAddr   string     `json:"mac"`
+	IP       netip.Addr `json:"ip"`
+	Hostname string     `json:"hostname"`
+}
+
+// leasesToStatic converts list of leases to their JSON form.
+func leasesToStatic(leases []*Lease) (static []*leaseStatic) {
+	static = make([]*leaseStatic, len(leases))
+
+	for i, l := range leases {
+		static[i] = &leaseStatic{
+			HWAddr:   l.HWAddr.String(),
+			IP:       l.IP,
+			Hostname: l.Hostname,
+		}
+	}
+
+	return static
+}
+
+// toLease converts leaseStatic to Lease or returns error.
+func (l *leaseStatic) toLease() (lease *Lease, err error) {
+	addr, err := net.ParseMAC(l.HWAddr)
+	if err != nil {
+		return nil, fmt.Errorf("couldn't parse MAC address: %w", err)
+	}
+
+	return &Lease{
+		HWAddr:   addr,
+		IP:       l.IP,
+		Hostname: l.Hostname,
+		IsStatic: true,
+	}, nil
+}
+
+// leaseDynamic is the JSON form of dynamic DHCP lease.
+type leaseDynamic struct {
+	HWAddr   string     `json:"mac"`
+	IP       netip.Addr `json:"ip"`
+	Hostname string     `json:"hostname"`
+	Expiry   string     `json:"expires"`
+}
+
+// leasesToDynamic converts list of leases to their JSON form.
+func leasesToDynamic(leases []*Lease) (dynamic []*leaseDynamic) {
+	dynamic = make([]*leaseDynamic, len(leases))
+
+	for i, l := range leases {
+		dynamic[i] = &leaseDynamic{
+			HWAddr:   l.HWAddr.String(),
+			IP:       l.IP,
+			Hostname: l.Hostname,
+			// The front-end is waiting for RFC 3999 format of the time
+			// value.
+			//
+			// See https://github.com/AdguardTeam/AdGuardHome/issues/2692.
+			Expiry: l.Expiry.Format(time.RFC3339),
+		}
+	}
+
+	return dynamic
+}
+
 func (s *server) handleDHCPStatus(w http.ResponseWriter, r *http.Request) {
 	status := &dhcpStatusResponse{
 		Enabled:   s.conf.Enabled,
@@ -76,8 +142,8 @@ func (s *server) handleDHCPStatus(w http.ResponseWriter, r *http.Request) {
 	s.srv4.WriteDiskConfig4(&status.V4)
 	s.srv6.WriteDiskConfig6(&status.V6)
 
-	status.Leases = s.Leases(LeasesDynamic)
+	status.Leases = leasesToDynamic(s.Leases(LeasesDynamic))
-	status.StaticLeases = s.Leases(LeasesStatic)
+	status.StaticLeases = leasesToStatic(s.Leases(LeasesStatic))
 
 	_ = aghhttp.WriteJSONResponse(w, r, status)
 }
@@ -488,7 +554,7 @@ func setOtherDHCPResult(ifaceName string, result *dhcpSearchResult) {
 }
 
 func (s *server) handleDHCPAddStaticLease(w http.ResponseWriter, r *http.Request) {
-	l := &Lease{}
+	l := &leaseStatic{}
 	err := json.NewDecoder(r.Body).Decode(l)
 	if err != nil {
 		aghhttp.Error(r, w, http.StatusBadRequest, "json.Decode: %s", err)
@@ -511,7 +577,14 @@ func (s *server) handleDHCPAddStaticLease(w http.ResponseWriter, r *http.Request
 		srv = s.srv6
 	}
 
-	err = srv.AddStaticLease(l)
+	lease, err := l.toLease()
+	if err != nil {
+		aghhttp.Error(r, w, http.StatusBadRequest, "parsing: %s", err)
+
+		return
+	}
+
+	err = srv.AddStaticLease(lease)
 	if err != nil {
 		aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)
 
@@ -520,7 +593,7 @@ func (s *server) handleDHCPAddStaticLease(w http.ResponseWriter, r *http.Request
 }
 
 func (s *server) handleDHCPRemoveStaticLease(w http.ResponseWriter, r *http.Request) {
-	l := &Lease{}
+	l := &leaseStatic{}
 	err := json.NewDecoder(r.Body).Decode(l)
 	if err != nil {
 		aghhttp.Error(r, w, http.StatusBadRequest, "json.Decode: %s", err)
@@ -543,7 +616,14 @@ func (s *server) handleDHCPRemoveStaticLease(w http.ResponseWriter, r *http.Requ
 		srv = s.srv6
 	}
 
-	err = srv.RemoveStaticLease(l)
+	lease, err := l.toLease()
+	if err != nil {
+		aghhttp.Error(r, w, http.StatusBadRequest, "parsing: %s", err)
+
+		return
+	}
+
+	err = srv.RemoveStaticLease(lease)
 	if err != nil {
 		aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)
 

internal/dhcpd/http_unix_test.go

@@ -5,28 +5,27 @@ package dhcpd
 import (
 	"bytes"
 	"encoding/json"
-	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/netip"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestServer_handleDHCPStatus(t *testing.T) {
-	const staticName = "static-client"
+	const (
+		staticName = "static-client"
+		staticMAC  = "aa:aa:aa:aa:aa:aa"
+	)
 
 	staticIP := netip.MustParseAddr("192.168.10.10")
-	staticMAC := net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA}
 
-	staticLease := &Lease{
+	staticLease := &leaseStatic{
-		Expiry:   time.Unix(leaseExpireStatic, 0),
-		Hostname: staticName,
 		HWAddr:   staticMAC,
 		IP:       staticIP,
+		Hostname: staticName,
 	}
 
 	s, err := Create(&ServerConfig{
@@ -65,8 +64,8 @@ func TestServer_handleDHCPStatus(t *testing.T) {
 		resp := &dhcpStatusResponse{
 			V4:           *conf4,
 			V6:           V6ServerConf{},
-			Leases:       []*Lease{},
+			Leases:       []*leaseDynamic{},
-			StaticLeases: []*Lease{},
+			StaticLeases: []*leaseStatic{},
 			Enabled:      true,
 		}
 
@@ -95,7 +94,7 @@ func TestServer_handleDHCPStatus(t *testing.T) {
 		assert.Equal(t, http.StatusOK, w.Code)
 
 		resp := defaultResponse()
-		resp.StaticLeases = []*Lease{staticLease}
+		resp.StaticLeases = []*leaseStatic{staticLease}
 
 		checkStatus(t, resp)
 	})
@@ -106,7 +105,7 @@ func TestServer_handleDHCPStatus(t *testing.T) {
 
 		b := &bytes.Buffer{}
 
-		err = json.NewEncoder(b).Encode(&Lease{})
+		err = json.NewEncoder(b).Encode(&leaseStatic{})
 		require.NoError(t, err)
 
 		var r *http.Request

internal/dhcpd/v4_unix.go

@@ -128,7 +128,7 @@ func (s *v4Server) ResetLeases(leases []*Lease) (err error) {
 	s.leases = nil
 
 	for _, l := range leases {
-		if !l.IsStatic() {
+		if !l.IsStatic {
 			l.Hostname = s.validHostnameForClient(l.Hostname, l.IP)
 		}
 		err = s.addLease(l)
@@ -190,7 +190,7 @@ func (s *v4Server) GetLeases(flags GetLeasesFlags) (leases []*Lease) {
 			continue
 		}
 
-		if getStatic && l.IsStatic() {
+		if getStatic && l.IsStatic {
 			leases = append(leases, l.Clone())
 		}
 	}
@@ -211,7 +211,7 @@ func (s *v4Server) FindMACbyIP(ip netip.Addr) (mac net.HardwareAddr) {
 
 	for _, l := range s.leases {
 		if l.IP == ip {
-			if l.Expiry.After(now) || l.IsStatic() {
+			if l.IsStatic || l.Expiry.After(now) {
 				return l.HWAddr
 			}
 		}
@@ -259,7 +259,7 @@ func (s *v4Server) rmLeaseByIndex(i int) {
 // Return error if a static lease is found
 func (s *v4Server) rmDynamicLease(lease *Lease) (err error) {
 	for i, l := range s.leases {
-		isStatic := l.IsStatic()
+		isStatic := l.IsStatic
 
 		if bytes.Equal(l.HWAddr, lease.HWAddr) || l.IP == lease.IP {
 			if isStatic {
@@ -292,7 +292,7 @@ func (s *v4Server) addLease(l *Lease) (err error) {
 	leaseIP := net.IP(l.IP.AsSlice())
 	offset, inOffset := r.offset(leaseIP)
 
-	if l.IsStatic() {
+	if l.IsStatic {
 		// TODO(a.garipov, d.seregin): Subnet can be nil when dhcp server is
 		// disabled.
 		if sn := s.conf.subnet; !sn.Contains(l.IP) {
@@ -359,6 +359,7 @@ func (s *v4Server) AddStaticLease(l *Lease) (err error) {
 	}
 
 	l.Expiry = time.Unix(leaseExpireStatic, 0)
+	l.IsStatic = true
 
 	err = netutil.ValidateMAC(l.HWAddr)
 	if err != nil {
@@ -528,7 +529,7 @@ func (s *v4Server) nextIP() (ip net.IP) {
 func (s *v4Server) findExpiredLease() int {
 	now := time.Now()
 	for i, lease := range s.leases {
-		if !lease.IsStatic() && lease.Expiry.Before(now) {
+		if !lease.IsStatic && lease.Expiry.Before(now) {
 			return i
 		}
 	}
@@ -860,7 +861,7 @@ func (s *v4Server) handleRequest(req, resp *dhcpv4.DHCPv4) (lease *Lease, needsR
 	s.leasesLock.Lock()
 	defer s.leasesLock.Unlock()
 
-	if lease.IsStatic() {
+	if lease.IsStatic {
 		if lease.Hostname != "" {
 			// TODO(e.burkov):  This option is used to update the server's DNS
 			// mapping.  The option should only be answered when it has been

internal/dhcpd/v4_unix_test.go

@@ -73,6 +73,7 @@ func TestV4Server_leasing(t *testing.T) {
 			Hostname: staticName,
 			HWAddr:   staticMAC,
 			IP:       staticIP,
+			IsStatic: true,
 		})
 		require.NoError(t, err)
 
@@ -82,6 +83,7 @@ func TestV4Server_leasing(t *testing.T) {
 				Hostname: staticName,
 				HWAddr:   anotherMAC,
 				IP:       anotherIP,
+				IsStatic: true,
 			})
 			assert.ErrorIs(t, err, ErrDupHostname)
 		})
@@ -96,6 +98,7 @@ func TestV4Server_leasing(t *testing.T) {
 				Hostname: anotherName,
 				HWAddr:   staticMAC,
 				IP:       anotherIP,
+				IsStatic: true,
 			})
 			testutil.AssertErrorMsg(t, wantErrMsg, err)
 		})
@@ -110,6 +113,7 @@ func TestV4Server_leasing(t *testing.T) {
 				Hostname: anotherName,
 				HWAddr:   anotherMAC,
 				IP:       staticIP,
+				IsStatic: true,
 			})
 			testutil.AssertErrorMsg(t, wantErrMsg, err)
 		})
@@ -326,7 +330,7 @@ func TestV4_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.True(t, l.IsStatic())
+		assert.True(t, l.IsStatic)
 	}
 }
 
@@ -890,6 +894,7 @@ func TestV4Server_FindMACbyIP(t *testing.T) {
 			Hostname: staticName,
 			HWAddr:   staticMAC,
 			IP:       staticIP,
+			IsStatic: true,
 		}, {
 			Expiry:   time.Unix(10, 0),
 			Hostname: anotherName,

internal/dhcpd/v6_unix.go

@@ -121,7 +121,7 @@ func (s *v6Server) FindMACbyIP(ip netip.Addr) (mac net.HardwareAddr) {
 
 	for _, l := range s.leases {
 		if l.IP == ip {
-			if l.Expiry.After(now) || l.IsStatic() {
+			if l.IsStatic || l.Expiry.After(now) {
 				return l.HWAddr
 			}
 		}

internal/dhcpd/v6_unix_test.go

@@ -331,6 +331,7 @@ func TestV6_FindMACbyIP(t *testing.T) {
 			Hostname: staticName,
 			HWAddr:   staticMAC,
 			IP:       staticIP,
+			IsStatic: true,
 		}, {
 			Expiry:   time.Unix(10, 0),
 			Hostname: anotherName,
@@ -344,6 +345,7 @@ func TestV6_FindMACbyIP(t *testing.T) {
 		Hostname: staticName,
 		HWAddr:   staticMAC,
 		IP:       staticIP,
+		IsStatic: true,
 	}, {
 		Expiry:   time.Unix(10, 0),
 		Hostname: anotherName,

openapi/openapi.yaml

@@ -1849,7 +1849,6 @@
       - 'mac'
       - 'ip'
       - 'hostname'
-      - 'expires'
       'properties':
         'mac':
           'type': 'string'