fix(rewrite): fix cname target rewrite for CNAME chains (#7853)

* fix(rewrite): fix cname target rewrite for CNAME chains

This fix corrects the cname target rewrite to handle CNAME chains:
- Preserves only the CNAME records before matching the rule
- Rewrites only the CNAME target that matches the rule
- Includes all records from the re-resolved upstream response

Signed-off-by: hide <hide@hide.net.eu.org>

* docs(rewrite): document how answer records are handled in CNAME target rewrite

Signed-off-by: hide <hide@hide.net.eu.org>

* fix(rewrite): simplify slice append per staticcheck S1011

Signed-off-by: hide <hide@hide.net.eu.org>

* docs(rewrite): add extra line between code and paragraph

Signed-off-by: hide <hide@hide.net.eu.org>

---------

Signed-off-by: hide <hide@hide.net.eu.org>
Co-authored-by: hide <hide@hide.net.eu.org>

plugin/rewrite/README.md

@@ -503,9 +503,9 @@ If only some calls contain the `revert` flag, then the value in the response wil
 
 ## CNAME Field Rewrites
 
-There might be a scenario where you want the `CNAME` target of the response to be rewritten. You can do this by using the `CNAME` field rewrite. This will generate new answer records according to the new `CNAME` target.
+There might be a scenario where you want the `CNAME` target of the response to be rewritten. You can do this by using the `CNAME` field rewrite. Answer records preceding the `CNAME` target are kept unchanged, the `CNAME` target is rewritten, and the subsequent records are replaced with the lookup result of the rewritten `CNAME` target.
 
-The syntax for the CNAME rewrite rule is as follows. The meaning of
+The syntax for the `CNAME` rewrite rule is as follows. The meaning of
 `exact|prefix|suffix|substring|regex` is the same as with the name rewrite rules.
 An omitted type is defaulted to `exact`.
 
@@ -527,7 +527,8 @@ $ dig @10.1.1.1 my-app.com
 ;my-app.com. IN A
 
 ;; ANSWER SECTION:
-my-app.com.                  200  IN  CNAME  my-app.com.cdn.example.net.
+my-app.com.                  200  IN  CNAME  my-app.example.
+my-app.example.              200  IN  CNAME  my-app.com.cdn.example.net.
 my-app.com.cdn.example.net.  300  IN  A      20.2.0.1
 my-app.com.cdn.example.net.  300  IN  A      20.2.0.2
 ```
@@ -541,7 +542,9 @@ $ dig @10.1.1.1 my-app.com
 ;my-app.com. IN A
 
 ;; ANSWER SECTION:
-my-app.com.                  200  IN  CNAME  my-app.com.other.cdn.com.
+my-app.com.                  200  IN  CNAME  my-app.example.
+my-app.example.              200  IN  CNAME  my-app.com.other.cdn.com.
 my-app.com.other.cdn.com.    100  IN  A      30.3.1.2
 ```
+
 Note that the answer will contain a completely different set of answer records after rewriting the `CNAME` target.

plugin/rewrite/cname_target.go

@@ -91,21 +91,21 @@ func (r *cnameTargetRuleWithReqState) RewriteResponse(res *dns.Msg, rr dns.RR) {
 				}
 
 				var newAnswer []dns.RR
-				// iterate over first upstram response
+				// iterate over first upstream response
 				// add the cname record to the new answer
 				for _, rr := range res.Answer {
 					if cname, ok := rr.(*dns.CNAME); ok {
-						// change the target name in the response
+						// preserve CNAME records until the rewrite target
-						cname.Target = toTarget
-						newAnswer = append(newAnswer, rr)
-					}
-				}
-				// iterate over upstream response received
-				for _, rr := range upRes.Answer {
-					if rr.Header().Name == toTarget {
 						newAnswer = append(newAnswer, rr)
+						if cname.Target == fromTarget {
+							// change the target name in the response
+							cname.Target = toTarget
+							break
+						}
 					}
 				}
+				// add the upstream response to the new answer
+				newAnswer = append(newAnswer, upRes.Answer...)
 				res.Answer = newAnswer
 				// if not propagated, the truncated response might get cached,
 				// and it will be impossible to resolve the full response

plugin/rewrite/cname_target_test.go

@@ -61,6 +61,12 @@ func (u *MockedUpstream) Lookup(ctx context.Context, state request.Request, name
 		}
 		m.Truncated = true
 		return m, nil
+	case "intermediate-2.staging.":
+		m.Answer = []dns.RR{
+			test.CNAME("intermediate-2.staging.   200  IN  CNAME  final.staging.net."),
+			test.A("final.staging.net.  120  IN  A  5.6.7.8"),
+		}
+		return m, nil
 	}
 	return &dns.Msg{}, nil
 }
@@ -76,6 +82,7 @@ func TestCNameTargetRewrite(t *testing.T) {
 		{[]string{"continue", "cname", "substring", "efgh", "zzzz.www"}, reflect.TypeFor[*cnameTargetRule]()},
 		{[]string{"continue", "cname", "regex", `(.*)\.web\.(.*)\.site\.`, `{1}.webapp.{2}.org.`}, reflect.TypeFor[*cnameTargetRule]()},
 		{[]string{"continue", "cname", "exact", "music.truncated.spotify.com.", "music.truncated.spotify.com."}, reflect.TypeFor[*cnameTargetRule]()},
+		{[]string{"continue", "cname", "suffix", "prod.", "staging."}, reflect.TypeFor[*cnameTargetRule]()},
 	}
 	rules := make([]Rule, 0, len(ruleset))
 	for i, r := range ruleset {
@@ -180,6 +187,21 @@ func doTestCNameTargetTests(t *testing.T, rules []Rule) {
 			},
 			true,
 		},
+		{"cname-chain.org.", dns.TypeA,
+			[]dns.RR{
+				test.CNAME("cname-chain.org.   200  IN  CNAME  intermediate-1.com"),
+				test.CNAME("intermediate-1.com   200  IN  CNAME  intermediate-2.prod."),
+				test.CNAME("intermediate-2.prod.   200  IN  CNAME  final.prod.net."),
+				test.A("final.prod.net.  120  IN  A  1.2.3.4"),
+			},
+			[]dns.RR{
+				test.CNAME("cname-chain.org.  200   IN  CNAME  intermediate-1.com"),
+				test.CNAME("intermediate-1.com   200  IN  CNAME  intermediate-2.staging."),
+				test.CNAME("intermediate-2.staging.   200  IN  CNAME  final.staging.net."),
+				test.A("final.staging.net.  120  IN  A  5.6.7.8"),
+			},
+			false,
+		},
 	}
 	ctx := context.TODO()
 	for i, tc := range tests {