Skip to content

Commit 9af641f

Browse files
jgloganRonitsabhaya75
authored andcommitted
Fix broken proxy configuration for default kernel fetch. (apple#747)
- Closes apple#466. ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context Proxy logic worked well enough for CI but broken in general. ## Testing - [x] Tested locally - [x] Added/updated tests - [ ] Added/updated docs
1 parent f4cf2c6 commit 9af641f

File tree

7 files changed

+119
-23
lines changed

7 files changed

+119
-23
lines changed

.github/workflows/common.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ jobs:
6767
6868
- name: Test the container project
6969
run: |
70-
launchctl setenv HTTP_PROXY $HTTP_PROXY
7170
APP_ROOT=$(mktemp -d -p "${RUNNER_TEMP}")
7271
trap 'rm -rf "${APP_ROOT}"; echo Removing data directory ${APP_ROOT}' EXIT
7372
echo "Created data directory ${APP_ROOT}"

Package.resolved

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Package.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import PackageDescription
2323
let releaseVersion = ProcessInfo.processInfo.environment["RELEASE_VERSION"] ?? "0.0.0"
2424
let gitCommit = ProcessInfo.processInfo.environment["GIT_COMMIT"] ?? "unspecified"
2525
let builderShimVersion = "0.6.1"
26-
let scVersion = "0.9.1"
26+
let scVersion = "0.10.0"
2727

2828
let package = Package(
2929
name: "container",

Sources/ContainerClient/FileDownloader.swift

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,24 +49,20 @@ public struct FileDownloader {
4949
}
5050
})
5151

52-
let client = FileDownloader.createClient()
52+
let client = FileDownloader.createClient(url: url)
5353
_ = try await client.execute(request: request, delegate: delegate).get()
5454
try await client.shutdown()
5555
}
5656

