-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Richfr/ Add windowsAddress support to WslcCreateContainer() API #40037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/wsl-for-apps
Are you sure you want to change the base?
Changes from 12 commits
fe043b9
04a4541
cf34fb8
954d1a0
31e28b4
7ca8e51
7f1eca4
eff50d2
41a39eb
895a604
a788da8
644ebab
47c28f2
9ed0f18
fb9c466
13ba738
e482e48
1e7656a
f4fd0ba
45b6841
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -201,6 +201,25 @@ void EnsureAbsolutePath(const std::filesystem::path& path, bool containerPath) | |
| } | ||
| } | ||
|
|
||
| HRESULT InetNtopToHresult(int af, const void* src, char* dst, size_t dstCount) | ||
| { | ||
| const char* result = inet_ntop(af, src, dst, dstCount); | ||
| if (result) | ||
| { | ||
| return S_OK; | ||
| } | ||
|
|
||
| switch (errno) | ||
| { | ||
| case EAFNOSUPPORT: | ||
| return E_INVALIDARG; | ||
| case ENOSPC: | ||
| return HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER); | ||
| default: | ||
| return E_FAIL; | ||
1wizkid marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| bool CopyProcessSettingsToRuntime(WSLCProcessOptions& runtimeOptions, const WslcContainerProcessOptionsInternal* initProcessOptions) | ||
| { | ||
| if (initProcessOptions) | ||
|
|
@@ -617,6 +636,8 @@ try | |
| auto result = std::make_unique<WslcContainerImpl>(); | ||
|
|
||
| WSLCContainerOptions containerOptions{}; | ||
| std::unique_ptr<WSLCPortMapping[]> convertedPorts; // this must stay in same scope as containerOptions since containerOptions.Ports is getting a raw pointer to the array owned by convertedPorts. | ||
|
|
||
| containerOptions.Image = internalContainerSettings->image; | ||
| containerOptions.Name = internalContainerSettings->runtimeName; | ||
| containerOptions.HostName = internalContainerSettings->HostName; | ||
|
|
@@ -659,7 +680,6 @@ try | |
| containerOptions.NamedVolumesCount = static_cast<ULONG>(internalContainerSettings->namedVolumesCount); | ||
| } | ||
|
|
||
| std::unique_ptr<WSLCPortMapping[]> convertedPorts; | ||
| if (internalContainerSettings->ports && internalContainerSettings->portsCount) | ||
| { | ||
| convertedPorts = std::make_unique<WSLCPortMapping[]>(internalContainerSettings->portsCount); | ||
|
|
@@ -671,14 +691,58 @@ try | |
| convertedPort.HostPort = internalPort.windowsPort; | ||
| convertedPort.ContainerPort = internalPort.containerPort; | ||
1wizkid marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // TODO: Ipv6 & custom binding address support. | ||
| convertedPort.Family = AF_INET; | ||
|
|
||
| // TODO: Consider using standard protocol numbers instead of our own enum. | ||
| convertedPort.Protocol = internalPort.protocol == WSLC_PORT_PROTOCOL_TCP ? IPPROTO_TCP : IPPROTO_UDP; | ||
| strcpy_s(convertedPort.BindingAddress, "127.0.0.1"); | ||
| // Validate IP address if provided and if valid, copy to runtime structure. | ||
| if (internalPort.windowsAddress != nullptr) | ||
| { | ||
| char addrBuf[INET6_ADDRSTRLEN]{}; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of having a temporary buffer, I recommend copying directly to |
||
| switch (internalPort.windowsAddress->ss_family) | ||
| { | ||
| case AF_INET: | ||
| { | ||
| const auto* addr4 = reinterpret_cast<const sockaddr_in*>(internalPort.windowsAddress); | ||
|
|
||
| HRESULT hr = InetNtopToHresult(AF_INET, &addr4->sin_addr, addrBuf, sizeof(addrBuf)); | ||
| if (FAILED(hr)) | ||
| { | ||
| THROW_HR_MSG(hr, "inet_ntop() failed for address family AF_INET"); | ||
| } | ||
| convertedPort.Family = AF_INET; | ||
| break; | ||
| } | ||
|
|
||
| case AF_INET6: | ||
| { | ||
| const auto* addr6 = reinterpret_cast<const sockaddr_in6*>(internalPort.windowsAddress); | ||
| HRESULT hr = InetNtopToHresult(AF_INET6, &addr6->sin6_addr, addrBuf, sizeof(addrBuf)); | ||
| if (FAILED(hr)) | ||
| { | ||
| THROW_HR_MSG(hr, "inet_ntop() failed for address family AF_INET6"); | ||
| } | ||
| convertedPort.Family = AF_INET6; | ||
| break; | ||
| } | ||
|
|
||
| default: | ||
| // Reject unsupported or malformed address families | ||
| THROW_HR(E_INVALIDARG); | ||
1wizkid marked this conversation as resolved.
Show resolved
Hide resolved
1wizkid marked this conversation as resolved.
Show resolved
Hide resolved
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I recommend logging the address family to make debugging easier |
||
| } | ||
| HRESULT hr = strncpy_s(convertedPort.BindingAddress, sizeof(convertedPort.BindingAddress), addrBuf, _TRUNCATE); | ||
|
|
||
| if (hr == STRUNCATE) | ||
| { | ||
| // Log this as a warning since the address is valid but was truncated to fit in the buffer, which means the port mapping may not work as expected. | ||
| } | ||
1wizkid marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| else | ||
| { | ||
| // If no binding address is provided, default to local host. | ||
| convertedPort.Family = AF_INET; | ||
| strcpy_s(convertedPort.BindingAddress, "127.0.0.1"); | ||
| } | ||
1wizkid marked this conversation as resolved.
Show resolved
Hide resolved
1wizkid marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| containerOptions.Ports = convertedPorts.get(); | ||
| containerOptions.Ports = convertedPorts.get(); // Make sure convertedPorts stays in scope for life of containerOptions | ||
| containerOptions.PortsCount = static_cast<ULONG>(internalContainerSettings->portsCount); | ||
| } | ||
|
|
||
|
|
@@ -809,7 +873,11 @@ try | |
|
|
||
| for (uint32_t i = 0; i < portMappingCount; ++i) | ||
| { | ||
| RETURN_HR_IF(E_NOTIMPL, portMappings[i].windowsAddress != nullptr); | ||
| if (portMappings[i].windowsAddress != nullptr) | ||
| { | ||
| const auto family = portMappings[i].windowsAddress->ss_family; | ||
| RETURN_HR_IF(E_INVALIDARG, family != AF_INET && family != AF_INET6); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I recommend logging the invalid address family here |
||
| } | ||
| RETURN_HR_IF(E_NOTIMPL, portMappings[i].protocol != 0); | ||
1wizkid marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend throwing the win32 error directly: