From 86d31dbd944980d28d0a644957de5c570ba8020e Mon Sep 17 00:00:00 2001 From: bryanm Date: Thu, 15 Oct 2026 14:24:30 -0700 Subject: [PATCH] Treat a missing log file as empty in container logs (#2393) - Fixes #2392. - `ContainersService.logs(id:)` opened `stdio.log` and `vminitd.log` eagerly, so either one missing failed the whole request. Both are created by `RuntimeLinux` at bootstrap, so a container that has never been started has neither. And only `RuntimeLinux` writes `vminitd.log`, so with another runtime plugin even plain `container logs` failed. - A missing log now reads as empty: it's opened as `/dev/null`, with a debug log line. Any other failure to open a log still throws. Co-authored-by: Bryan Mosley <342731628+bryanmosley@users.noreply.github.com> --- .../Server/Containers/ContainersService.swift | 50 +++++++- .../LogFileTests.swift | 120 ++++++++++++++++++ 2 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 Tests/ContainerAPIServiceTests/LogFileTests.swift diff --git a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift index 42e22895..b947212f 100644 --- a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift +++ b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift @@ -729,16 +729,19 @@ public actor ContainersService { } // Logs doesn't care if the container is running or not, just that - // the bundle is there, and that the files actually exist. We do + // the bundle is there; a log file that doesn't exist reads as empty. We do // first try and get the container state so we get a nicer error message // (container foo not found) however. do { _ = try _getContainerState(id: id) let path = self.containerRoot.appendingPathComponent(id) let bundle = ContainerResource.Bundle(path: path) + // A missing log means there are no messages yet, not that the bundle is broken: the + // runtime creates both at bootstrap, so a container that has never been started has + // neither. return [ - try FileHandle(forReadingFrom: bundle.containerLog), - try FileHandle(forReadingFrom: bundle.bootlog), + try self.openLog(bundle.containerLog, of: id), + try self.openLog(bundle.bootlog, of: id), ] } catch { throw ContainerizationError( @@ -748,6 +751,47 @@ public actor ContainersService { } } + /// The log at `url`, or an empty stand-in when it doesn't exist. Other failures throw. + private func openLog(_ url: URL, of id: String) throws -> FileHandle { + if let handle = try Self.openIfPresent(url) { + return handle + } + log.debug( + "ContainersService: log not present, returning it empty", + metadata: ["id": "\(id)", "log": "\(url.lastPathComponent)"] + ) + return try Self.emptyLog() + } + + /// A stand-in for a log that doesn't exist: `/dev/null` opened for reading. + /// + /// Not `FileHandle.nullDevice`, which has no file descriptor (`fileDescriptor` is -1): these + /// handles go to the client over XPC, a -1 dups back as -1, and the client's + /// `fileHandles(key:)` then returns nil, so `container logs` would still fail. + static func emptyLog() throws -> FileHandle { + let fd = open("/dev/null", O_RDONLY | O_CLOEXEC | O_NOFOLLOW) + guard fd >= 0 else { + throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + } + return FileHandle(fileDescriptor: fd, closeOnDealloc: true) + } + + /// Opens `url` for reading, or returns nil if it doesn't exist. Any other failure, such + /// as a permissions problem, still throws. + /// + /// `O_NOFOLLOW`: these are fixed paths inside a container's bundle that runtimes create as + /// regular files, so a symlink there is never legitimate, and following one would hand + /// the caller whatever file it points at. A symlink fails with ELOOP rather than ENOENT, + /// so it is refused, not read as an empty log. + static func openIfPresent(_ url: URL) throws -> FileHandle? { + let fd = open(url.path, O_RDONLY | O_CLOEXEC | O_NOFOLLOW) + guard fd >= 0 else { + if errno == ENOENT { return nil } + throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + } + return FileHandle(fileDescriptor: fd, closeOnDealloc: true) + } + /// Copy a file or directory from the host into the container. public func copyIn(id: String, source: String, destination: String, mode: UInt32, createParents: Bool = true) async throws { self.log.debug("\(#function)") diff --git a/Tests/ContainerAPIServiceTests/LogFileTests.swift b/Tests/ContainerAPIServiceTests/LogFileTests.swift new file mode 100644 index 00000000..2967ee35 --- /dev/null +++ b/Tests/ContainerAPIServiceTests/LogFileTests.swift @@ -0,0 +1,120 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerAPIClient +import ContainerXPC +import ContainerizationError +import Foundation +import Testing + +@testable import ContainerAPIService + +/// Opening a container's log files without touching its bundle. +/// +/// `stdio.log` and `vminitd.log` are both created by RuntimeLinux when it bootstraps a +/// container, and only it writes `vminitd.log`, so either can be legitimately absent. +struct LogFileTests { + private func makeBundleDirectory() throws -> URL { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("LogFileTests-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true) + return url + } + + @Test("A missing log is reported as absent, and nothing is created in the bundle") + func missingLog() throws { + let dir = try makeBundleDirectory() + defer { try? FileManager.default.removeItem(at: dir) } + let bootlog = dir.appendingPathComponent("vminitd.log") + + #expect(try ContainersService.openIfPresent(bootlog) == nil) + #expect(!FileManager.default.fileExists(atPath: bootlog.path)) + } + + @Test("An existing log is read as is") + func existingLog() throws { + let dir = try makeBundleDirectory() + defer { try? FileManager.default.removeItem(at: dir) } + let bootlog = dir.appendingPathComponent("vminitd.log") + try Data("booted\n".utf8).write(to: bootlog) + + let handle = try #require(try ContainersService.openIfPresent(bootlog)) + #expect(try handle.readToEnd() == Data("booted\n".utf8)) + } + + @Test("Failures other than a missing file still throw") + func unreadableLog() throws { + let dir = try makeBundleDirectory() + defer { try? FileManager.default.removeItem(at: dir) } + let bootlog = dir.appendingPathComponent("vminitd.log") + try Data("secret\n".utf8).write(to: bootlog) + try FileManager.default.setAttributes([.posixPermissions: 0o000], ofItemAtPath: bootlog.path) + defer { try? FileManager.default.setAttributes([.posixPermissions: 0o644], ofItemAtPath: bootlog.path) } + + #expect(throws: POSIXError.self) { + try ContainersService.openIfPresent(bootlog) + } + } + + @Test("A log that is a symlink is refused, not followed") + func symlinkedLogIsRefused() throws { + let dir = try makeBundleDirectory() + defer { try? FileManager.default.removeItem(at: dir) } + let secret = dir.appendingPathComponent("secret.txt") + try Data("not a log\n".utf8).write(to: secret) + let stdio = dir.appendingPathComponent("stdio.log") + try FileManager.default.createSymbolicLink(at: stdio, withDestinationURL: secret) + + #expect(throws: POSIXError.self) { + try ContainersService.openIfPresent(stdio) + } + } + + @Test("A dangling symlink is an error, not an empty log") + func danglingSymlinkIsAnError() throws { + let dir = try makeBundleDirectory() + defer { try? FileManager.default.removeItem(at: dir) } + let bootlog = dir.appendingPathComponent("vminitd.log") + try FileManager.default.createSymbolicLink( + at: bootlog, withDestinationURL: dir.appendingPathComponent("nowhere")) + + #expect(throws: POSIXError.self) { + try ContainersService.openIfPresent(bootlog) + } + } + + @Test("The stand-in for a missing log reads as empty") + func emptyLogReadsEmpty() throws { + #expect(try ContainersService.emptyLog().readToEnd() == nil) + } + + @Test("The stand-in survives the XPC reply, which FileHandle.nullDevice doesn't") + func emptyLogCrossesXPC() throws { + // logs(id:) replies with [stdio, boot log] over XPC, and the client's fileHandles(key:) + // dups both, returning nil if either fd is -1. Two handles, as in the real reply. + func roundTrip(_ bootLog: FileHandle) throws -> [FileHandle]? { + let message = XPCMessage(route: .containerLogs) + try message.set(key: "logs", value: [try ContainersService.emptyLog(), bootLog]) + return message.fileHandles(key: "logs") + } + + #expect(try roundTrip(try ContainersService.emptyLog())?.count == 2) + // nullDevice has no descriptor: fileDescriptor is -1, it dups to -1, and the client + // gets no handles at all, so `container logs` would still fail. + #expect(FileHandle.nullDevice.fileDescriptor == -1) + #expect(try roundTrip(FileHandle.nullDevice) == nil) + } +}