Repository: favonia/cloudflare-ddns
Pull request #885 (2024-08-23 13:24:07)

fix(updater): delete unmanaged IP addresses from lists


Patch:
diff --git a/README.markdown b/README.markdown
index 3f644ae..60c614b 100644
--- a/README.markdown
+++ b/README.markdown
@@ -361,7 +361,7 @@ _(Click to expand the following items.)_
 <details>
 <summary>📜 Web Application Firewalls (WAF) Lists</summary>
 
-> 🧪 The feature to manipulate WAF lists is experimental (introduced in 1.14.0) and is subject to changes.
+> 🧪 The feature to manipulate WAF lists is experimental (introduced in 1.14.0) and is subject to changes. In particular, the updater currently deletes unmanaged IPs from WAF lists (e.g., deleting IPv6 addresses if you are not using IPv6), but another reasonable implementation is to leave them alone. Please [open a GitHub issue](https://github.com/favonia/cloudflare-ddns/issues/new) to provide feedback. Thanks!
 
 | Name                      | Valid Values                                                                                                                                                                                                                                                                                                 | Meaning                                                                                                                                                                                                                            | Required? | Default Value |
 | ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ------------- |
diff --git a/internal/setter/setter.go b/internal/setter/setter.go
index 066ec17..6bf85f3 100644
--- a/internal/setter/setter.go
+++ b/internal/setter/setter.go
@@ -202,10 +202,10 @@ func (s setter) Delete(ctx context.Context, ppfmt pp.PP, ipnet ipnet.Type, domai
 
 // SetWAFList updates a WAF list.
 //
-// If detectedIPs contains a zero (invalid) IP, it means the detection is attempted but failed
+// If detectedIP contains a zero (invalid) IP, it means the detection is attempted but failed
 // and all matching IP addresses should be preserved.
 func (s setter) SetWAFList(ctx context.Context, ppfmt pp.PP,
-	list api.WAFList, listDescription string, detectedIPs map[ipnet.Type]netip.Addr, itemComment string,
+	list api.WAFList, listDescription string, detectedIP map[ipnet.Type]netip.Addr, itemComment string,
 ) ResponseCode {
 	listID, alreadyExisting, ok := s.Handle.EnsureWAFList(ctx, ppfmt, list, listDescription)
 	if !ok {
@@ -223,7 +223,7 @@ func (s setter) SetWAFList(ctx context.Context, ppfmt pp.PP,
 	var itemsToDelete []api.WAFListItem
 	var itemsToCreate []netip.Prefix
 	for _, ipNet := range [...]ipnet.Type{ipnet.IP4, ipnet.IP6} {
-		detectedIP, managed := detectedIPs[ipNet]
+		detectedIP, managed := detectedIP[ipNet]
 		covered := false
 		for _, item := range items {
 			if ipNet.Matches(item.Prefix.Addr()) {
diff --git a/internal/updater/updater.go b/internal/updater/updater.go
index e794723..de6d57d 100644
--- a/internal/updater/updater.go
+++ b/internal/updater/updater.go
@@ -126,14 +126,14 @@ func deleteIP(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Sette
 
 // setWAFList extracts relevant settings from the configuration and calls [setter.Setter.SetWAFList] with timeout.
 func setWAFLists(ctx context.Context, ppfmt pp.PP,
-	c *config.Config, s setter.Setter, detectedIPs map[ipnet.Type]netip.Addr,
+	c *config.Config, s setter.Setter, detectedIP map[ipnet.Type]netip.Addr,
 ) message.Message {
 	resps := emptySetterWAFListResponses()
 
 	for _, l := range c.WAFLists {
 		resps.register(l.Describe(),
 			wrapUpdateWithTimeout(ctx, ppfmt, c, func(ctx context.Context) setter.ResponseCode {
-				return s.SetWAFList(ctx, ppfmt, l, c.WAFListDescription, detectedIPs, "")
+				return s.SetWAFList(ctx, ppfmt, l, c.WAFListDescription, detectedIP, "")
 			}),
 		)
 	}
@@ -160,27 +160,28 @@ func deleteWAFLists(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter
 // UpdateIPs detect IP addresses and update DNS records of managed domains.
 func UpdateIPs(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Setter) message.Message {
 	var msgs []message.Message
-	detectedIPs := make(map[ipnet.Type]netip.Addr)
-
-	detectedAnyIP := false
+	detectedIP := map[ipnet.Type]netip.Addr{}
+	numManagedNetworks := 0
+	numValidIPs := 0
 	for _, ipNet := range [...]ipnet.Type{ipnet.IP4, ipnet.IP6} {
 		if c.Provider[ipNet] != nil {
+			numManagedNetworks++
 			ip, msg := detectIP(ctx, ppfmt, c, ipNet)
-			detectedIPs[ipNet] = ip
+			detectedIP[ipNet] = ip
 			msgs = append(msgs, msg)
 
 			// Note: If we can't detect the new IP address,
 			// it's probably better to leave existing records alone.
 			if msg.OK {
-				detectedAnyIP = true
+				numValidIPs++
 				msgs = append(msgs, setIP(ctx, ppfmt, c, s, ipNet, ip))
 			}
 		}
 	}
 
 	// Update WAF lists
-	if detectedAnyIP {
-		msgs = append(msgs, setWAFLists(ctx, ppfmt, c, s, detectedIPs))
+	if !(numManagedNetworks == 2 && numValidIPs == 0) {
+		msgs = append(msgs, setWAFLists(ctx, ppfmt, c, s, detectedIP))
 	}
 
 	return message.Merge(msgs...)


Test patch:
diff --git a/internal/updater/updater_test.go b/internal/updater/updater_test.go
index b2e1312..3879f71 100644
--- a/internal/updater/updater_test.go
+++ b/internal/updater/updater_test.go
@@ -598,7 +598,7 @@ func TestUpdateIPs(t *testing.T) {
 			[]string{"Failed to detect IPv4 address"},
 			[]string{"Failed to detect the IPv4 address."},
 			providerEnablers{ipnet.IP4: true},
-			func(p *mocks.MockPP, pv mockProviders, _ *mocks.MockSetter) {
+			func(p *mocks.MockPP, pv mockProviders, s *mocks.MockSetter) {
 				gomock.InOrder(
 					pv[ipnet.IP4].EXPECT().GetIP(gomock.Any(), p, ipnet.IP4, true).
 						DoAndReturn(
@@ -610,6 +610,8 @@ func TestUpdateIPs(t *testing.T) {
 					p.EXPECT().Noticef(pp.EmojiError, "Failed to detect the %s address", "IPv4"),
 					p.EXPECT().Hintf(pp.HintIP4DetectionFails, "If your network does not support IPv4, you can disable it with IP4_PROVIDER=none"),                    //nolint:lll
 					p.EXPECT().Hintf(pp.HintDetectionTimeouts, "If your network is experiencing high latency, consider increasing DETECTION_TIMEOUT=%v", time.Second), //nolint:lll
+					s.EXPECT().SetWAFList(gomock.Any(), p, list, wafListDescription, detectedIPs{ipnet.IP4: netip.Addr{}}, "").
+						Return(setter.ResponseNoop),
 				)
 			},
 		},