Various improvements to PullImage() + custom registry support

src/windows/common/wslutil.cpp

@@ -1073,6 +1073,44 @@ std::vector<DWORD> wsl::windows::common::wslutil::ListRunningProcesses()
     return pids;
 }
 
+std::pair<std::string, std::string> 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<wil::unique_hfile, wil::unique_hfile> wsl::windows::common::wslutil::OpenAnonymousPipe(DWORD Size, bool ReadPipeOverlapped, bool WritePipeOverlapped)
 {
     // Default to 4096 byte buffer, just like CreatePipe().

src/windows/common/wslutil.h

@@ -214,6 +214,8 @@ bool IsVirtualMachinePlatformInstalled();
 
 std::vector<DWORD> ListRunningProcesses();
 
+std::pair<std::string, std::string> NormalizeRepo(const std::string& Input);
+
 std::pair<wil::unique_hfile, wil::unique_hfile> OpenAnonymousPipe(DWORD Size, bool ReadPipeOverlapped, bool WritePipeOverlapped);
 
 wil::unique_handle OpenCallingProcess(_In_ DWORD access);

src/windows/inc/docker_schema.h

@@ -444,10 +444,11 @@ struct CreateImageProgress
 {
     std::string status;
     std::string id;
+    std::optional<ErrorResponse> 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

src/windows/wslcsession/DockerHTTPClient.cpp

@@ -122,8 +122,9 @@ std::unique_ptr<DockerHTTPClient::HTTPRequestContext> 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())
     {

src/windows/wslcsession/WSLCSession.cpp

@@ -328,17 +328,25 @@ try
 
     auto io = CreateIOContext();
 
-    std::optional<boost::beast::http::status> pullResult;
+    struct Response
+    {
+        boost::beast::http::status result;
+        bool isJson = false;
+    };
+
+    std::optional<Response> pullResponse;
 
     auto onHttpResponse = [&](const boost::beast::http::message<false, boost::beast::http::buffer_body>& response) {
         WSL_LOG("PullHttpResponse", TraceLoggingValue(static_cast<int>(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<std::string> reportedError;
     auto onChunk = [&](const gsl::span<char>& 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<docker_schema::CreateImageProgress>(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<docker_schema::CreateImageProgress>(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<int>(pullResult.value()) >= 400 && static_cast<int>(pullResult.value()) < 500)
+        if (pullResponse->isJson)
         {
             // pull failed, parse the error message.
             errorMessage = wsl::shared::FromJson<docker_schema::ErrorResponse>(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;
 }

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<WSLCImageInformation> images;
                 VERIFY_SUCCEEDED(m_defaultSession->ListImages(nullptr, images.addressof(), images.size_address<ULONG>()));
 
@@ -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,11 +6006,37 @@ 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);
         VERIFY_ARE_EQUAL(wil::ResultFromException([]() { ParseImage(":"); }), E_INVALIDARG);
         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");
+    }
 };

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});
     }