diff --git a/client/cmd/kubernetes.go b/client/cmd/kubernetes.go index cc91477c6..c04d8385e 100644 --- a/client/cmd/kubernetes.go +++ b/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 != nil { + if err := ctx.Err(); 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, ".") - if len(comps) < 2 { + kc, ok := probeCluster(ctx, httpClient, fqdn, nameFilter) + 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 { diff --git a/client/cmd/kubernetes_test.go b/client/cmd/kubernetes_test.go index c40d20996..c547c08c8 100644 --- a/client/cmd/kubernetes_test.go +++ b/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 {