plugin/k8s_external: Fixes a nil pointer dereference panic when upstream lookup returns no response (#8518)

* plugin/k8s_external: Fixes a nil pointer dereference panic when upstream lookup returns no response

When a CNAME-hosted service is resolved, k8s_external performs internal
upstream lookups for the target name. Upstream.Lookup can return a nil
message with a nil error when the internal self-query's plugin chain
returns a ClientWrite rcode without writing a response (for example
acl's drop action), and the a/aaaa/srv handlers then dereference
resp.Answer on a nil resp and panic.

Guard all four lookups with err == nil && resp != nil, matching the nil
checks already used in plugin/backend_lookup.go and the guard recently
added to plugin/dns64 (#8511).

Signed-off-by: zjncs <18910855655@163.com>

* plugin/k8s_external: silence unused-parameter lint in nil upstream test

The CI lint flagged the test handler's unused 'w' parameter. Rename it
to '_' so golangci-lint (revive unused-parameter) passes. No behavior
change.

Signed-off-by: zjncs <18910855655@163.com>

---------

Signed-off-by: zjncs <18910855655@163.com>

plugin/k8s_external/external_test.go

@@ -4,9 +4,12 @@ import (
 	"context"
 	"testing"
 
+	"github.com/coredns/coredns/core/dnsserver"
+	"github.com/coredns/coredns/plugin"
 	"github.com/coredns/coredns/plugin/kubernetes"
 	"github.com/coredns/coredns/plugin/kubernetes/object"
 	"github.com/coredns/coredns/plugin/pkg/dnstest"
+	"github.com/coredns/coredns/plugin/pkg/upstream"
 	"github.com/coredns/coredns/plugin/test"
 	"github.com/coredns/coredns/request"
 
@@ -55,6 +58,49 @@ func TestExternal(t *testing.T) {
 	}
 }
 
+// TestExternalCNAMENilUpstreamResponse checks that a CNAME-hosted service does not
+// panic when the internal upstream lookup returns no response, e.g. when a plugin
+// like acl's drop action returns success without writing.
+func TestExternalCNAMENilUpstreamResponse(t *testing.T) {
+	k := kubernetes.New([]string{"cluster.local."})
+	k.Namespaces = map[string]struct{}{"testns": {}}
+	k.APIConn = &external{}
+
+	cfg := &dnsserver.Config{
+		Zone: ".",
+		Plugin: []plugin.Plugin{
+			func(plugin.Handler) plugin.Handler {
+				return plugin.HandlerFunc(func(_ context.Context, _ dns.ResponseWriter, _ *dns.Msg) (int, error) {
+					return dns.RcodeSuccess, nil
+				})
+			},
+		},
+	}
+	srv, err := dnsserver.NewServer("", []*dnsserver.Config{cfg})
+	if err != nil {
+		t.Fatal(err)
+	}
+	ctx := context.WithValue(context.Background(), dnsserver.Key{}, srv)
+
+	e := New()
+	e.Zones = []string{"example.com."}
+	e.headless = true
+	e.Next = test.NextHandler(dns.RcodeSuccess, nil)
+	e.externalFunc = k.External
+	e.externalAddrFunc = externalAddress
+	e.externalSerialFunc = externalSerial
+	e.upstream = upstream.New()
+
+	m := new(dns.Msg)
+	m.SetQuestion("svc12.testns.example.com.", dns.TypeA)
+	w := dnstest.NewRecorder(&test.ResponseWriter{})
+
+	_, err = e.ServeDNS(ctx, w, m)
+	if err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+}
+
 var tests = []test.Case{
 	// PTR reverse lookup
 	{

plugin/k8s_external/msg_to_dns.go

@@ -21,7 +21,7 @@ func (e *External) a(ctx context.Context, services []msg.Service, state request.
 		case dns.TypeCNAME:
 			rr := s.NewCNAME(state.QName(), s.Host)
 			records = append(records, rr)
-			if resp, err := e.upstream.Lookup(ctx, state, dns.Fqdn(s.Host), dns.TypeA); err == nil {
+			if resp, err := e.upstream.Lookup(ctx, state, dns.Fqdn(s.Host), dns.TypeA); err == nil && resp != nil {
 				records = append(records, resp.Answer...)
 				if resp.Truncated {
 					truncated = true
@@ -53,7 +53,7 @@ func (e *External) aaaa(ctx context.Context, services []msg.Service, state reque
 		case dns.TypeCNAME:
 			rr := s.NewCNAME(state.QName(), s.Host)
 			records = append(records, rr)
-			if resp, err := e.upstream.Lookup(ctx, state, dns.Fqdn(s.Host), dns.TypeAAAA); err == nil {
+			if resp, err := e.upstream.Lookup(ctx, state, dns.Fqdn(s.Host), dns.TypeAAAA); err == nil && resp != nil {
 				records = append(records, resp.Answer...)
 				if resp.Truncated {
 					truncated = true
@@ -132,10 +132,10 @@ func (e *External) srv(ctx context.Context, services []msg.Service, state reques
 				records = append(records, srv)
 			}
 			if ok := isDuplicate(dup, srv.Target, addr, 0); !ok {
-				if resp, err := e.upstream.Lookup(ctx, state, addr, dns.TypeA); err == nil {
+				if resp, err := e.upstream.Lookup(ctx, state, addr, dns.TypeA); err == nil && resp != nil {
 					extra = append(extra, resp.Answer...)
 				}
-				if resp, err := e.upstream.Lookup(ctx, state, addr, dns.TypeAAAA); err == nil {
+				if resp, err := e.upstream.Lookup(ctx, state, addr, dns.TypeAAAA); err == nil && resp != nil {
 					extra = append(extra, resp.Answer...)
 				}
 			}