Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions localization/strings/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -2554,4 +2554,7 @@ On first run, creates the file with all settings commented out at their defaults
<data name="WSLCCLI_ImageLoadNoInputError" xml:space="preserve">
<value>Requested load but no input provided.</value>
</data>
<data name="WSLCCLI_ImageSaveStdoutIsTerminalError" xml:space="preserve">
<value>Cannot write image to terminal. Use the -o flag or redirect stdout.</value>
</data>
Comment thread
AmelBawa-msft marked this conversation as resolved.
</root>
2 changes: 1 addition & 1 deletion src/windows/wslc/commands/ImageSaveCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ std::vector<Argument> ImageSaveCommand::GetArguments() const
{
return {
Argument::Create(ArgType::ImageId, true),
Argument::Create(ArgType::Output, true),
Argument::Create(ArgType::Output),
Argument::Create(ArgType::Session),
};
}
Expand Down
9 changes: 7 additions & 2 deletions src/windows/wslc/services/ImageService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,14 @@ void ImageService::Save(wsl::windows::wslc::models::Session& session, const std:
CreateFileW(output.c_str(), GENERIC_WRITE, FILE_SHARE_READ, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)};
THROW_LAST_ERROR_IF(!outputFile);

Save(session, image, outputFile.get(), cancelEvent);
}

void ImageService::Save(wsl::windows::wslc::models::Session& session, const std::string& image, HANDLE outputHandle, HANDLE cancelEvent)
{
wsl::windows::common::HandleConsoleProgressBar progressBar(
outputFile.get(), L"Save in progress.", wsl::windows::common::HandleConsoleProgressBar::Format::FileSize);
THROW_IF_FAILED(session.Get()->SaveImage(ToCOMInputHandle(outputFile.get()), image.c_str(), nullptr, cancelEvent));
outputHandle, L"Save in progress.", wsl::windows::common::HandleConsoleProgressBar::Format::FileSize);
THROW_IF_FAILED(session.Get()->SaveImage(ToCOMInputHandle(outputHandle), image.c_str(), nullptr, cancelEvent));
}

void ImageService::Prune()
Expand Down
1 change: 1 addition & 0 deletions src/windows/wslc/services/ImageService.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class ImageService
static wsl::windows::common::wslc_schema::InspectImage Inspect(wsl::windows::wslc::models::Session& session, const std::string& image);
static void Pull(wsl::windows::wslc::models::Session& session, const std::string& image, IProgressCallback* callback);
static void Save(wsl::windows::wslc::models::Session& session, const std::string& image, const std::wstring& output, HANDLE cancelEvent = nullptr);
static void Save(wsl::windows::wslc::models::Session& session, const std::string& image, HANDLE outputHandle, HANDLE cancelEvent = nullptr);
static void Tag(wsl::windows::wslc::models::Session& session, const std::string& sourceImage, const std::string& targetImage);
void Push();
void Prune();
Expand Down
20 changes: 17 additions & 3 deletions src/windows/wslc/tasks/ImageTasks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Module Name:
#include "TableOutput.h"
#include "Task.h"
#include <format>
#include <wslutil.h>

using namespace wsl::shared;
using namespace wsl::windows::common::string;
Expand Down Expand Up @@ -190,11 +191,24 @@ void SaveImage(CLIExecutionContext& context)
{
WI_ASSERT(context.Data.Contains(Data::Session));
WI_ASSERT(context.Args.Contains(ArgType::ImageId));
WI_ASSERT(context.Args.Contains(ArgType::Output));
auto& session = context.Data.Get<Data::Session>();
auto& imageId = context.Args.Get<ArgType::ImageId>();
auto& output = context.Args.Get<ArgType::Output>();
services::ImageService::Save(session, WideToMultiByte(imageId), output, context.CreateCancelEvent());

if (context.Args.Contains(ArgType::Output))
{
auto& output = context.Args.Get<ArgType::Output>();
services::ImageService::Save(session, WideToMultiByte(imageId), output, context.CreateCancelEvent());
}
else
{
auto stdoutHandle = GetStdHandle(STD_OUTPUT_HANDLE);
if (wsl::windows::common::wslutil::IsConsoleHandle(stdoutHandle))
{
THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_ImageSaveStdoutIsTerminalError());
}

services::ImageService::Save(session, WideToMultiByte(imageId), stdoutHandle, context.CreateCancelEvent());
}
}

void TagImage(CLIExecutionContext& context)
Expand Down
42 changes: 36 additions & 6 deletions test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,6 @@ class WSLCE2EImageSaveTests
result.Verify({.Stdout = GetHelpMessage(), .Stderr = L"Required argument not provided: 'image'\r\n", .ExitCode = 1});
}

WSLC_TEST_METHOD(WSLCE2E_Image_Save_MissingOutputPath)
{
const auto result = RunWslc(std::format(L"image save {}", DebianImage.NameAndTag()));
result.Verify({.Stderr = L"Required argument not provided: 'output'\r\n", .ExitCode = 1});
}

WSLC_TEST_METHOD(WSLCE2E_Image_Save_ImageNotFound)
{
const auto result = RunWslc(std::format(L"image save --output \"{}\" {}", SavedArchivePath.wstring(), InvalidImage.NameAndTag()));
Expand Down Expand Up @@ -95,6 +89,42 @@ class WSLCE2EImageSaveTests
runResult.Verify({.Stdout = L"Hello from saved image!\n", .Stderr = L"", .ExitCode = 0});
}

WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToStdout_Success)
{
const auto result = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag()), SavedArchivePath);
result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});

