Image filters#40584
Open
kvega005 wants to merge 18 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors WSLC image list/prune filtering to use generic key/value filter arrays (closer to Docker’s filter model), removing bespoke option structs/flag enums and threading the new filter representation through the service, session, HTTP client, and tests.
Changes:
- Replaced image list/prune option structs and dangling-related enums with
WSLCFilterkey/value arrays in the WSLC COM API. - Updated
WSLCSessionandDockerHTTPClientto pass filters asstd::map<std::string, std::vector<std::string>>serialized into Docker’sfiltersquery parameter. - Updated tests and
ImageServicecall sites to construct and pass the new filter arrays.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/windows/WSLCTests.cpp | Updates image list/prune tests to use WSLCFilter arrays instead of specialized options/flags. |
| src/windows/wslcsession/WSLCSession.h | Updates IWSLCSession method signatures to use WSLCListImagesOptions and filter arrays for prune. |
| src/windows/wslcsession/WSLCSession.cpp | Refactors ListImages/PruneImages to parse WSLCFilter arrays via ParseKeyMultiValuePairs and pass maps to Docker client. |
| src/windows/wslcsession/DockerHTTPClient.h | Removes custom filter structs; updates ListImages/PruneImages to accept map<string, vector<string>>. |
| src/windows/wslcsession/DockerHTTPClient.cpp | Serializes the generic filter map directly into Docker’s filters query parameter. |
| src/windows/wslc/services/ImageService.cpp | Updates pruning to send dangling filter via WSLCFilter. |
| src/windows/service/inc/wslc.idl | Updates IDL structs/enums and changes IWSLCSession method signatures for list/prune. |
Comments suppressed due to low confidence (1)
src/windows/wslcsession/WSLCSession.cpp:1216
- ListImages now explicitly rejects any flags outside WSLCListImagesFlagsValid (including the previously-supported dangling flags). There doesn't appear to be a corresponding test ensuring invalid/legacy flag bits return E_INVALIDARG, which is important to lock in the new contract and prevent regressions. Consider adding a test that calls ListImages with an invalid bit (or the removed dangling bits) and verifies it fails with E_INVALIDARG.
}
HRESULT WSLCSession::ListImages(const WSLCListImagesOptions* Options, WSLCImageInformation** Images, ULONG* Count)
try
{
COMServiceExecutionContext context;
RETURN_HR_IF_NULL(E_POINTER, Images);
Comment on lines
714
to
718
|
|
||
| // Image management. | ||
| HRESULT PullImage([in] LPCSTR Image, [in, unique] LPCSTR RegistryAuthenticationInformation, [in, unique] IProgressCallback* ProgressCallback); | ||
| HRESULT BuildImage([in] const WSLCBuildImageOptions* Options, [in, unique] IProgressCallback* ProgressCallback, [in, unique, system_handle(sh_event)] HANDLE CancelEvent); | ||
| HRESULT LoadImage([in] WSLCHandle ImageHandle, [in, unique] IProgressCallback* ProgressCallback, [in] ULONGLONG ContentLength); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/windows/wslcsession/WSLCSession.cpp:1
- Similar to
ListImages, this callsParseKeyMultiValuePairs(Filters, FiltersCount)without validating thatFiltersis non-null whenFiltersCount > 0. If RPC/COM callers supply an inconsistent pointer/count pair, this can crash in-process. Add a guard (e.g., rejectFiltersCount > 0 && Filters == nullptr) before parsing.
/*++
| THROW_HR_IF_MSG( | ||
| E_INVALIDARG, | ||
| WI_IsAnyFlagSet(static_cast<WSLCListImagesFlags>(Options->Flags), ~WSLCListImagesFlagsValid), | ||
| "Invalid flags: 0x%x", |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors and simplifies how image listing and pruning filters are handled in the Windows Subsystem for Linux Containers (WSLC) service. It removes custom filter structures and flag enums in favor of using generic key-value filter arrays, making the API more flexible and closer to Docker's native filtering model. The changes also update related method signatures and test cases to match the new approach.
Key changes:
API and Data Structure Simplification
WSLCListImageOptionsand related filter fields (such asReference,Before,Since, andLabels) with a genericWSLCListImagesOptionsstruct that uses an array ofWSLCFilterkey-value pairs for filters. The corresponding flags for dangling images were removed, and only theAllandDigestsflags remain valid. (src/windows/service/inc/wslc.idl, [1] [2]WSLCPruneImagesFlagsenum andWSLCPruneImagesOptionsstruct. ThePruneImagesmethod now directly takes an array ofWSLCFilterkey-value pairs, aligning its interface with the new filtering model. (src/windows/service/inc/wslc.idl, [1] [2]Implementation Updates
WSLCSessionto use the new generic filter map, removing custom filter structs and flag logic. Filters are now parsed from the key-value array and passed directly to the Docker HTTP client. (src/windows/wslcsession/WSLCSession.cpp, [1] [2] [3]DockerHTTPClientinterface and implementation to accept filters as astd::map<std::string, std::vector<std::string>>instead of custom filter structs. Removed the now-unused filter conversion utility. (src/windows/wslcsession/DockerHTTPClient.h, [1];src/windows/wslcsession/DockerHTTPClient.cpp, [2] [3] [4]Test and Usage Updates
WSLCFilterarray approach instead of setting individual fields in the options struct. (test/windows/WSLCTests.cpp, [1] [2] [3];src/windows/wslc/services/ImageService.cpp, [4]Interface and Type Renaming
WSLCListImageOptionstoWSLCListImagesOptionsand updated method signatures accordingly to maintain consistency. (src/windows/service/inc/wslc.idl, [1];src/windows/wslcsession/WSLCSession.h, [2];src/windows/wslcsession/WSLCSession.cpp, [3]These changes make the WSLC image management API more flexible, maintainable, and consistent with Docker's own API conventions.
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed