diff --git a/internal/chart/v3/lint/rules/template_test.go b/internal/chart/v3/lint/rules/template_test.go index bf018137b..754a9e211 100644 --- a/internal/chart/v3/lint/rules/template_test.go +++ b/internal/chart/v3/lint/rules/template_test.go @@ -71,7 +71,9 @@ var ( func TestTemplateIntegrationHappyPath(t *testing.T) { // Rename file so it gets ignored by the linter require.NoError(t, os.Rename(wrongTemplatePath, ignoredTemplatePath)) - defer func() { _ = os.Rename(ignoredTemplatePath, wrongTemplatePath) }() + defer func() { + assert.NoError(t, os.Rename(ignoredTemplatePath, wrongTemplatePath)) + }() linter := support.Linter{ChartDir: templateTestBasedir} Templates(&linter, values, namespace, strict) diff --git a/pkg/chart/loader/archive/archive.go b/pkg/chart/loader/archive/archive.go index fa2639020..0bb78eb80 100644 --- a/pkg/chart/loader/archive/archive.go +++ b/pkg/chart/loader/archive/archive.go @@ -165,15 +165,20 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) { // Sometimes users will provide a values.yaml for an argument where a chart is expected. One common occurrence // of this is invoking `helm template values.yaml mychart` which would otherwise produce a confusing error // if we didn't check for this. -func EnsureArchive(name string, raw *os.File) error { +func EnsureArchive(name string, raw *os.File) (err error) { defer func() { - _, _ = raw.Seek(0, 0) // reset read offset to allow archive loading to proceed. + // Reset the read offset so archive loading can proceed. If the rewind + // fails and the check itself succeeded, surface the seek error rather + // than letting callers hit confusing follow-on errors. + if _, seekErr := raw.Seek(0, 0); seekErr != nil && err == nil { + err = fmt.Errorf("file '%s' cannot be reset: %w", name, seekErr) + } }() // Check the file format to give us a chance to provide the user with more actionable feedback. buffer := make([]byte, 512) - _, err := raw.Read(buffer) - if err != nil && err != io.EOF { + _, err = raw.Read(buffer) + if err != nil && !errors.Is(err, io.EOF) { return fmt.Errorf("file '%s' cannot be read: %w", name, err) } diff --git a/pkg/chart/v2/lint/rules/template_test.go b/pkg/chart/v2/lint/rules/template_test.go index c76ee8470..985f11a5d 100644 --- a/pkg/chart/v2/lint/rules/template_test.go +++ b/pkg/chart/v2/lint/rules/template_test.go @@ -72,7 +72,9 @@ var ( func TestTemplateIntegrationHappyPath(t *testing.T) { // Rename file so it gets ignored by the linter require.NoError(t, os.Rename(wrongTemplatePath, ignoredTemplatePath)) - defer func() { _ = os.Rename(ignoredTemplatePath, wrongTemplatePath) }() + defer func() { + assert.NoError(t, os.Rename(ignoredTemplatePath, wrongTemplatePath)) + }() linter := support.Linter{ChartDir: templateTestBasedir} Templates( diff --git a/pkg/repo/v1/repotest/server.go b/pkg/repo/v1/repotest/server.go index 4177fcfd0..30c8bb960 100644 --- a/pkg/repo/v1/repotest/server.go +++ b/pkg/repo/v1/repotest/server.go @@ -17,6 +17,7 @@ package repotest import ( "crypto/tls" + "errors" "fmt" "net" "net/http" @@ -215,7 +216,9 @@ func (srv *OCIServer) RunWithReturn(t *testing.T, opts ...OCIServerOpt) *OCIServ } go func() { - _ = srv.ListenAndServe() + if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { + t.Errorf("OCI test registry server failed: %v", err) + } }() credentialsFile := filepath.Join(srv.Dir, "config.json") diff --git a/pkg/storage/driver/sql.go b/pkg/storage/driver/sql.go index e057570b0..9e675f472 100644 --- a/pkg/storage/driver/sql.go +++ b/pkg/storage/driver/sql.go @@ -489,6 +489,8 @@ func (s *SQL) Create(key string, rel release.Releaser) error { return fmt.Errorf("error beginning transaction: %w", err) } + defer func() { _ = transaction.Rollback() }() + insertQuery, args, err := s.statementBuilder. Insert(sqlReleaseTableName). Columns( @@ -519,8 +521,6 @@ func (s *SQL) Create(key string, rel release.Releaser) error { } if _, err := transaction.Exec(insertQuery, args...); err != nil { - defer func() { _ = transaction.Rollback() }() - selectQuery, args, buildErr := s.statementBuilder. Select(sqlReleaseTableKeyColumn). From(sqlReleaseTableName). @@ -559,20 +559,17 @@ func (s *SQL) Create(key string, rel release.Releaser) error { v, ).ToSql() if err != nil { - defer func() { _ = transaction.Rollback() }() s.Logger().Debug("failed to build insert query", slog.Any("error", err)) return err } if _, err := transaction.Exec(insertLabelsQuery, args...); err != nil { - defer func() { _ = transaction.Rollback() }() s.Logger().Debug("failed to write Labels", slog.Any("error", err)) return err } } - defer func() { _ = transaction.Commit() }() - return nil + return transaction.Commit() } // Update updates a release. @@ -624,6 +621,9 @@ func (s *SQL) Delete(key string) (release.Releaser, error) { s.Logger().Debug("failed to start SQL transaction", slog.Any("error", err)) return nil, fmt.Errorf("error beginning transaction: %w", err) } + // Roll back the transaction on any early return. After a successful + // Commit this is a no-op that returns sql.ErrTxDone, which we ignore. + defer func() { _ = transaction.Rollback() }() selectQuery, args, err := s.statementBuilder. Select(sqlReleaseTableBodyColumn). @@ -646,10 +646,8 @@ func (s *SQL) Delete(key string) (release.Releaser, error) { release, err := decodeRelease(record.Body) if err != nil { s.Logger().Debug("failed to decode release", slog.String("key", key), slog.Any("error", err)) - _ = transaction.Rollback() return nil, err } - defer func() { _ = transaction.Commit() }() deleteQuery, args, err := s.statementBuilder. Delete(sqlReleaseTableName). @@ -664,7 +662,7 @@ func (s *SQL) Delete(key string) (release.Releaser, error) { _, err = transaction.Exec(deleteQuery, args...) if err != nil { s.Logger().Debug("failed perform delete query", slog.Any("error", err)) - return release, err + return nil, err } if release.Labels, err = s.getReleaseCustomLabels(key, s.namespace); err != nil { @@ -685,8 +683,17 @@ func (s *SQL) Delete(key string) (release.Releaser, error) { s.Logger().Debug("failed to build delete Labels query", slog.Any("error", err)) return nil, err } - _, err = transaction.Exec(deleteCustomLabelsQuery, args...) - return release, err + if _, err = transaction.Exec(deleteCustomLabelsQuery, args...); err != nil { + s.Logger().Debug("failed to perform delete Labels query", slog.Any("error", err)) + return nil, err + } + + if err := transaction.Commit(); err != nil { + s.Logger().Debug("failed to commit delete transaction", slog.Any("error", err)) + return nil, err + } + + return release, nil } // Get release custom labels from database