Skip to content

Commit aefba9d

Browse files
committed
containertool: Separate command line option processing from image publishing (apple#116)
Motivation ---------- Most of `containertool`'s work is handled in its `run()` method. * It starts by processing arguments and assigning defaults; * It then creates `RegistryClient` instance; * Finally it assembles the image and uploads it. All of this work takes place inline, in a single method called by `SwiftArgumentParser`. This complicates maintenance and makes it difficult to test the image assembly process without writing full end-to-end tests. Modifications ------------- * Extract the image building process into a standalone function. * `run()` is only responsible for processing arguments and creating the clients needed by the build function. Result ------ The image build process will be easier to test and maintain. It will be easier to add more complex argument handling, such as additional environment variables or a configuration file. Test Plan --------- Existing tests continue to pass. Future pull requests can test the image build process in integration or unit tests, instead of requiring full end-to-end tests.
1 parent 16b8b43 commit aefba9d

File tree

1 file changed

+85
-43
lines changed

1 file changed

+85
-43
lines changed

Sources/containertool/containertool.swift

Lines changed: 85 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ enum AllowHTTP: String, ExpressibleByArgument, CaseIterable { case source, desti
7272
private var netrcFile: String?
7373

7474
func run() async throws {
75-
let baseimage = try ImageReference(fromString: from, defaultRegistry: defaultRegistry)
76-
var destination_image = try ImageReference(fromString: repository, defaultRegistry: defaultRegistry)
75+
let baseImage = try ImageReference(fromString: from, defaultRegistry: defaultRegistry)
76+
let destinationImage = try ImageReference(fromString: repository, defaultRegistry: defaultRegistry)
7777

7878
let authProvider: AuthorizationProvider?
7979
if !netrc {
@@ -96,23 +96,23 @@ enum AllowHTTP: String, ExpressibleByArgument, CaseIterable { case source, desti
9696
source = nil
9797
} else {
9898
source = try await RegistryClient(
99-
registry: baseimage.registry,
99+
registry: baseImage.registry,
100100
insecure: allowInsecureHttp == .source || allowInsecureHttp == .both,
101101
auth: .init(username: username, password: password, auth: authProvider)
102102
)
103-
if verbose { log("Connected to source registry: \(baseimage.registry)") }
103+
if verbose { log("Connected to source registry: \(baseImage.registry)") }
104104
}
105105

106106
let destination = try await RegistryClient(
107-
registry: destination_image.registry,
107+
registry: destinationImage.registry,
108108
insecure: allowInsecureHttp == .destination || allowInsecureHttp == .both,
109109
auth: .init(username: username, password: password, auth: authProvider)
110110
)
111111

112-
if verbose { log("Connected to destination registry: \(destination_image.registry)") }
113-
if verbose { log("Using base image: \(baseimage)") }
112+
if verbose { log("Connected to destination registry: \(destinationImage.registry)") }
113+
if verbose { log("Using base image: \(baseImage)") }
114114

115-
// MARK: Find the base image
115+
// MARK: Detect the base image architecture
116116

117117
// Try to detect the architecture of the application executable so a suitable base image can be selected.
118118
// This reduces the risk of accidentally creating an image which stacks an aarch64 executable on top of an x86_64 base image.
@@ -125,27 +125,64 @@ enum AllowHTTP: String, ExpressibleByArgument, CaseIterable { case source, desti
125125
?? "amd64"
126126
if verbose { log("Base image architecture: \(architecture)") }
127127

128-
let baseimage_manifest: ImageManifest
129-
let baseimage_config: ImageConfiguration
128+
// MARK: Build the image
129+
130+
let finalImage = try await destination.publishContainerImage(
131+
baseImage: baseImage,
132+
destinationImage: destinationImage,
133+
source: source,
134+
architecture: architecture,
135+
os: os,
136+
resources: resources,
137+
tag: tag,
138+
verbose: verbose,
139+
executableURL: executableURL
140+
)
141+
142+
print(finalImage)
143+
}
144+
}
145+
146+
extension RegistryClient {
147+
func publishContainerImage(
148+
baseImage: ImageReference,
149+
destinationImage: ImageReference,
150+
source: RegistryClient?,
151+
architecture: String,
152+
os: String,
153+
resources: [String],
154+
tag: String?,
155+
verbose: Bool,
156+
executableURL: URL
157+
) async throws -> ImageReference {
158+
159+
// MARK: Find the base image
160+
161+
let baseImageManifest: ImageManifest
162+
let baseImageConfiguration: ImageConfiguration
130163
if let source {
131-
baseimage_manifest = try await source.getImageManifest(
132-
forImage: baseimage,
164+
baseImageManifest = try await source.getImageManifest(
165+
forImage: baseImage,
133166
architecture: architecture
134167
)
135-
log("Found base image manifest: \(baseimage_manifest.digest)")
168+
log("Found base image manifest: \(baseImageManifest.digest)")
136169

137-
baseimage_config = try await source.getImageConfiguration(
138-
forImage: baseimage,
139-
digest: baseimage_manifest.config.digest
170+
baseImageConfiguration = try await source.getImageConfiguration(
171+
forImage: baseImage,
172+
digest: baseImageManifest.config.digest
140173
)
141-
log("Found base image configuration: \(baseimage_manifest.config.digest)")
174+
log("Found base image configuration: \(baseImageManifest.config.digest)")
142175
} else {
143-
baseimage_manifest = .init(
176+
baseImageManifest = .init(
144177
schemaVersion: 2,
145178
config: .init(mediaType: "scratch", digest: "scratch", size: 0),
146179
layers: []
147180
)
148-
baseimage_config = .init(architecture: architecture, os: os, rootfs: .init(_type: "layers", diff_ids: []))
181+
baseImageConfiguration = .init(
182+
architecture: architecture,
183+
os: os,
184+
rootfs: .init(_type: "layers", diff_ids: [])
185+
)
149186
if verbose { log("Using scratch as base image") }
150187
}
151188

@@ -154,8 +191,8 @@ enum AllowHTTP: String, ExpressibleByArgument, CaseIterable { case source, desti
154191
var resourceLayers: [RegistryClient.ImageLayer] = []
155192
for resourceDir in resources {
156193
let resourceTardiff = try Archive().appendingRecursively(atPath: resourceDir).bytes
157-
let resourceLayer = try await destination.uploadLayer(
158-
repository: destination_image.repository,
194+
let resourceLayer = try await self.uploadLayer(
195+
repository: destinationImage.repository,
159196
contents: resourceTardiff
160197
)
161198

@@ -167,55 +204,59 @@ enum AllowHTTP: String, ExpressibleByArgument, CaseIterable { case source, desti
167204
}
168205

169206
// MARK: Upload the application layer
170-
let applicationLayer = try await destination.uploadLayer(
171-
repository: destination_image.repository,
207+
208+
let applicationLayer = try await self.uploadLayer(
209+
repository: destinationImage.repository,
172210
contents: try Archive().appendingFile(at: executableURL).bytes
173211
)
174212
if verbose {
175213
log("application layer: \(applicationLayer.descriptor.digest) (\(applicationLayer.descriptor.size) bytes)")
176214
}
177215

178216
// MARK: Create the application configuration
217+
179218
let timestamp = Date(timeIntervalSince1970: 0).ISO8601Format()
180219

181220
// Inherit the configuration of the base image - UID, GID, environment etc -
182221
// and override the entrypoint.
183-
var inherited_config = baseimage_config.config ?? .init()
184-
inherited_config.Entrypoint = ["/\(executableURL.lastPathComponent)"]
185-
inherited_config.Cmd = []
186-
inherited_config.WorkingDir = "/"
222+
var inheritedConfiguration = baseImageConfiguration.config ?? .init()
223+
inheritedConfiguration.Entrypoint = ["/\(executableURL.lastPathComponent)"]
224+
inheritedConfiguration.Cmd = []
225+
inheritedConfiguration.WorkingDir = "/"
187226

188227
let configuration = ImageConfiguration(
189228
created: timestamp,
190229
architecture: architecture,
191230
os: os,
192-
config: inherited_config,
231+
config: inheritedConfiguration,
193232
rootfs: .init(
194233
_type: "layers",
195234
// The diff_id is the digest of the _uncompressed_ layer archive.
196235
// It is used by the runtime, which might not store the layers in
197236
// the compressed form in which it received them from the registry.
198-
diff_ids: baseimage_config.rootfs.diff_ids
237+
diff_ids: baseImageConfiguration.rootfs.diff_ids
199238
+ resourceLayers.map { $0.diffID }
200239
+ [applicationLayer.diffID]
201240
),
202241
history: [.init(created: timestamp, created_by: "containertool")]
203242
)
204243

205-
let config_blob = try await destination.putImageConfiguration(
206-
forImage: destination_image,
244+
let configurationBlobReference = try await self.putImageConfiguration(
245+
forImage: destinationImage,
207246
configuration: configuration
208247
)
209248

210-
if verbose { log("image configuration: \(config_blob.digest) (\(config_blob.size) bytes)") }
249+
if verbose {
250+
log("image configuration: \(configurationBlobReference.digest) (\(configurationBlobReference.size) bytes)")
251+
}
211252

212253
// MARK: Create application manifest
213254

214255
let manifest = ImageManifest(
215256
schemaVersion: 2,
216257
mediaType: "application/vnd.oci.image.manifest.v1+json",
217-
config: config_blob,
218-
layers: baseimage_manifest.layers
258+
config: configurationBlobReference,
259+
layers: baseImageManifest.layers
219260
+ resourceLayers.map { $0.descriptor }
220261
+ [applicationLayer.descriptor]
221262
)
@@ -226,12 +267,12 @@ enum AllowHTTP: String, ExpressibleByArgument, CaseIterable { case source, desti
226267
// Layers could be checked and uploaded concurrently
227268
// This could also happen in parallel with the application image build
228269
if let source {
229-
for layer in baseimage_manifest.layers {
270+
for layer in baseImageManifest.layers {
230271
try await source.copyBlob(
231272
digest: layer.digest,
232-
fromRepository: baseimage.repository,
233-
toClient: destination,
234-
toRepository: destination_image.repository
273+
fromRepository: baseImage.repository,
274+
toClient: self,
275+
toRepository: destinationImage.repository
235276
)
236277
}
237278
}
@@ -242,15 +283,16 @@ enum AllowHTTP: String, ExpressibleByArgument, CaseIterable { case source, desti
242283
// To support multiarch images, we should also create an an index pointing to
243284
// this manifest.
244285
let reference = tag ?? manifest.digest
245-
let location = try await destination.putManifest(
246-
repository: destination_image.repository,
247-
reference: destination_image.reference,
286+
let location = try await self.putManifest(
287+
repository: destinationImage.repository,
288+
reference: destinationImage.reference,
248289
manifest: manifest
249290
)
250291

251292
if verbose { log(location) }
252293

253-
destination_image.reference = reference
254-
print(destination_image)
294+
var result = destinationImage
295+
result.reference = reference
296+
return result
255297
}
256298
}

0 commit comments

Comments
 (0)