From 94cdde992d5661bda1c88769d0a4319f3037c81e Mon Sep 17 00:00:00 2001
From: Dennis Marttinen <twelho@welho.tech>
Date: Mon, 27 Mar 2023 01:00:58 +0300
Subject: dhcpv4: change `NewRenewFromOffer` to `NewRenewFromAck`

Signed-off-by: Dennis Marttinen <twelho@welho.tech>
---
 dhcpv4/dhcpv4.go         | 22 +++++++++++++---------
 dhcpv4/dhcpv4_test.go    | 27 +++++++++++++--------------
 dhcpv4/nclient4/lease.go |  2 +-
 3 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/dhcpv4/dhcpv4.go b/dhcpv4/dhcpv4.go
index 89c7184..6043cd9 100644
--- a/dhcpv4/dhcpv4.go
+++ b/dhcpv4/dhcpv4.go
@@ -248,18 +248,22 @@ func NewRequestFromOffer(offer *DHCPv4, modifiers ...Modifier) (*DHCPv4, error)
 	)...)
 }
 
-// NewRenewFromOffer builds a DHCPv4 RENEW-style request from an offer. RENEW requests have minor
-// changes to their options compared to SELECT requests as specified by RFC 2131, section 4.3.2.
-func NewRenewFromOffer(offer *DHCPv4, modifiers ...Modifier) (*DHCPv4, error) {
-	return NewRequestFromOffer(offer, PrependModifiers(modifiers,
-		// The server identifier option must not be filled in
-		WithoutOption(OptionServerIdentifier),
-		// The requested IP address must not be filled in
-		WithoutOption(OptionRequestedIPAddress),
+// NewRenewFromAck builds a DHCPv4 RENEW-style request from the ACK of a lease. RENEW requests have
+// minor changes to their options compared to SELECT requests as specified by RFC 2131, section 4.3.2.
+func NewRenewFromAck(ack *DHCPv4, modifiers ...Modifier) (*DHCPv4, error) {
+	return New(PrependModifiers(modifiers,
+		WithReply(ack),
+		WithMessageType(MessageTypeRequest),
 		// The client IP must be filled in with the IP offered to the client
-		WithClientIP(offer.YourIPAddr),
+		WithClientIP(ack.YourIPAddr),
 		// The renewal request must use unicast
 		WithBroadcast(false),
+		WithRequestedOptions(
+			OptionSubnetMask,
+			OptionRouter,
+			OptionDomainName,
+			OptionDomainNameServer,
+		),
 	)...)
 }
 
diff --git a/dhcpv4/dhcpv4_test.go b/dhcpv4/dhcpv4_test.go
index d2d6edd..a961f78 100644
--- a/dhcpv4/dhcpv4_test.go
+++ b/dhcpv4/dhcpv4_test.go
@@ -266,23 +266,22 @@ func TestDHCPv4NewRequestFromOfferWithModifier(t *testing.T) {
 	require.Equal(t, MessageTypeRequest, req.MessageType())
 }
 
-func TestDHCPv4NewRenewFromOffer(t *testing.T) {
-	offer, err := New()
+func TestDHCPv4NewRenewFromAck(t *testing.T) {
+	ack, err := New()
 	require.NoError(t, err)
-	offer.SetBroadcast()
-	offer.UpdateOption(OptMessageType(MessageTypeOffer))
-	offer.UpdateOption(OptServerIdentifier(net.IPv4(192, 168, 0, 1)))
-	offer.UpdateOption(OptRequestedIPAddress(net.IPv4(192, 168, 0, 1)))
-	offer.YourIPAddr = net.IPv4(192, 168, 0, 1)
+	ack.SetBroadcast()
+	ack.UpdateOption(OptMessageType(MessageTypeAck))
+	ack.UpdateOption(OptServerIdentifier(net.IPv4(192, 168, 0, 1)))
+	ack.YourIPAddr = net.IPv4(192, 168, 0, 1)
 
-	// RFC 2131: RENEW-style requests will be unicast
 	var req *DHCPv4
-	req, err = NewRenewFromOffer(offer)
+	req, err = NewRenewFromAck(ack)
 	require.NoError(t, err)
 	require.Equal(t, MessageTypeRequest, req.MessageType())
 	require.Nil(t, req.GetOneOption(OptionServerIdentifier))
 	require.Nil(t, req.GetOneOption(OptionRequestedIPAddress))
-	require.Equal(t, offer.YourIPAddr, req.ClientIPAddr)
+	require.Equal(t, ack.YourIPAddr, req.ClientIPAddr)
+	// RFC 2131: RENEW-style requests will be unicast
 	require.True(t, req.IsUnicast())
 	require.False(t, req.IsBroadcast())
 	// Renewals should behave identically to initial requests regarding requested options
@@ -292,12 +291,12 @@ func TestDHCPv4NewRenewFromOffer(t *testing.T) {
 	require.True(t, req.IsOptionRequested(OptionDomainNameServer))
 }
 
-func TestDHCPv4NewRenewFromOfferWithModifier(t *testing.T) {
-	offer, err := New()
+func TestDHCPv4NewRenewFromAckWithModifier(t *testing.T) {
+	ack, err := New()
 	require.NoError(t, err)
-	offer.UpdateOption(OptMessageType(MessageTypeOffer))
+	ack.UpdateOption(OptMessageType(MessageTypeAck))
 	userClass := WithUserClass("linuxboot", false)
-	req, err := NewRenewFromOffer(offer, userClass)
+	req, err := NewRenewFromAck(ack, userClass)
 	require.NoError(t, err)
 	require.Equal(t, MessageTypeRequest, req.MessageType())
 	require.Contains(t, req.UserClass(), "linuxboot")
diff --git a/dhcpv4/nclient4/lease.go b/dhcpv4/nclient4/lease.go
index 9ee3109..8b1d9e6 100644
--- a/dhcpv4/nclient4/lease.go
+++ b/dhcpv4/nclient4/lease.go
@@ -47,7 +47,7 @@ func (c *Client) Renew(ctx context.Context, lease *Lease, modifiers ...dhcpv4.Mo
 		return nil, fmt.Errorf("lease is nil")
 	}
 
-	request, err := dhcpv4.NewRenewFromOffer(lease.Offer, dhcpv4.PrependModifiers(modifiers,
+	request, err := dhcpv4.NewRenewFromAck(lease.ACK, dhcpv4.PrependModifiers(modifiers,
 		dhcpv4.WithOption(dhcpv4.OptMaxMessageSize(MaxMessageSize)))...)
 	if err != nil {
 		return nil, fmt.Errorf("unable to create a request: %w", err)
-- 
cgit v1.2.3