Protect existing VMs during clone (#1331)

* fix(clone): protect existing VMs from overwrite

* fix(clone): preserve incomplete destination directories
This commit is contained in:
RKS
2026-09-28 12:58:03 -07:00
committed by GitHub
parent 5c1c6bd315
commit e92c3b900f
3 changed files with 78 additions and 2 deletions
+14
View File
@@ -34,6 +34,9 @@ struct Clone: AsyncParsableCommand {
@Flag(help: "create a stacked disk that uses the source image as an immutable base")
var stacked: Bool = false
@Flag(help: "overwrite an existing local VM")
var overwrite: Bool = false
@Option(help: ArgumentHelp("limit automatic pruning to n gigabytes", valueName: "n"))
var pruneLimit: UInt = 100
@@ -52,6 +55,8 @@ struct Clone: AsyncParsableCommand {
let localStorage = try VMStorageLocal()
let remoteName = try? RemoteName(sourceName)
try rejectExistingDestination(localStorage)
if stacked {
guard remoteName != nil else {
throw ValidationError("--stacked requires a remote image")
@@ -98,6 +103,8 @@ struct Clone: AsyncParsableCommand {
let lock = try FileLock(lockURL: Config().tartHomeDir)
try lock.lock()
try rejectExistingDestination(localStorage)
let sourceState = try sourceVM.state()
let generateMAC = try localStorage.hasVMsWithMACAddress(macAddress: sourceVM.macAddress())
&& sourceState != .Suspended
@@ -151,4 +158,11 @@ struct Clone: AsyncParsableCommand {
try? tmpVMDir.removeFromDisk()
})
}
private func rejectExistingDestination(_ localStorage: VMStorageLocal) throws {
let destinationURL = localStorage.baseURL.appendingPathComponent(newName, isDirectory: true)
if !overwrite && FileManager.default.fileExists(atPath: destinationURL.path) {
throw ValidationError("VM \"\(newName)\" already exists, use --overwrite to replace it")
}
}
}
@@ -122,6 +122,60 @@ final class CommandBehaviorTests: XCTestCase {
XCTAssertFalse(RuntimeError.VMIsRunning("running").isFileNotFound())
}
func testCloneRejectsExistingDestinationUnlessOverwriteIsRequested() async throws {
try await withTemporaryTartHome {
let source = try makeStandaloneVM(named: "source", diskContents: "source")
let destination = try makeStandaloneVM(named: "destination", diskContents: "existing")
let command = try Clone.parseAsRoot(["source", "destination"]) as! Clone
do {
try await command.run()
XCTFail("expected cloning over an existing VM to be rejected")
} catch let error as ValidationError {
XCTAssertEqual(error.message, "VM \"destination\" already exists, use --overwrite to replace it")
}
XCTAssertEqual(try Data(contentsOf: destination.diskURL), Data("existing".utf8))
XCTAssertTrue(FileManager.default.fileExists(atPath: source.diskURL.path))
let overwriteCommand = try Clone.parseAsRoot(["--overwrite", "source", "destination"]) as! Clone
try await overwriteCommand.run()
XCTAssertEqual(try Data(contentsOf: destination.diskURL), Data("source".utf8))
}
}
func testClonePreservesIncompleteDestination() async throws {
try await withTemporaryTartHome {
_ = try makeStandaloneVM(named: "source", diskContents: "source")
let storage = try VMStorageLocal()
let destination = storage.baseURL.appendingPathComponent("destination", isDirectory: true)
try FileManager.default.createDirectory(at: destination, withIntermediateDirectories: true)
let diskURL = destination.appendingPathComponent("disk.img")
try Data("existing".utf8).write(to: diskURL)
XCTAssertFalse(storage.exists("destination"))
// Check both a local source and rejection before any remote registry access.
for source in ["source", "invalid.invalid/image:latest"] {
let command = try Clone.parseAsRoot([source, "destination"]) as! Clone
do {
try await command.run()
XCTFail("expected an incomplete destination to be preserved")
} catch let error as ValidationError {
XCTAssertEqual(error.message, "VM \"destination\" already exists, use --overwrite to replace it")
}
XCTAssertEqual(try Data(contentsOf: diskURL), Data("existing".utf8))
XCTAssertFalse(storage.exists("destination"))
}
let command = try Clone.parseAsRoot(["--overwrite", "source", "destination"]) as! Clone
try await command.run()
XCTAssertEqual(try Data(contentsOf: diskURL), Data("source".utf8))
XCTAssertTrue(storage.exists("destination"))
}
}
func testSetDiskRejectsStackedVMBeforeSavingConfig() async throws {
try await withTemporaryTartHome {
let vmDir = try VMStorageLocal().create("stacked")
@@ -208,6 +262,14 @@ final class CommandBehaviorTests: XCTestCase {
)
}
private func makeStandaloneVM(named name: String, diskContents: String) throws -> VMDirectory {
let vmDir = try VMStorageLocal().create(name)
try config().save(toURL: vmDir.configURL)
XCTAssertTrue(FileManager.default.createFile(atPath: vmDir.nvramURL.path, contents: Data()))
XCTAssertTrue(FileManager.default.createFile(atPath: vmDir.diskURL.path, contents: Data(diskContents.utf8)))
return vmDir
}
private func temporaryEntries() throws -> [URL] {
try FileManager.default.contentsOfDirectory(
at: Config().tartTmpDir,
+2 -2
View File
@@ -46,7 +46,7 @@ func TestClonePreservesRunningDestination(t *testing.T) {
t.Fatal(err)
}
for range 2 {
_, stderr, err := tart.Tart(t, "clone", "source", "destination")
_, stderr, err := tart.Tart(t, "clone", "--overwrite", "source", "destination")
if err == nil || !strings.Contains(strings.ToLower(stderr), "running") {
t.Fatalf("clone must reject the running destination: %v: %s", err, stderr)
}
@@ -64,7 +64,7 @@ func TestClonePreservesRunningDestination(t *testing.T) {
if err := syscall.FcntlFlock(held.Fd(), syscall.F_SETLK, &lock); err != nil {
t.Fatal(err)
}
if _, stderr, err := tart.Tart(t, "clone", "source", "destination"); err != nil {
if _, stderr, err := tart.Tart(t, "clone", "--overwrite", "source", "destination"); err != nil {
t.Fatalf("clone after shutdown: %v: %s", err, stderr)
}
currentInfo, err := os.Stat(config)