Skip to content

Fix wslc create network filaure without driver specified#40788

Open
chemwolf6922 wants to merge 4 commits into
masterfrom
user/chemwolf6922/fix-create-network-without-driver
Open

Fix wslc create network filaure without driver specified#40788
chemwolf6922 wants to merge 4 commits into
masterfrom
user/chemwolf6922/fix-create-network-without-driver

Conversation

@chemwolf6922

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

The WSLCSession::CreateNetwork expects Options->Driver to be populated. However, when the driver is not specified in the cli. It's skipped and default to NULL. This PR populates it with the default "bridge" driver when not specified.

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

Validation Steps Performed

Enable test:
WSLCE2ETests::WSLCE2ENetworkCreateTests::WSLCE2E_Network_Create_DefaultDriver_Success

Copilot AI review requested due to automatic review settings June 12, 2026 09:43
@chemwolf6922 chemwolf6922 requested a review from a team as a code owner June 12, 2026 09:43

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

This PR fixes wslc network create failing when --driver is not provided by ensuring the CLI task always supplies a default network driver (bridge) to the underlying WSLC session API. This aligns runtime behavior with the CLI help text and enables the previously-disabled E2E coverage for the default-driver path.

Changes:

  • Default CreateNetworkOptions.Driver to "bridge" when --driver is omitted in network create.
  • Re-enable the E2E test that validates creating a network without specifying a driver.

Reviewed changes

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

File Description
test/windows/wslc/e2e/WSLCE2ENetworkCreateTests.cpp Re-enables the default-driver network create E2E test and asserts the created network uses bridge.
src/windows/wslc/tasks/NetworkTasks.cpp Ensures options.Driver is always populated (defaults to "bridge" when not specified).

beena352
beena352 previously approved these changes Jun 12, 2026
Comment thread src/windows/wslc/tasks/NetworkTasks.cpp Outdated

@dkbennett 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.

I don't think it is correct for the CLI to be setting a default if the service sets one. The service should handle that and the CLI should not be setting its own default.

@dkbennett

Copy link
Copy Markdown
Member

To add some additional context:

  • If the driver needs to be specified, then it should be a required argument
  • If the driver is only required in combination with another argument in the command, then we have ValidateArgumentsInternal() for that where such custom validation can be checked and an appropriate error issued.
  • If no driver specified is valid and the service sets a default, then the CLI should do nothing and let the service validate it.
  • If the driver specification is an Enum or has a limited set of valid options, then the CLI can validate that argument with a validator against the Enum map or similar (see how the Signal argument is processed for a complex example).

Copilot AI review requested due to automatic review settings June 15, 2026 05:38

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 3 out of 3 changed files in this pull request and generated 2 comments.


std::string name = Options->Name;
std::string driver = Options->Driver;
std::string driver = (Options->Driver != nullptr && Options->Driver[0] != '\0') ? Options->Driver : WSLCBridgeNetworkDriver;
options.DriverOptsCount = 0;

for (const char* driver : {"overlay", "Bridge", ""})
for (const char* driver : {"overlay", "Bridge"})
@chemwolf6922

Copy link
Copy Markdown
Contributor Author

@OneBlue Do we want to allow "" being treated as default?

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.

6 participants