From 817125ef46511d24372f54afb67a90b7547bb532 Mon Sep 17 00:00:00 2001 From: favonia Date: Thu, 9 Nov 2023 07:18:08 -0600 Subject: [PATCH] refactor: move message generation to updater (part 2 of shoutrrr support) (#640) refactor: move message gen to updater (part 2 of supporting shoutrrr) --- cmd/ddns/ddns.go | 2 +- internal/mocks/mock_setter.go | 47 +++++++-------- internal/setter/base.go | 18 +++++- internal/setter/setter.go | 93 ++++++++++++++-------------- internal/setter/setter_test.go | 77 ++++++++++-------------- internal/updater/updater.go | 45 ++++++++------ internal/updater/updater_test.go | 100 ++++++++++++++++++------------- 7 files changed, 198 insertions(+), 184 deletions(-) diff --git a/cmd/ddns/ddns.go b/cmd/ddns/ddns.go index 7ea684e9..4ce73c20 100644 --- a/cmd/ddns/ddns.go +++ b/cmd/ddns/ddns.go @@ -56,7 +56,7 @@ func initConfig(ctx context.Context, ppfmt pp.PP) (*config.Config, setter.Setter func stopUpdating(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Setter) { if c.DeleteOnStop { - if ok, msg := updater.ClearIPs(ctx, ppfmt, c, s); ok { + if ok, msg := updater.DeleteIPs(ctx, ppfmt, c, s); ok { monitor.LogAll(ctx, ppfmt, msg, c.Monitors) } else { monitor.FailureAll(ctx, ppfmt, msg, c.Monitors) diff --git a/internal/mocks/mock_setter.go b/internal/mocks/mock_setter.go index f56690fd..2b21ad6a 100644 --- a/internal/mocks/mock_setter.go +++ b/internal/mocks/mock_setter.go @@ -17,6 +17,7 @@ import ( domain "github.com/favonia/cloudflare-ddns/internal/domain" ipnet "github.com/favonia/cloudflare-ddns/internal/ipnet" pp "github.com/favonia/cloudflare-ddns/internal/pp" + setter "github.com/favonia/cloudflare-ddns/internal/setter" gomock "go.uber.org/mock/gomock" ) @@ -43,52 +44,50 @@ func (m *MockSetter) EXPECT() *MockSetterMockRecorder { return m.recorder } -// Clear mocks base method. -func (m *MockSetter) Clear(arg0 context.Context, arg1 pp.PP, arg2 domain.Domain, arg3 ipnet.Type) (bool, string) { +// Delete mocks base method. +func (m *MockSetter) Delete(arg0 context.Context, arg1 pp.PP, arg2 domain.Domain, arg3 ipnet.Type) setter.ResponseCode { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Clear", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(string) - return ret0, ret1 + ret := m.ctrl.Call(m, "Delete", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(setter.ResponseCode) + return ret0 } -// Clear indicates an expected call of Clear. -func (mr *MockSetterMockRecorder) Clear(arg0, arg1, arg2, arg3 any) *SetterClearCall { +// Delete indicates an expected call of Delete. +func (mr *MockSetterMockRecorder) Delete(arg0, arg1, arg2, arg3 any) *SetterDeleteCall { mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Clear", reflect.TypeOf((*MockSetter)(nil).Clear), arg0, arg1, arg2, arg3) - return &SetterClearCall{Call: call} + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockSetter)(nil).Delete), arg0, arg1, arg2, arg3) + return &SetterDeleteCall{Call: call} } -// SetterClearCall wrap *gomock.Call -type SetterClearCall struct { +// SetterDeleteCall wrap *gomock.Call +type SetterDeleteCall struct { *gomock.Call } // Return rewrite *gomock.Call.Return -func (c *SetterClearCall) Return(arg0 bool, arg1 string) *SetterClearCall { - c.Call = c.Call.Return(arg0, arg1) +func (c *SetterDeleteCall) Return(arg0 setter.ResponseCode) *SetterDeleteCall { + c.Call = c.Call.Return(arg0) return c } // Do rewrite *gomock.Call.Do -func (c *SetterClearCall) Do(f func(context.Context, pp.PP, domain.Domain, ipnet.Type) (bool, string)) *SetterClearCall { +func (c *SetterDeleteCall) Do(f func(context.Context, pp.PP, domain.Domain, ipnet.Type) setter.ResponseCode) *SetterDeleteCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *SetterClearCall) DoAndReturn(f func(context.Context, pp.PP, domain.Domain, ipnet.Type) (bool, string)) *SetterClearCall { +func (c *SetterDeleteCall) DoAndReturn(f func(context.Context, pp.PP, domain.Domain, ipnet.Type) setter.ResponseCode) *SetterDeleteCall { c.Call = c.Call.DoAndReturn(f) return c } // Set mocks base method. -func (m *MockSetter) Set(arg0 context.Context, arg1 pp.PP, arg2 domain.Domain, arg3 ipnet.Type, arg4 netip.Addr, arg5 api.TTL, arg6 bool) (bool, string) { +func (m *MockSetter) Set(arg0 context.Context, arg1 pp.PP, arg2 domain.Domain, arg3 ipnet.Type, arg4 netip.Addr, arg5 api.TTL, arg6 bool) setter.ResponseCode { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Set", arg0, arg1, arg2, arg3, arg4, arg5, arg6) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(string) - return ret0, ret1 + ret0, _ := ret[0].(setter.ResponseCode) + return ret0 } // Set indicates an expected call of Set. @@ -104,19 +103,19 @@ type SetterSetCall struct { } // Return rewrite *gomock.Call.Return -func (c *SetterSetCall) Return(arg0 bool, arg1 string) *SetterSetCall { - c.Call = c.Call.Return(arg0, arg1) +func (c *SetterSetCall) Return(arg0 setter.ResponseCode) *SetterSetCall { + c.Call = c.Call.Return(arg0) return c } // Do rewrite *gomock.Call.Do -func (c *SetterSetCall) Do(f func(context.Context, pp.PP, domain.Domain, ipnet.Type, netip.Addr, api.TTL, bool) (bool, string)) *SetterSetCall { +func (c *SetterSetCall) Do(f func(context.Context, pp.PP, domain.Domain, ipnet.Type, netip.Addr, api.TTL, bool) setter.ResponseCode) *SetterSetCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *SetterSetCall) DoAndReturn(f func(context.Context, pp.PP, domain.Domain, ipnet.Type, netip.Addr, api.TTL, bool) (bool, string)) *SetterSetCall { +func (c *SetterSetCall) DoAndReturn(f func(context.Context, pp.PP, domain.Domain, ipnet.Type, netip.Addr, api.TTL, bool) setter.ResponseCode) *SetterSetCall { c.Call = c.Call.DoAndReturn(f) return c } diff --git a/internal/setter/base.go b/internal/setter/base.go index 1285a19f..c414991c 100644 --- a/internal/setter/base.go +++ b/internal/setter/base.go @@ -17,6 +17,18 @@ import ( //go:generate mockgen -typed -destination=../mocks/mock_setter.go -package=mocks . Setter +// ResponseCode encodes the minimum information to generate messages for monitors and notifiers. +type ResponseCode int + +const ( + // No updates were needed. The records were already okay. + ResponseNoUpdatesNeeded = iota + // Updates were needed and they are done. + ResponseUpdatesApplied + // Updates were needed and they did not fully complete. The records may be inconsistent. + ResponseUpdatesFailed +) + // Setter uses [api.Handle] to update DNS records. type Setter interface { // Set sets a particular domain to the given IP address. @@ -28,13 +40,13 @@ type Setter interface { IP netip.Addr, ttl api.TTL, proxied bool, - ) (bool, string) + ) ResponseCode // Clear removes DNS records of a particular domain. - Clear( + Delete( ctx context.Context, ppfmt pp.PP, Domain domain.Domain, IPNetwork ipnet.Type, - ) (bool, string) + ) ResponseCode } diff --git a/internal/setter/setter.go b/internal/setter/setter.go index aa72260c..ccac2d49 100644 --- a/internal/setter/setter.go +++ b/internal/setter/setter.go @@ -2,7 +2,6 @@ package setter import ( "context" - "fmt" "net/netip" "sort" @@ -49,66 +48,61 @@ func New(_ppfmt pp.PP, handle api.Handle) (Setter, bool) { //nolint:funlen func (s *setter) Set(ctx context.Context, ppfmt pp.PP, domain domain.Domain, ipnet ipnet.Type, ip netip.Addr, ttl api.TTL, proxied bool, -) (bool, string) { +) ResponseCode { recordType := ipnet.RecordType() domainDescription := domain.Describe() rs, ok := s.Handle.ListRecords(ctx, ppfmt, domain, ipnet) if !ok { ppfmt.Errorf(pp.EmojiError, "Failed to retrieve the current %s records of %q", recordType, domainDescription) - return false, fmt.Sprintf("Failed to set %s %s", recordType, domainDescription) + return ResponseUpdatesFailed } // The intention of these two lists is to find or create a good record and then delete everything else. // We prefer recycling existing records (if possible) so that existing TTL and proxy can be preserved. // However, when ip is not valid, we will delete all DNS records. - matchedIDs, unmatchedIDsToUpdate := partitionRecords(rs, ip) + unprocessedMatched, unprocessedUnmatched := partitionRecords(rs, ip) - // uptodate remembers whether the correct DNS record is already present. - uptodate := false - - // duplicateMatchedIDs are to be deleted; this is set when uptodate becomes true. - var duplicateMatchedIDs []string + // foundMatched remembers whether the correct DNS record is already present. + // Stale records may still exist even when foundMatched is true. + foundMatched := false // If it's still not up to date, and there's a matched ID, use it and delete everything else. - // Note that this implies ip is valid, for otherwise uptodate would have been true. - if !uptodate && len(matchedIDs) > 0 { - uptodate = true - duplicateMatchedIDs = matchedIDs[1:] + // Note that this implies ip is valid, for otherwise foundMatched would have been true. + if !foundMatched && len(unprocessedMatched) > 0 { + foundMatched = true + unprocessedMatched = unprocessedMatched[1:] } // If it's up to date and there are no other records, we are done! - if uptodate && len(duplicateMatchedIDs) == 0 && len(unmatchedIDsToUpdate) == 0 { + if foundMatched && len(unprocessedMatched) == 0 && len(unprocessedUnmatched) == 0 { ppfmt.Infof(pp.EmojiAlreadyDone, "The %s records of %q are already up to date", recordType, domainDescription) - return true, "" + return ResponseNoUpdatesNeeded } // This counts the stale records that have not being deleted yet. // - // Message for monitor (e.g. Healthchecks) - monitorMessage := "" - - // We need a different counter (instead of using len(unmatchedIDsToUpdate) all the times) + // We need a different counter (instead of using len(unprocessedUnmatched) all the times) // because when we fail to delete a record, we will give up and remove that record from - // unmatchedIDsToUpdate, but the stale record that could not be deleted should still be + // unprocessedUnmatched, but the stale record that could not be deleted should still be // counted in numUndeletedUnmatched so that we know we have failed to complete the updating. - numUndeletedUnmatched := len(unmatchedIDsToUpdate) + numUndeletedUnmatched := len(unprocessedUnmatched) // If somehow it's still not up to date, it means there are no matched records but ip is valid. // This means we should update one stale record or create a new one with the desired ip. // // Again, we prefer updating stale records instead of creating new ones so that we can // preserve the current TTL and proxy setting. - if !uptodate { - // Temporary local variable for the new unmatchedIDsToUpdate - var unhandledUnmatchedIDs []string + if !foundMatched { + // Temporary local variable for the new unprocessedUnmatched + var newUnprocessedUnmatched []string // Let's go through all stale records - for i, id := range unmatchedIDsToUpdate { + for i, id := range unprocessedUnmatched { // Let's try to update it first. if ok := s.Handle.UpdateRecord(ctx, ppfmt, domain, ipnet, id, ip); !ok { // If the updating fails, we will delete it. - if s.Handle.DeleteRecord(ctx, ppfmt, domain, ipnet, id) { + if ok := s.Handle.DeleteRecord(ctx, ppfmt, domain, ipnet, id); ok { ppfmt.Noticef(pp.EmojiDeleteRecord, "Deleted a stale %s record of %q (ID: %q)", recordType, domainDescription, id) @@ -121,36 +115,37 @@ func (s *setter) Set(ctx context.Context, ppfmt pp.PP, } // If the updating succeeds, we can move on to the next stage! + // + // Note that there can still be stale records at this point. ppfmt.Noticef(pp.EmojiUpdateRecord, "Updated a stale %s record of %q (ID: %q)", recordType, domainDescription, id) - monitorMessage = fmt.Sprintf("Set %s %s to %s", recordType, domainDescription, ip.String()) - // Now it's up to date! Note that matchedIDs must be empty; otherwise uptodate would have been true. - uptodate = true + // Now it's up to date! Note that unprocessedMatched must be empty + // otherwise foundMatched would have been true. + foundMatched = true numUndeletedUnmatched-- - unhandledUnmatchedIDs = unmatchedIDsToUpdate[i+1:] + newUnprocessedUnmatched = unprocessedUnmatched[i+1:] break } - unmatchedIDsToUpdate = unhandledUnmatchedIDs + unprocessedUnmatched = newUnprocessedUnmatched } // If it's still not up to date at this point, it means there are no stale records or that we failed to update - // any one of them. This leaves us no choices---we have to create a new record with the correct ip. - if !uptodate { + // any one of them. This leaves us no choices---we have to create a new record with the correct IP. + if !foundMatched { if id, ok := s.Handle.CreateRecord(ctx, ppfmt, domain, ipnet, ip, ttl, proxied); ok { ppfmt.Noticef(pp.EmojiCreateRecord, "Added a new %s record of %q (ID: %q)", recordType, domainDescription, id) - monitorMessage = fmt.Sprintf("Set %s %s to %s", recordType, domainDescription, ip.String()) - // Now it's up to date! matchedIDs and unmatchedIDsToUpdate must both be empty at this point - uptodate = true + // Now it's up to date! unprocessedMatched and unprocessedUnmatched must both be empty at this point + foundMatched = true } } // Now, we should try to delete all remaining stale records. - for _, id := range unmatchedIDsToUpdate { + for _, id := range unprocessedUnmatched { if s.Handle.DeleteRecord(ctx, ppfmt, domain, ipnet, id) { ppfmt.Noticef(pp.EmojiDeleteRecord, "Deleted a stale %s record of %q (ID: %q)", recordType, domainDescription, id) numUndeletedUnmatched-- @@ -159,7 +154,7 @@ func (s *setter) Set(ctx context.Context, ppfmt pp.PP, // We should also delete all duplicate records even if they are up to date. // This has lower priority than deleting the stale records. - for _, id := range duplicateMatchedIDs { + for _, id := range unprocessedMatched { if s.Handle.DeleteRecord(ctx, ppfmt, domain, ipnet, id) { ppfmt.Noticef(pp.EmojiDeleteRecord, "Deleted a duplicate %s record of %q (ID: %q)", recordType, domainDescription, id) @@ -167,25 +162,25 @@ func (s *setter) Set(ctx context.Context, ppfmt pp.PP, } // Check whether we are done. It is okay to have duplicates, but it is not okay to have remaining stale records. - if !uptodate || numUndeletedUnmatched > 0 { + if !foundMatched || numUndeletedUnmatched > 0 { ppfmt.Errorf(pp.EmojiError, "Failed to complete updating of %s records of %q; records might be inconsistent", recordType, domainDescription) - return false, fmt.Sprintf("Failed to set %s %s", recordType, domainDescription) + return ResponseUpdatesFailed } - return true, monitorMessage + return ResponseUpdatesApplied } -// Clear deletes all managed DNS records. -func (s *setter) Clear(ctx context.Context, ppfmt pp.PP, domain domain.Domain, ipnet ipnet.Type) (bool, string) { +// Delete deletes all managed DNS records. +func (s *setter) Delete(ctx context.Context, ppfmt pp.PP, domain domain.Domain, ipnet ipnet.Type) ResponseCode { recordType := ipnet.RecordType() domainDescription := domain.Describe() rmap, ok := s.Handle.ListRecords(ctx, ppfmt, domain, ipnet) if !ok { ppfmt.Errorf(pp.EmojiError, "Failed to retrieve the current %s records of %q", recordType, domainDescription) - return false, fmt.Sprintf("Failed to clear %s %s", recordType, domainDescription) + return ResponseUpdatesFailed } // Sorting is not needed for correctness, but it will make the function deterministic. @@ -196,13 +191,13 @@ func (s *setter) Clear(ctx context.Context, ppfmt pp.PP, domain domain.Domain, i sort.Strings(unmatchedIDs) if len(unmatchedIDs) == 0 { - ppfmt.Infof(pp.EmojiAlreadyDone, "The %s records of %q are already cleared", recordType, domainDescription) - return true, "" + ppfmt.Infof(pp.EmojiAlreadyDone, "The %s records of %q were already deleted", recordType, domainDescription) + return ResponseNoUpdatesNeeded } allOk := true for _, id := range unmatchedIDs { - if !s.Handle.DeleteRecord(ctx, ppfmt, domain, ipnet, id) { + if ok := s.Handle.DeleteRecord(ctx, ppfmt, domain, ipnet, id); !ok { allOk = false continue } @@ -213,8 +208,8 @@ func (s *setter) Clear(ctx context.Context, ppfmt pp.PP, domain domain.Domain, i ppfmt.Errorf(pp.EmojiError, "Failed to complete deleting of %s records of %q; records might be inconsistent", recordType, domainDescription) - return false, fmt.Sprintf("Failed to clear %s %s", recordType, domainDescription) + return ResponseUpdatesFailed } - return true, fmt.Sprintf("Deleted %s %s", recordType, domainDescription) + return ResponseUpdatesApplied } diff --git a/internal/setter/setter_test.go b/internal/setter/setter_test.go index ec0c0a2b..67c40ab7 100644 --- a/internal/setter/setter_test.go +++ b/internal/setter/setter_test.go @@ -34,8 +34,7 @@ func TestSet(t *testing.T) { for name, tc := range map[string]struct { ip netip.Addr - ok bool - msg string + resp setter.ResponseCode ttl api.TTL proxied bool prepareMockPP func(m *mocks.MockPP) @@ -43,8 +42,7 @@ func TestSet(t *testing.T) { }{ "0/1-false": { ip1, - true, - `Set AAAA sub.test.org to ::1`, + setter.ResponseUpdatesApplied, 1, false, func(m *mocks.MockPP) { @@ -59,8 +57,7 @@ func TestSet(t *testing.T) { }, "1unmatched/300-false": { ip1, - true, - `Set AAAA sub.test.org to ::1`, + setter.ResponseUpdatesApplied, 300, false, func(m *mocks.MockPP) { @@ -80,8 +77,7 @@ func TestSet(t *testing.T) { }, "1unmatched-updatefail/300-false": { ip1, - true, - `Set AAAA sub.test.org to ::1`, + setter.ResponseUpdatesApplied, 300, false, func(m *mocks.MockPP) { @@ -101,8 +97,7 @@ func TestSet(t *testing.T) { }, "1matched/300-false": { ip1, - true, - ``, + setter.ResponseNoUpdatesNeeded, 300, false, func(m *mocks.MockPP) { @@ -114,8 +109,7 @@ func TestSet(t *testing.T) { }, "2matched/300-false": { ip1, - true, - ``, + setter.ResponseUpdatesApplied, 300, false, func(m *mocks.MockPP) { @@ -136,8 +130,7 @@ func TestSet(t *testing.T) { }, "2matched-deletefail/300-false": { ip1, - true, - ``, + setter.ResponseUpdatesApplied, 300, false, nil, @@ -150,8 +143,7 @@ func TestSet(t *testing.T) { }, "2unmatched/300-false": { ip1, - true, - `Set AAAA sub.test.org to ::1`, + setter.ResponseUpdatesApplied, 300, false, func(m *mocks.MockPP) { @@ -163,7 +155,11 @@ func TestSet(t *testing.T) { "sub.test.org", record1, ), - m.EXPECT().Noticef(pp.EmojiDeleteRecord, "Deleted a stale %s record of %q (ID: %q)", "AAAA", "sub.test.org", record2), //nolint:lll + m.EXPECT().Noticef(pp.EmojiDeleteRecord, + "Deleted a stale %s record of %q (ID: %q)", + "AAAA", + "sub.test.org", + record2), ) }, func(ctx context.Context, ppfmt pp.PP, m *mocks.MockHandle) { @@ -176,8 +172,7 @@ func TestSet(t *testing.T) { }, "2unmatched-updatefail/300-false": { ip1, - true, - `Set AAAA sub.test.org to ::1`, + setter.ResponseUpdatesApplied, 300, false, func(m *mocks.MockPP) { @@ -203,8 +198,7 @@ func TestSet(t *testing.T) { }, "2unmatched-updatefailtwice/300-false": { ip1, - true, - `Set AAAA sub.test.org to ::1`, + setter.ResponseUpdatesApplied, 300, false, func(m *mocks.MockPP) { @@ -228,8 +222,7 @@ func TestSet(t *testing.T) { //nolint:dupl "2unmatched-updatefail-deletefail-updatefail/300-false": { ip1, - false, - `Failed to set AAAA sub.test.org`, + setter.ResponseUpdatesFailed, 300, false, func(m *mocks.MockPP) { @@ -253,8 +246,7 @@ func TestSet(t *testing.T) { //nolint:dupl "2unmatched-updatefailtwice-createfail/300-false": { ip1, - false, - `Failed to set AAAA sub.test.org`, + setter.ResponseUpdatesFailed, 300, false, func(m *mocks.MockPP) { @@ -277,8 +269,7 @@ func TestSet(t *testing.T) { }, "listfail/300-false": { ip1, - false, - `Failed to set AAAA sub.test.org`, + setter.ResponseUpdatesFailed, 300, false, func(m *mocks.MockPP) { @@ -308,15 +299,14 @@ func TestSet(t *testing.T) { s, ok := setter.New(mockPP, mockHandle) require.True(t, ok) - ok, msg := s.Set(ctx, mockPP, domain, ipNetwork, tc.ip, tc.ttl, tc.proxied) - require.Equal(t, tc.ok, ok) - require.Equal(t, tc.msg, msg) + resp := s.Set(ctx, mockPP, domain, ipNetwork, tc.ip, tc.ttl, tc.proxied) + require.Equal(t, tc.resp, resp) }) } } //nolint:funlen -func TestClear(t *testing.T) { +func TestDelete(t *testing.T) { t.Parallel() const ( @@ -332,24 +322,21 @@ func TestClear(t *testing.T) { ) for name, tc := range map[string]struct { - ok bool - msg string + resp setter.ResponseCode prepareMockPP func(m *mocks.MockPP) prepareMockHandle func(ctx context.Context, ppfmt pp.PP, m *mocks.MockHandle) }{ "0": { - true, - ``, + setter.ResponseNoUpdatesNeeded, func(m *mocks.MockPP) { - m.EXPECT().Infof(pp.EmojiAlreadyDone, "The %s records of %q are already cleared", "AAAA", "sub.test.org") + m.EXPECT().Infof(pp.EmojiAlreadyDone, "The %s records of %q were already deleted", "AAAA", "sub.test.org") }, func(ctx context.Context, ppfmt pp.PP, m *mocks.MockHandle) { m.EXPECT().ListRecords(ctx, ppfmt, domain, ipNetwork).Return(map[string]netip.Addr{}, true) }, }, "1unmatched": { - true, - `Deleted AAAA sub.test.org`, + setter.ResponseUpdatesApplied, func(m *mocks.MockPP) { m.EXPECT().Noticef(pp.EmojiDeleteRecord, "Deleted a stale %s record of %q (ID: %q)", "AAAA", "sub.test.org", record1) //nolint:lll }, @@ -361,8 +348,7 @@ func TestClear(t *testing.T) { }, }, "1unmatched/fail": { - false, - `Failed to clear AAAA sub.test.org`, + setter.ResponseUpdatesFailed, func(m *mocks.MockPP) { m.EXPECT().Errorf(pp.EmojiError, "Failed to complete deleting of %s records of %q; records might be inconsistent", "AAAA", "sub.test.org") //nolint:lll }, @@ -374,8 +360,7 @@ func TestClear(t *testing.T) { }, }, "impossible-records": { - true, - `Deleted AAAA sub.test.org`, + setter.ResponseUpdatesApplied, func(m *mocks.MockPP) { gomock.InOrder( m.EXPECT().Noticef(pp.EmojiDeleteRecord, "Deleted a stale %s record of %q (ID: %q)", "AAAA", "sub.test.org", record1), //nolint:lll @@ -391,8 +376,7 @@ func TestClear(t *testing.T) { }, }, "listfail": { - false, - `Failed to clear AAAA sub.test.org`, + setter.ResponseUpdatesFailed, func(m *mocks.MockPP) { m.EXPECT().Errorf(pp.EmojiError, "Failed to retrieve the current %s records of %q", "AAAA", "sub.test.org") }, @@ -420,9 +404,8 @@ func TestClear(t *testing.T) { s, ok := setter.New(mockPP, mockHandle) require.True(t, ok) - ok, msg := s.Clear(ctx, mockPP, domain, ipNetwork) - require.Equal(t, tc.ok, ok) - require.Equal(t, tc.msg, msg) + resp := s.Delete(ctx, mockPP, domain, ipNetwork) + require.Equal(t, tc.resp, resp) }) } } diff --git a/internal/updater/updater.go b/internal/updater/updater.go index 002d5a48..b9f18738 100644 --- a/internal/updater/updater.go +++ b/internal/updater/updater.go @@ -63,23 +63,28 @@ func setIP(ctx context.Context, ppfmt pp.PP, ctx, cancel := context.WithTimeoutCause(ctx, c.UpdateTimeout, errSettingTimeout) defer cancel() - ok, msg := s.Set(ctx, ppfmt, domain, ipNet, ip, c.TTL, getProxied(ppfmt, c, domain)) - allOk = allOk && ok - if msg != "" { - msgs[ok] = append(msgs[ok], msg) - } - - if !ok && ShouldDisplayHints["update-timeout"] && errors.Is(context.Cause(ctx), errSettingTimeout) { - ppfmt.Infof(pp.EmojiConfig, "If your network is working but with high latency, consider increasing the value of UPDATE_TIMEOUT") //nolint:lll - ShouldDisplayHints["update-timeout"] = false + resp := s.Set(ctx, ppfmt, domain, ipNet, ip, c.TTL, getProxied(ppfmt, c, domain)) + switch resp { + case setter.ResponseUpdatesApplied: + msgs[true] = append(msgs[true], fmt.Sprintf("Set %s %s to %s", domain.Describe(), ipNet.RecordType(), ip.String())) + case setter.ResponseUpdatesFailed: + allOk = false + msgs[false] = append(msgs[false], fmt.Sprintf("Failed to set %s %s", domain.Describe(), ipNet.RecordType())) + if ShouldDisplayHints["update-timeout"] && errors.Is(context.Cause(ctx), errSettingTimeout) { + ppfmt.Infof(pp.EmojiConfig, + "If your network is working but with high latency, consider increasing the value of UPDATE_TIMEOUT", + ) + ShouldDisplayHints["update-timeout"] = false + } + case setter.ResponseNoUpdatesNeeded: } } return allOk, msgs[allOk] } -// clearIP extracts relevant settings from the configuration and calls [setter.Setter.Clear] with a deadline. -func clearIP(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Setter, ipNet ipnet.Type) (bool, []string) { +// deleteIP extracts relevant settings from the configuration and calls [setter.Setter.Clear] with a deadline. +func deleteIP(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Setter, ipNet ipnet.Type) (bool, []string) { allOk := true // [msgs[false]] collects all the error messages and [msgs[true]] collects all the success messages. @@ -89,10 +94,14 @@ func clearIP(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Setter ctx, cancel := context.WithTimeout(ctx, c.UpdateTimeout) defer cancel() - ok, msg := s.Clear(ctx, ppfmt, domain, ipNet) - allOk = allOk && ok - if msg != "" { - msgs[ok] = append(msgs[ok], msg) + resp := s.Delete(ctx, ppfmt, domain, ipNet) + switch resp { + case setter.ResponseUpdatesApplied: + msgs[true] = append(msgs[true], fmt.Sprintf("Deleted %s %s", domain.Describe(), ipNet.RecordType())) + case setter.ResponseUpdatesFailed: + allOk = false + msgs[false] = append(msgs[false], fmt.Sprintf("Failed to delete %s %s", domain.Describe(), ipNet.RecordType())) + case setter.ResponseNoUpdatesNeeded: } } @@ -160,8 +169,8 @@ func UpdateIPs(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Sett return allOk, allMsg } -// ClearIPs removes all DNS records of managed domains. -func ClearIPs(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Setter) (bool, string) { +// DeleteIPs removes all DNS records of managed domains. +func DeleteIPs(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Setter) (bool, string) { allOk := true // [msgs[false]] collects all the error messages and [msgs[true]] collects all the success messages. @@ -169,7 +178,7 @@ func ClearIPs(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Sette for _, ipNet := range [...]ipnet.Type{ipnet.IP4, ipnet.IP6} { if c.Provider[ipNet] != nil { - ok, msg := clearIP(ctx, ppfmt, c, s, ipNet) + ok, msg := deleteIP(ctx, ppfmt, c, s, ipNet) allOk = allOk && ok msgs[ok] = append(msgs[ok], msg...) } diff --git a/internal/updater/updater_test.go b/internal/updater/updater_test.go index cf371b96..435ab699 100644 --- a/internal/updater/updater_test.go +++ b/internal/updater/updater_test.go @@ -15,6 +15,7 @@ import ( "github.com/favonia/cloudflare-ddns/internal/ipnet" "github.com/favonia/cloudflare-ddns/internal/mocks" "github.com/favonia/cloudflare-ddns/internal/pp" + "github.com/favonia/cloudflare-ddns/internal/setter" "github.com/favonia/cloudflare-ddns/internal/updater" ) @@ -78,40 +79,44 @@ func TestUpdateIPs(t *testing.T) { pp4only, mockproviders{ipnet.IP4: provider4}, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, false).Return(true, "") + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, false). + Return(setter.ResponseNoUpdatesNeeded) }, }, "ip4only/setfail": { proxiedBoth, false, - "error", + "Failed to set ip4.hello A", allHints, pp4only, mockproviders{ipnet.IP4: provider4}, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, true).Return(false, "error") //nolint:lll + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, true). + Return(setter.ResponseUpdatesFailed) }, }, "ip6only": { proxiedNone, true, - "ok", + "Set ip6.hello AAAA to ::1", allHints, pp6only, mockproviders{ipnet.IP6: provider6}, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, false).Return(true, "ok") + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, false). + Return(setter.ResponseUpdatesApplied) }, }, "ip6only/setfail": { proxiedBoth, false, - "bad", + "Failed to set ip6.hello AAAA", allHints, pp6only, mockproviders{ipnet.IP6: provider6}, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, true).Return(false, "bad") //nolint:lll + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, true). + Return(setter.ResponseUpdatesFailed) }, }, "both": { @@ -123,36 +128,42 @@ func TestUpdateIPs(t *testing.T) { mockproviders{ipnet.IP4: provider4, ipnet.IP6: provider6}, func(ppfmt pp.PP, m *mocks.MockSetter) { gomock.InOrder( - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, false).Return(true, ""), - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, false).Return(true, ""), + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, false). + Return(setter.ResponseNoUpdatesNeeded), + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, false). + Return(setter.ResponseNoUpdatesNeeded), ) }, }, "both/setfail1": { proxiedBoth, false, - "hey!", + "Failed to set ip4.hello A", allHints, ppBoth, mockproviders{ipnet.IP4: provider4, ipnet.IP6: provider6}, func(ppfmt pp.PP, m *mocks.MockSetter) { gomock.InOrder( - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, true).Return(false, "hey!"), //nolint:lll - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, true).Return(true, ""), + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, true). + Return(setter.ResponseUpdatesFailed), + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, true). + Return(setter.ResponseNoUpdatesNeeded), ) }, }, "both/setfail2": { proxiedNone, false, - "wrong", + "Failed to set ip6.hello AAAA", allHints, ppBoth, mockproviders{ipnet.IP4: provider4, ipnet.IP6: provider6}, func(ppfmt pp.PP, m *mocks.MockSetter) { gomock.InOrder( - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, false).Return(true, ""), - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, false).Return(false, "wrong"), //nolint:lll + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, false). + Return(setter.ResponseNoUpdatesNeeded), + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, false). + Return(setter.ResponseUpdatesFailed), ) }, }, @@ -175,7 +186,8 @@ func TestUpdateIPs(t *testing.T) { ipnet.IP6: provider6, }, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, true).Return(true, "looking good") //nolint:lll + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip6.hello"), ipnet.IP6, ip6, api.TTLAuto, true). + Return(setter.ResponseNoUpdatesNeeded) }, }, "ip6fails": { @@ -199,7 +211,8 @@ func TestUpdateIPs(t *testing.T) { }, }, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, false).Return(true, "good") //nolint:lll + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, false). + Return(setter.ResponseNoUpdatesNeeded) }, }, "ip6fails/again": { @@ -220,7 +233,8 @@ func TestUpdateIPs(t *testing.T) { }, }, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, true).Return(true, "") + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, true). + Return(setter.ResponseNoUpdatesNeeded) }, }, "bothfail": { @@ -251,7 +265,7 @@ func TestUpdateIPs(t *testing.T) { "ip4only-proxied-nil": { mockproxied{}, true, - "response", + "Set ip4.hello A to 127.0.0.1", allHints, func(m *mocks.MockPP) { gomock.InOrder( @@ -264,13 +278,14 @@ func TestUpdateIPs(t *testing.T) { }, mockproviders{ipnet.IP4: provider4}, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, false).Return(true, "response") //nolint:lll + m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, false). + Return(setter.ResponseUpdatesApplied) }, }, "slow-setting": { proxiedNone, false, - "response", + "Failed to set ip4.hello A", allHints, func(m *mocks.MockPP) { gomock.InOrder( @@ -286,9 +301,9 @@ func TestUpdateIPs(t *testing.T) { func(ppfmt pp.PP, m *mocks.MockSetter) { m.EXPECT().Set(gomock.Any(), ppfmt, domain.FQDN("ip4.hello"), ipnet.IP4, ip4, api.TTLAuto, false). DoAndReturn( - func(_ context.Context, _ pp.PP, _ domain.Domain, _ ipnet.Type, _ netip.Addr, _ api.TTL, _ bool) (bool, string) { //nolint:lll + func(_ context.Context, _ pp.PP, _ domain.Domain, _ ipnet.Type, _ netip.Addr, _ api.TTL, _ bool) setter.ResponseCode { //nolint:lll time.Sleep(2 * time.Second) - return false, "response" + return setter.ResponseUpdatesFailed }) }, }, @@ -331,7 +346,7 @@ func TestUpdateIPs(t *testing.T) { } //nolint:funlen,paralleltest // updater.IPv6MessageDisplayed is a global variable -func TestClearIPs(t *testing.T) { +func TestDeleteIPs(t *testing.T) { domain4 := domain.FQDN("ip4.hello") domain6 := domain.FQDN("ip6.hello") domains := map[ipnet.Type][]domain.Domain{ @@ -365,86 +380,87 @@ func TestClearIPs(t *testing.T) { "ip4only": { proxiedNone, true, - "hello", + "Deleted ip4.hello A", allHints, nil, mockproviders{ipnet.IP4: true}, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Clear(gomock.Any(), ppfmt, domain4, ipnet.IP4).Return(true, "hello") + m.EXPECT().Delete(gomock.Any(), ppfmt, domain4, ipnet.IP4). + Return(setter.ResponseUpdatesApplied) }, }, "ip4only/setfail": { proxiedNone, false, - "err", + "Failed to delete ip4.hello A", allHints, nil, mockproviders{ipnet.IP4: true}, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Clear(gomock.Any(), ppfmt, domain4, ipnet.IP4).Return(false, "err") + m.EXPECT().Delete(gomock.Any(), ppfmt, domain4, ipnet.IP4).Return(setter.ResponseUpdatesFailed) }, }, "ip6only": { proxiedNone, true, - "", + "Deleted ip6.hello AAAA", allHints, nil, mockproviders{ipnet.IP6: true}, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Clear(gomock.Any(), ppfmt, domain6, ipnet.IP6).Return(true, "") + m.EXPECT().Delete(gomock.Any(), ppfmt, domain6, ipnet.IP6).Return(setter.ResponseUpdatesApplied) }, }, "ip6only/setfail": { proxiedNone, false, - "test", + "Failed to delete ip6.hello AAAA", allHints, nil, mockproviders{ipnet.IP6: true}, func(ppfmt pp.PP, m *mocks.MockSetter) { - m.EXPECT().Clear(gomock.Any(), ppfmt, domain6, ipnet.IP6).Return(false, "test") + m.EXPECT().Delete(gomock.Any(), ppfmt, domain6, ipnet.IP6).Return(setter.ResponseUpdatesFailed) }, }, "both": { proxiedNone, true, - "both\nneither", + "Deleted ip4.hello A\nDeleted ip6.hello AAAA", allHints, nil, mockproviders{ipnet.IP4: true, ipnet.IP6: true}, func(ppfmt pp.PP, m *mocks.MockSetter) { gomock.InOrder( - m.EXPECT().Clear(gomock.Any(), ppfmt, domain4, ipnet.IP4).Return(true, "both"), - m.EXPECT().Clear(gomock.Any(), ppfmt, domain6, ipnet.IP6).Return(true, "neither"), + m.EXPECT().Delete(gomock.Any(), ppfmt, domain4, ipnet.IP4).Return(setter.ResponseUpdatesApplied), + m.EXPECT().Delete(gomock.Any(), ppfmt, domain6, ipnet.IP6).Return(setter.ResponseUpdatesApplied), ) }, }, "both/setfail1": { proxiedNone, false, - "", + "Failed to delete ip4.hello A", allHints, nil, mockproviders{ipnet.IP4: true, ipnet.IP6: true}, func(ppfmt pp.PP, m *mocks.MockSetter) { gomock.InOrder( - m.EXPECT().Clear(gomock.Any(), ppfmt, domain4, ipnet.IP4).Return(false, ""), - m.EXPECT().Clear(gomock.Any(), ppfmt, domain6, ipnet.IP6).Return(true, "999"), + m.EXPECT().Delete(gomock.Any(), ppfmt, domain4, ipnet.IP4).Return(setter.ResponseUpdatesFailed), + m.EXPECT().Delete(gomock.Any(), ppfmt, domain6, ipnet.IP6).Return(setter.ResponseNoUpdatesNeeded), ) }, }, "both/setfail2": { proxiedNone, false, - "2", + "Failed to delete ip6.hello AAAA", allHints, nil, mockproviders{ipnet.IP4: true, ipnet.IP6: true}, func(ppfmt pp.PP, m *mocks.MockSetter) { gomock.InOrder( - m.EXPECT().Clear(gomock.Any(), ppfmt, domain4, ipnet.IP4).Return(true, "1"), - m.EXPECT().Clear(gomock.Any(), ppfmt, domain6, ipnet.IP6).Return(false, "2"), + m.EXPECT().Delete(gomock.Any(), ppfmt, domain4, ipnet.IP4).Return(setter.ResponseNoUpdatesNeeded), + m.EXPECT().Delete(gomock.Any(), ppfmt, domain6, ipnet.IP6).Return(setter.ResponseUpdatesFailed), ) }, }, @@ -476,7 +492,7 @@ func TestClearIPs(t *testing.T) { if tc.prepareMockSetter != nil { tc.prepareMockSetter(mockPP, mockSetter) } - ok, msg := updater.ClearIPs(ctx, mockPP, conf, mockSetter) + ok, msg := updater.DeleteIPs(ctx, mockPP, conf, mockSetter) require.Equal(t, tc.ok, ok) require.Equal(t, tc.msg, msg) })