From d83f3023bc1a01e1cdf3407d700d5e8273e13313 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 24 Jun 2026 09:33:05 -0700 Subject: [PATCH 01/12] Add --mount flag to container run/create Add a Docker-style --mount option to `wslc container run` and `wslc container create`. The flag accepts comma-separated key=value pairs (type=bind|volume|tmpfs, source/src, target/destination/dst, readonly/ro) and is routed into the existing volume/tmpfs plumbing. - Parse --mount into a ParsedMount (ArgumentValidation) - Register the Mount argument for run/create - Wire parsed mounts into ContainerOptions (ContainerTasks) - Add localization strings (MountArgDescription, InvalidMountError) - Add e2e tests (tmpfs, named volume, readonly-via-inspect, invalid type) and update run/create help-text expectations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- localization/strings/en-US/Resources.resw | 7 ++ .../wslc/arguments/ArgumentDefinitions.h | 1 + .../wslc/arguments/ArgumentValidation.cpp | 7 ++ src/windows/wslc/arguments/SpecParsing.cpp | 89 +++++++++++++++++++ src/windows/wslc/arguments/SpecParsing.h | 10 +++ .../wslc/commands/ContainerCreateCommand.cpp | 1 + .../wslc/commands/ContainerRunCommand.cpp | 1 + src/windows/wslc/tasks/ContainerTasks.cpp | 16 ++++ .../wslc/e2e/WSLCE2EContainerRunTests.cpp | 54 +++++++++++ 9 files changed, 186 insertions(+) diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index d87472d5cd..407a12dd39 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2948,6 +2948,10 @@ On first run, creates the file with all settings commented out at their defaults Invalid argument "{}" for '-f, --filter' flag: bad format of filter (expected name=value) {FixedPlaceholder="{}"}{Locked="--filter'"}Command line arguments, file names and string inserts should not be translated + + Invalid argument "{}" for '--mount' flag: bad format of mount (expected type=bind|volume|tmpfs,source=...,target=...) + {FixedPlaceholder="{}"}{Locked="--mount'"}Command line arguments, file names and string inserts should not be translated + Follow log output @@ -3012,6 +3016,9 @@ On first run, creates the file with all settings commented out at their defaults Memory limit (e.g. 512M, 1G) {Locked="512M"}{Locked="1G"}Command line argument example values should not be translated + + Attach a filesystem mount to the container + Container host name diff --git a/src/windows/wslc/arguments/ArgumentDefinitions.h b/src/windows/wslc/arguments/ArgumentDefinitions.h index 8fbe4bacab..e33435adea 100644 --- a/src/windows/wslc/arguments/ArgumentDefinitions.h +++ b/src/windows/wslc/arguments/ArgumentDefinitions.h @@ -87,6 +87,7 @@ _(Latest, "latest", L"l", Kind::Flag, L _(Link, "link", NO_ALIAS, Kind::Value, Localization::WSLCCLI_LinkArgDescription()) \ _(LinkLocalIp, "link-local-ip", NO_ALIAS, Kind::Value, Localization::WSLCCLI_LinkLocalIpArgDescription()) \ _(Memory, "memory", L"m", Kind::Value, Localization::WSLCCLI_MemoryArgDescription()) \ +_(Mount, "mount", NO_ALIAS, Kind::Value, Localization::WSLCCLI_MountArgDescription()) \ _(Name, "name", NO_ALIAS, Kind::Value, Localization::WSLCCLI_NameArgDescription()) \ _(Network, "network", NO_ALIAS, Kind::Value, Localization::WSLCCLI_NetworkArgDescription()) \ _(NetworkAlias, "network-alias", NO_ALIAS, Kind::Value, Localization::WSLCCLI_NetworkAliasArgDescription()) \ diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp index cca569d9b3..be4676898a 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.cpp +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -132,6 +132,13 @@ void Argument::Validate(const ArgMap& execArgs) const validation::ValidateVolumeMount(execArgs.GetAll()); break; + case ArgType::Mount: + for (const auto& value : execArgs.GetAll()) + { + std::ignore = validation::ParseMount(value); + } + break; + case ArgType::WorkDir: { const auto& value = execArgs.Get(); diff --git a/src/windows/wslc/arguments/SpecParsing.cpp b/src/windows/wslc/arguments/SpecParsing.cpp index 42fa3939f9..3e148c11de 100644 --- a/src/windows/wslc/arguments/SpecParsing.cpp +++ b/src/windows/wslc/arguments/SpecParsing.cpp @@ -457,6 +457,95 @@ std::pair ParseFilter(const std::wstring& value) return {WideToMultiByte(kv.Key), WideToMultiByte(kv.Value)}; } +ParsedMount ParseMount(const std::wstring& value) +{ + std::wstring type; + std::wstring source; + std::wstring target; + bool readOnly = false; + + const auto fail = [&]() { throw ArgumentException(Localization::WSLCCLI_InvalidMountError(value)); }; + + size_t position = 0; + while (position < value.size()) + { + const auto comma = value.find(L',', position); + const auto token = value.substr(position, comma == std::wstring::npos ? std::wstring::npos : comma - position); + position = comma == std::wstring::npos ? value.size() : comma + 1; + + if (token.empty()) + { + continue; + } + + const auto keyValue = SplitKeyValue(token); + if (IsEqual(keyValue.Key, L"type", true)) + { + type = keyValue.Value; + } + else if (IsEqual(keyValue.Key, L"source", true) || IsEqual(keyValue.Key, L"src", true)) + { + source = keyValue.Value; + } + else if ( + IsEqual(keyValue.Key, L"target", true) || IsEqual(keyValue.Key, L"destination", true) || IsEqual(keyValue.Key, L"dst", true)) + { + target = keyValue.Value; + } + else if (IsEqual(keyValue.Key, L"readonly", true) || IsEqual(keyValue.Key, L"ro", true)) + { + readOnly = keyValue.Value.empty() || IsEqual(keyValue.Value, L"true", true) || IsEqual(keyValue.Value, L"1", true); + } + else + { + fail(); + } + } + + if (type.empty()) + { + type = L"volume"; + } + + if (target.empty()) + { + fail(); + } + + ParsedMount result; + if (IsEqual(type, L"tmpfs", true)) + { + if (!source.empty()) + { + fail(); + } + + result.IsTmpfs = true; + result.TmpfsSpec = WideToMultiByte(target); + } + else if (IsEqual(type, L"bind", true) || IsEqual(type, L"volume", true)) + { + if (source.empty()) + { + fail(); + } + + result.VolumeSpec = source + L":" + target; + if (readOnly) + { + result.VolumeSpec += L":ro"; + } + + std::ignore = models::VolumeMount::Parse(result.VolumeSpec); + } + else + { + fail(); + } + + return result; +} + // Map of signal names to WSLCSignal enum values static const std::unordered_map SignalMap = { {L"SIGHUP", WSLCSignalSIGHUP}, {L"SIGINT", WSLCSignalSIGINT}, {L"SIGQUIT", WSLCSignalSIGQUIT}, diff --git a/src/windows/wslc/arguments/SpecParsing.h b/src/windows/wslc/arguments/SpecParsing.h index bb093c46de..6019dc8036 100644 --- a/src/windows/wslc/arguments/SpecParsing.h +++ b/src/windows/wslc/arguments/SpecParsing.h @@ -72,6 +72,16 @@ std::pair ParseDriverOption(const std::wstring& value) // Parses a --filter spec ("key=value"); the separator is required. std::pair ParseFilter(const std::wstring& value); +struct ParsedMount +{ + bool IsTmpfs = false; + std::wstring VolumeSpec; + std::string TmpfsSpec; +}; + +// Parses a Docker-style --mount spec into the existing volume or tmpfs representation. +ParsedMount ParseMount(const std::wstring& value); + // Parses a signal by name ("SIGKILL"/"KILL", case-insensitive) or number ("9") into a WSLCSignal. WSLCSignal GetWSLCSignalFromString(const std::wstring& input, const std::wstring& argName = {}); diff --git a/src/windows/wslc/commands/ContainerCreateCommand.cpp b/src/windows/wslc/commands/ContainerCreateCommand.cpp index 8292a692d9..3e7f2dc823 100644 --- a/src/windows/wslc/commands/ContainerCreateCommand.cpp +++ b/src/windows/wslc/commands/ContainerCreateCommand.cpp @@ -52,6 +52,7 @@ std::vector ContainerCreateCommand::GetArguments() const Argument::Create(ArgType::Interactive), Argument::Create(ArgType::Label, false, Limit::Unlimited), Argument::Create(ArgType::Memory), + Argument::Create(ArgType::Mount, false, NO_LIMIT), Argument::Create(ArgType::Name), Argument::Create(ArgType::Network, false, Limit::Unlimited), Argument::Create(ArgType::NetworkAlias, false, Limit::Unlimited), diff --git a/src/windows/wslc/commands/ContainerRunCommand.cpp b/src/windows/wslc/commands/ContainerRunCommand.cpp index 0babfc4a26..f563a76403 100644 --- a/src/windows/wslc/commands/ContainerRunCommand.cpp +++ b/src/windows/wslc/commands/ContainerRunCommand.cpp @@ -52,6 +52,7 @@ std::vector ContainerRunCommand::GetArguments() const Argument::Create(ArgType::Interactive), Argument::Create(ArgType::Label, false, Limit::Unlimited), Argument::Create(ArgType::Memory), + Argument::Create(ArgType::Mount, false, NO_LIMIT), Argument::Create(ArgType::Name), Argument::Create(ArgType::Network, false, Limit::Unlimited), Argument::Create(ArgType::NetworkAlias, false, Limit::Unlimited), diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index 8d0c3c00ed..9bbeed862c 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -708,6 +708,22 @@ void SetContainerOptionsFromArgs(CLIExecutionContext& context) } } + if (context.Args.Contains(ArgType::Mount)) + { + for (const auto& value : context.Args.GetAll()) + { + auto parsed = validation::ParseMount(value); + if (parsed.IsTmpfs) + { + options.Tmpfs.emplace_back(std::move(parsed.TmpfsSpec)); + } + else + { + options.Volumes.emplace_back(std::move(parsed.VolumeSpec)); + } + } + } + if (context.Args.GetFlag()) { options.Remove = true; diff --git a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp index 74ac853a64..227df84137 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp @@ -999,6 +999,60 @@ class WSLCE2EContainerRunTests result.Verify({.Stderr = L"", .ExitCode = 0}); } + WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_Tmpfs_Success) + { + auto result = RunWslc(std::format( + L"container run --rm --mount type=tmpfs,target=/wslc-tmpfs {} sh -c \"echo -n 'tmpfs_test' > /wslc-tmpfs/data && cat " + L"/wslc-tmpfs/data\"", + DebianImage.NameAndTag())); + result.Verify({.Stdout = L"tmpfs_test", .Stderr = L"", .ExitCode = 0}); + } + + WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_Volume_Success) + { + auto result = RunWslc(std::format( + L"container run --rm --mount type=volume,source={},target=/data {} sh -c \"echo -n 'WSLC Mount Volume Test' > " + L"/data/test.txt\"", + WslcVolumeName, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = 0}); + + result = RunWslc(std::format( + L"container run --rm --mount type=volume,source={},target=/data {} cat /data/test.txt", WslcVolumeName, DebianImage.NameAndTag())); + result.Verify({.Stdout = L"WSLC Mount Volume Test", .Stderr = L"", .ExitCode = 0}); + } + + WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_ReadOnly_IsReadOnly) + { + auto result = RunWslc(std::format( + L"container run --name {} --mount type=volume,source={},target=/data,readonly {} echo ok", + WslcContainerName, + WslcVolumeName, + DebianImage.NameAndTag())); + result.Verify({.Stdout = L"ok\n", .Stderr = L"", .ExitCode = 0}); + + const auto inspect = InspectContainer(WslcContainerName); + bool found = false; + for (const auto& mount : inspect.Mounts) + { + if (mount.Destination == "/data") + { + found = true; + VERIFY_IS_FALSE(mount.ReadWrite); + } + } + + VERIFY_IS_TRUE(found); + } + + WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_InvalidType_Fails) + { + auto result = RunWslc(std::format(L"container run --rm --mount type=bogus,target=/x {} true", DebianImage.NameAndTag())); + VERIFY_ARE_EQUAL(1u, result.ExitCode.value()); + VERIFY_IS_TRUE(result.Stderr.has_value()); + VERIFY_ARE_NOT_EQUAL(std::wstring::npos, result.Stderr->find(L"for '--mount' flag")); + } + WSLC_TEST_METHOD(WSLCE2E_Container_Run_WithLabel_Success) { auto result = RunWslc(std::format( From 5b413d0818453e375d8c6542071c17c280f12b95 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 12 Aug 2026 11:02:53 -0700 Subject: [PATCH 02/12] Update and refactor --- localization/strings/en-US/Resources.resw | 8 +- .../wslc/arguments/ArgumentValidation.cpp | 1 + .../wslc/arguments/MountSpecParsing.cpp | 500 ++++++++++++++++++ src/windows/wslc/arguments/MountSpecParsing.h | 30 ++ src/windows/wslc/arguments/SpecParsing.cpp | 90 +--- src/windows/wslc/arguments/SpecParsing.h | 10 - .../wslc/commands/ContainerCreateCommand.cpp | 2 +- .../wslc/commands/ContainerRunCommand.cpp | 2 +- src/windows/wslc/services/ContainerModel.cpp | 74 +++ src/windows/wslc/services/ContainerModel.h | 2 + src/windows/wslc/tasks/ContainerTasks.cpp | 3 + .../wslc/WSLCCLIMountParserUnitTests.cpp | 194 +++++++ .../wslc/e2e/WSLCE2EContainerRunTests.cpp | 35 +- 13 files changed, 834 insertions(+), 117 deletions(-) create mode 100644 src/windows/wslc/arguments/MountSpecParsing.cpp create mode 100644 src/windows/wslc/arguments/MountSpecParsing.h create mode 100644 test/windows/wslc/WSLCCLIMountParserUnitTests.cpp diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index 407a12dd39..9acf0b7442 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2949,8 +2949,12 @@ On first run, creates the file with all settings commented out at their defaults Invalid argument "{}" for '-f, --filter' flag: bad format of filter (expected name=value) {FixedPlaceholder="{}"}{Locked="--filter'"}Command line arguments, file names and string inserts should not be translated - Invalid argument "{}" for '--mount' flag: bad format of mount (expected type=bind|volume|tmpfs,source=...,target=...) - {FixedPlaceholder="{}"}{Locked="--mount'"}Command line arguments, file names and string inserts should not be translated + Invalid argument "{}" for '--mount' flag: {} + {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}{Locked="--mount'"}Command line arguments, file names and string inserts should not be translated + + + Duplicate mount point: {} + {FixedPlaceholder="{}"}File names and string inserts should not be translated Follow log output diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp index be4676898a..08dc1decef 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.cpp +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -20,6 +20,7 @@ Module Name: #include "Exceptions.h" #include "ImageService.h" #include "Localization.h" +#include "MountSpecParsing.h" #include #include diff --git a/src/windows/wslc/arguments/MountSpecParsing.cpp b/src/windows/wslc/arguments/MountSpecParsing.cpp new file mode 100644 index 0000000000..0c389c84d3 --- /dev/null +++ b/src/windows/wslc/arguments/MountSpecParsing.cpp @@ -0,0 +1,500 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + MountSpecParsing.cpp + +Abstract: + + Parser for Docker-style --mount specifications. + +--*/ + +#include "precomp.h" +#include "MountSpecParsing.h" +#include "ContainerModel.h" +#include "Exceptions.h" +#include "Localization.h" +#include "SpecParsing.h" +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace wsl::windows::common; +using namespace wsl::shared; +using namespace wsl::shared::string; + +namespace wsl::windows::wslc::validation { + +namespace { + + struct DockerMountSpec + { + std::wstring Type = L"volume"; + std::wstring Source; + std::wstring Target; + bool ReadOnly = false; + bool HasVolumeOptions = false; + bool HasBindOptions = false; + bool HasTmpfsOptions = false; + bool BindReadOnlyNonRecursive = false; + bool BindReadOnlyForceRecursive = false; + std::wstring BindPropagation; + std::optional TmpfsSizeBytes; + std::optional TmpfsMode; + std::optional UnsupportedOption; + }; + + [[noreturn]] void ThrowInvalidMount(const std::wstring& spec, const std::wstring& reason) + { + throw ArgumentException(Localization::WSLCCLI_InvalidMountError(spec, reason)); + } + + void RecordUnsupportedOption(DockerMountSpec& mount, const std::wstring& option) + { + if (!mount.UnsupportedOption.has_value()) + { + mount.UnsupportedOption = option; + } + } + + std::optional ParseDockerRamInBytes(const std::wstring& value) + { + const auto input = WideToMultiByte(value); + const auto separator = input.find_last_of("01234567890. "); + if (separator == std::string::npos) + { + return std::nullopt; + } + + std::string number; + std::string suffix; + if (input[separator] == ' ') + { + number = input.substr(0, separator); + suffix = input.substr(separator + 1); + } + else + { + number = input.substr(0, separator + 1); + suffix = input.substr(separator + 1); + } + + if (number.empty() || std::isspace(static_cast(number.front()))) + { + return std::nullopt; + } + + std::string_view numberView(number); + if (numberView.front() == '+') + { + numberView.remove_prefix(1); + if (numberView.empty()) + { + return std::nullopt; + } + } + + double parsed{}; + const auto parseResult = + std::from_chars(numberView.data(), numberView.data() + numberView.size(), parsed, std::chars_format::general); + if (parseResult.ec != std::errc() || parseResult.ptr != numberView.data() + numberView.size() || !std::isfinite(parsed) || parsed < 0) + { + return std::nullopt; + } + + double bytes = parsed; + if (!suffix.empty()) + { + suffix = AsciiToLower(std::string_view(suffix)); + if (suffix.size() > 3) + { + return std::nullopt; + } + + if (suffix.front() == 'b') + { + if (suffix.size() != 1) + { + return std::nullopt; + } + } + else + { + uint64_t factor{}; + switch (suffix.front()) + { + case 'k': + factor = 1ULL << 10; + break; + case 'm': + factor = 1ULL << 20; + break; + case 'g': + factor = 1ULL << 30; + break; + case 't': + factor = 1ULL << 40; + break; + case 'p': + factor = 1ULL << 50; + break; + default: + return std::nullopt; + } + + if ((suffix.size() == 2 && suffix[1] != 'b') || (suffix.size() == 3 && suffix.substr(1) != "ib")) + { + return std::nullopt; + } + + bytes *= static_cast(factor); + } + } + + constexpr double c_int64Limit = 9223372036854775808.0; + if (!std::isfinite(bytes) || bytes >= c_int64Limit) + { + return std::nullopt; + } + + return static_cast(bytes); + } + + std::optional ParseDockerTmpfsMode(const std::wstring& value) + { + if (value.empty() || value.front() == L'-') + { + return std::nullopt; + } + + size_t position = value.front() == L'+' ? 1 : 0; + if (position == value.size()) + { + return std::nullopt; + } + + uint64_t result = 0; + for (; position < value.size(); ++position) + { + const auto digit = value[position]; + if (digit < L'0' || digit > L'7') + { + return std::nullopt; + } + + result = (result * 8) + static_cast(digit - L'0'); + if (result > std::numeric_limits::max()) + { + return std::nullopt; + } + } + + return static_cast(result); + } + + std::string FormatDockerTmpfsSize(int64_t sizeBytes) + { + for (const auto& [suffix, divisor] : std::array, 3>{{{'g', 1LL << 30}, {'m', 1LL << 20}, {'k', 1LL << 10}}}) + { + if ((sizeBytes % divisor) == 0) + { + return std::format("{}{}", sizeBytes / divisor, suffix); + } + } + + return std::to_string(sizeBytes); + } + +} // namespace + +ParsedMount ParseMount(const std::wstring& value) +{ + // Keep this parser aligned with docker/cli v25.0.3 opts/mount.go. If the bundled Docker + // backend is updated, revisit both the parsing rules and the WSLC support gate below. + const auto fields = SplitCsvFields(value); + if (!fields.has_value()) + { + ThrowInvalidMount(value, L"malformed CSV"); + } + + DockerMountSpec mount; + + for (const auto& field : *fields) + { + const auto keyValue = SplitKeyValue(field); + const auto key = AsciiToLower(std::wstring_view(keyValue.Key)); + + if (!keyValue.HadSeparator) + { + if (key == L"readonly" || key == L"ro") + { + mount.ReadOnly = true; + continue; + } + + if (key == L"volume-nocopy") + { + mount.HasVolumeOptions = true; + RecordUnsupportedOption(mount, key); + continue; + } + + if (key == L"bind-nonrecursive") + { + mount.HasBindOptions = true; + RecordUnsupportedOption(mount, key); + continue; + } + + ThrowInvalidMount(value, std::format(L"invalid field '{}' must be a key=value pair", field)); + } + + if (key == L"type") + { + mount.Type = AsciiToLower(std::wstring_view(keyValue.Value)); + } + else if (key == L"source" || key == L"src") + { + mount.Source = keyValue.Value; + if (mount.Source == L"." || mount.Source.starts_with(L".\\")) + { + std::error_code error; + auto absolutePath = std::filesystem::absolute(mount.Source, error); + if (!error) + { + mount.Source = absolutePath.lexically_normal().wstring(); + } + } + } + else if (key == L"target" || key == L"dst" || key == L"destination") + { + mount.Target = keyValue.Value; + } + else if (key == L"readonly" || key == L"ro") + { + const auto parsed = ParseBool(keyValue.Value.c_str(), true); + if (!parsed.has_value()) + { + ThrowInvalidMount(value, std::format(L"invalid value for {}: {}", key, keyValue.Value)); + } + + mount.ReadOnly = parsed.value(); + } + else if (key == L"consistency") + { + RecordUnsupportedOption(mount, key); + } + else if (key == L"bind-propagation") + { + mount.HasBindOptions = true; + mount.BindPropagation = AsciiToLower(std::wstring_view(keyValue.Value)); + RecordUnsupportedOption(mount, key); + } + else if (key == L"bind-nonrecursive") + { + if (!ParseBool(keyValue.Value.c_str(), true).has_value()) + { + ThrowInvalidMount(value, std::format(L"invalid value for {}: {}", key, keyValue.Value)); + } + + mount.HasBindOptions = true; + RecordUnsupportedOption(mount, key); + } + else if (key == L"bind-recursive") + { + if (keyValue.Value == L"enabled") + { + continue; + } + + mount.HasBindOptions = true; + RecordUnsupportedOption(mount, key); + if (keyValue.Value == L"disabled") + { + continue; + } + if (keyValue.Value == L"writable") + { + mount.BindReadOnlyNonRecursive = true; + continue; + } + if (keyValue.Value == L"readonly") + { + mount.BindReadOnlyForceRecursive = true; + continue; + } + + ThrowInvalidMount( + value, + std::format( + L"invalid value for {}: {} (must be \"enabled\", \"disabled\", \"writable\", or \"readonly\")", key, keyValue.Value)); + } + else if (key == L"volume-nocopy") + { + if (!ParseBool(keyValue.Value.c_str(), true).has_value()) + { + ThrowInvalidMount(value, std::format(L"invalid value for volume-nocopy: {}", keyValue.Value)); + } + + mount.HasVolumeOptions = true; + RecordUnsupportedOption(mount, key); + } + else if (key == L"volume-label" || key == L"volume-driver" || key == L"volume-opt") + { + mount.HasVolumeOptions = true; + RecordUnsupportedOption(mount, key); + } + else if (key == L"tmpfs-size") + { + mount.TmpfsSizeBytes = ParseDockerRamInBytes(keyValue.Value); + if (!mount.TmpfsSizeBytes.has_value()) + { + ThrowInvalidMount(value, std::format(L"invalid value for {}: {}", key, keyValue.Value)); + } + + mount.HasTmpfsOptions = true; + } + else if (key == L"tmpfs-mode") + { + mount.TmpfsMode = ParseDockerTmpfsMode(keyValue.Value); + if (!mount.TmpfsMode.has_value()) + { + ThrowInvalidMount(value, std::format(L"invalid value for {}: {}", key, keyValue.Value)); + } + + mount.HasTmpfsOptions = true; + } + else + { + ThrowInvalidMount(value, std::format(L"unexpected key '{}' in '{}'", key, field)); + } + } + + if (mount.Type.empty()) + { + ThrowInvalidMount(value, L"type is required"); + } + + if (mount.Target.empty()) + { + ThrowInvalidMount(value, L"target is required"); + } + + if (mount.HasVolumeOptions && mount.Type != L"volume") + { + ThrowInvalidMount(value, std::format(L"cannot mix 'volume-*' options with mount type '{}'", mount.Type)); + } + if (mount.HasBindOptions && mount.Type != L"bind") + { + ThrowInvalidMount(value, std::format(L"cannot mix 'bind-*' options with mount type '{}'", mount.Type)); + } + if (mount.HasTmpfsOptions && mount.Type != L"tmpfs") + { + ThrowInvalidMount(value, std::format(L"cannot mix 'tmpfs-*' options with mount type '{}'", mount.Type)); + } + + if (mount.BindReadOnlyNonRecursive && !mount.ReadOnly) + { + ThrowInvalidMount(value, L"option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction"); + } + if (mount.BindReadOnlyForceRecursive) + { + if (!mount.ReadOnly) + { + ThrowInvalidMount(value, L"option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction"); + } + if (mount.BindPropagation != L"rprivate") + { + ThrowInvalidMount( + value, L"option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction"); + } + } + + if (mount.Type != L"bind" && mount.Type != L"volume" && mount.Type != L"tmpfs") + { + ThrowInvalidMount(value, std::format(L"mount type '{}' is not supported by WSLC", mount.Type)); + } + if (mount.UnsupportedOption.has_value()) + { + ThrowInvalidMount(value, std::format(L"option '{}' is not supported by WSLC", mount.UnsupportedOption.value())); + } + if (mount.Target.find(L':') != std::wstring::npos) + { + ThrowInvalidMount(value, L"target paths containing ':' are not supported by WSLC"); + } + + ParsedMount result; + if (mount.Type == L"tmpfs") + { + if (!mount.Source.empty()) + { + ThrowInvalidMount(value, L"source is not supported for tmpfs mounts"); + } + + std::vector options; + if (mount.ReadOnly) + { + options.emplace_back("ro"); + } + if (mount.TmpfsMode.has_value() && mount.TmpfsMode.value() != 0) + { + options.emplace_back(std::format("mode={:o}", mount.TmpfsMode.value())); + } + if (mount.TmpfsSizeBytes.has_value() && mount.TmpfsSizeBytes.value() != 0) + { + options.emplace_back(std::format("size={}", FormatDockerTmpfsSize(mount.TmpfsSizeBytes.value()))); + } + + result.IsTmpfs = true; + result.TmpfsSpec = WideToMultiByte(mount.Target); + if (!options.empty()) + { + result.TmpfsSpec += ":" + wsl::shared::string::Join(options, ','); + } + } + else + { + if (mount.Source.empty()) + { + if (mount.Type == L"volume") + { + ThrowInvalidMount(value, L"anonymous volume mounts are not supported by WSLC"); + } + + ThrowInvalidMount(value, L"source is required"); + } + + if (mount.Type == L"bind" && !std::filesystem::path(mount.Source).is_absolute()) + { + ThrowInvalidMount(value, L"bind source path must be absolute"); + } + if (mount.Type == L"volume" && !models::VolumeMount::IsValidNamedVolumeName(mount.Source)) + { + ThrowInvalidMount(value, L"volume source must be a valid named volume"); + } + + result.VolumeSpec = mount.Source + L":" + mount.Target; + if (mount.ReadOnly) + { + result.VolumeSpec += L":ro"; + } + + const auto parsed = models::VolumeMount::Parse(result.VolumeSpec); + if ((mount.Type == L"bind" && parsed.IsNamedVolume()) || (mount.Type == L"volume" && !parsed.IsNamedVolume())) + { + ThrowInvalidMount(value, std::format(L"source is not valid for mount type '{}'", mount.Type)); + } + } + + return result; +} + +} // namespace wsl::windows::wslc::validation diff --git a/src/windows/wslc/arguments/MountSpecParsing.h b/src/windows/wslc/arguments/MountSpecParsing.h new file mode 100644 index 0000000000..7ea931cd31 --- /dev/null +++ b/src/windows/wslc/arguments/MountSpecParsing.h @@ -0,0 +1,30 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + MountSpecParsing.h + +Abstract: + + Declarations for parsing Docker-style --mount specifications. + +--*/ +#pragma once + +#include + +namespace wsl::windows::wslc::validation { + +struct ParsedMount +{ + bool IsTmpfs = false; + std::wstring VolumeSpec; + std::string TmpfsSpec; +}; + +// Parses a Docker-style --mount spec into the existing volume or tmpfs representation. +ParsedMount ParseMount(const std::wstring& value); + +} // namespace wsl::windows::wslc::validation diff --git a/src/windows/wslc/arguments/SpecParsing.cpp b/src/windows/wslc/arguments/SpecParsing.cpp index 3e148c11de..5d4b045a64 100644 --- a/src/windows/wslc/arguments/SpecParsing.cpp +++ b/src/windows/wslc/arguments/SpecParsing.cpp @@ -21,6 +21,7 @@ Module Name: #include "ImageService.h" #include "Localization.h" #include +#include #include #include #include @@ -457,95 +458,6 @@ std::pair ParseFilter(const std::wstring& value) return {WideToMultiByte(kv.Key), WideToMultiByte(kv.Value)}; } -ParsedMount ParseMount(const std::wstring& value) -{ - std::wstring type; - std::wstring source; - std::wstring target; - bool readOnly = false; - - const auto fail = [&]() { throw ArgumentException(Localization::WSLCCLI_InvalidMountError(value)); }; - - size_t position = 0; - while (position < value.size()) - { - const auto comma = value.find(L',', position); - const auto token = value.substr(position, comma == std::wstring::npos ? std::wstring::npos : comma - position); - position = comma == std::wstring::npos ? value.size() : comma + 1; - - if (token.empty()) - { - continue; - } - - const auto keyValue = SplitKeyValue(token); - if (IsEqual(keyValue.Key, L"type", true)) - { - type = keyValue.Value; - } - else if (IsEqual(keyValue.Key, L"source", true) || IsEqual(keyValue.Key, L"src", true)) - { - source = keyValue.Value; - } - else if ( - IsEqual(keyValue.Key, L"target", true) || IsEqual(keyValue.Key, L"destination", true) || IsEqual(keyValue.Key, L"dst", true)) - { - target = keyValue.Value; - } - else if (IsEqual(keyValue.Key, L"readonly", true) || IsEqual(keyValue.Key, L"ro", true)) - { - readOnly = keyValue.Value.empty() || IsEqual(keyValue.Value, L"true", true) || IsEqual(keyValue.Value, L"1", true); - } - else - { - fail(); - } - } - - if (type.empty()) - { - type = L"volume"; - } - - if (target.empty()) - { - fail(); - } - - ParsedMount result; - if (IsEqual(type, L"tmpfs", true)) - { - if (!source.empty()) - { - fail(); - } - - result.IsTmpfs = true; - result.TmpfsSpec = WideToMultiByte(target); - } - else if (IsEqual(type, L"bind", true) || IsEqual(type, L"volume", true)) - { - if (source.empty()) - { - fail(); - } - - result.VolumeSpec = source + L":" + target; - if (readOnly) - { - result.VolumeSpec += L":ro"; - } - - std::ignore = models::VolumeMount::Parse(result.VolumeSpec); - } - else - { - fail(); - } - - return result; -} - // Map of signal names to WSLCSignal enum values static const std::unordered_map SignalMap = { {L"SIGHUP", WSLCSignalSIGHUP}, {L"SIGINT", WSLCSignalSIGINT}, {L"SIGQUIT", WSLCSignalSIGQUIT}, diff --git a/src/windows/wslc/arguments/SpecParsing.h b/src/windows/wslc/arguments/SpecParsing.h index 6019dc8036..bb093c46de 100644 --- a/src/windows/wslc/arguments/SpecParsing.h +++ b/src/windows/wslc/arguments/SpecParsing.h @@ -72,16 +72,6 @@ std::pair ParseDriverOption(const std::wstring& value) // Parses a --filter spec ("key=value"); the separator is required. std::pair ParseFilter(const std::wstring& value); -struct ParsedMount -{ - bool IsTmpfs = false; - std::wstring VolumeSpec; - std::string TmpfsSpec; -}; - -// Parses a Docker-style --mount spec into the existing volume or tmpfs representation. -ParsedMount ParseMount(const std::wstring& value); - // Parses a signal by name ("SIGKILL"/"KILL", case-insensitive) or number ("9") into a WSLCSignal. WSLCSignal GetWSLCSignalFromString(const std::wstring& input, const std::wstring& argName = {}); diff --git a/src/windows/wslc/commands/ContainerCreateCommand.cpp b/src/windows/wslc/commands/ContainerCreateCommand.cpp index 3e7f2dc823..5d9042aaad 100644 --- a/src/windows/wslc/commands/ContainerCreateCommand.cpp +++ b/src/windows/wslc/commands/ContainerCreateCommand.cpp @@ -52,7 +52,7 @@ std::vector ContainerCreateCommand::GetArguments() const Argument::Create(ArgType::Interactive), Argument::Create(ArgType::Label, false, Limit::Unlimited), Argument::Create(ArgType::Memory), - Argument::Create(ArgType::Mount, false, NO_LIMIT), + Argument::Create(ArgType::Mount, false, Limit::Unlimited), Argument::Create(ArgType::Name), Argument::Create(ArgType::Network, false, Limit::Unlimited), Argument::Create(ArgType::NetworkAlias, false, Limit::Unlimited), diff --git a/src/windows/wslc/commands/ContainerRunCommand.cpp b/src/windows/wslc/commands/ContainerRunCommand.cpp index f563a76403..dd31e367e0 100644 --- a/src/windows/wslc/commands/ContainerRunCommand.cpp +++ b/src/windows/wslc/commands/ContainerRunCommand.cpp @@ -52,7 +52,7 @@ std::vector ContainerRunCommand::GetArguments() const Argument::Create(ArgType::Interactive), Argument::Create(ArgType::Label, false, Limit::Unlimited), Argument::Create(ArgType::Memory), - Argument::Create(ArgType::Mount, false, NO_LIMIT), + Argument::Create(ArgType::Mount, false, Limit::Unlimited), Argument::Create(ArgType::Name), Argument::Create(ArgType::Network, false, Limit::Unlimited), Argument::Create(ArgType::NetworkAlias, false, Limit::Unlimited), diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp index baaaa562fb..90b4f88a7b 100644 --- a/src/windows/wslc/services/ContainerModel.cpp +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -13,12 +13,64 @@ Module Name: #include "precomp.h" #include "ContainerModel.h" +#include namespace wsl::windows::wslc::models { using namespace wsl::shared; using namespace wsl::shared::string; +namespace { + + std::string NormalizeMountDestination(std::string destination) + { + std::replace(destination.begin(), destination.end(), '\\', '/'); + + std::vector components; + size_t start = 0; + while (start <= destination.size()) + { + const auto end = destination.find('/', start); + const auto component = destination.substr(start, end - start); + if (!component.empty() && component != ".") + { + if (component == "..") + { + if (!components.empty()) + { + components.pop_back(); + } + } + else + { + components.emplace_back(component); + } + } + + if (end == std::string::npos) + { + break; + } + + start = end + 1; + } + + std::string result = "/"; + for (const auto& component : components) + { + if (result.size() > 1) + { + result += '/'; + } + + result += component; + } + + return result; + } + +} // namespace + PublishPort::PortRange PublishPort::PortRange::ParsePortPart(const std::string& portPart) { static auto parsePort = [](const std::string& value, const std::string& errorMessage) -> uint16_t { @@ -333,6 +385,28 @@ TmpfsMount TmpfsMount::Parse(const std::string& value) return result; } +void ValidateUniqueMountDestinations(const ContainerOptions& options) +{ + std::unordered_set destinations; + const auto addDestination = [&](const std::string& destination) { + const auto normalizedDestination = NormalizeMountDestination(destination); + THROW_HR_WITH_USER_ERROR_IF( + E_INVALIDARG, + Localization::WSLCCLI_DuplicateMountDestinationError(MultiByteToWide(normalizedDestination)), + !destinations.emplace(normalizedDestination).second); + }; + + for (const auto& volumeSpec : options.Volumes) + { + addDestination(VolumeMount::Parse(volumeSpec).ContainerPath()); + } + + for (const auto& tmpfsSpec : options.Tmpfs) + { + addDestination(TmpfsMount::Parse(tmpfsSpec).ContainerPath()); + } +} + CidFile::CidFile(const std::optional& path) { if (!path.has_value()) diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index 4d8ff2cc59..a7f0cc95e4 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -308,6 +308,8 @@ struct TmpfsMount std::string m_options; }; +void ValidateUniqueMountDestinations(const ContainerOptions& options); + class CidFile { public: diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index 9bbeed862c..917080d153 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -19,6 +19,7 @@ Module Name: #include "ContainerService.h" #include "ContainerTasks.h" #include "ImageModel.h" +#include "MountSpecParsing.h" #include "SessionModel.h" #include "SessionService.h" #include "TableOutput.h" @@ -903,6 +904,8 @@ void SetContainerOptionsFromArgs(CLIExecutionContext& context) } } + ValidateUniqueMountDestinations(options); + if (context.Args.Contains(ArgType::Label)) { for (const auto& label : context.Args.GetAll()) diff --git a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp new file mode 100644 index 0000000000..e9778eb5a6 --- /dev/null +++ b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp @@ -0,0 +1,194 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + WSLCCLIMountParserUnitTests.cpp + +Abstract: + + Unit tests for Docker-compatible --mount parsing. + +--*/ + +#include "precomp.h" +#include "windows/Common.h" +#include "WSLCCLITestHelpers.h" +#include "ArgumentValidation.h" +#include "ContainerModel.h" +#include "Exceptions.h" +#include "MountSpecParsing.h" + +using namespace wsl::windows::wslc; +using namespace wsl::windows::wslc::models; +using namespace WEX::Logging; +using namespace WEX::Common; + +namespace WSLCCLIMountParserUnitTests { + +class WSLCCLIMountParserUnitTests +{ + WSLC_TEST_CLASS(WSLCCLIMountParserUnitTests) + + static void VerifyVolume(const std::wstring& spec, const std::wstring& expected) + { + const auto mount = validation::ParseMount(spec); + VERIFY_IS_FALSE(mount.IsTmpfs); + VERIFY_ARE_EQUAL(expected, mount.VolumeSpec); + VERIFY_IS_TRUE(mount.TmpfsSpec.empty()); + } + + static void VerifyTmpfs(const std::wstring& spec, const std::string& expected) + { + const auto mount = validation::ParseMount(spec); + VERIFY_IS_TRUE(mount.IsTmpfs); + VERIFY_ARE_EQUAL(expected, mount.TmpfsSpec); + VERIFY_IS_TRUE(mount.VolumeSpec.empty()); + } + + static void VerifyInvalid(const std::wstring& spec, const std::wstring& expectedReason) + { + Log::Comment(String().Format(L"Rejecting: %ls", spec.c_str())); + try + { + (void)validation::ParseMount(spec); + VERIFY_FAIL(L"Expected ArgumentException for invalid mount spec"); + } + catch (const ArgumentException& ex) + { + const auto& message = ex.Message(); + VERIFY_IS_TRUE(message.find(L"for '--mount' flag") != std::wstring::npos); + VERIFY_IS_TRUE(message.find(expectedReason) != std::wstring::npos); + } + } + + TEST_METHOD(Mount_KeysAndTypeAreCaseInsensitive) + { + VerifyVolume(L"TYPE=VOLUME,SOURCE=data-volume,TARGET=/data", L"data-volume:/data"); + } + + TEST_METHOD(Mount_AliasesMatchDocker) + { + VerifyVolume(L"type=volume,src=data-volume,dst=/data,ro", L"data-volume:/data:ro"); + VerifyVolume(L"type=volume,source=data-volume,destination=/data", L"data-volume:/data"); + } + + TEST_METHOD(Mount_DefaultTypeIsVolume) + { + VerifyVolume(L"source=data-volume,target=/data", L"data-volume:/data"); + } + + TEST_METHOD(Mount_ReadOnlyUsesGoBooleanSpellings) + { + VerifyVolume(L"type=volume,source=data-volume,target=/data,readonly=t", L"data-volume:/data:ro"); + VerifyVolume(L"type=volume,source=data-volume,target=/data,readonly=TRUE", L"data-volume:/data:ro"); + VerifyVolume(L"type=volume,source=data-volume,target=/data,readonly=0", L"data-volume:/data"); + VerifyVolume(L"type=volume,source=data-volume,target=/data,readonly=F", L"data-volume:/data"); + } + + TEST_METHOD(Mount_CsvQuotedFieldPreservesComma) + { + VerifyVolume(L"type=bind,\"source=C:\\mount,a\",target=/data", L"C:\\mount,a:/data"); + } + + TEST_METHOD(Mount_BindRecursiveEnabledIsDefaultBehavior) + { + VerifyVolume(L"type=bind,source=C:\\mount,target=/data,bind-recursive=enabled", L"C:\\mount:/data"); + } + + TEST_METHOD(Mount_TmpfsOptionsMatchDockerConversion) + { + VerifyTmpfs(L"type=tmpfs,target=/tmp,tmpfs-size=1MB,tmpfs-mode=0700,readonly", "/tmp:ro,mode=700,size=1m"); + VerifyTmpfs(L"type=tmpfs,target=/tmp,tmpfs-size=1.5MB", "/tmp:size=1536k"); + VerifyTmpfs(L"type=tmpfs,target=/tmp,tmpfs-size=1536", "/tmp:size=1536"); + VerifyTmpfs(L"type=tmpfs,target=/tmp,tmpfs-size=0,tmpfs-mode=0000", "/tmp"); + } + + TEST_METHOD(Mount_InvalidFieldsMatchDocker) + { + VerifyInvalid(L"type=volume,bogus", L"invalid field 'bogus' must be a key=value pair"); + VerifyInvalid(L"type=volume,bogus=value", L"unexpected key 'bogus'"); + VerifyInvalid(L"type=volume,source=data-volume,target=/data,readonly=no", L"invalid value for readonly: no"); + VerifyInvalid(L"type=tmpfs,target=/tmp,tmpfs-size=bad", L"invalid value for tmpfs-size: bad"); + VerifyInvalid(L"type=tmpfs,target=/tmp,\"tmpfs-size=1,5MB\"", L"invalid value for tmpfs-size: 1,5MB"); + VerifyInvalid( + L"type=tmpfs,target=/tmp,tmpfs-size=9223372036854775808", L"invalid value for tmpfs-size: 9223372036854775808"); + VerifyInvalid(L"type=tmpfs,target=/tmp,tmpfs-mode=0899", L"invalid value for tmpfs-mode: 0899"); + VerifyInvalid(L"type=bind,source=C:\\mount,target=/data,bind-recursive=Enabled", L"invalid value for bind-recursive"); + } + + TEST_METHOD(Mount_RequiredFieldsMatchDocker) + { + VerifyInvalid(L"type=,source=data-volume,target=/data", L"type is required"); + VerifyInvalid(L"type=volume,source=data-volume", L"target is required"); + } + + TEST_METHOD(Mount_OptionTypeConflictsMatchDocker) + { + VerifyInvalid( + L"type=bind,source=C:\\mount,target=/data,volume-nocopy=true", + L"cannot mix 'volume-*' options with mount type 'bind'"); + VerifyInvalid( + L"type=volume,source=data-volume,target=/data,bind-propagation=rprivate", + L"cannot mix 'bind-*' options with mount type 'volume'"); + VerifyInvalid( + L"type=volume,source=data-volume,target=/data,tmpfs-size=1m", + L"cannot mix 'tmpfs-*' options with mount type 'volume'"); + } + + TEST_METHOD(Mount_BindRecursiveValidationMatchesDocker) + { + VerifyInvalid( + L"type=bind,source=C:\\mount,target=/data,bind-recursive=writable", + L"requires 'readonly' to be specified in conjunction"); + VerifyInvalid( + L"type=bind,source=C:\\mount,target=/data,bind-recursive=readonly,readonly", + L"requires 'bind-propagation=rprivate' to be specified in conjunction"); + } + + TEST_METHOD(Mount_UnsupportedBackendFeaturesAreExplicit) + { + VerifyInvalid( + L"type=volume,source=data-volume,target=/data,volume-nocopy", L"option 'volume-nocopy' is not supported by WSLC"); + VerifyInvalid( + L"type=bind,source=C:\\mount,target=/data,consistency=cached", L"option 'consistency' is not supported by WSLC"); + VerifyInvalid(L"type=cluster,source=data-volume,target=/data", L"mount type 'cluster' is not supported by WSLC"); + VerifyInvalid(L"type=volume,target=/data", L"anonymous volume mounts are not supported by WSLC"); + } + + TEST_METHOD(Mount_BackendRepresentationLimitsAreExplicit) + { + VerifyInvalid(L"type=bind,source=relative,target=/data", L"bind source path must be absolute"); + VerifyInvalid(L"type=volume,source=C:\\mount,target=/data", L"volume source must be a valid named volume"); + VerifyInvalid(L"type=tmpfs,source=data-volume,target=/data", L"source is not supported for tmpfs mounts"); + VerifyInvalid( + L"type=volume,source=data-volume,target=/data:part", L"target paths containing ':' are not supported by WSLC"); + } + + TEST_METHOD(Mount_MalformedCsvIsRejected) + { + VerifyInvalid(L"type=bind,\"source=C:\\mount,target=/data", L"malformed CSV"); + } + + TEST_METHOD(Mount_DuplicateDestinationsAreRejected) + { + ContainerOptions options; + options.Tmpfs = {"/data", "/data/"}; + VERIFY_THROWS(ValidateUniqueMountDestinations(options), wil::ResultException); + + options.Tmpfs = {"/data/../cache"}; + options.Volumes = {L"data-volume:/cache"}; + VERIFY_THROWS(ValidateUniqueMountDestinations(options), wil::ResultException); + } + + TEST_METHOD(Mount_UniqueDestinationsAreAccepted) + { + ContainerOptions options; + options.Tmpfs = {"/cache"}; + options.Volumes = {L"data-volume:/data"}; + VERIFY_NO_THROW(ValidateUniqueMountDestinations(options)); + } +}; + +} // namespace WSLCCLIMountParserUnitTests diff --git a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp index 227df84137..43ff82eb0f 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp @@ -1025,24 +1025,20 @@ class WSLCE2EContainerRunTests WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_ReadOnly_IsReadOnly) { auto result = RunWslc(std::format( - L"container run --name {} --mount type=volume,source={},target=/data,readonly {} echo ok", - WslcContainerName, + L"container run --rm --mount type=volume,source={},target=/data {} sh -c \"echo -n original > /data/value\"", WslcVolumeName, DebianImage.NameAndTag())); - result.Verify({.Stdout = L"ok\n", .Stderr = L"", .ExitCode = 0}); + result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); - const auto inspect = InspectContainer(WslcContainerName); - bool found = false; - for (const auto& mount : inspect.Mounts) - { - if (mount.Destination == "/data") - { - found = true; - VERIFY_IS_FALSE(mount.ReadWrite); - } - } + result = RunWslc(std::format( + L"container run --rm --mount type=volume,source={},target=/data,readonly {} sh -c \"echo changed > /data/value\"", + WslcVolumeName, + DebianImage.NameAndTag())); + result.Verify({.Stdout = L"", .Stderr = L"sh: 1: cannot create /data/value: Read-only file system\n", .ExitCode = 2}); - VERIFY_IS_TRUE(found); + result = RunWslc(std::format( + L"container run --rm --mount type=volume,source={},target=/data {} cat /data/value", WslcVolumeName, DebianImage.NameAndTag())); + result.Verify({.Stdout = L"original", .Stderr = L"", .ExitCode = 0}); } WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_InvalidType_Fails) @@ -1053,6 +1049,17 @@ class WSLCE2EContainerRunTests VERIFY_ARE_NOT_EQUAL(std::wstring::npos, result.Stderr->find(L"for '--mount' flag")); } + WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_DuplicateDestination_Fails) + { + auto result = RunWslc(std::format( + L"container run --rm --name {} --mount type=tmpfs,target=/data --mount type=tmpfs,target=/data/ {} true", + WslcContainerName, + DebianImage.NameAndTag())); + result.Verify({.Stdout = L"", .ExitCode = 1}); + VERIFY_IS_TRUE(result.StderrContainsSubstring(L"Duplicate mount point: /data")); + EnsureContainerDoesNotExist(WslcContainerName); + } + WSLC_TEST_METHOD(WSLCE2E_Container_Run_WithLabel_Success) { auto result = RunWslc(std::format( From 8267ea04caab3e693801383f5e70b91ca489ecce Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 12 Aug 2026 16:01:39 -0700 Subject: [PATCH 03/12] Refactor mount spec location --- src/windows/common/CMakeLists.txt | 2 + src/windows/common/MountSpecParsing.cpp | 634 ++++++++++++++++++ src/windows/common/MountSpecParsing.h | 71 ++ .../wslc/arguments/ArgumentConvertedTypes.h | 4 +- .../wslc/arguments/ArgumentValidation.cpp | 14 +- .../wslc/arguments/MountSpecParsing.cpp | 500 -------------- src/windows/wslc/arguments/MountSpecParsing.h | 30 - src/windows/wslc/services/ContainerModel.cpp | 61 +- src/windows/wslc/services/ContainerModel.h | 4 + .../wslc/services/ContainerService.cpp | 20 + src/windows/wslc/tasks/ContainerTasks.cpp | 13 +- .../wslc/WSLCCLIMountParserUnitTests.cpp | 457 +++++++++---- 12 files changed, 1084 insertions(+), 726 deletions(-) create mode 100644 src/windows/common/MountSpecParsing.cpp create mode 100644 src/windows/common/MountSpecParsing.h delete mode 100644 src/windows/wslc/arguments/MountSpecParsing.cpp delete mode 100644 src/windows/wslc/arguments/MountSpecParsing.h diff --git a/src/windows/common/CMakeLists.txt b/src/windows/common/CMakeLists.txt index 008eafab10..97fb2dfd8e 100644 --- a/src/windows/common/CMakeLists.txt +++ b/src/windows/common/CMakeLists.txt @@ -26,6 +26,7 @@ set(SOURCES LxssMessagePort.cpp LxssSecurity.cpp LxssServerPort.cpp + MountSpecParsing.cpp NatNetworking.cpp notifications.cpp Redirector.cpp @@ -111,6 +112,7 @@ set(HEADERS LxssPort.h LxssSecurity.h LxssServerPort.h + MountSpecParsing.h NatNetworking.h notifications.h precomp.h diff --git a/src/windows/common/MountSpecParsing.cpp b/src/windows/common/MountSpecParsing.cpp new file mode 100644 index 0000000000..7b52e2637e --- /dev/null +++ b/src/windows/common/MountSpecParsing.cpp @@ -0,0 +1,634 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + MountSpecParsing.cpp + +Abstract: + + Docker-compatible mount specification parsing. + +--*/ + +#include "precomp.h" +#include "MountSpecParsing.h" +#include +#include +#include +#include +#include +#include +#include + +using namespace wsl::shared; +using namespace wsl::shared::string; + +namespace wsl::windows::common::mount { + +namespace { + + enum class Field + { + Type, + Source, + Target, + ReadOnly, + Consistency, + BindPropagation, + BindNonRecursive, + BindRecursive, + VolumeNoCopy, + VolumeLabel, + VolumeDriver, + VolumeOption, + TmpfsSize, + TmpfsMode, + }; + + enum class Family + { + General, + Bind, + Volume, + Tmpfs, + }; + + enum class Support + { + Supported, + Unsupported, + ValueDependent, + }; + + struct FieldDefinition + { + std::wstring_view Name; + Field Id; + Family OptionFamily; + bool AllowsBareForm; + Support SupportLevel; + }; + + // Keep this table aligned with docker/cli v25.0.3 opts/mount.go. It is the + // authoritative list of accepted fields, aliases, option families, and WSLC support. + constexpr std::array c_fieldDefinitions{ + FieldDefinition{L"type", Field::Type, Family::General, false, Support::ValueDependent}, + FieldDefinition{L"source", Field::Source, Family::General, false, Support::Supported}, + FieldDefinition{L"src", Field::Source, Family::General, false, Support::Supported}, + FieldDefinition{L"target", Field::Target, Family::General, false, Support::Supported}, + FieldDefinition{L"dst", Field::Target, Family::General, false, Support::Supported}, + FieldDefinition{L"destination", Field::Target, Family::General, false, Support::Supported}, + FieldDefinition{L"readonly", Field::ReadOnly, Family::General, true, Support::Supported}, + FieldDefinition{L"ro", Field::ReadOnly, Family::General, true, Support::Supported}, + FieldDefinition{L"consistency", Field::Consistency, Family::General, false, Support::Unsupported}, + FieldDefinition{L"bind-propagation", Field::BindPropagation, Family::Bind, false, Support::Unsupported}, + FieldDefinition{L"bind-nonrecursive", Field::BindNonRecursive, Family::Bind, true, Support::Unsupported}, + FieldDefinition{L"bind-recursive", Field::BindRecursive, Family::Bind, false, Support::ValueDependent}, + FieldDefinition{L"volume-nocopy", Field::VolumeNoCopy, Family::Volume, true, Support::Unsupported}, + FieldDefinition{L"volume-label", Field::VolumeLabel, Family::Volume, false, Support::Unsupported}, + FieldDefinition{L"volume-driver", Field::VolumeDriver, Family::Volume, false, Support::Unsupported}, + FieldDefinition{L"volume-opt", Field::VolumeOption, Family::Volume, false, Support::Unsupported}, + FieldDefinition{L"tmpfs-size", Field::TmpfsSize, Family::Tmpfs, false, Support::Supported}, + FieldDefinition{L"tmpfs-mode", Field::TmpfsMode, Family::Tmpfs, false, Support::Supported}, + }; + + struct DockerMountSpec + { + std::wstring Type = L"volume"; + std::wstring Source; + std::wstring Target; + bool ReadOnly = false; + bool HasVolumeOptions = false; + bool HasBindOptions = false; + bool HasTmpfsOptions = false; + bool BindReadOnlyNonRecursive = false; + bool BindReadOnlyForceRecursive = false; + std::wstring BindPropagation; + std::optional TmpfsSizeBytes; + std::optional TmpfsMode; + std::optional UnsupportedOption; + }; + + struct KeyValue + { + std::wstring Key; + std::wstring Value; + bool HadSeparator; + }; + + [[noreturn]] void ThrowInvalid(std::wstring reason) + { + throw ParseException(std::move(reason)); + } + + KeyValue SplitKeyValue(const std::wstring& value) + { + const auto position = value.find(L'='); + if (position == std::wstring::npos) + { + return {.Key = value, .HadSeparator = false}; + } + + return {.Key = value.substr(0, position), .Value = value.substr(position + 1), .HadSeparator = true}; + } + + const FieldDefinition* FindField(std::wstring_view name) + { + const auto found = std::ranges::find_if(c_fieldDefinitions, [&](const auto& definition) { return definition.Name == name; }); + return found == c_fieldDefinitions.end() ? nullptr : &*found; + } + + void RecordUnsupportedOption(DockerMountSpec& mount, std::wstring_view option) + { + if (!mount.UnsupportedOption.has_value()) + { + mount.UnsupportedOption = option; + } + } + + std::optional ParseDockerRamInBytes(const std::wstring& value) + { + const auto input = WideToMultiByte(value); + const auto separator = input.find_last_of("01234567890. "); + if (separator == std::string::npos) + { + return std::nullopt; + } + + std::string number; + std::string suffix; + if (input[separator] == ' ') + { + number = input.substr(0, separator); + suffix = input.substr(separator + 1); + } + else + { + number = input.substr(0, separator + 1); + suffix = input.substr(separator + 1); + } + + if (number.empty() || std::isspace(static_cast(number.front()))) + { + return std::nullopt; + } + + std::string_view numberView(number); + if (numberView.front() == '+') + { + numberView.remove_prefix(1); + if (numberView.empty()) + { + return std::nullopt; + } + } + + double parsed{}; + const auto parseResult = + std::from_chars(numberView.data(), numberView.data() + numberView.size(), parsed, std::chars_format::general); + if (parseResult.ec != std::errc() || parseResult.ptr != numberView.data() + numberView.size() || !std::isfinite(parsed) || parsed < 0) + { + return std::nullopt; + } + + double bytes = parsed; + if (!suffix.empty()) + { + suffix = AsciiToLower(std::string_view(suffix)); + if (suffix.size() > 3) + { + return std::nullopt; + } + + if (suffix.front() == 'b') + { + if (suffix.size() != 1) + { + return std::nullopt; + } + } + else + { + uint64_t factor{}; + switch (suffix.front()) + { + case 'k': + factor = 1ULL << 10; + break; + case 'm': + factor = 1ULL << 20; + break; + case 'g': + factor = 1ULL << 30; + break; + case 't': + factor = 1ULL << 40; + break; + case 'p': + factor = 1ULL << 50; + break; + default: + return std::nullopt; + } + + if ((suffix.size() == 2 && suffix[1] != 'b') || (suffix.size() == 3 && suffix.substr(1) != "ib")) + { + return std::nullopt; + } + + bytes *= static_cast(factor); + } + } + + constexpr double c_int64Limit = 9223372036854775808.0; + if (!std::isfinite(bytes) || bytes >= c_int64Limit) + { + return std::nullopt; + } + + return static_cast(bytes); + } + + std::optional ParseDockerTmpfsMode(const std::wstring& value) + { + if (value.empty() || value.front() == L'-') + { + return std::nullopt; + } + + size_t position = value.front() == L'+' ? 1 : 0; + if (position == value.size()) + { + return std::nullopt; + } + + uint64_t result = 0; + for (; position < value.size(); ++position) + { + const auto digit = value[position]; + if (digit < L'0' || digit > L'7') + { + return std::nullopt; + } + + result = (result * 8) + static_cast(digit - L'0'); + if (result > std::numeric_limits::max()) + { + return std::nullopt; + } + } + + return static_cast(result); + } + + std::string FormatDockerTmpfsSize(int64_t sizeBytes) + { + for (const auto& [suffix, divisor] : std::array, 3>{{{'g', 1LL << 30}, {'m', 1LL << 20}, {'k', 1LL << 10}}}) + { + if ((sizeBytes % divisor) == 0) + { + return std::format("{}{}", sizeBytes / divisor, suffix); + } + } + + return std::to_string(sizeBytes); + } + +} // namespace + +Spec Parse(const std::wstring& value) +{ + const auto fields = SplitCsvFields(value); + if (!fields.has_value()) + { + ThrowInvalid(L"malformed CSV"); + } + + DockerMountSpec mount; + + for (const auto& field : *fields) + { + const auto keyValue = SplitKeyValue(field); + const auto key = AsciiToLower(std::wstring_view(keyValue.Key)); + const auto definition = FindField(key); + if (definition == nullptr) + { + if (!keyValue.HadSeparator) + { + ThrowInvalid(std::format(L"invalid field '{}' must be a key=value pair", field)); + } + + ThrowInvalid(std::format(L"unexpected key '{}' in '{}'", key, field)); + } + + if (!keyValue.HadSeparator && !definition->AllowsBareForm) + { + ThrowInvalid(std::format(L"invalid field '{}' must be a key=value pair", field)); + } + + switch (definition->Id) + { + case Field::Type: + mount.Type = AsciiToLower(std::wstring_view(keyValue.Value)); + break; + + case Field::Source: + mount.Source = keyValue.Value; + if (mount.Source == L"." || mount.Source.starts_with(L".\\")) + { + std::error_code error; + auto absolutePath = std::filesystem::absolute(mount.Source, error); + if (!error) + { + mount.Source = absolutePath.lexically_normal().wstring(); + } + } + break; + + case Field::Target: + mount.Target = keyValue.Value; + break; + + case Field::ReadOnly: + if (!keyValue.HadSeparator) + { + mount.ReadOnly = true; + break; + } + + if (const auto parsed = ParseBool(keyValue.Value.c_str(), true); parsed.has_value()) + { + mount.ReadOnly = parsed.value(); + } + else + { + ThrowInvalid(std::format(L"invalid value for {}: {}", key, keyValue.Value)); + } + break; + + case Field::Consistency: + break; + + case Field::BindPropagation: + mount.HasBindOptions = true; + mount.BindPropagation = AsciiToLower(std::wstring_view(keyValue.Value)); + break; + + case Field::BindNonRecursive: + if (keyValue.HadSeparator && !ParseBool(keyValue.Value.c_str(), true).has_value()) + { + ThrowInvalid(std::format(L"invalid value for {}: {}", key, keyValue.Value)); + } + + mount.HasBindOptions = true; + break; + + case Field::BindRecursive: + if (keyValue.Value == L"enabled") + { + break; + } + + mount.HasBindOptions = true; + RecordUnsupportedOption(mount, key); + if (keyValue.Value == L"disabled") + { + break; + } + if (keyValue.Value == L"writable") + { + mount.BindReadOnlyNonRecursive = true; + break; + } + if (keyValue.Value == L"readonly") + { + mount.BindReadOnlyForceRecursive = true; + break; + } + + ThrowInvalid(std::format( + L"invalid value for {}: {} (must be \"enabled\", \"disabled\", \"writable\", or \"readonly\")", key, keyValue.Value)); + + case Field::VolumeNoCopy: + if (keyValue.HadSeparator && !ParseBool(keyValue.Value.c_str(), true).has_value()) + { + ThrowInvalid(std::format(L"invalid value for volume-nocopy: {}", keyValue.Value)); + } + + mount.HasVolumeOptions = true; + break; + + case Field::VolumeLabel: + case Field::VolumeDriver: + case Field::VolumeOption: + mount.HasVolumeOptions = true; + break; + + case Field::TmpfsSize: + mount.TmpfsSizeBytes = ParseDockerRamInBytes(keyValue.Value); + if (!mount.TmpfsSizeBytes.has_value()) + { + ThrowInvalid(std::format(L"invalid value for {}: {}", key, keyValue.Value)); + } + + mount.HasTmpfsOptions = true; + break; + + case Field::TmpfsMode: + mount.TmpfsMode = ParseDockerTmpfsMode(keyValue.Value); + if (!mount.TmpfsMode.has_value()) + { + ThrowInvalid(std::format(L"invalid value for {}: {}", key, keyValue.Value)); + } + + mount.HasTmpfsOptions = true; + break; + } + + if (definition->SupportLevel == Support::Unsupported) + { + RecordUnsupportedOption(mount, key); + } + } + + if (mount.Type.empty()) + { + ThrowInvalid(L"type is required"); + } + if (mount.Target.empty()) + { + ThrowInvalid(L"target is required"); + } + + if (mount.HasVolumeOptions && mount.Type != L"volume") + { + ThrowInvalid(std::format(L"cannot mix 'volume-*' options with mount type '{}'", mount.Type)); + } + if (mount.HasBindOptions && mount.Type != L"bind") + { + ThrowInvalid(std::format(L"cannot mix 'bind-*' options with mount type '{}'", mount.Type)); + } + if (mount.HasTmpfsOptions && mount.Type != L"tmpfs") + { + ThrowInvalid(std::format(L"cannot mix 'tmpfs-*' options with mount type '{}'", mount.Type)); + } + + if (mount.BindReadOnlyNonRecursive && !mount.ReadOnly) + { + ThrowInvalid(L"option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction"); + } + if (mount.BindReadOnlyForceRecursive) + { + if (!mount.ReadOnly) + { + ThrowInvalid(L"option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction"); + } + if (mount.BindPropagation != L"rprivate") + { + ThrowInvalid(L"option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction"); + } + } + + Type type; + if (mount.Type == L"bind") + { + type = Type::Bind; + } + else if (mount.Type == L"volume") + { + type = Type::Volume; + } + else if (mount.Type == L"tmpfs") + { + type = Type::Tmpfs; + } + else + { + ThrowInvalid(std::format(L"mount type '{}' is not supported.", mount.Type)); + } + + if (mount.UnsupportedOption.has_value()) + { + ThrowInvalid(std::format(L"option '{}' is not supported.", mount.UnsupportedOption.value())); + } + if (mount.Target.find(L':') != std::wstring::npos) + { + ThrowInvalid(L"target paths containing ':' are not supported."); + } + + if (type == Type::Tmpfs) + { + if (!mount.Source.empty()) + { + ThrowInvalid(L"source is not supported for tmpfs mounts"); + } + } + else + { + if (mount.Source.empty()) + { + if (type == Type::Volume) + { + ThrowInvalid(L"anonymous volume mounts are not supported."); + } + + ThrowInvalid(L"source is required"); + } + + if (type == Type::Bind && !std::filesystem::path(mount.Source).is_absolute()) + { + ThrowInvalid(L"bind source path must be absolute"); + } + if (type == Type::Volume && !IsValidNamedVolumeName(mount.Source)) + { + ThrowInvalid(L"volume source must be a valid named volume"); + } + } + + return { + .MountType = type, + .Source = std::move(mount.Source), + .Target = WideToMultiByte(mount.Target), + .ReadOnly = mount.ReadOnly, + .TmpfsSizeBytes = mount.TmpfsSizeBytes, + .TmpfsMode = mount.TmpfsMode, + }; +} + +std::string FormatTmpfsOptions(const Spec& mount) +{ + WI_ASSERT(mount.MountType == Type::Tmpfs); + + std::vector options; + if (mount.ReadOnly) + { + options.emplace_back("ro"); + } + if (mount.TmpfsMode.has_value() && mount.TmpfsMode.value() != 0) + { + options.emplace_back(std::format("mode={:o}", mount.TmpfsMode.value())); + } + if (mount.TmpfsSizeBytes.has_value() && mount.TmpfsSizeBytes.value() != 0) + { + options.emplace_back(std::format("size={}", FormatDockerTmpfsSize(mount.TmpfsSizeBytes.value()))); + } + + return wsl::shared::string::Join(options, ','); +} + +std::string NormalizeDestination(std::string destination) +{ + std::replace(destination.begin(), destination.end(), '\\', '/'); + + std::vector components; + size_t start = 0; + while (start <= destination.size()) + { + const auto end = destination.find('/', start); + const auto component = destination.substr(start, end - start); + if (!component.empty() && component != ".") + { + if (component == "..") + { + if (!components.empty()) + { + components.pop_back(); + } + } + else + { + components.emplace_back(component); + } + } + + if (end == std::string::npos) + { + break; + } + + start = end + 1; + } + + std::string result = "/"; + for (const auto& component : components) + { + if (result.size() > 1) + { + result += '/'; + } + + result += component; + } + + return result; +} + +bool IsValidNamedVolumeName(std::wstring_view name) +{ + static const std::wregex c_namedVolumeRegex(LR"(^[a-zA-Z0-9][a-zA-Z0-9_.-]{1,}$)"); + return std::regex_match(name.begin(), name.end(), c_namedVolumeRegex); +} + +} // namespace wsl::windows::common::mount diff --git a/src/windows/common/MountSpecParsing.h b/src/windows/common/MountSpecParsing.h new file mode 100644 index 0000000000..9bd8ceafcf --- /dev/null +++ b/src/windows/common/MountSpecParsing.h @@ -0,0 +1,71 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + MountSpecParsing.h + +Abstract: + + Docker-compatible mount specification parsing. + +--*/ + +#pragma once + +#include +#include +#include +#include +#include +#include + +namespace wsl::windows::common::mount { + +inline constexpr std::string_view c_dockerCliMountGrammarVersion = "25.0.3"; + +enum class Type +{ + Bind, + Volume, + Tmpfs, +}; + +struct Spec +{ + Type MountType = Type::Volume; + std::wstring Source; + std::string Target; + bool ReadOnly = false; + std::optional TmpfsSizeBytes; + std::optional TmpfsMode; +}; + +class ParseException : public std::exception +{ +public: + explicit ParseException(std::wstring reason) : m_reason(std::move(reason)) + { + } + + const char* what() const noexcept override + { + return "invalid mount specification"; + } + + const std::wstring& Reason() const noexcept + { + return m_reason; + } + +private: + std::wstring m_reason; +}; + +Spec Parse(const std::wstring& value); +std::string FormatTmpfsOptions(const Spec& mount); +std::string NormalizeDestination(std::string destination); +bool IsValidNamedVolumeName(std::wstring_view name); + +} // namespace wsl::windows::common::mount diff --git a/src/windows/wslc/arguments/ArgumentConvertedTypes.h b/src/windows/wslc/arguments/ArgumentConvertedTypes.h index 68a682b8d0..10b95f1e01 100644 --- a/src/windows/wslc/arguments/ArgumentConvertedTypes.h +++ b/src/windows/wslc/arguments/ArgumentConvertedTypes.h @@ -33,6 +33,8 @@ struct BuildSecret; namespace wsl::windows::wslc::argument::details { +namespace mount = wsl::windows::common::mount; + // Local aliases so the ConvertedType tokens in the WSLC_ARGUMENTS X-macro (ArgumentDefinitions.h) // resolve here regardless of include order. Aggregate converted types must be aliased because their // commas would otherwise break X-macro argument parsing if written inline in the table. @@ -45,7 +47,7 @@ using UlimitValue = std::tuple; using KeyValuePair = std::pair; using BuildOutput = wsl::windows::wslc::services::BuildOutput; using BuildSecret = wsl::windows::wslc::services::BuildSecret; -using ParsedMount = wsl::windows::wslc::validation::ParsedMount; +using ParsedMount = mount::Spec; // Generate the ArgType -> converted type mapping from the X-macro. Every ArgType gets a // specialization; arguments that are not converted map to NoConversion (their raw string is used diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp index 9ad17a3fb7..81d304a170 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.cpp +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -32,6 +32,8 @@ using namespace wsl::shared::string; namespace wsl::windows::wslc { +namespace mount = wsl::windows::common::mount; + namespace argument::details { struct RawArgMapAccess { @@ -219,8 +221,16 @@ void Argument::Validate(ArgMap& execArgs) const break; case ArgType::Mount: - CacheConverted( - execArgs, m_name, [](const std::wstring& value, const std::wstring&) { return validation::ParseMount(value); }); + CacheConverted(execArgs, m_name, [](const std::wstring& value, const std::wstring&) { + try + { + return mount::Parse(value); + } + catch (const mount::ParseException& ex) + { + throw ArgumentException(Localization::WSLCCLI_InvalidMountError(value, ex.Reason())); + } + }); break; case ArgType::WorkDir: diff --git a/src/windows/wslc/arguments/MountSpecParsing.cpp b/src/windows/wslc/arguments/MountSpecParsing.cpp deleted file mode 100644 index 0c389c84d3..0000000000 --- a/src/windows/wslc/arguments/MountSpecParsing.cpp +++ /dev/null @@ -1,500 +0,0 @@ -/*++ - -Copyright (c) Microsoft. All rights reserved. - -Module Name: - - MountSpecParsing.cpp - -Abstract: - - Parser for Docker-style --mount specifications. - ---*/ - -#include "precomp.h" -#include "MountSpecParsing.h" -#include "ContainerModel.h" -#include "Exceptions.h" -#include "Localization.h" -#include "SpecParsing.h" -#include -#include -#include -#include -#include -#include -#include -#include - -using namespace wsl::windows::common; -using namespace wsl::shared; -using namespace wsl::shared::string; - -namespace wsl::windows::wslc::validation { - -namespace { - - struct DockerMountSpec - { - std::wstring Type = L"volume"; - std::wstring Source; - std::wstring Target; - bool ReadOnly = false; - bool HasVolumeOptions = false; - bool HasBindOptions = false; - bool HasTmpfsOptions = false; - bool BindReadOnlyNonRecursive = false; - bool BindReadOnlyForceRecursive = false; - std::wstring BindPropagation; - std::optional TmpfsSizeBytes; - std::optional TmpfsMode; - std::optional UnsupportedOption; - }; - - [[noreturn]] void ThrowInvalidMount(const std::wstring& spec, const std::wstring& reason) - { - throw ArgumentException(Localization::WSLCCLI_InvalidMountError(spec, reason)); - } - - void RecordUnsupportedOption(DockerMountSpec& mount, const std::wstring& option) - { - if (!mount.UnsupportedOption.has_value()) - { - mount.UnsupportedOption = option; - } - } - - std::optional ParseDockerRamInBytes(const std::wstring& value) - { - const auto input = WideToMultiByte(value); - const auto separator = input.find_last_of("01234567890. "); - if (separator == std::string::npos) - { - return std::nullopt; - } - - std::string number; - std::string suffix; - if (input[separator] == ' ') - { - number = input.substr(0, separator); - suffix = input.substr(separator + 1); - } - else - { - number = input.substr(0, separator + 1); - suffix = input.substr(separator + 1); - } - - if (number.empty() || std::isspace(static_cast(number.front()))) - { - return std::nullopt; - } - - std::string_view numberView(number); - if (numberView.front() == '+') - { - numberView.remove_prefix(1); - if (numberView.empty()) - { - return std::nullopt; - } - } - - double parsed{}; - const auto parseResult = - std::from_chars(numberView.data(), numberView.data() + numberView.size(), parsed, std::chars_format::general); - if (parseResult.ec != std::errc() || parseResult.ptr != numberView.data() + numberView.size() || !std::isfinite(parsed) || parsed < 0) - { - return std::nullopt; - } - - double bytes = parsed; - if (!suffix.empty()) - { - suffix = AsciiToLower(std::string_view(suffix)); - if (suffix.size() > 3) - { - return std::nullopt; - } - - if (suffix.front() == 'b') - { - if (suffix.size() != 1) - { - return std::nullopt; - } - } - else - { - uint64_t factor{}; - switch (suffix.front()) - { - case 'k': - factor = 1ULL << 10; - break; - case 'm': - factor = 1ULL << 20; - break; - case 'g': - factor = 1ULL << 30; - break; - case 't': - factor = 1ULL << 40; - break; - case 'p': - factor = 1ULL << 50; - break; - default: - return std::nullopt; - } - - if ((suffix.size() == 2 && suffix[1] != 'b') || (suffix.size() == 3 && suffix.substr(1) != "ib")) - { - return std::nullopt; - } - - bytes *= static_cast(factor); - } - } - - constexpr double c_int64Limit = 9223372036854775808.0; - if (!std::isfinite(bytes) || bytes >= c_int64Limit) - { - return std::nullopt; - } - - return static_cast(bytes); - } - - std::optional ParseDockerTmpfsMode(const std::wstring& value) - { - if (value.empty() || value.front() == L'-') - { - return std::nullopt; - } - - size_t position = value.front() == L'+' ? 1 : 0; - if (position == value.size()) - { - return std::nullopt; - } - - uint64_t result = 0; - for (; position < value.size(); ++position) - { - const auto digit = value[position]; - if (digit < L'0' || digit > L'7') - { - return std::nullopt; - } - - result = (result * 8) + static_cast(digit - L'0'); - if (result > std::numeric_limits::max()) - { - return std::nullopt; - } - } - - return static_cast(result); - } - - std::string FormatDockerTmpfsSize(int64_t sizeBytes) - { - for (const auto& [suffix, divisor] : std::array, 3>{{{'g', 1LL << 30}, {'m', 1LL << 20}, {'k', 1LL << 10}}}) - { - if ((sizeBytes % divisor) == 0) - { - return std::format("{}{}", sizeBytes / divisor, suffix); - } - } - - return std::to_string(sizeBytes); - } - -} // namespace - -ParsedMount ParseMount(const std::wstring& value) -{ - // Keep this parser aligned with docker/cli v25.0.3 opts/mount.go. If the bundled Docker - // backend is updated, revisit both the parsing rules and the WSLC support gate below. - const auto fields = SplitCsvFields(value); - if (!fields.has_value()) - { - ThrowInvalidMount(value, L"malformed CSV"); - } - - DockerMountSpec mount; - - for (const auto& field : *fields) - { - const auto keyValue = SplitKeyValue(field); - const auto key = AsciiToLower(std::wstring_view(keyValue.Key)); - - if (!keyValue.HadSeparator) - { - if (key == L"readonly" || key == L"ro") - { - mount.ReadOnly = true; - continue; - } - - if (key == L"volume-nocopy") - { - mount.HasVolumeOptions = true; - RecordUnsupportedOption(mount, key); - continue; - } - - if (key == L"bind-nonrecursive") - { - mount.HasBindOptions = true; - RecordUnsupportedOption(mount, key); - continue; - } - - ThrowInvalidMount(value, std::format(L"invalid field '{}' must be a key=value pair", field)); - } - - if (key == L"type") - { - mount.Type = AsciiToLower(std::wstring_view(keyValue.Value)); - } - else if (key == L"source" || key == L"src") - { - mount.Source = keyValue.Value; - if (mount.Source == L"." || mount.Source.starts_with(L".\\")) - { - std::error_code error; - auto absolutePath = std::filesystem::absolute(mount.Source, error); - if (!error) - { - mount.Source = absolutePath.lexically_normal().wstring(); - } - } - } - else if (key == L"target" || key == L"dst" || key == L"destination") - { - mount.Target = keyValue.Value; - } - else if (key == L"readonly" || key == L"ro") - { - const auto parsed = ParseBool(keyValue.Value.c_str(), true); - if (!parsed.has_value()) - { - ThrowInvalidMount(value, std::format(L"invalid value for {}: {}", key, keyValue.Value)); - } - - mount.ReadOnly = parsed.value(); - } - else if (key == L"consistency") - { - RecordUnsupportedOption(mount, key); - } - else if (key == L"bind-propagation") - { - mount.HasBindOptions = true; - mount.BindPropagation = AsciiToLower(std::wstring_view(keyValue.Value)); - RecordUnsupportedOption(mount, key); - } - else if (key == L"bind-nonrecursive") - { - if (!ParseBool(keyValue.Value.c_str(), true).has_value()) - { - ThrowInvalidMount(value, std::format(L"invalid value for {}: {}", key, keyValue.Value)); - } - - mount.HasBindOptions = true; - RecordUnsupportedOption(mount, key); - } - else if (key == L"bind-recursive") - { - if (keyValue.Value == L"enabled") - { - continue; - } - - mount.HasBindOptions = true; - RecordUnsupportedOption(mount, key); - if (keyValue.Value == L"disabled") - { - continue; - } - if (keyValue.Value == L"writable") - { - mount.BindReadOnlyNonRecursive = true; - continue; - } - if (keyValue.Value == L"readonly") - { - mount.BindReadOnlyForceRecursive = true; - continue; - } - - ThrowInvalidMount( - value, - std::format( - L"invalid value for {}: {} (must be \"enabled\", \"disabled\", \"writable\", or \"readonly\")", key, keyValue.Value)); - } - else if (key == L"volume-nocopy") - { - if (!ParseBool(keyValue.Value.c_str(), true).has_value()) - { - ThrowInvalidMount(value, std::format(L"invalid value for volume-nocopy: {}", keyValue.Value)); - } - - mount.HasVolumeOptions = true; - RecordUnsupportedOption(mount, key); - } - else if (key == L"volume-label" || key == L"volume-driver" || key == L"volume-opt") - { - mount.HasVolumeOptions = true; - RecordUnsupportedOption(mount, key); - } - else if (key == L"tmpfs-size") - { - mount.TmpfsSizeBytes = ParseDockerRamInBytes(keyValue.Value); - if (!mount.TmpfsSizeBytes.has_value()) - { - ThrowInvalidMount(value, std::format(L"invalid value for {}: {}", key, keyValue.Value)); - } - - mount.HasTmpfsOptions = true; - } - else if (key == L"tmpfs-mode") - { - mount.TmpfsMode = ParseDockerTmpfsMode(keyValue.Value); - if (!mount.TmpfsMode.has_value()) - { - ThrowInvalidMount(value, std::format(L"invalid value for {}: {}", key, keyValue.Value)); - } - - mount.HasTmpfsOptions = true; - } - else - { - ThrowInvalidMount(value, std::format(L"unexpected key '{}' in '{}'", key, field)); - } - } - - if (mount.Type.empty()) - { - ThrowInvalidMount(value, L"type is required"); - } - - if (mount.Target.empty()) - { - ThrowInvalidMount(value, L"target is required"); - } - - if (mount.HasVolumeOptions && mount.Type != L"volume") - { - ThrowInvalidMount(value, std::format(L"cannot mix 'volume-*' options with mount type '{}'", mount.Type)); - } - if (mount.HasBindOptions && mount.Type != L"bind") - { - ThrowInvalidMount(value, std::format(L"cannot mix 'bind-*' options with mount type '{}'", mount.Type)); - } - if (mount.HasTmpfsOptions && mount.Type != L"tmpfs") - { - ThrowInvalidMount(value, std::format(L"cannot mix 'tmpfs-*' options with mount type '{}'", mount.Type)); - } - - if (mount.BindReadOnlyNonRecursive && !mount.ReadOnly) - { - ThrowInvalidMount(value, L"option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction"); - } - if (mount.BindReadOnlyForceRecursive) - { - if (!mount.ReadOnly) - { - ThrowInvalidMount(value, L"option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction"); - } - if (mount.BindPropagation != L"rprivate") - { - ThrowInvalidMount( - value, L"option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction"); - } - } - - if (mount.Type != L"bind" && mount.Type != L"volume" && mount.Type != L"tmpfs") - { - ThrowInvalidMount(value, std::format(L"mount type '{}' is not supported by WSLC", mount.Type)); - } - if (mount.UnsupportedOption.has_value()) - { - ThrowInvalidMount(value, std::format(L"option '{}' is not supported by WSLC", mount.UnsupportedOption.value())); - } - if (mount.Target.find(L':') != std::wstring::npos) - { - ThrowInvalidMount(value, L"target paths containing ':' are not supported by WSLC"); - } - - ParsedMount result; - if (mount.Type == L"tmpfs") - { - if (!mount.Source.empty()) - { - ThrowInvalidMount(value, L"source is not supported for tmpfs mounts"); - } - - std::vector options; - if (mount.ReadOnly) - { - options.emplace_back("ro"); - } - if (mount.TmpfsMode.has_value() && mount.TmpfsMode.value() != 0) - { - options.emplace_back(std::format("mode={:o}", mount.TmpfsMode.value())); - } - if (mount.TmpfsSizeBytes.has_value() && mount.TmpfsSizeBytes.value() != 0) - { - options.emplace_back(std::format("size={}", FormatDockerTmpfsSize(mount.TmpfsSizeBytes.value()))); - } - - result.IsTmpfs = true; - result.TmpfsSpec = WideToMultiByte(mount.Target); - if (!options.empty()) - { - result.TmpfsSpec += ":" + wsl::shared::string::Join(options, ','); - } - } - else - { - if (mount.Source.empty()) - { - if (mount.Type == L"volume") - { - ThrowInvalidMount(value, L"anonymous volume mounts are not supported by WSLC"); - } - - ThrowInvalidMount(value, L"source is required"); - } - - if (mount.Type == L"bind" && !std::filesystem::path(mount.Source).is_absolute()) - { - ThrowInvalidMount(value, L"bind source path must be absolute"); - } - if (mount.Type == L"volume" && !models::VolumeMount::IsValidNamedVolumeName(mount.Source)) - { - ThrowInvalidMount(value, L"volume source must be a valid named volume"); - } - - result.VolumeSpec = mount.Source + L":" + mount.Target; - if (mount.ReadOnly) - { - result.VolumeSpec += L":ro"; - } - - const auto parsed = models::VolumeMount::Parse(result.VolumeSpec); - if ((mount.Type == L"bind" && parsed.IsNamedVolume()) || (mount.Type == L"volume" && !parsed.IsNamedVolume())) - { - ThrowInvalidMount(value, std::format(L"source is not valid for mount type '{}'", mount.Type)); - } - } - - return result; -} - -} // namespace wsl::windows::wslc::validation diff --git a/src/windows/wslc/arguments/MountSpecParsing.h b/src/windows/wslc/arguments/MountSpecParsing.h deleted file mode 100644 index 7ea931cd31..0000000000 --- a/src/windows/wslc/arguments/MountSpecParsing.h +++ /dev/null @@ -1,30 +0,0 @@ -/*++ - -Copyright (c) Microsoft. All rights reserved. - -Module Name: - - MountSpecParsing.h - -Abstract: - - Declarations for parsing Docker-style --mount specifications. - ---*/ -#pragma once - -#include - -namespace wsl::windows::wslc::validation { - -struct ParsedMount -{ - bool IsTmpfs = false; - std::wstring VolumeSpec; - std::string TmpfsSpec; -}; - -// Parses a Docker-style --mount spec into the existing volume or tmpfs representation. -ParsedMount ParseMount(const std::wstring& value); - -} // namespace wsl::windows::wslc::validation diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp index 90b4f88a7b..0eb4b7dc76 100644 --- a/src/windows/wslc/services/ContainerModel.cpp +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -20,57 +20,6 @@ namespace wsl::windows::wslc::models { using namespace wsl::shared; using namespace wsl::shared::string; -namespace { - - std::string NormalizeMountDestination(std::string destination) - { - std::replace(destination.begin(), destination.end(), '\\', '/'); - - std::vector components; - size_t start = 0; - while (start <= destination.size()) - { - const auto end = destination.find('/', start); - const auto component = destination.substr(start, end - start); - if (!component.empty() && component != ".") - { - if (component == "..") - { - if (!components.empty()) - { - components.pop_back(); - } - } - else - { - components.emplace_back(component); - } - } - - if (end == std::string::npos) - { - break; - } - - start = end + 1; - } - - std::string result = "/"; - for (const auto& component : components) - { - if (result.size() > 1) - { - result += '/'; - } - - result += component; - } - - return result; - } - -} // namespace - PublishPort::PortRange PublishPort::PortRange::ParsePortPart(const std::string& portPart) { static auto parsePort = [](const std::string& value, const std::string& errorMessage) -> uint16_t { @@ -207,8 +156,7 @@ void PublishPort::Validate() const // Source: https://github.com/moby/moby/blob/master/volume/validate.go bool VolumeMount::IsValidNamedVolumeName(const std::wstring& name) { - static const std::wregex namedVolumeRegex(LR"(^[a-zA-Z0-9][a-zA-Z0-9_.-]{1,}$)"); - return std::regex_match(name, namedVolumeRegex); + return mount::IsValidNamedVolumeName(name); } VolumeMount VolumeMount::Parse(const std::wstring& value) @@ -389,7 +337,7 @@ void ValidateUniqueMountDestinations(const ContainerOptions& options) { std::unordered_set destinations; const auto addDestination = [&](const std::string& destination) { - const auto normalizedDestination = NormalizeMountDestination(destination); + const auto normalizedDestination = mount::NormalizeDestination(destination); THROW_HR_WITH_USER_ERROR_IF( E_INVALIDARG, Localization::WSLCCLI_DuplicateMountDestinationError(MultiByteToWide(normalizedDestination)), @@ -405,6 +353,11 @@ void ValidateUniqueMountDestinations(const ContainerOptions& options) { addDestination(TmpfsMount::Parse(tmpfsSpec).ContainerPath()); } + + for (const auto& mount : options.Mounts) + { + addDestination(mount.Target); + } } CidFile::CidFile(const std::optional& path) diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index 7c1d4d4475..a68d74ca1d 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -14,12 +14,15 @@ Module Name: #pragma once +#include "MountSpecParsing.h" #include #include #include namespace wsl::windows::wslc::models { +namespace mount = wsl::windows::common::mount; + // Valid formats for container list output. enum class FormatType { @@ -56,6 +59,7 @@ struct ContainerOptions bool Gpu = false; std::vector Ports; std::vector Volumes; + std::vector Mounts; std::string WorkingDirectory; std::vector Entrypoint; std::optional User{}; diff --git a/src/windows/wslc/services/ContainerService.cpp b/src/windows/wslc/services/ContainerService.cpp index 5b594c5bb6..504f6262f4 100644 --- a/src/windows/wslc/services/ContainerService.cpp +++ b/src/windows/wslc/services/ContainerService.cpp @@ -29,6 +29,8 @@ Module Name: #include namespace wsl::windows::wslc::services { +namespace mount = wsl::windows::common::mount; + using wsl::windows::common::ClientRunningWSLCProcess; using wsl::windows::common::wslc_schema::InspectContainer; using namespace wsl::windows::common::wslutil; @@ -141,6 +143,24 @@ static wsl::windows::common::RunningWSLCContainer CreateInternal(Terminal& termi } } + for (const auto& mountSpec : options.Mounts) + { + switch (mountSpec.MountType) + { + case mount::Type::Bind: + containerLauncher.AddVolume(mountSpec.Source, mountSpec.Target, mountSpec.ReadOnly); + break; + + case mount::Type::Volume: + containerLauncher.AddNamedVolume(string::WideToMultiByte(mountSpec.Source), mountSpec.Target, mountSpec.ReadOnly); + break; + + case mount::Type::Tmpfs: + containerLauncher.AddTmpfs(mountSpec.Target, mount::FormatTmpfsOptions(mountSpec)); + break; + } + } + containerLauncher.SetContainerFlags(containerFlags); if (options.StopSignal != WSLCSignalNone) diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index 1d4304a8bd..a9ef2f8e68 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -692,18 +692,7 @@ void SetContainerOptionsFromArgs(CLIExecutionContext& context) if (context.Args.Contains(ArgType::Mount)) { - auto mounts = context.Args.GetAllValues(); - for (auto& parsed : mounts) - { - if (parsed.IsTmpfs) - { - options.Tmpfs.emplace_back(std::move(parsed.TmpfsSpec)); - } - else - { - options.Volumes.emplace_back(std::move(parsed.VolumeSpec)); - } - } + options.Mounts = context.Args.GetAllValues(); } options.Remove = context.Args.GetValue(); diff --git a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp index e9778eb5a6..e801cd3b5a 100644 --- a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp @@ -15,169 +15,369 @@ Module Name: #include "precomp.h" #include "windows/Common.h" #include "WSLCCLITestHelpers.h" -#include "ArgumentValidation.h" #include "ContainerModel.h" -#include "Exceptions.h" #include "MountSpecParsing.h" -using namespace wsl::windows::wslc; +using namespace wsl::windows::common; using namespace wsl::windows::wslc::models; using namespace WEX::Logging; using namespace WEX::Common; namespace WSLCCLIMountParserUnitTests { -class WSLCCLIMountParserUnitTests -{ - WSLC_TEST_CLASS(WSLCCLIMountParserUnitTests) - - static void VerifyVolume(const std::wstring& spec, const std::wstring& expected) - { - const auto mount = validation::ParseMount(spec); - VERIFY_IS_FALSE(mount.IsTmpfs); - VERIFY_ARE_EQUAL(expected, mount.VolumeSpec); - VERIFY_IS_TRUE(mount.TmpfsSpec.empty()); - } - - static void VerifyTmpfs(const std::wstring& spec, const std::string& expected) - { - const auto mount = validation::ParseMount(spec); - VERIFY_IS_TRUE(mount.IsTmpfs); - VERIFY_ARE_EQUAL(expected, mount.TmpfsSpec); - VERIFY_IS_TRUE(mount.VolumeSpec.empty()); - } - - static void VerifyInvalid(const std::wstring& spec, const std::wstring& expectedReason) - { - Log::Comment(String().Format(L"Rejecting: %ls", spec.c_str())); - try - { - (void)validation::ParseMount(spec); - VERIFY_FAIL(L"Expected ArgumentException for invalid mount spec"); - } - catch (const ArgumentException& ex) - { - const auto& message = ex.Message(); - VERIFY_IS_TRUE(message.find(L"for '--mount' flag") != std::wstring::npos); - VERIFY_IS_TRUE(message.find(expectedReason) != std::wstring::npos); - } - } +namespace { - TEST_METHOD(Mount_KeysAndTypeAreCaseInsensitive) + struct ValidMountCase { - VerifyVolume(L"TYPE=VOLUME,SOURCE=data-volume,TARGET=/data", L"data-volume:/data"); - } + const wchar_t* Input; + mount::Type Type; + const wchar_t* Source; + const char* Target; + bool ReadOnly; + std::optional TmpfsSizeBytes; + std::optional TmpfsMode; + const char* TmpfsOptions; + }; - TEST_METHOD(Mount_AliasesMatchDocker) + struct InvalidMountCase { - VerifyVolume(L"type=volume,src=data-volume,dst=/data,ro", L"data-volume:/data:ro"); - VerifyVolume(L"type=volume,source=data-volume,destination=/data", L"data-volume:/data"); - } + const wchar_t* Input; + const wchar_t* ExpectedReason; + }; - TEST_METHOD(Mount_DefaultTypeIsVolume) - { - VerifyVolume(L"source=data-volume,target=/data", L"data-volume:/data"); - } - - TEST_METHOD(Mount_ReadOnlyUsesGoBooleanSpellings) - { - VerifyVolume(L"type=volume,source=data-volume,target=/data,readonly=t", L"data-volume:/data:ro"); - VerifyVolume(L"type=volume,source=data-volume,target=/data,readonly=TRUE", L"data-volume:/data:ro"); - VerifyVolume(L"type=volume,source=data-volume,target=/data,readonly=0", L"data-volume:/data"); - VerifyVolume(L"type=volume,source=data-volume,target=/data,readonly=F", L"data-volume:/data"); - } + constexpr ValidMountCase c_validMountCases[] = { + {L"source=data-volume,target=/data", mount::Type::Volume, L"data-volume", "/data", false, {}, {}, ""}, + {L"TYPE=VOLUME,SOURCE=data-volume,TARGET=/data", mount::Type::Volume, L"data-volume", "/data", false, {}, {}, ""}, + {L"type=VoLuMe,source=data-volume,target=/data", mount::Type::Volume, L"data-volume", "/data", false, {}, {}, ""}, + {L"type=volume,src=data-volume,dst=/data", mount::Type::Volume, L"data-volume", "/data", false, {}, {}, ""}, + {L"type=volume,src=data-volume,destination=/data", mount::Type::Volume, L"data-volume", "/data", false, {}, {}, ""}, + {L"type=volume,source=first,source=second,target=/data", mount::Type::Volume, L"second", "/data", false, {}, {}, ""}, + {L"type=volume,source=data-volume,target=/first,target=/second", + mount::Type::Volume, + L"data-volume", + "/second", + false, + {}, + {}, + ""}, + {L"type=volume,type=bind,source=C:\\data,target=/data", mount::Type::Bind, L"C:\\data", "/data", false, {}, {}, ""}, + {L"type=volume,source=data-volume,target=/data,readonly", mount::Type::Volume, L"data-volume", "/data", true, {}, {}, ""}, + {L"type=volume,source=data-volume,target=/data,ro", mount::Type::Volume, L"data-volume", "/data", true, {}, {}, ""}, + {L"type=volume,source=data-volume,target=/data,readonly=1", + mount::Type::Volume, + L"data-volume", + "/data", + true, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=t", + mount::Type::Volume, + L"data-volume", + "/data", + true, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=T", + mount::Type::Volume, + L"data-volume", + "/data", + true, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=TRUE", + mount::Type::Volume, + L"data-volume", + "/data", + true, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=true", + mount::Type::Volume, + L"data-volume", + "/data", + true, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=True", + mount::Type::Volume, + L"data-volume", + "/data", + true, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=0", + mount::Type::Volume, + L"data-volume", + "/data", + false, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=f", + mount::Type::Volume, + L"data-volume", + "/data", + false, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=F", + mount::Type::Volume, + L"data-volume", + "/data", + false, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=FALSE", + mount::Type::Volume, + L"data-volume", + "/data", + false, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=false", + mount::Type::Volume, + L"data-volume", + "/data", + false, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=False", + mount::Type::Volume, + L"data-volume", + "/data", + false, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,readonly=true,readonly=false", + mount::Type::Volume, + L"data-volume", + "/data", + false, + {}, + {}, + ""}, + {L"type=bind,\"source=C:\\mount,a\",target=/data", mount::Type::Bind, L"C:\\mount,a", "/data", false, {}, {}, ""}, + {L"type=bind,source=C:\\mount with spaces,target=/data", + mount::Type::Bind, + L"C:\\mount with spaces", + "/data", + false, + {}, + {}, + ""}, + {L"type=bind,source=C:\\,target=/data", mount::Type::Bind, L"C:\\", "/data", false, {}, {}, ""}, + {L"type=bind,source=\\\\server\\share,target=/data", mount::Type::Bind, L"\\\\server\\share", "/data", false, {}, {}, ""}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=enabled", + mount::Type::Bind, + L"C:\\mount", + "/data", + false, + {}, + {}, + ""}, + {L"type=volume,source=data-volume,target=/data,bind-recursive=enabled", + mount::Type::Volume, + L"data-volume", + "/data", + false, + {}, + {}, + ""}, + {L"type=volume,source=A_,target=/data", mount::Type::Volume, L"A_", "/data", false, {}, {}, ""}, + {L"type=volume,source=data.volume-1,target=/data", mount::Type::Volume, L"data.volume-1", "/data", false, {}, {}, ""}, + {L"type=tmpfs,target=/tmp", mount::Type::Tmpfs, L"", "/tmp", false, {}, {}, ""}, + {L"type=tmpfs,target=/tmp,readonly", mount::Type::Tmpfs, L"", "/tmp", true, {}, {}, "ro"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=0", mount::Type::Tmpfs, L"", "/tmp", false, 0, {}, ""}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1", mount::Type::Tmpfs, L"", "/tmp", false, 1, {}, "size=1"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1024", mount::Type::Tmpfs, L"", "/tmp", false, 1024, {}, "size=1k"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1536", mount::Type::Tmpfs, L"", "/tmp", false, 1536, {}, "size=1536"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1k", mount::Type::Tmpfs, L"", "/tmp", false, 1024, {}, "size=1k"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1KB", mount::Type::Tmpfs, L"", "/tmp", false, 1024, {}, "size=1k"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1KiB", mount::Type::Tmpfs, L"", "/tmp", false, 1024, {}, "size=1k"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1MB", mount::Type::Tmpfs, L"", "/tmp", false, 1LL << 20, {}, "size=1m"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1MiB", mount::Type::Tmpfs, L"", "/tmp", false, 1LL << 20, {}, "size=1m"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1GB", mount::Type::Tmpfs, L"", "/tmp", false, 1LL << 30, {}, "size=1g"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1.5MB", mount::Type::Tmpfs, L"", "/tmp", false, 1536LL << 10, {}, "size=1536k"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=+1MB", mount::Type::Tmpfs, L"", "/tmp", false, 1LL << 20, {}, "size=1m"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1e3", mount::Type::Tmpfs, L"", "/tmp", false, 1000, {}, "size=1000"}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=0000", mount::Type::Tmpfs, L"", "/tmp", false, {}, 0, ""}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=0700", mount::Type::Tmpfs, L"", "/tmp", false, {}, 0700, "mode=700"}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=+0700", mount::Type::Tmpfs, L"", "/tmp", false, {}, 0700, "mode=700"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1MB,tmpfs-mode=0700,readonly", + mount::Type::Tmpfs, + L"", + "/tmp", + true, + 1LL << 20, + 0700, + "ro,mode=700,size=1m"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=0,tmpfs-mode=0000,readonly=false", mount::Type::Tmpfs, L"", "/tmp", false, 0, 0, ""}, + }; - TEST_METHOD(Mount_CsvQuotedFieldPreservesComma) - { - VerifyVolume(L"type=bind,\"source=C:\\mount,a\",target=/data", L"C:\\mount,a:/data"); - } + constexpr InvalidMountCase c_invalidMountCases[] = { + {L"", L"invalid field '' must be a key=value pair"}, + {L",", L"invalid field '' must be a key=value pair"}, + {L"type=volume,source=data-volume,target=/data,", L"invalid field '' must be a key=value pair"}, + {L",type=volume,source=data-volume,target=/data", L"invalid field '' must be a key=value pair"}, + {L"type=bind,\"source=C:\\mount,target=/data", L"malformed CSV"}, + {L"type=volume,bogus", L"invalid field 'bogus' must be a key=value pair"}, + {L"type=volume,bogus=value", L"unexpected key 'bogus'"}, + {L"type", L"invalid field 'type' must be a key=value pair"}, + {L"source", L"invalid field 'source' must be a key=value pair"}, + {L"target", L"invalid field 'target' must be a key=value pair"}, + {L"type=,source=data-volume,target=/data", L"type is required"}, + {L"type=volume,source=data-volume", L"target is required"}, + {L"type=volume,source=data-volume,target=", L"target is required"}, + {L"type=volume,source=data-volume,dst=", L"target is required"}, + {L"type=cluster,source=data-volume,target=/data", L"mount type 'cluster' is not supported."}, + {L"type=npipe,source=data-volume,target=/data", L"mount type 'npipe' is not supported."}, + {L"type=bogus,source=data-volume,target=/data", L"mount type 'bogus' is not supported."}, + {L"type=CLUSTER,source=data-volume,target=/data", L"mount type 'cluster' is not supported."}, + {L"type=volume,target=/data", L"anonymous volume mounts are not supported."}, + {L"type=bind,target=/data", L"source is required"}, + {L"type=bind,source=relative,target=/data", L"bind source path must be absolute"}, + {L"type=volume,source=a,target=/data", L"volume source must be a valid named volume"}, + {L"type=volume,source=data/volume,target=/data", L"volume source must be a valid named volume"}, + {L"type=volume,source=C:\\mount,target=/data", L"volume source must be a valid named volume"}, + {L"type=tmpfs,source=data-volume,target=/data", L"source is not supported for tmpfs mounts"}, + {L"type=volume,source=data-volume,target=/data:part", L"target paths containing ':' are not supported."}, + {L"type=volume,source=data-volume,target=/data,readonly=no", L"invalid value for readonly: no"}, + {L"type=volume,source=data-volume,target=/data,readonly=yes", L"invalid value for readonly: yes"}, + {L"type=volume,source=data-volume,target=/data,readonly=", L"invalid value for readonly: "}, + {L"type=volume,source=data-volume,target=/data,readonly=2", L"invalid value for readonly: 2"}, + {L"type=volume,source=data-volume,target=/data,volume-nocopy=no", L"invalid value for volume-nocopy: no"}, + {L"type=volume,source=data-volume,target=/data,volume-nocopy=", L"invalid value for volume-nocopy: "}, + {L"type=bind,source=C:\\mount,target=/data,bind-nonrecursive=no", L"invalid value for bind-nonrecursive: no"}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=", L"invalid value for bind-recursive: "}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=Enabled", L"invalid value for bind-recursive: Enabled"}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=bogus", L"invalid value for bind-recursive: bogus"}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=writable", + L"option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction"}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=readonly", + L"option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction"}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=readonly,readonly", + L"option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction"}, + {L"type=bind,source=C:\\mount,target=/data,consistency=cached", L"option 'consistency' is not supported."}, + {L"type=bind,source=C:\\mount,target=/data,bind-propagation=rprivate", L"option 'bind-propagation' is not supported."}, + {L"type=bind,source=C:\\mount,target=/data,bind-nonrecursive", L"option 'bind-nonrecursive' is not supported."}, + {L"type=bind,source=C:\\mount,target=/data,bind-nonrecursive=true", L"option 'bind-nonrecursive' is not supported."}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=disabled", L"option 'bind-recursive' is not supported."}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=writable,readonly", + L"option 'bind-recursive' is not supported."}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=readonly,readonly,bind-propagation=rprivate", + L"option 'bind-recursive' is not supported."}, + {L"type=volume,source=data-volume,target=/data,volume-nocopy", L"option 'volume-nocopy' is not supported."}, + {L"type=volume,source=data-volume,target=/data,volume-nocopy=true", L"option 'volume-nocopy' is not supported."}, + {L"type=volume,source=data-volume,target=/data,volume-label=a=b", L"option 'volume-label' is not supported."}, + {L"type=volume,source=data-volume,target=/data,volume-driver=local", L"option 'volume-driver' is not supported."}, + {L"type=volume,source=data-volume,target=/data,volume-opt=a=b", L"option 'volume-opt' is not supported."}, + {L"type=bind,source=C:\\mount,target=/data,volume-nocopy=true", L"cannot mix 'volume-*' options with mount type 'bind'"}, + {L"type=volume,source=data-volume,target=/data,bind-propagation=rprivate", + L"cannot mix 'bind-*' options with mount type 'volume'"}, + {L"type=volume,source=data-volume,target=/data,tmpfs-size=1m", L"cannot mix 'tmpfs-*' options with mount type 'volume'"}, + {L"type=tmpfs,target=/tmp,volume-label=a=b", L"cannot mix 'volume-*' options with mount type 'tmpfs'"}, + {L"type=tmpfs,target=/tmp,bind-nonrecursive", L"cannot mix 'bind-*' options with mount type 'tmpfs'"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=", L"invalid value for tmpfs-size: "}, + {L"type=tmpfs,target=/tmp,tmpfs-size=bad", L"invalid value for tmpfs-size: bad"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=-1", L"invalid value for tmpfs-size: -1"}, + {L"type=tmpfs,target=/tmp,\"tmpfs-size=1,5MB\"", L"invalid value for tmpfs-size: 1,5MB"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1XB", L"invalid value for tmpfs-size: 1XB"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1Ki", L"invalid value for tmpfs-size: 1Ki"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1BB", L"invalid value for tmpfs-size: 1BB"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=nan", L"invalid value for tmpfs-size: nan"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=inf", L"invalid value for tmpfs-size: inf"}, + {L"type=tmpfs,target=/tmp,tmpfs-size=9223372036854775808", L"invalid value for tmpfs-size: 9223372036854775808"}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=", L"invalid value for tmpfs-mode: "}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=-1", L"invalid value for tmpfs-mode: -1"}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=8", L"invalid value for tmpfs-mode: 8"}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=0899", L"invalid value for tmpfs-mode: 0899"}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=0x700", L"invalid value for tmpfs-mode: 0x700"}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=40000000000", L"invalid value for tmpfs-mode: 40000000000"}, + }; - TEST_METHOD(Mount_BindRecursiveEnabledIsDefaultBehavior) - { - VerifyVolume(L"type=bind,source=C:\\mount,target=/data,bind-recursive=enabled", L"C:\\mount:/data"); - } +} // namespace - TEST_METHOD(Mount_TmpfsOptionsMatchDockerConversion) - { - VerifyTmpfs(L"type=tmpfs,target=/tmp,tmpfs-size=1MB,tmpfs-mode=0700,readonly", "/tmp:ro,mode=700,size=1m"); - VerifyTmpfs(L"type=tmpfs,target=/tmp,tmpfs-size=1.5MB", "/tmp:size=1536k"); - VerifyTmpfs(L"type=tmpfs,target=/tmp,tmpfs-size=1536", "/tmp:size=1536"); - VerifyTmpfs(L"type=tmpfs,target=/tmp,tmpfs-size=0,tmpfs-mode=0000", "/tmp"); - } +class WSLCCLIMountParserUnitTests +{ + WSLC_TEST_CLASS(WSLCCLIMountParserUnitTests) - TEST_METHOD(Mount_InvalidFieldsMatchDocker) + TEST_METHOD(Mount_ValidCases) { - VerifyInvalid(L"type=volume,bogus", L"invalid field 'bogus' must be a key=value pair"); - VerifyInvalid(L"type=volume,bogus=value", L"unexpected key 'bogus'"); - VerifyInvalid(L"type=volume,source=data-volume,target=/data,readonly=no", L"invalid value for readonly: no"); - VerifyInvalid(L"type=tmpfs,target=/tmp,tmpfs-size=bad", L"invalid value for tmpfs-size: bad"); - VerifyInvalid(L"type=tmpfs,target=/tmp,\"tmpfs-size=1,5MB\"", L"invalid value for tmpfs-size: 1,5MB"); - VerifyInvalid( - L"type=tmpfs,target=/tmp,tmpfs-size=9223372036854775808", L"invalid value for tmpfs-size: 9223372036854775808"); - VerifyInvalid(L"type=tmpfs,target=/tmp,tmpfs-mode=0899", L"invalid value for tmpfs-mode: 0899"); - VerifyInvalid(L"type=bind,source=C:\\mount,target=/data,bind-recursive=Enabled", L"invalid value for bind-recursive"); - } + for (const auto& testCase : c_validMountCases) + { + Log::Comment(String().Format(L"Accepting: %ls", testCase.Input)); - TEST_METHOD(Mount_RequiredFieldsMatchDocker) - { - VerifyInvalid(L"type=,source=data-volume,target=/data", L"type is required"); - VerifyInvalid(L"type=volume,source=data-volume", L"target is required"); - } + const auto actual = mount::Parse(testCase.Input); + VERIFY_ARE_EQUAL(static_cast(testCase.Type), static_cast(actual.MountType)); + VERIFY_ARE_EQUAL(std::wstring(testCase.Source), actual.Source); + VERIFY_ARE_EQUAL(std::string(testCase.Target), actual.Target); + VERIFY_ARE_EQUAL(testCase.ReadOnly, actual.ReadOnly); + VERIFY_ARE_EQUAL(testCase.TmpfsSizeBytes.has_value(), actual.TmpfsSizeBytes.has_value()); + if (testCase.TmpfsSizeBytes.has_value() && actual.TmpfsSizeBytes.has_value()) + { + VERIFY_ARE_EQUAL(testCase.TmpfsSizeBytes.value(), actual.TmpfsSizeBytes.value()); + } - TEST_METHOD(Mount_OptionTypeConflictsMatchDocker) - { - VerifyInvalid( - L"type=bind,source=C:\\mount,target=/data,volume-nocopy=true", - L"cannot mix 'volume-*' options with mount type 'bind'"); - VerifyInvalid( - L"type=volume,source=data-volume,target=/data,bind-propagation=rprivate", - L"cannot mix 'bind-*' options with mount type 'volume'"); - VerifyInvalid( - L"type=volume,source=data-volume,target=/data,tmpfs-size=1m", - L"cannot mix 'tmpfs-*' options with mount type 'volume'"); - } + VERIFY_ARE_EQUAL(testCase.TmpfsMode.has_value(), actual.TmpfsMode.has_value()); + if (testCase.TmpfsMode.has_value() && actual.TmpfsMode.has_value()) + { + VERIFY_ARE_EQUAL(testCase.TmpfsMode.value(), actual.TmpfsMode.value()); + } - TEST_METHOD(Mount_BindRecursiveValidationMatchesDocker) - { - VerifyInvalid( - L"type=bind,source=C:\\mount,target=/data,bind-recursive=writable", - L"requires 'readonly' to be specified in conjunction"); - VerifyInvalid( - L"type=bind,source=C:\\mount,target=/data,bind-recursive=readonly,readonly", - L"requires 'bind-propagation=rprivate' to be specified in conjunction"); + const auto actualTmpfsOptions = actual.MountType == mount::Type::Tmpfs ? mount::FormatTmpfsOptions(actual) : std::string{}; + VERIFY_ARE_EQUAL(std::string(testCase.TmpfsOptions), actualTmpfsOptions); + } } - TEST_METHOD(Mount_UnsupportedBackendFeaturesAreExplicit) + TEST_METHOD(Mount_InvalidCases) { - VerifyInvalid( - L"type=volume,source=data-volume,target=/data,volume-nocopy", L"option 'volume-nocopy' is not supported by WSLC"); - VerifyInvalid( - L"type=bind,source=C:\\mount,target=/data,consistency=cached", L"option 'consistency' is not supported by WSLC"); - VerifyInvalid(L"type=cluster,source=data-volume,target=/data", L"mount type 'cluster' is not supported by WSLC"); - VerifyInvalid(L"type=volume,target=/data", L"anonymous volume mounts are not supported by WSLC"); - } + for (const auto& testCase : c_invalidMountCases) + { + Log::Comment(String().Format(L"Rejecting: %ls", testCase.Input)); - TEST_METHOD(Mount_BackendRepresentationLimitsAreExplicit) - { - VerifyInvalid(L"type=bind,source=relative,target=/data", L"bind source path must be absolute"); - VerifyInvalid(L"type=volume,source=C:\\mount,target=/data", L"volume source must be a valid named volume"); - VerifyInvalid(L"type=tmpfs,source=data-volume,target=/data", L"source is not supported for tmpfs mounts"); - VerifyInvalid( - L"type=volume,source=data-volume,target=/data:part", L"target paths containing ':' are not supported by WSLC"); + try + { + (void)mount::Parse(testCase.Input); + VERIFY_FAIL(L"Expected ParseException for invalid mount spec"); + } + catch (const mount::ParseException& ex) + { + VERIFY_IS_TRUE(ex.Reason().find(testCase.ExpectedReason) != std::wstring::npos); + } + } } - TEST_METHOD(Mount_MalformedCsvIsRejected) + TEST_METHOD(Mount_DotRelativeBindSourceUsesCurrentDirectory) { - VerifyInvalid(L"type=bind,\"source=C:\\mount,target=/data", L"malformed CSV"); + const auto expected = (std::filesystem::current_path() / L"mount").lexically_normal().wstring(); + const auto actual = mount::Parse(L"type=bind,source=.\\mount,target=/data"); + VERIFY_ARE_EQUAL(expected, actual.Source); } TEST_METHOD(Mount_DuplicateDestinationsAreRejected) { ContainerOptions options; - options.Tmpfs = {"/data", "/data/"}; + options.Tmpfs = {"/data"}; + options.Mounts = { + {.MountType = mount::Type::Volume, .Source = L"data-volume", .Target = "/data/"}, + }; VERIFY_THROWS(ValidateUniqueMountDestinations(options), wil::ResultException); - options.Tmpfs = {"/data/../cache"}; + options.Tmpfs.clear(); + options.Mounts = { + {.MountType = mount::Type::Tmpfs, .Target = "/data/../cache"}, + }; options.Volumes = {L"data-volume:/cache"}; VERIFY_THROWS(ValidateUniqueMountDestinations(options), wil::ResultException); } @@ -187,6 +387,9 @@ class WSLCCLIMountParserUnitTests ContainerOptions options; options.Tmpfs = {"/cache"}; options.Volumes = {L"data-volume:/data"}; + options.Mounts = { + {.MountType = mount::Type::Bind, .Source = L"C:\\logs", .Target = "/logs"}, + }; VERIFY_NO_THROW(ValidateUniqueMountDestinations(options)); } }; From 5d4212c3f40b5e95b7aa28d5de99f40bbeaf86fc Mon Sep 17 00:00:00 2001 From: David Bennett Date: Thu, 13 Aug 2026 09:14:05 -0700 Subject: [PATCH 04/12] PR feedback and format fix --- localization/strings/en-US/Resources.resw | 2 +- src/windows/common/MountSpecParsing.cpp | 7 + .../wslc/WSLCCLIMountParserUnitTests.cpp | 5 + .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 127 ++++++++++++++++++ .../wslc/e2e/WSLCE2EContainerRunTests.cpp | 14 ++ 5 files changed, 154 insertions(+), 1 deletion(-) diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index 83b020f94a..0d0578e9d4 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2981,7 +2981,7 @@ On first run, creates the file with all settings commented out at their defaults Duplicate mount point: {} - {FixedPlaceholder="{}"}File names and string inserts should not be translated + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated Follow log output diff --git a/src/windows/common/MountSpecParsing.cpp b/src/windows/common/MountSpecParsing.cpp index 7b52e2637e..d99569f8e1 100644 --- a/src/windows/common/MountSpecParsing.cpp +++ b/src/windows/common/MountSpecParsing.cpp @@ -15,6 +15,7 @@ Module Name: #include "precomp.h" #include "MountSpecParsing.h" #include +#include #include #include #include @@ -513,11 +514,17 @@ Spec Parse(const std::wstring& value) { ThrowInvalid(std::format(L"option '{}' is not supported.", mount.UnsupportedOption.value())); } + if (mount.Target.find(L':') != std::wstring::npos) { ThrowInvalid(L"target paths containing ':' are not supported."); } + if (!mount.Target.starts_with(L'/')) + { + ThrowInvalid(L"target path must be absolute"); + } + if (type == Type::Tmpfs) { if (!mount.Source.empty()) diff --git a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp index e801cd3b5a..355ba21ba6 100644 --- a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp @@ -252,6 +252,11 @@ namespace { {L"type=volume,source=C:\\mount,target=/data", L"volume source must be a valid named volume"}, {L"type=tmpfs,source=data-volume,target=/data", L"source is not supported for tmpfs mounts"}, {L"type=volume,source=data-volume,target=/data:part", L"target paths containing ':' are not supported."}, + {L"type=volume,source=data-volume,target=data", L"target path must be absolute"}, + {L"type=volume,source=data-volume,dst=.", L"target path must be absolute"}, + {L"type=volume,source=data-volume,destination=\\data", L"target path must be absolute"}, + {L"type=bind,source=C:\\mount,target=data", L"target path must be absolute"}, + {L"type=tmpfs,target=data", L"target path must be absolute"}, {L"type=volume,source=data-volume,target=/data,readonly=no", L"invalid value for readonly: no"}, {L"type=volume,source=data-volume,target=/data,readonly=yes", L"invalid value for readonly: yes"}, {L"type=volume,source=data-volume,target=/data,readonly=", L"invalid value for readonly: "}, diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index 9dc0291992..1e7a27968f 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -46,6 +46,7 @@ class WSLCE2EContainerCreateTests EnsureImageIsDeleted(AlpineImage); EnsureImageIsDeleted(DebianImage); EnsureImageIsDeleted(HelloWorldImage); + EnsureVolumeDoesNotExist(WslcVolumeName); EnsureNetworkDoesNotExist(TestNetworkName); VERIFY_IS_TRUE(::SetEnvironmentVariableW(HostEnvVariableName.c_str(), nullptr)); @@ -61,6 +62,7 @@ class WSLCE2EContainerCreateTests VolumeTestFile1 = wsl::windows::common::filesystem::GetTempFilename(); VolumeTestFile2 = wsl::windows::common::filesystem::GetTempFilename(); EnsureContainerDoesNotExist(WslcContainerName); + EnsureVolumeDoesNotExist(WslcVolumeName); EnsureNetworkDoesNotExist(TestNetworkName); return true; } @@ -711,6 +713,128 @@ class WSLCE2EContainerCreateTests result.StderrContainsSubstring(L"invalid mount path: '' mount path must be absolute\r\nError code: E_FAIL")); } + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_Tmpfs_Success) + { + auto result = RunWslc(std::format( + L"container create --name {} --mount type=tmpfs,target=/wslc-tmpfs {} sh -c \"echo -n 'tmpfs_test' > " + L"/wslc-tmpfs/data && cat /wslc-tmpfs/data\"", + WslcContainerName, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = 0}); + + result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); + result.Verify({.Stdout = L"tmpfs_test", .Stderr = L"", .ExitCode = 0}); + } + + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_Bind_Success) + { + WriteTestFileContent(VolumeTestFile1, "WSLC Mount Bind Test"); + + const auto hostDirectory = VolumeTestFile1.parent_path(); + const auto fileName = VolumeTestFile1.filename().wstring(); + auto result = RunWslc(std::format( + L"container create --name {} --mount \"type=bind,source={},target=/data,readonly\" {} cat /data/{}", + WslcContainerName, + hostDirectory.wstring(), + DebianImage.NameAndTag(), + fileName)); + result.Verify({.Stderr = L"", .ExitCode = 0}); + + result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); + result.Verify({.Stdout = L"WSLC Mount Bind Test", .Stderr = L"", .ExitCode = 0}); + } + + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_Volume_Success) + { + auto result = RunWslc(std::format( + L"container create --name {} --mount type=volume,source={},target=/data {} sh -c \"echo -n 'WSLC Mount Volume " + L"Test' > /data/test.txt\"", + WslcContainerName, + WslcVolumeName, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = 0}); + + result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); + result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); + EnsureContainerDoesNotExist(WslcContainerName); + + result = RunWslc(std::format( + L"container create --name {} --mount type=volume,source={},target=/data {} cat /data/test.txt", + WslcContainerName, + WslcVolumeName, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = 0}); + + result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); + result.Verify({.Stdout = L"WSLC Mount Volume Test", .Stderr = L"", .ExitCode = 0}); + } + + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_ReadOnly_IsReadOnly) + { + auto result = RunWslc(std::format( + L"container create --name {} --mount type=volume,source={},target=/data {} sh -c \"echo -n original > /data/value\"", + WslcContainerName, + WslcVolumeName, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = 0}); + + result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); + result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); + EnsureContainerDoesNotExist(WslcContainerName); + + result = RunWslc(std::format( + L"container create --name {} --mount type=volume,source={},target=/data,readonly {} sh -c \"echo changed > " + L"/data/value\"", + WslcContainerName, + WslcVolumeName, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = 0}); + + result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); + result.Verify({.Stdout = L"", .Stderr = L"sh: 1: cannot create /data/value: Read-only file system\n", .ExitCode = 2}); + EnsureContainerDoesNotExist(WslcContainerName); + + result = RunWslc(std::format( + L"container create --name {} --mount type=volume,source={},target=/data {} cat /data/value", + WslcContainerName, + WslcVolumeName, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = 0}); + + result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); + result.Verify({.Stdout = L"original", .Stderr = L"", .ExitCode = 0}); + } + + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_InvalidType_Fails) + { + auto result = RunWslc(std::format( + L"container create --name {} --mount type=bogus,target=/x {} true", WslcContainerName, DebianImage.NameAndTag())); + VERIFY_ARE_EQUAL(1u, result.ExitCode.value()); + VERIFY_IS_TRUE(result.Stderr.has_value()); + VERIFY_ARE_NOT_EQUAL(std::wstring::npos, result.Stderr->find(L"for '--mount' flag")); + EnsureContainerDoesNotExist(WslcContainerName); + } + + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_RelativeTarget_Fails) + { + auto result = RunWslc(std::format( + L"container create --name {} --mount type=tmpfs,target=data {} true", WslcContainerName, DebianImage.NameAndTag())); + result.Verify({.Stdout = L"", .ExitCode = 1}); + VERIFY_IS_TRUE(result.StderrContainsSubstring(L"target path must be absolute")); + EnsureContainerDoesNotExist(WslcContainerName); + } + + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_DuplicateDestination_Fails) + { + auto result = RunWslc(std::format( + L"container create --name {} --mount type=tmpfs,target=/data --mount type=tmpfs,target=/data/ {} true", + WslcContainerName, + DebianImage.NameAndTag())); + result.Verify({.Stdout = L"", .ExitCode = 1}); + VERIFY_IS_TRUE(result.StderrContainsSubstring(L"Duplicate mount point: /data")); + EnsureContainerDoesNotExist(WslcContainerName); + } + WSLC_TEST_METHOD(WSLCE2E_Container_Create_WorkDir) { auto result = @@ -1536,6 +1660,9 @@ while True: // Test network name const std::wstring TestNetworkName = L"wslc-test-network"; + // Test named volume + const std::wstring WslcVolumeName = L"wslc-test-volume"; + // Test environment variables const std::wstring HostEnvVariableName = L"WSLC_TEST_HOST_ENV"; const std::wstring HostEnvVariableName2 = L"WSLC_TEST_HOST_ENV2"; diff --git a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp index d704b5074a..7e76fc568c 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp @@ -1047,6 +1047,20 @@ class WSLCE2EContainerRunTests result.Verify({.Stdout = L"tmpfs_test", .Stderr = L"", .ExitCode = 0}); } + WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_Bind_Success) + { + WriteTestFileContent(EnvTestFile1, "WSLC Mount Bind Test"); + + const auto hostDirectory = EnvTestFile1.parent_path(); + const auto fileName = EnvTestFile1.filename().wstring(); + auto result = RunWslc(std::format( + L"container run --rm --mount \"type=bind,source={},target=/data,readonly\" {} cat /data/{}", + hostDirectory.wstring(), + DebianImage.NameAndTag(), + fileName)); + result.Verify({.Stdout = L"WSLC Mount Bind Test", .Stderr = L"", .ExitCode = 0}); + } + WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_Volume_Success) { auto result = RunWslc(std::format( From f10b9616c3d1b7c16ec6521aa068b43e217e85dd Mon Sep 17 00:00:00 2001 From: David Bennett Date: Thu, 13 Aug 2026 09:46:47 -0700 Subject: [PATCH 05/12] Remove magic number, fix headers --- src/windows/common/MountSpecParsing.cpp | 7 +++++-- src/windows/wslc/arguments/SpecParsing.cpp | 1 - 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/windows/common/MountSpecParsing.cpp b/src/windows/common/MountSpecParsing.cpp index d99569f8e1..18ecead366 100644 --- a/src/windows/common/MountSpecParsing.cpp +++ b/src/windows/common/MountSpecParsing.cpp @@ -14,6 +14,7 @@ Module Name: #include "precomp.h" #include "MountSpecParsing.h" +#include #include #include #include @@ -22,6 +23,8 @@ Module Name: #include #include #include +#include +#include using namespace wsl::shared; using namespace wsl::shared::string; @@ -243,8 +246,8 @@ namespace { } } - constexpr double c_int64Limit = 9223372036854775808.0; - if (!std::isfinite(bytes) || bytes >= c_int64Limit) + constexpr double c_int64ExclusiveUpperBound = static_cast(uint64_t{1} << std::numeric_limits::digits); + if (!std::isfinite(bytes) || bytes >= c_int64ExclusiveUpperBound) { return std::nullopt; } diff --git a/src/windows/wslc/arguments/SpecParsing.cpp b/src/windows/wslc/arguments/SpecParsing.cpp index caac75c49a..c1fbdd20ea 100644 --- a/src/windows/wslc/arguments/SpecParsing.cpp +++ b/src/windows/wslc/arguments/SpecParsing.cpp @@ -22,7 +22,6 @@ Module Name: #include "JsonUtils.h" #include "Localization.h" #include -#include #include #include #include From 2fa0e3f0e98f675b0d21274c97b04ab08a007138 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Thu, 13 Aug 2026 12:48:36 -0700 Subject: [PATCH 06/12] Refactor --- src/windows/common/MountSpecParsing.cpp | 99 +++++++++++++------ src/windows/common/MountSpecParsing.h | 34 ++++++- .../wslc/arguments/ArgumentValidation.cpp | 6 +- src/windows/wslc/services/ContainerModel.cpp | 14 +++ .../wslc/WSLCCLIMountParserUnitTests.cpp | 79 ++++++++++++++- 5 files changed, 192 insertions(+), 40 deletions(-) diff --git a/src/windows/common/MountSpecParsing.cpp b/src/windows/common/MountSpecParsing.cpp index 18ecead366..097fd053bb 100644 --- a/src/windows/common/MountSpecParsing.cpp +++ b/src/windows/common/MountSpecParsing.cpp @@ -24,6 +24,7 @@ Module Name: #include #include #include +#include #include using namespace wsl::shared; @@ -124,7 +125,7 @@ namespace { [[noreturn]] void ThrowInvalid(std::wstring reason) { - throw ParseException(std::move(reason)); + throw ValidationException(std::move(reason)); } KeyValue SplitKeyValue(const std::wstring& value) @@ -302,7 +303,7 @@ namespace { } // namespace -Spec Parse(const std::wstring& value) +Spec ParseDockerMountString(const std::wstring& value) { const auto fields = SplitCsvFields(value); if (!fields.has_value()) @@ -461,10 +462,6 @@ Spec Parse(const std::wstring& value) { ThrowInvalid(L"type is required"); } - if (mount.Target.empty()) - { - ThrowInvalid(L"target is required"); - } if (mount.HasVolumeOptions && mount.Type != L"volume") { @@ -518,53 +515,97 @@ Spec Parse(const std::wstring& value) ThrowInvalid(std::format(L"option '{}' is not supported.", mount.UnsupportedOption.value())); } - if (mount.Target.find(L':') != std::wstring::npos) + return { + .MountType = type, + .Source = std::move(mount.Source), + .Target = WideToMultiByte(mount.Target), + .ReadOnly = mount.ReadOnly, + .TmpfsSizeBytes = mount.TmpfsSizeBytes, + .TmpfsMode = mount.TmpfsMode, + }; +} + +void ValidateMountSpec(const Spec& mount) +{ + if (mount.Target.empty()) + { + ThrowInvalid(L"target is required"); + } + + if (mount.Target.find(':') != std::string::npos) { ThrowInvalid(L"target paths containing ':' are not supported."); } - if (!mount.Target.starts_with(L'/')) + if (!mount.Target.starts_with('/')) { ThrowInvalid(L"target path must be absolute"); } - if (type == Type::Tmpfs) + if (mount.MountType != Type::Tmpfs && (mount.TmpfsSizeBytes.has_value() || mount.TmpfsMode.has_value())) { - if (!mount.Source.empty()) - { - ThrowInvalid(L"source is not supported for tmpfs mounts"); - } + ThrowInvalid(L"tmpfs options are only supported for tmpfs mounts"); } - else + + if (mount.TmpfsSizeBytes.has_value() && mount.TmpfsSizeBytes.value() < 0) + { + ThrowInvalid(L"tmpfs size must not be negative"); + } + + switch (mount.MountType) { + case Type::Bind: if (mount.Source.empty()) { - if (type == Type::Volume) - { - ThrowInvalid(L"anonymous volume mounts are not supported."); - } - ThrowInvalid(L"source is required"); } - if (type == Type::Bind && !std::filesystem::path(mount.Source).is_absolute()) + if (!std::filesystem::path(mount.Source).is_absolute()) { ThrowInvalid(L"bind source path must be absolute"); } - if (type == Type::Volume && !IsValidNamedVolumeName(mount.Source)) + break; + + case Type::Volume: + if (mount.Source.empty()) + { + ThrowInvalid(L"anonymous volume mounts are not supported."); + } + + if (!IsValidNamedVolumeName(mount.Source)) { ThrowInvalid(L"volume source must be a valid named volume"); } + break; + + case Type::Tmpfs: + if (!mount.Source.empty()) + { + ThrowInvalid(L"source is not supported for tmpfs mounts"); + } + break; + + default: + ThrowInvalid(L"mount type is not supported"); } +} - return { - .MountType = type, - .Source = std::move(mount.Source), - .Target = WideToMultiByte(mount.Target), - .ReadOnly = mount.ReadOnly, - .TmpfsSizeBytes = mount.TmpfsSizeBytes, - .TmpfsMode = mount.TmpfsMode, - }; +void ValidateMountCollection(std::span mounts) +{ + std::unordered_set destinations; + for (const auto& mount : mounts) + { + ValidateMountSpec(mount); + + auto destination = NormalizeDestination(mount.Target); + if (!destinations.emplace(destination).second) + { + throw ValidationException( + ValidationError::DuplicateDestination, + std::format(L"duplicate mount point: {}", MultiByteToWide(destination)), + std::move(destination)); + } + } } std::string FormatTmpfsOptions(const Spec& mount) diff --git a/src/windows/common/MountSpecParsing.h b/src/windows/common/MountSpecParsing.h index 9bd8ceafcf..fb6a5b56a1 100644 --- a/src/windows/common/MountSpecParsing.h +++ b/src/windows/common/MountSpecParsing.h @@ -17,6 +17,7 @@ Module Name: #include #include #include +#include #include #include #include @@ -42,16 +43,27 @@ struct Spec std::optional TmpfsMode; }; -class ParseException : public std::exception +enum class ValidationError +{ + InvalidSpecification, + DuplicateDestination, +}; + +class ValidationException : public std::exception { public: - explicit ParseException(std::wstring reason) : m_reason(std::move(reason)) + explicit ValidationException(std::wstring reason) : m_reason(std::move(reason)) + { + } + + ValidationException(ValidationError error, std::wstring reason, std::string destination) : + m_error(error), m_reason(std::move(reason)), m_destination(std::move(destination)) { } const char* what() const noexcept override { - return "invalid mount specification"; + return "invalid mount"; } const std::wstring& Reason() const noexcept @@ -59,11 +71,25 @@ class ParseException : public std::exception return m_reason; } + ValidationError Error() const noexcept + { + return m_error; + } + + const std::string& Destination() const noexcept + { + return m_destination; + } + private: + ValidationError m_error = ValidationError::InvalidSpecification; std::wstring m_reason; + std::string m_destination; }; -Spec Parse(const std::wstring& value); +Spec ParseDockerMountString(const std::wstring& value); +void ValidateMountSpec(const Spec& mount); +void ValidateMountCollection(std::span mounts); std::string FormatTmpfsOptions(const Spec& mount); std::string NormalizeDestination(std::string destination); bool IsValidNamedVolumeName(std::wstring_view name); diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp index 81d304a170..bcd4a40557 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.cpp +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -224,9 +224,11 @@ void Argument::Validate(ArgMap& execArgs) const CacheConverted(execArgs, m_name, [](const std::wstring& value, const std::wstring&) { try { - return mount::Parse(value); + auto mountSpec = mount::ParseDockerMountString(value); + mount::ValidateMountSpec(mountSpec); + return mountSpec; } - catch (const mount::ParseException& ex) + catch (const mount::ValidationException& ex) { throw ArgumentException(Localization::WSLCCLI_InvalidMountError(value, ex.Reason())); } diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp index 0eb4b7dc76..fa298f4f76 100644 --- a/src/windows/wslc/services/ContainerModel.cpp +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -335,6 +335,20 @@ TmpfsMount TmpfsMount::Parse(const std::string& value) void ValidateUniqueMountDestinations(const ContainerOptions& options) { + try + { + mount::ValidateMountCollection(options.Mounts); + } + catch (const mount::ValidationException& ex) + { + if (ex.Error() == mount::ValidationError::DuplicateDestination) + { + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_DuplicateMountDestinationError(MultiByteToWide(ex.Destination()))); + } + + throw; + } + std::unordered_set destinations; const auto addDestination = [&](const std::string& destination) { const auto normalizedDestination = mount::NormalizeDestination(destination); diff --git a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp index 355ba21ba6..34512149ea 100644 --- a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp @@ -27,6 +27,13 @@ namespace WSLCCLIMountParserUnitTests { namespace { + mount::Spec ParseAndValidate(const std::wstring& value) + { + auto mountSpec = mount::ParseDockerMountString(value); + mount::ValidateMountSpec(mountSpec); + return mountSpec; + } + struct ValidMountCase { const wchar_t* Input; @@ -323,7 +330,7 @@ class WSLCCLIMountParserUnitTests { Log::Comment(String().Format(L"Accepting: %ls", testCase.Input)); - const auto actual = mount::Parse(testCase.Input); + const auto actual = ParseAndValidate(testCase.Input); VERIFY_ARE_EQUAL(static_cast(testCase.Type), static_cast(actual.MountType)); VERIFY_ARE_EQUAL(std::wstring(testCase.Source), actual.Source); VERIFY_ARE_EQUAL(std::string(testCase.Target), actual.Target); @@ -353,10 +360,10 @@ class WSLCCLIMountParserUnitTests try { - (void)mount::Parse(testCase.Input); - VERIFY_FAIL(L"Expected ParseException for invalid mount spec"); + (void)ParseAndValidate(testCase.Input); + VERIFY_FAIL(L"Expected ValidationException for invalid mount spec"); } - catch (const mount::ParseException& ex) + catch (const mount::ValidationException& ex) { VERIFY_IS_TRUE(ex.Reason().find(testCase.ExpectedReason) != std::wstring::npos); } @@ -366,10 +373,72 @@ class WSLCCLIMountParserUnitTests TEST_METHOD(Mount_DotRelativeBindSourceUsesCurrentDirectory) { const auto expected = (std::filesystem::current_path() / L"mount").lexically_normal().wstring(); - const auto actual = mount::Parse(L"type=bind,source=.\\mount,target=/data"); + const auto actual = ParseAndValidate(L"type=bind,source=.\\mount,target=/data"); VERIFY_ARE_EQUAL(expected, actual.Source); } + TEST_METHOD(Mount_TypedSpecsAreValidated) + { + const mount::Spec relativeBind{ + .MountType = mount::Type::Bind, + .Source = L"relative", + .Target = "/data", + }; + VERIFY_THROWS(mount::ValidateMountSpec(relativeBind), mount::ValidationException); + + const mount::Spec relativeTarget{ + .MountType = mount::Type::Volume, + .Source = L"data-volume", + .Target = "data", + }; + VERIFY_THROWS(mount::ValidateMountSpec(relativeTarget), mount::ValidationException); + + const mount::Spec tmpfsWithSource{ + .MountType = mount::Type::Tmpfs, + .Source = L"data-volume", + .Target = "/data", + }; + VERIFY_THROWS(mount::ValidateMountSpec(tmpfsWithSource), mount::ValidationException); + + const mount::Spec bindWithTmpfsOptions{ + .MountType = mount::Type::Bind, + .Source = L"C:\\data", + .Target = "/data", + .TmpfsSizeBytes = 1024, + }; + VERIFY_THROWS(mount::ValidateMountSpec(bindWithTmpfsOptions), mount::ValidationException); + + const mount::Spec negativeTmpfsSize{ + .MountType = mount::Type::Tmpfs, + .Target = "/data", + .TmpfsSizeBytes = -1, + }; + VERIFY_THROWS(mount::ValidateMountSpec(negativeTmpfsSize), mount::ValidationException); + + const mount::Spec tmpfs{ + .MountType = mount::Type::Tmpfs, + .Target = "/data", + .TmpfsSizeBytes = 1024, + .TmpfsMode = 0700, + }; + VERIFY_NO_THROW(mount::ValidateMountSpec(tmpfs)); + + const mount::Spec duplicateMounts[] = { + {.MountType = mount::Type::Tmpfs, .Target = "/data"}, + {.MountType = mount::Type::Volume, .Source = L"data-volume", .Target = "/data/"}, + }; + try + { + mount::ValidateMountCollection(duplicateMounts); + VERIFY_FAIL(L"Expected ValidationException for duplicate destinations"); + } + catch (const mount::ValidationException& ex) + { + VERIFY_ARE_EQUAL(static_cast(mount::ValidationError::DuplicateDestination), static_cast(ex.Error())); + VERIFY_ARE_EQUAL(std::string("/data"), ex.Destination()); + } + } + TEST_METHOD(Mount_DuplicateDestinationsAreRejected) { ContainerOptions options; From 241ff0230d4483f5adde5b56d4c026692269ba2f Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 17 Aug 2026 12:47:44 -0700 Subject: [PATCH 07/12] Update for PR feedback, MountHosts support --- localization/strings/en-US/Resources.resw | 91 ++++- src/windows/common/MountSpecParsing.cpp | 82 +++-- src/windows/common/MountSpecParsing.h | 26 +- src/windows/common/WSLCContainerLauncher.cpp | 43 +++ src/windows/common/WSLCContainerLauncher.h | 5 + src/windows/inc/docker_schema.h | 11 +- src/windows/service/inc/wslc.idl | 31 ++ .../wslc/arguments/ArgumentValidation.cpp | 6 +- src/windows/wslc/services/ContainerModel.cpp | 2 +- .../wslc/services/ContainerService.cpp | 15 +- src/windows/wslcsession/WSLCContainer.cpp | 346 ++++++++++++++---- .../wslcsession/WSLCContainerMetadata.h | 3 +- .../wslc/WSLCCLIMountParserUnitTests.cpp | 245 ++++++++----- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 116 +++++- .../wslc/e2e/WSLCE2EContainerRunTests.cpp | 3 +- 15 files changed, 795 insertions(+), 230 deletions(-) diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index 0d0578e9d4..a5f3626add 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2471,6 +2471,14 @@ For privacy information about this product please visit https://aka.ms/privacy.< Failed to create volume '{}': {} {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + + Bind source path does not exist: '{}' + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + + + Failed to access bind source path '{}': {} + {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + Too many volumes have been mounted (limit: {}). Restart the session to mount more volumes. This will be fixed in a future release. {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated @@ -2976,13 +2984,94 @@ On first run, creates the file with all settings commented out at their defaults Invalid argument "{}" for '-f, --filter' flag: bad format of filter (expected name=value) {FixedPlaceholder="{}"}{Locked="--filter'"}Command line arguments, file names and string inserts should not be translated - Invalid argument "{}" for '--mount' flag: {} + Invalid argument "{}" for '--mount' option: {} + {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}{Locked="--mount'"}Command line arguments, file names and string inserts should not be translated + + + Argument "{}" for '--mount' option uses an unsupported feature: {} {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}{Locked="--mount'"}Command line arguments, file names and string inserts should not be translated Duplicate mount point: {} {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + + The CSV input is malformed. + {Locked="CSV"}Command line arguments should not be translated + + + The field '{}' must be a key=value pair. + {FixedPlaceholder="{}"}{Locked="key=value"}Command line arguments and string inserts should not be translated + + + The key '{}' is unexpected in '{}'. + {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments and string inserts should not be translated + + + The value for '{}' is invalid: '{}'. + {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments and string inserts should not be translated + + + The value for '{}' is invalid: '{}'. Valid values are "enabled", "disabled", "writable", and "readonly". + {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}{Locked="enabled"}{Locked="disabled"}{Locked="writable"}{Locked="readonly"}Command line arguments and string inserts should not be translated + + + The mount type is required. + + + Options matching '{}' cannot be used with mount type '{}'. + {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments and string inserts should not be translated + + + Option '{}' requires the 'readonly' option. + {FixedPlaceholder="{}"}{Locked="readonly"}Command line arguments and string inserts should not be translated + + + Option 'bind-recursive=readonly' requires the 'bind-propagation=rprivate' option. + {Locked="bind-recursive=readonly"}{Locked="bind-propagation=rprivate"}Command line arguments should not be translated + + + Mount type '{}' is not supported. + {FixedPlaceholder="{}"}Command line arguments and string inserts should not be translated + + + Option '{}' is not supported. + {FixedPlaceholder="{}"}Command line arguments and string inserts should not be translated + + + The mount target is required. + + + The mount target path must be absolute. + + + The tmpfs options are supported only for tmpfs mounts. + {Locked="tmpfs"}Command line arguments should not be translated + + + The tmpfs size cannot be negative. + {Locked="tmpfs"}Command line arguments should not be translated + + + The mount source is required. + + + The bind source path must be absolute. + {Locked="bind"}Command line arguments should not be translated + + + Anonymous volume mounts are not supported. + + + The volume source must be a valid named volume. + + + The source option is not supported for tmpfs mounts. + {Locked="tmpfs"}Command line arguments should not be translated + + + The mount type is not supported. + Follow log output diff --git a/src/windows/common/MountSpecParsing.cpp b/src/windows/common/MountSpecParsing.cpp index 097fd053bb..a23ec69cae 100644 --- a/src/windows/common/MountSpecParsing.cpp +++ b/src/windows/common/MountSpecParsing.cpp @@ -123,9 +123,19 @@ namespace { bool HadSeparator; }; - [[noreturn]] void ThrowInvalid(std::wstring reason) + [[noreturn]] void ThrowValidation(std::wstring reason) { - throw ValidationException(std::move(reason)); + throw MountValidationException(std::move(reason)); + } + + [[noreturn]] void ThrowParse(std::wstring reason) + { + throw MountParseException(std::move(reason)); + } + + [[noreturn]] void ThrowUnsupported(std::wstring reason) + { + throw MountUnsupportedException(std::move(reason)); } KeyValue SplitKeyValue(const std::wstring& value) @@ -308,7 +318,7 @@ Spec ParseDockerMountString(const std::wstring& value) const auto fields = SplitCsvFields(value); if (!fields.has_value()) { - ThrowInvalid(L"malformed CSV"); + ThrowParse(Localization::WSLCCLI_MountMalformedCsvError()); } DockerMountSpec mount; @@ -322,15 +332,15 @@ Spec ParseDockerMountString(const std::wstring& value) { if (!keyValue.HadSeparator) { - ThrowInvalid(std::format(L"invalid field '{}' must be a key=value pair", field)); + ThrowParse(Localization::WSLCCLI_MountFieldKeyValueRequiredError(field)); } - ThrowInvalid(std::format(L"unexpected key '{}' in '{}'", key, field)); + ThrowParse(Localization::WSLCCLI_MountUnexpectedKeyError(key, field)); } if (!keyValue.HadSeparator && !definition->AllowsBareForm) { - ThrowInvalid(std::format(L"invalid field '{}' must be a key=value pair", field)); + ThrowParse(Localization::WSLCCLI_MountFieldKeyValueRequiredError(field)); } switch (definition->Id) @@ -369,7 +379,7 @@ Spec ParseDockerMountString(const std::wstring& value) } else { - ThrowInvalid(std::format(L"invalid value for {}: {}", key, keyValue.Value)); + ThrowParse(Localization::WSLCCLI_MountInvalidValueError(key, keyValue.Value)); } break; @@ -384,7 +394,7 @@ Spec ParseDockerMountString(const std::wstring& value) case Field::BindNonRecursive: if (keyValue.HadSeparator && !ParseBool(keyValue.Value.c_str(), true).has_value()) { - ThrowInvalid(std::format(L"invalid value for {}: {}", key, keyValue.Value)); + ThrowParse(Localization::WSLCCLI_MountInvalidValueError(key, keyValue.Value)); } mount.HasBindOptions = true; @@ -413,13 +423,12 @@ Spec ParseDockerMountString(const std::wstring& value) break; } - ThrowInvalid(std::format( - L"invalid value for {}: {} (must be \"enabled\", \"disabled\", \"writable\", or \"readonly\")", key, keyValue.Value)); + ThrowParse(Localization::WSLCCLI_MountInvalidBindRecursiveValueError(key, keyValue.Value)); case Field::VolumeNoCopy: if (keyValue.HadSeparator && !ParseBool(keyValue.Value.c_str(), true).has_value()) { - ThrowInvalid(std::format(L"invalid value for volume-nocopy: {}", keyValue.Value)); + ThrowParse(Localization::WSLCCLI_MountInvalidValueError(L"volume-nocopy", keyValue.Value)); } mount.HasVolumeOptions = true; @@ -435,7 +444,7 @@ Spec ParseDockerMountString(const std::wstring& value) mount.TmpfsSizeBytes = ParseDockerRamInBytes(keyValue.Value); if (!mount.TmpfsSizeBytes.has_value()) { - ThrowInvalid(std::format(L"invalid value for {}: {}", key, keyValue.Value)); + ThrowParse(Localization::WSLCCLI_MountInvalidValueError(key, keyValue.Value)); } mount.HasTmpfsOptions = true; @@ -445,7 +454,7 @@ Spec ParseDockerMountString(const std::wstring& value) mount.TmpfsMode = ParseDockerTmpfsMode(keyValue.Value); if (!mount.TmpfsMode.has_value()) { - ThrowInvalid(std::format(L"invalid value for {}: {}", key, keyValue.Value)); + ThrowParse(Localization::WSLCCLI_MountInvalidValueError(key, keyValue.Value)); } mount.HasTmpfsOptions = true; @@ -460,35 +469,35 @@ Spec ParseDockerMountString(const std::wstring& value) if (mount.Type.empty()) { - ThrowInvalid(L"type is required"); + ThrowParse(Localization::WSLCCLI_MountTypeRequiredError()); } if (mount.HasVolumeOptions && mount.Type != L"volume") { - ThrowInvalid(std::format(L"cannot mix 'volume-*' options with mount type '{}'", mount.Type)); + ThrowParse(Localization::WSLCCLI_MountOptionFamilyMismatchError(L"volume-*", mount.Type)); } if (mount.HasBindOptions && mount.Type != L"bind") { - ThrowInvalid(std::format(L"cannot mix 'bind-*' options with mount type '{}'", mount.Type)); + ThrowParse(Localization::WSLCCLI_MountOptionFamilyMismatchError(L"bind-*", mount.Type)); } if (mount.HasTmpfsOptions && mount.Type != L"tmpfs") { - ThrowInvalid(std::format(L"cannot mix 'tmpfs-*' options with mount type '{}'", mount.Type)); + ThrowParse(Localization::WSLCCLI_MountOptionFamilyMismatchError(L"tmpfs-*", mount.Type)); } if (mount.BindReadOnlyNonRecursive && !mount.ReadOnly) { - ThrowInvalid(L"option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction"); + ThrowParse(Localization::WSLCCLI_MountOptionRequiresReadonlyError(L"bind-recursive=writable")); } if (mount.BindReadOnlyForceRecursive) { if (!mount.ReadOnly) { - ThrowInvalid(L"option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction"); + ThrowParse(Localization::WSLCCLI_MountOptionRequiresReadonlyError(L"bind-recursive=readonly")); } if (mount.BindPropagation != L"rprivate") { - ThrowInvalid(L"option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction"); + ThrowParse(Localization::WSLCCLI_MountBindRecursiveReadonlyRequiresPropagationError()); } } @@ -507,12 +516,12 @@ Spec ParseDockerMountString(const std::wstring& value) } else { - ThrowInvalid(std::format(L"mount type '{}' is not supported.", mount.Type)); + ThrowUnsupported(Localization::WSLCCLI_MountTypeUnsupportedError(mount.Type)); } if (mount.UnsupportedOption.has_value()) { - ThrowInvalid(std::format(L"option '{}' is not supported.", mount.UnsupportedOption.value())); + ThrowUnsupported(Localization::WSLCCLI_MountOptionUnsupportedError(mount.UnsupportedOption.value())); } return { @@ -529,27 +538,22 @@ void ValidateMountSpec(const Spec& mount) { if (mount.Target.empty()) { - ThrowInvalid(L"target is required"); - } - - if (mount.Target.find(':') != std::string::npos) - { - ThrowInvalid(L"target paths containing ':' are not supported."); + ThrowValidation(Localization::WSLCCLI_MountTargetRequiredError()); } if (!mount.Target.starts_with('/')) { - ThrowInvalid(L"target path must be absolute"); + ThrowValidation(Localization::WSLCCLI_MountTargetAbsoluteError()); } if (mount.MountType != Type::Tmpfs && (mount.TmpfsSizeBytes.has_value() || mount.TmpfsMode.has_value())) { - ThrowInvalid(L"tmpfs options are only supported for tmpfs mounts"); + ThrowValidation(Localization::WSLCCLI_MountTmpfsOptionsTypeError()); } if (mount.TmpfsSizeBytes.has_value() && mount.TmpfsSizeBytes.value() < 0) { - ThrowInvalid(L"tmpfs size must not be negative"); + ThrowValidation(Localization::WSLCCLI_MountTmpfsSizeNegativeError()); } switch (mount.MountType) @@ -557,36 +561,36 @@ void ValidateMountSpec(const Spec& mount) case Type::Bind: if (mount.Source.empty()) { - ThrowInvalid(L"source is required"); + ThrowValidation(Localization::WSLCCLI_MountSourceRequiredError()); } if (!std::filesystem::path(mount.Source).is_absolute()) { - ThrowInvalid(L"bind source path must be absolute"); + ThrowValidation(Localization::WSLCCLI_MountBindSourceAbsoluteError()); } break; case Type::Volume: if (mount.Source.empty()) { - ThrowInvalid(L"anonymous volume mounts are not supported."); + ThrowUnsupported(Localization::WSLCCLI_MountAnonymousVolumeUnsupportedError()); } if (!IsValidNamedVolumeName(mount.Source)) { - ThrowInvalid(L"volume source must be a valid named volume"); + ThrowValidation(Localization::WSLCCLI_MountVolumeSourceInvalidError()); } break; case Type::Tmpfs: if (!mount.Source.empty()) { - ThrowInvalid(L"source is not supported for tmpfs mounts"); + ThrowValidation(Localization::WSLCCLI_MountTmpfsSourceUnsupportedError()); } break; default: - ThrowInvalid(L"mount type is not supported"); + ThrowUnsupported(Localization::WSLCCLI_MountTypeUnsupportedGenericError()); } } @@ -600,9 +604,9 @@ void ValidateMountCollection(std::span mounts) auto destination = NormalizeDestination(mount.Target); if (!destinations.emplace(destination).second) { - throw ValidationException( + throw MountValidationException( ValidationError::DuplicateDestination, - std::format(L"duplicate mount point: {}", MultiByteToWide(destination)), + Localization::WSLCCLI_DuplicateMountDestinationError(MultiByteToWide(destination)), std::move(destination)); } } diff --git a/src/windows/common/MountSpecParsing.h b/src/windows/common/MountSpecParsing.h index fb6a5b56a1..6e4020c5f4 100644 --- a/src/windows/common/MountSpecParsing.h +++ b/src/windows/common/MountSpecParsing.h @@ -49,21 +49,21 @@ enum class ValidationError DuplicateDestination, }; -class ValidationException : public std::exception +class MountException : public std::exception { public: - explicit ValidationException(std::wstring reason) : m_reason(std::move(reason)) + explicit MountException(std::wstring reason) : m_reason(std::move(reason)) { } - ValidationException(ValidationError error, std::wstring reason, std::string destination) : + MountException(ValidationError error, std::wstring reason, std::string destination) : m_error(error), m_reason(std::move(reason)), m_destination(std::move(destination)) { } const char* what() const noexcept override { - return "invalid mount"; + return "mount error"; } const std::wstring& Reason() const noexcept @@ -87,6 +87,24 @@ class ValidationException : public std::exception std::string m_destination; }; +class MountParseException : public MountException +{ +public: + using MountException::MountException; +}; + +class MountUnsupportedException : public MountException +{ +public: + using MountException::MountException; +}; + +class MountValidationException : public MountException +{ +public: + using MountException::MountException; +}; + Spec ParseDockerMountString(const std::wstring& value); void ValidateMountSpec(const Spec& mount); void ValidateMountCollection(std::span mounts); diff --git a/src/windows/common/WSLCContainerLauncher.cpp b/src/windows/common/WSLCContainerLauncher.cpp index 77b98cdd46..064c5e4ce9 100644 --- a/src/windows/common/WSLCContainerLauncher.cpp +++ b/src/windows/common/WSLCContainerLauncher.cpp @@ -254,6 +254,46 @@ void wsl::windows::common::WSLCContainerLauncher::AddNamedVolume(const std::stri m_namedVolumes.push_back(volume); } +void wsl::windows::common::WSLCContainerLauncher::AddMount(const mount::Spec& Mount) +{ + WSLCMountSpec mount{}; + switch (Mount.MountType) + { + case mount::Type::Bind: + mount.Type = WSLCMountTypeBind; + break; + + case mount::Type::Volume: + mount.Type = WSLCMountTypeVolume; + break; + + case mount::Type::Tmpfs: + mount.Type = WSLCMountTypeTmpfs; + break; + } + + if (!Mount.Source.empty()) + { + mount.Source = m_mountSources.emplace_back(Mount.Source).c_str(); + } + + mount.Target = m_mountTargets.emplace_back(Mount.Target).c_str(); + mount.ReadOnly = Mount.ReadOnly ? TRUE : FALSE; + if (Mount.TmpfsSizeBytes.has_value()) + { + WI_SetFlag(mount.Flags, WSLCMountSpecFlagsTmpfsSize); + mount.TmpfsSizeBytes = Mount.TmpfsSizeBytes.value(); + } + + if (Mount.TmpfsMode.has_value()) + { + WI_SetFlag(mount.Flags, WSLCMountSpecFlagsTmpfsMode); + mount.TmpfsMode = Mount.TmpfsMode.value(); + } + + m_mounts.push_back(mount); +} + void wsl::windows::common::WSLCContainerLauncher::AddLabel(const std::string& Key, const std::string& Value) { // Store a copy of the key/value strings to the launcher to ensure the pointers in WSLCLabel remain valid. @@ -413,6 +453,9 @@ std::pair> WSLCContainerLauncher::C options.NamedVolumesCount = static_cast(m_namedVolumes.size()); options.NamedVolumes = m_namedVolumes.size() > 0 ? m_namedVolumes.data() : nullptr; + options.MountsCount = static_cast(m_mounts.size()); + options.Mounts = m_mounts.size() > 0 ? m_mounts.data() : nullptr; + options.LabelsCount = static_cast(m_labels.size()); options.Labels = m_labels.size() > 0 ? m_labels.data() : nullptr; diff --git a/src/windows/common/WSLCContainerLauncher.h b/src/windows/common/WSLCContainerLauncher.h index 0e9a63f42b..18f47750ee 100644 --- a/src/windows/common/WSLCContainerLauncher.h +++ b/src/windows/common/WSLCContainerLauncher.h @@ -13,6 +13,7 @@ Module Name: --*/ #pragma once +#include "MountSpecParsing.h" #include "WSLCProcessLauncher.h" #include "docker_schema.h" #include "wslc_schema.h" @@ -59,6 +60,7 @@ class WSLCContainerLauncher : private WSLCProcessLauncher void AddVolume(const std::wstring& HostPath, const std::string& ContainerPath, bool ReadOnly); void AddNamedVolume(const std::string& Name, const std::string& ContainerPath, bool ReadOnly); + void AddMount(const mount::Spec& Mount); void AddPort(uint16_t WindowsPort, uint16_t ContainerPort, int Family, int Protocol = IPPROTO_TCP, const std::optional& BindingAddress = {}); void AddLabel(const std::string& Key, const std::string& Value); void AddTmpfs(const std::string& ContainerPath, const std::string& Options); @@ -104,9 +106,12 @@ class WSLCContainerLauncher : private WSLCProcessLauncher std::vector m_ports; std::vector m_volumes; std::vector m_namedVolumes; + std::vector m_mounts; std::deque m_hostPaths; std::deque m_volumeNames; std::deque m_containerPaths; + std::deque m_mountSources; + std::deque m_mountTargets; std::string m_networkMode; std::vector m_entrypoint; WSLCSignal m_stopSignal = WSLCSignalNone; diff --git a/src/windows/inc/docker_schema.h b/src/windows/inc/docker_schema.h index 5fc2f40c67..0ae901e0f7 100644 --- a/src/windows/inc/docker_schema.h +++ b/src/windows/inc/docker_schema.h @@ -239,6 +239,14 @@ inline void to_json(nlohmann::json& j, const ContainerNetworkRequest& v) } } +struct MountTmpfsOptions +{ + std::int64_t SizeBytes{}; + std::uint32_t Mode{}; + + NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(MountTmpfsOptions, SizeBytes, Mode); +}; + struct Mount { std::string Name; @@ -246,8 +254,9 @@ struct Mount std::string Target; std::string Type; bool ReadOnly{}; + std::optional TmpfsOptions; - NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(Mount, Name, Target, Source, Type, ReadOnly); + NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(Mount, Name, Target, Source, Type, ReadOnly, TmpfsOptions); }; struct DeviceMapping diff --git a/src/windows/service/inc/wslc.idl b/src/windows/service/inc/wslc.idl index 178d6cf0c2..78f18084fe 100644 --- a/src/windows/service/inc/wslc.idl +++ b/src/windows/service/inc/wslc.idl @@ -245,6 +245,34 @@ typedef struct _WSLCTmpfsMount [unique] LPCSTR Options; } WSLCTmpfsMount; +typedef enum _WSLCMountType +{ + WSLCMountTypeBind, + WSLCMountTypeVolume, + WSLCMountTypeTmpfs, +} WSLCMountType; + +typedef enum _WSLCMountSpecFlags +{ + WSLCMountSpecFlagsNone = 0, + WSLCMountSpecFlagsTmpfsSize = 1, + WSLCMountSpecFlagsTmpfsMode = 2, +} WSLCMountSpecFlags; + +cpp_quote("#define WSLCMountSpecFlagsValid (WSLCMountSpecFlagsTmpfsSize | WSLCMountSpecFlagsTmpfsMode)") +cpp_quote("DEFINE_ENUM_FLAG_OPERATORS(WSLCMountSpecFlags);") + +typedef struct _WSLCMountSpec +{ + WSLCMountType Type; + [unique, string] LPCWSTR Source; + [string] LPCSTR Target; + BOOL ReadOnly; + WSLCMountSpecFlags Flags; + LONGLONG TmpfsSizeBytes; + ULONG TmpfsMode; +} WSLCMountSpec; + typedef struct _WSLCUlimit { [string] LPCSTR Name; @@ -325,6 +353,9 @@ typedef struct _WSLCContainerOptions LONGLONG HealthTimeoutNs; LONGLONG HealthStartPeriodNs; LONG HealthRetries; + + [unique, size_is(MountsCount)] const WSLCMountSpec* Mounts; + ULONG MountsCount; } WSLCContainerOptions; typedef char WSLCContainerId[WSLC_CONTAINER_ID_LENGTH + 1] ; diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp index bcd4a40557..f54a065b89 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.cpp +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -228,7 +228,11 @@ void Argument::Validate(ArgMap& execArgs) const mount::ValidateMountSpec(mountSpec); return mountSpec; } - catch (const mount::ValidationException& ex) + catch (const mount::MountUnsupportedException& ex) + { + throw ArgumentException(Localization::WSLCCLI_UnsupportedMountError(value, ex.Reason())); + } + catch (const mount::MountException& ex) { throw ArgumentException(Localization::WSLCCLI_InvalidMountError(value, ex.Reason())); } diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp index fa298f4f76..1009c0d6e6 100644 --- a/src/windows/wslc/services/ContainerModel.cpp +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -339,7 +339,7 @@ void ValidateUniqueMountDestinations(const ContainerOptions& options) { mount::ValidateMountCollection(options.Mounts); } - catch (const mount::ValidationException& ex) + catch (const mount::MountException& ex) { if (ex.Error() == mount::ValidationError::DuplicateDestination) { diff --git a/src/windows/wslc/services/ContainerService.cpp b/src/windows/wslc/services/ContainerService.cpp index 504f6262f4..27ff5a1465 100644 --- a/src/windows/wslc/services/ContainerService.cpp +++ b/src/windows/wslc/services/ContainerService.cpp @@ -145,20 +145,7 @@ static wsl::windows::common::RunningWSLCContainer CreateInternal(Terminal& termi for (const auto& mountSpec : options.Mounts) { - switch (mountSpec.MountType) - { - case mount::Type::Bind: - containerLauncher.AddVolume(mountSpec.Source, mountSpec.Target, mountSpec.ReadOnly); - break; - - case mount::Type::Volume: - containerLauncher.AddNamedVolume(string::WideToMultiByte(mountSpec.Source), mountSpec.Target, mountSpec.ReadOnly); - break; - - case mount::Type::Tmpfs: - containerLauncher.AddTmpfs(mountSpec.Target, mount::FormatTmpfsOptions(mountSpec)); - break; - } + containerLauncher.AddMount(mountSpec); } containerLauncher.SetContainerFlags(containerFlags); diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index d3fc1b21ec..0da7acfcee 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -24,6 +24,8 @@ Module Name: #include "WSLCProcessIO.h" #include "WSLCVolumes.h" #include "APICompat.h" +#include "MountSpecParsing.h" +#include namespace apicompat = wsl::windows::common::apicompat; @@ -414,9 +416,21 @@ auto MountVolumes(std::vector& volumes, WSLCVirtualMachine& par for (auto& volume : volumes) { - // Create a new directory if it doesn't exist. - if (!std::filesystem::exists(volume.HostPath)) + std::error_code error; + const auto sourceExists = std::filesystem::exists(volume.HostPath, error); + if (error) { + throw wsl::windows::common::mount::MountValidationException( + Localization::MessageWslcBindSourcePathError(volume.HostPath, error.message())); + } + + if (!sourceExists) + { + if (!volume.CreateSourceIfMissing) + { + throw wsl::windows::common::mount::MountValidationException(Localization::MessageWslcBindSourcePathNotFound(volume.HostPath)); + } + auto result = wil::CreateDirectoryDeepNoThrow(volume.HostPath.c_str()); if (FAILED(result)) { @@ -433,6 +447,18 @@ auto MountVolumes(std::vector& volumes, WSLCVirtualMachine& par return std::move(errorCleanup); } +auto MountVolumesWithUserError(std::vector& volumes, WSLCVirtualMachine& parentVM) +{ + try + { + return MountVolumes(volumes, parentVM); + } + catch (const wsl::windows::common::mount::MountValidationException& ex) + { + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, ex.Reason()); + } +} + WSLCContainerState DockerStateToWSLCState(ContainerState state) { // TODO: Handle other states like Paused, Restarting, etc. @@ -521,6 +547,183 @@ std::map StripInternalLabels(std::optional{})); } +std::vector ConvertAndValidateMounts(const WSLCContainerOptions& containerOptions) +{ + namespace mount = wsl::windows::common::mount; + + THROW_HR_IF(E_INVALIDARG, containerOptions.MountsCount > 0 && containerOptions.Mounts == nullptr); + + std::vector mounts; + mounts.reserve(containerOptions.MountsCount); + for (ULONG i = 0; i < containerOptions.MountsCount; ++i) + { + const auto& value = containerOptions.Mounts[i]; + THROW_HR_IF_NULL_MSG(E_INVALIDARG, value.Target, "Mount at index %lu has null Target", i); + THROW_HR_IF_MSG( + E_INVALIDARG, + WI_IsAnyFlagSet(value.Flags, ~WSLCMountSpecFlagsValid), + "Mount at index %lu has invalid flags: 0x%x", + i, + value.Flags); + + mount::Type type; + switch (value.Type) + { + case WSLCMountTypeBind: + type = mount::Type::Bind; + break; + + case WSLCMountTypeVolume: + type = mount::Type::Volume; + break; + + case WSLCMountTypeTmpfs: + type = mount::Type::Tmpfs; + break; + + default: + THROW_HR_MSG(E_INVALIDARG, "Mount at index %lu has invalid type: %d", i, value.Type); + } + + mounts.push_back({ + .MountType = type, + .Source = value.Source != nullptr ? value.Source : L"", + .Target = value.Target, + .ReadOnly = static_cast(value.ReadOnly), + .TmpfsSizeBytes = WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsTmpfsSize) ? std::optional{value.TmpfsSizeBytes} : std::nullopt, + .TmpfsMode = WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsTmpfsMode) ? std::optional{value.TmpfsMode} : std::nullopt, + }); + } + + try + { + mount::ValidateMountCollection(mounts); + for (const auto& mount : mounts) + { + if (mount.MountType == mount::Type::Bind) + { + std::error_code error; + const auto sourceExists = std::filesystem::exists(mount.Source, error); + if (error) + { + throw mount::MountValidationException(Localization::MessageWslcBindSourcePathError(mount.Source, error.message())); + } + + if (!sourceExists) + { + throw mount::MountValidationException(Localization::MessageWslcBindSourcePathNotFound(mount.Source)); + } + } + } + } + catch (const mount::MountException& ex) + { + if (ex.Error() == mount::ValidationError::DuplicateDestination) + { + THROW_HR_WITH_USER_ERROR( + E_INVALIDARG, Localization::WSLCCLI_DuplicateMountDestinationError(wsl::shared::string::MultiByteToWide(ex.Destination()))); + } + + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, ex.Reason()); + } + + std::unordered_set destinations; + const auto addDestination = [&](const char* destination) { + THROW_HR_IF_NULL(E_INVALIDARG, destination); + + const auto normalizedDestination = mount::NormalizeDestination(destination); + THROW_HR_WITH_USER_ERROR_IF( + E_INVALIDARG, + Localization::WSLCCLI_DuplicateMountDestinationError(wsl::shared::string::MultiByteToWide(normalizedDestination)), + !destinations.emplace(normalizedDestination).second); + }; + + for (const auto& mount : mounts) + { + addDestination(mount.Target.c_str()); + } + + THROW_HR_IF(E_INVALIDARG, containerOptions.VolumesCount > 0 && containerOptions.Volumes == nullptr); + for (ULONG i = 0; i < containerOptions.VolumesCount; ++i) + { + THROW_HR_IF_NULL_MSG(E_INVALIDARG, containerOptions.Volumes[i].HostPath, "Volumes[%lu].HostPath is null", i); + addDestination(containerOptions.Volumes[i].ContainerPath); + } + + THROW_HR_IF(E_INVALIDARG, containerOptions.NamedVolumesCount > 0 && containerOptions.NamedVolumes == nullptr); + for (ULONG i = 0; i < containerOptions.NamedVolumesCount; ++i) + { + THROW_HR_IF_NULL_MSG(E_INVALIDARG, containerOptions.NamedVolumes[i].Name, "NamedVolume at index %lu has null Name", i); + addDestination(containerOptions.NamedVolumes[i].ContainerPath); + } + + THROW_HR_IF(E_INVALIDARG, containerOptions.TmpfsCount > 0 && containerOptions.Tmpfs == nullptr); + for (ULONG i = 0; i < containerOptions.TmpfsCount; ++i) + { + addDestination(containerOptions.Tmpfs[i].Destination); + } + + return mounts; +} + +struct PreparedBindMount +{ + WSLCVolumeMount Volume; + std::string DockerSource; +}; + +enum class MissingBindSource +{ + Create, + Reject, +}; + +PreparedBindMount PrepareBindMount(const std::wstring& source, const std::string& target, bool readOnly, MissingBindSource missingSource) +{ + GUID volumeId; + THROW_IF_FAILED(CoCreateGuid(&volumeId)); + + auto parentVMPath = std::format("/mnt/{}", wsl::shared::string::GuidToString(volumeId)); + std::filesystem::path hostPath = source; + THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessagePathNotAbsolute(source), !hostPath.is_absolute()); + + std::wstring sourceFilename; + { + std::error_code ec; + hostPath = std::filesystem::canonical(hostPath, ec); + if (!ec) + { + if (std::filesystem::is_regular_file(hostPath)) + { + sourceFilename = hostPath.filename().wstring(); + hostPath = hostPath.parent_path(); + } + } + else if (ec == std::errc::no_such_file_or_directory) + { + hostPath = source; + } + else + { + THROW_HR_WITH_USER_ERROR(E_FAIL, Localization::MessageWslcFailedToMountVolume(source, ec.message())); + } + } + + auto dockerSource = sourceFilename.empty() ? parentVMPath : std::format("{}/{}", parentVMPath, sourceFilename); + return { + .Volume = + { + .HostPath = std::move(hostPath), + .ParentVMPath = std::move(parentVMPath), + .ContainerPath = target, + .ReadOnly = readOnly, + .SourceFilename = std::move(sourceFilename), + .CreateSourceIfMissing = missingSource == MissingBindSource::Create, + }, + .DockerSource = std::move(dockerSource), + }; +} + void ProcessNamedVolumes(const WSLCContainerOptions& containerOptions, wsl::windows::common::docker_schema::CreateContainer& request) { THROW_HR_IF(E_INVALIDARG, containerOptions.NamedVolumesCount > 0 && containerOptions.NamedVolumes == nullptr); @@ -900,7 +1103,7 @@ void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessSt Localization::MessageWslcVolumeNotAvailable(wsl::shared::string::Join(unavailableVolumes, ',')), !unavailableVolumes.empty()); - auto volumeCleanup = MountVolumes(m_mountedVolumes, m_runtime.Vm()); + auto volumeCleanup = MountVolumesWithUserError(m_mountedVolumes, m_runtime.Vm()); auto portCleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [this]() { UnmapPorts(); }); MapPorts(); @@ -1600,7 +1803,7 @@ WslcInspectContainer WSLCContainerImpl::BuildInspectContainer(const DockerInspec } // Map volume mounts using WSLC's host-side data. - wslcInspect.Mounts.reserve(m_mountedVolumes.size() + dockerInspect.HostConfig.Tmpfs.size()); + wslcInspect.Mounts.reserve(m_mountedVolumes.size() + dockerInspect.HostConfig.Tmpfs.size() + dockerInspect.HostConfig.Mounts.size()); for (const auto& volume : m_mountedVolumes) { wslc_schema::InspectMount mountInfo{}; @@ -1636,6 +1839,20 @@ WslcInspectContainer WSLCContainerImpl::BuildInspectContainer(const DockerInspec wslcInspect.Mounts.push_back(std::move(mountInfo)); } + // Bind mounts are populated from m_mountedVolumes so their inspect source is the Windows host path. + for (const auto& mount : dockerInspect.HostConfig.Mounts) + { + if (mount.Type == "volume" || mount.Type == "tmpfs") + { + wslc_schema::InspectMount mountInfo{}; + mountInfo.Type = mount.Type; + mountInfo.Source = mount.Source; + mountInfo.Destination = mount.Target; + mountInfo.ReadWrite = !mount.ReadOnly; + wslcInspect.Mounts.push_back(std::move(mountInfo)); + } + } + // Config.Labels is the Docker-shape location; top-level Labels is a legacy alias. wslcInspect.Config.Labels = m_labels; wslcInspect.Labels = m_labels; @@ -1676,6 +1893,7 @@ std::shared_ptr WSLCContainerImpl::Create( auto& virtualMachine = runtime.Vm(); auto& DockerClient = runtime.Docker(); auto& EventTracker = runtime.Events(); + const auto mounts = ConvertAndValidateMounts(containerOptions); common::docker_schema::CreateContainer request; request.Image = containerOptions.Image; @@ -1837,73 +2055,22 @@ std::shared_ptr WSLCContainerImpl::Create( request.Healthcheck = std::move(health); } - if (containerOptions.VolumesCount > 0) - { - THROW_HR_IF_NULL_MSG(E_INVALIDARG, containerOptions.Volumes, "Volumes is null with VolumesCount=%lu", containerOptions.VolumesCount); - } - // Build bind mount list from container options. std::vector volumes; - volumes.reserve(containerOptions.VolumesCount); + volumes.reserve(containerOptions.VolumesCount + mounts.size()); std::vector binds; binds.reserve(containerOptions.VolumesCount); for (ULONG i = 0; i < containerOptions.VolumesCount; i++) { - GUID volumeId; - THROW_IF_FAILED(CoCreateGuid(&volumeId)); - - auto parentVMPath = std::format("/mnt/{}", wsl::shared::string::GuidToString(volumeId)); auto volume = containerOptions.Volumes[i]; - - THROW_HR_IF_NULL_MSG(E_INVALIDARG, volume.HostPath, "Volumes[%lu].HostPath is null", i); - THROW_HR_IF_NULL_MSG(E_INVALIDARG, volume.ContainerPath, "Volumes[%lu].ContainerPath is null", i); - - std::filesystem::path hostPath = volume.HostPath; - THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessagePathNotAbsolute(volume.HostPath), !hostPath.is_absolute()); - - std::wstring sourceFilename; - - { - // Resolve symlinks. - std::error_code ec; - hostPath = std::filesystem::canonical(hostPath, ec); - if (!ec) - { - // When the host path is a file, mount the parent directory in the VM - // and bind only the specific file into the container via Docker. - if (std::filesystem::is_regular_file(hostPath)) - { - sourceFilename = hostPath.filename().wstring(); - hostPath = hostPath.parent_path(); - } - } - else - { - if (ec == std::errc::no_such_file_or_directory) - { - // Path doesn't exist, assume directory. - hostPath = volume.HostPath; - } - else - { - THROW_HR_WITH_USER_ERROR(E_FAIL, Localization::MessageWslcFailedToMountVolume(volume.HostPath, ec.message())); - } - } - } - - volumes.push_back(WSLCVolumeMount{hostPath, parentVMPath, volume.ContainerPath, static_cast(volume.ReadOnly), sourceFilename}); - - auto options = volume.ReadOnly ? "ro" : "rw"; - auto bindSource = sourceFilename.empty() ? parentVMPath : std::format("{}/{}", parentVMPath, sourceFilename); - auto bind = std::format("{}:{}:{}", bindSource, volume.ContainerPath, options); - - binds.push_back(std::move(bind)); + auto prepared = + PrepareBindMount(volume.HostPath, volume.ContainerPath, static_cast(volume.ReadOnly), MissingBindSource::Create); + binds.push_back(std::format("{}:{}:{}", prepared.DockerSource, volume.ContainerPath, volume.ReadOnly ? "ro" : "rw")); + volumes.push_back(std::move(prepared.Volume)); } - request.HostConfig.Binds = std::move(binds); - // Process tmpfs mounts from container options. if (containerOptions.TmpfsCount > 0) { @@ -1921,6 +2088,47 @@ std::shared_ptr WSLCContainerImpl::Create( ProcessNamedVolumes(containerOptions, request); + for (const auto& mount : mounts) + { + common::docker_schema::Mount dockerMount{ + .Target = mount.Target, + .ReadOnly = mount.ReadOnly, + }; + + switch (mount.MountType) + { + case wsl::windows::common::mount::Type::Bind: + { + // Docker's colon-delimited bind format cannot represent ':' in the target. + auto prepared = PrepareBindMount(mount.Source, mount.Target, mount.ReadOnly, MissingBindSource::Reject); + dockerMount.Source = std::move(prepared.DockerSource); + dockerMount.Type = "bind"; + volumes.push_back(std::move(prepared.Volume)); + break; + } + + case wsl::windows::common::mount::Type::Volume: + dockerMount.Source = wsl::shared::string::WideToMultiByte(mount.Source); + dockerMount.Type = "volume"; + break; + + case wsl::windows::common::mount::Type::Tmpfs: + dockerMount.Type = "tmpfs"; + if (mount.TmpfsSizeBytes.has_value() || mount.TmpfsMode.has_value()) + { + dockerMount.TmpfsOptions = common::docker_schema::MountTmpfsOptions{ + .SizeBytes = mount.TmpfsSizeBytes.value_or(0), + .Mode = mount.TmpfsMode.value_or(0), + }; + } + break; + } + + request.HostConfig.Mounts.push_back(std::move(dockerMount)); + } + + request.HostConfig.Binds = std::move(binds); + // Configure GPU support if requested. if (WI_IsFlagSet(containerOptions.Flags, WSLCContainerFlagsGpu)) { @@ -2058,8 +2266,12 @@ std::shared_ptr WSLCContainerImpl::Create( request.Labels[WSLCContainerMetadataLabel] = SerializeContainerMetadata(metadata); request.Labels.insert(requestedLabels.begin(), requestedLabels.end()); - // Send the request to docker. - auto result = DockerClient.CreateContainer(request, containerName); + // Docker validates structured bind sources during container creation, so their VM paths must exist here. + // Release the temporary shares before returning; Start remounts them for the container lifetime. + auto result = [&]() { + auto volumeCleanup = MountVolumesWithUserError(volumes, virtualMachine); + return DockerClient.CreateContainer(request, containerName); + }(); // Surface any warnings returned by Docker (e.g., deprecated features, configuration issues). for (const auto& warning : result.Warnings) @@ -2113,12 +2325,20 @@ std::shared_ptr WSLCContainerImpl::Create( // Collect the names of referenced docker named volumes so Start() can verify // they are available before running the container. std::vector namedVolumes; - namedVolumes.reserve(containerOptions.NamedVolumesCount); + namedVolumes.reserve(containerOptions.NamedVolumesCount + mounts.size()); for (ULONG i = 0; i < containerOptions.NamedVolumesCount; i++) { namedVolumes.emplace_back(containerOptions.NamedVolumes[i].Name); } + for (const auto& mount : mounts) + { + if (mount.MountType == wsl::windows::common::mount::Type::Volume) + { + namedVolumes.emplace_back(wsl::shared::string::WideToMultiByte(mount.Source)); + } + } + auto mergedLabels = StripInternalLabels(std::move(inspectData.Config.Labels)); auto container = std::make_shared( diff --git a/src/windows/wslcsession/WSLCContainerMetadata.h b/src/windows/wslcsession/WSLCContainerMetadata.h index 0d19c88ad0..a81a46526c 100644 --- a/src/windows/wslcsession/WSLCContainerMetadata.h +++ b/src/windows/wslcsession/WSLCContainerMetadata.h @@ -44,11 +44,12 @@ struct WSLCVolumeMount // Non-empty when the mount target is a single file rather than a directory. std::wstring SourceFilename; + bool CreateSourceIfMissing{true}; // Runtime-only field. Not serialized to JSON. bool Mounted{}; - NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(WSLCVolumeMount, HostPath, ParentVMPath, ContainerPath, ReadOnly, SourceFilename); + NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(WSLCVolumeMount, HostPath, ParentVMPath, ContainerPath, ReadOnly, SourceFilename, CreateSourceIfMissing); }; struct WSLCContainerMetadataV1 diff --git a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp index 34512149ea..a01420aea5 100644 --- a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp @@ -20,6 +20,7 @@ Module Name: using namespace wsl::windows::common; using namespace wsl::windows::wslc::models; +using namespace wsl::shared; using namespace WEX::Logging; using namespace WEX::Common; @@ -46,14 +47,30 @@ namespace { const char* TmpfsOptions; }; + enum class ExpectedException + { + Parse, + Unsupported, + Validation, + }; + struct InvalidMountCase { const wchar_t* Input; - const wchar_t* ExpectedReason; + std::wstring ExpectedReason; + ExpectedException Exception = ExpectedException::Parse; }; constexpr ValidMountCase c_validMountCases[] = { {L"source=data-volume,target=/data", mount::Type::Volume, L"data-volume", "/data", false, {}, {}, ""}, + {L"type=volume,source=data-volume,target=/path:voldir", + mount::Type::Volume, + L"data-volume", + "/path:voldir", + false, + {}, + {}, + ""}, {L"TYPE=VOLUME,SOURCE=data-volume,TARGET=/data", mount::Type::Volume, L"data-volume", "/data", false, {}, {}, ""}, {L"type=VoLuMe,source=data-volume,target=/data", mount::Type::Volume, L"data-volume", "/data", false, {}, {}, ""}, {L"type=volume,src=data-volume,dst=/data", mount::Type::Volume, L"data-volume", "/data", false, {}, {}, ""}, @@ -183,6 +200,7 @@ namespace { {}, {}, ""}, + {L"type=bind,source=C:\\mount,target=/path:mntdir", mount::Type::Bind, L"C:\\mount", "/path:mntdir", false, {}, {}, ""}, {L"type=bind,source=C:\\,target=/data", mount::Type::Bind, L"C:\\", "/data", false, {}, {}, ""}, {L"type=bind,source=\\\\server\\share,target=/data", mount::Type::Bind, L"\\\\server\\share", "/data", false, {}, {}, ""}, {L"type=bind,source=C:\\mount,target=/data,bind-recursive=enabled", @@ -204,6 +222,7 @@ namespace { {L"type=volume,source=A_,target=/data", mount::Type::Volume, L"A_", "/data", false, {}, {}, ""}, {L"type=volume,source=data.volume-1,target=/data", mount::Type::Volume, L"data.volume-1", "/data", false, {}, {}, ""}, {L"type=tmpfs,target=/tmp", mount::Type::Tmpfs, L"", "/tmp", false, {}, {}, ""}, + {L"type=tmpfs,target=/path:tmpfs", mount::Type::Tmpfs, L"", "/path:tmpfs", false, {}, {}, ""}, {L"type=tmpfs,target=/tmp,readonly", mount::Type::Tmpfs, L"", "/tmp", true, {}, {}, "ro"}, {L"type=tmpfs,target=/tmp,tmpfs-size=0", mount::Type::Tmpfs, L"", "/tmp", false, 0, {}, ""}, {L"type=tmpfs,target=/tmp,tmpfs-size=1", mount::Type::Tmpfs, L"", "/tmp", false, 1, {}, "size=1"}, @@ -232,90 +251,124 @@ namespace { {L"type=tmpfs,target=/tmp,tmpfs-size=0,tmpfs-mode=0000,readonly=false", mount::Type::Tmpfs, L"", "/tmp", false, 0, 0, ""}, }; - constexpr InvalidMountCase c_invalidMountCases[] = { - {L"", L"invalid field '' must be a key=value pair"}, - {L",", L"invalid field '' must be a key=value pair"}, - {L"type=volume,source=data-volume,target=/data,", L"invalid field '' must be a key=value pair"}, - {L",type=volume,source=data-volume,target=/data", L"invalid field '' must be a key=value pair"}, - {L"type=bind,\"source=C:\\mount,target=/data", L"malformed CSV"}, - {L"type=volume,bogus", L"invalid field 'bogus' must be a key=value pair"}, - {L"type=volume,bogus=value", L"unexpected key 'bogus'"}, - {L"type", L"invalid field 'type' must be a key=value pair"}, - {L"source", L"invalid field 'source' must be a key=value pair"}, - {L"target", L"invalid field 'target' must be a key=value pair"}, - {L"type=,source=data-volume,target=/data", L"type is required"}, - {L"type=volume,source=data-volume", L"target is required"}, - {L"type=volume,source=data-volume,target=", L"target is required"}, - {L"type=volume,source=data-volume,dst=", L"target is required"}, - {L"type=cluster,source=data-volume,target=/data", L"mount type 'cluster' is not supported."}, - {L"type=npipe,source=data-volume,target=/data", L"mount type 'npipe' is not supported."}, - {L"type=bogus,source=data-volume,target=/data", L"mount type 'bogus' is not supported."}, - {L"type=CLUSTER,source=data-volume,target=/data", L"mount type 'cluster' is not supported."}, - {L"type=volume,target=/data", L"anonymous volume mounts are not supported."}, - {L"type=bind,target=/data", L"source is required"}, - {L"type=bind,source=relative,target=/data", L"bind source path must be absolute"}, - {L"type=volume,source=a,target=/data", L"volume source must be a valid named volume"}, - {L"type=volume,source=data/volume,target=/data", L"volume source must be a valid named volume"}, - {L"type=volume,source=C:\\mount,target=/data", L"volume source must be a valid named volume"}, - {L"type=tmpfs,source=data-volume,target=/data", L"source is not supported for tmpfs mounts"}, - {L"type=volume,source=data-volume,target=/data:part", L"target paths containing ':' are not supported."}, - {L"type=volume,source=data-volume,target=data", L"target path must be absolute"}, - {L"type=volume,source=data-volume,dst=.", L"target path must be absolute"}, - {L"type=volume,source=data-volume,destination=\\data", L"target path must be absolute"}, - {L"type=bind,source=C:\\mount,target=data", L"target path must be absolute"}, - {L"type=tmpfs,target=data", L"target path must be absolute"}, - {L"type=volume,source=data-volume,target=/data,readonly=no", L"invalid value for readonly: no"}, - {L"type=volume,source=data-volume,target=/data,readonly=yes", L"invalid value for readonly: yes"}, - {L"type=volume,source=data-volume,target=/data,readonly=", L"invalid value for readonly: "}, - {L"type=volume,source=data-volume,target=/data,readonly=2", L"invalid value for readonly: 2"}, - {L"type=volume,source=data-volume,target=/data,volume-nocopy=no", L"invalid value for volume-nocopy: no"}, - {L"type=volume,source=data-volume,target=/data,volume-nocopy=", L"invalid value for volume-nocopy: "}, - {L"type=bind,source=C:\\mount,target=/data,bind-nonrecursive=no", L"invalid value for bind-nonrecursive: no"}, - {L"type=bind,source=C:\\mount,target=/data,bind-recursive=", L"invalid value for bind-recursive: "}, - {L"type=bind,source=C:\\mount,target=/data,bind-recursive=Enabled", L"invalid value for bind-recursive: Enabled"}, - {L"type=bind,source=C:\\mount,target=/data,bind-recursive=bogus", L"invalid value for bind-recursive: bogus"}, + const InvalidMountCase c_invalidMountCases[] = { + {L"", Localization::WSLCCLI_MountFieldKeyValueRequiredError(L"")}, + {L",", Localization::WSLCCLI_MountFieldKeyValueRequiredError(L"")}, + {L"type=volume,source=data-volume,target=/data,", Localization::WSLCCLI_MountFieldKeyValueRequiredError(L"")}, + {L",type=volume,source=data-volume,target=/data", Localization::WSLCCLI_MountFieldKeyValueRequiredError(L"")}, + {L"type=bind,\"source=C:\\mount,target=/data", Localization::WSLCCLI_MountMalformedCsvError()}, + {L"type=volume,bogus", Localization::WSLCCLI_MountFieldKeyValueRequiredError(L"bogus")}, + {L"type=volume,bogus=value", Localization::WSLCCLI_MountUnexpectedKeyError(L"bogus", L"bogus=value")}, + {L"type", Localization::WSLCCLI_MountFieldKeyValueRequiredError(L"type")}, + {L"source", Localization::WSLCCLI_MountFieldKeyValueRequiredError(L"source")}, + {L"target", Localization::WSLCCLI_MountFieldKeyValueRequiredError(L"target")}, + {L"type=,source=data-volume,target=/data", Localization::WSLCCLI_MountTypeRequiredError()}, + {L"type=volume,source=data-volume", Localization::WSLCCLI_MountTargetRequiredError(), ExpectedException::Validation}, + {L"type=volume,source=data-volume,target=", Localization::WSLCCLI_MountTargetRequiredError(), ExpectedException::Validation}, + {L"type=volume,source=data-volume,dst=", Localization::WSLCCLI_MountTargetRequiredError(), ExpectedException::Validation}, + {L"type=cluster,source=data-volume,target=/data", Localization::WSLCCLI_MountTypeUnsupportedError(L"cluster"), ExpectedException::Unsupported}, + {L"type=npipe,source=data-volume,target=/data", Localization::WSLCCLI_MountTypeUnsupportedError(L"npipe"), ExpectedException::Unsupported}, + {L"type=bogus,source=data-volume,target=/data", Localization::WSLCCLI_MountTypeUnsupportedError(L"bogus"), ExpectedException::Unsupported}, + {L"type=CLUSTER,source=data-volume,target=/data", Localization::WSLCCLI_MountTypeUnsupportedError(L"cluster"), ExpectedException::Unsupported}, + {L"type=volume,target=/data", Localization::WSLCCLI_MountAnonymousVolumeUnsupportedError(), ExpectedException::Unsupported}, + {L"type=bind,target=/data", Localization::WSLCCLI_MountSourceRequiredError(), ExpectedException::Validation}, + {L"type=bind,source=relative,target=/data", Localization::WSLCCLI_MountBindSourceAbsoluteError(), ExpectedException::Validation}, + {L"type=volume,source=a,target=/data", Localization::WSLCCLI_MountVolumeSourceInvalidError(), ExpectedException::Validation}, + {L"type=volume,source=data/volume,target=/data", Localization::WSLCCLI_MountVolumeSourceInvalidError(), ExpectedException::Validation}, + {L"type=volume,source=C:\\mount,target=/data", Localization::WSLCCLI_MountVolumeSourceInvalidError(), ExpectedException::Validation}, + {L"type=tmpfs,source=data-volume,target=/data", Localization::WSLCCLI_MountTmpfsSourceUnsupportedError(), ExpectedException::Validation}, + {L"type=volume,source=data-volume,target=data", Localization::WSLCCLI_MountTargetAbsoluteError(), ExpectedException::Validation}, + {L"type=volume,source=data-volume,dst=.", Localization::WSLCCLI_MountTargetAbsoluteError(), ExpectedException::Validation}, + {L"type=volume,source=data-volume,destination=\\data", Localization::WSLCCLI_MountTargetAbsoluteError(), ExpectedException::Validation}, + {L"type=bind,source=C:\\mount,target=data", Localization::WSLCCLI_MountTargetAbsoluteError(), ExpectedException::Validation}, + {L"type=tmpfs,target=data", Localization::WSLCCLI_MountTargetAbsoluteError(), ExpectedException::Validation}, + {L"type=volume,source=data-volume,target=/data,readonly=no", + Localization::WSLCCLI_MountInvalidValueError(L"readonly", L"no")}, + {L"type=volume,source=data-volume,target=/data,readonly=yes", + Localization::WSLCCLI_MountInvalidValueError(L"readonly", L"yes")}, + {L"type=volume,source=data-volume,target=/data,readonly=", Localization::WSLCCLI_MountInvalidValueError(L"readonly", L"")}, + {L"type=volume,source=data-volume,target=/data,readonly=2", + Localization::WSLCCLI_MountInvalidValueError(L"readonly", L"2")}, + {L"type=volume,source=data-volume,target=/data,volume-nocopy=no", + Localization::WSLCCLI_MountInvalidValueError(L"volume-nocopy", L"no")}, + {L"type=volume,source=data-volume,target=/data,volume-nocopy=", + Localization::WSLCCLI_MountInvalidValueError(L"volume-nocopy", L"")}, + {L"type=bind,source=C:\\mount,target=/data,bind-nonrecursive=no", + Localization::WSLCCLI_MountInvalidValueError(L"bind-nonrecursive", L"no")}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=", + Localization::WSLCCLI_MountInvalidBindRecursiveValueError(L"bind-recursive", L"")}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=Enabled", + Localization::WSLCCLI_MountInvalidBindRecursiveValueError(L"bind-recursive", L"Enabled")}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=bogus", + Localization::WSLCCLI_MountInvalidBindRecursiveValueError(L"bind-recursive", L"bogus")}, {L"type=bind,source=C:\\mount,target=/data,bind-recursive=writable", - L"option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction"}, + Localization::WSLCCLI_MountOptionRequiresReadonlyError(L"bind-recursive=writable")}, {L"type=bind,source=C:\\mount,target=/data,bind-recursive=readonly", - L"option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction"}, + Localization::WSLCCLI_MountOptionRequiresReadonlyError(L"bind-recursive=readonly")}, {L"type=bind,source=C:\\mount,target=/data,bind-recursive=readonly,readonly", - L"option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction"}, - {L"type=bind,source=C:\\mount,target=/data,consistency=cached", L"option 'consistency' is not supported."}, - {L"type=bind,source=C:\\mount,target=/data,bind-propagation=rprivate", L"option 'bind-propagation' is not supported."}, - {L"type=bind,source=C:\\mount,target=/data,bind-nonrecursive", L"option 'bind-nonrecursive' is not supported."}, - {L"type=bind,source=C:\\mount,target=/data,bind-nonrecursive=true", L"option 'bind-nonrecursive' is not supported."}, - {L"type=bind,source=C:\\mount,target=/data,bind-recursive=disabled", L"option 'bind-recursive' is not supported."}, + Localization::WSLCCLI_MountBindRecursiveReadonlyRequiresPropagationError()}, + {L"type=bind,source=C:\\mount,target=/data,consistency=cached", + Localization::WSLCCLI_MountOptionUnsupportedError(L"consistency"), + ExpectedException::Unsupported}, + {L"type=bind,source=C:\\mount,target=/data,bind-propagation=rprivate", + Localization::WSLCCLI_MountOptionUnsupportedError(L"bind-propagation"), + ExpectedException::Unsupported}, + {L"type=bind,source=C:\\mount,target=/data,bind-nonrecursive", + Localization::WSLCCLI_MountOptionUnsupportedError(L"bind-nonrecursive"), + ExpectedException::Unsupported}, + {L"type=bind,source=C:\\mount,target=/data,bind-nonrecursive=true", + Localization::WSLCCLI_MountOptionUnsupportedError(L"bind-nonrecursive"), + ExpectedException::Unsupported}, + {L"type=bind,source=C:\\mount,target=/data,bind-recursive=disabled", + Localization::WSLCCLI_MountOptionUnsupportedError(L"bind-recursive"), + ExpectedException::Unsupported}, {L"type=bind,source=C:\\mount,target=/data,bind-recursive=writable,readonly", - L"option 'bind-recursive' is not supported."}, + Localization::WSLCCLI_MountOptionUnsupportedError(L"bind-recursive"), + ExpectedException::Unsupported}, {L"type=bind,source=C:\\mount,target=/data,bind-recursive=readonly,readonly,bind-propagation=rprivate", - L"option 'bind-recursive' is not supported."}, - {L"type=volume,source=data-volume,target=/data,volume-nocopy", L"option 'volume-nocopy' is not supported."}, - {L"type=volume,source=data-volume,target=/data,volume-nocopy=true", L"option 'volume-nocopy' is not supported."}, - {L"type=volume,source=data-volume,target=/data,volume-label=a=b", L"option 'volume-label' is not supported."}, - {L"type=volume,source=data-volume,target=/data,volume-driver=local", L"option 'volume-driver' is not supported."}, - {L"type=volume,source=data-volume,target=/data,volume-opt=a=b", L"option 'volume-opt' is not supported."}, - {L"type=bind,source=C:\\mount,target=/data,volume-nocopy=true", L"cannot mix 'volume-*' options with mount type 'bind'"}, + Localization::WSLCCLI_MountOptionUnsupportedError(L"bind-recursive"), + ExpectedException::Unsupported}, + {L"type=volume,source=data-volume,target=/data,volume-nocopy", + Localization::WSLCCLI_MountOptionUnsupportedError(L"volume-nocopy"), + ExpectedException::Unsupported}, + {L"type=volume,source=data-volume,target=/data,volume-nocopy=true", + Localization::WSLCCLI_MountOptionUnsupportedError(L"volume-nocopy"), + ExpectedException::Unsupported}, + {L"type=volume,source=data-volume,target=/data,volume-label=a=b", + Localization::WSLCCLI_MountOptionUnsupportedError(L"volume-label"), + ExpectedException::Unsupported}, + {L"type=volume,source=data-volume,target=/data,volume-driver=local", + Localization::WSLCCLI_MountOptionUnsupportedError(L"volume-driver"), + ExpectedException::Unsupported}, + {L"type=volume,source=data-volume,target=/data,volume-opt=a=b", + Localization::WSLCCLI_MountOptionUnsupportedError(L"volume-opt"), + ExpectedException::Unsupported}, + {L"type=bind,source=C:\\mount,target=/data,volume-nocopy=true", + Localization::WSLCCLI_MountOptionFamilyMismatchError(L"volume-*", L"bind")}, {L"type=volume,source=data-volume,target=/data,bind-propagation=rprivate", - L"cannot mix 'bind-*' options with mount type 'volume'"}, - {L"type=volume,source=data-volume,target=/data,tmpfs-size=1m", L"cannot mix 'tmpfs-*' options with mount type 'volume'"}, - {L"type=tmpfs,target=/tmp,volume-label=a=b", L"cannot mix 'volume-*' options with mount type 'tmpfs'"}, - {L"type=tmpfs,target=/tmp,bind-nonrecursive", L"cannot mix 'bind-*' options with mount type 'tmpfs'"}, - {L"type=tmpfs,target=/tmp,tmpfs-size=", L"invalid value for tmpfs-size: "}, - {L"type=tmpfs,target=/tmp,tmpfs-size=bad", L"invalid value for tmpfs-size: bad"}, - {L"type=tmpfs,target=/tmp,tmpfs-size=-1", L"invalid value for tmpfs-size: -1"}, - {L"type=tmpfs,target=/tmp,\"tmpfs-size=1,5MB\"", L"invalid value for tmpfs-size: 1,5MB"}, - {L"type=tmpfs,target=/tmp,tmpfs-size=1XB", L"invalid value for tmpfs-size: 1XB"}, - {L"type=tmpfs,target=/tmp,tmpfs-size=1Ki", L"invalid value for tmpfs-size: 1Ki"}, - {L"type=tmpfs,target=/tmp,tmpfs-size=1BB", L"invalid value for tmpfs-size: 1BB"}, - {L"type=tmpfs,target=/tmp,tmpfs-size=nan", L"invalid value for tmpfs-size: nan"}, - {L"type=tmpfs,target=/tmp,tmpfs-size=inf", L"invalid value for tmpfs-size: inf"}, - {L"type=tmpfs,target=/tmp,tmpfs-size=9223372036854775808", L"invalid value for tmpfs-size: 9223372036854775808"}, - {L"type=tmpfs,target=/tmp,tmpfs-mode=", L"invalid value for tmpfs-mode: "}, - {L"type=tmpfs,target=/tmp,tmpfs-mode=-1", L"invalid value for tmpfs-mode: -1"}, - {L"type=tmpfs,target=/tmp,tmpfs-mode=8", L"invalid value for tmpfs-mode: 8"}, - {L"type=tmpfs,target=/tmp,tmpfs-mode=0899", L"invalid value for tmpfs-mode: 0899"}, - {L"type=tmpfs,target=/tmp,tmpfs-mode=0x700", L"invalid value for tmpfs-mode: 0x700"}, - {L"type=tmpfs,target=/tmp,tmpfs-mode=40000000000", L"invalid value for tmpfs-mode: 40000000000"}, + Localization::WSLCCLI_MountOptionFamilyMismatchError(L"bind-*", L"volume")}, + {L"type=volume,source=data-volume,target=/data,tmpfs-size=1m", + Localization::WSLCCLI_MountOptionFamilyMismatchError(L"tmpfs-*", L"volume")}, + {L"type=tmpfs,target=/tmp,volume-label=a=b", Localization::WSLCCLI_MountOptionFamilyMismatchError(L"volume-*", L"tmpfs")}, + {L"type=tmpfs,target=/tmp,bind-nonrecursive", Localization::WSLCCLI_MountOptionFamilyMismatchError(L"bind-*", L"tmpfs")}, + {L"type=tmpfs,target=/tmp,tmpfs-size=", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-size", L"")}, + {L"type=tmpfs,target=/tmp,tmpfs-size=bad", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-size", L"bad")}, + {L"type=tmpfs,target=/tmp,tmpfs-size=-1", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-size", L"-1")}, + {L"type=tmpfs,target=/tmp,\"tmpfs-size=1,5MB\"", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-size", L"1,5MB")}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1XB", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-size", L"1XB")}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1Ki", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-size", L"1Ki")}, + {L"type=tmpfs,target=/tmp,tmpfs-size=1BB", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-size", L"1BB")}, + {L"type=tmpfs,target=/tmp,tmpfs-size=nan", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-size", L"nan")}, + {L"type=tmpfs,target=/tmp,tmpfs-size=inf", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-size", L"inf")}, + {L"type=tmpfs,target=/tmp,tmpfs-size=9223372036854775808", + Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-size", L"9223372036854775808")}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-mode", L"")}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=-1", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-mode", L"-1")}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=8", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-mode", L"8")}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=0899", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-mode", L"0899")}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=0x700", Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-mode", L"0x700")}, + {L"type=tmpfs,target=/tmp,tmpfs-mode=40000000000", + Localization::WSLCCLI_MountInvalidValueError(L"tmpfs-mode", L"40000000000")}, }; } // namespace @@ -361,11 +414,25 @@ class WSLCCLIMountParserUnitTests try { (void)ParseAndValidate(testCase.Input); - VERIFY_FAIL(L"Expected ValidationException for invalid mount spec"); + VERIFY_FAIL(L"Expected MountException for invalid mount spec"); } - catch (const mount::ValidationException& ex) + catch (const mount::MountException& ex) { - VERIFY_IS_TRUE(ex.Reason().find(testCase.ExpectedReason) != std::wstring::npos); + VERIFY_ARE_EQUAL(testCase.ExpectedReason, ex.Reason()); + switch (testCase.Exception) + { + case ExpectedException::Parse: + VERIFY_IS_TRUE(dynamic_cast(&ex) != nullptr); + break; + + case ExpectedException::Unsupported: + VERIFY_IS_TRUE(dynamic_cast(&ex) != nullptr); + break; + + case ExpectedException::Validation: + VERIFY_IS_TRUE(dynamic_cast(&ex) != nullptr); + break; + } } } } @@ -384,21 +451,21 @@ class WSLCCLIMountParserUnitTests .Source = L"relative", .Target = "/data", }; - VERIFY_THROWS(mount::ValidateMountSpec(relativeBind), mount::ValidationException); + VERIFY_THROWS(mount::ValidateMountSpec(relativeBind), mount::MountValidationException); const mount::Spec relativeTarget{ .MountType = mount::Type::Volume, .Source = L"data-volume", .Target = "data", }; - VERIFY_THROWS(mount::ValidateMountSpec(relativeTarget), mount::ValidationException); + VERIFY_THROWS(mount::ValidateMountSpec(relativeTarget), mount::MountValidationException); const mount::Spec tmpfsWithSource{ .MountType = mount::Type::Tmpfs, .Source = L"data-volume", .Target = "/data", }; - VERIFY_THROWS(mount::ValidateMountSpec(tmpfsWithSource), mount::ValidationException); + VERIFY_THROWS(mount::ValidateMountSpec(tmpfsWithSource), mount::MountValidationException); const mount::Spec bindWithTmpfsOptions{ .MountType = mount::Type::Bind, @@ -406,14 +473,14 @@ class WSLCCLIMountParserUnitTests .Target = "/data", .TmpfsSizeBytes = 1024, }; - VERIFY_THROWS(mount::ValidateMountSpec(bindWithTmpfsOptions), mount::ValidationException); + VERIFY_THROWS(mount::ValidateMountSpec(bindWithTmpfsOptions), mount::MountValidationException); const mount::Spec negativeTmpfsSize{ .MountType = mount::Type::Tmpfs, .Target = "/data", .TmpfsSizeBytes = -1, }; - VERIFY_THROWS(mount::ValidateMountSpec(negativeTmpfsSize), mount::ValidationException); + VERIFY_THROWS(mount::ValidateMountSpec(negativeTmpfsSize), mount::MountValidationException); const mount::Spec tmpfs{ .MountType = mount::Type::Tmpfs, @@ -430,9 +497,9 @@ class WSLCCLIMountParserUnitTests try { mount::ValidateMountCollection(duplicateMounts); - VERIFY_FAIL(L"Expected ValidationException for duplicate destinations"); + VERIFY_FAIL(L"Expected MountValidationException for duplicate destinations"); } - catch (const mount::ValidationException& ex) + catch (const mount::MountValidationException& ex) { VERIFY_ARE_EQUAL(static_cast(mount::ValidationError::DuplicateDestination), static_cast(ex.Error())); VERIFY_ARE_EQUAL(std::string("/data"), ex.Destination()); diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index 1e7a27968f..30c4b508a9 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -716,16 +716,49 @@ class WSLCE2EContainerCreateTests WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_Tmpfs_Success) { auto result = RunWslc(std::format( - L"container create --name {} --mount type=tmpfs,target=/wslc-tmpfs {} sh -c \"echo -n 'tmpfs_test' > " - L"/wslc-tmpfs/data && cat /wslc-tmpfs/data\"", + L"container create --name {} --mount type=tmpfs,target=/path:tmpfs {} sh -c \"echo -n 'tmpfs_test' > " + L"/path:tmpfs/data && cat /path:tmpfs/data\"", WslcContainerName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = 0}); + const auto inspect = InspectContainer(WslcContainerName); + VERIFY_ARE_EQUAL(1u, inspect.Mounts.size()); + VERIFY_ARE_EQUAL("tmpfs", inspect.Mounts[0].Type); + VERIFY_ARE_EQUAL("", inspect.Mounts[0].Source); + VERIFY_ARE_EQUAL("/path:tmpfs", inspect.Mounts[0].Destination); + VERIFY_IS_TRUE(inspect.Mounts[0].ReadWrite); + result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); result.Verify({.Stdout = L"tmpfs_test", .Stderr = L"", .ExitCode = 0}); } + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_Tmpfs_PreservesMountForm) + { + auto result = RunWslc(std::format( + L"container create --name {} --tmpfs /legacy-tmpfs --mount type=tmpfs,target=/modern-tmpfs,readonly {} true", + WslcContainerName, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = 0}); + + const auto inspect = InspectContainer(WslcContainerName); + VERIFY_ARE_EQUAL(2u, inspect.Mounts.size()); + + const auto legacyMount = + std::ranges::find_if(inspect.Mounts, [](const auto& mount) { return mount.Destination == "/legacy-tmpfs"; }); + VERIFY_IS_TRUE(legacyMount != inspect.Mounts.end()); + VERIFY_ARE_EQUAL("tmpfs", legacyMount->Type); + VERIFY_ARE_EQUAL("", legacyMount->Source); + VERIFY_IS_TRUE(legacyMount->ReadWrite); + + const auto modernMount = + std::ranges::find_if(inspect.Mounts, [](const auto& mount) { return mount.Destination == "/modern-tmpfs"; }); + VERIFY_IS_TRUE(modernMount != inspect.Mounts.end()); + VERIFY_ARE_EQUAL("tmpfs", modernMount->Type); + VERIFY_ARE_EQUAL("", modernMount->Source); + VERIFY_IS_FALSE(modernMount->ReadWrite); + } + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_Bind_Success) { WriteTestFileContent(VolumeTestFile1, "WSLC Mount Bind Test"); @@ -733,33 +766,83 @@ class WSLCE2EContainerCreateTests const auto hostDirectory = VolumeTestFile1.parent_path(); const auto fileName = VolumeTestFile1.filename().wstring(); auto result = RunWslc(std::format( - L"container create --name {} --mount \"type=bind,source={},target=/data,readonly\" {} cat /data/{}", + L"container create --name {} --mount \"type=bind,source={},target=/path:mntdir,readonly\" {} cat /path:mntdir/{}", WslcContainerName, hostDirectory.wstring(), DebianImage.NameAndTag(), fileName)); result.Verify({.Stderr = L"", .ExitCode = 0}); + const auto inspect = InspectContainer(WslcContainerName); + VERIFY_ARE_EQUAL(1u, inspect.Mounts.size()); + VERIFY_ARE_EQUAL("bind", inspect.Mounts[0].Type); + VERIFY_ARE_EQUAL(std::filesystem::canonical(hostDirectory).string(), inspect.Mounts[0].Source); + VERIFY_ARE_EQUAL("/path:mntdir", inspect.Mounts[0].Destination); + VERIFY_IS_FALSE(inspect.Mounts[0].ReadWrite); + result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); result.Verify({.Stdout = L"WSLC Mount Bind Test", .Stderr = L"", .ExitCode = 0}); } + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_Bind_MissingSource_Fails) + { + const auto source = VolumeTestFile1; + VERIFY_IS_TRUE(DeleteFileW(source.c_str())); + auto cleanupSource = wil::scope_exit([&]() { + std::error_code error; + std::filesystem::remove_all(source, error); + }); + + auto result = RunWslc(std::format( + L"container run --name {} --mount \"type=bind,source={},target=/data\" {} true", + WslcContainerName, + source.wstring(), + AlpineImage.NameAndTag())); + result.Verify({.Stdout = L"", .ExitCode = 1}); + VERIFY_IS_TRUE(result.StderrContainsSubstring(std::format(L"Bind source path does not exist: '{}'", source.wstring()))); + VERIFY_IS_FALSE(std::filesystem::exists(source)); + EnsureContainerDoesNotExist(WslcContainerName); + } + + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Volume_MissingSource_CreatesDirectory) + { + const auto source = VolumeTestFile1; + VERIFY_IS_TRUE(DeleteFileW(source.c_str())); + auto cleanupSource = wil::scope_exit([&]() { + std::error_code error; + std::filesystem::remove_all(source, error); + }); + + auto result = RunWslc(std::format( + L"container run --name {} --volume \"{}:/data\" {} true", WslcContainerName, source.wstring(), AlpineImage.NameAndTag())); + result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); + VERIFY_IS_TRUE(std::filesystem::is_directory(source)); + EnsureContainerDoesNotExist(WslcContainerName); + } + WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_Volume_Success) { auto result = RunWslc(std::format( - L"container create --name {} --mount type=volume,source={},target=/data {} sh -c \"echo -n 'WSLC Mount Volume " - L"Test' > /data/test.txt\"", + L"container create --name {} --mount type=volume,source={},target=/path:voldir {} sh -c \"echo -n 'WSLC Mount Volume " + L"Test' > /path:voldir/test.txt\"", WslcContainerName, WslcVolumeName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = 0}); + const auto inspect = InspectContainer(WslcContainerName); + VERIFY_ARE_EQUAL(1u, inspect.Mounts.size()); + VERIFY_ARE_EQUAL("volume", inspect.Mounts[0].Type); + VERIFY_ARE_EQUAL(string::WideToMultiByte(WslcVolumeName), inspect.Mounts[0].Source); + VERIFY_ARE_EQUAL("/path:voldir", inspect.Mounts[0].Destination); + VERIFY_IS_TRUE(inspect.Mounts[0].ReadWrite); + result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); EnsureContainerDoesNotExist(WslcContainerName); result = RunWslc(std::format( - L"container create --name {} --mount type=volume,source={},target=/data {} cat /data/test.txt", + L"container create --name {} --mount type=volume,source={},target=/path:voldir {} cat /path:voldir/test.txt", WslcContainerName, WslcVolumeName, DebianImage.NameAndTag())); @@ -807,20 +890,23 @@ class WSLCE2EContainerCreateTests WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_InvalidType_Fails) { - auto result = RunWslc(std::format( - L"container create --name {} --mount type=bogus,target=/x {} true", WslcContainerName, DebianImage.NameAndTag())); - VERIFY_ARE_EQUAL(1u, result.ExitCode.value()); - VERIFY_IS_TRUE(result.Stderr.has_value()); - VERIFY_ARE_NOT_EQUAL(std::wstring::npos, result.Stderr->find(L"for '--mount' flag")); + constexpr auto mount = L"type=bogus,target=/x"; + auto result = + RunWslc(std::format(L"container create --name {} --mount {} {} true", WslcContainerName, mount, DebianImage.NameAndTag())); + result.Verify({.Stdout = L"", .ExitCode = 1}); + VERIFY_IS_TRUE(result.StderrContainsSubstring( + Localization::WSLCCLI_UnsupportedMountError(mount, Localization::WSLCCLI_MountTypeUnsupportedError(L"bogus")))); EnsureContainerDoesNotExist(WslcContainerName); } WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_RelativeTarget_Fails) { - auto result = RunWslc(std::format( - L"container create --name {} --mount type=tmpfs,target=data {} true", WslcContainerName, DebianImage.NameAndTag())); + constexpr auto mount = L"type=tmpfs,target=data"; + auto result = + RunWslc(std::format(L"container create --name {} --mount {} {} true", WslcContainerName, mount, DebianImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE(result.StderrContainsSubstring(L"target path must be absolute")); + VERIFY_IS_TRUE(result.StderrContainsSubstring( + Localization::WSLCCLI_InvalidMountError(mount, Localization::WSLCCLI_MountTargetAbsoluteError()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -831,7 +917,7 @@ class WSLCE2EContainerCreateTests WslcContainerName, DebianImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE(result.StderrContainsSubstring(L"Duplicate mount point: /data")); + VERIFY_IS_TRUE(result.StderrContainsSubstring(Localization::WSLCCLI_DuplicateMountDestinationError(L"/data"))); EnsureContainerDoesNotExist(WslcContainerName); } diff --git a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp index 7e76fc568c..c19bd61a8e 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp @@ -1099,7 +1099,8 @@ class WSLCE2EContainerRunTests auto result = RunWslc(std::format(L"container run --rm --mount type=bogus,target=/x {} true", DebianImage.NameAndTag())); VERIFY_ARE_EQUAL(1u, result.ExitCode.value()); VERIFY_IS_TRUE(result.Stderr.has_value()); - VERIFY_ARE_NOT_EQUAL(std::wstring::npos, result.Stderr->find(L"for '--mount' flag")); + VERIFY_ARE_NOT_EQUAL(std::wstring::npos, result.Stderr->find(L"for '--mount' option")); + VERIFY_ARE_NOT_EQUAL(std::wstring::npos, result.Stderr->find(L"uses an unsupported feature")); } WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_DuplicateDestination_Fails) From 56e2593a64e8f7c00414d66abdce10227bafe631 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 17 Aug 2026 13:18:14 -0700 Subject: [PATCH 08/12] Update test errors to use loc strings --- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 6 ++---- test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp | 14 +++++++------- test/windows/wslc/e2e/WSLCE2EHelpers.h | 5 +++++ 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index 30c4b508a9..de9f978ab9 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -798,8 +798,7 @@ class WSLCE2EContainerCreateTests WslcContainerName, source.wstring(), AlpineImage.NameAndTag())); - result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE(result.StderrContainsSubstring(std::format(L"Bind source path does not exist: '{}'", source.wstring()))); + result.Verify({.Stdout = L"", .Stderr = FormatWslcError(Localization::MessageWslcBindSourcePathNotFound(source.wstring())), .ExitCode = 1}); VERIFY_IS_FALSE(std::filesystem::exists(source)); EnsureContainerDoesNotExist(WslcContainerName); } @@ -916,8 +915,7 @@ class WSLCE2EContainerCreateTests L"container create --name {} --mount type=tmpfs,target=/data --mount type=tmpfs,target=/data/ {} true", WslcContainerName, DebianImage.NameAndTag())); - result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE(result.StderrContainsSubstring(Localization::WSLCCLI_DuplicateMountDestinationError(L"/data"))); + result.Verify({.Stdout = L"", .Stderr = FormatWslcError(Localization::WSLCCLI_DuplicateMountDestinationError(L"/data")), .ExitCode = 1}); EnsureContainerDoesNotExist(WslcContainerName); } diff --git a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp index c19bd61a8e..0162259b75 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp @@ -17,6 +17,7 @@ Module Name: #include "WSLCE2EHelpers.h" namespace WSLCE2ETests { +using namespace wsl::shared; class WSLCE2EContainerRunTests { @@ -1096,11 +1097,11 @@ class WSLCE2EContainerRunTests WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_InvalidType_Fails) { - auto result = RunWslc(std::format(L"container run --rm --mount type=bogus,target=/x {} true", DebianImage.NameAndTag())); - VERIFY_ARE_EQUAL(1u, result.ExitCode.value()); - VERIFY_IS_TRUE(result.Stderr.has_value()); - VERIFY_ARE_NOT_EQUAL(std::wstring::npos, result.Stderr->find(L"for '--mount' option")); - VERIFY_ARE_NOT_EQUAL(std::wstring::npos, result.Stderr->find(L"uses an unsupported feature")); + constexpr auto mount = L"type=bogus,target=/x"; + auto result = RunWslc(std::format(L"container run --rm --mount {} {} true", mount, DebianImage.NameAndTag())); + result.Verify({.Stdout = L"", .ExitCode = 1}); + VERIFY_IS_TRUE(result.StderrContainsSubstring( + Localization::WSLCCLI_UnsupportedMountError(mount, Localization::WSLCCLI_MountTypeUnsupportedError(L"bogus")))); } WSLC_TEST_METHOD(WSLCE2E_Container_Run_Mount_DuplicateDestination_Fails) @@ -1109,8 +1110,7 @@ class WSLCE2EContainerRunTests L"container run --rm --name {} --mount type=tmpfs,target=/data --mount type=tmpfs,target=/data/ {} true", WslcContainerName, DebianImage.NameAndTag())); - result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE(result.StderrContainsSubstring(L"Duplicate mount point: /data")); + result.Verify({.Stdout = L"", .Stderr = FormatWslcError(Localization::WSLCCLI_DuplicateMountDestinationError(L"/data")), .ExitCode = 1}); EnsureContainerDoesNotExist(WslcContainerName); } diff --git a/test/windows/wslc/e2e/WSLCE2EHelpers.h b/test/windows/wslc/e2e/WSLCE2EHelpers.h index 9c692aab0b..3555743020 100644 --- a/test/windows/wslc/e2e/WSLCE2EHelpers.h +++ b/test/windows/wslc/e2e/WSLCE2EHelpers.h @@ -23,6 +23,11 @@ Module Name: namespace WSLCE2ETests { +inline std::wstring FormatWslcError(const std::wstring& message, std::wstring_view errorCode = L"E_INVALIDARG") +{ + return std::format(L"{}\r\nError code: {}\r\n", message, errorCode); +} + // VT sequence constants and helpers for TTY testing. // Sequences are sourced from wsl::windows::common::vt (VTSupport.h). namespace VT { From 4763936e27b6ea74c43fd42fd0b7b98dd070c577 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 17 Aug 2026 14:33:56 -0700 Subject: [PATCH 09/12] Option family, formatting fixes --- localization/strings/en-US/Resources.resw | 19 +++++------- src/windows/common/MountSpecParsing.cpp | 29 ++++++++++--------- .../wslc/WSLCCLIMountParserUnitTests.cpp | 4 ++- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 27 +++++++++++++++-- 4 files changed, 51 insertions(+), 28 deletions(-) diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index bb1176480a..8e8ddaf6ff 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -3031,30 +3031,30 @@ On first run, creates the file with all settings commented out at their defaults The field '{}' must be a key=value pair. - {FixedPlaceholder="{}"}{Locked="key=value"}Command line arguments and string inserts should not be translated + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated The key '{}' is unexpected in '{}'. - {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments and string inserts should not be translated + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated The value for '{}' is invalid: '{}'. - {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments and string inserts should not be translated + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated The value for '{}' is invalid: '{}'. Valid values are "enabled", "disabled", "writable", and "readonly". - {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}{Locked="enabled"}{Locked="disabled"}{Locked="writable"}{Locked="readonly"}Command line arguments and string inserts should not be translated + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated The mount type is required. Options matching '{}' cannot be used with mount type '{}'. - {FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments and string inserts should not be translated + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated Option '{}' requires the 'readonly' option. - {FixedPlaceholder="{}"}{Locked="readonly"}Command line arguments and string inserts should not be translated + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated Option 'bind-recursive=readonly' requires the 'bind-propagation=rprivate' option. @@ -3062,11 +3062,11 @@ On first run, creates the file with all settings commented out at their defaults Mount type '{}' is not supported. - {FixedPlaceholder="{}"}Command line arguments and string inserts should not be translated + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated Option '{}' is not supported. - {FixedPlaceholder="{}"}Command line arguments and string inserts should not be translated + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated The mount target is required. @@ -3089,9 +3089,6 @@ On first run, creates the file with all settings commented out at their defaults The bind source path must be absolute. {Locked="bind"}Command line arguments should not be translated - - Anonymous volume mounts are not supported. - The volume source must be a valid named volume. diff --git a/src/windows/common/MountSpecParsing.cpp b/src/windows/common/MountSpecParsing.cpp index a23ec69cae..0a43196c7f 100644 --- a/src/windows/common/MountSpecParsing.cpp +++ b/src/windows/common/MountSpecParsing.cpp @@ -343,6 +343,21 @@ Spec ParseDockerMountString(const std::wstring& value) ThrowParse(Localization::WSLCCLI_MountFieldKeyValueRequiredError(field)); } + switch (definition->OptionFamily) + { + case Family::General: + break; + case Family::Bind: + mount.HasBindOptions = true; + break; + case Family::Volume: + mount.HasVolumeOptions = true; + break; + case Family::Tmpfs: + mount.HasTmpfsOptions = true; + break; + } + switch (definition->Id) { case Field::Type: @@ -387,7 +402,6 @@ Spec ParseDockerMountString(const std::wstring& value) break; case Field::BindPropagation: - mount.HasBindOptions = true; mount.BindPropagation = AsciiToLower(std::wstring_view(keyValue.Value)); break; @@ -397,7 +411,6 @@ Spec ParseDockerMountString(const std::wstring& value) ThrowParse(Localization::WSLCCLI_MountInvalidValueError(key, keyValue.Value)); } - mount.HasBindOptions = true; break; case Field::BindRecursive: @@ -406,7 +419,6 @@ Spec ParseDockerMountString(const std::wstring& value) break; } - mount.HasBindOptions = true; RecordUnsupportedOption(mount, key); if (keyValue.Value == L"disabled") { @@ -431,13 +443,11 @@ Spec ParseDockerMountString(const std::wstring& value) ThrowParse(Localization::WSLCCLI_MountInvalidValueError(L"volume-nocopy", keyValue.Value)); } - mount.HasVolumeOptions = true; break; case Field::VolumeLabel: case Field::VolumeDriver: case Field::VolumeOption: - mount.HasVolumeOptions = true; break; case Field::TmpfsSize: @@ -447,7 +457,6 @@ Spec ParseDockerMountString(const std::wstring& value) ThrowParse(Localization::WSLCCLI_MountInvalidValueError(key, keyValue.Value)); } - mount.HasTmpfsOptions = true; break; case Field::TmpfsMode: @@ -457,7 +466,6 @@ Spec ParseDockerMountString(const std::wstring& value) ThrowParse(Localization::WSLCCLI_MountInvalidValueError(key, keyValue.Value)); } - mount.HasTmpfsOptions = true; break; } @@ -571,12 +579,7 @@ void ValidateMountSpec(const Spec& mount) break; case Type::Volume: - if (mount.Source.empty()) - { - ThrowUnsupported(Localization::WSLCCLI_MountAnonymousVolumeUnsupportedError()); - } - - if (!IsValidNamedVolumeName(mount.Source)) + if (!mount.Source.empty() && !IsValidNamedVolumeName(mount.Source)) { ThrowValidation(Localization::WSLCCLI_MountVolumeSourceInvalidError()); } diff --git a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp index a01420aea5..243260c044 100644 --- a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp @@ -62,6 +62,7 @@ namespace { }; constexpr ValidMountCase c_validMountCases[] = { + {L"type=volume,target=/data", mount::Type::Volume, L"", "/data", false, {}, {}, ""}, {L"source=data-volume,target=/data", mount::Type::Volume, L"data-volume", "/data", false, {}, {}, ""}, {L"type=volume,source=data-volume,target=/path:voldir", mount::Type::Volume, @@ -270,7 +271,6 @@ namespace { {L"type=npipe,source=data-volume,target=/data", Localization::WSLCCLI_MountTypeUnsupportedError(L"npipe"), ExpectedException::Unsupported}, {L"type=bogus,source=data-volume,target=/data", Localization::WSLCCLI_MountTypeUnsupportedError(L"bogus"), ExpectedException::Unsupported}, {L"type=CLUSTER,source=data-volume,target=/data", Localization::WSLCCLI_MountTypeUnsupportedError(L"cluster"), ExpectedException::Unsupported}, - {L"type=volume,target=/data", Localization::WSLCCLI_MountAnonymousVolumeUnsupportedError(), ExpectedException::Unsupported}, {L"type=bind,target=/data", Localization::WSLCCLI_MountSourceRequiredError(), ExpectedException::Validation}, {L"type=bind,source=relative,target=/data", Localization::WSLCCLI_MountBindSourceAbsoluteError(), ExpectedException::Validation}, {L"type=volume,source=a,target=/data", Localization::WSLCCLI_MountVolumeSourceInvalidError(), ExpectedException::Validation}, @@ -347,6 +347,8 @@ namespace { Localization::WSLCCLI_MountOptionFamilyMismatchError(L"volume-*", L"bind")}, {L"type=volume,source=data-volume,target=/data,bind-propagation=rprivate", Localization::WSLCCLI_MountOptionFamilyMismatchError(L"bind-*", L"volume")}, + {L"type=volume,source=data-volume,target=/data,bind-recursive=enabled", + Localization::WSLCCLI_MountOptionFamilyMismatchError(L"bind-*", L"volume")}, {L"type=volume,source=data-volume,target=/data,tmpfs-size=1m", Localization::WSLCCLI_MountOptionFamilyMismatchError(L"tmpfs-*", L"volume")}, {L"type=tmpfs,target=/tmp,volume-label=a=b", Localization::WSLCCLI_MountOptionFamilyMismatchError(L"volume-*", L"tmpfs")}, diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index e6651bb454..673ee786ad 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -716,8 +716,9 @@ class WSLCE2EContainerCreateTests WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_Tmpfs_Success) { auto result = RunWslc(std::format( - L"container create --name {} --mount type=tmpfs,target=/path:tmpfs {} sh -c \"echo -n 'tmpfs_test' > " - L"/path:tmpfs/data && cat /path:tmpfs/data\"", + L"container create --name {} --mount type=tmpfs,target=/path:tmpfs,tmpfs-size=1MB,tmpfs-mode=0700 {} sh -c " + L"\"echo -n 'tmpfs_test' > /path:tmpfs/data && cat /path:tmpfs/data && echo && stat -c '%a' /path:tmpfs && " + L"df -k /path:tmpfs | awk 'NR == 2 {{print $2}}'\"", WslcContainerName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = 0}); @@ -730,7 +731,7 @@ class WSLCE2EContainerCreateTests VERIFY_IS_TRUE(inspect.Mounts[0].ReadWrite); result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); - result.Verify({.Stdout = L"tmpfs_test", .Stderr = L"", .ExitCode = 0}); + result.Verify({.Stdout = L"tmpfs_test\n700\n1024\n", .Stderr = L"", .ExitCode = 0}); } WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_Tmpfs_PreservesMountForm) @@ -849,6 +850,26 @@ class WSLCE2EContainerCreateTests result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); result.Verify({.Stdout = L"WSLC Mount Volume Test", .Stderr = L"", .ExitCode = 0}); + EnsureContainerDoesNotExist(WslcContainerName); + + result = RunWslc(std::format( + L"container create --rm --name {} --mount type=volume,target=/anonymous {} sh -c " + L"\"echo -n anonymous-volume > /anonymous/value && cat /anonymous/value\"", + WslcContainerName, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = 0}); + + const auto anonymousInspect = InspectContainer(WslcContainerName); + VERIFY_ARE_EQUAL(1u, anonymousInspect.Mounts.size()); + VERIFY_ARE_EQUAL("volume", anonymousInspect.Mounts[0].Type); + VERIFY_IS_FALSE(anonymousInspect.Mounts[0].Name.empty()); + VERIFY_IS_TRUE(anonymousInspect.Mounts[0].Source.empty()); + VERIFY_ARE_EQUAL("/anonymous", anonymousInspect.Mounts[0].Destination); + VERIFY_IS_TRUE(anonymousInspect.Mounts[0].ReadWrite); + + result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); + result.Verify({.Stdout = L"anonymous-volume", .Stderr = L"", .ExitCode = 0}); + EnsureContainerDoesNotExist(WslcContainerName); } WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_ReadOnly_IsReadOnly) From 3f0d07b1031f5521f8f582e45fd614c5cb4f8362 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 17 Aug 2026 14:40:57 -0700 Subject: [PATCH 10/12] Incorporate new storage parser from master --- src/windows/common/MountSpecParsing.cpp | 103 +----------------------- 1 file changed, 4 insertions(+), 99 deletions(-) diff --git a/src/windows/common/MountSpecParsing.cpp b/src/windows/common/MountSpecParsing.cpp index 0a43196c7f..d14c125852 100644 --- a/src/windows/common/MountSpecParsing.cpp +++ b/src/windows/common/MountSpecParsing.cpp @@ -14,16 +14,13 @@ Module Name: #include "precomp.h" #include "MountSpecParsing.h" +#include "string.hpp" #include #include -#include -#include -#include #include #include #include #include -#include #include #include @@ -165,105 +162,13 @@ namespace { std::optional ParseDockerRamInBytes(const std::wstring& value) { - const auto input = WideToMultiByte(value); - const auto separator = input.find_last_of("01234567890. "); - if (separator == std::string::npos) + const auto parsed = wsl::windows::common::string::ParseStorageSize(value, wsl::windows::common::string::StorageSizeUnit::Binary); + if (!parsed.has_value() || parsed.value() > std::numeric_limits::max()) { return std::nullopt; } - std::string number; - std::string suffix; - if (input[separator] == ' ') - { - number = input.substr(0, separator); - suffix = input.substr(separator + 1); - } - else - { - number = input.substr(0, separator + 1); - suffix = input.substr(separator + 1); - } - - if (number.empty() || std::isspace(static_cast(number.front()))) - { - return std::nullopt; - } - - std::string_view numberView(number); - if (numberView.front() == '+') - { - numberView.remove_prefix(1); - if (numberView.empty()) - { - return std::nullopt; - } - } - - double parsed{}; - const auto parseResult = - std::from_chars(numberView.data(), numberView.data() + numberView.size(), parsed, std::chars_format::general); - if (parseResult.ec != std::errc() || parseResult.ptr != numberView.data() + numberView.size() || !std::isfinite(parsed) || parsed < 0) - { - return std::nullopt; - } - - double bytes = parsed; - if (!suffix.empty()) - { - suffix = AsciiToLower(std::string_view(suffix)); - if (suffix.size() > 3) - { - return std::nullopt; - } - - if (suffix.front() == 'b') - { - if (suffix.size() != 1) - { - return std::nullopt; - } - } - else - { - uint64_t factor{}; - switch (suffix.front()) - { - case 'k': - factor = 1ULL << 10; - break; - case 'm': - factor = 1ULL << 20; - break; - case 'g': - factor = 1ULL << 30; - break; - case 't': - factor = 1ULL << 40; - break; - case 'p': - factor = 1ULL << 50; - break; - default: - return std::nullopt; - } - - if ((suffix.size() == 2 && suffix[1] != 'b') || (suffix.size() == 3 && suffix.substr(1) != "ib")) - { - return std::nullopt; - } - - bytes *= static_cast(factor); - } - } - - constexpr double c_int64ExclusiveUpperBound = static_cast(uint64_t{1} << std::numeric_limits::digits); - if (!std::isfinite(bytes) || bytes >= c_int64ExclusiveUpperBound) - { - return std::nullopt; - } - - return static_cast(bytes); + return static_cast(parsed.value()); } std::optional ParseDockerTmpfsMode(const std::wstring& value) From afdecbe4332f9ae515219915be6f171e9dfeb386 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 17 Aug 2026 15:41:51 -0700 Subject: [PATCH 11/12] Merge volume and tmpfs specs into mount spec, fix build break --- src/windows/common/MountSpecParsing.cpp | 77 +++++++++++++++- src/windows/common/MountSpecParsing.h | 8 ++ src/windows/common/WSLCContainerLauncher.cpp | 43 ++++----- src/windows/common/WSLCContainerLauncher.h | 5 -- src/windows/service/inc/wslc.idl | 3 +- .../wslc/arguments/ArgumentDefinitions.h | 2 +- .../wslc/arguments/ArgumentValidation.cpp | 21 +++-- .../wslc/arguments/ArgumentValidation.h | 1 - src/windows/wslc/services/ContainerModel.cpp | 89 +++---------------- src/windows/wslc/services/ContainerModel.h | 11 --- .../wslc/services/ContainerService.cpp | 16 ---- src/windows/wslc/tasks/ContainerTasks.cpp | 9 +- src/windows/wslcsession/WSLCContainer.cpp | 20 ++++- .../wslc/WSLCCLIMountParserUnitTests.cpp | 29 +++--- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 83 ++++++++--------- 15 files changed, 205 insertions(+), 212 deletions(-) diff --git a/src/windows/common/MountSpecParsing.cpp b/src/windows/common/MountSpecParsing.cpp index d14c125852..83ea521a0c 100644 --- a/src/windows/common/MountSpecParsing.cpp +++ b/src/windows/common/MountSpecParsing.cpp @@ -163,7 +163,7 @@ namespace { std::optional ParseDockerRamInBytes(const std::wstring& value) { const auto parsed = wsl::windows::common::string::ParseStorageSize(value, wsl::windows::common::string::StorageSizeUnit::Binary); - if (!parsed.has_value() || parsed.value() > std::numeric_limits::max()) + if (!parsed.has_value() || parsed.value() > static_cast(std::numeric_limits::max())) { return std::nullopt; } @@ -447,6 +447,81 @@ Spec ParseDockerMountString(const std::wstring& value) }; } +Spec ParseDockerVolumeString(const std::wstring& value) +{ + const auto formatUsage = Localization::WSLCCLI_VolumeFormatUsage(); + const auto lastColon = value.rfind(L':'); + if (lastColon == std::wstring::npos) + { + ThrowParse(Localization::WSLCCLI_VolumeInvalidSpec(value, formatUsage)); + } + + auto splitColon = lastColon; + bool readOnly = false; + std::wstring_view lastToken{value.data() + lastColon + 1, value.size() - lastColon - 1}; + if (lastToken == L"ro" || lastToken == L"rw") + { + readOnly = lastToken == L"ro"; + if (lastColon == 0) + { + ThrowParse(Localization::WSLCCLI_VolumeInvalidSpec(value, formatUsage)); + } + + splitColon = value.rfind(L':', lastColon - 1); + if (splitColon == std::wstring::npos) + { + ThrowParse(Localization::WSLCCLI_VolumeInvalidSpec(value, formatUsage)); + } + } + + const auto targetEnd = lastToken == L"ro" || lastToken == L"rw" ? lastColon : value.size(); + const auto target = value.substr(splitColon + 1, targetEnd - splitColon - 1); + if (target.empty()) + { + ThrowParse(Localization::WSLCCLI_VolumeContainerPathEmpty(value, formatUsage)); + } + + if (target.front() != L'/') + { + ThrowParse(Localization::WSLCCLI_VolumeContainerPathNotAbsolute(value, formatUsage)); + } + + const auto rawSource = value.substr(0, splitColon); + if (rawSource.empty()) + { + ThrowParse(Localization::WSLCCLI_VolumeHostPathEmpty(value, formatUsage)); + } + + if (IsValidNamedVolumeName(rawSource)) + { + return { + .MountType = Type::Volume, + .Source = rawSource, + .Target = WideToMultiByte(target), + .ReadOnly = readOnly, + }; + } + + std::wstring source; + if (FAILED(wil::GetFullPathNameW(rawSource.c_str(), source))) + { + ThrowParse(Localization::WSLCCLI_VolumeHostPathInvalid(value, rawSource)); + } + + if (GetFileAttributesW(source.c_str()) == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_INVALID_NAME) + { + ThrowParse(Localization::WSLCCLI_VolumeHostPathInvalid(value, rawSource)); + } + + return { + .MountType = Type::Bind, + .Source = std::move(source), + .Target = WideToMultiByte(target), + .ReadOnly = readOnly, + .BindSource = BindSourcePolicy::CreateIfMissing, + }; +} + void ValidateMountSpec(const Spec& mount) { if (mount.Target.empty()) diff --git a/src/windows/common/MountSpecParsing.h b/src/windows/common/MountSpecParsing.h index 6e4020c5f4..e330bd1959 100644 --- a/src/windows/common/MountSpecParsing.h +++ b/src/windows/common/MountSpecParsing.h @@ -33,12 +33,19 @@ enum class Type Tmpfs, }; +enum class BindSourcePolicy +{ + RequireExisting, + CreateIfMissing, +}; + struct Spec { Type MountType = Type::Volume; std::wstring Source; std::string Target; bool ReadOnly = false; + BindSourcePolicy BindSource = BindSourcePolicy::RequireExisting; std::optional TmpfsSizeBytes; std::optional TmpfsMode; }; @@ -106,6 +113,7 @@ class MountValidationException : public MountException }; Spec ParseDockerMountString(const std::wstring& value); +Spec ParseDockerVolumeString(const std::wstring& value); void ValidateMountSpec(const Spec& mount); void ValidateMountCollection(std::span mounts); std::string FormatTmpfsOptions(const Spec& mount); diff --git a/src/windows/common/WSLCContainerLauncher.cpp b/src/windows/common/WSLCContainerLauncher.cpp index 146c3c2fe2..a967893e9d 100644 --- a/src/windows/common/WSLCContainerLauncher.cpp +++ b/src/windows/common/WSLCContainerLauncher.cpp @@ -229,29 +229,23 @@ void WSLCContainerLauncher::AddUlimit(const std::string& Name, std::int64_t Soft void wsl::windows::common::WSLCContainerLauncher::AddVolume(const std::wstring& HostPath, const std::string& ContainerPath, bool ReadOnly) { - // Store a copy of the path strings to the launcher to ensure the pointers in WSLCVolume remain valid. - const auto& hostPath = m_hostPaths.emplace_back(HostPath); - const auto& containerPath = m_containerPaths.emplace_back(ContainerPath); - - WSLCVolume vol{}; - vol.HostPath = hostPath.c_str(); - vol.ContainerPath = containerPath.c_str(); - vol.ReadOnly = ReadOnly ? TRUE : FALSE; - - m_volumes.push_back(vol); + AddMount({ + .MountType = mount::Type::Bind, + .Source = HostPath, + .Target = ContainerPath, + .ReadOnly = ReadOnly, + .BindSource = mount::BindSourcePolicy::CreateIfMissing, + }); } void wsl::windows::common::WSLCContainerLauncher::AddNamedVolume(const std::string& Name, const std::string& ContainerPath, bool ReadOnly) { - const auto& name = m_volumeNames.emplace_back(Name); - const auto& containerPath = m_containerPaths.emplace_back(ContainerPath); - - WSLCNamedVolume volume{}; - volume.Name = name.c_str(); - volume.ContainerPath = containerPath.c_str(); - volume.ReadOnly = ReadOnly ? TRUE : FALSE; - - m_namedVolumes.push_back(volume); + AddMount({ + .MountType = mount::Type::Volume, + .Source = wsl::shared::string::MultiByteToWide(Name), + .Target = ContainerPath, + .ReadOnly = ReadOnly, + }); } void wsl::windows::common::WSLCContainerLauncher::AddMount(const mount::Spec& Mount) @@ -279,6 +273,11 @@ void wsl::windows::common::WSLCContainerLauncher::AddMount(const mount::Spec& Mo mount.Target = m_mountTargets.emplace_back(Mount.Target).c_str(); mount.ReadOnly = Mount.ReadOnly ? TRUE : FALSE; + if (Mount.MountType == mount::Type::Bind && Mount.BindSource == mount::BindSourcePolicy::CreateIfMissing) + { + WI_SetFlag(mount.Flags, WSLCMountSpecFlagsCreateSourceIfMissing); + } + if (Mount.TmpfsSizeBytes.has_value()) { WI_SetFlag(mount.Flags, WSLCMountSpecFlagsTmpfsSize); @@ -452,12 +451,6 @@ std::pair> WSLCContainerLauncher::C options.InitProcessOptions.CurrentDirectory = m_workingDirectory.c_str(); } - options.VolumesCount = static_cast(m_volumes.size()); - options.Volumes = m_volumes.size() > 0 ? m_volumes.data() : nullptr; - - options.NamedVolumesCount = static_cast(m_namedVolumes.size()); - options.NamedVolumes = m_namedVolumes.size() > 0 ? m_namedVolumes.data() : nullptr; - options.MountsCount = static_cast(m_mounts.size()); options.Mounts = m_mounts.size() > 0 ? m_mounts.data() : nullptr; diff --git a/src/windows/common/WSLCContainerLauncher.h b/src/windows/common/WSLCContainerLauncher.h index 4b43aa4f03..39cf63dfe1 100644 --- a/src/windows/common/WSLCContainerLauncher.h +++ b/src/windows/common/WSLCContainerLauncher.h @@ -111,12 +111,7 @@ class WSLCContainerLauncher : private WSLCProcessLauncher std::string m_image; std::string m_name; std::vector m_ports; - std::vector m_volumes; - std::vector m_namedVolumes; std::vector m_mounts; - std::deque m_hostPaths; - std::deque m_volumeNames; - std::deque m_containerPaths; std::deque m_mountSources; std::deque m_mountTargets; std::string m_networkMode; diff --git a/src/windows/service/inc/wslc.idl b/src/windows/service/inc/wslc.idl index 6ac1b443b0..f24e13d459 100644 --- a/src/windows/service/inc/wslc.idl +++ b/src/windows/service/inc/wslc.idl @@ -257,9 +257,10 @@ typedef enum _WSLCMountSpecFlags WSLCMountSpecFlagsNone = 0, WSLCMountSpecFlagsTmpfsSize = 1, WSLCMountSpecFlagsTmpfsMode = 2, + WSLCMountSpecFlagsCreateSourceIfMissing = 4, } WSLCMountSpecFlags; -cpp_quote("#define WSLCMountSpecFlagsValid (WSLCMountSpecFlagsTmpfsSize | WSLCMountSpecFlagsTmpfsMode)") +cpp_quote("#define WSLCMountSpecFlagsValid (WSLCMountSpecFlagsTmpfsSize | WSLCMountSpecFlagsTmpfsMode | WSLCMountSpecFlagsCreateSourceIfMissing)") cpp_quote("DEFINE_ENUM_FLAG_OPERATORS(WSLCMountSpecFlags);") typedef struct _WSLCMountSpec diff --git a/src/windows/wslc/arguments/ArgumentDefinitions.h b/src/windows/wslc/arguments/ArgumentDefinitions.h index 0f5d4d65de..ac57ed8cdd 100644 --- a/src/windows/wslc/arguments/ArgumentDefinitions.h +++ b/src/windows/wslc/arguments/ArgumentDefinitions.h @@ -140,7 +140,7 @@ _(Username, "username", L"u", Kind::Value, _(Verbose, "verbose", NO_ALIAS, Kind::Flag, NoConversion, Localization::WSLCCLI_VerboseArgDescription()) \ _(Version, "version", L"v", Kind::Flag, NoConversion, Localization::WSLCCLI_VersionArgDescription()) \ /*_(Virtual, "virtualization", NO_ALIAS, Kind::Value, NoConversion, Localization::WSLCCLI_VirtualArgDescription())*/ \ -_(Volume, "volume", L"v", Kind::Value, NoConversion, Localization::WSLCCLI_VolumeArgDescription()) \ +_(Volume, "volume", L"v", Kind::Value, ParsedMount, Localization::WSLCCLI_VolumeArgDescription()) \ _(VolumeName, "volume-name", NO_ALIAS, Kind::Positional, NoConversion, Localization::WSLCCLI_VolumeNameArgDescription()) \ _(Volumes, "volumes", L"v", Kind::Flag, NoConversion, Localization::WSLCCLI_RemoveVolumesArgDescription()) \ _(WorkDir, "workdir", L"w", Kind::Value, NoConversion, Localization::WSLCCLI_WorkingDirArgDescription()) \ diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp index 8ca6628259..ddd57f8b9e 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.cpp +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -231,7 +231,18 @@ void Argument::Validate(ArgMap& execArgs) const break; case ArgType::Volume: - validation::ValidateVolumeMount(RawArgMapAccess::GetAll(execArgs)); + CacheConverted(execArgs, m_name, [](const std::wstring& value, const std::wstring&) { + try + { + auto mountSpec = mount::ParseDockerVolumeString(value); + mount::ValidateMountSpec(mountSpec); + return mountSpec; + } + catch (const mount::MountException& ex) + { + throw ArgumentException(ex.Reason()); + } + }); break; case ArgType::Mount: @@ -326,14 +337,6 @@ void ValidateWSLCSignalFromString(const std::vector& values, const } } -void ValidateVolumeMount(const std::vector& values) -{ - for (const auto& value : values) - { - std::ignore = models::VolumeMount::Parse(value); - } -} - // Validates that each --filter argument is in the form "key=value". Rejects entries without an '='; // the runtime validates the key and value for specific objects. void ValidateFilter(const std::vector& values) diff --git a/src/windows/wslc/arguments/ArgumentValidation.h b/src/windows/wslc/arguments/ArgumentValidation.h index 880d344ae9..f0b84f6262 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.h +++ b/src/windows/wslc/arguments/ArgumentValidation.h @@ -78,7 +78,6 @@ void ValidateFormatTypeFromString(const std::vector& values, const void ValidateGpus(const std::vector& values, const std::wstring& argName); -void ValidateVolumeMount(const std::vector& values); void ValidateFilter(const std::vector& values); } // namespace wsl::windows::wslc::validation diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp index 1009c0d6e6..88357a9847 100644 --- a/src/windows/wslc/services/ContainerModel.cpp +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -161,83 +161,23 @@ bool VolumeMount::IsValidNamedVolumeName(const std::wstring& name) VolumeMount VolumeMount::Parse(const std::wstring& value) { - auto lastColon = value.rfind(':'); - if (lastColon == std::wstring::npos) - { - THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_VolumeInvalidSpec(value, Localization::WSLCCLI_VolumeFormatUsage())); - } - - VolumeMount vm; - auto splitColon = lastColon; - const auto lastToken = value.substr(lastColon + 1); - if (IsValidMode(lastToken)) - { - vm.m_isReadOnlyMode = IsReadOnlyMode(lastToken); - if (lastColon == 0) - { - THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_VolumeInvalidSpec(value, Localization::WSLCCLI_VolumeFormatUsage())); - } - - splitColon = value.rfind(':', lastColon - 1); - if (splitColon == std::wstring::npos) - { - THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_VolumeInvalidSpec(value, Localization::WSLCCLI_VolumeFormatUsage())); - } - - vm.m_containerPath = WideToMultiByte(value.substr(splitColon + 1, lastColon - splitColon - 1)); - } - else - { - vm.m_containerPath = WideToMultiByte(lastToken); - } - - if (vm.m_containerPath.empty()) - { - THROW_HR_WITH_USER_ERROR( - E_INVALIDARG, Localization::WSLCCLI_VolumeContainerPathEmpty(value, Localization::WSLCCLI_VolumeFormatUsage())); - } - - if (vm.m_containerPath[0] != '/') - { - THROW_HR_WITH_USER_ERROR( - E_INVALIDARG, Localization::WSLCCLI_VolumeContainerPathNotAbsolute(value, Localization::WSLCCLI_VolumeFormatUsage())); - } - - const auto rawHostPath = value.substr(0, splitColon); - if (rawHostPath.empty()) - { - THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_VolumeHostPathEmpty(value, Localization::WSLCCLI_VolumeFormatUsage())); - } - - // This is where we need to check if the user is referencing a named volume. - // This can be either an existing named volume or a new named volume that will be created. - if (VolumeMount::IsValidNamedVolumeName(rawHostPath)) + mount::Spec mountSpec; + try { - vm.m_isNamedVolume = true; - vm.m_host = rawHostPath; + mountSpec = mount::ParseDockerVolumeString(value); + mount::ValidateMountSpec(mountSpec); } - else + catch (const mount::MountException& ex) { - // Not a named volume, so it must be a path. - // Use wil::GetFullPathNameW to resolve relative paths against the CWD. - std::wstring resolvedHostPath; - const auto hr = wil::GetFullPathNameW(rawHostPath.c_str(), resolvedHostPath); - if (FAILED(hr)) - { - THROW_HR_WITH_USER_ERROR(hr, Localization::WSLCCLI_VolumeHostPathInvalid(value, rawHostPath)); - } - - // GetFileAttributesW validates the resolved path syntax without requiring existence. - // ERROR_INVALID_NAME indicates illegal characters in the path (e.g. ":" as a component). - if (GetFileAttributesW(resolvedHostPath.c_str()) == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_INVALID_NAME) - { - THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_VolumeHostPathInvalid(value, rawHostPath)); - } - - vm.m_host = std::move(resolvedHostPath); + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, ex.Reason()); } - return vm; + VolumeMount volume; + volume.m_host = std::move(mountSpec.Source); + volume.m_containerPath = std::move(mountSpec.Target); + volume.m_isReadOnlyMode = mountSpec.ReadOnly; + volume.m_isNamedVolume = mountSpec.MountType == mount::Type::Volume; + return volume; } std::optional EnvironmentVariable::Parse(const std::wstring& entry) @@ -358,11 +298,6 @@ void ValidateUniqueMountDestinations(const ContainerOptions& options) !destinations.emplace(normalizedDestination).second); }; - for (const auto& volumeSpec : options.Volumes) - { - addDestination(VolumeMount::Parse(volumeSpec).ContainerPath()); - } - for (const auto& tmpfsSpec : options.Tmpfs) { addDestination(TmpfsMount::Parse(tmpfsSpec).ContainerPath()); diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index 543215ccb4..14b332dcfa 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -76,7 +76,6 @@ struct ContainerOptions bool NoHealthcheck = false; bool Gpu = false; std::vector Ports; - std::vector Volumes; std::vector Mounts; std::string WorkingDirectory; std::vector Entrypoint; @@ -309,16 +308,6 @@ struct VolumeMount std::string m_containerPath; bool m_isReadOnlyMode = false; bool m_isNamedVolume = false; - - static bool IsReadOnlyMode(const std::wstring& mode) - { - return mode == L"ro"; - } - - static bool IsValidMode(const std::wstring& mode) - { - return IsReadOnlyMode(mode) || mode == L"rw"; - } }; struct TmpfsMount diff --git a/src/windows/wslc/services/ContainerService.cpp b/src/windows/wslc/services/ContainerService.cpp index ef7f9446c3..7d7ca445d8 100644 --- a/src/windows/wslc/services/ContainerService.cpp +++ b/src/windows/wslc/services/ContainerService.cpp @@ -150,22 +150,6 @@ static wsl::windows::common::RunningWSLCContainer CreateInternal(Terminal& termi } } - // Add volumes if specified - for (const auto& volumeSpec : options.Volumes) - { - auto volume = VolumeMount::Parse(volumeSpec); - auto host = volume.Host(); - auto container = volume.ContainerPath(); - if (volume.IsNamedVolume()) - { - containerLauncher.AddNamedVolume(string::WideToMultiByte(host), container, volume.IsReadOnly()); - } - else - { - containerLauncher.AddVolume(host, container, volume.IsReadOnly()); - } - } - for (const auto& mountSpec : options.Mounts) { containerLauncher.AddMount(mountSpec); diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index 2bc00ef46a..af6ef07e76 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -659,16 +659,13 @@ void SetContainerOptionsFromArgs(CLIExecutionContext& context) if (context.Args.Contains(ArgType::Volume)) { auto volumes = context.Args.GetAllValues(); - options.Volumes.reserve(options.Volumes.size() + volumes.size()); - for (const auto& volume : volumes) - { - options.Volumes.emplace_back(volume); - } + options.Mounts.insert(options.Mounts.end(), std::make_move_iterator(volumes.begin()), std::make_move_iterator(volumes.end())); } if (context.Args.Contains(ArgType::Mount)) { - options.Mounts = context.Args.GetAllValues(); + auto mounts = context.Args.GetAllValues(); + options.Mounts.insert(options.Mounts.end(), std::make_move_iterator(mounts.begin()), std::make_move_iterator(mounts.end())); } options.Remove = context.Args.GetValue(); diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 15dde33e6e..63d30b0759 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -595,11 +595,19 @@ std::vector ConvertAndValidateMounts(const WS THROW_HR_MSG(E_INVALIDARG, "Mount at index %lu has invalid type: %d", i, value.Type); } + THROW_HR_IF_MSG( + E_INVALIDARG, + type != mount::Type::Bind && WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsCreateSourceIfMissing), + "Mount at index %lu specifies create-source-if-missing for a non-bind mount", + i); + mounts.push_back({ .MountType = type, .Source = value.Source != nullptr ? value.Source : L"", .Target = value.Target, .ReadOnly = static_cast(value.ReadOnly), + .BindSource = WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsCreateSourceIfMissing) ? mount::BindSourcePolicy::CreateIfMissing + : mount::BindSourcePolicy::RequireExisting, .TmpfsSizeBytes = WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsTmpfsSize) ? std::optional{value.TmpfsSizeBytes} : std::nullopt, .TmpfsMode = WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsTmpfsMode) ? std::optional{value.TmpfsMode} : std::nullopt, }); @@ -612,6 +620,11 @@ std::vector ConvertAndValidateMounts(const WS { if (mount.MountType == mount::Type::Bind) { + if (mount.BindSource == mount::BindSourcePolicy::CreateIfMissing) + { + continue; + } + std::error_code error; const auto sourceExists = std::filesystem::exists(mount.Source, error); if (error) @@ -2141,7 +2154,10 @@ std::shared_ptr WSLCContainerImpl::Create( case wsl::windows::common::mount::Type::Bind: { // Docker's colon-delimited bind format cannot represent ':' in the target. - auto prepared = PrepareBindMount(mount.Source, mount.Target, mount.ReadOnly, MissingBindSource::Reject); + const auto missingSource = mount.BindSource == wsl::windows::common::mount::BindSourcePolicy::CreateIfMissing + ? MissingBindSource::Create + : MissingBindSource::Reject; + auto prepared = PrepareBindMount(mount.Source, mount.Target, mount.ReadOnly, missingSource); dockerMount.Source = std::move(prepared.DockerSource); dockerMount.Type = "bind"; volumes.push_back(std::move(prepared.Volume)); @@ -2374,7 +2390,7 @@ std::shared_ptr WSLCContainerImpl::Create( for (const auto& mount : mounts) { - if (mount.MountType == wsl::windows::common::mount::Type::Volume) + if (mount.MountType == wsl::windows::common::mount::Type::Volume && !mount.Source.empty()) { namedVolumes.emplace_back(wsl::shared::string::WideToMultiByte(mount.Source)); } diff --git a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp index 243260c044..6847d0e09a 100644 --- a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp @@ -212,14 +212,6 @@ namespace { {}, {}, ""}, - {L"type=volume,source=data-volume,target=/data,bind-recursive=enabled", - mount::Type::Volume, - L"data-volume", - "/data", - false, - {}, - {}, - ""}, {L"type=volume,source=A_,target=/data", mount::Type::Volume, L"A_", "/data", false, {}, {}, ""}, {L"type=volume,source=data.volume-1,target=/data", mount::Type::Volume, L"data.volume-1", "/data", false, {}, {}, ""}, {L"type=tmpfs,target=/tmp", mount::Type::Tmpfs, L"", "/tmp", false, {}, {}, ""}, @@ -439,6 +431,23 @@ class WSLCCLIMountParserUnitTests } } + TEST_METHOD(Volume_ValidCases) + { + const auto bind = mount::ParseDockerVolumeString(LR"(C:\hostPath:/data:ro)"); + VERIFY_ARE_EQUAL(static_cast(mount::Type::Bind), static_cast(bind.MountType)); + VERIFY_ARE_EQUAL(std::wstring(LR"(C:\hostPath)"), bind.Source); + VERIFY_ARE_EQUAL(std::string("/data"), bind.Target); + VERIFY_IS_TRUE(bind.ReadOnly); + VERIFY_ARE_EQUAL(static_cast(mount::BindSourcePolicy::CreateIfMissing), static_cast(bind.BindSource)); + + const auto volume = mount::ParseDockerVolumeString(L"named-volume:/data"); + VERIFY_ARE_EQUAL(static_cast(mount::Type::Volume), static_cast(volume.MountType)); + VERIFY_ARE_EQUAL(std::wstring(L"named-volume"), volume.Source); + VERIFY_ARE_EQUAL(std::string("/data"), volume.Target); + VERIFY_IS_FALSE(volume.ReadOnly); + VERIFY_ARE_EQUAL(static_cast(mount::BindSourcePolicy::RequireExisting), static_cast(volume.BindSource)); + } + TEST_METHOD(Mount_DotRelativeBindSourceUsesCurrentDirectory) { const auto expected = (std::filesystem::current_path() / L"mount").lexically_normal().wstring(); @@ -520,8 +529,8 @@ class WSLCCLIMountParserUnitTests options.Tmpfs.clear(); options.Mounts = { {.MountType = mount::Type::Tmpfs, .Target = "/data/../cache"}, + {.MountType = mount::Type::Volume, .Source = L"data-volume", .Target = "/cache"}, }; - options.Volumes = {L"data-volume:/cache"}; VERIFY_THROWS(ValidateUniqueMountDestinations(options), wil::ResultException); } @@ -529,8 +538,8 @@ class WSLCCLIMountParserUnitTests { ContainerOptions options; options.Tmpfs = {"/cache"}; - options.Volumes = {L"data-volume:/data"}; options.Mounts = { + {.MountType = mount::Type::Volume, .Source = L"data-volume", .Target = "/data"}, {.MountType = mount::Type::Bind, .Source = L"C:\\logs", .Target = "/logs"}, }; VERIFY_NO_THROW(ValidateUniqueMountDestinations(options)); diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index 85ec63b752..5ada146960 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -354,8 +354,7 @@ class WSLCE2EContainerCreateTests RunWslc(std::format(L"container run --name {} --volume :/containerPath {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: ':/containerPath'. Host path cannot be empty. Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + Localization::WSLCCLI_VolumeHostPathEmpty(L":/containerPath", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -364,8 +363,7 @@ class WSLCE2EContainerCreateTests std::format(L"container run --name {} --volume C:\\hostPath::ro {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: 'C:\\hostPath::ro'. Container path cannot be empty. Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + Localization::WSLCCLI_VolumeContainerPathEmpty(L"C:\\hostPath::ro", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -374,17 +372,15 @@ class WSLCE2EContainerCreateTests std::format(L"container run --name {} --volume :/containerPath:ro {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: ':/containerPath:ro'. Host path cannot be empty. Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + Localization::WSLCCLI_VolumeHostPathEmpty(L":/containerPath:ro", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } { auto result = RunWslc(std::format(L"container run --name {} --volume \"\" {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE( - result.StderrContainsSubstring(L"Invalid volume specifications: ''. Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + VERIFY_IS_TRUE(result.StderrContainsSubstring( + Localization::WSLCCLI_VolumeInvalidSpec(L"", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -393,8 +389,7 @@ class WSLCE2EContainerCreateTests RunWslc(std::format(L"container run --name {} --volume C:\\hostPath: {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: 'C:\\hostPath:'. Container path cannot be empty. Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + Localization::WSLCCLI_VolumeContainerPathEmpty(L"C:\\hostPath:", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -403,17 +398,15 @@ class WSLCE2EContainerCreateTests RunWslc(std::format(L"container run --name {} --volume C:\\hostPath:ro {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: 'C:\\hostPath:ro'. Container path must be an absolute path (starting with '/'). " - L"Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + Localization::WSLCCLI_VolumeContainerPathNotAbsolute(L"C:\\hostPath:ro", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } { auto result = RunWslc(std::format(L"container run --name {} --volume :ro {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE( - result.StderrContainsSubstring(L"Invalid volume specifications: ':ro'. Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + VERIFY_IS_TRUE(result.StderrContainsSubstring( + Localization::WSLCCLI_VolumeInvalidSpec(L":ro", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -422,8 +415,7 @@ class WSLCE2EContainerCreateTests std::format(L"container run --name {} --volume C:\\hostPath::rw {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: 'C:\\hostPath::rw'. Container path cannot be empty. Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + Localization::WSLCCLI_VolumeContainerPathEmpty(L"C:\\hostPath::rw", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -431,10 +423,8 @@ class WSLCE2EContainerCreateTests auto result = RunWslc(std::format( L"container run --name {} --volume C:\\hostPath:/containerPath:invalid_mode {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: 'C:\\hostPath:/containerPath:invalid_mode'. Container path must be an absolute " - L"path (starting with '/'). Expected format: :[:mode]\r\nError code: " - L"E_INVALIDARG")); + VERIFY_IS_TRUE(result.StderrContainsSubstring(Localization::WSLCCLI_VolumeContainerPathNotAbsolute( + L"C:\\hostPath:/containerPath:invalid_mode", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -442,10 +432,8 @@ class WSLCE2EContainerCreateTests auto result = RunWslc(std::format( L"container run --name {} --volume C:\\hostPath:/containerPath:ro:extra {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: 'C:\\hostPath:/containerPath:ro:extra'. Container path must be an absolute path " - L"(starting with '/'). Expected format: :[:mode]\r\nError code: " - L"E_INVALIDARG")); + VERIFY_IS_TRUE(result.StderrContainsSubstring(Localization::WSLCCLI_VolumeContainerPathNotAbsolute( + L"C:\\hostPath:/containerPath:ro:extra", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -453,9 +441,8 @@ class WSLCE2EContainerCreateTests auto result = RunWslc(std::format( L"container run --name {} --volume C:\\hostPath:/containerPath: {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: 'C:\\hostPath:/containerPath:'. Container path cannot be empty. Expected " - L"format: :[:mode]\r\nError code: E_INVALIDARG")); + VERIFY_IS_TRUE(result.StderrContainsSubstring(Localization::WSLCCLI_VolumeContainerPathEmpty( + L"C:\\hostPath:/containerPath:", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -464,9 +451,7 @@ class WSLCE2EContainerCreateTests auto result = RunWslc( std::format(L"container run --name {} --volume \"::/container:ro\" {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE( - result.StderrContainsSubstring(L"Invalid volume specifications: '::/container:ro'. Host path ':' is not a valid " - L"Windows path.\r\nError code: E_INVALIDARG")); + VERIFY_IS_TRUE(result.StderrContainsSubstring(Localization::WSLCCLI_VolumeHostPathInvalid(L"::/container:ro", L":"))); EnsureContainerDoesNotExist(WslcContainerName); } } @@ -481,8 +466,7 @@ class WSLCE2EContainerCreateTests std::format(L"container run --name {} --volume \"C:\\hostPath\" {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: 'C:\\hostPath'. Container path must be an absolute path (starting with '/'). " - L"Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + Localization::WSLCCLI_VolumeContainerPathNotAbsolute(L"C:\\hostPath", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -490,8 +474,7 @@ class WSLCE2EContainerCreateTests auto result = RunWslc(std::format(L"container run --name {} --volume \":\" {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: ':'. Container path cannot be empty. Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + Localization::WSLCCLI_VolumeContainerPathEmpty(L":", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -501,8 +484,7 @@ class WSLCE2EContainerCreateTests RunWslc(std::format(L"container run --name {} --volume \"::\" {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"Invalid volume specifications: '::'. Container path cannot be empty. Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + Localization::WSLCCLI_VolumeContainerPathEmpty(L"::", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } @@ -510,9 +492,8 @@ class WSLCE2EContainerCreateTests auto result = RunWslc(std::format(L"container run --name {} --volume \"e2e_test\" {}", WslcContainerName, AlpineImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE( - result.StderrContainsSubstring(L"Invalid volume specifications: 'e2e_test'. Expected format: :[:mode]\r\nError code: E_INVALIDARG")); + VERIFY_IS_TRUE(result.StderrContainsSubstring( + Localization::WSLCCLI_VolumeInvalidSpec(L"e2e_test", Localization::WSLCCLI_VolumeFormatUsage()))); EnsureContainerDoesNotExist(WslcContainerName); } } @@ -932,12 +913,20 @@ class WSLCE2EContainerCreateTests WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_DuplicateDestination_Fails) { - auto result = RunWslc(std::format( - L"container create --name {} --mount type=tmpfs,target=/data --mount type=tmpfs,target=/data/ {} true", - WslcContainerName, - DebianImage.NameAndTag())); - result.Verify({.Stdout = L"", .Stderr = FormatWslcError(Localization::WSLCCLI_DuplicateMountDestinationError(L"/data")), .ExitCode = 1}); - EnsureContainerDoesNotExist(WslcContainerName); + constexpr std::wstring_view duplicateMountArguments[] = { + L"--mount type=tmpfs,target=/data --mount type=tmpfs,target=/data/", + L"--tmpfs /data --volume data-volume:/data/", + L"--tmpfs /data --mount type=volume,source=data-volume,target=/data/", + L"--volume data-volume:/data --mount type=tmpfs,target=/data/", + }; + + for (const auto arguments : duplicateMountArguments) + { + const auto result = + RunWslc(std::format(L"container create --name {} {} {} true", WslcContainerName, arguments, DebianImage.NameAndTag())); + result.Verify({.Stdout = L"", .Stderr = FormatWslcError(Localization::WSLCCLI_DuplicateMountDestinationError(L"/data")), .ExitCode = 1}); + EnsureContainerDoesNotExist(WslcContainerName); + } } WSLC_TEST_METHOD(WSLCE2E_Container_Create_WorkDir) From 15b39a7887bcde77bbb7ede1046f2698b051deaa Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 17 Aug 2026 15:59:07 -0700 Subject: [PATCH 12/12] Merge tmpfs into unified mount spec, update tests accordingly --- src/windows/common/MountSpecParsing.cpp | 20 +++++++- src/windows/common/MountSpecParsing.h | 2 + src/windows/common/WSLCContainerLauncher.cpp | 23 ++++----- src/windows/common/WSLCContainerLauncher.h | 4 +- src/windows/service/inc/wslc.idl | 4 +- .../wslc/arguments/ArgumentDefinitions.h | 2 +- .../wslc/arguments/ArgumentValidation.cpp | 15 ++++++ src/windows/wslc/services/ContainerModel.cpp | 34 ------------- src/windows/wslc/services/ContainerModel.h | 18 ------- .../wslc/services/ContainerService.cpp | 6 --- src/windows/wslc/tasks/ContainerTasks.cpp | 6 +-- src/windows/wslcsession/WSLCContainer.cpp | 20 ++++++++ .../wslc/WSLCCLIMountParserUnitTests.cpp | 5 +- .../wslc/WSLCCLITmpfsParserUnitTests.cpp | 51 ++++++++++++------- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 6 +-- 15 files changed, 109 insertions(+), 107 deletions(-) diff --git a/src/windows/common/MountSpecParsing.cpp b/src/windows/common/MountSpecParsing.cpp index 83ea521a0c..1a10798299 100644 --- a/src/windows/common/MountSpecParsing.cpp +++ b/src/windows/common/MountSpecParsing.cpp @@ -522,6 +522,19 @@ Spec ParseDockerVolumeString(const std::wstring& value) }; } +Spec ParseDockerTmpfsString(const std::wstring& value) +{ + const auto colon = value.find(L':'); + const auto target = value.substr(0, colon); + const auto options = colon == std::wstring::npos ? std::wstring_view{} : std::wstring_view{value}.substr(colon + 1); + + return { + .MountType = Type::Tmpfs, + .Target = WideToMultiByte(target), + .TmpfsOptions = WideToMultiByte(std::wstring{options}), + }; +} + void ValidateMountSpec(const Spec& mount) { if (mount.Target.empty()) @@ -534,7 +547,7 @@ void ValidateMountSpec(const Spec& mount) ThrowValidation(Localization::WSLCCLI_MountTargetAbsoluteError()); } - if (mount.MountType != Type::Tmpfs && (mount.TmpfsSizeBytes.has_value() || mount.TmpfsMode.has_value())) + if (mount.MountType != Type::Tmpfs && (mount.TmpfsSizeBytes.has_value() || mount.TmpfsMode.has_value() || mount.TmpfsOptions.has_value())) { ThrowValidation(Localization::WSLCCLI_MountTmpfsOptionsTypeError()); } @@ -599,6 +612,11 @@ std::string FormatTmpfsOptions(const Spec& mount) { WI_ASSERT(mount.MountType == Type::Tmpfs); + if (mount.TmpfsOptions.has_value()) + { + return mount.TmpfsOptions.value(); + } + std::vector options; if (mount.ReadOnly) { diff --git a/src/windows/common/MountSpecParsing.h b/src/windows/common/MountSpecParsing.h index e330bd1959..84f76aadac 100644 --- a/src/windows/common/MountSpecParsing.h +++ b/src/windows/common/MountSpecParsing.h @@ -48,6 +48,7 @@ struct Spec BindSourcePolicy BindSource = BindSourcePolicy::RequireExisting; std::optional TmpfsSizeBytes; std::optional TmpfsMode; + std::optional TmpfsOptions; }; enum class ValidationError @@ -114,6 +115,7 @@ class MountValidationException : public MountException Spec ParseDockerMountString(const std::wstring& value); Spec ParseDockerVolumeString(const std::wstring& value); +Spec ParseDockerTmpfsString(const std::wstring& value); void ValidateMountSpec(const Spec& mount); void ValidateMountCollection(std::span mounts); std::string FormatTmpfsOptions(const Spec& mount); diff --git a/src/windows/common/WSLCContainerLauncher.cpp b/src/windows/common/WSLCContainerLauncher.cpp index a967893e9d..59e488d680 100644 --- a/src/windows/common/WSLCContainerLauncher.cpp +++ b/src/windows/common/WSLCContainerLauncher.cpp @@ -290,6 +290,12 @@ void wsl::windows::common::WSLCContainerLauncher::AddMount(const mount::Spec& Mo mount.TmpfsMode = Mount.TmpfsMode.value(); } + if (Mount.TmpfsOptions.has_value()) + { + WI_SetFlag(mount.Flags, WSLCMountSpecFlagsTmpfsOptions); + mount.TmpfsOptions = m_mountTmpfsOptions.emplace_back(Mount.TmpfsOptions.value()).c_str(); + } + m_mounts.push_back(mount); } @@ -308,15 +314,11 @@ void wsl::windows::common::WSLCContainerLauncher::AddLabel(const std::string& Ke void wsl::windows::common::WSLCContainerLauncher::AddTmpfs(const std::string& ContainerPath, const std::string& Options) { - // Store a copy of the path/options strings to the launcher to ensure the pointers in WSLCTmpfsMount remain valid. - const auto& containerPath = m_tmpfsContainerPaths.emplace_back(ContainerPath); - const auto& options = m_tmpfsOptions.emplace_back(Options); - - WSLCTmpfsMount tmpfs{}; - tmpfs.Destination = containerPath.c_str(); - tmpfs.Options = options.c_str(); - - m_tmpfsMounts.push_back(tmpfs); + AddMount({ + .MountType = mount::Type::Tmpfs, + .Target = ContainerPath, + .TmpfsOptions = Options, + }); } void wsl::windows::common::WSLCContainerLauncher::AddAdditionalNetwork(const std::string& Name) @@ -457,9 +459,6 @@ std::pair> WSLCContainerLauncher::C options.LabelsCount = static_cast(m_labels.size()); options.Labels = m_labels.size() > 0 ? m_labels.data() : nullptr; - options.TmpfsCount = static_cast(m_tmpfsMounts.size()); - options.Tmpfs = m_tmpfsMounts.size() > 0 ? m_tmpfsMounts.data() : nullptr; - options.ContainerNetwork.NetworkMode = m_networkMode.c_str(); // Each additional network becomes an entry in NetworkingConfig.EndpointsConfig. diff --git a/src/windows/common/WSLCContainerLauncher.h b/src/windows/common/WSLCContainerLauncher.h index 39cf63dfe1..52e6e29977 100644 --- a/src/windows/common/WSLCContainerLauncher.h +++ b/src/windows/common/WSLCContainerLauncher.h @@ -114,6 +114,7 @@ class WSLCContainerLauncher : private WSLCProcessLauncher std::vector m_mounts; std::deque m_mountSources; std::deque m_mountTargets; + std::deque m_mountTmpfsOptions; std::string m_networkMode; std::vector m_entrypoint; WSLCSignal m_stopSignal = WSLCSignalNone; @@ -135,9 +136,6 @@ class WSLCContainerLauncher : private WSLCProcessLauncher std::vector m_labels; std::deque m_labelKeys; std::deque m_labelValues; - std::vector m_tmpfsMounts; - std::deque m_tmpfsContainerPaths; - std::deque m_tmpfsOptions; std::int64_t m_memoryBytes = 0; std::int64_t m_nanoCpus = 0; std::vector m_ulimits; diff --git a/src/windows/service/inc/wslc.idl b/src/windows/service/inc/wslc.idl index f24e13d459..e6d1b2be14 100644 --- a/src/windows/service/inc/wslc.idl +++ b/src/windows/service/inc/wslc.idl @@ -258,9 +258,10 @@ typedef enum _WSLCMountSpecFlags WSLCMountSpecFlagsTmpfsSize = 1, WSLCMountSpecFlagsTmpfsMode = 2, WSLCMountSpecFlagsCreateSourceIfMissing = 4, + WSLCMountSpecFlagsTmpfsOptions = 8, } WSLCMountSpecFlags; -cpp_quote("#define WSLCMountSpecFlagsValid (WSLCMountSpecFlagsTmpfsSize | WSLCMountSpecFlagsTmpfsMode | WSLCMountSpecFlagsCreateSourceIfMissing)") +cpp_quote("#define WSLCMountSpecFlagsValid (WSLCMountSpecFlagsTmpfsSize | WSLCMountSpecFlagsTmpfsMode | WSLCMountSpecFlagsCreateSourceIfMissing | WSLCMountSpecFlagsTmpfsOptions)") cpp_quote("DEFINE_ENUM_FLAG_OPERATORS(WSLCMountSpecFlags);") typedef struct _WSLCMountSpec @@ -272,6 +273,7 @@ typedef struct _WSLCMountSpec WSLCMountSpecFlags Flags; LONGLONG TmpfsSizeBytes; ULONG TmpfsMode; + [unique, string] LPCSTR TmpfsOptions; } WSLCMountSpec; typedef struct _WSLCUlimit diff --git a/src/windows/wslc/arguments/ArgumentDefinitions.h b/src/windows/wslc/arguments/ArgumentDefinitions.h index ac57ed8cdd..3a37150cb5 100644 --- a/src/windows/wslc/arguments/ArgumentDefinitions.h +++ b/src/windows/wslc/arguments/ArgumentDefinitions.h @@ -131,7 +131,7 @@ _(Tail, "tail", L"n", Kind::Value, _(Tag, "tag", L"t", Kind::Value, NoConversion, Localization::WSLCCLI_TagArgDescription()) \ _(Target, "target", NO_ALIAS, Kind::Positional, NoConversion, Localization::WSLCCLI_TargetArgDescription()) \ _(Time, "time", L"t", Kind::Value, LONG, Localization::WSLCCLI_TimeArgDescription()) \ -_(TMPFS, "tmpfs", NO_ALIAS, Kind::Value, NoConversion, Localization::WSLCCLI_TMPFSArgDescription()) \ +_(TMPFS, "tmpfs", NO_ALIAS, Kind::Value, ParsedMount, Localization::WSLCCLI_TMPFSArgDescription()) \ _(TTY, "tty", L"t", Kind::Flag, NoConversion, Localization::WSLCCLI_TTYArgDescription()) \ _(Type, "type", L"t", Kind::Value, InspectType, Localization::WSLCCLI_TypeArgDescription()) \ _(Ulimit, "ulimit", NO_ALIAS, Kind::Value, UlimitValue, Localization::WSLCCLI_UlimitArgDescription()) \ diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp index ddd57f8b9e..c919844ee6 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.cpp +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -245,6 +245,21 @@ void Argument::Validate(ArgMap& execArgs) const }); break; + case ArgType::TMPFS: + CacheConverted(execArgs, m_name, [](const std::wstring& value, const std::wstring&) { + try + { + auto mountSpec = mount::ParseDockerTmpfsString(value); + mount::ValidateMountSpec(mountSpec); + return mountSpec; + } + catch (const mount::MountException& ex) + { + throw ArgumentException(ex.Reason()); + } + }); + break; + case ArgType::Mount: CacheConverted(execArgs, m_name, [](const std::wstring& value, const std::wstring&) { try diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp index 88357a9847..d1b3018b98 100644 --- a/src/windows/wslc/services/ContainerModel.cpp +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -258,21 +258,6 @@ std::vector EnvironmentVariable::ParseFile(const std::wstring& fil return envVars; } -TmpfsMount TmpfsMount::Parse(const std::string& value) -{ - TmpfsMount result{}; - auto colonPos = value.find(':'); - if (colonPos == std::string::npos) - { - result.m_containerPath = value; - return result; - } - - result.m_containerPath = value.substr(0, colonPos); - result.m_options = value.substr(colonPos + 1); - return result; -} - void ValidateUniqueMountDestinations(const ContainerOptions& options) { try @@ -288,25 +273,6 @@ void ValidateUniqueMountDestinations(const ContainerOptions& options) throw; } - - std::unordered_set destinations; - const auto addDestination = [&](const std::string& destination) { - const auto normalizedDestination = mount::NormalizeDestination(destination); - THROW_HR_WITH_USER_ERROR_IF( - E_INVALIDARG, - Localization::WSLCCLI_DuplicateMountDestinationError(MultiByteToWide(normalizedDestination)), - !destinations.emplace(normalizedDestination).second); - }; - - for (const auto& tmpfsSpec : options.Tmpfs) - { - addDestination(TmpfsMount::Parse(tmpfsSpec).ContainerPath()); - } - - for (const auto& mount : options.Mounts) - { - addDestination(mount.Target); - } } CidFile::CidFile(const std::optional& path) diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index 14b332dcfa..964ea2f192 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -87,7 +87,6 @@ struct ContainerOptions std::vector DnsOptions; std::vector Networks; std::vector NetworkAliases; - std::vector Tmpfs; std::vector> Labels; std::optional CidFile{}; std::optional MemoryBytes{}; @@ -310,23 +309,6 @@ struct VolumeMount bool m_isNamedVolume = false; }; -struct TmpfsMount -{ - std::string ContainerPath() const - { - return m_containerPath; - } - std::string Options() const - { - return m_options; - } - static TmpfsMount Parse(const std::string& value); - -private: - std::string m_containerPath; - std::string m_options; -}; - void ValidateUniqueMountDestinations(const ContainerOptions& options); class CidFile diff --git a/src/windows/wslc/services/ContainerService.cpp b/src/windows/wslc/services/ContainerService.cpp index 7d7ca445d8..bcdc969c9c 100644 --- a/src/windows/wslc/services/ContainerService.cpp +++ b/src/windows/wslc/services/ContainerService.cpp @@ -259,12 +259,6 @@ static wsl::windows::common::RunningWSLCContainer CreateInternal(Terminal& termi containerLauncher.SetDnsOptions(std::vector(options.DnsOptions)); } - for (const auto& tmpfsSpec : options.Tmpfs) - { - auto tmpfsMount = TmpfsMount::Parse(tmpfsSpec); - containerLauncher.AddTmpfs(tmpfsMount.ContainerPath(), tmpfsMount.Options()); - } - for (const auto& [key, value] : options.Labels) { containerLauncher.AddLabel(key, value); diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index af6ef07e76..cb5b1cde58 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -830,11 +830,7 @@ void SetContainerOptionsFromArgs(CLIExecutionContext& context) if (context.Args.Contains(ArgType::TMPFS)) { auto tmpfs = context.Args.GetAllValues(); - options.Tmpfs.reserve(options.Tmpfs.size() + tmpfs.size()); - for (const auto& value : tmpfs) - { - options.Tmpfs.emplace_back(WideToMultiByte(value)); - } + options.Mounts.insert(options.Mounts.end(), std::make_move_iterator(tmpfs.begin()), std::make_move_iterator(tmpfs.end())); } ValidateUniqueMountDestinations(options); diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 63d30b0759..5bc4ce583f 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -600,6 +600,17 @@ std::vector ConvertAndValidateMounts(const WS type != mount::Type::Bind && WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsCreateSourceIfMissing), "Mount at index %lu specifies create-source-if-missing for a non-bind mount", i); + THROW_HR_IF_MSG( + E_INVALIDARG, + type != mount::Type::Tmpfs && WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsTmpfsOptions), + "Mount at index %lu specifies tmpfs options for a non-tmpfs mount", + i); + THROW_HR_IF_MSG( + E_INVALIDARG, + WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsTmpfsOptions) && + WI_IsAnyFlagSet(value.Flags, WSLCMountSpecFlagsTmpfsSize | WSLCMountSpecFlagsTmpfsMode), + "Mount at index %lu combines legacy and structured tmpfs options", + i); mounts.push_back({ .MountType = type, @@ -610,6 +621,9 @@ std::vector ConvertAndValidateMounts(const WS : mount::BindSourcePolicy::RequireExisting, .TmpfsSizeBytes = WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsTmpfsSize) ? std::optional{value.TmpfsSizeBytes} : std::nullopt, .TmpfsMode = WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsTmpfsMode) ? std::optional{value.TmpfsMode} : std::nullopt, + .TmpfsOptions = WI_IsFlagSet(value.Flags, WSLCMountSpecFlagsTmpfsOptions) + ? std::optional{value.TmpfsOptions != nullptr ? value.TmpfsOptions : ""} + : std::nullopt, }); } @@ -2170,6 +2184,12 @@ std::shared_ptr WSLCContainerImpl::Create( break; case wsl::windows::common::mount::Type::Tmpfs: + if (mount.TmpfsOptions.has_value()) + { + request.HostConfig.Tmpfs[mount.Target] = mount.TmpfsOptions.value(); + continue; + } + dockerMount.Type = "tmpfs"; if (mount.TmpfsSizeBytes.has_value() || mount.TmpfsMode.has_value()) { diff --git a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp index 6847d0e09a..d9667a9158 100644 --- a/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIMountParserUnitTests.cpp @@ -520,13 +520,12 @@ class WSLCCLIMountParserUnitTests TEST_METHOD(Mount_DuplicateDestinationsAreRejected) { ContainerOptions options; - options.Tmpfs = {"/data"}; options.Mounts = { + mount::ParseDockerTmpfsString(L"/data"), {.MountType = mount::Type::Volume, .Source = L"data-volume", .Target = "/data/"}, }; VERIFY_THROWS(ValidateUniqueMountDestinations(options), wil::ResultException); - options.Tmpfs.clear(); options.Mounts = { {.MountType = mount::Type::Tmpfs, .Target = "/data/../cache"}, {.MountType = mount::Type::Volume, .Source = L"data-volume", .Target = "/cache"}, @@ -537,8 +536,8 @@ class WSLCCLIMountParserUnitTests TEST_METHOD(Mount_UniqueDestinationsAreAccepted) { ContainerOptions options; - options.Tmpfs = {"/cache"}; options.Mounts = { + mount::ParseDockerTmpfsString(L"/cache"), {.MountType = mount::Type::Volume, .Source = L"data-volume", .Target = "/data"}, {.MountType = mount::Type::Bind, .Source = L"C:\\logs", .Target = "/logs"}, }; diff --git a/test/windows/wslc/WSLCCLITmpfsParserUnitTests.cpp b/test/windows/wslc/WSLCCLITmpfsParserUnitTests.cpp index bf96f83740..ddf26b410a 100644 --- a/test/windows/wslc/WSLCCLITmpfsParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLITmpfsParserUnitTests.cpp @@ -13,10 +13,9 @@ Module Name: #include "precomp.h" #include "windows/Common.h" -#include "WSLCCLITestHelpers.h" -#include "ContainerModel.h" +#include "MountSpecParsing.h" -using namespace wsl::windows::wslc; +using namespace wsl::windows::common; namespace WSLCCLITmpfsParserUnitTests { @@ -26,26 +25,40 @@ class WSLCCLITmpfsParserUnitTests TEST_METHOD(WSLCCLITmpfsMount_Parse) { - std::vector> validTmpfsSpecs = { - {"", "", ""}, - {"/tmp", "/tmp", ""}, - {"/tmp:size=50m", "/tmp", "size=50m"}, - {"/var/tmp:size=1g", "/var/tmp", "size=1g"}, - {"/tmp:size=50m,mode=1777", "/tmp", "size=50m,mode=1777"}, - {"/cache:uid=1000,gid=1000", "/cache", "uid=1000,gid=1000"}, - {"/mnt/ramdisk:size=256k,nr_inodes=1k", "/mnt/ramdisk", "size=256k,nr_inodes=1k"}, - {"/securetmp:mode=0700", "/securetmp", "mode=0700"}, - {"/scratch:nosuid,nodev,noexec", "/scratch", "nosuid,nodev,noexec"}, - {"/wsl/tmp:size=2g,uid=0,gid=0,mode=1777", "/wsl/tmp", "size=2g,uid=0,gid=0,mode=1777"}, + const std::vector> validTmpfsSpecs = { + {L"", "", ""}, + {L"/tmp", "/tmp", ""}, + {L"/tmp:size=50m", "/tmp", "size=50m"}, + {L"/var/tmp:size=1g", "/var/tmp", "size=1g"}, + {L"/tmp:size=50m,mode=1777", "/tmp", "size=50m,mode=1777"}, + {L"/cache:uid=1000,gid=1000", "/cache", "uid=1000,gid=1000"}, + {L"/mnt/ramdisk:size=256k,nr_inodes=1k", "/mnt/ramdisk", "size=256k,nr_inodes=1k"}, + {L"/securetmp:mode=0700", "/securetmp", "mode=0700"}, + {L"/scratch:nosuid,nodev,noexec", "/scratch", "nosuid,nodev,noexec"}, + {L"/wsl/tmp:size=2g,uid=0,gid=0,mode=1777", "/wsl/tmp", "size=2g,uid=0,gid=0,mode=1777"}, }; - for (const auto& [input, expectedContainerPath, expectedOptions] : validTmpfsSpecs) + for (const auto& [input, expectedTarget, expectedOptions] : validTmpfsSpecs) { - auto result = models::TmpfsMount::Parse(input); - VERIFY_ARE_EQUAL(expectedContainerPath, result.ContainerPath()); - VERIFY_ARE_EQUAL(expectedOptions, result.Options()); + const auto result = mount::ParseDockerTmpfsString(input); + VERIFY_ARE_EQUAL(static_cast(mount::Type::Tmpfs), static_cast(result.MountType)); + VERIFY_ARE_EQUAL(expectedTarget, result.Target); + VERIFY_IS_TRUE(result.TmpfsOptions.has_value()); + VERIFY_ARE_EQUAL(expectedOptions, result.TmpfsOptions.value()); } } + + TEST_METHOD(WSLCCLITmpfsMount_Validate) + { + auto valid = mount::ParseDockerTmpfsString(L"/tmp:size=50m"); + VERIFY_NO_THROW(mount::ValidateMountSpec(valid)); + + auto empty = mount::ParseDockerTmpfsString(L":size=50m"); + VERIFY_THROWS(mount::ValidateMountSpec(empty), mount::MountValidationException); + + auto relative = mount::ParseDockerTmpfsString(L"tmp:size=50m"); + VERIFY_THROWS(mount::ValidateMountSpec(relative), mount::MountValidationException); + } }; -} // namespace WSLCCLITmpfsParserUnitTests \ No newline at end of file +} // namespace WSLCCLITmpfsParserUnitTests diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index 5ada146960..90a4a743d2 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -681,8 +681,7 @@ class WSLCE2EContainerCreateTests auto result = RunWslc(std::format(L"container create --name {} --tmpfs wslc-tmpfs {}", WslcContainerName, DebianImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE(result.StderrContainsSubstring( - L"invalid mount path: 'wslc-tmpfs' mount path must be absolute\r\nError code: E_FAIL")); + VERIFY_IS_TRUE(result.StderrContainsSubstring(Localization::WSLCCLI_MountTargetAbsoluteError())); } WSLC_TEST_METHOD(WSLCE2E_Container_Create_Tmpfs_EmptyDestination_Fails) @@ -690,8 +689,7 @@ class WSLCE2EContainerCreateTests auto result = RunWslc(std::format(L"container create --name {} --tmpfs :size=64k {}", WslcContainerName, DebianImage.NameAndTag())); result.Verify({.Stdout = L"", .ExitCode = 1}); - VERIFY_IS_TRUE( - result.StderrContainsSubstring(L"invalid mount path: '' mount path must be absolute\r\nError code: E_FAIL")); + VERIFY_IS_TRUE(result.StderrContainsSubstring(Localization::WSLCCLI_MountTargetRequiredError())); } WSLC_TEST_METHOD(WSLCE2E_Container_Create_Mount_Tmpfs_Success)