Skip to content

Commit ecf5a7f

Browse files
aarongableaarongable
committed
Update VA and bdns
1 parent 9f3392c commit ecf5a7f

File tree

15 files changed

+394
-382
lines changed

15 files changed

+394
-382
lines changed

‎bdns/dns.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"io"
9+
"log/slog"
910
"net"
1011
"net/http"
1112
"strconv"
@@ -17,7 +18,7 @@ import (
1718
"github.com/prometheus/client_golang/prometheus"
1819
"github.com/prometheus/client_golang/prometheus/promauto"
1920

20-
blog "github.com/letsencrypt/boulder/log"
21+
"github.com/letsencrypt/boulder/blog"
2122
"github.com/letsencrypt/boulder/metrics"
2223
)
2324

@@ -67,7 +68,6 @@ type impl struct {
6768
servers ServerProvider
6869
maxTries int
6970
clk clock.Clock
70-
log blog.Logger
7171

7272
queryTime *prometheus.HistogramVec
7373
totalLookupTime *prometheus.HistogramVec
@@ -86,7 +86,6 @@ func New(
8686
clk clock.Clock,
8787
maxTries int,
8888
userAgent string,
89-
log blog.Logger,
9089
tlsConfig *tls.Config,
9190
) Client {
9291
// Clone the default transport because it comes with various settings that we
@@ -143,7 +142,6 @@ func New(
143142
queryTime: queryTime,
144143
totalLookupTime: totalLookupTime,
145144
timeoutCounter: timeoutCounter,
146-
log: log,
147145
}
148146
}
149147

@@ -226,7 +224,12 @@ func (c *impl) exchangeOne(ctx context.Context, hostname string, qtype uint16) (
226224
}).Observe(rtt.Seconds())
227225

228226
if err != nil {
229-
c.log.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]", chosenServer, hostname, qtypeStr, err)
227+
blog.Info(ctx, "logDNSError",
228+
slog.String("chosenServer", chosenServer),
229+
slog.String("hostname", hostname),
230+
slog.String("qtype", qtypeStr),
231+
blog.ErrAttr(err),
232+
)
230233

231234
// Check if the error is a network timeout, rather than a local context
232235
// timeout. If it is, retry instead of giving up.

‎bdns/dns_test.go‎

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
"github.com/miekg/dns"
2323
"github.com/prometheus/client_golang/prometheus"
2424

