Skip to content

Commit fe23dab

Browse files
Daniel McCarneyjsha
authored andcommitted
va: add challenge type to remote VA differentials. (letsencrypt#4410)
This will make data analysis of the differentials easier. Along the way I also added a unit test for `logRemoteValidationDifferentials`.
1 parent 4a6e34f commit fe23dab

File tree

2 files changed

+69
-1
lines changed

2 files changed

+69
-1
lines changed

va/va.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ func (va *ValidationAuthorityImpl) processRemoteResults(
476476
// If we are using `features.MultiVAFullResults` then we haven't returned
477477
// early and can now log the differential between what the primary VA saw and
478478
// what all of the remote VAs saw.
479-
va.logRemoteValidationDifferentials(domain, primaryResult, remoteProbs)
479+
va.logRemoteValidationDifferentials(domain, challengeType, primaryResult, remoteProbs)
480480

481481
// Based on the threshold of good/bad return nil or a problem.
482482
if good >= required {
@@ -496,6 +496,7 @@ func (va *ValidationAuthorityImpl) processRemoteResults(
496496
// that contains the primary VA result and the results each remote VA returned.
497497
func (va *ValidationAuthorityImpl) logRemoteValidationDifferentials(
498498
domain string,
499+
challengeType string,
499500
primaryResult *probs.ProblemDetails,
500501
remoteProbs []*probs.ProblemDetails) {
501502

@@ -528,11 +529,13 @@ func (va *ValidationAuthorityImpl) logRemoteValidationDifferentials(
528529

529530
logOb := struct {
530531
Domain string
532+
ChallengeType string
531533
PrimaryResult *probs.ProblemDetails
532534
RemoteSuccesses int
533535
RemoteFailures []*probs.ProblemDetails
534536
}{
535537
Domain: domain,
538+
ChallengeType: challengeType,
536539
PrimaryResult: primaryResult,
537540
RemoteSuccesses: len(successes),
538541
RemoteFailures: failures,

va/va_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,3 +611,68 @@ func TestDetailedError(t *testing.T) {
611611
}
612612
}
613613
}
614+
615+
func TestLogRemoteValidationDifferentials(t *testing.T) {
616+
// Create some remote VAs
617+
remoteVA1, _ := setup(nil, 0, "remote 1", nil)
618+
remoteVA2, _ := setup(nil, 0, "remote 2", nil)
619+
remoteVA3, _ := setup(nil, 0, "remote 3", nil)
620+
remoteVAs := []RemoteVA{
621+
{remoteVA1, "remote 1"},
622+
{remoteVA2, "remote 2"},
623+
{remoteVA3, "remote 3"},
624+
}
625+
626+
// Set up a local VA that allows a max of 2 remote failures.
627+
localVA, mockLog := setup(nil, 2, "local 1", remoteVAs)
628+
629+
egProbA := probs.DNS("root DNS servers closed at 4:30pm")
630+
egProbB := probs.OrderNotReady("please take a number")
631+
632+
testCases := []struct {
633+
name string
634+
primaryResult *probs.ProblemDetails
635+
remoteProbs []*probs.ProblemDetails
636+
expectedLog string
637+
}{
638+
{
639+
name: "remote and primary results equal (all nil)",
640+
primaryResult: nil,
641+
remoteProbs: nil,
642+
},
643+
{
644+
name: "remote and primary results equal (not nil)",
645+
primaryResult: egProbA,
646+
remoteProbs: []*probs.ProblemDetails{egProbA, egProbA, egProbA},
647+
},
648+
{
649+
name: "remote and primary differ (primary nil)",
650+
primaryResult: nil,
651+
remoteProbs: []*probs.ProblemDetails{egProbA, nil, egProbB},
652+
expectedLog: `INFO: remoteVADifferentials JSON={"Domain":"example.com","ChallengeType":"blorpus-01","PrimaryResult":null,"RemoteSuccesses":1,"RemoteFailures":[{"type":"dns","detail":"root DNS servers closed at 4:30pm","status":400},{"type":"orderNotReady","detail":"please take a number","status":403}]}`,
653+
},
654+
{
655+
name: "remote and primary differ (primary not nil)",
656+
primaryResult: egProbA,
657+
remoteProbs: []*probs.ProblemDetails{nil, egProbB, nil},
658+
expectedLog: `INFO: remoteVADifferentials JSON={"Domain":"example.com","ChallengeType":"blorpus-01","PrimaryResult":{"type":"dns","detail":"root DNS servers closed at 4:30pm","status":400},"RemoteSuccesses":2,"RemoteFailures":[{"type":"orderNotReady","detail":"please take a number","status":403}]}`,
659+
},
660+
}
661+
662+
for _, tc := range testCases {
663+
t.Run(tc.name, func(t *testing.T) {
664+
mockLog.Clear()
665+
666+
localVA.logRemoteValidationDifferentials(
667+
"example.com", "blorpus-01", tc.primaryResult, tc.remoteProbs)
668+
669+
lines := mockLog.GetAllMatching("remoteVADifferentials JSON=.*")
670+
if tc.expectedLog != "" {
671+
test.AssertEquals(t, len(lines), 1)
672+
test.AssertEquals(t, lines[0], tc.expectedLog)
673+
} else {
674+
test.AssertEquals(t, len(lines), 0)
675+
}
676+
})
677+
}
678+
}

0 commit comments

Comments
 (0)