Skip to content

Commit eefc216

Browse files
committed
fix: switched to meshkit loggers and addressed other review comments
Signed-off-by: SHIGRAF SALIK <[email protected]>
1 parent 1f71e83 commit eefc216

File tree

3 files changed

+39
-23
lines changed

3 files changed

+39
-23
lines changed

generators/github/git_repo.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,28 @@ import (
99
"strings"
1010

1111
"github.com/meshery/meshkit/generators/models"
12+
"github.com/meshery/meshkit/logger"
1213
"github.com/meshery/meshkit/utils"
1314
"github.com/meshery/meshkit/utils/helm"
1415
"github.com/meshery/meshkit/utils/walker"
1516
)
1617

1718
type GitRepo struct {
1819
// <git://github.com/owner/repo/branch/versiontag/root(path to the directory/file)>
19-
URL *url.URL
20+
URL *url.URL
2021
PackageName string
2122
}
2223

2324
// Assumpations: 1. Always a K8s manifest
2425
// 2. Unzipped/unarchived File type
2526

2627
func (gr GitRepo) GetContent() (models.Package, error) {
28+
log, err := logger.New("generators-github", logger.Options{})
29+
if err != nil {
30+
// If logger fails to initialize, we can't proceed without logging capabilities.
31+
// Alternatively, one could fall back to fmt.Println and continue.
32+
return nil, err
33+
}
2734
gitWalker := walker.NewGit()
2835

2936
owner, repo, branch, root, err := gr.extractRepoDetailsFromSourceURL()
@@ -48,11 +55,11 @@ func (gr GitRepo) GetContent() (models.Package, error) {
4855

4956
defer func() {
5057
if err := br.Flush(); err != nil {
51-
fmt.Printf("Warning: failed to flush writer: %v\n", err)
58+
log.Warnf("failed to flush writer: %v", err)
5259
}
5360

5461
if err := fd.Close(); err != nil {
55-
fmt.Printf("Warning: failed to close file: %v\n", err)
62+
log.Warnf("failed to close file: %v", err)
5663
}
5764
}()
5865

utils/unarchive.go

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -73,24 +73,23 @@ func readData(name string) (buffer []byte, err error) {
7373

7474
func ExtractZip(path, artifactPath string) error {
7575
zipReader, err := zip.OpenReader(artifactPath)
76+
if err != nil {
77+
return ErrExtractZip(err, path)
78+
}
7679
defer func() {
7780
_ = zipReader.Close()
7881
}()
7982

80-
if err != nil {
81-
return ErrExtractZip(err, path)
82-
}
8383
buffer := make([]byte, 1<<4)
8484
for _, file := range zipReader.File {
8585

8686
fd, err := file.Open()
87-
defer func() {
88-
_ = fd.Close()
89-
}()
90-
9187
if err != nil {
9288
return ErrExtractZip(err, path)
9389
}
90+
defer func() {
91+
_ = fd.Close()
92+
}()
9493

9594
filePath := filepath.Join(path, file.Name)
9695

@@ -106,31 +105,30 @@ func ExtractZip(path, artifactPath string) error {
106105
}
107106
_, err = io.CopyBuffer(openedFile, fd, buffer)
108107
if err != nil {
108+
// We need to close the file, but the error from CopyBuffer is more important.
109+
_ = openedFile.Close()
110+
return ErrExtractZip(err, path)
111+
}
112+
if err := openedFile.Close(); err != nil {
109113
return ErrExtractZip(err, path)
110114
}
111-
defer func() {
112-
_ = openedFile.Close()
113-
}()
114115
}
115-
116116
}
117117
return nil
118-
119118
}
120119

121120
func ExtractTarGz(path, downloadfilePath string) error {
122121
gzipStream, err := os.Open(downloadfilePath)
123122
if err != nil {
124123
return ErrReadFile(err, downloadfilePath)
125124
}
125+
defer gzipStream.Close()
126+
126127
uncompressedStream, err := gzip.NewReader(gzipStream)
127128
if err != nil {
128129
return ErrExtractTarXZ(err, path)
129130
}
130-
defer func() {
131-
_ = uncompressedStream.Close()
132-
_ = gzipStream.Close()
133-
}()
131+
defer uncompressedStream.Close()
134132

135133
tarReader := tar.NewReader(uncompressedStream)
136134

@@ -158,23 +156,35 @@ func ExtractTarGz(path, downloadfilePath string) error {
158156
return ErrExtractTarXZ(err, path)
159157
}
160158
if _, err = io.Copy(outFile, tarReader); err != nil {
161-
outFile.Close()
159+
// The subsequent Close() call will handle closing the file.
160+
// Return the more critical I/O error immediately.
161+
_ = outFile.Close() // Attempt to close, but ignore error as we are returning the copy error.
162162
return ErrExtractTarXZ(err, path)
163163
}
164164

165165
// Close the file and check for errors
166166
if err := outFile.Close(); err != nil {
167-
return fmt.Errorf("failed to close output file %s: %w", header.Name, err)
167+
return ErrFileClose(err, header.Name)
168168
}
169169

170170
default:
171-
return ErrExtractTarXZ(err, path)
171+
return ErrUnsupportedTarHeaderType(header.Typeflag, header.Name)
172172
}
173173
}
174174

175175
return nil
176176
}
177177

178+
// ErrFileClose creates a standardized error for file closing failures.
179+
func ErrFileClose(err error, filename string) error {
180+
return fmt.Errorf("failed to close file %s: %w", filename, err)
181+
}
182+
183+
// ErrUnsupportedTarHeaderType creates an error for unsupported tar header types.
184+
func ErrUnsupportedTarHeaderType(typeflag byte, name string) error {
185+
return fmt.Errorf("unsupported tar header type %v for file %s", typeflag, name)
186+
}
187+
178188
func ProcessContent(filePath string, f func(path string) error) error {
179189
pathInfo, err := os.Stat(filePath)
180190
if err != nil {

utils/utils.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ func CreateFile(contents []byte, filename string, location string) error {
145145
return err
146146
}
147147

148-
// Write contents to the file
149148
if _, err = fd.Write(contents); err != nil {
150149
fd.Close() // Close the file before returning the error
151150
return err

0 commit comments

Comments
 (0)