- remove Lease.IDOptions and update corresponding struct / function

- add NewReleaseFromLease and update Client.Release() accordingly
- update lease_test.go accordingly
- remove example_lease_test.go

Signed-off-by: Hu Jun <hujun.work@gmail.com>

4 files changed, 34 insertions, 139 deletions

diff --git a/dhcpv4/nclient4/client.go b/dhcpv4/nclient4/client.go
index feeb4e6..83ed065 100644
--- a/dhcpv4/nclient4/client.go
+++ b/dhcpv4/nclient4/client.go
@@ -162,11 +162,6 @@ type Client struct {
 	// TransactionID. receiveLoop uses this map to determine which channel
 	// to send a new DHCP message to.
 	pending map[dhcpv4.TransactionID]*pendingCh
-
-	//clientIdOptions is a list of DHCPv4 option code that DHCP server used to
-	//identify client other than the HWAddress,
-	//like client-id, option82/remote-id..etc
-	clientIDOptions dhcpv4.OptionCodeList
 }
 
 // New returns a client usable with an unconfigured interface.
@@ -190,9 +185,8 @@ func new(iface string, conn net.PacketConn, ifaceHWAddr net.HardwareAddr, opts .
 		conn:        conn,
 		logger:      EmptyLogger{},
 
-		done:            make(chan struct{}),
-		pending:         make(map[dhcpv4.TransactionID]*pendingCh),
-		clientIDOptions: dhcpv4.OptionCodeList{},
+		done:    make(chan struct{}),
+		pending: make(map[dhcpv4.TransactionID]*pendingCh),
 	}
 
 	for _, opt := range opts {
@@ -462,11 +456,6 @@ func (c *Client) Request(ctx context.Context, modifiers ...dhcpv4.Modifier) (lea
 	lease.ACK = ack
 	lease.Offer = offer
 	lease.CreationTime = time.Now()
-	lease.IDOptions = dhcpv4.Options{}
-	for _, optioncode := range c.clientIDOptions {
-		v := request.Options.Get(optioncode)
-		lease.IDOptions.Update(dhcpv4.OptGeneric(optioncode, v))
-	}
 	return
 }
 
diff --git a/dhcpv4/nclient4/example_lease_test.go b/dhcpv4/nclient4/example_lease_test.go
deleted file mode 100644
index fa59a72..0000000
--- a/dhcpv4/nclient4/example_lease_test.go
+++ /dev/null
@@ -1,37 +0,0 @@
-package nclient4_test
-
-import (
-	"context"
-	"log"
-
-	"github.com/insomniacslk/dhcp/dhcpv4"
-	"github.com/insomniacslk/dhcp/dhcpv4/nclient4"
-)
-
-func Example() {
-	ifname := "enp0s10"
-	clntid := "client-1"
-	var idoptlist dhcpv4.OptionCodeList
-	//specify option61 is part of client identification used by DHCPv4 server
-	idoptlist.Add(dhcpv4.OptionClientIdentifier)
-	clntOptions := []nclient4.ClientOpt{nclient4.WithClientIDOptions(idoptlist), nclient4.WithDebugLogger()}
-	clnt, err := nclient4.New(ifname, clntOptions...)
-	if err != nil {
-		log.Fatalf("failed to create dhcpv4 client,%v", err)
-	}
-	//add option61 to discovery and request
-	option61 := dhcpv4.OptClientIdentifier([]byte(clntid))
-	lease, err := clnt.Request(context.Background(), dhcpv4.WithOption(option61))
-	if err != nil {
-		log.Fatal(err)
-	}
-	//print the lease
-	log.Printf("Got lease:\n%+v", lease)
-	//release the lease
-	log.Print("Releasing lease...")
-	err = clnt.Release(lease)
-	if err != nil {
-		log.Fatal(err)
-	}
-	log.Print("done")
-}
diff --git a/dhcpv4/nclient4/lease.go b/dhcpv4/nclient4/lease.go
index 838e39b..bd684b0 100644
--- a/dhcpv4/nclient4/lease.go
+++ b/dhcpv4/nclient4/lease.go
@@ -1,4 +1,4 @@
-//This is lease support for nclient4
+// This is lease support for nclient4
 
 package nclient4
 
@@ -10,51 +10,42 @@ import (
 	"github.com/insomniacslk/dhcp/dhcpv4"
 )
 
-//Lease contains a DHCPv4 lease after DORA.
-//note: Lease doesn't include binding interface name
+// Lease contains a DHCPv4 lease after DORA.
+// note: Lease doesn't include binding interface name
 type Lease struct {
 	Offer        *dhcpv4.DHCPv4
 	ACK          *dhcpv4.DHCPv4
 	CreationTime time.Time
-	IDOptions    dhcpv4.Options //DHCPv4 options to identify the client like client-id, option82/remote-id
 }
 
-// WithClientIDOptions configures a list of DHCPv4 option code that DHCP server
-// uses to identify client, beside the MAC address.
-func WithClientIDOptions(cidl dhcpv4.OptionCodeList) ClientOpt {
-	return func(c *Client) (err error) {
-		c.clientIDOptions = cidl
-		return
-	}
-}
-
-//Release send DHCPv4 release messsage to server, based on specified lease.
-//release is sent as unicast per RFC2131, section 4.4.4.
-//Note: some DHCP server requries of using assigned IP address as source IP,
-//use nclient4.WithUnicast to create client for such case.
-func (c *Client) Release(lease *Lease) error {
-	if lease == nil {
-		return fmt.Errorf("lease is nil")
-	}
-	modList := []dhcpv4.Modifier{
+// NewReleaseFromLease creates a DHCPv4 Release message from the lease.
+// default Release message without any Modifer is created as following:
+//  - option Message Type is Release
+//  - ClientIP is set to lease.ACK.YourIPAddr
+//  - ClientHWAddr is set to lease.ACK.ClientHWAddr
+//  - Unicast
+//  - option Server Identifier is set to ServerIdentifier of lease.ACK
+func NewReleaseFromLease(lease *Lease, modifiers ...dhcpv4.Modifier) (*dhcpv4.DHCPv4, error) {
+	return dhcpv4.New(dhcpv4.PrependModifiers(modifiers,
 		dhcpv4.WithMessageType(dhcpv4.MessageTypeRelease),
 		dhcpv4.WithClientIP(lease.ACK.YourIPAddr),
 		dhcpv4.WithHwAddr(lease.ACK.ClientHWAddr),
 		dhcpv4.WithBroadcast(false),
 		dhcpv4.WithOption(dhcpv4.OptServerIdentifier(lease.ACK.ServerIdentifier())),
-	}
+	)...)
+}
 
-	req, err := dhcpv4.New(modList...)
-	if err != nil {
-		return err
+// Release send DHCPv4 release messsage to server, based on specified lease.
+// release is sent as unicast per RFC2131, section 4.4.4.
+// Note: some DHCP server requries of using assigned IP address as source IP,
+// use nclient4.WithUnicast to create client for such case.
+func (c *Client) Release(lease *Lease, modifiers ...dhcpv4.Modifier) error {
+	if lease == nil {
+		return fmt.Errorf("lease is nil")
 	}
-	//This is to make sure use same client identification options used during
-	//DORA, so that DHCP server could identify the required lease
-	for t := range lease.IDOptions {
-		req.UpdateOption(
-			dhcpv4.OptGeneric(dhcpv4.GenericOptionCode(t),
-				lease.IDOptions.Get(dhcpv4.GenericOptionCode(t))),
-		)
+	req, err := NewReleaseFromLease(lease, modifiers...)
+	if err != nil {
+		return fmt.Errorf("fail to create release request,%w", err)
 	}
 	_, err = c.conn.WriteTo(req.ToBytes(), &net.UDPAddr{IP: lease.ACK.Options.Get(dhcpv4.OptionServerIdentifier), Port: ServerPort})
 	if err == nil {
diff --git a/dhcpv4/nclient4/lease_test.go b/dhcpv4/nclient4/lease_test.go
index 96dfae0..361847f 100644
--- a/dhcpv4/nclient4/lease_test.go
+++ b/dhcpv4/nclient4/lease_test.go
@@ -3,7 +3,6 @@
 package nclient4
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"log"
@@ -18,14 +17,10 @@ import (
 )
 
 type testLeaseKey struct {
-	mac       net.HardwareAddr
-	idOptions dhcpv4.Options
+	mac net.HardwareAddr
 }
 
 func (lk testLeaseKey) compare(b testLeaseKey) bool {
-	if !bytes.Equal(lk.idOptions.ToBytes(), b.idOptions.ToBytes()) {
-		return false
-	}
 	for i := 0; i < 6; i++ {
 		if lk.mac[i] != b.mac[i] {
 			return false
@@ -43,14 +38,12 @@ type testServerLease struct {
 
 type testServerLeaseList struct {
 	list               []*testServerLease
-	clientIDOptions    dhcpv4.OptionCodeList
 	lastTestSvrErr     error
 	lastTestSvrErrLock *sync.RWMutex
 }
 
 func newtestServerLeaseList(l dhcpv4.OptionCodeList) *testServerLeaseList {
 	r := &testServerLeaseList{}
-	r.clientIDOptions = l
 	r.lastTestSvrErrLock = &sync.RWMutex{}
 	return r
 }
@@ -67,11 +60,6 @@ func (sll testServerLeaseList) get(k *testLeaseKey) *testServerLease {
 func (sll *testServerLeaseList) getKey(m *dhcpv4.DHCPv4) *testLeaseKey {
 	key := &testLeaseKey{}
 	key.mac = m.ClientHWAddr
-	key.idOptions = make(dhcpv4.Options)
-	for _, optioncode := range sll.clientIDOptions {
-		v := m.Options.Get(optioncode)
-		key.idOptions.Update(dhcpv4.OptGeneric(optioncode, v))
-	}
 	return key
 
 }
@@ -118,9 +106,6 @@ func (sll *testServerLeaseList) getCheckList() (mustHaveOpts, mayHaveOpts map[ui
 	mustHaveOpts[dhcpv4.OptionDHCPMessageType.Code()] = false
 	mustHaveOpts[dhcpv4.OptionServerIdentifier.Code()] = false
 
-	for _, o := range sll.clientIDOptions {
-		mustHaveOpts[o.Code()] = false
-	}
 	mayHaveOpts[dhcpv4.OptionClassIdentifier.Code()] = false
 	mayHaveOpts[dhcpv4.OptionMessage.Code()] = false
 	return
@@ -231,10 +216,10 @@ func (sll *testServerLeaseList) runTest(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		modList := []dhcpv4.Modifier{}
-		for op, val := range l.key.idOptions {
-			modList = append(modList, dhcpv4.WithOption(dhcpv4.OptGeneric(dhcpv4.GenericOptionCode(op), val)))
-		}
+		// modList := []dhcpv4.Modifier{}
+		// for op, val := range l.key.idOptions {
+		// 	modList = append(modList, dhcpv4.WithOption(dhcpv4.OptGeneric(dhcpv4.GenericOptionCode(op), val)))
+		// }
 		chkerr := func(err, lastsvrerr error, shouldfail bool, t *testing.T) bool {
 			if err != nil || lastsvrerr != nil {
 				if !shouldfail {
@@ -247,7 +232,7 @@ func (sll *testServerLeaseList) runTest(t *testing.T) {
 			return true
 		}
 
-		lease, err := clnt.Request(context.Background(), modList...)
+		lease, err := clnt.Request(context.Background())
 		sll.lastTestSvrErrLock.RLock()
 		keepgoing := chkerr(err, sll.lastTestSvrErr, l.ShouldFail, t)
 		sll.lastTestSvrErrLock.RUnlock()
@@ -267,11 +252,6 @@ func (sll *testServerLeaseList) runTest(t *testing.T) {
 func testCreateClientWithServerLease(conn net.PacketConn, sl *testServerLease) (*Client, error) {
 	clntModList := []ClientOpt{WithRetry(1), WithTimeout(2 * time.Second)}
 	clntModList = append(clntModList, WithHWAddr(sl.key.mac))
-	var idoptlist dhcpv4.OptionCodeList
-	for op := range sl.key.idOptions {
-		idoptlist.Add(dhcpv4.GenericOptionCode(op))
-	}
-	clntModList = append(clntModList, WithClientIDOptions(idoptlist))
 	clnt, err := NewWithConn(conn, sl.key.mac, clntModList...)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create dhcpv4 client,%v", err)
@@ -289,9 +269,6 @@ func TestLease(t *testing.T) {
 			assignedAddr: net.ParseIP("192.168.1.1"),
 			key: &testLeaseKey{
 				mac: net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 1, 1},
-				idOptions: dhcpv4.Options{
-					uint8(dhcpv4.OptionClientIdentifier): []byte("client-1"),
-				},
 			},
 		},
 
@@ -299,19 +276,7 @@ func TestLease(t *testing.T) {
 			assignedAddr: net.ParseIP("192.168.1.2"),
 			key: &testLeaseKey{
 				mac: net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 1, 2},
-				idOptions: dhcpv4.Options{
-					uint8(dhcpv4.OptionClientIdentifier): []byte("client-2"),
-				},
-			},
-		},
-		//negative case
-		&testServerLease{
-			assignedAddr: net.ParseIP("192.168.2.2"),
-			key: &testLeaseKey{
-				mac:       net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 1, 3},
-				idOptions: dhcpv4.Options{},
 			},
-			ShouldFail: true,
 		},
 	}
 	sll.runTest(t)
@@ -322,28 +287,15 @@ func TestLease(t *testing.T) {
 		&testServerLease{
 			assignedAddr: net.ParseIP("192.168.2.1"),
 			key: &testLeaseKey{
-				mac:       net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 2, 1},
-				idOptions: dhcpv4.Options{},
+				mac: net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 2, 1},
 			},
 		},
 
 		&testServerLease{
 			assignedAddr: net.ParseIP("192.168.2.2"),
 			key: &testLeaseKey{
-				mac:       net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 2, 2},
-				idOptions: dhcpv4.Options{},
-			},
-		},
-		//negative case
-		&testServerLease{
-			assignedAddr: net.ParseIP("192.168.2.2"),
-			key: &testLeaseKey{
-				mac: net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 2, 3},
-				idOptions: dhcpv4.Options{
-					uint8(dhcpv4.OptionClientIdentifier): []byte("client-fake"),
-				},
+				mac: net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 2, 2},
 			},
-			ShouldFail: true,
 		},
 	}
 	sll.runTest(t)