VA: Make PerformValidation more like DoDCV

[beautifulentropy]

• Remove Perspective and RIR from ValidationRecords
• Make ValidationResultToPB Perspective and RIR aware
• Update comment for VA.PerformValidation
• Make verificationRequestEvent more like doDCVAuditLog
• Update language used in problems created by performRemoteValidation to be more like doRemoteDCV.

[aarongable]

Just noting that removing the channel as an argument to this anonymous function is not a necessary change.

I added the channel as an argument here in #7522 upon Jacob's reminder that "explicit is better than implicit" -- it's better for the anonymous function to be explicit about what objects it is using than to implicitly close over everything.

It's a fully arbitrary line -- obviously I added the channel as an argument but didn't bother to add ctx as an argument too. I think there are perfectly good arguments to be made that this anonymous function should take no arguments, just the rva (effectively the loop variable), or take everything as arguments. Just noting that this is a personal preference change for which we don't have an established team-wide best practice.

[jsha]

Also worth noting I recently added a subCtx here (which is captured in the same way as ctx is) and considered adding it to the explicit argument list but wound up not doing so, since I thought folks might not like it stylistically. So there's some local pattern matching possible for "does this anonymous function need to receive all things as parameters? or is taking some of them as captures okay?"

[jsha]

Oh and I remembered the specific reason I backed out that change! I was going to pass subCtx as a parameter, but call that parameter ctx locally. So inside the function we would have ctx shadowed and have no chance of using the original ctx by accident. But I decided that was too clever by half and backed it out.

[beautifulentropy]

Ah, interesting! Personally, I prefer closing over everything. Initially, I wasn’t planning to backport this change and was instead going to update the MPIC code. However, I noticed we’re doing the same thing in this function’s offshoot in va/caa.go, so I figured this was a consistency win:

boulder/va/caa.go

Lines 239 to 273 in a8cdaf8

	func (va *ValidationAuthorityImpl) performRemoteCAACheck(
	ctx context.Context,
	req *vapb.IsCAAValidRequest,
	results chan<- *remoteVAResult) {
	for _, i := range rand.Perm(len(va.remoteVAs)) {
	remoteVA := va.remoteVAs[i]
	go func(rva RemoteVA) {
	result := &remoteVAResult{
	VAHostname: rva.Address,
	}
	res, err := rva.IsCAAValid(ctx, req)
	if err != nil {
	if canceled.Is(err) {
	// Handle the cancellation error.
	result.Problem = probs.ServerInternal("Remote VA IsCAAValid RPC cancelled")
	} else {
	// Handle validation error.
	va.log.Errf("Remote VA %q.IsCAAValid failed: %s", rva.Address, err)
	result.Problem = probs.ServerInternal("Remote VA IsCAAValid RPC failed")
	}
	} else if res.Problem != nil {
	prob, err := bgrpc.PBToProblemDetails(res.Problem)
	if err != nil {
	va.log.Infof("Remote VA %q.IsCAAValid returned malformed problem: %s", rva.Address, err)
	result.Problem = probs.ServerInternal(
	fmt.Sprintf("Remote VA IsCAAValid RPC returned malformed result: %s", err))
	} else {
	va.log.Infof("Remote VA %q.IsCAAValid returned problem: %s", rva.Address, prob)
	result.Problem = prob
	}
	}
	results <- result
	}(remoteVA)
	}
	}