diff --git a/src/windows/common/wslutil.cpp b/src/windows/common/wslutil.cpp index 3b1a1c542..5326aa27f 100644 --- a/src/windows/common/wslutil.cpp +++ b/src/windows/common/wslutil.cpp @@ -1073,6 +1073,44 @@ std::vector wsl::windows::common::wslutil::ListRunningProcesses() return pids; } +std::pair wsl::windows::common::wslutil::NormalizeRepo(const std::string& Input) +{ + // See: https://github.com/distribution/reference/blob/ff14fafe2236e51c2894ac07d4bdfc778e96d682/normalize.go#L126 + + constexpr auto defaultDomain = "docker.io"; + constexpr auto officialPrefix = "library/"; + constexpr auto legacyDomain = "index.docker.io"; + constexpr auto localhost = "localhost"; + + auto slash = Input.find('/'); + if (slash == std::string::npos) + { + return {defaultDomain, officialPrefix + Input}; + } + + auto domain = Input.substr(0, slash); + auto path = Input.substr(slash + 1); + + if (domain == legacyDomain) + { + domain = defaultDomain; + } + else if (domain != localhost && domain.find_first_of(".:") == std::string::npos && !std::ranges::any_of(domain, [](unsigned char e) { + return std::isupper(e); + })) + { + domain = defaultDomain; + path = Input; + } + + if (domain == defaultDomain && path.find('/') == std::string::npos) + { + path = "library/" + path; + } + + return {domain, path}; +} + std::pair wsl::windows::common::wslutil::OpenAnonymousPipe(DWORD Size, bool ReadPipeOverlapped, bool WritePipeOverlapped) { // Default to 4096 byte buffer, just like CreatePipe(). diff --git a/src/windows/common/wslutil.h b/src/windows/common/wslutil.h index 2b8b9712a..b1f74228d 100644 --- a/src/windows/common/wslutil.h +++ b/src/windows/common/wslutil.h @@ -214,6 +214,8 @@ bool IsVirtualMachinePlatformInstalled(); std::vector ListRunningProcesses(); +std::pair NormalizeRepo(const std::string& Input); + std::pair OpenAnonymousPipe(DWORD Size, bool ReadPipeOverlapped, bool WritePipeOverlapped); wil::unique_handle OpenCallingProcess(_In_ DWORD access); diff --git a/src/windows/inc/docker_schema.h b/src/windows/inc/docker_schema.h index f59464a6c..e27a5b711 100644 --- a/src/windows/inc/docker_schema.h +++ b/src/windows/inc/docker_schema.h @@ -444,10 +444,11 @@ struct CreateImageProgress { std::string status; std::string id; + std::optional errorDetail; CreateImageProgressDetails progressDetail; - NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(CreateImageProgress, status, id, progressDetail); + NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(CreateImageProgress, status, id, progressDetail, errorDetail); }; } // namespace wsl::windows::common::docker_schema \ No newline at end of file diff --git a/src/windows/wslcsession/DockerHTTPClient.cpp b/src/windows/wslcsession/DockerHTTPClient.cpp index 8332d0bd7..a77a02a8b 100644 --- a/src/windows/wslcsession/DockerHTTPClient.cpp +++ b/src/windows/wslcsession/DockerHTTPClient.cpp @@ -122,8 +122,9 @@ std::unique_ptr DockerHTTPClient::PullImag { auto url = URL::Create("/images/create"); - // TODO: Support pulling from other registries. - url.SetParameter("fromImage", std::format("library/{}", Repo)); + // Normalize the repo server & path + auto [server, path] = wslutil::NormalizeRepo(Repo); + url.SetParameter("fromImage", std::format("{}/{}", server, path)); if (tagOrDigest.has_value()) { diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index 66b08936f..7a9762fef 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -328,17 +328,25 @@ try auto io = CreateIOContext(); - std::optional pullResult; + struct Response + { + boost::beast::http::status result; + bool isJson = false; + }; + + std::optional pullResponse; auto onHttpResponse = [&](const boost::beast::http::message& response) { WSL_LOG("PullHttpResponse", TraceLoggingValue(static_cast(response.result()), "StatusCode")); - pullResult = response.result(); + auto it = response.find(boost::beast::http::field::content_type); + pullResponse.emplace(response.result(), it != response.end() && it->value().starts_with("application/json")); }; std::string errorJson; + std::optional reportedError; auto onChunk = [&](const gsl::span& Content) { - if (pullResult.has_value() && pullResult.value() != boost::beast::http::status::ok) + if (pullResponse.has_value() && pullResponse->result != boost::beast::http::status::ok) { // If the status code is an error, then this is an error message, not a progress update. errorJson.append(Content.data(), Content.size()); @@ -348,15 +356,28 @@ try std::string contentString{Content.begin(), Content.end()}; WSL_LOG("ImagePullProgress", TraceLoggingValue(Image, "Image"), TraceLoggingValue(contentString.c_str(), "Content")); - if (ProgressCallback == nullptr) + auto parsed = wsl::shared::FromJson(contentString.c_str()); + + if (parsed.errorDetail.has_value()) { + if (reportedError.has_value()) + { + LOG_HR_MSG( + E_UNEXPECTED, + "Received multiple error messages during image pull. Previous: %hs, New: %hs", + reportedError->c_str(), + parsed.errorDetail->message.c_str()); + } + + reportedError = parsed.errorDetail->message; return; } - auto parsed = wsl::shared::FromJson(contentString.c_str()); - - THROW_IF_FAILED(ProgressCallback->OnProgress( - parsed.status.c_str(), parsed.id.c_str(), parsed.progressDetail.current, parsed.progressDetail.total)); + if (ProgressCallback != nullptr) + { + THROW_IF_FAILED(ProgressCallback->OnProgress( + parsed.status.c_str(), parsed.id.c_str(), parsed.progressDetail.current, parsed.progressDetail.total)); + } }; auto onCompleted = [&]() { io.Cancel(); }; @@ -366,22 +387,27 @@ try io.Run({}); - THROW_HR_IF(E_UNEXPECTED, !pullResult.has_value()); + THROW_HR_IF(E_UNEXPECTED, !pullResponse.has_value()); - if (pullResult.value() != boost::beast::http::status::ok) + if (pullResponse->result != boost::beast::http::status::ok) { std::string errorMessage; - if (static_cast(pullResult.value()) >= 400 && static_cast(pullResult.value()) < 500) + if (pullResponse->isJson) { // pull failed, parse the error message. errorMessage = wsl::shared::FromJson(errorJson.c_str()).message; } + else + { + // If no error message was explicitly returned, use the response body, if any. + errorMessage = errorJson; + } - if (pullResult.value() == boost::beast::http::status::not_found) + if (pullResponse->result == boost::beast::http::status::not_found) { THROW_HR_WITH_USER_ERROR(WSLC_E_IMAGE_NOT_FOUND, errorMessage); } - else if (pullResult.value() == boost::beast::http::status::bad_request) + else if (pullResponse->result == boost::beast::http::status::bad_request) { THROW_HR_WITH_USER_ERROR(E_INVALIDARG, errorMessage); } @@ -390,6 +416,11 @@ try THROW_HR_WITH_USER_ERROR(E_FAIL, errorMessage); } } + else if (reportedError.has_value()) + { + // Can happen if an error is returned during progress after receiving an OK status. + THROW_HR_WITH_USER_ERROR(E_FAIL, reportedError.value().c_str()); + } return S_OK; } diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index 6df741b8a..83a3e09a7 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -120,7 +120,7 @@ class WSLCTests settings.MemoryMb = 2048; settings.BootTimeoutMs = 30 * 1000; settings.StoragePath = enableStorage ? m_storagePath.c_str() : nullptr; - settings.MaximumStorageSizeMb = 4096; // 4GB. + settings.MaximumStorageSizeMb = 1024 * 20; // 20GB. settings.NetworkingMode = networkingMode; return settings; @@ -452,7 +452,6 @@ class WSLCTests if (!ExpectedTag.has_value()) { - wil::unique_cotaskmem_array_ptr images; VERIFY_SUCCEEDED(m_defaultSession->ListImages(nullptr, images.addressof(), images.size_address())); @@ -483,11 +482,31 @@ class WSLCTests }; validatePull("ubuntu@sha256:2e863c44b718727c860746568e1d54afd13b2fa71b160f5cd9058fc436217b30", {}); - validatePull("ubuntu", "ubuntu:latest"); validatePull("debian:bookworm", "debian:bookworm"); + validatePull("pytorch/pytorch", "pytorch/pytorch:latest"); + validatePull("registry.k8s.io/pause:3.2", "registry.k8s.io/pause:3.2"); + + // Validate that PullImage() fails appropriately when the session runs out of space. + { + auto settings = GetDefaultSessionSettings(L"wslc-pull-image-out-of-space", false); + settings.NetworkingMode = WSLCNetworkingModeVirtioProxy; + settings.MemoryMb = 1024; + auto session = CreateSession(settings); - // TODO: Add test coverage with custom registries once supported. + VERIFY_ARE_EQUAL(session->PullImage("pytorch/pytorch", nullptr, nullptr), E_FAIL); + + auto comError = wsl::windows::common::wslutil::GetCOMErrorInfo(); + VERIFY_IS_TRUE(comError.has_value()); + + // The error message can't be compared directly because it contains an unpredicable path: + // "write /var/lib/docker/tmp/GetImageBlob1760660623: no space left on device" + if (StrStrW(comError->Message.get(), L"no space left on device") == nullptr) + { + LogError("Unexpected error message: %ls", comError->Message.get()); + VERIFY_FAIL(); + } + } } TEST_METHOD(ListImages) @@ -5987,6 +6006,7 @@ class WSLCTests ValidateImageParsing("pytorch/pytorch", "pytorch/pytorch", {}); // Invalid inputs + VERIFY_ARE_EQUAL(wil::ResultFromException([]() { ParseImage(""); }), E_INVALIDARG); VERIFY_ARE_EQUAL(wil::ResultFromException([]() { ParseImage(":debian:latest"); }), E_INVALIDARG); VERIFY_ARE_EQUAL(wil::ResultFromException([]() { ParseImage("debian:latest@"); }), E_INVALIDARG); VERIFY_ARE_EQUAL(wil::ResultFromException([]() { ParseImage(""); }), E_INVALIDARG); @@ -5994,4 +6014,29 @@ class WSLCTests VERIFY_ARE_EQUAL(wil::ResultFromException([]() { ParseImage("a:"); }), E_INVALIDARG); VERIFY_ARE_EQUAL(wil::ResultFromException([]() { ParseImage(":b"); }), E_INVALIDARG); } + + TEST_METHOD(RepoParsing) + { + using wsl::windows::common::wslutil::NormalizeRepo; + + auto ValidateRepoParsing = [](const std::string& input, const std::string& expectedServer, const std::string& expectedPath) { + auto [server, path] = NormalizeRepo(input); + VERIFY_ARE_EQUAL(server, expectedServer); + VERIFY_ARE_EQUAL(path, expectedPath); + }; + + ValidateRepoParsing("ubuntu", "docker.io", "library/ubuntu"); + ValidateRepoParsing("docker.io/ubuntu", "docker.io", "library/ubuntu"); + ValidateRepoParsing("index.docker.io/ubuntu", "docker.io", "library/ubuntu"); + ValidateRepoParsing("index.docker.io/library/ubuntu", "docker.io", "library/ubuntu"); + ValidateRepoParsing("docker.io/library/ubuntu", "docker.io", "library/ubuntu"); + ValidateRepoParsing("microsoft.com/ubuntu", "microsoft.com", "ubuntu"); + ValidateRepoParsing("microsoft.com:80/ubuntu", "microsoft.com:80", "ubuntu"); + ValidateRepoParsing("microsoft.com:80/ubuntu/foo/bar", "microsoft.com:80", "ubuntu/foo/bar"); + ValidateRepoParsing("127.0.0.1:80/ubuntu/foo/bar", "127.0.0.1:80", "ubuntu/foo/bar"); + ValidateRepoParsing("pytorch/pytorch", "docker.io", "pytorch/pytorch"); + ValidateRepoParsing("2001:0db8:85a3:0000:0000:8a2e:0370:7334/path", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", "path"); + ValidateRepoParsing( + "2001:0db8:85a3:0000:0000:8a2e:0370:7334:80/path", "2001:0db8:85a3:0000:0000:8a2e:0370:7334:80", "path"); + } }; diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index f4d36869c..ad0b96372 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -99,8 +99,8 @@ class WSLCE2EContainerCreateTests auto result = RunWslc(L"container create --name " + WslcContainerName + L" " + InvalidImage.NameAndTag()); std::wstringstream expectedError; expectedError << L"Image '" << InvalidImage.NameAndTag() << L"' not found, pulling\r\n" - << L"pull access denied for library/" - << InvalidImage.Name << L", repository does not exist or may require 'docker login': denied: requested access to the resource is denied\r\n" + << L"manifest for " << InvalidImage.NameAndTag() + << L" not found: manifest unknown: manifest tagged by \"latest\" is not found\r\n" << L"Error code: WSLC_E_IMAGE_NOT_FOUND\r\n"; result.Verify({.Stderr = expectedError.str(), .ExitCode = 1}); }