[client] Skip peers without a PTR record in kubernetes commands (#8360)

* [client] Skip peers without a PTR record in kubernetes commands

getKubernetesClusters returned on the first failed reverse lookup, so a
single peer without a PTR record made `netbird kubernetes list` and
`write-kubeconfig` fail even when a netbird-kubeapi-proxy peer was
reachable later in the list. Log the failure at debug and keep scanning,
stopping only when the context is cancelled.

* [client] Return cancellation when the last peer's lookup is skipped

A failed lookup or fingerprint on the final peer was skipped and the
loop ended with a nil error, so a cancelled command reported no clusters
instead of the cancellation.

* [client] Report the lookup error when no peer resolves

Skipping every failed reverse lookup hid a broken resolver behind "No
Kubernetes clusters available" or "cluster not found", with the cause
only in the debug log. When no lookup succeeds, return the last lookup
error. One resolvable peer still means the resolver works, so per-peer
failures stay skipped.

* [client] Stop the kubernetes peer scan when the resolver is down

With a dead resolver every peer waited out the resolver timeout in turn
before the error was reported. A missing PTR record answers NXDOMAIN, so
a timeout or SERVFAIL before any peer resolved means the resolver itself
is failing: return it at once. After one peer resolved, later failures
are still skipped.

* [client] Extract kubernetes cluster probing from the peer scan

The lookup error handling pushed findKubernetesClusters past the
cognitive complexity limit. Move the FQDN match and fingerprint into
probeCluster so the scan loop only handles lookups and deduplication.

client/cmd/kubernetes.go

@@ -132,48 +132,89 @@ func getKubernetesClusters(ctx context.Context, peers []*proto.PeerState, nameFi
 		// https://github.com/golang/go/issues/17093
 		PreferGo: true,
 	}
+	return findKubernetesClusters(ctx, httpClient, resolver.LookupAddr, peers, nameFilter)
+}
 
