I read through all four changed files and traced the full path from networkBootOutput.routerAddress through to the generated buildkitd.toml. This is a tight, focused fix with a clear problem statement.
The problem is real and the fix is correct. BuildKit strips loopback nameservers (the whole 127.0.0.0/8 range) and falls back to 8.8.8.8 when constructing resolv.conf for build steps. On hosts using systemd-resolved's 127.0.0.53 stub this silently breaks builds behind internal or egress-filtered resolvers. Pointing the [dns] section at miren's bridge router address — a non-loopback address reachable from the host netns — routes around this correctly.
Implementation details I checked:
network.routerAddress.IsValid() guards the zero-value case before constructing the nameserver list. If the network boot hasn't set a router address for any reason, dnsNameservers stays nil and generateConfig emits no [dns] section, leaving buildkit's default behavior intact. Safe fallback.fmt.Sprintf("%q", ns), which for netip.Addr.String() output (a well-formed IP address) produces valid TOML string literals. No injection risk since the value comes from a typed netip.Addr, not user input.%[4]s slot replaces the blank line between [log] and [grpc] in the template. When dnsSection is non-empty it starts with \n and ends with \n, so the section boundary stays clean. When it's empty the blank line disappears — cosmetically different but perfectly valid TOML.network.output flows into newBuildkitBoot → startEmbedded, parallel to how registryHostMapping was already wired.Tests: The two new test cases cover what matters — single and multiple nameservers produce the right TOML, and no [dns] section appears when the slice is nil. The existing "no hardcoded DNS" test was correctly reframed to check absence of [dns] rather than nameservers=.
One small cosmetic note: the generateConfig signature now takes four arguments where the first three were positional and the fourth is the DNS slice. The %[1]d/%[2]d/%[3]s/%[4]s explicit indexing in the format string makes the argument mapping clear and immune to accidental reordering. Good call.
Nothing blocking here — this is ready to merge.
Verdict: ready
{
"owner": "mirendev",
"repo": "runtime",
"number": 1103,
"verdict": "ready",
"event": "approve",
"summary": "I read through all four changed files and traced the full path from `networkBootOutput.routerAddress` through to the generated `buildkitd.toml`. This is a tight, focused fix with a clear problem statement.\n\n**The problem is real and the fix is correct.** BuildKit strips loopback nameservers (the whole `127.0.0.0/8` range) and falls back to `8.8.8.8` when constructing resolv.conf for build steps. On hosts using systemd-resolved's `127.0.0.53` stub this silently breaks builds behind internal or egress-filtered resolvers. Pointing the `[dns]` section at miren's bridge router address — a non-loopback address reachable from the host netns — routes around this correctly.\n\n**Implementation details I checked:**\n\n- `network.routerAddress.IsValid()` guards the zero-value case before constructing the nameserver list. If the network boot hasn't set a router address for any reason, `dnsNameservers` stays `nil` and `generateConfig` emits no `[dns]` section, leaving buildkit's default behavior intact. Safe fallback.\n- Nameserver quoting uses `fmt.Sprintf(\"%q\", ns)`, which for `netip.Addr.String()` output (a well-formed IP address) produces valid TOML string literals. No injection risk since the value comes from a typed `netip.Addr`, not user input.\n- The `%[4]s` slot replaces the blank line between `[log]` and `[grpc]` in the template. When `dnsSection` is non-empty it starts with `\\n` and ends with `\\n`, so the section boundary stays clean. When it's empty the blank line disappears — cosmetically different but perfectly valid TOML.\n- The boot graph wiring is clean: `network.output` flows into `newBuildkitBoot` → `startEmbedded`, parallel to how `registryHostMapping` was already wired.\n\n**Tests:** The two new test cases cover what matters — single and multiple nameservers produce the right TOML, and no `[dns]` section appears when the slice is `nil`. The existing \"no hardcoded DNS\" test was correctly reframed to check absence of `[dns]` rather than `nameservers=`.\n\nOne small cosmetic note: the `generateConfig` signature now takes four arguments where the first three were positional and the fourth is the DNS slice. The `%[1]d/%[2]d/%[3]s/%[4]s` explicit indexing in the format string makes the argument mapping clear and immune to accidental reordering. Good call.\n\nNothing blocking here — this is ready to merge.",
"posted_to_pr": true
}