Repository: favonia/cloudflare-ddns
Pull request #1022 (2025-01-24 22:44:22)

feat(api): warn about non-empty WAF list item comments
Related: #819.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/1022?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 95.04%. Comparing base [(`c358727`)](https://app.codecov.io/gh/favonia/cloudflare-ddns/commit/c358727d47f5cfcef2c79c8bb594538ea044bc47?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) to head [(`e71bd48`)](https://app.codecov.io/gh/favonia/cloudflare-ddns/commit/e71bd48e00ef024a4d29f16e591fdb8878c052a3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia).

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main    #1022   +/-   ##
=======================================
  Coverage   95.04%   95.04%           
=======================================
  Files          62       62           
  Lines        3430     3434    +4     
=======================================
+ Hits         3260     3264    +4     
  Misses        158      158           
  Partials       12       12           
```

| [Flag](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/1022/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/1022/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) | `95.04% <100.00%> (+<0.01%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/1022?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia).


Patch:
diff --git a/internal/api/cloudflare_waf.go b/internal/api/cloudflare_waf.go
index 07d3373..7d246ef 100644
--- a/internal/api/cloudflare_waf.go
+++ b/internal/api/cloudflare_waf.go
@@ -188,6 +188,10 @@ func readWAFListItems(ppfmt pp.PP, list WAFList, rawItems []cloudflare.ListItem)
 				*rawItem.IP, list.Describe())
 			return nil, false
 		}
+		if rawItem.Comment != "" {
+			ppfmt.Noticef(pp.EmojiWarning, "The IP range/address %q in the list %s has a non-empty comment %q; the comment might be lost during an IP update.", //nolint:lll
+				*rawItem.IP, list.Describe(), rawItem.Comment)
+		}
 		items = append(items, WAFListItem{ID: ID(rawItem.ID), Prefix: p})
 	}
 	return items, true


Test patch:
diff --git a/internal/api/cloudflare_waf_test.go b/internal/api/cloudflare_waf_test.go
index 97953c6..9fd7e28 100644
--- a/internal/api/cloudflare_waf_test.go
+++ b/internal/api/cloudflare_waf_test.go
@@ -532,21 +532,24 @@ func TestFinalClearWAFListAsync(t *testing.T) {
 	}
 }
 
-type listItem = string
+type listItem struct {
+	IP      string
+	Comment string
+}
 
 func mockListItem(listItem listItem) cloudflare.ListItem {
 	var ip *string
-	if listItem != "" {
-		ip = &listItem
+	if listItem.IP != "" {
+		ip = &listItem.IP
 	}
 
 	return cloudflare.ListItem{
-		ID:         string(mockID(listItem, 0)),
+		ID:         string(mockID(listItem.IP, 0)),
 		IP:         ip,
 		Redirect:   nil,
 		Hostname:   nil,
 		ASN:        nil,
-		Comment:    "",
+		Comment:    listItem.Comment,
 		CreatedOn:  nil,
 		ModifiedOn: nil,
 	}
@@ -616,7 +619,7 @@ func TestListWAFListItems(t *testing.T) {
 			1,
 			emptyListMeta,
 			0,
-			[]listItem{"10.0.0.1", "2001:db8::/32", "10.0.0.0/20"},
+			[]listItem{{"10.0.0.1", ""}, {"2001:db8::/32", ""}, {"10.0.0.0/20", ""}},
 			1,
 			true, true,
 			[]api.WAFListItem{
@@ -666,7 +669,7 @@ func TestListWAFListItems(t *testing.T) {
 			1,
 			emptyListMeta,
 			0,
-			[]listItem{"10.0.0.1"},
+			[]listItem{{"10.0.0.1", ""}},
 			0,
 			false, false, nil,
 			func(ppfmt *mocks.MockPP) {
@@ -678,7 +681,7 @@ func TestListWAFListItems(t *testing.T) {
 			1,
 			emptyListMeta,
 			0,
-			[]listItem{"invalid item"},
+			[]listItem{{"invalid item", ""}},
 			1,
 			false, false, nil,
 			func(ppfmt *mocks.MockPP) {
@@ -694,7 +697,7 @@ func TestListWAFListItems(t *testing.T) {
 			1,
 			emptyListMeta,
 			0,
-			[]listItem{""},
+			[]listItem{{"", ""}},
 			1,
 			false, false, nil,
 			func(ppfmt *mocks.MockPP) {
@@ -703,6 +706,23 @@ func TestListWAFListItems(t *testing.T) {
 					"account456/list")
 			},
 		},
+		"comment": {
+			[]listMeta{{name: "list", size: 5, kind: cloudflare.ListTypeIP}},
+			1,
+			emptyListMeta,
+			0,
+			[]listItem{{"10.0.0.1", "hello"}},
+			1,
+			true, true,
+			[]api.WAFListItem{
+				{ID: (mockID("10.0.0.1", 0)), Prefix: netip.MustParsePrefix("10.0.0.1/32")},
+			},
+			func(ppfmt *mocks.MockPP) {
+				ppfmt.EXPECT().Noticef(pp.EmojiWarning,
+					"The IP range/address %q in the list %s has a non-empty comment %q; the comment might be lost during an IP update.",
+					"10.0.0.1", "account456/list", "hello")
+			},
+		},
 	} {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
@@ -837,7 +857,7 @@ func TestDeleteWAFListItems(t *testing.T) {
 			1,
 			[]api.ID{"id1", "id2", "id3"},
 			1,
-			[]listItem{"10.0.0.1/32", "2001:db8::/32", "10.0.0.0/20"},
+			[]listItem{{"10.0.0.1/32", ""}, {"2001:db8::/32", ""}, {"10.0.0.0/20", ""}},
 			1, true,
 			nil,
 		},
@@ -867,7 +887,7 @@ func TestDeleteWAFListItems(t *testing.T) {
 			1,
 			[]api.ID{"id1", "id2", "id3"},
 			1,
-			[]listItem{"10.0.0.1/32", "2001:db8::/32", "invalid item"},
+			[]listItem{{"10.0.0.1/32", ""}, {"2001:db8::/32", ""}, {"invalid item", ""}},
 			1,
 			false,
 			func(ppfmt *mocks.MockPP) {
@@ -1010,7 +1030,7 @@ func TestCreateWAFListItems(t *testing.T) {
 			1,
 			[]netip.Prefix{netip.MustParsePrefix("10.0.0.1/16"), netip.MustParsePrefix("2001:db8::/50")},
 			1,
-			[]listItem{"10.0.0.1/32", "2001:db8::/32", "10.0.0.0/20"},
+			[]listItem{{"10.0.0.1/32", ""}, {"2001:db8::/32", ""}, {"10.0.0.0/20", ""}},
 			1,
 			true,
 			nil,
@@ -1041,7 +1061,7 @@ func TestCreateWAFListItems(t *testing.T) {
 			1,
 			[]netip.Prefix{netip.MustParsePrefix("10.0.0.1/16"), netip.MustParsePrefix("2001:db8::/50")},
 			1,
-			[]listItem{"10.0.0.1/32", "2001:db8::/32", "invalid item"},
+			[]listItem{{"10.0.0.1/32", ""}, {"2001:db8::/32", ""}, {"invalid item", ""}},
 			1,
 			false,
 			func(ppfmt *mocks.MockPP) {