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
32 changes: 28 additions & 4 deletions Sources/ContainerCommands/Image/ImageLoad.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,38 @@ extension Application {
transform: { str in
URL(fileURLWithPath: str, relativeTo: .currentDirectory()).absoluteURL.path(percentEncoded: false)
})
var input: String
var input: String?

@OptionGroup
var global: Flags.Global

public func run() async throws {
guard FileManager.default.fileExists(atPath: input) else {
print("File does not exist \(input)")
let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).tar")
defer {
try? FileManager.default.removeItem(at: tempFile)
}

// Read from stdin
if input == nil {
guard FileManager.default.createFile(atPath: tempFile.path(), contents: nil) else {
throw ContainerizationError(.internalError, message: "unable to create temporary file")
}

guard let outputHandle = try? FileHandle(forWritingTo: tempFile) else {
throw ContainerizationError(.internalError, message: "unable to open temporary file for writing")
}

let bufferSize = 4096
while true {
let chunk = FileHandle.standardInput.readData(ofLength: bufferSize)
if chunk.isEmpty { break }
outputHandle.write(chunk)
}
try outputHandle.close()
}

guard FileManager.default.fileExists(atPath: input ?? tempFile.path()) else {
print("File does not exist \(input ?? tempFile.path())")
Application.exit(withError: ArgumentParser.ExitCode(1))
}

Expand All @@ -57,7 +81,7 @@ extension Application {
progress.start()

progress.set(description: "Loading tar archive")
let loaded = try await ClientImage.load(from: input)
let loaded = try await ClientImage.load(from: input ?? tempFile.path())
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: What do you think about restructuring load like to only run the code that's needed for each case? The only bit of code that repeats is the ClientImage.load line, and we would be removing a conditional from that.

var loaded: ClientImage
if input == nil {
    // create tmpfile, defer delete, copy stdin to file
    loaded = try await ClientImage.load(from: tempFile.path())
} else {
    // ensure input exists
    loaded = loaded = try await ClientImage.load(from: input)
}

Copy link
Contributor Author

@saehejkang saehejkang Nov 15, 2025

Choose a reason for hiding this comment

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

Any reason for this update besides readability (coding standards, etc)? I thought using the ternary operator makes sense in that it is clear to use the input path first, if not nil, and the temp file path otherwise. Just thought calling load once makes more sense and is cleaner. Not 100% adamant on one way but will defer to you for the final decision.


let taskManager = ProgressTaskCoordinator()
let unpackTask = await taskManager.startTask()
Expand Down
29 changes: 27 additions & 2 deletions Sources/ContainerCommands/Image/ImageSave.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ extension Application {
transform: { str in
URL(fileURLWithPath: str, relativeTo: .currentDirectory()).absoluteURL.path(percentEncoded: false)
})
var output: String
var output: String?

@Option(
help: "Platform for the saved image (format: os/arch[/variant], takes precedence over --os and --arch)"
Expand Down Expand Up @@ -90,7 +90,32 @@ extension Application {
throw ContainerizationError(.invalidArgument, message: "failed to save image(s)")

}
try await ClientImage.save(references: references, out: output, platform: p)

let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).tar")
defer {
try? FileManager.default.removeItem(at: tempFile)
}

guard FileManager.default.createFile(atPath: tempFile.path(), contents: nil) else {
throw ContainerizationError(.internalError, message: "unable to create temporary file")
}

try await ClientImage.save(references: references, out: output ?? tempFile.path(), platform: p)

// Write to stdout
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for this one, we could put an else on this that just saves the image to output, and move the above code into the output == nil case.

if output == nil {
guard let outputHandle = try? FileHandle(forReadingFrom: tempFile) else {
throw ContainerizationError(.internalError, message: "unable to open temporary file for reading")
}

let bufferSize = 4096
while true {
let chunk = outputHandle.readData(ofLength: bufferSize)
if chunk.isEmpty { break }
FileHandle.standardOutput.write(chunk)
}
try outputHandle.close()
}

progress.finish()
for reference in references {
Expand Down
61 changes: 61 additions & 0 deletions Tests/CLITests/Subcommands/Images/TestCLIImages.swift
Original file line number Diff line number Diff line change
Expand Up @@ -359,4 +359,65 @@ extension TestCLIImagesCommand {
return
}
}

@Test func testImageSaveAndLoadStdinStdout() throws {
do {
// 1. pull image
try doPull(imageName: alpine)
try doPull(imageName: busybox)

// 2. Tag image so we can safely remove later
let alpineRef: Reference = try Reference.parse(alpine)
let alpineTagged = "\(alpineRef.name):testImageSaveAndLoadStdinStdout"
try doImageTag(image: alpine, newName: alpineTagged)
let alpineTaggedImagePresent = try isImagePresent(targetImage: alpineTagged)
#expect(alpineTaggedImagePresent, "expected to see image \(alpineTagged) tagged")

let busyboxRef: Reference = try Reference.parse(busybox)
let busyboxTagged = "\(busyboxRef.name):testImageSaveAndLoadStdinStdout"
try doImageTag(image: busybox, newName: busyboxTagged)
let busyboxTaggedImagePresent = try isImagePresent(targetImage: busyboxTagged)
#expect(busyboxTaggedImagePresent, "expected to see image \(busyboxTagged) tagged")

// 3. save the image and output to stdout
let saveArgs = [
"image",
"save",
alpineTagged,
busyboxTagged,
]
let (stdoutData, _, error, status) = try run(arguments: saveArgs)
if status != 0 {
throw CLIError.executionFailed("command failed: \(error)")
}

// 4. remove the image through container
try doRemoveImages(images: [alpineTagged, busyboxTagged])

// 5. verify image is no longer present
let alpineImageRemoved = try !isImagePresent(targetImage: alpineTagged)
#expect(alpineImageRemoved, "expected image \(alpineTagged) to be removed")
let busyboxImageRemoved = try !isImagePresent(targetImage: busyboxTagged)
#expect(busyboxImageRemoved, "expected image \(busyboxTagged) to be removed")

// 6. load the tarball from the stdout data as stdin
let loadArgs = [
"image",
"load",
]
let (_, _, loadErr, loadStatus) = try run(arguments: loadArgs, stdin: stdoutData)
if loadStatus != 0 {
throw CLIError.executionFailed("command failed: \(loadErr)")
}

// 7. verify image is in the list again
let alpineImagePresent = try isImagePresent(targetImage: alpineTagged)
#expect(alpineImagePresent, "expected \(alpineTagged) to be present")
let busyboxImagePresent = try isImagePresent(targetImage: busyboxTagged)
#expect(busyboxImagePresent, "expected \(busyboxTagged) to be present")
} catch {
Issue.record("failed to save and load image \(error)")
return
}
}
}