Skip to content

Add --filter support to wslc network list - #41318

Open
beena352 wants to merge 3 commits into
microsoft:masterfrom
beena352:users/beenachauhan/wslc-network-list-filter
Open

Add --filter support to wslc network list#41318
beena352 wants to merge 3 commits into
microsoft:masterfrom
beena352:users/beenachauhan/wslc-network-list-filter

Conversation

@beena352

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

Adds --filter to wslc network list so users can filter results by label, driver, or name, the same way container list and image list already work. When no filter is passed, behavior is unchanged (cache-only, no Docker call). When a filter is passed, we query Docker with the user's filters plus the WSLC managed-network label so results stay scoped to WSLC-managed networks.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Mirrors the existing shape used by container list --filter and image list --filter.

Validation Steps Performed

  • New unit tests in WSLCTests.cpp cover label filters (key-only, key=value, AND), driver hit/miss, the explicit managed-label being idempotent, and null filter key/value returning E_POINTER
  • New E2E tests in WSLCE2ENetworkListTests.cpp cover malformed value, invalid key, driver, label, name substring match, and NDJSON stdout being exactly empty on zero matches.

Copilot AI lite review requested due to automatic review settings August 11, 2026 22:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds --filter support to wslc network list, aligning it with existing list commands (containers/images) by allowing filtering by Docker-supported keys (e.g., label, driver, name). The implementation preserves the current “cache-only” behavior when no filters are provided, and switches to a Docker query (scoped to WSLC-managed networks) when filters are present.

Changes:

  • Extends the IWSLCSession::ListNetworks COM API to accept Docker-style filters and threads the filters through CLI → service layers.
  • Updates the Docker HTTP client GET /networks call to support a filters= query parameter.
  • Adds unit and E2E coverage for filter parsing/validation and filtered listing behavior (including NDJSON empty-output semantics).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/windows/WSLCTests.cpp Updates callers for the new ListNetworks signature and adds unit coverage for filter behavior and E_POINTER cases.
test/windows/wslc/e2e/WSLCE2ENetworkListTests.cpp Adds E2E coverage for filter parsing errors, Docker invalid keys, driver/label/name filtering, and empty NDJSON output.
src/windows/wslcsession/WSLCSession.h Updates WSLCSession::ListNetworks signature to accept filters.
src/windows/wslcsession/WSLCSession.cpp Implements filtered list behavior: cache-only when no filters; Docker query + WSLC scoping when filters are present.
src/windows/wslcsession/DockerHTTPClient.h Extends ListNetworks to accept an optional filters map.
src/windows/wslcsession/DockerHTTPClient.cpp Adds filters query parameter serialization for Docker network listing.
src/windows/wslc/tasks/NetworkTasks.cpp Plumbs CLI --filter values into the network list retrieval path.
src/windows/wslc/services/NetworkService.h Extends NetworkService::List to accept filters.
src/windows/wslc/services/NetworkService.cpp Converts parsed CLI filters into WSLCFilter entries and calls the updated session API.
src/windows/wslc/commands/NetworkListCommand.cpp Adds --filter argument support to the network list command.
src/windows/service/inc/wslc.idl Updates IWSLCSession::ListNetworks method signature to accept filter array + count.
Suppressed comments (1)

src/windows/wslcsession/WSLCSession.cpp:3062

  • When Docker returns networks but none of them are present in m_networks, index stays 0 and this path still releases a non-null *Networks buffer with *Count == 0. That’s inconsistent with other list methods (they leave the out pointer null when the count is 0) and can waste allocations for filter queries that match only non-managed networks.

    *Networks = output.release();
    *Count = index;

    return S_OK;

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI review requested due to automatic review settings August 11, 2026 23:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/windows/wslcsession/WSLCSession.cpp:3062

  • When Docker returns networks that don't exist in m_networks (e.g., stale WSLC-managed networks from another session), the loop skips them and can leave index == 0. The current code still releases output and returns *Networks != nullptr with *Count == 0, which is inconsistent with the earlier empty-result behavior and can confuse COM callers. Prefer returning *Networks == nullptr when *Count == 0 (only release the buffer if at least one entry is written).

    *Networks = output.release();
    *Count = index;

    return S_OK;

Copilot AI review requested due to automatic review settings August 12, 2026 16:41
@beena352
beena352 marked this pull request as ready for review August 12, 2026 16:42
@beena352
beena352 requested review from a team as code owners August 12, 2026 16:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/windows/wslcsession/WSLCSession.cpp:3049

  • This loop may skip entries when dockerNetworks contains names not present in m_networks (e.g., races/out-of-sync state). In that case index can remain 0, but the function still releases the allocated output buffer to *Networks, returning a non-null pointer with *Count == 0 and leaving elements uninitialized. It would be safer and consistent with the empty-list behavior to keep *Networks == nullptr when index == 0 and only release the buffer when at least one element was written.
    for (const auto& dockerNetwork : dockerNetworks)
    {
        auto it = m_networks.find(dockerNetwork.Name);
        if (it == m_networks.end())
        {

@dkbennett David Bennett (dkbennett) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good overall, just need a new ArgType definition that is otherwise identical but has an alias for 'f' to be consistent with Docker CLI.

Edit: I was wrong, list has -f, it is prune that is using the aliased filter incorrectly since in Docker -f is for force.

std::vector<Argument> NetworkListCommand::GetArguments() const
{
return {
Argument::Create(ArgType::Filter, false, Limit::Unlimited),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need a new argtype for this one "NetworkFilter" that is almost a duplicate of Filter because ArgType::Filter has no alias for '-f', and docker's network list does alias '-f' to filter.

docker network ls: --filter has the -f alias.
docker network prune: --filter has no short alias (because -f means --force in this command)

The resolution is to copy the Filter ArgType and make a new one like "FilterAliased" or "ListFilter" or some other enum value that has "f" as the alias and use that one for list and use the regular Filter for prune.

This is a seam in the argument model where the idenity of an ArgType is its name, alias, kind, and conversion type and if any of those need to change it must be a new ArgType. I may rework this later to allow alias override, but its not urgent and adding another enum value is an easy workaround.

@dkbennett David Bennett (dkbennett) Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And i realized I have this backwards. It's prune that has the bug, list is fine. Prune has a bug to fix (to create one without the alias). Updating to approve this.

auto it = m_networks.find(dockerNetwork.Name);
if (it == m_networks.end())
{
continue;

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.

nit: This is probably worth an ETL event since this would be unexpected


if (filters.empty())
{
std::lock_guard networksLock(m_networksLock);

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.

I recommend factoring the with & without filter blocks. We could do something like this:

std::lock_guard networksLock(m_networksLock);

std::optional<std::vector<docker_schema::Network> dockerNetworks> selectedNetworks;
if (!filters.empty())
{
    try
    {
        selectedNetworks = m_runtime.Docker().ListNetworks(filters);
    }
    CATCH_AND_THROW_DOCKER_USER_ERROR("Failed to list networks");
}

for (const auto& [name, entry] : m_networks)
{
    if (selectedNetworks.has_value())
    {
           // Skip if the network was not returned by the docker query
           [...]
           continue;
    }
   [...]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants