Skip to content

va: log experiment results without base64 encoding#8707

Merged
jsha merged 4 commits intomainfrom
nolog-b64
Apr 9, 2026
Merged

va: log experiment results without base64 encoding#8707
jsha merged 4 commits intomainfrom
nolog-b64

Conversation

@jsha
Copy link
Copy Markdown
Contributor

@jsha jsha commented Apr 8, 2026

Go's JSON marshaling encodes []byte in base64. Previously, experiment results were logging *vapb.ValidationResult, which contained repeated corepb.ValidationRecord. In turn, corepb.ValidationRecord contains three []byte fields:

  repeated bytes addressesResolved = 3; // netip.Addr.MarshalText()
  bytes addressUsed = 4; // netip.Addr.MarshalText()
  repeated bytes addressesTried = 7; // netip.Addr.MarshalText()

By contrast, core.ValidationRecord (a non-protobuf struct), has fields of type netip.Addr:

	AddressesResolved []netip.Addr `json:"addressesResolved,omitempty"`
	AddressUsed       netip.Addr   `json:"addressUsed"`
	AddressesTried []netip.Addr `json:"addressesTried,omitempty"`

Those get serialized like 127.0.01, which is what we want.

This PR reworks the runExperiment function so that instead of receiving remoteResult (which could contain *vapb.IsCAAValidResponse or *vapb.ValidationResult), it receives a problem and an (optional) core.ValidationRecord. This allows us to more straightforwardly log things without running into base64-encoding problems.

To test this I modified TestExperimentalVAConcurrence to use HTTP-01 validation, since the affected log fields don't show up under DNS-01 validation. I also tweaked ipFakeDNS to allow customizing the IP address it returns.

Fixes #8701

@jsha jsha marked this pull request as ready for review April 8, 2026 22:31
@jsha jsha requested a review from a team as a code owner April 8, 2026 22:32
@jsha jsha requested a review from aarongable April 8, 2026 22:32
va/va.go Outdated
Comment on lines +344 to +345
b, _ := json.Marshal(logArgs)
fmt.Println("oh no", string(b))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove debugging print

Suggested change
b, _ := json.Marshal(logArgs)
fmt.Println("oh no", string(b))

va/va_test.go Outdated
name string
primaryDNS bdns.Client
experimentalDNS bdns.Client
domain string
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've deleted domain from all three test cases below.

va/va_test.go Outdated
Comment on lines +473 to +477
matchingLogs := mockLog.GetAllMatching(tc.expectLog)
if len(matchingLogs) != 1 {
t.Errorf("Expected log line matching %q, got: %s",
tc.expectLog, strings.Join(mockLog.GetAll(), "\n"))
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have shorthand for this: mocklog.ExpectMatch().

@jsha jsha merged commit dbe74e4 into main Apr 9, 2026
17 checks passed
@jsha jsha deleted the nolog-b64 branch April 9, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Experimental VA logs IPs in base64

2 participants