Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions components/buildkit/buildkit.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ type Config struct {

// RegistryHost is the hostname for the cluster-local registry (e.g., cluster.local:5000)
RegistryHost string

// DNSServer is the Miren interface DNS server used by build containers.
DNSServer string
}

// Component manages a persistent BuildKit daemon as a containerd container,
Expand Down Expand Up @@ -145,7 +148,7 @@ func (c *Component) Start(ctx context.Context, config Config) error {
}

// Generate buildkitd.toml config
configContent := c.generateConfig(gcKeepStorage, gcKeepDuration, config.RegistryHost)
configContent := c.generateConfig(gcKeepStorage, gcKeepDuration, config.RegistryHost, config.DNSServer)
configPath := filepath.Join(dataPath, "buildkitd.toml")
if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil {
return fmt.Errorf("failed to write buildkit config: %w", err)
Expand Down Expand Up @@ -310,7 +313,7 @@ func (c *Component) Client(ctx context.Context) (*buildkitclient.Client, error)
// least-recently-used first. This is what breaks the pinning above, since
// removing a fresh leaf frees the ancestors under it.
// 4. Last resort: sweep internal and frontend cache too, to hold the line.
func (c *Component) generateConfig(gcKeepStorage, gcKeepDuration int64, registryHost string) string {
func (c *Component) generateConfig(gcKeepStorage, gcKeepDuration int64, registryHost, dnsServer string) string {
if registryHost == "" {
registryHost = "cluster.local:5000"
}
Expand All @@ -323,6 +326,9 @@ insecure-entitlements = [ "network.host", "security.insecure" ]
[log]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There's no guard for an empty dnsServer here, unlike the registryHost fallback just above. If dnsServer == "" (e.g. from a zero netip.Addr), the template emits nameservers = [ "" ], which is a semantically invalid entry. Consider wrapping the [dns] block in a conditional so it's only emitted when a real address is available — matching the same defensive pattern used for registryHost.

🤖 Prompt for AI Agents
In components/buildkit/buildkit.go, the
generateConfig function emits a [dns] nameservers
block unconditionally using the dnsServer
parameter (line ~326-330 of the PR head). If
dnsServer is an empty string, the output is
`nameservers = [ "" ]`, which is semantically
invalid for buildkitd. Add a guard so the [dns]
block is only emitted when dnsServer is non-empty
— similar to the existing guard for registryHost.
In the fmt.Sprintf template, replace the
unconditional [dns] block with a conditional
string built before the Sprintf call (e.g. compute
a dnsBlock string that is either the full [dns]\n
nameservers = [ \"%s\" ]\n\n or empty, then inject
it as an additional format argument).

format = "text"

[dns]
nameservers = [ "%[4]s" ]

[grpc]
address = [ "unix:///run/buildkit/buildkitd.sock" ]
uid = 0
Expand Down Expand Up @@ -360,7 +366,7 @@ insecure-entitlements = [ "network.host", "security.insecure" ]
[registry."%[3]s"]
insecure = true
http = true
`, gcKeepStorage, gcKeepDuration, registryHost)
`, gcKeepStorage, gcKeepDuration, registryHost, dnsServer)
}

// writeHostsFile creates or updates the custom /etc/hosts file for the BuildKit container.
Expand Down
12 changes: 5 additions & 7 deletions components/buildkit/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

func TestGenerateConfig(t *testing.T) {
c := &Component{}
config := c.generateConfig(10*1024*1024*1024, 86400, "registry.example.com:5000")
config := c.generateConfig(10*1024*1024*1024, 86400, "registry.example.com:5000", "10.8.0.1")

// gcPolicyBlocks splits the config into its individual gcpolicy bodies
// (text from one "[[worker.oci.gcpolicy]]" header to the next section).
Expand Down Expand Up @@ -83,16 +83,14 @@ func TestGenerateConfig(t *testing.T) {

t.Run("uses default registry host when empty", func(t *testing.T) {
r := require.New(t)
defaultConfig := c.generateConfig(10*1024*1024*1024, 86400, "")
defaultConfig := c.generateConfig(10*1024*1024*1024, 86400, "", "10.8.0.1")
r.Contains(defaultConfig, `[registry."cluster.local:5000"]`)
})

t.Run("does not hardcode DNS nameservers", func(t *testing.T) {
t.Run("uses the configured Miren interface DNS server", func(t *testing.T) {
r := require.New(t)
// MIR-1643: DNS is delegated to buildkitd, which derives each build's
// resolv.conf from the host. A hardcoded nameserver directive would
// override that and break builds on hosts with an internal resolver.
r.NotContains(config, "nameservers=")
r.Contains(config, "[dns]")
r.Contains(config, `nameservers = [ "10.8.0.1" ]`)
r.NotContains(config, "1.1.1.1")
r.NotContains(config, "8.8.8.8")
})
Expand Down
7 changes: 4 additions & 3 deletions components/server/boot_buildkit.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ func buildkitInputs(options StartOptions) buildkitBootInputs {
return buildkitBootInputs{config: options.Config.Buildkit, dataPath: options.Config.Server.GetDataPath()}
}

func newBuildkitBoot(inputs buildkitBootInputs, containerd boot.Output[containerdBootOutput], registryHostMapping boot.Output[registryHostMappingBootOutput], observability boot.Output[observabilityBootOutput]) *buildkitBoot {
func newBuildkitBoot(inputs buildkitBootInputs, containerd boot.Output[containerdBootOutput], network boot.Output[networkBootOutput], registryHostMapping boot.Output[registryHostMappingBootOutput], observability boot.Output[observabilityBootOutput]) *buildkitBoot {
b := &buildkitBoot{inputs: inputs}
stop := boot.WithStop(b.stop, componentStopTimeout)
switch {
case inputs.config.GetStartEmbedded():
b.component, b.output = boot.Provide3("buildkit", containerd, registryHostMapping, observability, b.startEmbedded, stop)
b.component, b.output = boot.Provide4("buildkit", containerd, network, registryHostMapping, observability, b.startEmbedded, stop)
case inputs.config.GetSocketPath() != "":
b.component, b.output = boot.Provide1("buildkit", observability, b.start, stop)
default:
Expand All @@ -64,7 +64,7 @@ func (b *buildkitBoot) startDisabled(context.Context) (buildkitBootOutput, error
return buildkitBootOutput{}, nil
}

func (b *buildkitBoot) startEmbedded(ctx context.Context, containerd containerdBootOutput, hostMapping registryHostMappingBootOutput, observability observabilityBootOutput) (buildkitBootOutput, error) {
func (b *buildkitBoot) startEmbedded(ctx context.Context, containerd containerdBootOutput, network networkBootOutput, hostMapping registryHostMappingBootOutput, observability observabilityBootOutput) (buildkitBootOutput, error) {
b.observability = observability
log := observability.log
log.Info("starting embedded buildkit daemon", "socket-dir", b.inputs.config.GetSocketDir())
Expand All @@ -89,6 +89,7 @@ func (b *buildkitBoot) startEmbedded(ctx context.Context, containerd containerdB
GCKeepStorage: int64(gcStorage.Bytes()),
GCKeepDuration: int64(gcDuration.Seconds()),
RegistryHost: ocireg.Host,
DNSServer: network.routerAddress.String(),
}); err != nil {
return buildkitBootOutput{}, err
}
Expand Down
2 changes: 1 addition & 1 deletion components/server/startup.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func newStartup(runtime *Runtime, options StartOptions) *startup {
etcd := newEtcdBoot(etcdInputs(options), ipDiscovery.output, containerd.output, observability.output)
network := newNetworkBoot(networkInputs(options), etcd.output, observability.output)
registryHostMapping := newRegistryHostMappingBoot(registryHostMappingInputs(hostMapper), network.output)
buildkit := newBuildkitBoot(buildkitInputs(options), containerd.output, registryHostMapping.output, observability.output)
buildkit := newBuildkitBoot(buildkitInputs(options), containerd.output, network.output, registryHostMapping.output, observability.output)
coordinator := newCoordinatorBoot(
coordinatorInputs(options, resolver, secretRegistry, address),
ipDiscovery.output,
Expand Down
Loading