fix: close outFile before returning on copy error in Unpack and DeCompress#7648
fix: close outFile before returning on copy error in Unpack and DeCompress#7648Ahmad-Faraj wants to merge 1 commit into
Conversation
…press Both Unpack (operator/pkg/util) and DeCompress (karmadactl/cmdinit/utils) open a file per tar entry but do not close it when ioCopyN returns an error, leaking the file descriptor. Also the successful close call ignored the return value; on some filesystems (e.g. NFS) close can fail after a buffered write, so the error is now propagated. Signed-off-by: Ahmed Faraj <ahmed.faraj@datajar.co> Signed-off-by: Ahmad-Faraj <ahmedfrag4040@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @Ahmad-Faraj! It looks like this is your first PR to karmada-io/karmada 🎉 |
karmada-bot
added
the
size/S
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the robustness of the Unpack and DeCompress utility functions by addressing potential file descriptor leaks and silent write failures. By ensuring that file handles are properly closed during error scenarios and validating the return status of file closures, the system can now reliably detect and report issues that were previously ignored. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves resource cleanup and error handling during file decompression and unpacking. Specifically, in operator/pkg/util/util.go and pkg/karmadactl/cmdinit/utils/util.go, the code now ensures that the output file is closed if ioCopyN fails, and it properly checks and handles any error returned by outFile.Close() during normal execution. There are no review comments, so I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7648 +/- ##
=======================================
Coverage 42.04% 42.05%
=======================================
Files 879 879
Lines 54827 54831 +4
=======================================
+ Hits 23053 23058 +5
+ Misses 30027 30026 -1
Partials 1747 1747
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/retest |
4 participants
What type of PR is this?
/kind bug
What this PR does / why we need it
Unpackinoperator/pkg/utilandDeCompressinpkg/karmadactl/cmdinit/utilsboth open a file per tar entry inside a loop. WhenioCopyNreturns an error, the function returns without closing the open*os.File, leaking the file descriptor.The successful
outFile.Close()was also discarding its return value. On NFS and similar filesystems, write errors only surface at close time, so a failed write could return success with a truncated file.Which issue(s) this PR fixes
None (self-found)
Special notes for your reviewer
Both functions have the same bug;
Unpackis inoperator/andDeCompressinkarmadactl/cmdinit/utils/.Does this PR introduce a user-facing change?
No.