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 Sources/ContainerCommands/BuildCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,17 @@ extension Application {
progress.start()

progress.set(description: "Dialing builder")

let dnsNameservers = self.dns.nameservers
let builder: Builder? = try await withThrowingTaskGroup(of: Builder.self) { [vsockPort, cpus, memory, dnsNameservers] group in
let dnsDomain = self.dns.domain
let dnsSearchDomains = self.dns.searchDomains
let dnsOptions = self.dns.options
let builder: Builder? = try await withThrowingTaskGroup(of: Builder.self) {
[vsockPort, cpus, memory, dnsNameservers, dnsDomain, dnsSearchDomains, dnsOptions] group in
defer {
group.cancelAll()
}

group.addTask { [vsockPort, cpus, memory, log, dnsNameservers] in
group.addTask { [vsockPort, cpus, memory, log, dnsNameservers, dnsDomain, dnsSearchDomains, dnsOptions] in
let client = ContainerClient()
while true {
do {
Expand All @@ -193,6 +196,9 @@ extension Application {
memory: memory,
log: log,
dnsNameservers: dnsNameservers,
dnsDomain: dnsDomain,
dnsSearchDomains: dnsSearchDomains,
dnsOptions: dnsOptions,
progressUpdate: progress.handler,
containerSystemConfig: containerSystemConfig,
)
Expand Down
45 changes: 26 additions & 19 deletions Sources/ContainerCommands/Builder/BuilderStart.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,16 @@ extension Application {
progressUpdate: @escaping ProgressUpdateHandler,
containerSystemConfig: ContainerSystemConfig,
) async throws {
let dns = Utility.dnsConfiguration(
from: .init(
domain: dnsDomain,
nameservers: dnsNameservers,
options: dnsOptions,
searchDomains: dnsSearchDomains
),
defaults: containerSystemConfig.container.dns
)

await progressUpdate([
.setDescription("Fetching BuildKit image"),
.setItemsName("blobs"),
Expand Down Expand Up @@ -150,21 +160,7 @@ extension Application {
let imageChanged = existingImage != builderImage
let cpuChanged = existingResources.cpus != resources.cpus
let memChanged = existingResources.memoryInBytes != resources.memoryInBytes
let dnsChanged = {
if !dnsNameservers.isEmpty {
return existingDNS?.nameservers != dnsNameservers
}
if dnsDomain != nil {
return existingDNS?.domain != dnsDomain
}
if !dnsSearchDomains.isEmpty {
return existingDNS?.searchDomains != dnsSearchDomains
}
if !dnsOptions.isEmpty {
return existingDNS?.options != dnsOptions
}
return false
}()
let dnsChanged = Self.dnsConfigurationChanged(existing: existingDNS, desired: dns)

switch existingContainer.status {
case .running:
Expand Down Expand Up @@ -270,10 +266,10 @@ extension Application {
AttachmentConfiguration(network: defaultNetwork.id, options: AttachmentOptions(hostname: Builder.builderContainerId))
]
config.dns = ContainerConfiguration.DNSConfiguration(
nameservers: dnsNameservers,
domain: dnsDomain,
searchDomains: dnsSearchDomains,
options: dnsOptions
nameservers: dns.nameservers,
domain: dns.domain,
searchDomains: dns.searchDomains,
options: dns.options
)

let kernel = try await {
Expand All @@ -299,6 +295,17 @@ extension Application {
try await startBuildKit(client: client, id: Builder.builderContainerId, progressUpdate, taskManager)
log.debug("starting BuildKit and BuildKit-shim")
}

static func dnsConfigurationChanged(
existing: ContainerConfiguration.DNSConfiguration?,
desired: ContainerConfiguration.DNSConfiguration
) -> Bool {
let existing = existing ?? .init()
return existing.nameservers != desired.nameservers
|| existing.domain != desired.domain
|| existing.searchDomains != desired.searchDomains
|| existing.options != desired.options
}
}
}

Expand Down
40 changes: 39 additions & 1 deletion Sources/ContainerPersistence/ContainerSystemConfig.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,16 +113,23 @@ final public class ContainerConfig: Codable, Sendable {

public let cpus: Int
public let memory: MemorySize
public let dns: ContainerDNSConfig

public init(cpus: Int = defaultCPUs, memory: MemorySize = defaultMemory) {
public init(
cpus: Int = defaultCPUs,
memory: MemorySize = defaultMemory,
dns: ContainerDNSConfig = .init()
) {
self.cpus = cpus
self.memory = memory
self.dns = dns
}

public init(from decoder: any Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
self.cpus = try container.decodeIfPresent(Int.self, forKey: .cpus) ?? Self.defaultCPUs
self.memory = try container.decodeIfPresent(MemorySize.self, forKey: .memory) ?? Self.defaultMemory
self.dns = try container.decodeIfPresent(ContainerDNSConfig.self, forKey: .dns) ?? .init()
}
}

Expand All @@ -139,6 +146,37 @@ final public class DNSConfig: Codable, Sendable {
}
}

final public class ContainerDNSConfig: Codable, Sendable {
public static let defaultNameservers: [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.

Today there is a subtle difference between what the default DNS settings are for container aka the apiserver itself, which would determine the hostname resolution (aka the A record) for a given container on the host, and the DNS settings for setting up the resolv.conf in a container. I think we should maintain this distinction. There could be scenarios where a user does not want the default DNS domain on the host to necessarily match the default DNS domain that the container application uses in the container itself.

I think we should add a new field on the ContainerConfig type that has the default DNS settings for running a container.

Essentially we'd end up with something like this in the TOML:

[dns] <-------- default DNS settings for the APIServer
domain = "test"

[container.dns] <----------- default DNS settings for containers 
server = "8.8.8.8"
domain = "foo"
search = ["foo", "test"]
options = ["haha"]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, good insight. I removed that extra builder startup call.

public static let defaultSearchDomains: [String] = []
public static let defaultOptions: [String] = []

public let domain: String?
public let nameservers: [String]
public let searchDomains: [String]
public let options: [String]

public init(
domain: String? = nil,
nameservers: [String] = defaultNameservers,
searchDomains: [String] = defaultSearchDomains,
options: [String] = defaultOptions
) {
self.domain = domain
self.nameservers = nameservers
self.searchDomains = searchDomains
self.options = options
}

public init(from decoder: any Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
self.domain = try container.decodeIfPresent(String.self, forKey: .domain)
self.nameservers = try container.decodeIfPresent([String].self, forKey: .nameservers) ?? Self.defaultNameservers
self.searchDomains = try container.decodeIfPresent([String].self, forKey: .searchDomains) ?? Self.defaultSearchDomains
self.options = try container.decodeIfPresent([String].self, forKey: .options) ?? Self.defaultOptions
}
}

final public class VminitConfig: Codable, Sendable {
public static var defaultImage: String {
let tag = String(cString: get_swift_containerization_version())
Expand Down
36 changes: 30 additions & 6 deletions Sources/Services/ContainerAPIService/Client/Utility.swift
Original file line number Diff line number Diff line change
Expand Up @@ -223,12 +223,10 @@ public struct Utility {
if management.dnsDisabled {
config.dns = nil
} else {
let domain = management.dns.domain ?? containerSystemConfig.dns.domain
config.dns = .init(
nameservers: management.dns.nameservers,
domain: domain,
searchDomains: management.dns.searchDomains,
options: management.dns.options
config.dns = dnsConfiguration(
from: management.dns,
defaults: containerSystemConfig.container.dns,
hostDomainFallback: containerSystemConfig.dns.domain
)
}

Expand Down Expand Up @@ -330,6 +328,32 @@ public struct Utility {
return [AttachmentConfiguration(network: builtinNetworkId, options: AttachmentOptions(hostname: fqdn ?? containerId, macAddress: nil, mtu: 1280))]
}

public static func dnsConfiguration(
from flags: Flags.DNS,
defaults: ContainerDNSConfig,
hostDomainFallback: String? = nil
) -> ContainerConfiguration.DNSConfiguration {
let nameservers =
flags.nameservers.isEmpty
? defaults.nameservers
: flags.nameservers
let domain = flags.domain ?? defaults.domain ?? hostDomainFallback
let searchDomains =
flags.searchDomains.isEmpty
? defaults.searchDomains
: flags.searchDomains
let options =
flags.options.isEmpty
? defaults.options
: flags.options
return .init(
nameservers: nameservers,
domain: domain,
searchDomains: searchDomains,
options: options
)
}

private static func getKernel(management: Flags.Management) async throws -> Kernel {
// For the image itself we'll take the user input and try with it as we can do userspace
// emulation for x86, but for the kernel we need it to match the hosts architecture.
Expand Down
2 changes: 1 addition & 1 deletion Tests/CLITests/Subcommands/Run/TestCLIRunCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,7 @@ class TestCLIRunCommand3: CLITest {

func getDefaultDomain() throws -> String? {
let config = try getSystemConfig()
return config.dns.domain
return config.container.dns.domain
}

@Test func testPrivilegedPortError() throws {
Expand Down
101 changes: 101 additions & 0 deletions Tests/ContainerAPIClientTests/UtilityTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// limitations under the License.
//===----------------------------------------------------------------------===//

import ContainerPersistence
import ContainerResource
import ContainerizationError
import Foundation
Expand Down Expand Up @@ -90,6 +91,106 @@ struct UtilityTests {
}
}

@Test
func testDNSConfigurationUsesDefaultsWhenFlagsAbsent() {
let defaults = ContainerDNSConfig(
domain: "corp.local",
nameservers: ["1.1.1.1"],
searchDomains: ["corp.local"],
options: ["ndots:2"]
)

let flags = Flags.DNS(
domain: nil,
nameservers: [],
options: [],
searchDomains: []
)

let result = Utility.dnsConfiguration(from: flags, defaults: defaults)

#expect(result.domain == "corp.local")
#expect(result.nameservers == ["1.1.1.1"])
#expect(result.searchDomains == ["corp.local"])
#expect(result.options == ["ndots:2"])
}

@Test
func testDNSConfigurationFlagsOverrideDefaults() {
let defaults = ContainerDNSConfig(
domain: "corp.local",
nameservers: ["1.1.1.1"],
searchDomains: ["corp.local"],
options: ["ndots:2"]
)
let flags = Flags.DNS(
domain: "cli.local",
nameservers: ["8.8.8.8"],
options: ["debug"],
searchDomains: ["cli.local"]
)

let result = Utility.dnsConfiguration(from: flags, defaults: defaults)

#expect(result.domain == "cli.local")
#expect(result.nameservers == ["8.8.8.8"])
#expect(result.searchDomains == ["cli.local"])
#expect(result.options == ["debug"])
}

@Test
func testDNSConfigurationPartialFlagsFallbackToDefaults() {
let defaults = ContainerDNSConfig(
domain: "corp.local",
nameservers: ["1.1.1.1"],
searchDomains: ["corp.local"],
options: ["ndots:2"]
)
let flags = Flags.DNS(
domain: nil,
nameservers: ["8.8.8.8"],
options: [],
searchDomains: []
)

let result = Utility.dnsConfiguration(from: flags, defaults: defaults)

#expect(result.domain == "corp.local")
#expect(result.nameservers == ["8.8.8.8"])
#expect(result.searchDomains == ["corp.local"])
#expect(result.options == ["ndots:2"])
}

@Test
func testDNSConfigurationDomainFallsBackToHostDomain() {
let defaults = ContainerDNSConfig()
let flags = Flags.DNS(
domain: nil,
nameservers: [],
options: [],
searchDomains: []
)

let result = Utility.dnsConfiguration(from: flags, defaults: defaults, hostDomainFallback: "host.local")

#expect(result.domain == "host.local")
}

@Test
func testDNSConfigurationContainerDomainWinsOverHostDomain() {
let defaults = ContainerDNSConfig(domain: "container.local")
let flags = Flags.DNS(
domain: nil,
nameservers: [],
options: [],
searchDomains: []
)

let result = Utility.dnsConfiguration(from: flags, defaults: defaults, hostDomainFallback: "host.local")

#expect(result.domain == "container.local")
}

@Test
func testPublishPortParser() throws {
let ports = try Parser.publishPorts([
Expand Down
54 changes: 54 additions & 0 deletions Tests/ContainerCommandsTests/BuilderStartTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
//===----------------------------------------------------------------------===//
// Copyright © 2026 Apple Inc. and the container project authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//===----------------------------------------------------------------------===//

import ContainerResource
import Testing

@testable import ContainerCommands

struct BuilderStartTests {
@Test
func dnsConfigurationChangedTreatsMissingAndEmptyAsEquivalent() {
let desired = ContainerConfiguration.DNSConfiguration()

#expect(!Application.BuilderStart.dnsConfigurationChanged(existing: nil, desired: desired))
}

@Test
func dnsConfigurationChangedDetectsRemovedConfiguration() {
let existing = ContainerConfiguration.DNSConfiguration(
nameservers: ["1.1.1.1"],
domain: "corp.local",
searchDomains: ["corp.local"],
options: ["ndots:2"]
)
let desired = ContainerConfiguration.DNSConfiguration()

#expect(Application.BuilderStart.dnsConfigurationChanged(existing: existing, desired: desired))
}

@Test
func dnsConfigurationChangedDetectsAddedConfiguration() {
let desired = ContainerConfiguration.DNSConfiguration(
nameservers: ["8.8.8.8"],
domain: "lab.local",
searchDomains: ["lab.local"],
options: ["timeout:1"]
)

#expect(Application.BuilderStart.dnsConfigurationChanged(existing: nil, desired: desired))
}
}
Loading