-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add cp command for file copy to/from instances #18
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
Conversation
Add 'hypeman cp' command for copying files to/from running instances, with docker cp compatible semantics: Features: - Copy files and directories to instances: hypeman cp file.txt instance:/path - Copy from instances: hypeman cp instance:/path file.txt - STDIN/STDOUT streaming with '-' argument for tar archives - Recursive directory copying with proper tar packing/unpacking - Symlink handling with --follow-links flag - Archive mode (-a) to preserve UID/GID ownership - Quiet mode (-q) to suppress progress output - Docker cp path resolution semantics: - SRC_PATH/. copies contents, not the directory itself - Trailing slash handling for directory vs file detection The command communicates with the API server via WebSocket, streaming file data as binary chunks with JSON metadata headers.
Update to e893d90 with circular symlink detection and root ownership fix.
87ddc40 to
04713b9
Compare
go.mod
Outdated
| google.golang.org/grpc v1.75.1 // indirect | ||
| ) | ||
|
|
||
| replace github.com/onkernel/hypeman-go => ../hypeman-go |
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.
will update after hypeman-go merge
When copying a directory from instance to local, the /. suffix (copy contents only) was computed but not used. Now: - Without /.: creates source directory inside destination (docker cp semantics) - With /.: copies contents directly into destination
| if endMarker.Final { | ||
| receivedFinal = true | ||
| return nil | ||
| } |
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.
Missing error handling for JSON unmarshal operations
The json.Unmarshal calls on lines 962 and 970 don't check for errors. If unmarshaling fails on line 962, endMarker.Final defaults to false, causing the function to continue looping even when the server sent a valid final end marker with an unexpected format. This would eventually produce a misleading "copy stream ended without completion marker" error instead of the actual parse error.
Additional Locations (1)
| case tar.TypeSymlink: | ||
| // TODO: Handle symlinks if needed | ||
| fmt.Fprintf(os.Stderr, "Warning: skipping symlink %s -> %s\n", header.Name, header.Linkname) | ||
| } |
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.
Hard links in tar archives silently dropped without warning
The switch statement handling tar entry types only processes TypeDir, TypeReg, and TypeSymlink (which gets a warning). Hard links (tar.TypeLink) and other valid tar types like character devices, block devices, and FIFOs are silently ignored with no warning. This inconsistency is problematic because hard links are commonly used in tar archives for file deduplication. Users extracting tar archives containing hard links would experience silent data loss without any indication that files were skipped.
Update to official v0.8.0 release which adds the file copy SDK functions (CpToInstance, CpFromInstance).
9f50c6c to
cdc3257
Compare
| if !dstStat.Exists { | ||
| // DEST will be created | ||
| return dstPath, nil | ||
| } |
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.
Missing trailing slash validation for directory copy destinations
When copying a directory to a non-existent destination path ending with a trailing slash (e.g., instance:/nonexistent/), the code proceeds instead of returning an error. For file sources, the code correctly checks dstEndsWithSlash && !dstExists and errors with "destination directory does not exist" (lines 341-343 and 636-638). However, the directory source branches at lines 362-365 and 652-653 skip this check. In docker cp, a trailing slash indicates "copy into this directory" and failing when that directory doesn't exist is expected behavior. This breaks the claimed docker cp path compatibility.
Summary
Add
hypeman cpcommand for copying files to/from running VM instances with docker cp compatible semantics.Usage
Features
SRC_PATH/.copies contents, not the directory itself-argument for tar archives-a) to preserve UID/GID ownership--follow-linksflag-q) to suppress progress outputDependencies
Requires corresponding changes in:
Note
Introduces a new
cpsubcommand enabling bi-directional file and directory copy between the local filesystem and instances with docker cp-compatible behavior.pkg/cmd/cp.goimplementing copy to/from instance, includingSRC/.and trailing-slash semantics, and robust path parsing (local vsinstance:/path) with Windows path handling-, including WebSocket cp endpoint usage and progress callbacks--archive/-a(preserve UID/GID),--follow-links/-L, and--quiet/-qpkg/cmd/cmd.go; bumpsgithub.com/onkernel/hypeman-goto v0.8.0 ingo.mod/go.sumWritten by Cursor Bugbot for commit cdc3257. This will update automatically on new commits. Configure here.