25-
blog "github.com/letsencrypt/boulder/log"
25+
"github.com/letsencrypt/boulder/blog"
2626
"github.com/letsencrypt/boulder/metrics"
2727
"github.com/letsencrypt/boulder/test"
2828
)
@@ -283,21 +283,21 @@ func TestDNSNoServers(t *testing.T) {
283283
staticProvider, err := NewStaticProvider([]string{})
284284
test.AssertNotError(t, err, "Got error creating StaticProvider")
285285

286-
obj := New(time.Hour, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
286+
obj := New(time.Hour, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
287287

288-
_, resolver, err := obj.LookupA(context.Background(), "letsencrypt.org")
288+
_, resolver, err := obj.LookupA(blog.MockContext(t), "letsencrypt.org")
289289
test.AssertEquals(t, resolver, "")
290290
test.AssertError(t, err, "No servers")
291291

292-
_, resolver, err = obj.LookupAAAA(context.Background(), "letsencrypt.org")
292+
_, resolver, err = obj.LookupAAAA(blog.MockContext(t), "letsencrypt.org")
293293
test.AssertEquals(t, resolver, "")
294294
test.AssertError(t, err, "No servers")
295295

296-
_, resolver, err = obj.LookupTXT(context.Background(), "letsencrypt.org")
296+
_, resolver, err = obj.LookupTXT(blog.MockContext(t), "letsencrypt.org")
297297
test.AssertEquals(t, resolver, "")
298298
test.AssertError(t, err, "No servers")
299299

300-
_, resolver, err = obj.LookupCAA(context.Background(), "letsencrypt.org")
300+
_, resolver, err = obj.LookupCAA(blog.MockContext(t), "letsencrypt.org")
301301
test.AssertEquals(t, resolver, "")
302302
test.AssertError(t, err, "No servers")
303303
}
@@ -306,9 +306,9 @@ func TestDNSOneServer(t *testing.T) {
306306
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
307307
test.AssertNotError(t, err, "Got error creating StaticProvider")
308308

309-
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
309+
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
310310

311-
_, resolver, err := obj.LookupA(context.Background(), "letsencrypt.org")
311+
_, resolver, err := obj.LookupA(blog.MockContext(t), "letsencrypt.org")
312312
test.AssertNotError(t, err, "No message")
313313
test.AssertEquals(t, resolver, "127.0.0.1:4053")
314314
}
@@ -317,9 +317,9 @@ func TestDNSDuplicateServers(t *testing.T) {
317317
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr, dnsLoopbackAddr})
318318
test.AssertNotError(t, err, "Got error creating StaticProvider")
319319

320-
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
320+
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
321321

322-
_, resolver, err := obj.LookupA(context.Background(), "letsencrypt.org")
322+
_, resolver, err := obj.LookupA(blog.MockContext(t), "letsencrypt.org")
323323
test.AssertNotError(t, err, "No message")
324324
test.AssertEquals(t, resolver, "127.0.0.1:4053")
325325
}
@@ -328,32 +328,32 @@ func TestDNSServFail(t *testing.T) {
328328
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
329329
test.AssertNotError(t, err, "Got error creating StaticProvider")
330330

331-
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
331+
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
332332
bad := "servfail.com"
333333

334-
_, _, err = obj.LookupTXT(context.Background(), "servfail.com")
334+
_, _, err = obj.LookupTXT(blog.MockContext(t), "servfail.com")
335335
test.AssertError(t, err, "LookupTXT didn't return an error")
336336

337-
_, _, err = obj.LookupA(context.Background(), bad)
337+
_, _, err = obj.LookupA(blog.MockContext(t), bad)
338338
test.AssertError(t, err, "LookupA didn't return an error")
339339

340-
_, _, err = obj.LookupAAAA(context.Background(), bad)
340+
_, _, err = obj.LookupAAAA(blog.MockContext(t), bad)
341341
test.AssertError(t, err, "LookupAAAA didn't return an error")
342342

343-
_, _, err = obj.LookupCAA(context.Background(), bad)
343+
_, _, err = obj.LookupCAA(blog.MockContext(t), bad)
344344
test.AssertError(t, err, "LookupCAA didn't return an error")
345345
}
346346

