Use FilePath for PublishSocket#27
Closed
chrisgeo wants to merge 16 commits into
Closed
Conversation
chrisgeo
force-pushed
the
feat/chaos-1459-publish-socket-filepath
branch
2 times, most recently
from
f8ec45a to
b7299a8
Compare
- Closes apple#1557. - Replaces `executableURL` utility function for getting app executable path with `executablePath`. - Adds `FilePath.resolvingSymlinks()` extension. - Also converts for FilePath for types in `ContainerVersion` target.
…ple#1600) ## Type of Change - [x] Bug fix ## Motivation and Context Many CLI commands need to reference the system configurations for `container`. Previously, CLI commands would try to load the system configurations from the default application root location, regardless of if `container` had been started with a custom application root location. This PR fixes that issue by having each CLI command ping the APIServer's health check service to get the correct app root path. Closes apple#1576 ## Testing - [x] Tested locally Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
## Type of Change - [x] Bug fix ## Motivation and Context Ensure all fields are initialized in the management flags' init function. Without this change, if someone calls init() on this set of flags, they will get an error like ``` Can't read a value from a parsable argument definition. This error indicates that a property declared with an `@Argument`, `@Option`, `@Flag`, or `@OptionGroup` property wrapper was neither initialized to a value nor decoded from command-line arguments. To get a valid value, either call one of the static parsing methods (`parse`, `parseAsRoot`, or `main`) or define an initializer that initializes _every_ property of your parsable type. ``` ## Testing - [x] Tested locally Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
- Fixes apple#1509. The CLI's own help text tells users to run `container help <subcommand>`, but every form of that results in an error. - Added a captured subcommand path, walked Application`'s `subcommands` + `groupedSubcommands` tree (matching `commandName` and `aliases`), and printed `Application.helpMessage(for:)` for the resolved target. Empty path keeps existing plugin-aware top-level help; unknown path throws `ValidationError`.
chrisgeo
force-pushed
the
feat/chaos-1459-publish-socket-filepath
branch
from
b7299a8 to
1e7f495
Compare
…#1608) ## Type of Change - [x] New feature ## Motivation and Context When developing, there are times when I want to run a specific set of CLI tests. This PR allows users to set what integration tests they want to run by setting the makefile variable `INTEGRATION_TEST_SUITES`. Example usage: ``` % INTEGRATION_TEST_SUITES="TestCLIVolumes TestCLIAnonymousVolumes" make all integration ``` ## Testing - [x] Tested locally Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
- Part of apple#1404. - Use `configuration` for configuration properties, `state` for current state label, `status` for status properties.
## Type of Change - [x] New feature ## Motivation and Context Related to apple#1404. This PR adds the initial work to have volume resources conform to ManagedResource, in alignment with other resources such as networks (see [here](apple#1421)). Further work is necessary to move the use of `VolumeResource` down to the APIServer (in the VolumesService) and in the volume client. Volumes do not currently have any plugin or runtime state, so that information is not included in the `VolumeResource`, but could be added later if needed. ## Testing - [x] Tested locally Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
- Closes apple#1610. - Discovered, and originally filed as a security advisory, by: PresidentL <131139636+liyander@users.noreply.github.com>. - `PublishPort` currently can store invalid combinations of starting port and range that can overflow UInt16 values when summed, crashing the process. - Updates `PublishPort` to validate inputs on initialization.
Closes CHAOS-1459. Unblocks CHAOS-1463 (Parser).
- PublishSocket.init now throws and validates absolute paths so objects
are correct by construction; adds DocC for parameters and constraints
(addresses comment on PublishSocket.swift:33 and :72).
- Adds deprecation/migration note on the struct documenting that the
decoder accepts both new `FilePath` and legacy `URL` forms for one
release (addresses comment on PublishSocket.swift:21).
- Wire format pre-1.0 breaking change: encoder now emits the plain
absolute path (e.g. "/var/run/docker.sock") instead of the legacy
file-URL form ("file:///var/run/docker.sock"). The decoder still
accepts both forms so persisted bundles from earlier releases
continue to load; that compatibility will be removed in a later
release (addresses comment on PublishSocket.swift:58).
- Bumps containerization to 0.33.2 and replaces
URL(fileURLWithPath:).absoluteURL.path with
ContainerizationOS.FilePathOps.absolutePath in Parser.publishSocket,
removing the duplicate "must be absolute" check now that init()
enforces it (addresses comments on Parser.swift:754 and :762).
Enforce absoluteness directly in the decode helper (in addition to the by-construction check in init) so decoded PublishSocket paths are lexically correct even when read from manually edited or corrupt persisted configs. Non-absolute decoded paths now throw DecodingError.dataCorrupted at the decode layer. Adds testDecodeRelativeHostPathThrows covering a relative hostPath on decode and clarifies the existing relative-containerPath test. Addresses PR apple#1594 review comments on PublishSocket.swift:90 and PublishSocketTests.swift:99.
chrisgeo
force-pushed
the
feat/chaos-1459-publish-socket-filepath
branch
from
01ec891 to
605408b
Compare
- Part of apple#1404. - Updates containerization to 0.33.2. - Reorganizes network plugin targets into: - `ContainerNetworkClient` - network plugin client and default types - `ContainerNetworkServer` - separate protocols for `Network` which manages the underlying virtual network, `NetworkService`, which takes a network and implements the API, and an actor `NetworkHarness` that marshals between the API and the XPC protocol. The service-harness separation will help us ensure XPC protocol compatibility in both directions as we evolve the plugin APIs. - Removes `disableAllocator()` which is no longer used since apple#1545 switched over to using XPC connections between runtime and network plugin instances to track whether a network has attached containers.
- Refactor network model types: replace `NetworkState` enum and phase-based NetworkStatus with a flat `NetworkStatus` struct. - Simplify API server ↔ plugin protocol: plugin `status()` returns runtime status only, API server owns configuration. - `NetworksService` `list()`/`create()` now return `NetworkResource` directly. - Remove lifecycle phase checks and state machine guards throughout CLI and API server. - `variant` is plugin-specific, it's not a required property. This PR replaces `NetworkPluginInfo` with a `plugin` name property on `NetworkConfiguration` and an `options` list similar to that for volumes. - Moved `variant` to the option list.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PublishSocketfromURLtoSystemPackage.FilePathforcontainerPathandhostPath(4 references in the type itself).init(containerPath: URL, hostPath: URL, …)→init(containerPath: FilePath, hostPath: FilePath, …). Stored properties typed accordingly.permissions: FilePermissions?unchanged.Sources/Services/ContainerAPIService/Client/Parser.swift:URL(fileURLWithPath: …)→FilePath(…)for both arguments. Mounts/volumes parsing in the same file is intentionally untouched — that is CHAOS-1463 territory.Sources/Services/ContainerSandboxService/Server/SandboxService.swift: theContainerization.UnixSocketConfigurationinitializer takesURL, so we convert at the boundary withURL(filePath: publishedSocket.containerPath.string)(and same forhostPath) and add a comment justifying the leak.Closes CHAOS-1459. Sibling of #26 (Bundle), part of CHAOS-1448 epic. Unblocks CHAOS-1463.
Codable wire format — preserved
PublishSocketisCodableand embedded insideContainerConfiguration(alsoCodable), which is persisted to disk in container bundles (Sources/ContainerResource/Container/Bundle.swiftwritesconfig.jsonviaJSONEncoder) and sent over XPC (Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift:195decodes it). Naively switching the field types would change the JSON wire format two ways:JSONEncoderspecial-casesURLto encode asabsoluteString→"file:///var/run/docker.sock".FilePath's synthesizedCodableconformance uses a keyed container →{"_storage":"…"}.Either change would break decoding of existing on-disk container configs. To avoid that, this PR adds an explicit
Codableimplementation onPublishSocket:containerPath.string,hostPath.string). New writes are clean:"hostPath":"/var/run/docker.sock".URL.absoluteStringlike"file:///var/run/docker.sock", detected byfile://prefix and parsed as aURLto recover the path).Net result: existing container bundles persisted before this migration still decode; new bundles produce a cleaner string-only wire format.
Test plan
swift build— full project, 3107/3107 artifacts, no warnings.swift test --filter ContainerAPIClient— 182 tests pass (coversParsertest suite).swift test --filter ContainerResource— 36 tests pass (coversPublishPort/sockets-adjacent types).lsp_diagnosticsclean on all three changed files.--publish-socketand confirmconfig.jsonround-trips on disk."file:///…"strings) and confirm it still decodes.Files changed
Sources/ContainerResource/Container/PublishSocket.swift— types + explicit Codable.Sources/Services/ContainerAPIService/Client/Parser.swift—SystemPackageimport + the twoPublishSocketconstructor arguments. No other changes (volumes/mounts parsing is left for CHAOS-1463).Sources/Services/ContainerSandboxService/Server/SandboxService.swift— boundary conversion toURLforUnixSocketConfiguration(Containerization SDK still takesURL).