-
Notifications
You must be signed in to change notification settings - Fork 766
Use FilePath for PublishSocket #1594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,27 +14,148 @@ | |
| // limitations under the License. | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| import ContainerizationError | ||
| import Foundation | ||
| import SystemPackage | ||
|
|
||
| /// Represents a socket that should be published from container to host. | ||
| /// | ||
| /// - Deprecated: New for 1.0.0, path types changed from `URL` to `FilePath`. | ||
| /// - Note: Decoder handles `FilePath` and `URL` for persistent data compatibility; | ||
| /// this compatibility will be removed in a later release. | ||
| public struct PublishSocket: Sendable, Codable { | ||
| /// The path to the socket in the container. | ||
| public var containerPath: URL | ||
| /// Absolute path to the socket inside the container. | ||
| public var containerPath: FilePath | ||
|
|
||
| /// The path where the socket should appear on the host. | ||
| public var hostPath: URL | ||
| /// Absolute path where the socket appears on the host. | ||
| public var hostPath: FilePath | ||
|
|
||
| /// File permissions for the socket on the host. | ||
| public var permissions: FilePermissions? | ||
|
|
||
| /// Creates a `PublishSocket` with validated absolute paths. | ||
| /// | ||
| /// - Parameters: | ||
| /// - containerPath: Absolute path to the socket inside the container. | ||
| /// Must begin with `/`. | ||
| /// - hostPath: Absolute path where the socket appears on the host. | ||
| /// Must begin with `/`. | ||
| /// - permissions: File permissions applied to the socket on the host. | ||
| /// - Throws: `ContainerizationError` with code `.invalidArgument` if | ||
| /// either path is not absolute. | ||
| public init( | ||
| containerPath: URL, | ||
| hostPath: URL, | ||
| containerPath: FilePath, | ||
| hostPath: FilePath, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What constraints/invariants exist on these paths today? If any do exist, should we enforce them in init() so that PublishSocket objects are correct by construction? We should docc the parameters here and if there are constraints, describe them as well. |
||
| permissions: FilePermissions? = nil | ||
| ) { | ||
| ) throws { | ||
| guard containerPath.isAbsolute else { | ||
| throw ContainerizationError( | ||
| .invalidArgument, | ||
| message: "containerPath must be absolute: \(containerPath)" | ||
| ) | ||
| } | ||
| guard hostPath.isAbsolute else { | ||
| throw ContainerizationError( | ||
| .invalidArgument, | ||
| message: "hostPath must be absolute: \(hostPath)" | ||
| ) | ||
| } | ||
| self.containerPath = containerPath | ||
| self.hostPath = hostPath | ||
| self.permissions = permissions | ||
| } | ||
|
|
||
| private enum CodingKeys: String, CodingKey { | ||
| case containerPath | ||
| case hostPath | ||
| case permissions | ||
| } | ||
|
|
||
| /// Encodes each path as its plain absolute string (e.g. `"/var/run/docker.sock"`). | ||
| /// | ||
| /// Pre-1.0 wire-format change from the prior `URL`-typed encoding which | ||
| /// emitted `URL.absoluteString` (`"file:///var/run/docker.sock"`). The | ||
| /// decoder accepts both forms for compatibility with persisted bundles | ||
| /// from earlier releases; that compatibility will be removed in a later | ||
| /// release. | ||
| public func encode(to encoder: any Encoder) throws { | ||
| var container = encoder.container(keyedBy: CodingKeys.self) | ||
| try container.encode(containerPath.string, forKey: .containerPath) | ||
| try container.encode(hostPath.string, forKey: .hostPath) | ||
| try container.encodeIfPresent(permissions, forKey: .permissions) | ||
| } | ||
|
|
||
| public init(from decoder: any Decoder) throws { | ||
| let container = try decoder.container(keyedBy: CodingKeys.self) | ||
| let containerPath = try Self.decodePath(from: container, forKey: .containerPath) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about enforcing absolute path here (or in decodePath, since it's a private func dedicated to just these paths) as well? Legacy should always be absolute, but we should protect against bad inputs should someone muck about with a config manually.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's a valid path to cover until it's phased out.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 01ec891. |
||
| let hostPath = try Self.decodePath(from: container, forKey: .hostPath) | ||
| let permissions = try container.decodeIfPresent(FilePermissions.self, forKey: .permissions) | ||
| do { | ||
| try self.init( | ||
| containerPath: containerPath, | ||
| hostPath: hostPath, | ||
| permissions: permissions | ||
| ) | ||
| } catch let error as ContainerizationError { | ||
| throw DecodingError.dataCorruptedError( | ||
| forKey: .containerPath, | ||
| in: container, | ||
| debugDescription: String(describing: error) | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /// Decodes a `FilePath` accepting either the new plain-path form | ||
| /// (`"/var/run/docker.sock"`) or the legacy file-URL form emitted by | ||
| /// older releases (`"file:///var/run/docker.sock"`). Throws | ||
| /// `DecodingError.dataCorrupted` on a malformed file URL, empty input, or a | ||
| /// non-absolute path — validating decoded paths here guards against | ||
| /// manually edited or corrupt persisted configs, complementing the | ||
| /// by-construction check in `init(containerPath:hostPath:permissions:)`. | ||
| private static func decodePath( | ||
| from container: KeyedDecodingContainer<CodingKeys>, | ||
| forKey key: CodingKeys | ||
| ) throws -> FilePath { | ||
| let raw = try container.decode(String.self, forKey: key) | ||
|
|
||
| let path: String | ||
| if raw.hasPrefix("file:") { | ||
| guard let url = URL(string: raw), url.isFileURL else { | ||
| throw DecodingError.dataCorruptedError( | ||
| forKey: key, | ||
| in: container, | ||
| debugDescription: "malformed file URL: \(raw)" | ||
| ) | ||
| } | ||
| if let host = url.host(), !host.isEmpty, host != "localhost" { | ||
| throw DecodingError.dataCorruptedError( | ||
| forKey: key, | ||
| in: container, | ||
| debugDescription: "file URL host must be empty or 'localhost': \(raw)" | ||
| ) | ||
| } | ||
| path = url.path(percentEncoded: false) | ||
| } else { | ||
| path = raw | ||
| } | ||
|
|
||
| guard !path.isEmpty else { | ||
| throw DecodingError.dataCorruptedError( | ||
| forKey: key, | ||
| in: container, | ||
| debugDescription: "decoded socket path is empty: \(raw)" | ||
| ) | ||
| } | ||
|
|
||
| let filePath = FilePath(path) | ||
| guard filePath.isAbsolute else { | ||
| throw DecodingError.dataCorruptedError( | ||
| forKey: key, | ||
| in: container, | ||
| debugDescription: "decoded socket path must be absolute: \(raw)" | ||
| ) | ||
| } | ||
|
|
||
| return filePath | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.