347347
func TestDNSLookupTXT(t *testing.T) {
348348
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
349349
test.AssertNotError(t, err, "Got error creating StaticProvider")
350350

351-
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
351+
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
352352

353-
_, _, err = obj.LookupTXT(context.Background(), "letsencrypt.org")
353+
_, _, err = obj.LookupTXT(blog.MockContext(t), "letsencrypt.org")
354354
test.AssertNotError(t, err, "No message")
355355

356-
txt, _, err := obj.LookupTXT(context.Background(), "split-txt.letsencrypt.org")
356+
txt, _, err := obj.LookupTXT(blog.MockContext(t), "split-txt.letsencrypt.org")
357357
test.AssertNotError(t, err, "No message")
358358
test.AssertEquals(t, len(txt.Final), 1)
359359
test.AssertEquals(t, strings.Join(txt.Final[0].Txt, ""), "abc")
@@ -363,7 +363,7 @@ func TestDNSLookupA(t *testing.T) {
363363
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
364364
test.AssertNotError(t, err, "Got error creating StaticProvider")
365365

366-
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
366+
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
367367

368368
for _, tc := range []struct {
369369
name string
@@ -413,7 +413,7 @@ func TestDNSLookupA(t *testing.T) {
413413
},
414414
} {
415415
t.Run(tc.name, func(t *testing.T) {
416-
res, resolver, err := obj.LookupA(context.Background(), tc.hostname)
416+
res, resolver, err := obj.LookupA(blog.MockContext(t), tc.hostname)
417417

418418
wantResolver := "127.0.0.1:4053"
419419
if resolver != wantResolver {
@@ -448,7 +448,7 @@ func TestDNSLookupAAAA(t *testing.T) {
448448
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
449449
test.AssertNotError(t, err, "Got error creating StaticProvider")
450450

451-
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
451+
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
452452

453453
for _, tc := range []struct {
454454
name string
@@ -498,7 +498,7 @@ func TestDNSLookupAAAA(t *testing.T) {
498498
},
499499
} {
500500
t.Run(tc.name, func(t *testing.T) {
501-
res, resolver, err := obj.LookupAAAA(context.Background(), tc.hostname)
501+
res, resolver, err := obj.LookupAAAA(blog.MockContext(t), tc.hostname)
502502

503503
wantResolver := "127.0.0.1:4053"
504504
if resolver != wantResolver {
@@ -533,16 +533,16 @@ func TestDNSNXDOMAIN(t *testing.T) {
533533
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
534534
test.AssertNotError(t, err, "Got error creating StaticProvider")
535535

536-
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
536+
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
537537
hostname := "nxdomain.letsencrypt.org"
538538

539-
_, _, err = obj.LookupA(context.Background(), hostname)
539+
_, _, err = obj.LookupA(blog.MockContext(t), hostname)
540540
test.AssertContains(t, err.Error(), "NXDOMAIN looking up A for")
541541

542-
_, _, err = obj.LookupAAAA(context.Background(), hostname)
542+
_, _, err = obj.LookupAAAA(blog.MockContext(t), hostname)
543543
test.AssertContains(t, err.Error(), "NXDOMAIN looking up AAAA for")
544544

545-
_, _, err = obj.LookupTXT(context.Background(), hostname)
545+
_, _, err = obj.LookupTXT(blog.MockContext(t), hostname)
546546
expected := Error{dns.TypeTXT, hostname, nil, dns.RcodeNameError, nil}
547547
test.AssertDeepEquals(t, err, expected)
548548
}
@@ -551,10 +551,10 @@ func TestDNSLookupCAA(t *testing.T) {
551551
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
552552
test.AssertNotError(t, err, "Got error creating StaticProvider")
553553

554-
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
554+
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
555555
removeIDExp := regexp.MustCompile(" id: [[:digit:]]+")
556556

557-
caas, resolver, err := obj.LookupCAA(context.Background(), "bracewel.net")
557+
caas, resolver, err := obj.LookupCAA(blog.MockContext(t), "bracewel.net")
558558
test.AssertNotError(t, err, "CAA lookup failed")
559559
test.Assert(t, len(caas.Final) > 0, "Should have CAA records")
560560
test.AssertEquals(t, resolver, "127.0.0.1:4053")
@@ -569,17 +569,17 @@ bracewel.net. 0 IN CAA 1 issue "letsencrypt.org"
569569
`
570570
test.AssertEquals(t, removeIDExp.ReplaceAllString(caas.String(), " id: XXXX"), expectedResp)
571571

572-
caas, resolver, err = obj.LookupCAA(context.Background(), "nonexistent.letsencrypt.org")
572+
caas, resolver, err = obj.LookupCAA(blog.MockContext(t), "nonexistent.letsencrypt.org")
573573
test.AssertNotError(t, err, "CAA lookup failed")
574574
test.Assert(t, len(caas.Final) == 0, "Shouldn't have CAA records")
575575
test.AssertEquals(t, resolver, "127.0.0.1:4053")
576576

577-
caas, resolver, err = obj.LookupCAA(context.Background(), "nxdomain.letsencrypt.org")
577+
caas, resolver, err = obj.LookupCAA(blog.MockContext(t), "nxdomain.letsencrypt.org")
578578
test.AssertNotError(t, err, "CAA lookup failed")
579579
test.Assert(t, len(caas.Final) == 0, "Shouldn't have CAA records")
580580
test.AssertEquals(t, resolver, "127.0.0.1:4053")
581581

582-
caas, resolver, err = obj.LookupCAA(context.Background(), "cname.example.com")
582+
caas, resolver, err = obj.LookupCAA(blog.MockContext(t), "cname.example.com")
583583
test.AssertNotError(t, err, "CAA lookup failed")
584584
test.Assert(t, len(caas.Final) > 0, "Should follow CNAME to find CAA")
585585
test.AssertEquals(t, resolver, "127.0.0.1:4053")
@@ -594,7 +594,7 @@ caa.example.com. 0 IN CAA 1 issue "letsencrypt.org"
594594
`
595595
test.AssertEquals(t, removeIDExp.ReplaceAllString(caas.String(), " id: XXXX"), expectedResp)
596596

597-
_, resolver, err = obj.LookupCAA(context.Background(), "gonetld")
597+
_, resolver, err = obj.LookupCAA(blog.MockContext(t), "gonetld")
598598
test.AssertError(t, err, "should fail for TLD NXDOMAIN")
599599
test.AssertContains(t, err.Error(), "NXDOMAIN")
600600
test.AssertEquals(t, resolver, "127.0.0.1:4053")
@@ -759,10 +759,10 @@ func TestRetry(t *testing.T) {
759759
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
760760
test.AssertNotError(t, err, "Got error creating StaticProvider")
761761

762-
testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), tc.maxTries, "", blog.UseMock(), tlsConfig)
762+
testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), tc.maxTries, "", tlsConfig)
763763
dr := testClient.(*impl)
764764
dr.exchanger = tc.te
765-
_, _, err = dr.LookupTXT(context.Background(), "example.com")
765+
_, _, err = dr.LookupTXT(blog.MockContext(t), "example.com")
766766
if err == errTooManyRequests {
767767
t.Errorf("#%d, sent more requests than the test case handles", i)
768768
}
@@ -796,10 +796,10 @@ func TestRetryMetrics(t *testing.T) {
796796
// context itself being cancelled. It should never see the error in the
797797
// testExchanger, because the fake exchanger (like the real http package)
798798
// checks for cancellation before doing any work.
799-
testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig)
799+
testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", tlsConfig)
800800
dr := testClient.(*impl)
801801
dr.exchanger = &testExchanger{errs: []error{errors.New("oops")}}
802-
ctx, cancel := context.WithCancel(t.Context())
802+
ctx, cancel := context.WithCancel(blog.MockContext(t))
803803
cancel()
804804
_, _, err = dr.LookupTXT(ctx, "example.com")
805805
if err == nil ||
@@ -815,10 +815,10 @@ func TestRetryMetrics(t *testing.T) {
815815

816816
// Same as above, except rather than cancelling the context ourselves, we
817817
// let the go runtime cancel it as a result of a deadline in the past.
818-
testClient = New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig)
818+
testClient = New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", tlsConfig)
819819
dr = testClient.(*impl)
820820
dr.exchanger = &testExchanger{errs: []error{errors.New("oops")}}
821-
ctx, cancel = context.WithTimeout(t.Context(), -10*time.Hour)
821+
ctx, cancel = context.WithTimeout(blog.MockContext(t), -10*time.Hour)
822822
defer cancel()
823823
_, _, err = dr.LookupTXT(ctx, "example.com")
824824
if err == nil ||
@@ -883,7 +883,7 @@ func TestRotateServerOnErr(t *testing.T) {
883883
test.AssertNotError(t, err, "Got error creating StaticProvider")
884884

885885
maxTries := 5
886-
client := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), maxTries, "", blog.UseMock(), tlsConfig)
886+
client := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), maxTries, "", tlsConfig)
887887

888888
// Configure a mock exchanger that will always return a retryable error for
889889
// servers A and B. This will force server "[2606:4700:4700::1111]:53" to do
@@ -903,7 +903,7 @@ func TestRotateServerOnErr(t *testing.T) {
903903
// servers *all* queries should eventually succeed by being retried against
904904
// server "[2606:4700:4700::1111]:53".
905905
for range maxTries * 2 {
906-
_, resolver, err := client.LookupTXT(context.Background(), "example.com")
906+
_, resolver, err := client.LookupTXT(blog.MockContext(t), "example.com")
907907
test.AssertEquals(t, resolver, "[2606:4700:4700::1111]:53")
908908
// Any errors are unexpected - server "[2606:4700:4700::1111]:53" should
909909
// have responded without error.
@@ -948,15 +948,15 @@ func TestDOHMetric(t *testing.T) {
948948
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
949949
test.AssertNotError(t, err, "Got error creating StaticProvider")
950950

951-
testClient := New(time.Second*11, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 0, "", blog.UseMock(), tlsConfig)
951+
testClient := New(time.Second*11, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 0, "", tlsConfig)
952952
resolver := testClient.(*impl)
953953
resolver.exchanger = &dohAlwaysRetryExchanger{err: &url.Error{Op: "read", Err: testTimeoutError(true)}}
954954

955955
// Starting out, we should count 0 "out of retries" errors.
956956
test.AssertMetricWithLabelsEquals(t, resolver.timeoutCounter, prometheus.Labels{"qtype": "None", "type": "out of retries", "resolver": "127.0.0.1", "isTLD": "false"}, 0)
957957

958958
// Trigger the error.
959-
_, _, _ = resolver.exchangeOne(context.Background(), "example.com", 0)
959+
_, _, _ = resolver.exchangeOne(blog.MockContext(t), "example.com", 0)
960960

961961
// Now, we should count 1 "out of retries" errors.
962962
test.AssertMetricWithLabelsEquals(t, resolver.timeoutCounter, prometheus.Labels{"qtype": "None", "type": "out of retries", "resolver": "127.0.0.1", "isTLD": "false"}, 1)

‎cmd/boulder-va/main.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/jmhodges/clock"
1010

1111
"github.com/letsencrypt/boulder/bdns"
12+
"github.com/letsencrypt/boulder/blog"
1213
"github.com/letsencrypt/boulder/cmd"
1314
"github.com/letsencrypt/boulder/config"
1415
"github.com/letsencrypt/boulder/features"
@@ -60,7 +61,7 @@ type Config struct {
6061
Features features.Config
6162
}
6263

63-
Syslog cmd.SyslogConfig
64+
Syslog blog.Config
6465
OpenTelemetry cmd.OpenTelemetryConfig
6566
}
6667

@@ -107,7 +108,6 @@ func main() {
107108
clk,
108109
c.VA.DNSTries,
109110
c.VA.UserAgent,
110-
logger,
111111
tlsConfig)
112112

113113
var remotes []va.RemoteVA
@@ -137,7 +137,6 @@ func main() {
137137
c.VA.IssuerDomain,
138138
scope,
139139
clk,
140-
logger,
141140
c.VA.AccountURIPrefixes,
142141
va.PrimaryPerspective,
143142
"",
@@ -147,9 +146,9 @@ func main() {
147146
)
148147
cmd.FailOnError(err, "Unable to create VA server")
149148

150-
start, err := bgrpc.NewServer(c.VA.GRPC, logger).Add(
149+
start, err := bgrpc.NewServer(c.VA.GRPC).Add(
151150
&vapb.VA_ServiceDesc, vai).Add(
152-
&vapb.CAA_ServiceDesc, vai).Build(tlsConfig, scope, clk)
151+
&vapb.CAA_ServiceDesc, vai).Build(tlsConfig, logger, scope, clk)
153152
cmd.FailOnError(err, "Unable to setup VA gRPC server")
154153
cmd.FailOnError(start(), "VA gRPC service failed")
155154
}

0 commit comments

Comments
 (0)