Remove leading slash in Windows path#3304
Conversation
|
Waiting for the changes from Set stderr to empty string when using terminal on Windows to be available in the latest Containerd v2 release. |
TinaMor
changed the title
|
Hey @TinaMor 2.0.0 rc4 just dropped and we moved nerdctl (and testing env) to it. Curious if the change in containerd got in? |
Signed-off-by: Christine Murimi <[email protected]>
| ioCreator = cio.TerminalBinaryIO(u.Path, map[string]string{ | ||
| parsedPath := u.Path | ||
| // For Windows, remove the leading slash | ||
| if (runtime.GOOS == "windows") && (strings.HasPrefix(parsedPath, "/")) { |
There was a problem hiding this comment.
Is this the right place to remove the extra slash?
From the report "binary:///C:/Program Files/nerdctl.exe?_NERDCTL_INTERNAL_LOGGING=C%3A%5CProgramData%5Cnerdctl%5C052055e3" it seems it has 3 slashes in front of the file path: binary:///
Where is that generated and why does it have three slashes?
There was a problem hiding this comment.
The path is prefixed with "/" in containerd/containerd LogURIGenerator(). See the comment in LogURIGenerator().
There was a problem hiding this comment.
ah interesting, it is expected that url.parse returns /c:/path golang/go@844b625
This actually feels like a bug in cio.TerminalBinaryIO when it is checking abs path logic but I there might be some nuance that I am missing.
5 participants
PR Description
This PR resolves #2966
Background
On Windows, the LogURIGenerator() function in containerd/containerd fails when it checks if the
logURIpath is absolute. This is due to the leading slash in the parsed logURI, which causes the check to fail and results in an error, ultimately leading to the command's failure.The logURI parameter is a binary path formatted as a URI, for example:
When parsed with
url.Parse(logURI), the resultingu.Pathis:The leading slash is incorrect for Windows paths and causes issues when the path is later checked for being absolute.
Solution
To resolve this issue, the leading slash must be removed from the
u.Pathbefore the absolute path check is performed. This ensures the path is correctly recognized as an absolute path on Windows systems.Additional tasks
Remove Windows check in TestRunWithTtyAndDetached. 'json-file' logging for Windows implemented by PR #1468 .
nerdctl/cmd/nerdctl/container_run_test.go
Lines 461 to 464 in 74f2755