Skip to content

Commit

Permalink
Improve LDAP desktop discovery (#30383)
Browse files Browse the repository at this point in the history
In environments with many desktops where some DNS queries time out,
we can run in to situations where the discovery loop doesn't complete
before the desktop's 10m TTL expires, causing desktops to disappear
and reappear sporadically.

- Increase the TTL for LDAP-discovered desktops to 30m. This won't
  harm UX, since desktops that stop being discovered are already
  purged prior to their expiration.
- Perform both DNS queries (via the default resolver and by hitting
  the LDAP server directly) in parallel. This reduces the maximum
  time a single lookup can take from 20s to 5s.
  • Loading branch information
zmb3 committed Aug 14, 2023
1 parent 2f5c62c commit a801029
Showing 1 changed file with 54 additions and 13 deletions.
67 changes: 54 additions & 13 deletions lib/srv/desktop/discovery.go
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"net"
"net/netip"
"strings"
"time"

Expand Down Expand Up @@ -183,21 +184,58 @@ func (s *WindowsService) applyLabelsFromLDAP(entry *ldap.Entry, labels map[strin

// lookupDesktop does a DNS lookup for the provided hostname.
// It checks using the default system resolver first, and falls
// back to making a DNS query of the configured LDAP server
// if the system resolver fails.
func (s *WindowsService) lookupDesktop(ctx context.Context, hostname string) (addrs []string, err error) {
tctx, cancel := context.WithTimeout(ctx, 10*time.Second)
defer cancel()
// back to the configured LDAP server if the system resolver fails.
func (s *WindowsService) lookupDesktop(ctx context.Context, hostname string) ([]string, error) {
stringAddrs := func(addrs []netip.Addr) []string {
result := make([]string, 0, len(addrs))
for _, addr := range addrs {
result = append(result, addr.String())
}
return result
}

const queryTimeout = 5 * time.Second

addrs, err = net.DefaultResolver.LookupHost(tctx, hostname)
if err == nil && len(addrs) > 0 {
return addrs, nil
queryResolver := func(resolver *net.Resolver, resolverName string) chan []netip.Addr {
ch := make(chan []netip.Addr, 1)
if resolver != nil {
go func() {
tctx, cancel := context.WithTimeout(ctx, queryTimeout)
defer cancel()

addrs, err := resolver.LookupNetIP(tctx, "ip4", hostname)
if err != nil {
s.cfg.Log.Debugf("DNS lookup for %v failed with %s resolver: %v",
hostname, resolverName, err)
}
if len(addrs) > 0 {
ch <- addrs
}
}()
}
return ch
}
if s.dnsResolver == nil {
return nil, trace.NewAggregate(err, trace.Errorf("DNS lookup for %q failed and there's no LDAP server to fallback to", hostname))

// kick off both DNS queries in parallel
defaultResult := queryResolver(net.DefaultResolver, "default")
ldapResult := queryResolver(s.dnsResolver, "LDAP")

// wait 5 seconds for the default resolver to return
select {
case addrs := <-defaultResult:
return stringAddrs(addrs), nil
case <-s.cfg.Clock.After(5 * time.Second):
}

// If we didn't get a result from the default resolver,
// the result from the LDAP resolver is either available
// now or we're done. There's no more waiting.
select {
case addrs := <-ldapResult:
return stringAddrs(addrs), nil
default:
return nil, trace.Errorf("could not resolve %v in time", hostname)
}
s.cfg.Log.WithError(err).Debugf("DNS lookup for %q failed, falling back to LDAP server", hostname)
return s.dnsResolver.LookupHost(ctx, hostname)
}

// ldapEntryToWindowsDesktop generates the Windows Desktop resource
Expand Down Expand Up @@ -237,6 +275,9 @@ func (s *WindowsService) ldapEntryToWindowsDesktop(ctx context.Context, entry *l
return nil, trace.Wrap(err)
}

desktop.SetExpiry(s.cfg.Clock.Now().UTC().Add(apidefaults.ServerAnnounceTTL))
// We use a longer TTL for discovered desktops, because the reconciler will manually
// purge them if they stop being detected, and discovery of large Windows fleets can
// take a long time.
desktop.SetExpiry(s.cfg.Clock.Now().UTC().Add(apidefaults.ServerAnnounceTTL * 3))
return desktop, nil
}

0 comments on commit a801029

Please sign in to comment.