+func findKubernetesClusters(
+	ctx context.Context,
+	httpClient *http.Client,
+	lookupAddr func(context.Context, string) ([]string, error),
+	peers []*proto.PeerState,
+	nameFilter string,
+) ([]kubernetesCluster, error) {
 	kcs := []kubernetesCluster{}
 	attempted := map[string]struct{}{}
+	var lookupErr error
+	resolvedAny := false
 	for _, peer := range peers {
-		fqdns, err := resolver.LookupAddr(ctx, peer.IP)
+		if err := ctx.Err(); err != nil {
-		if err != nil {
 			return nil, err
 		}
+		// Peers without a DNS record have no PTR, which must not hide the remaining peers.
+		fqdns, err := lookupAddr(ctx, peer.IP)
+		if err != nil {
+			log.Debugf("reverse lookup of peer %s: %v", peer.IP, err)
+			// Before any success, a timeout or SERVFAIL means the resolver is down; waiting
+			// on each remaining peer would only repeat the timeout.
+			if !resolvedAny && !isNotFoundErr(err) {
+				return nil, fmt.Errorf("reverse lookup of peers: %w", err)
+			}
+			lookupErr = err
+			continue
+		}
+		resolvedAny = true
 		for _, fqdn := range fqdns {
 			if _, ok := attempted[fqdn]; ok {
 				continue
 			}
 			attempted[fqdn] = struct{}{}
-			comps := strings.Split(fqdn, ".")
+			kc, ok := probeCluster(ctx, httpClient, fqdn, nameFilter)
-			if len(comps) < 2 {
+			if !ok {
 				continue
 			}
-			if comps[1] != KubernetesDNSSuffix {
-				continue
-			}
-			if nameFilter != "" && nameFilter != comps[0] {
-				continue
-			}
-			clusterURL, clusterVersion, err := fingerprintClusters(ctx, httpClient, fqdn)
-			if err != nil {
-				log.Debugf("could not fingerprint Kubernetes cluster %s %q", fqdn, err)
-				continue
-			}
-			kc := kubernetesCluster{
-				name:    comps[0],
-				url:     clusterURL,
-				version: clusterVersion,
-			}
 			if nameFilter != "" {
 				return []kubernetesCluster{kc}, nil
 			}
 			kcs = append(kcs, kc)
 		}
 	}
+	// A skipped failure on the last peer would otherwise hide a cancellation.
+	if err := ctx.Err(); err != nil {
+		return nil, err
+	}
+	// Every lookup failing points at the resolver rather than at individual peers.
+	if !resolvedAny && lookupErr != nil {
+		return nil, fmt.Errorf("reverse lookup of peers: %w", lookupErr)
+	}
 	return kcs, nil
 }
 
+// probeCluster fingerprints fqdn when it names a netbird-kubeapi-proxy peer matching nameFilter.
+func probeCluster(ctx context.Context, httpClient *http.Client, fqdn, nameFilter string) (kubernetesCluster, bool) {
+	comps := strings.Split(fqdn, ".")
+	if len(comps) < 2 || comps[1] != KubernetesDNSSuffix {
+		return kubernetesCluster{}, false
+	}
+	if nameFilter != "" && nameFilter != comps[0] {
+		return kubernetesCluster{}, false
+	}
+	clusterURL, clusterVersion, err := fingerprintClusters(ctx, httpClient, fqdn)
+	if err != nil {
+		log.Debugf("could not fingerprint Kubernetes cluster %s %q", fqdn, err)
+		return kubernetesCluster{}, false
+	}
+	return kubernetesCluster{
+		name:    comps[0],
+		url:     clusterURL,
+		version: clusterVersion,
+	}, true
+}
+
+func isNotFoundErr(err error) bool {
+	var dnsErr *net.DNSError
+	return errors.As(err, &dnsErr) && dnsErr.IsNotFound
+}
+
 func fingerprintClusters(ctx context.Context, httpClient *http.Client, fqdn string) (*url.URL, string, error) {
 	clusterURL, err := url.Parse("https://" + fqdn)
 	if err != nil {

client/cmd/kubernetes_test.go

@@ -1,6 +1,10 @@
 package cmd
 
 import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -9,7 +13,10 @@ import (
 	"testing"
 
 	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/client/proto"
 )
 
 func TestFingerprintClusters(t *testing.T) {
@@ -27,6 +34,107 @@ func TestFingerprintClusters(t *testing.T) {
 	require.Equal(t, "foobar", clusterVersion)
 }
 
+func TestFindKubernetesClustersSkipsFailedLookup(t *testing.T) {
+	t.Parallel()
+
+	srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte(`{"gitVersion": "v1.30.0"}`)) //nolint:errcheck
+	}))
+	defer srv.Close()
+
+	// Route every cluster FQDN to the test server, regardless of the resolved name.
+	httpClient := &http.Client{Transport: &http.Transport{
+		TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec
+		DialContext: func(ctx context.Context, network, _ string) (net.Conn, error) {
+			return (&net.Dialer{}).DialContext(ctx, network, srv.Listener.Addr().String())
+		},
+	}}
+
+	// The first peer has no DNS record in NetBird, so its reverse lookup fails.
+	lookupAddr := func(_ context.Context, addr string) ([]string, error) {
+		switch addr {
+		case "100.64.0.1":
+			return nil, &net.DNSError{Err: "no such host", Name: "1.0.64.100.in-addr.arpa.", IsNotFound: true}
+		case "100.64.0.2":
+			return []string{"homelab." + KubernetesDNSSuffix + ".netbird.cloud."}, nil
+		}
+		return nil, errors.New("unexpected address " + addr)
+	}
+	peers := []*proto.PeerState{{IP: "100.64.0.1"}, {IP: "100.64.0.2"}}
+
+	for _, filter := range []string{"", "homelab"} {
+		kcs, err := findKubernetesClusters(t.Context(), httpClient, lookupAddr, peers, filter)
+		require.NoError(t, err)
+		require.Len(t, kcs, 1, "cluster behind the failing peer must be found (filter %q)", filter)
+		require.Equal(t, "homelab", kcs[0].name)
+		require.Equal(t, "v1.30.0", kcs[0].version)
+	}
+
+	ctx, cancel := context.WithCancel(t.Context())
+	cancel()
+	_, err := findKubernetesClusters(ctx, httpClient, lookupAddr, peers, "homelab")
+	require.ErrorIs(t, err, context.Canceled)
+
+	// Cancellation during the last peer's lookup must surface instead of an empty result.
+	ctx, cancel = context.WithCancel(t.Context())
+	cancelOnLookup := func(ctx context.Context, _ string) ([]string, error) {
+		cancel()
+		return nil, ctx.Err()
+	}
+	_, err = findKubernetesClusters(ctx, httpClient, cancelOnLookup, peers[:1], "")
+	require.ErrorIs(t, err, context.Canceled)
+}
+
+func TestFindKubernetesClustersReportsDeadResolver(t *testing.T) {
+	t.Parallel()
+
+	peers := []*proto.PeerState{{IP: "100.64.0.1"}, {IP: "100.64.0.2"}, {IP: "100.64.0.3"}}
+	timeoutErr := &net.DNSError{Err: "i/o timeout", Name: "1.0.64.100.in-addr.arpa.", IsTimeout: true}
+	notFoundErr := &net.DNSError{Err: "no such host", Name: "1.0.64.100.in-addr.arpa.", IsNotFound: true}
+
+	// A timeout before any peer resolved means the resolver is down: stop at the first
+	// peer instead of waiting out the timeout once per peer.
+	for _, filter := range []string{"", "homelab"} {
+		calls := 0
+		timeoutAll := func(context.Context, string) ([]string, error) {
+			calls++
+			return nil, timeoutErr
+		}
+		kcs, err := findKubernetesClusters(t.Context(), http.DefaultClient, timeoutAll, peers, filter)
+		require.ErrorIs(t, err, timeoutErr, "filter %q", filter)
+		assert.Empty(t, kcs)
+		assert.Equal(t, 1, calls, "scan must stop after the first timeout (filter %q)", filter)
+	}
+
+	// No PTR anywhere is not a resolver outage, so every peer is tried before the
+	// last lookup error is reported instead of an empty cluster list.
+	calls := 0
+	notFoundAll := func(context.Context, string) ([]string, error) {
+		calls++
+		return nil, notFoundErr
+	}
+	kcs, err := findKubernetesClusters(t.Context(), http.DefaultClient, notFoundAll, peers, "")
+	require.ErrorIs(t, err, notFoundErr)
+	assert.Empty(t, kcs)
+	assert.Equal(t, len(peers), calls, "every peer must be tried when lookups only miss")
+
+	// Once one peer resolved the resolver is known to work, so a later timeout is
+	// skipped like any other per-peer failure.
+	resolveThenTimeout := func(_ context.Context, addr string) ([]string, error) {
+		if addr == "100.64.0.1" {
+			return []string{"laptop.netbird.cloud."}, nil
+		}
+		return nil, timeoutErr
+	}
+	kcs, err = findKubernetesClusters(t.Context(), http.DefaultClient, resolveThenTimeout, peers, "")
+	require.NoError(t, err)
+	assert.Empty(t, kcs)
+
+	kcs, err = findKubernetesClusters(t.Context(), http.DefaultClient, notFoundAll, nil, "")
+	require.NoError(t, err, "an empty peer list is not a resolver failure")
+	assert.Empty(t, kcs)
+}
+
 func TestResolveKubeconfigPath(t *testing.T) {
 	home, err := os.UserHomeDir()
 	if err != nil {