feat: deprecate migration from the old rules syntax to the new syntax

BREAKING CHANGE: removes the migration path to the new rules syntax
muhlba91 · Mar 2, 2024 · 4afa607 · 4afa607
1 parent 743d056
commit 4afa607
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 82 deletions.
diff --git 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
@@ -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{