Skip to content

Commit 77dbf28

Browse files
committed
refactor(file-handling): improve error handling and file cleanup
Signed-off-by: ShigrafS <[email protected]>
1 parent 3c002e7 commit 77dbf28

File tree

6 files changed

+91
-35
lines changed

6 files changed

+91
-35
lines changed

files/tests/sanitization_test.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,17 @@ func TestSanitizeFile(t *testing.T) {
146146
".zip": true,
147147
}
148148

149-
tempDir, _ := os.MkdirTemp(" ", "temp-tests")
150-
defer os.RemoveAll(tempDir)
151-
// tempDir := "./temp"
149+
// Create a temporary directory for the tests and handle potential errors.
150+
tempDir, err := os.MkdirTemp(" ", "temp-tests")
151+
if err != nil {
152+
t.Fatalf("Failed to create temporary directory: %v", err)
153+
}
154+
// Defer cleanup and log any error that occurs during cleanup.
155+
defer func() {
156+
if err := os.RemoveAll(tempDir); err != nil {
157+
t.Logf("Warning: failed to remove temporary directory %s: %v", tempDir, err)
158+
}
159+
}()
152160

153161
for _, tc := range testCases {
154162

generators/github/git_repo.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,15 @@ func (gr GitRepo) GetContent() (models.Package, error) {
4747
br := bufio.NewWriter(fd)
4848

4949
defer func() {
50-
br.Flush()
51-
fd.Close()
50+
if err := br.Flush(); err != nil {
51+
fmt.Printf("Warning: failed to flush writer: %v\n", err)
52+
}
53+
54+
if err := fd.Close(); err != nil {
55+
fmt.Printf("Warning: failed to close file: %v\n", err)
56+
}
5257
}()
58+
5359
gw := gitWalker.
5460
Owner(owner).
5561
Repo(repo).

utils/kubernetes/kompose/convert.go

Lines changed: 55 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package kompose
22

33
import (
4+
"fmt"
45
"os"
56
"path/filepath"
67
"strconv"
@@ -42,52 +43,93 @@ func Convert(dockerCompose DockerComposeFile) (string, error) {
4243
tempFilePath := filepath.Join(mesheryDir, "temp.data")
4344
resultFilePath := filepath.Join(mesheryDir, "result.yaml")
4445

46+
// Create the temp file
4547
if err := utils.CreateFile(dockerCompose, "temp.data", mesheryDir); err != nil {
4648
return "", ErrCvrtKompose(err)
4749
}
4850

49-
defer func() {
50-
os.Remove(tempFilePath)
51-
os.Remove(resultFilePath)
52-
}()
51+
// Format Docker Compose file for Kompose
52+
err = formatComposeFile(&dockerCompose)
53+
if err != nil {
54+
return "", ErrCvrtKompose(err)
55+
}
5356

54-
formatComposeFile(&dockerCompose)
57+
// Check version compatibility
5558
err = versionCheck(dockerCompose)
5659
if err != nil {
5760
return "", ErrCvrtKompose(err)
5861
}
5962

63+
// Initialize Kompose client
6064
komposeClient, err := client.NewClient()
6165
if err != nil {
6266
return "", ErrCvrtKompose(err)
6367
}
6468

69+
// Set up Convert options
6570
ConvertOpt := client.ConvertOptions{
6671
InputFiles: []string{tempFilePath},
6772
OutFile: resultFilePath,
6873
GenerateNetworkPolicies: true,
6974
}
7075

76+
// Convert using Kompose client
7177
_, err = komposeClient.Convert(ConvertOpt)
7278
if err != nil {
7379
return "", ErrCvrtKompose(err)
7480
}
7581

82+
// Read the result file
7683
result, err := os.ReadFile(resultFilePath)
7784
if err != nil {
7885
return "", ErrCvrtKompose(err)
7986
}
8087

88+
// Clean up temporary files
89+
cleanupErr := cleanup(tempFilePath, resultFilePath)
90+
if cleanupErr != nil {
91+
return "", cleanupErr
92+
}
93+
8194
return string(result), nil
8295
}
8396

84-
type composeFile struct {
85-
Version string `yaml:"version,omitempty"`
97+
// cleanup removes temporary files
98+
func cleanup(tempFilePath, resultFilePath string) error {
99+
// Try to remove tempFilePath
100+
if err := os.Remove(tempFilePath); err != nil {
101+
return fmt.Errorf("failed to remove temp file %s: %w", tempFilePath, err)
102+
}
103+
104+
// Try to remove resultFilePath
105+
if err := os.Remove(resultFilePath); err != nil {
106+
return fmt.Errorf("failed to remove result file %s: %w", resultFilePath, err)
107+
}
108+
109+
return nil // No errors
110+
}
111+
112+
// formatComposeFile takes in a pointer to the compose file byte array and formats it
113+
// so that it is compatible with Kompose. It expects a validated Docker Compose file.
114+
func formatComposeFile(yamlManifest *DockerComposeFile) error {
115+
data := composeFile{}
116+
err := yaml.Unmarshal(*yamlManifest, &data)
117+
if err != nil {
118+
return fmt.Errorf("failed to unmarshal compose file: %w", err)
119+
}
120+
121+
// Marshal it again to ensure it is in the correct format for Kompose
122+
out, err := yaml.Marshal(data)
123+
if err != nil {
124+
return fmt.Errorf("failed to marshal compose file: %w", err)
125+
}
126+
127+
*yamlManifest = out
128+
return nil
86129
}
87130

88-
// checks if the version is compatible with `kompose`
89-
// expects a valid docker compose yaml
90-
// error = nil means it is compatible
131+
// versionCheck checks if the version in the Docker Compose file is compatible with Kompose.
132+
// It expects a valid Docker Compose YAML and returns an error if the version is not supported.
91133
func versionCheck(dc DockerComposeFile) error {
92134
cf := composeFile{}
93135
err := yaml.Unmarshal(dc, &cf)
@@ -108,18 +150,7 @@ func versionCheck(dc DockerComposeFile) error {
108150
return nil
109151
}
110152

111-
// formatComposeFile takes in a pointer to the compose file byte array and formats it so that it is compatible with `Kompose`
112-
// it expects a validated docker compose file and does not validate
113-
func formatComposeFile(yamlManifest *DockerComposeFile) {
114-
data := composeFile{}
115-
err := yaml.Unmarshal(*yamlManifest, &data)
116-
if err != nil {
117-
return
118-
}
119-
// so that "3.3" and 3.3 are treated differently by `Kompose`
120-
out, err := yaml.Marshal(data)
121-
if err != nil {
122-
return
123-
}
124-
*yamlManifest = out
153+
// composeFile represents the structure of the Docker Compose file version.
154+
type composeFile struct {
155+
Version string `yaml:"version,omitempty"`
125156
}

utils/unarchive.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"archive/tar"
55
"archive/zip"
66
"compress/gzip"
7+
"fmt"
78
"io"
89
"net/http"
910
"os"
@@ -149,22 +150,28 @@ func ExtractTarGz(path, downloadfilePath string) error {
149150
return ErrExtractTarXZ(err, path)
150151
}
151152
case tar.TypeReg:
153+
// Ensure that the directory for the file exists
152154
_ = os.MkdirAll(filepath.Join(path, filepath.Dir(header.Name)), 0755)
153155
var outFile *os.File
154156
outFile, err = os.Create(filepath.Join(path, header.Name))
155157
if err != nil {
156158
return ErrExtractTarXZ(err, path)
157159
}
158160
if _, err = io.Copy(outFile, tarReader); err != nil {
161+
outFile.Close()
159162
return ErrExtractTarXZ(err, path)
160163
}
161-
outFile.Close()
164+
165+
// Close the file and check for errors
166+
if err := outFile.Close(); err != nil {
167+
return fmt.Errorf("failed to close output file %s: %w", header.Name, err)
168+
}
162169

163170
default:
164171
return ErrExtractTarXZ(err, path)
165172
}
166-
167173
}
174+
168175
return nil
169176
}
170177

utils/utils.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,13 +145,15 @@ func CreateFile(contents []byte, filename string, location string) error {
145145
return err
146146
}
147147

148+
// Write contents to the file
148149
if _, err = fd.Write(contents); err != nil {
149-
fd.Close()
150+
fd.Close() // Close the file before returning the error
150151
return err
151152
}
152153

153-
if err = fd.Close(); err != nil {
154-
return err
154+
// Check error while closing the file
155+
if err := fd.Close(); err != nil {
156+
return fmt.Errorf("failed to close file descriptor: %w", err)
155157
}
156158

157159
return nil

utils/walker/git.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,9 @@ func clonewalk(g *Git) error {
144144
var wg sync.WaitGroup
145145
defer func() {
146146
wg.Wait()
147-
os.RemoveAll(path)
147+
if err := os.RemoveAll(path); err != nil {
148+
fmt.Println(fmt.Errorf("failed to remove path %s: %w", path, err))
149+
}
148150
}()
149151
var err error
150152
cloneOptions := &git.CloneOptions{

0 commit comments

Comments
 (0)