From 8864112a0cf8434b67e993e9252bed7ebaae786e Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Fri, 5 May 2023 12:19:25 -0700
Subject: [PATCH] ipn/ipnlocal: bound how long cert fetchher checks for
 existing DNS records

It was supposed to be best effort but in some cases (macsys at least,
per @marwan-at-work) it hangs and exhausts the whole context.Context
deadline so we fail to make the SetDNS call to the server.

Updates #8067
Updates #3273 etc

Change-Id: Ie1f04abe9689951484748aecdeae312afbafdb0f
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 ipn/ipnlocal/cert.go | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/ipn/ipnlocal/cert.go b/ipn/ipnlocal/cert.go
index cabcae1fb..ef19a6571 100644
--- a/ipn/ipnlocal/cert.go
+++ b/ipn/ipnlocal/cert.go
@@ -31,6 +31,7 @@ import (
 	"time"
 
 	"golang.org/x/crypto/acme"
+	"golang.org/x/exp/slices"
 	"tailscale.com/atomicfile"
 	"tailscale.com/envknob"
 	"tailscale.com/hostinfo"
@@ -361,17 +362,16 @@ func (b *LocalBackend) getCertPEM(ctx context.Context, cs certStore, logf logger
 				}
 				key := "_acme-challenge." + domain
 
+				// Do a best-effort lookup to see if we've already created this DNS name
+				// in a previous attempt. Don't burn too much time on it, though. Worst
+				// case we ask the server to create something that already exists.
 				var resolver net.Resolver
-				var ok bool
-				txts, _ := resolver.LookupTXT(ctx, key)
-				for _, txt := range txts {
-					if txt == rec {
-						ok = true
-						logf("TXT record already existed")
-						break
-					}
-				}
-				if !ok {
+				lookupCtx, lookupCancel := context.WithTimeout(ctx, 500*time.Millisecond)
+				txts, _ := resolver.LookupTXT(lookupCtx, key)
+				lookupCancel()
+				if slices.Contains(txts, rec) {
+					logf("TXT record already existed")
+				} else {
 					logf("starting SetDNS call...")
 					err = b.SetDNS(ctx, key, rec)
 					if err != nil {