Add --ip, --link, --link-local-ip, --driver-opt, --network-alias flag…#41070
Open
beena352 wants to merge 10 commits into
Open
Add --ip, --link, --link-local-ip, --driver-opt, --network-alias flag…#41070beena352 wants to merge 10 commits into
beena352 wants to merge 10 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends wslc network connect to accept additional Docker EndpointConfig-style settings at connect time (--network-alias, --ip, --link, --link-local-ip, --driver-opt) and fixes container-create forwarding so primary endpoint settings (beyond Aliases) are no longer parsed-but-dropped.
Changes:
- Add endpoint-settings parsing/validation (
Aliases,IPAddress,Links,LinkLocalIPs,DriverOpts) and forward the fullEndpointConfigin both create and connect flows. - Extend the CLI surface + plumbing (
ArgumentDefinitions→NetworkConnectCommand→NetworkTasks→NetworkService) to emit KVP settings intoConnectToNetwork. - Update/expand unit + e2e tests to cover new flags, validation, and inspect round-trips.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/WSLCTests.cpp | Updates/extends unit tests for endpoint settings at create/connect time. |
| test/windows/wslc/e2e/WSLCE2ENetworkTests.cpp | Adds e2e coverage for new network connect flags and help output. |
| src/windows/wslcsession/WSLCContainer.cpp | Implements unified endpoint-settings parsing + forwards full EndpointConfig. |
| src/windows/wslc/tasks/NetworkTasks.cpp | Collects new CLI args into a ConnectNetworkOptions model. |
| src/windows/wslc/services/NetworkService.h | Updates Connect API to take ConnectNetworkOptions. |
| src/windows/wslc/services/NetworkService.cpp | Builds KVP settings and calls ConnectToNetwork with settings payload. |
| src/windows/wslc/services/NetworkModel.h | Introduces ConnectNetworkOptions. |
| src/windows/wslc/commands/NetworkConnectCommand.cpp | Registers new repeatable/non-repeatable connect flags. |
| src/windows/wslc/arguments/ArgumentDefinitions.h | Defines new CLI arguments (driver-opt, ip, link, link-local-ip). |
| src/windows/service/inc/wslc.idl | Changes WSLCNetworkConnectionOptions to carry Settings KVPs. |
| src/windows/inc/wslc_schema.h | Extends inspect schema to include Links, DriverOpts, IPAMConfig. |
| src/windows/inc/docker_schema.h | Extends Docker request/inspect schema to include full EndpointConfig. |
| localization/strings/en-US/Resources.resw | Adds new user-facing/localizable strings for endpoint settings + flags. |
| localization/strings/en-GB/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/zh-TW/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/zh-CN/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/tr-TR/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/sv-SE/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/ru-RU/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/pt-PT/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/pt-BR/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/pl-PL/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/nl-NL/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/nb-NO/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/ko-KR/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/ja-JP/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/it-IT/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/hu-HU/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/fr-FR/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/fi-FI/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/es-ES/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/de-DE/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/da-DK/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
| localization/strings/cs-CZ/Resources.resw | Updates locale resource strings (see review comments re: locale policy). |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
localization/strings/fr-FR/Resources.resw:2390
- This PR modifies a non-
en-USlocalized Resources.resw file and adds/changes string values in English. Per CONTRIBUTING.md ("Localization and translation issues"), per-localelocalization/strings/<locale>/Resources.reswfiles are generated downstream and translation edits in PRs here are overwritten and cannot be merged; onlylocalization/strings/en-US/Resources.reswshould be updated in this repo (unless this is an automated localization-service PR). Please revert the changes in this file (and other non-en-USlocales touched by this PR) and keep only theen-USupdates.
<data name="MessageWslcEndpointSettingsNotSupported" xml:space="preserve">
<value>Les paramètres de point de terminaison ne sont pas encore pris en charge (réseau « {} »).</value>
<comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
</data>
<data name="MessageWslcAliasEmpty" xml:space="preserve">
<value>L’alias réseau ne peut pas être vide.</value>
</data>
<data name="MessageWslcAliasRequiresUserDefinedNetwork" xml:space="preserve">
<value>Les alias réseau nécessitent un réseau défini par l’utilisateur. Utilisez --network pour en spécifier un.</value>
localization/strings/zh-TW/Resources.resw:3003
- This non-
en-USlocalized Resources.resw file has been edited to add/replace string values with English (e.g.,WSLCCLI_NetworkAliasArgDescription,WSLCCLI_IpAddressArgDescription, etc.). CONTRIBUTING.md states these per-locale files are generated downstream and PR translation edits here cannot be merged; onlylocalization/strings/en-US/Resources.reswshould be updated (unless this is an automated localization-service PR). Please revert the changes in this locale file.
<value>顯示此工具的版本資訊</value>
</data>
<data name="WSLCCLI_VolumeArgDescription" xml:space="preserve">
<value>將磁碟區繫結掛接至容器</value>
</data>
<data name="WSLCCLI_VolumeForceArgDescription" xml:space="preserve">
<value>如果此磁碟區不存在，請勿顯示錯誤</value>
</data>
<data name="WSLCCLI_WorkingDirArgDescription" xml:space="preserve">
<value>容器內的工作目錄</value>
…slc network connect
beena352
force-pushed
the
user/beenachauhan/network-connect-endpoint-settings
branch
from
July 14, 2026 23:04
1c44677 to
dba9f54
Compare
dkbennett
reviewed
Jul 15, 2026
dkbennett
left a comment
dkbennett
left a comment
Member
There was a problem hiding this comment.
Couple minor refactor-ish things, nice tests and overall work looks good!
| return { | ||
| Argument::Create(ArgType::NetworkName, true), | ||
| Argument::Create(ArgType::ContainerId, true), | ||
| Argument::Create(ArgType::NetworkAlias, false, NO_LIMIT), |
Member
There was a problem hiding this comment.
These should be alphabetical order after positional arguments (this affects how they will be ordered in the help output).
So NetworkName & ContainerId are correct as-is since they are positional, but then it should be DriverOpt, IpAddress, etc. A little bizarre and I should probably add sorting to the help output so it doesn't matter, but nice to be consistent either way.
| const auto networkName = WideToMultiByte(context.Args.Get<ArgType::NetworkName>()); | ||
| const auto containerId = WideToMultiByte(context.Args.Get<ArgType::ContainerId>()); | ||
| NetworkService::Connect(context.Data.Get<Data::Session>(), networkName, containerId); | ||
| models::ConnectNetworkOptions options{}; |
Member
There was a problem hiding this comment.
We could be more compositional here with the tasks: e.g. have a new task that just sets network options, which is then consumed by Connect. So if we have other network commands that also need to set options we avoid duplicate code.
A good example would be SetContainerOptionsFromArgs, where we have a task which just sets container options based on the args present and adds it to context data, which then other container tasks consume as needed. I think this should be similar for more modularity in future network commands and code reuse.
Comment on lines
+315
to
+321
| auto key = entry.substr(0, separator); | ||
| auto value = entry.substr(separator + 1); | ||
| THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcDriverOptInvalid(entry), isBlank(key)); | ||
| THROW_HR_WITH_USER_ERROR_IF( | ||
| E_INVALIDARG, | ||
| Localization::MessageWslcAliasEmpty(), | ||
| alias.empty() || std::all_of(alias.begin(), alias.end(), [](unsigned char ch) { return std::isspace(ch); })); | ||
| Localization::MessageWslcDriverOptDuplicate(key), | ||
| !driverOpts.try_emplace(std::move(key), std::move(value)).second); |
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.
Summary of the Pull Request
Adds endpoint configuration flags to
wslc network connectand fixes the connect/disconnect mode gate to rejectcontainer:*in addition tohostandnone.PR Checklist
Detailed Description of the Pull Request / Additional comments
New flags on
wslc network connect--network-alias,--ip,--link,--link-local-ip,--driver-opt. All flags are repeatable except--ip(one static IP per network). Validation includes: unknown setting key rejection, malformed IP rejection, empty alias/link rejection, malformed/blank/duplicate driver-opt rejection. Empty driver-opt values (key=) are accepted intentionally to match Docker CLI behavior.The create path (
wslc container create --endpoint-settings) also picks up the same parser, soIPAddress,Links,LinkLocalIPs, andDriverOptsnow flow through to the Docker create request for the primary network (previously onlyAliaseswas forwarded).Bug fix: container:* mode gate
Previously the connect/disconnect gate only checked for
"host"and"none"modes. Containers usingcontainer:<name>(shared network namespace) were incorrectly allowed through. Now usesNetworkModeAllocatesVmPorts()which covers all three cases. Added new localized error messageMessageWslcNetworkModeNoAdditionalNetworks.Test changes
NetworkAliasCreateTestblock (expectedE_NOTIMPLforIPAddress) withBogusKey/E_INVALIDARGunknown-key test.container:<name>mode rejection test (substring match - mode resolves to container ID internally).key=value=extrapreserves everything after first=;key=(empty value) accepted per Docker CLI parity.WSLCE2E_Network_Connect_NoneMode_Rejectedpattern to match new error message.--driver-optand--linkflag roundtrips viawslc network connect+ inspect.Validation Steps Performed
WSLCTests::NetworkAliasCreateTestsub-blocks - unknown key rejection, positive combo roundtrip, and Links roundtrip all pass.WSLCTests::ContainerNetworkEndpointSettingsUnknownKeyTestandNetworkConnectEndpointSettingsTest- all sub-blocks pass.WSLCE2E_Network_Connect_DriverOpt_RoundTrips,WSLCE2E_Network_Connect_Link_RoundTrips.wslc network connect --helprenders all new flags with correct padding and descriptions.