From dc3a26b18c92990e9fe4d2a4d5e6d47ea6153ffb Mon Sep 17 00:00:00 2001 From: favonia Date: Thu, 12 Sep 2024 05:56:04 -0500 Subject: [PATCH] fix: reduce unnecessary quotations in logging (#925) --- docs/DESIGN.markdown | 7 +++- internal/api/cloudflare_records.go | 29 +++++++------- internal/api/cloudflare_records_test.go | 52 ++++++++++++------------- internal/api/cloudflare_test.go | 2 +- internal/api/cloudflare_waf.go | 4 +- internal/api/cloudflare_waf_test.go | 10 ++--- internal/monitor/uptimekuma.go | 2 +- internal/monitor/uptimekuma_test.go | 2 +- internal/setter/setter.go | 32 +++++++-------- internal/setter/setter_test.go | 48 +++++++++++------------ 10 files changed, 95 insertions(+), 93 deletions(-) diff --git a/docs/DESIGN.markdown b/docs/DESIGN.markdown index c5f1c49b..be69adb3 100644 --- a/docs/DESIGN.markdown +++ b/docs/DESIGN.markdown @@ -29,8 +29,11 @@ The source code follows the [standard Go project layout](https://github.com/gola Here is some arbitrary coding convention that I chose to follow. It may change in the future, but the whole codebase should be consistent with it at any time: -1. Cloudflare IDs (zone IDs, DNS record IDs, etc.) are already designed to use only “very safe” characters and should not be quoted. The formatter `%s` should be used instead of `%q`. -2. A variable name of type `map[..]...` is not in a plural form just because it is of type `map[...]...`. For example, a mapping from IP networks to detected IPs should be named `detectedIP` not `detectedIPs`. +1. These are in general not quoted in the logging because they use only “safe” characters and usually do not cause confusion in a textual context; the formatter `%s` should be used instead of `%q`: + - Cloudflare IDs (DNS zone IDs, DNS record IDs, WAF list IDs, etc.) + - Domain names + - Full list references (`account/name`) +2. A variable of type `map[..]...` should not be named in a plural form just because it is of type `map[...]...`. For example, a mapping from IP networks to detected IPs should be named `detectedIP` not `detectedIPs`. ## Network Security Threat Model diff --git a/internal/api/cloudflare_records.go b/internal/api/cloudflare_records.go index 05f7abd3..0d4a81b0 100644 --- a/internal/api/cloudflare_records.go +++ b/internal/api/cloudflare_records.go @@ -45,7 +45,7 @@ func (h CloudflareHandle) ListZones(ctx context.Context, ppfmt pp.PP, name strin res, err := h.cf.ListZonesContext(ctx, cloudflare.WithZoneFilters(name, "", "")) if err != nil { - ppfmt.Noticef(pp.EmojiError, "Failed to check the existence of a zone named %q: %v", name, err) + ppfmt.Noticef(pp.EmojiError, "Failed to check the existence of a zone named %s: %v", name, err) hintRecordPermission(ppfmt, err) return nil, false } @@ -62,14 +62,13 @@ func (h CloudflareHandle) ListZones(ctx context.Context, ppfmt pp.PP, name strin "initializing", // the setup was just started? "moved", // domain registrar not pointing to Cloudflare "pending": // the setup was not completed - ppfmt.Noticef(pp.EmojiWarning, "Zone %q is %q; your Cloudflare setup is incomplete; some features (e.g., proxying) might not work as expected", name, zone.Status) //nolint:lll + ppfmt.Noticef(pp.EmojiWarning, "DNS zone %s is %q in your Cloudflare account; some features (e.g., proxying) might not work as expected", name, zone.Status) //nolint:lll ids = append(ids, ID(zone.ID)) case "deleted": // archived, pending/moved for too long - ppfmt.Infof(pp.EmojiWarning, "Zone %q is %q and thus skipped", name, zone.Status) - // skip these + ppfmt.Infof(pp.EmojiWarning, "DNS zone %s is %q in your Cloudflare account and thus skipped", name, zone.Status) default: - ppfmt.Noticef(pp.EmojiImpossible, "Zone %q is in an undocumented status %q; please report this at %s", + ppfmt.Noticef(pp.EmojiImpossible, "DNS zone %s is in an undocumented status %q in your Cloudflare account; please report this at %s", //nolint:lll name, zone.Status, pp.IssueReportingURL) ids = append(ids, ID(zone.ID)) } @@ -103,13 +102,13 @@ zoneSearch: return zones[0], true default: // len(zones) > 1 ppfmt.Noticef(pp.EmojiImpossible, - "Found multiple active zones named %q (IDs: %s); please report this at %s", + "Found multiple active zones named %s (IDs: %s); please report this at %s", zoneName, pp.EnglishJoinMap(ID.String, zones), pp.IssueReportingURL) return "", false } } - ppfmt.Noticef(pp.EmojiError, "Failed to find the zone of %q", domain.Describe()) + ppfmt.Noticef(pp.EmojiError, "Failed to find the zone of %s", domain.Describe()) return "", false } @@ -135,7 +134,7 @@ func (h CloudflareHandle) ListRecords(ctx context.Context, ppfmt pp.PP, ipNet ip }) if err != nil { ppfmt.Noticef(pp.EmojiError, - "Failed to retrieve %s records of %q: %v", + "Failed to retrieve %s records of %s: %v", ipNet.RecordType(), domain.Describe(), err) hintRecordPermission(ppfmt, err) return nil, false, false @@ -146,7 +145,7 @@ func (h CloudflareHandle) ListRecords(ctx context.Context, ppfmt pp.PP, ipNet ip ip, err := netip.ParseAddr(r.Content) if err != nil { ppfmt.Noticef(pp.EmojiImpossible, - "Failed to parse the IP address in an %s record of %q (ID: %s): %v", + "Failed to parse the IP address in an %s record of %s (ID: %s): %v", ipNet.RecordType(), domain.Describe(), r.ID, err) return nil, false, false } @@ -171,7 +170,7 @@ func (h CloudflareHandle) DeleteRecord(ctx context.Context, ppfmt pp.PP, } if err := h.cf.DeleteDNSRecord(ctx, cloudflare.ZoneIdentifier(string(zone)), string(id)); err != nil { - ppfmt.Noticef(pp.EmojiError, "Failed to delete a stale %s record of %q (ID: %s): %v", + ppfmt.Noticef(pp.EmojiError, "Failed to delete a stale %s record of %s (ID: %s): %v", ipNet.RecordType(), domain.Describe(), id, err) hintRecordPermission(ppfmt, err) if mode == RegularDelitionMode { @@ -205,7 +204,7 @@ func (h CloudflareHandle) UpdateRecord(ctx context.Context, ppfmt pp.PP, r, err := h.cf.UpdateDNSRecord(ctx, cloudflare.ZoneIdentifier(string(zone)), params) if err != nil { - ppfmt.Noticef(pp.EmojiError, "Failed to update a stale %s record of %q (ID: %s): %v", + ppfmt.Noticef(pp.EmojiError, "Failed to update a stale %s record of %s (ID: %s): %v", ipNet.RecordType(), domain.Describe(), id, err) hintRecordPermission(ppfmt, err) @@ -216,7 +215,7 @@ func (h CloudflareHandle) UpdateRecord(ctx context.Context, ppfmt pp.PP, if TTL(r.TTL) != expectedTTL { ppfmt.Infof(pp.EmojiUserWarning, - "The TTL of the %s record of %q (ID: %s) to be updated differs from the value of TTL (%s) and will be kept", + "The TTL of the %s record of %s (ID: %s) to be updated differs from the value of TTL (%s) and will be kept", ipNet.RecordType(), domain.Describe(), id, expectedTTL.Describe(), ) hintMismatchedRecordAttributes(ppfmt) @@ -225,14 +224,14 @@ func (h CloudflareHandle) UpdateRecord(ctx context.Context, ppfmt pp.PP, if r.Proxied == nil && expectedProxied || r.Proxied != nil && *r.Proxied != expectedProxied { ppfmt.Infof(pp.EmojiUserWarning, - "The proxy status of the %s record of %q (ID: %s) to be updated differs from the value of PROXIED (%v for this domain) and will be kept", //nolint:lll + "The proxy status of the %s record of %s (ID: %s) to be updated differs from the value of PROXIED (%v for this domain) and will be kept", //nolint:lll ipNet.RecordType(), domain.Describe(), id, expectedProxied, ) hintMismatchedRecordAttributes(ppfmt) } if r.Comment != expectedRecordComment { ppfmt.Infof(pp.EmojiUserWarning, - "The comment of the %s record of %q (ID: %s) to be updated differs from the value of RECORD_COMMENT (%q) and will be kept", //nolint:lll + "The comment of the %s record of %s (ID: %s) to be updated differs from the value of RECORD_COMMENT (%q) and will be kept", //nolint:lll ipNet.RecordType(), domain.Describe(), id, expectedRecordComment, ) hintMismatchedRecordAttributes(ppfmt) @@ -270,7 +269,7 @@ func (h CloudflareHandle) CreateRecord(ctx context.Context, ppfmt pp.PP, res, err := h.cf.CreateDNSRecord(ctx, cloudflare.ZoneIdentifier(string(zone)), params) if err != nil { - ppfmt.Noticef(pp.EmojiError, "Failed to add a new %s record of %q: %v", + ppfmt.Noticef(pp.EmojiError, "Failed to add a new %s record of %s: %v", ipNet.RecordType(), domain.Describe(), err) hintRecordPermission(ppfmt, err) diff --git a/internal/api/cloudflare_records_test.go b/internal/api/cloudflare_records_test.go index 78a8494b..9db2c737 100644 --- a/internal/api/cloudflare_records_test.go +++ b/internal/api/cloudflare_records_test.go @@ -136,7 +136,7 @@ func TestListZonesTwo(t *testing.T) { mockPP = mocks.NewMockPP(mockCtrl) mockPP.EXPECT().Noticef( pp.EmojiError, - "Failed to check the existence of a zone named %q: %v", + "Failed to check the existence of a zone named %s: %v", "test.org", gomock.Any(), ) @@ -169,7 +169,7 @@ func TestZoneIDOfDomain(t *testing.T) { map[string][]string{}, 3, "", false, func(m *mocks.MockPP) { - m.EXPECT().Noticef(pp.EmojiError, "Failed to find the zone of %q", "sub.test.org") + m.EXPECT().Noticef(pp.EmojiError, "Failed to find the zone of %s", "sub.test.org") }, }, "none/wildcard": { @@ -177,7 +177,7 @@ func TestZoneIDOfDomain(t *testing.T) { map[string][]string{}, 2, "", false, func(m *mocks.MockPP) { - m.EXPECT().Noticef(pp.EmojiError, "Failed to find the zone of %q", "*.test.org") + m.EXPECT().Noticef(pp.EmojiError, "Failed to find the zone of %s", "*.test.org") }, }, "multiple": { @@ -188,7 +188,7 @@ func TestZoneIDOfDomain(t *testing.T) { gomock.InOrder( m.EXPECT().Noticef( pp.EmojiImpossible, - "Found multiple active zones named %q (IDs: %s); please report this at %s", + "Found multiple active zones named %s (IDs: %s); please report this at %s", "test.org", pp.EnglishJoin(mockIDsAsStrings("test.org", 0, 1)), pp.IssueReportingURL, ), ) @@ -202,7 +202,7 @@ func TestZoneIDOfDomain(t *testing.T) { gomock.InOrder( m.EXPECT().Noticef( pp.EmojiImpossible, - "Found multiple active zones named %q (IDs: %s); please report this at %s", + "Found multiple active zones named %s (IDs: %s); please report this at %s", "test.org", pp.EnglishJoin(mockIDsAsStrings("test.org", 0, 1)), pp.IssueReportingURL, ), ) @@ -214,8 +214,8 @@ func TestZoneIDOfDomain(t *testing.T) { 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().Noticef(pp.EmojiError, "Failed to find the zone of %q", "test.org"), + m.EXPECT().Infof(pp.EmojiWarning, "DNS zone %s is %q in your Cloudflare account and thus skipped", "test.org", "deleted"), //nolint:lll + m.EXPECT().Noticef(pp.EmojiError, "Failed to find the zone of %s", "test.org"), ) }, }, @@ -225,7 +225,7 @@ func TestZoneIDOfDomain(t *testing.T) { 1, mockID("test.org", 0), true, func(m *mocks.MockPP) { gomock.InOrder( - m.EXPECT().Noticef(pp.EmojiWarning, "Zone %q is %q; your Cloudflare setup is incomplete; some features (e.g., proxying) might not work as expected", "test.org", "pending"), //nolint:lll + m.EXPECT().Noticef(pp.EmojiWarning, "DNS zone %s is %q in your Cloudflare account; some features (e.g., proxying) might not work as expected", "test.org", "pending"), //nolint:lll ) }, }, @@ -236,7 +236,7 @@ func TestZoneIDOfDomain(t *testing.T) { func(m *mocks.MockPP) { gomock.InOrder( m.EXPECT().Noticef(pp.EmojiWarning, - "Zone %q is %q; your Cloudflare setup is incomplete; some features (e.g., proxying) might not work as expected", + "DNS zone %s is %q in your Cloudflare account; some features (e.g., proxying) might not work as expected", "test.org", "initializing"), ) }, @@ -247,7 +247,7 @@ func TestZoneIDOfDomain(t *testing.T) { 1, mockID("test.org", 0), true, func(m *mocks.MockPP) { m.EXPECT().Noticef(pp.EmojiImpossible, - "Zone %q is in an undocumented status %q; please report this at %s", + "DNS zone %s is in an undocumented status %q in your Cloudflare account; please report this at %s", "test.org", "some-undocumented-status", pp.IssueReportingURL) }, }, @@ -292,7 +292,7 @@ func TestZoneIDOfDomainInvalid(t *testing.T) { mockPP.EXPECT().Noticef( pp.EmojiError, - "Failed to check the existence of a zone named %q: %v", + "Failed to check the existence of a zone named %s: %v", "sub.test.org", gomock.Any(), ) @@ -415,7 +415,7 @@ func TestListRecords(t *testing.T) { func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef( pp.EmojiError, - "Failed to retrieve %s records of %q: %v", "AAAA", "sub.test.org", gomock.Any(), + "Failed to retrieve %s records of %s: %v", "AAAA", "sub.test.org", gomock.Any(), ) ppfmt.EXPECT().Hintf(pp.HintRecordPermission, "Double check your API token. "+ @@ -425,7 +425,7 @@ func TestListRecords(t *testing.T) { func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef( pp.EmojiError, - "Failed to retrieve %s records of %q: %v", "AAAA", "sub.test.org", gomock.Any(), + "Failed to retrieve %s records of %s: %v", "AAAA", "sub.test.org", gomock.Any(), ) ppfmt.EXPECT().Hintf(pp.HintRecordPermission, "Double check your API token. "+ @@ -441,7 +441,7 @@ func TestListRecords(t *testing.T) { func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef( pp.EmojiError, - "Failed to check the existence of a zone named %q: %v", + "Failed to check the existence of a zone named %s: %v", "sub.test.org", gomock.Any(), ) }, @@ -449,7 +449,7 @@ func TestListRecords(t *testing.T) { func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef( pp.EmojiError, - "Failed to check the existence of a zone named %q: %v", + "Failed to check the existence of a zone named %s: %v", "sub.test.org", gomock.Any(), ) }, @@ -466,7 +466,7 @@ func TestListRecords(t *testing.T) { func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef( pp.EmojiImpossible, - "Failed to parse the IP address in an %s record of %q (ID: %s): %v", + "Failed to parse the IP address in an %s record of %s (ID: %s): %v", "AAAA", "sub.test.org", "record2", gomock.Any(), ) }, @@ -474,7 +474,7 @@ func TestListRecords(t *testing.T) { func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef( pp.EmojiError, - "Failed to retrieve %s records of %q: %v", + "Failed to retrieve %s records of %s: %v", "AAAA", "sub.test.org", gomock.Any(), ) ppfmt.EXPECT().Hintf(pp.HintRecordPermission, @@ -579,7 +579,7 @@ func TestDeleteRecord(t *testing.T) { false, func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef(pp.EmojiError, - "Failed to check the existence of a zone named %q: %v", + "Failed to check the existence of a zone named %s: %v", "sub.test.org", gomock.Any(), ) }, @@ -589,7 +589,7 @@ func TestDeleteRecord(t *testing.T) { false, func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef(pp.EmojiError, - "Failed to delete a stale %s record of %q (ID: %s): %v", + "Failed to delete a stale %s record of %s (ID: %s): %v", "AAAA", "sub.test.org", api.ID("record1"), gomock.Any(), ) }, @@ -703,7 +703,7 @@ func TestUpdateRecord(t *testing.T) { false, func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef(pp.EmojiError, - "Failed to check the existence of a zone named %q: %v", + "Failed to check the existence of a zone named %s: %v", "sub.test.org", gomock.Any(), ) }, @@ -714,7 +714,7 @@ func TestUpdateRecord(t *testing.T) { false, func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef(pp.EmojiError, - "Failed to update a stale %s record of %q (ID: %s): %v", + "Failed to update a stale %s record of %s (ID: %s): %v", "AAAA", "sub.test.org", api.ID("record1"), gomock.Any(), ) }, @@ -729,17 +729,17 @@ func TestUpdateRecord(t *testing.T) { gomock.InOrder( ppfmt.EXPECT().Infof(pp.EmojiUserWarning, - "The TTL of the %s record of %q (ID: %s) to be updated differs from the value of TTL (%s) and will be kept", //nolint:lll + "The TTL of the %s record of %s (ID: %s) to be updated differs from the value of TTL (%s) and will be kept", //nolint:lll "AAAA", "sub.test.org", api.ID("record1"), "1 (auto)", ), ppfmt.EXPECT().Hintf(pp.HintMismatchedRecordAttributes, hintText), ppfmt.EXPECT().Infof(pp.EmojiUserWarning, - "The proxy status of the %s record of %q (ID: %s) to be updated differs from the value of PROXIED (%v for this domain) and will be kept", //nolint:lll + "The proxy status of the %s record of %s (ID: %s) to be updated differs from the value of PROXIED (%v for this domain) and will be kept", //nolint:lll "AAAA", "sub.test.org", api.ID("record1"), true, ), ppfmt.EXPECT().Hintf(pp.HintMismatchedRecordAttributes, hintText), ppfmt.EXPECT().Infof(pp.EmojiUserWarning, - "The comment of the %s record of %q (ID: %s) to be updated differs from the value of RECORD_COMMENT (%q) and will be kept", //nolint:lll + "The comment of the %s record of %s (ID: %s) to be updated differs from the value of RECORD_COMMENT (%q) and will be kept", //nolint:lll "AAAA", "sub.test.org", api.ID("record1"), "hello", ), ppfmt.EXPECT().Hintf(pp.HintMismatchedRecordAttributes, hintText), @@ -859,7 +859,7 @@ func TestCreateRecord(t *testing.T) { false, func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef(pp.EmojiError, - "Failed to check the existence of a zone named %q: %v", + "Failed to check the existence of a zone named %s: %v", "sub.test.org", gomock.Any(), ).Times(2) }, @@ -869,7 +869,7 @@ func TestCreateRecord(t *testing.T) { false, func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef(pp.EmojiError, - "Failed to add a new %s record of %q: %v", + "Failed to add a new %s record of %s: %v", "AAAA", "sub.test.org", gomock.Any(), ) }, diff --git a/internal/api/cloudflare_test.go b/internal/api/cloudflare_test.go index a440e4d6..4f546a77 100644 --- a/internal/api/cloudflare_test.go +++ b/internal/api/cloudflare_test.go @@ -71,7 +71,7 @@ func mockResponse() cloudflare.Response { const ( mockToken = "token123" mockAuthString = "Bearer " + mockToken - mockAccountID = "account456" + mockAccountID = api.ID("account456") ) func newServerAuth(t *testing.T) (*http.ServeMux, api.CloudflareAuth) { diff --git a/internal/api/cloudflare_waf.go b/internal/api/cloudflare_waf.go index 73bf0a04..8523b9a1 100644 --- a/internal/api/cloudflare_waf.go +++ b/internal/api/cloudflare_waf.go @@ -94,8 +94,8 @@ func (h CloudflareHandle) WAFListID(ctx context.Context, ppfmt pp.PP, list WAFLi count++ if count > 1 { ppfmt.Noticef(pp.EmojiImpossible, - "Found multiple lists named %q (IDs: %s and %s); please report this at %s", - l.Name, listID, l.ID, pp.IssueReportingURL, + "Found multiple lists named %q within the account %s (IDs: %s and %s); please report this at %s", + list.Name, list.AccountID, listID, l.ID, pp.IssueReportingURL, ) return "", false, false } diff --git a/internal/api/cloudflare_waf_test.go b/internal/api/cloudflare_waf_test.go index 02ca9d5a..4cb36d5b 100644 --- a/internal/api/cloudflare_waf_test.go +++ b/internal/api/cloudflare_waf_test.go @@ -196,8 +196,8 @@ func TestWAFListID(t *testing.T) { false, false, "", func(ppfmt *mocks.MockPP) { ppfmt.EXPECT().Noticef(pp.EmojiImpossible, - "Found multiple lists named %q (IDs: %s and %s); please report this at %s", - "list", mockID("list", 0), mockID("list", 2), pp.IssueReportingURL, + "Found multiple lists named %q within the account %s (IDs: %s and %s); please report this at %s", + "list", mockAccountID, mockID("list", 0), mockID("list", 2), pp.IssueReportingURL, ) }, }, @@ -226,7 +226,7 @@ func TestWAFListID(t *testing.T) { ppfmt.EXPECT().Hintf(pp.HintMismatchedWAFListAttributes, "The updater will not overwrite WAF list descriptions; "+ "you can change them at https://dash.cloudflare.com/%s/configurations/lists", - api.ID("account456"), + mockAccountID, ), ) }, @@ -356,8 +356,8 @@ func TestFindWAFList(t *testing.T) { func(ppfmt *mocks.MockPP) { gomock.InOrder( ppfmt.EXPECT().Noticef(pp.EmojiImpossible, - "Found multiple lists named %q (IDs: %s and %s); please report this at %s", - "list", mockID("list", 0), mockID("list", 2), pp.IssueReportingURL, + "Found multiple lists named %q within the account %s (IDs: %s and %s); please report this at %s", + "list", mockAccountID, mockID("list", 0), mockID("list", 2), pp.IssueReportingURL, ), ppfmt.EXPECT().Noticef(pp.EmojiError, "Failed to find the list %s", "account456/list"), ) diff --git a/internal/monitor/uptimekuma.go b/internal/monitor/uptimekuma.go index 511ac619..47b366db 100644 --- a/internal/monitor/uptimekuma.go +++ b/internal/monitor/uptimekuma.go @@ -147,7 +147,7 @@ func (h UptimeKuma) ping(ctx context.Context, ppfmt pp.PP, param UptimeKumaReque return false } if !parsedResp.OK { - ppfmt.Noticef(pp.EmojiError, "Failed to ping Uptime Kuma: %q", parsedResp.Msg) + ppfmt.Noticef(pp.EmojiError, "Failed to ping Uptime Kuma: %s", parsedResp.Msg) return false } diff --git a/internal/monitor/uptimekuma_test.go b/internal/monitor/uptimekuma_test.go index 7185981b..3a9cdf29 100644 --- a/internal/monitor/uptimekuma_test.go +++ b/internal/monitor/uptimekuma_test.go @@ -143,7 +143,7 @@ func TestUptimeKumaEndPoints(t *testing.T) { func(m *mocks.MockPP) { gomock.InOrder( m.EXPECT().Noticef(pp.EmojiUserWarning, httpUnsafeMsg), - m.EXPECT().Noticef(pp.EmojiError, "Failed to ping Uptime Kuma: %q", "bad"), + m.EXPECT().Noticef(pp.EmojiError, "Failed to ping Uptime Kuma: %s", "bad"), ) }, }, diff --git a/internal/setter/setter.go b/internal/setter/setter.go index 8dbc2f90..c35d07fc 100644 --- a/internal/setter/setter.go +++ b/internal/setter/setter.go @@ -66,11 +66,11 @@ func (s setter) Set(ctx context.Context, ppfmt pp.PP, if foundMatched && len(unprocessedMatched) == 0 && len(unprocessedUnmatched) == 0 { if cached { ppfmt.Infof(pp.EmojiAlreadyDone, - "The %s records of %q are already up to date (cached)", + "The %s records of %s are already up to date (cached)", recordType, domainDescription) } else { ppfmt.Infof(pp.EmojiAlreadyDone, - "The %s records of %q are already up to date", + "The %s records of %s are already up to date", recordType, domainDescription) } return ResponseNoop @@ -86,7 +86,7 @@ func (s setter) Set(ctx context.Context, ppfmt pp.PP, ttl, proxied, recordComment, ); !ok { ppfmt.Noticef(pp.EmojiError, - "Failed to properly update %s records of %q; records might be inconsistent", + "Failed to properly update %s records of %s; records might be inconsistent", recordType, domainDescription) return ResponseFailed } @@ -95,7 +95,7 @@ func (s setter) Set(ctx context.Context, ppfmt pp.PP, // // Note that there can still be stale records at this point. ppfmt.Noticef(pp.EmojiUpdate, - "Updated a stale %s record of %q (ID: %s)", + "Updated a stale %s record of %s (ID: %s)", recordType, domainDescription, unprocessedUnmatched[0]) // Now it's up to date! Note that unprocessedMatched must be empty @@ -110,7 +110,7 @@ func (s setter) Set(ctx context.Context, ppfmt pp.PP, id, ok := s.Handle.CreateRecord(ctx, ppfmt, ipnet, domain, ip, ttl, proxied, recordComment) if !ok { ppfmt.Noticef(pp.EmojiError, - "Failed to properly update %s records of %q; records might be inconsistent", + "Failed to properly update %s records of %s; records might be inconsistent", recordType, domainDescription) return ResponseFailed } @@ -118,20 +118,20 @@ func (s setter) Set(ctx context.Context, ppfmt pp.PP, // Note that both unprocessedMatched and unprocessedUnmatched must be empty at this point // and the records are updated. ppfmt.Noticef(pp.EmojiCreation, - "Added a new %s record of %q (ID: %s)", recordType, domainDescription, id) + "Added a new %s record of %s (ID: %s)", recordType, domainDescription, id) } // Now, we should try to delete all remaining stale records. for _, id := range unprocessedUnmatched { if ok := s.Handle.DeleteRecord(ctx, ppfmt, ipnet, domain, id, api.RegularDelitionMode); !ok { ppfmt.Noticef(pp.EmojiError, - "Failed to properly update %s records of %q; records might be inconsistent", + "Failed to properly update %s records of %s; records might be inconsistent", recordType, domainDescription) return ResponseFailed } ppfmt.Noticef(pp.EmojiDeletion, - "Deleted a stale %s record of %q (ID: %s)", recordType, domainDescription, id) + "Deleted a stale %s record of %s (ID: %s)", recordType, domainDescription, id) } // We should also delete all duplicate records even if they are up to date. @@ -139,7 +139,7 @@ func (s setter) Set(ctx context.Context, ppfmt pp.PP, for _, id := range unprocessedMatched { if ok := s.Handle.DeleteRecord(ctx, ppfmt, ipnet, domain, id, api.RegularDelitionMode); ok { ppfmt.Noticef(pp.EmojiDeletion, - "Deleted a duplicate %s record of %q (ID: %s)", recordType, domainDescription, id) + "Deleted a duplicate %s record of %s (ID: %s)", recordType, domainDescription, id) } if ctx.Err() != nil { return ResponseUpdated @@ -167,9 +167,9 @@ func (s setter) FinalDelete(ctx context.Context, ppfmt pp.PP, ipnet ipnet.Type, if len(unmatchedIDs) == 0 { if cached { - ppfmt.Infof(pp.EmojiAlreadyDone, "The %s records of %q were already deleted (cached)", recordType, domainDescription) + ppfmt.Infof(pp.EmojiAlreadyDone, "The %s records of %s were already deleted (cached)", recordType, domainDescription) } else { - ppfmt.Infof(pp.EmojiAlreadyDone, "The %s records of %q were already deleted", recordType, domainDescription) + ppfmt.Infof(pp.EmojiAlreadyDone, "The %s records of %s were already deleted", recordType, domainDescription) } return ResponseNoop } @@ -181,18 +181,18 @@ func (s setter) FinalDelete(ctx context.Context, ppfmt pp.PP, ipnet ipnet.Type, if ctx.Err() != nil { ppfmt.Infof(pp.EmojiTimeout, - "Deletion of %s records of %q aborted by timeout or signals; records might be inconsistent", + "Deletion of %s records of %s aborted by timeout or signals; records might be inconsistent", recordType, domainDescription) return ResponseFailed } continue } - ppfmt.Noticef(pp.EmojiDeletion, "Deleted a stale %s record of %q (ID: %s)", recordType, domainDescription, id) + ppfmt.Noticef(pp.EmojiDeletion, "Deleted a stale %s record of %s (ID: %s)", recordType, domainDescription, id) } if !allOK { ppfmt.Noticef(pp.EmojiError, - "Failed to properly delete %s records of %q; records might be inconsistent", + "Failed to properly delete %s records of %s; records might be inconsistent", recordType, domainDescription) return ResponseFailed } @@ -280,10 +280,10 @@ func (s setter) FinalClearWAFList(ctx context.Context, ppfmt pp.PP, list api.WAF deleted, ok := s.Handle.FinalClearWAFListAsync(ctx, ppfmt, list, listDescription) switch { case ok && deleted: - ppfmt.Noticef(pp.EmojiDeletion, "The list %q was deleted", list.Name) + ppfmt.Noticef(pp.EmojiDeletion, "The list %s was deleted", list.Describe()) return ResponseUpdated case ok && !deleted: - ppfmt.Noticef(pp.EmojiClear, "The list %q is being cleared (asynchronously)", list.Name) + ppfmt.Noticef(pp.EmojiClear, "The list %s is being cleared (asynchronously)", list.Describe()) return ResponseUpdating default: return ResponseFailed diff --git a/internal/setter/setter_test.go b/internal/setter/setter_test.go index 81feb077..52562f4d 100644 --- a/internal/setter/setter_test.go +++ b/internal/setter/setter_test.go @@ -51,7 +51,7 @@ func TestSet(t *testing.T) { gomock.InOrder( h.EXPECT().ListRecords(ctx, p, ipNetwork, domain).Return([]api.Record{}, true, true), h.EXPECT().CreateRecord(ctx, p, ipNetwork, domain, ip1, api.TTLAuto, false, "hello").Return(record1, true), - p.EXPECT().Noticef(pp.EmojiCreation, "Added a new %s record of %q (ID: %s)", "AAAA", "sub.test.org", record1), + p.EXPECT().Noticef(pp.EmojiCreation, "Added a new %s record of %s (ID: %s)", "AAAA", "sub.test.org", record1), ) }, }, @@ -62,7 +62,7 @@ func TestSet(t *testing.T) { gomock.InOrder( h.EXPECT().ListRecords(ctx, p, ipNetwork, domain).Return([]api.Record{}, true, true), h.EXPECT().CreateRecord(ctx, p, ipNetwork, domain, ip1, api.TTLAuto, false, "hello").Return(record1, false), - p.EXPECT().Noticef(pp.EmojiError, "Failed to properly update %s records of %q; records might be inconsistent", "AAAA", "sub.test.org"), //nolint:lll + p.EXPECT().Noticef(pp.EmojiError, "Failed to properly update %s records of %s; records might be inconsistent", "AAAA", "sub.test.org"), //nolint:lll ) }, }, @@ -74,7 +74,7 @@ func TestSet(t *testing.T) { h.EXPECT().ListRecords(ctx, p, ipNetwork, domain).Return([]api.Record{{ID: record1, IP: ip2}}, true, true), h.EXPECT().UpdateRecord(ctx, p, ipNetwork, domain, record1, ip1, api.TTLAuto, false, "hello").Return(true), p.EXPECT().Noticef(pp.EmojiUpdate, - "Updated a stale %s record of %q (ID: %s)", + "Updated a stale %s record of %s (ID: %s)", "AAAA", "sub.test.org", record1, @@ -89,7 +89,7 @@ func TestSet(t *testing.T) { gomock.InOrder( h.EXPECT().ListRecords(ctx, p, ipNetwork, domain).Return([]api.Record{{ID: record1, IP: ip2}}, true, true), h.EXPECT().UpdateRecord(ctx, p, ipNetwork, domain, record1, ip1, api.TTLAuto, false, "hello").Return(false), - p.EXPECT().Noticef(pp.EmojiError, "Failed to properly update %s records of %q; records might be inconsistent", "AAAA", "sub.test.org"), //nolint:lll + p.EXPECT().Noticef(pp.EmojiError, "Failed to properly update %s records of %s; records might be inconsistent", "AAAA", "sub.test.org"), //nolint:lll ) }, }, @@ -100,7 +100,7 @@ func TestSet(t *testing.T) { gomock.InOrder( h.EXPECT().ListRecords(ctx, p, ipNetwork, domain).Return([]api.Record{{ID: record1, IP: ip1}}, true, true), p.EXPECT().Infof(pp.EmojiAlreadyDone, - "The %s records of %q are already up to date (cached)", "AAAA", "sub.test.org"), + "The %s records of %s are already up to date (cached)", "AAAA", "sub.test.org"), ) }, }, @@ -110,7 +110,7 @@ func TestSet(t *testing.T) { func(ctx context.Context, _ func(), p *mocks.MockPP, h *mocks.MockHandle) { gomock.InOrder( h.EXPECT().ListRecords(ctx, p, ipNetwork, domain).Return([]api.Record{{ID: record1, IP: ip1}}, false, true), - p.EXPECT().Infof(pp.EmojiAlreadyDone, "The %s records of %q are already up to date", "AAAA", "sub.test.org"), + p.EXPECT().Infof(pp.EmojiAlreadyDone, "The %s records of %s are already up to date", "AAAA", "sub.test.org"), ) }, }, @@ -124,7 +124,7 @@ func TestSet(t *testing.T) { h.EXPECT().DeleteRecord(ctx, p, ipNetwork, domain, record2, api.RegularDelitionMode).Return(true), p.EXPECT().Noticef( pp.EmojiDeletion, - "Deleted a duplicate %s record of %q (ID: %s)", + "Deleted a duplicate %s record of %s (ID: %s)", "AAAA", "sub.test.org", record2, @@ -132,7 +132,7 @@ func TestSet(t *testing.T) { h.EXPECT().DeleteRecord(ctx, p, ipNetwork, domain, record3, api.RegularDelitionMode).Return(true), p.EXPECT().Noticef( pp.EmojiDeletion, - "Deleted a duplicate %s record of %q (ID: %s)", + "Deleted a duplicate %s record of %s (ID: %s)", "AAAA", "sub.test.org", record3, @@ -151,7 +151,7 @@ func TestSet(t *testing.T) { h.EXPECT().DeleteRecord(ctx, p, ipNetwork, domain, record3, api.RegularDelitionMode).Return(true), p.EXPECT().Noticef( pp.EmojiDeletion, - "Deleted a duplicate %s record of %q (ID: %s)", + "Deleted a duplicate %s record of %s (ID: %s)", "AAAA", "sub.test.org", record3, @@ -169,7 +169,7 @@ func TestSet(t *testing.T) { h.EXPECT().DeleteRecord(ctx, p, ipNetwork, domain, record2, api.RegularDelitionMode).Return(true), p.EXPECT().Noticef( pp.EmojiDeletion, - "Deleted a duplicate %s record of %q (ID: %s)", + "Deleted a duplicate %s record of %s (ID: %s)", "AAAA", "sub.test.org", record2, @@ -200,14 +200,14 @@ func TestSet(t *testing.T) { h.EXPECT().UpdateRecord(ctx, p, ipNetwork, domain, record1, ip1, api.TTLAuto, false, "hello").Return(true), p.EXPECT().Noticef( pp.EmojiUpdate, - "Updated a stale %s record of %q (ID: %s)", + "Updated a stale %s record of %s (ID: %s)", "AAAA", "sub.test.org", record1, ), h.EXPECT().DeleteRecord(ctx, p, ipNetwork, domain, record2, api.RegularDelitionMode).Return(true), p.EXPECT().Noticef(pp.EmojiDeletion, - "Deleted a stale %s record of %q (ID: %s)", + "Deleted a stale %s record of %s (ID: %s)", "AAAA", "sub.test.org", record2), @@ -224,14 +224,14 @@ func TestSet(t *testing.T) { h.EXPECT().UpdateRecord(ctx, p, ipNetwork, domain, record1, ip1, api.TTLAuto, false, "hello").Return(true), p.EXPECT().Noticef( pp.EmojiUpdate, - "Updated a stale %s record of %q (ID: %s)", + "Updated a stale %s record of %s (ID: %s)", "AAAA", "sub.test.org", record1, ), h.EXPECT().DeleteRecord(ctx, p, ipNetwork, domain, record2, api.RegularDelitionMode). Do(wrapCancelAsDelete(cancel)).Return(false), - p.EXPECT().Noticef(pp.EmojiError, "Failed to properly update %s records of %q; records might be inconsistent", "AAAA", "sub.test.org"), //nolint:lll + p.EXPECT().Noticef(pp.EmojiError, "Failed to properly update %s records of %s; records might be inconsistent", "AAAA", "sub.test.org"), //nolint:lll ) }, }, @@ -243,7 +243,7 @@ func TestSet(t *testing.T) { h.EXPECT().ListRecords(ctx, p, ipNetwork, domain). Return([]api.Record{{ID: record1, IP: ip2}, {ID: record2, IP: ip2}}, true, true), h.EXPECT().UpdateRecord(ctx, p, ipNetwork, domain, record1, ip1, api.TTLAuto, false, "hello").Return(false), - p.EXPECT().Noticef(pp.EmojiError, "Failed to properly update %s records of %q; records might be inconsistent", "AAAA", "sub.test.org"), //nolint:lll + p.EXPECT().Noticef(pp.EmojiError, "Failed to properly update %s records of %s; records might be inconsistent", "AAAA", "sub.test.org"), //nolint:lll ) }, }, @@ -301,7 +301,7 @@ func TestFinalDelete(t *testing.T) { func(ctx context.Context, _ func(), p *mocks.MockPP, h *mocks.MockHandle) { gomock.InOrder( h.EXPECT().ListRecords(ctx, p, ipNetwork, domain).Return([]api.Record{}, true, true), - p.EXPECT().Infof(pp.EmojiAlreadyDone, "The %s records of %q were already deleted (cached)", "AAAA", "sub.test.org"), //nolint:lll + p.EXPECT().Infof(pp.EmojiAlreadyDone, "The %s records of %s were already deleted (cached)", "AAAA", "sub.test.org"), //nolint:lll ) }, }, @@ -310,7 +310,7 @@ func TestFinalDelete(t *testing.T) { func(ctx context.Context, _ func(), p *mocks.MockPP, h *mocks.MockHandle) { gomock.InOrder( h.EXPECT().ListRecords(ctx, p, ipNetwork, domain).Return([]api.Record{}, false, true), - p.EXPECT().Infof(pp.EmojiAlreadyDone, "The %s records of %q were already deleted", "AAAA", "sub.test.org"), + p.EXPECT().Infof(pp.EmojiAlreadyDone, "The %s records of %s were already deleted", "AAAA", "sub.test.org"), ) }, }, @@ -320,7 +320,7 @@ func TestFinalDelete(t *testing.T) { gomock.InOrder( h.EXPECT().ListRecords(ctx, p, ipNetwork, domain).Return([]api.Record{{ID: record1, IP: ip1}}, true, true), h.EXPECT().DeleteRecord(ctx, p, ipNetwork, domain, record1, api.FinalDeletionMode).Return(true), - p.EXPECT().Noticef(pp.EmojiDeletion, "Deleted a stale %s record of %q (ID: %s)", "AAAA", "sub.test.org", record1), //nolint:lll + p.EXPECT().Noticef(pp.EmojiDeletion, "Deleted a stale %s record of %s (ID: %s)", "AAAA", "sub.test.org", record1), //nolint:lll ) }, }, @@ -330,7 +330,7 @@ func TestFinalDelete(t *testing.T) { gomock.InOrder( h.EXPECT().ListRecords(ctx, p, ipNetwork, domain).Return([]api.Record{{ID: record1, IP: ip1}}, true, true), h.EXPECT().DeleteRecord(ctx, p, ipNetwork, domain, record1, api.FinalDeletionMode).Return(false), - p.EXPECT().Noticef(pp.EmojiError, "Failed to properly delete %s records of %q; records might be inconsistent", "AAAA", "sub.test.org"), //nolint:lll + p.EXPECT().Noticef(pp.EmojiError, "Failed to properly delete %s records of %s; records might be inconsistent", "AAAA", "sub.test.org"), //nolint:lll ) }, }, @@ -343,7 +343,7 @@ func TestFinalDelete(t *testing.T) { h.EXPECT().DeleteRecord(ctx, p, ipNetwork, domain, record1, api.FinalDeletionMode). Do(wrapCancelAsDelete(cancel)).Return(false), p.EXPECT().Infof(pp.EmojiTimeout, - "Deletion of %s records of %q aborted by timeout or signals; records might be inconsistent", + "Deletion of %s records of %s aborted by timeout or signals; records might be inconsistent", "AAAA", "sub.test.org"), ) }, @@ -354,9 +354,9 @@ func TestFinalDelete(t *testing.T) { gomock.InOrder( h.EXPECT().ListRecords(ctx, p, ipNetwork, domain).Return([]api.Record{{ID: record1, IP: ip1}, {ID: record2, IP: invalidIP}}, true, true), //nolint:lll h.EXPECT().DeleteRecord(ctx, p, ipNetwork, domain, record1, api.FinalDeletionMode).Return(true), - p.EXPECT().Noticef(pp.EmojiDeletion, "Deleted a stale %s record of %q (ID: %s)", "AAAA", "sub.test.org", record1), //nolint:lll + p.EXPECT().Noticef(pp.EmojiDeletion, "Deleted a stale %s record of %s (ID: %s)", "AAAA", "sub.test.org", record1), //nolint:lll h.EXPECT().DeleteRecord(ctx, p, ipNetwork, domain, record2, api.FinalDeletionMode).Return(true), - p.EXPECT().Noticef(pp.EmojiDeletion, "Deleted a stale %s record of %q (ID: %s)", "AAAA", "sub.test.org", record2), //nolint:lll + p.EXPECT().Noticef(pp.EmojiDeletion, "Deleted a stale %s record of %s (ID: %s)", "AAAA", "sub.test.org", record2), //nolint:lll ) }, }, @@ -666,7 +666,7 @@ func TestFinalClearWAFListAsync(t *testing.T) { func(ctx context.Context, _ func(), p *mocks.MockPP, m *mocks.MockHandle) { gomock.InOrder( m.EXPECT().FinalClearWAFListAsync(ctx, p, wafList, listDescription).Return(true, true), - p.EXPECT().Noticef(pp.EmojiDeletion, "The list %q was deleted", listName), + p.EXPECT().Noticef(pp.EmojiDeletion, "The list %s was deleted", wafList.Describe()), ) }, }, @@ -675,7 +675,7 @@ func TestFinalClearWAFListAsync(t *testing.T) { func(ctx context.Context, _ func(), p *mocks.MockPP, m *mocks.MockHandle) { gomock.InOrder( m.EXPECT().FinalClearWAFListAsync(ctx, p, wafList, listDescription).Return(false, true), - p.EXPECT().Noticef(pp.EmojiClear, "The list %q is being cleared (asynchronously)", listName), + p.EXPECT().Noticef(pp.EmojiClear, "The list %s is being cleared (asynchronously)", wafList.Describe()), ) }, },