Repository: favonia/cloudflare-ddns
Pull request #875 (2024-08-12 17:12:38)

fix(api): decouple account IDs from operations on DNS records
This will completely prevent #691 from happening again!

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/875?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 94.76%. Comparing base [(`7d4e834`)](https://app.codecov.io/gh/favonia/cloudflare-ddns/commit/7d4e83457be095437c56bb8de8643064b9255cac?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) to head [(`e356784`)](https://app.codecov.io/gh/favonia/cloudflare-ddns/commit/e356784a3b1254ab2f99d5708879d3ec7f34be8d?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     #875      +/-   ##
==========================================
- Coverage   94.76%   94.76%   -0.01%     
==========================================
  Files          58       58              
  Lines        2618     2616       -2     
==========================================
- Hits         2481     2479       -2     
  Misses        125      125              
  Partials       12       12              
```

| [Flag](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/875/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/875/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) | `94.76% <100.00%> (-0.01%)` | :arrow_down: |

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/875?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_records.go b/internal/api/cloudflare_records.go
index 0053e28..029c4e8 100644
--- a/internal/api/cloudflare_records.go
+++ b/internal/api/cloudflare_records.go
@@ -25,7 +25,7 @@ func (h CloudflareHandle) ListZones(ctx context.Context, ppfmt pp.PP, name strin
 		return ids.Value(), true
 	}
 
-	res, err := h.cf.ListZonesContext(ctx, cloudflare.WithZoneFilters(name, h.accountID, ""))
+	res, err := h.cf.ListZonesContext(ctx, cloudflare.WithZoneFilters(name, "", ""))
 	if err != nil {
 		ppfmt.Warningf(pp.EmojiError, "Failed to check the existence of a zone named %q: %v", name, err)
 		return nil, false
@@ -98,9 +98,6 @@ zoneSearch:
 	}
 
 	ppfmt.Warningf(pp.EmojiError, "Failed to find the zone of %q", domain.Describe())
-	if h.accountID != "" {
-		ppfmt.Infof(pp.EmojiHint, "Double-check the value of CF_ACCOUNT_ID; you can usually leave it blank unless you are updating WAF lists") //nolint:lll
-	}
 
 	return "", false
 }


Test patch:
diff --git a/internal/api/cloudflare_records_test.go b/internal/api/cloudflare_records_test.go
index 8ff88ae..fe95c2c 100644
--- a/internal/api/cloudflare_records_test.go
+++ b/internal/api/cloudflare_records_test.go
@@ -53,7 +53,7 @@ func mockZonesResponse(zoneName string, zoneStatuses []string) cloudflare.ZonesR
 	}
 }
 
-func newZonesHandler(t *testing.T, mux *http.ServeMux, accountID string, zoneStatuses map[string][]string) httpHandler {
+func newZonesHandler(t *testing.T, mux *http.ServeMux, zoneStatuses map[string][]string) httpHandler {
 	t.Helper()
 
 	var requestLimit int
@@ -67,23 +67,12 @@ func newZonesHandler(t *testing.T, mux *http.ServeMux, accountID string, zoneSta
 		zoneName := r.URL.Query().Get("name")
 		zoneStatuses := zoneStatuses[zoneName]
 
-		if accountID == "" {
-			if !assert.Equal(t, url.Values{
-				"name":     {zoneName},
-				"per_page": {strconv.Itoa(zonePageSize)},
-			}, r.URL.Query()) {
-				w.WriteHeader(http.StatusBadRequest)
-				return
-			}
-		} else {
-			if !assert.Equal(t, url.Values{
-				"account.id": {mockAccountID},
-				"name":       {zoneName},
-				"per_page":   {strconv.Itoa(zonePageSize)},
-			}, r.URL.Query()) {
-				w.WriteHeader(http.StatusBadRequest)
-				return
-			}
+		if !assert.Equal(t, url.Values{
+			"name":     {zoneName},
+			"per_page": {strconv.Itoa(zonePageSize)},
+		}, r.URL.Query()) {
+			w.WriteHeader(http.StatusBadRequest)
+			return
 		}
 
 		w.Header().Set("Content-Type", "application/json")
@@ -132,7 +121,7 @@ func TestListZonesTwo(t *testing.T) {
 			mockPP := mocks.NewMockPP(mockCtrl)
 			mux, h, ok := newGoodHandle(t, mockPP)
 			require.True(t, ok)
-			zh := newZonesHandler(t, mux, mockAccountID, tc.zones)
+			zh := newZonesHandler(t, mux, tc.zones)
 
 			zh.setRequestLimit(tc.requestLimit)
 			mockPP = mocks.NewMockPP(mockCtrl)
@@ -172,7 +161,6 @@ func TestZoneOfDomain(t *testing.T) {
 	t.Parallel()
 
 	for name, tc := range map[string]struct {
-		accountID     string
 		zone          string
 		domain        domain.Domain
 		zoneStatuses  map[string][]string
@@ -181,33 +169,27 @@ func TestZoneOfDomain(t *testing.T) {
 		ok            bool
 		prepareMockPP func(*mocks.MockPP)
 	}{
-		"root":     {mockAccountID, "test.org", domain.FQDN("test.org"), map[string][]string{"test.org": {"active"}}, 1, mockID("test.org", 0), true, nil},     //nolint:lll
-		"wildcard": {mockAccountID, "test.org", domain.Wildcard("test.org"), map[string][]string{"test.org": {"active"}}, 1, mockID("test.org", 0), true, nil}, //nolint:lll
-		"one":      {mockAccountID, "test.org", domain.FQDN("sub.test.org"), map[string][]string{"test.org": {"active"}}, 2, mockID("test.org", 0), true, nil}, //nolint:lll
+		"root":     {"test.org", domain.FQDN("test.org"), map[string][]string{"test.org": {"active"}}, 1, mockID("test.org", 0), true, nil},     //nolint:lll
+		"wildcard": {"test.org", domain.Wildcard("test.org"), map[string][]string{"test.org": {"active"}}, 1, mockID("test.org", 0), true, nil}, //nolint:lll
+		"one":      {"test.org", domain.FQDN("sub.test.org"), map[string][]string{"test.org": {"active"}}, 2, mockID("test.org", 0), true, nil}, //nolint:lll
 		"none": {
-			mockAccountID, "test.org", domain.FQDN("sub.test.org"),
+			"test.org", domain.FQDN("sub.test.org"),
 			map[string][]string{},
 			3, "", false,
 			func(m *mocks.MockPP) {
-				gomock.InOrder(
-					m.EXPECT().Warningf(pp.EmojiError, "Failed to find the zone of %q", "sub.test.org"),
-					m.EXPECT().Infof(pp.EmojiHint, "Double-check the value of CF_ACCOUNT_ID; you can usually leave it blank unless you are updating WAF lists"), //nolint:lll
-				)
+				m.EXPECT().Warningf(pp.EmojiError, "Failed to find the zone of %q", "sub.test.org")
 			},
 		},
 		"none/wildcard": {
-			mockAccountID, "test.org", domain.Wildcard("test.org"),
+			"test.org", domain.Wildcard("test.org"),
 			map[string][]string{},
 			2, "", false,
 			func(m *mocks.MockPP) {
-				gomock.InOrder(
-					m.EXPECT().Warningf(pp.EmojiError, "Failed to find the zone of %q", "*.test.org"),
-					m.EXPECT().Infof(pp.EmojiHint, "Double-check the value of CF_ACCOUNT_ID; you can usually leave it blank unless you are updating WAF lists"), //nolint:lll
-				)
+				m.EXPECT().Warningf(pp.EmojiError, "Failed to find the zone of %q", "*.test.org")
 			},
 		},
 		"multiple": {
-			mockAccountID, "test.org", domain.FQDN("sub.test.org"),
+			"test.org", domain.FQDN("sub.test.org"),
 			map[string][]string{"test.org": {"active", "active"}},
 			2, "", false,
 			func(m *mocks.MockPP) {
@@ -221,7 +203,7 @@ func TestZoneOfDomain(t *testing.T) {
 			},
 		},
 		"multiple/wildcard": {
-			mockAccountID, "test.org", domain.Wildcard("test.org"),
+			"test.org", domain.Wildcard("test.org"),
 			map[string][]string{"test.org": {"active", "active"}},
 			1, "", false,
 			func(m *mocks.MockPP) {
@@ -235,19 +217,7 @@ func TestZoneOfDomain(t *testing.T) {
 			},
 		},
 		"deleted": {
-			mockAccountID, "test.org", domain.FQDN("test.org"),
-			map[string][]string{"test.org": {"deleted"}},
-			2, "", false,
-			func(m *mocks.MockPP) {
-				gomock.InOrder(
-					m.EXPECT().Infof(pp.EmojiWarning, "Zone %q is %q and thus skipped", "test.org", "deleted"),
-					m.EXPECT().Warningf(pp.EmojiError, "Failed to find the zone of %q", "test.org"),
-					m.EXPECT().Infof(pp.EmojiHint, "Double-check the value of CF_ACCOUNT_ID; you can usually leave it blank unless you are updating WAF lists"), //nolint:lll
-				)
-			},
-		},
-		"deleted/empty-account": {
-			"", "test.org", domain.FQDN("test.org"),
+			"test.org", domain.FQDN("test.org"),
 			map[string][]string{"test.org": {"deleted"}},
 			2, "", false,
 			func(m *mocks.MockPP) {
@@ -258,7 +228,7 @@ func TestZoneOfDomain(t *testing.T) {
 			},
 		},
 		"pending": {
-			mockAccountID, "test.org", domain.FQDN("test.org"),
+			"test.org", domain.FQDN("test.org"),
 			map[string][]string{"test.org": {"pending"}},
 			1, mockID("test.org", 0), true,
 			func(m *mocks.MockPP) {
@@ -268,7 +238,7 @@ func TestZoneOfDomain(t *testing.T) {
 			},
 		},
 		"initializing": {
-			mockAccountID, "test.org", domain.FQDN("test.org"),
+			"test.org", domain.FQDN("test.org"),
 			map[string][]string{"test.org": {"initializing"}},
 			1, mockID("test.org", 0), true,
 			func(m *mocks.MockPP) {
@@ -280,7 +250,7 @@ func TestZoneOfDomain(t *testing.T) {
 			},
 		},
 		"undocumented": {
-			mockAccountID, "test.org", domain.FQDN("test.org"),
+			"test.org", domain.FQDN("test.org"),
 			map[string][]string{"test.org": {"some-undocumented-status"}},
 			1, mockID("test.org", 0), true,
 			func(m *mocks.MockPP) {
@@ -294,10 +264,9 @@ func TestZoneOfDomain(t *testing.T) {
 			t.Parallel()
 			mockCtrl := gomock.NewController(t)
 			mockPP := mocks.NewMockPP(mockCtrl)
-			mux, h, ok := newHandle(t, mockPP, tc.accountID,
-				http.StatusOK, mockTokenVerifyResponse, http.StatusOK, mockAccountResponse)
+			mux, h, ok := newGoodHandle(t, mockPP)
 			require.True(t, ok)
-			zh := newZonesHandler(t, mux, tc.accountID, tc.zoneStatuses)
+			zh := newZonesHandler(t, mux, tc.zoneStatuses)
 
 			zh.setRequestLimit(tc.requestLimit)
 			mockPP = mocks.NewMockPP(mockCtrl)
@@ -412,7 +381,7 @@ func TestListRecords(t *testing.T) {
 	mux, h, ok := newGoodHandle(t, mockPP)
 	require.True(t, ok)
 
-	zh := newZonesHandler(t, mux, mockAccountID, map[string][]string{"test.org": {"active"}})
+	zh := newZonesHandler(t, mux, map[string][]string{"test.org": {"active"}})
 	zh.setRequestLimit(2)
 
 	lrh := newListRecordsHandler(t, mux, ipnet.IP6, "sub.test.org",
@@ -444,7 +413,7 @@ func TestListRecordsInvalidIPAddress(t *testing.T) {
 	mux, h, ok := newGoodHandle(t, mockPP)
 	require.True(t, ok)
 
-	zh := newZonesHandler(t, mux, mockAccountID, map[string][]string{"test.org": {"active"}})
+	zh := newZonesHandler(t, mux, map[string][]string{"test.org": {"active"}})
 	zh.setRequestLimit(2)
 
 	lrh := newListRecordsHandler(t, mux, ipnet.IP6, "sub.test.org",
@@ -490,7 +459,7 @@ func TestListRecordsWildcard(t *testing.T) {
 	mux, h, ok := newGoodHandle(t, mockPP)
 	require.True(t, ok)
 
-	zh := newZonesHandler(t, mux, mockAccountID, map[string][]string{"test.org": {"active"}})
+	zh := newZonesHandler(t, mux, map[string][]string{"test.org": {"active"}})
 	zh.setRequestLimit(2)
 
 	lrh := newListRecordsHandler(t, mux, ipnet.IP6, "*.test.org",
@@ -521,7 +490,7 @@ func TestListRecordsInvalidDomain(t *testing.T) {
 	mux, h, ok := newGoodHandle(t, mockPP)
 	require.True(t, ok)
 
-	zh := newZonesHandler(t, mux, mockAccountID, map[string][]string{"test.org": {"active"}})
+	zh := newZonesHandler(t, mux, map[string][]string{"test.org": {"active"}})
 	zh.setRequestLimit(2)
 
 	mockPP.EXPECT().Warningf(pp.EmojiError, "Failed to retrieve %s records of %q: %v", "A", "sub.test.org", gomock.Any())
@@ -616,7 +585,7 @@ func TestDeleteRecordValid(t *testing.T) {
 	mux, h, ok := newGoodHandle(t, mockPP)
 	require.True(t, ok)
 
-	zh := newZonesHandler(t, mux, mockAccountID, map[string][]string{"test.org": {"active"}})
+	zh := newZonesHandler(t, mux, map[string][]string{"test.org": {"active"}})
 	zh.setRequestLimit(2)
 
 	lrh := newListRecordsHandler(t, mux, ipnet.IP6, "sub.test.org", []formattedRecord{{"record1", "::1"}})
@@ -648,7 +617,7 @@ func TestDeleteRecordInvalid(t *testing.T) {
 	mux, h, ok := newGoodHandle(t, mockPP)
 	require.True(t, ok)
 
-	zh := newZonesHandler(t, mux, mockAccountID, map[string][]string{"test.org": {"active"}})
+	zh := newZonesHandler(t, mux, map[string][]string{"test.org": {"active"}})
 	zh.setRequestLimit(2)
 
 	mockPP.EXPECT().Warningf(pp.EmojiError, "Failed to delete a stale %s record of %q (ID: %s): %v",
@@ -722,7 +691,7 @@ func TestUpdateRecordValid(t *testing.T) {
 	mux, h, ok := newGoodHandle(t, mockPP)
 	require.True(t, ok)
 
-	zh := newZonesHandler(t, mux, mockAccountID, map[string][]string{"test.org": {"active"}})
+	zh := newZonesHandler(t, mux, map[string][]string{"test.org": {"active"}})
 	zh.setRequestLimit(2)
 
 	lrh := newListRecordsHandler(t, mux, ipnet.IP6, "sub.test.org", []formattedRecord{{"record1", "::1"}})
@@ -753,7 +722,7 @@ func TestUpdateRecordInvalid(t *testing.T) {
 	mux, h, ok := newGoodHandle(t, mockPP)
 	require.True(t, ok)
 
-	zh := newZonesHandler(t, mux, mockAccountID, map[string][]string{"test.org": {"active"}})
+	zh := newZonesHandler(t, mux, map[string][]string{"test.org": {"active"}})
 	zh.setRequestLimit(2)
 
 	mockPP.EXPECT().Warningf(pp.EmojiError, "Failed to update a stale %s record of %q (ID: %s): %v",
@@ -834,7 +803,7 @@ func TestCreateRecordValid(t *testing.T) {
 	mux, h, ok := newGoodHandle(t, mockPP)
 	require.True(t, ok)
 
-	zh := newZonesHandler(t, mux, mockAccountID, map[string][]string{"test.org": {"active"}})
+	zh := newZonesHandler(t, mux, map[string][]string{"test.org": {"active"}})
 	zh.setRequestLimit(2)
 
 	lrh := newListRecordsHandler(t, mux, ipnet.IP6, "sub.test.org", []formattedRecord{})
@@ -860,7 +829,7 @@ func TestCreateRecordInvalid(t *testing.T) {
 	mockPP := mocks.NewMockPP(mockCtrl)
 	mux, h, ok := newGoodHandle(t, mockPP)
 	require.True(t, ok)
-	zh := newZonesHandler(t, mux, mockAccountID, map[string][]string{"test.org": {"active"}})
+	zh := newZonesHandler(t, mux, map[string][]string{"test.org": {"active"}})
 	zh.setRequestLimit(2)
 	mockPP = mocks.NewMockPP(mockCtrl)
 	mockPP.EXPECT().Warningf(pp.EmojiError, "Failed to add a new %s record of %q: %v",