From 4afa6071ede1327eafaeee975bcf855a53b0cb9e Mon Sep 17 00:00:00 2001 From: Daniel Muehlbachler-Pietrzykowski Date: Sat, 2 Mar 2024 11:21:39 +0100 Subject: [PATCH] feat: deprecate migration from the old rules syntax to the new syntax BREAKING CHANGE: removes the migration path to the new rules syntax --- internal/adguard/provider.go | 15 +++--- internal/adguard/provider_test.go | 81 +++---------------------------- 2 files changed, 14 insertions(+), 82 deletions(-) diff --git a/internal/adguard/provider.go b/internal/adguard/provider.go index 3e438b7..785b5a4 100644 --- a/internal/adguard/provider.go +++ b/internal/adguard/provider.go @@ -67,7 +67,7 @@ func (p *Provider) ApplyChanges(ctx context.Context, changes *plan.Changes) erro // collect all non-managed rules and existing endpoints managed by external-dns for _, r := range or { - ep, err := deserializeToEndpoint(r, true) + ep, err := deserializeToEndpoint(r) if err != nil { // rules not managed by external-dns are kept if errors.Is(err, errNotManaged) { @@ -86,8 +86,7 @@ func (p *Provider) ApplyChanges(ctx context.Context, changes *plan.Changes) erro } // delete all records to be updated or deleted - // TODO: remove appending changes.Create once we remove backward compatibility - for _, dep := range append(changes.Create, append(changes.UpdateOld, changes.Delete...)...) { + for _, dep := range append(changes.UpdateOld, changes.Delete...) { epk := dep.DNSName + dep.RecordType if ep, ok := eps[epk]; ok { for _, t := range dep.Targets { @@ -147,7 +146,7 @@ func (p *Provider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) { // endpoints are referenced by dns name and record type eps := make(map[string]*endpoint.Endpoint) for _, rule := range resp { - ep, err := deserializeToEndpoint(rule, false) + ep, err := deserializeToEndpoint(rule) if err != nil { // unmanaged rules are ignored if err == errNotManaged { @@ -185,22 +184,20 @@ func endpointSupported(e *endpoint.Endpoint) bool { e.RecordType == endpoint.RecordTypeMX } -func deserializeToEndpoint(rule string, migration bool) (*endpoint.Endpoint, error) { +func deserializeToEndpoint(rule string) (*endpoint.Endpoint, error) { // format: "|DNS.NAME^dnsrewrite=NOERROR;RECORD_TYPE;TARGET" p := strings.SplitN(rule, ";", 3) if len(p) != 3 { return nil, errNotManaged } dp := strings.SplitN(p[0], "^", 2) - // TODO: we support backward compatibility with the old format, but we should remove it in the future (remove !migration) - if !migration && strings.HasPrefix(dp[0], "||") { + if strings.HasPrefix(dp[0], "||") { return nil, errNotManaged } if len(dp) != 2 { return nil, fmt.Errorf("invalid rule: %s", rule) } - // TODO: once we remove backward compatibility, we should remove the replace - d := strings.TrimPrefix(strings.Replace(dp[0], "||", "|", 1), "|") + d := strings.TrimPrefix(dp[0], "|") // see serializeToString for the format r := &endpoint.Endpoint{ diff --git a/internal/adguard/provider_test.go b/internal/adguard/provider_test.go index c90ff32..f46577f 100644 --- a/internal/adguard/provider_test.go +++ b/internal/adguard/provider_test.go @@ -182,41 +182,16 @@ func TestDeserializeToEndpoint(t *testing.T) { text: "@@||abc.com", expectedErr: true, }, - } - - for i, tc := range testCases { - t.Run(fmt.Sprintf("%d. %s", i+1, tc.name), func(t *testing.T) { - ep, err := deserializeToEndpoint(tc.text, false) - if tc.expectedErr { - require.Error(t, err) - } else { - require.NoError(t, err) - require.Equal(t, tc.endpoint, ep) - } - }) - } -} - -// TODO: remove this once we removed backward compatibility -func TestDeserializeToEndpointMigration(t *testing.T) { - log.SetLevel(log.DebugLevel) - - testCases := []struct { - name string - text string - endpoint *endpoint.Endpoint - expectedErr bool - }{ { - name: "migration A record", - text: "||domain.com^$dnsrewrite=NOERROR;A;1.1.1.1", - endpoint: &endpoint.Endpoint{DNSName: "domain.com", RecordType: endpoint.RecordTypeA, Targets: []string{"1.1.1.1"}}, + name: "migration A record", + text: "||domain.com^$dnsrewrite=NOERROR;A;1.1.1.1", + expectedErr: true, }, } for i, tc := range testCases { t.Run(fmt.Sprintf("%d. %s", i+1, tc.name), func(t *testing.T) { - ep, err := deserializeToEndpoint(tc.text, true) + ep, err := deserializeToEndpoint(tc.text) if tc.expectedErr { require.Error(t, err) } else { @@ -488,49 +463,7 @@ func TestApplyChanges(t *testing.T) { }, }, { - name: "valid create of existing resources", - hasError: false, - domainFilter: endpoint.DomainFilter{}, - filteringRules: getFilteringRules{ - UserRules: []string{ - "|domain.com^$dnsrewrite=NOERROR;A;2.2.2.2", - }, - }, - rules: []string{ - "|domain.com^$dnsrewrite=NOERROR;A;1.1.1.1", - "|domain.com^$dnsrewrite=NOERROR;A;2.2.2.2", - "|domain.com^$dnsrewrite=NOERROR;AAAA;1111:1111::1", - "|other.org^$dnsrewrite=NOERROR;A;3.3.3.3", - }, - changes: &plan.Changes{ - Create: []*endpoint.Endpoint{ - { - DNSName: "domain.com", - RecordType: endpoint.RecordTypeA, - Targets: []string{ - "1.1.1.1", - "2.2.2.2", - }, - }, - { - DNSName: "domain.com", - RecordType: endpoint.RecordTypeAAAA, - Targets: []string{ - "1111:1111::1", - }, - }, - { - DNSName: "other.org", - RecordType: endpoint.RecordTypeA, - Targets: []string{ - "3.3.3.3", - }, - }, - }, - }, - }, - { - name: "valid create of existing resources with old format", // TODO: fix once we removed backward compatibility + name: "create of existing resources with old format", hasError: false, domainFilter: endpoint.DomainFilter{}, filteringRules: getFilteringRules{ @@ -539,6 +472,7 @@ func TestApplyChanges(t *testing.T) { }, }, rules: []string{ + "||domain.com^$dnsrewrite=NOERROR;A;2.2.2.2", "|domain.com^$dnsrewrite=NOERROR;A;1.1.1.1", "|domain.com^$dnsrewrite=NOERROR;A;2.2.2.2", "|domain.com^$dnsrewrite=NOERROR;AAAA;1111:1111::1", @@ -800,7 +734,7 @@ func TestApplyChanges(t *testing.T) { }, }, { - name: "update same rule with old rule format", // TODO: fix this test case once we removed backward compatibility + name: "update same rule with old rule format", hasError: false, domainFilter: endpoint.DomainFilter{}, filteringRules: getFilteringRules{ @@ -809,6 +743,7 @@ func TestApplyChanges(t *testing.T) { }, }, rules: []string{ + "||domain.com^$dnsrewrite=NOERROR;A;1.1.1.1", "|domain.com^$dnsrewrite=NOERROR;A;1.1.1.1", }, changes: &plan.Changes{