-
Notifications
You must be signed in to change notification settings - Fork 40.9k
fix(kubectl): recognize absolute Windows paths #132592
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: master
Are you sure you want to change the base?
fix(kubectl): recognize absolute Windows paths #132592
Conversation
|
Welcome @adalinesimonian! |
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Hi @adalinesimonian. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: adalinesimonian The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/kind bug |
Fixes issue 101985 Before this change, kubectl cp would misinterpret Windows absolute paths like "C:\temp\file.txt" as pod specifications because it would split on the colon character, treating "C" as a pod name and "\temp\file.txt" as the file path. This fix adds Windows path detection in extractFileSpec() by checking for the pattern [drive letter]:[backslash or forward slash] before attempting to parse pod specifications. It also adjusts path handling to properly handle Windows drive letters in tar operations, and all tests at staging/src/k8s.io/kubectl/pkg/cmd/ now pass on Windows systems. Changes made: - Added isWindowsAbsolutePath() to detect Windows absolute paths - Added stripWindowsDriveLetter() to strip the drive letter from Windows paths for consistent processing - Modified extractFileSpec() to check for Windows paths first when running on Windows - Modified stripPathShortcuts() to handle Windows path separators - Added test coverage for Windows path handling
7bf1a4b
to
f785bce
Compare
Assign |
What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR fixes a bug where
kubectl cp
would incorrectly interpret Windows absolute paths as pod specifications. Before this change, paths likeC:\temp\file.txt
would be parsed as a pod namedC
with file path\temp\file.txt
because the colon character was used as a delimiter to separate pod specifications from file paths.The fix adds Windows-specific path detection that checks for the Windows absolute path pattern before attempting to parse pod specifications. This ensures that Windows users can use absolute paths with
kubectl cp
without running into parsing errors.Which issue(s) this PR is related to:
Fixes #101985
Special notes for your reviewer:
runtime.GOOS == "windows"
.staging/src/k8s.io/kubectl/pkg/cmd/
now pass on Windows after these changes.Besides the above, just a disclaimer that I am completely new to contributing to this repository. I don't have a good mental model of how all of the pieces fit together here. Though, I've done my best to try, and I am happy for any feedback or guidance.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
N/A