57-
private static func createClient() -> HTTPClient {
57+
private static func createClient(url: URL) -> HTTPClient {
5858
var httpConfiguration = HTTPClient.Configuration()
59-
let proxyConfig: HTTPClient.Configuration.Proxy? = {
60-
let proxyEnv = ProcessInfo.processInfo.environment["HTTP_PROXY"]
61-
guard let proxyEnv else {
62-
return nil
59+
if let host = url.host {
60+
let proxyURL = ProxyUtils.proxyFromEnvironment(scheme: url.scheme, host: host)
61+
if let proxyURL, let proxyHost = proxyURL.host {
62+
httpConfiguration.proxy = HTTPClient.Configuration.Proxy.server(host: proxyHost, port: proxyURL.port ?? 8080)
6363
}
64-
guard let url = URL(string: proxyEnv), let host = url.host(), let port = url.port else {
65-
return nil
66-
}
67-
return .server(host: host, port: port)
68-
}()
69-
httpConfiguration.proxy = proxyConfig
64+
}
65+
7066
return HTTPClient(eventLoopGroupProvider: .singleton, configuration: httpConfiguration)
7167
}
7268
}

Sources/ContainerCommands/System/SystemStart.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,8 @@ extension Application {
6969
args.append("start")
7070
let apiServerDataUrl = appRoot.appending(path: "apiserver")
7171
try! FileManager.default.createDirectory(at: apiServerDataUrl, withIntermediateDirectories: true)
72-
var env = ProcessInfo.processInfo.environment.filter { key, _ in
73-
key.hasPrefix("CONTAINER_")
74-
}
72+
73+
var env = PluginLoader.filterEnvironment()
7574
env[ApplicationRoot.environmentName] = appRoot.path(percentEncoded: false)
7675
env[InstallRoot.environmentName] = installRoot.path(percentEncoded: false)
7776

Sources/ContainerPlugin/PluginLoader.swift

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,12 @@ extension PluginLoader {
196196
}
197197

198198
extension PluginLoader {
199+
public static let proxyKeys = Set([
200+
"http_proxy", "HTTP_PROXY",
201+
"https_proxy", "HTTPS_PROXY",
202+
"no_proxy", "NO_PROXY",
203+
])
204+
199205
public func registerWithLaunchd(
200206
plugin: Plugin,
201207
pluginStateRoot: URL? = nil,
@@ -212,9 +218,8 @@ extension PluginLoader {
212218
log?.info("Registering plugin", metadata: ["id": "\(id)"])
213219
let rootURL = pluginStateRoot ?? self.pluginResourceRoot.appending(path: plugin.name)
214220
try FileManager.default.createDirectory(at: rootURL, withIntermediateDirectories: true)
215-
var env = ProcessInfo.processInfo.environment.filter { key, _ in
216-
key.hasPrefix("CONTAINER_")
217-
}
221+
222+
var env = Self.filterEnvironment()
218223
env[ApplicationRoot.environmentName] = appRoot.path(percentEncoded: false)
219224
env[InstallRoot.environmentName] = installRoot.path(percentEncoded: false)
220225

@@ -244,4 +249,13 @@ extension PluginLoader {
244249
log?.info("Deregistering plugin", metadata: ["id": "\(plugin.getLaunchdLabel())"])
245250
try ServiceManager.deregister(fullServiceLabel: label)
246251
}
252+
253+
public static func filterEnvironment(
254+
env: [String: String] = ProcessInfo.processInfo.environment,
255+
additionalAllowKeys: Set<String> = Self.proxyKeys
256+
) -> [String: String] {
257+
env.filter { key, _ in
258+
key.hasPrefix("CONTAINER_") || additionalAllowKeys.contains(key)
259+
}
260+
}
247261
}

Tests/ContainerPluginTests/PluginLoaderTest.swift

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,94 @@ struct PluginLoaderTest {
8585
#expect(loader.findPlugin(name: "throw") == nil)
8686
}
8787

88+
@Test
89+
func testFilterEnvironmentWithContainerPrefix() async throws {
90+
let env = [
91+
"CONTAINER_FOO": "bar",
92+
"CONTAINER_BAZ": "qux",
93+
"OTHER_VAR": "value",
94+
]
95+
let filtered = PluginLoader.filterEnvironment(env: env, additionalAllowKeys: [])
96+
97+
#expect(filtered == ["CONTAINER_FOO": "bar", "CONTAINER_BAZ": "qux"])
98+
}
99+
100+
@Test
101+
func testFilterEnvironmentWithProxyKeys() async throws {
102+
let env = [
103+
"http_proxy": "http://proxy:8080",
104+
"HTTP_PROXY": "http://proxy:8080",
105+
"https_proxy": "https://proxy:8443",
106+
"HTTPS_PROXY": "https://proxy:8443",
107+
"no_proxy": "localhost,127.0.0.1",
108+
"NO_PROXY": "localhost,127.0.0.1",
109+
"OTHER_VAR": "value",
110+
]
111+
let filtered = PluginLoader.filterEnvironment(env: env)
112+
113+
#expect(
114+
filtered == [
115+
"http_proxy": "http://proxy:8080",
116+
"HTTP_PROXY": "http://proxy:8080",
117+
"https_proxy": "https://proxy:8443",
118+
"HTTPS_PROXY": "https://proxy:8443",
119+
"no_proxy": "localhost,127.0.0.1",
120+
"NO_PROXY": "localhost,127.0.0.1",
121+
])
122+
}
123+
124+
@Test
125+
func testFilterEnvironmentWithBothContainerAndProxy() async throws {
126+
let env = [
127+
"CONTAINER_FOO": "bar",
128+
"http_proxy": "http://proxy:8080",
129+
"OTHER_VAR": "value",
130+
"ANOTHER_VAR": "value2",
131+
]
132+
let filtered = PluginLoader.filterEnvironment(env: env)
133+
134+
#expect(
135+
filtered == [
136+
"CONTAINER_FOO": "bar",
137+
"http_proxy": "http://proxy:8080",
138+
])
139+
}
140+
141+
@Test
142+
func testFilterEnvironmentWithCustomAllowKeys() async throws {
143+
let env = [
144+
"CONTAINER_FOO": "bar",
145+
"CUSTOM_KEY": "custom_value",
146+
"OTHER_VAR": "value",
147+
]
148+
let filtered = PluginLoader.filterEnvironment(env: env, additionalAllowKeys: ["CUSTOM_KEY"])
149+
150+
#expect(
151+
filtered == [
152+
"CONTAINER_FOO": "bar",
153+
"CUSTOM_KEY": "custom_value",
154+
])
155+
}
156+
157+
@Test
158+
func testFilterEnvironmentEmpty() async throws {
159+
let filtered = PluginLoader.filterEnvironment(env: [:])
160+
161+
#expect(filtered.isEmpty)
162+
}
163+
164+
@Test
165+
func testFilterEnvironmentNoMatches() async throws {
166+
let env = [
167+
"PATH": "/usr/bin",
168+
"HOME": "/Users/test",
169+
"USER": "testuser",
170+
]
171+
let filtered = PluginLoader.filterEnvironment(env: env, additionalAllowKeys: [])
172+
173+
#expect(filtered.isEmpty)
174+
}
175+
88176
private func setupMock(tempURL: URL) throws -> MockPluginFactory {
89177
let cliConfig = PluginConfig(abstract: "cli", author: "CLI", servicesConfig: nil)
90178
let cliPlugin: Plugin = Plugin(binaryURL: URL(filePath: "/bin/cli"), config: cliConfig)

0 commit comments

Comments
 (0)