VERIFY_IS_TRUE(std::filesystem::exists(SavedArchivePath));
auto sourceFileSize = std::filesystem::file_size(DebianImage.Path);
auto archiveFileSize = std::filesystem::file_size(SavedArchivePath);
VERIFY_ARE_EQUAL(sourceFileSize, archiveFileSize);
}

WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToTerminal_Fail)
{
const auto result = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag()));
result.Verify(
{.Stderr = L"Cannot write image to terminal. Use the -o flag or redirect stdout.\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1});
}
Comment thread
AmelBawa-msft marked this conversation as resolved.

WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToStdout_Load)
{
// Save source image
auto saveResult = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag()), SavedArchivePath);
saveResult.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});

// Delete source image
EnsureImageIsDeleted(DebianImage);

// Load from saved archive
auto loadResult = RunWslc(std::format(L"image load --input \"{}\"", SavedArchivePath.wstring()));
loadResult.Verify({.Stderr = L"", .ExitCode = 0});

// Run a container from the loaded image to verify it works
auto runResult = RunWslc(std::format(L"container run --rm {} echo Hello from saved image!", DebianImage.NameAndTag()));
runResult.Verify({.Stdout = L"Hello from saved image!\n", .Stderr = L"", .ExitCode = 0});
}

private:
const TestImage DebianImage = DebianTestImage();
const TestImage& InvalidImage = InvalidTestImage();
Expand Down
57 changes: 57 additions & 0 deletions test/windows/wslc/e2e/WSLCExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,63 @@ void RunWslcAndVerify(const std::wstring& cmd, const WSLCExecutionResult& expect
RunWslc(cmd, elevationType).Verify(expected);
}

WSLCExecutionResult RunWslcAndRedirectToFile(const std::wstring& commandLine, std::optional<std::filesystem::path> outputPath, ElevationType elevationType)
{
auto cmd = L"\"" + GetWslcPath() + L"\" " + commandLine;
Comment thread
AmelBawa-msft marked this conversation as resolved.
wsl::windows::common::SubProcess process(nullptr, cmd.c_str());

// If running non-elevated we need to keep the token alive until it completes.
wil::unique_handle nonElevatedToken;
if (elevationType == ElevationType::NonElevated)
{
nonElevatedToken = GetNonElevatedPrimaryToken();
process.SetToken(nonElevatedToken.get());
}

auto [parentStderrRead, childStderrWrite] = wsl::windows::common::wslutil::OpenAnonymousPipe(0, true, false);
THROW_IF_WIN32_BOOL_FALSE(SetHandleInformation(childStderrWrite.get(), HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT));

wil::unique_hfile redirectedStdout;
HANDLE stdoutHandle = nullptr;

std::wstring effectiveCommandLine = commandLine;
if (outputPath.has_value())
{
SECURITY_ATTRIBUTES securityAttributes{};
securityAttributes.nLength = sizeof(securityAttributes);
securityAttributes.bInheritHandle = TRUE;
redirectedStdout.reset(CreateFileW(
outputPath->c_str(), GENERIC_WRITE, FILE_SHARE_READ, &securityAttributes, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr));
THROW_LAST_ERROR_IF(!redirectedStdout);
stdoutHandle = redirectedStdout.get();
effectiveCommandLine = std::format(L"{} > \"{}\"", commandLine, outputPath->wstring());
}
else
{
// Open CONOUT$ so the child process receives a real console handle regardless of
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's clever ! I think this should be OK in most cases, but this might fail if the tests are run in a console-less context.

If that happens, we can always revisit

// how the test runner has configured its own stdout (e.g. piped in CI). This
// makes IsConsoleHandle() return true inside wslc, which is the condition under
// test in WSLCE2E_Image_Save_ToTerminal_Fail.
SECURITY_ATTRIBUTES securityAttributes{};
securityAttributes.nLength = sizeof(securityAttributes);
securityAttributes.bInheritHandle = TRUE;
redirectedStdout.reset(
CreateFileW(L"CONOUT$", GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, &securityAttributes, OPEN_EXISTING, 0, nullptr));
THROW_LAST_ERROR_IF(!redirectedStdout);
stdoutHandle = redirectedStdout.get();
}

process.SetStdHandles(nullptr, stdoutHandle, childStderrWrite.get());

const auto processHandle = process.Start();
childStderrWrite.reset();

const auto exitCode = wsl::windows::common::SubProcess::GetExitCode(processHandle.get());
const auto stdErrOutput = wsl::shared::string::MultiByteToWide(ReadToString(parentStderrRead.get()));

return {.CommandLine = std::move(effectiveCommandLine), .Stdout = L"", .Stderr = stdErrOutput, .ExitCode = exitCode};
}

std::wstring GetWslcHeader()
{
std::wstringstream header;
Expand Down
4 changes: 4 additions & 0 deletions test/windows/wslc/e2e/WSLCExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ struct WSLCInteractiveSession
};

WSLCExecutionResult RunWslc(const std::wstring& commandLine, ElevationType elevationType = ElevationType::Elevated);
WSLCExecutionResult RunWslcAndRedirectToFile(
const std::wstring& commandLine,
std::optional<std::filesystem::path> outputPath = std::nullopt,
ElevationType elevationType = ElevationType::Elevated);
void RunWslcAndVerify(const std::wstring& cmd, const WSLCExecutionResult& expected, ElevationType elevationType = ElevationType::Elevated);

std::wstring GetWslcHeader();
Expand Down
Loading