msb-public
/
helm
mirror of https://github.com/helm/helm
1
0
Code Issues Projects Releases Wiki Activity

Merge 11717c94c6 into 8a9993eff0

Browse Source
pull/32471/merge
Gates Wang 3 days ago committed by GitHub
parent 8a9993eff0 11717c94c6
commit a075b6cd7e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 179 additions and 13 deletions
Show Stats Download Patch File Download Diff File

36
pkg/storage/driver/sql.go
Unescape View File

@ -17,6 +17,7 @@ limitations under the License.
package driver
import (
"context"
"fmt"
"log/slog"
"maps"
@ -488,6 +489,7 @@ func (s *SQL) Create(key string, rel release.Releaser) error {
s.Logger().Debug("failed to start SQL transaction", slog.Any("error", err))
return fmt.Errorf("error beginning transaction: %w", err)
}
defer transaction.Rollback()
insertQuery, args, err := s.statementBuilder.
Insert(sqlReleaseTableName).
@ -518,9 +520,7 @@ func (s *SQL) Create(key string, rel release.Releaser) error {
return err
}
if _, err := transaction.Exec(insertQuery, args...); err != nil {
defer transaction.Rollback()
if _, err := transaction.ExecContext(context.Background(), insertQuery, args...); err != nil {
selectQuery, args, buildErr := s.statementBuilder.
Select(sqlReleaseTableKeyColumn).
From(sqlReleaseTableName).
@ -559,18 +559,20 @@ func (s *SQL) Create(key string, rel release.Releaser) error {
v,
).ToSql()
if err != nil {
defer 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 transaction.Rollback()
if _, err := transaction.ExecContext(context.Background(), insertLabelsQuery, args...); err != nil {
s.Logger().Debug("failed to write Labels", slog.Any("error", err))
return err
}
}
defer transaction.Commit()
if err := transaction.Commit(); err != nil {
s.Logger().Debug("failed to commit release creation transaction", slog.Any("error", err))
return fmt.Errorf("error committing transaction: %w", err)
}
return nil
}
@ -623,7 +625,7 @@ func (s *SQL) Update(key string, rel release.Releaser) error {
return err
}
if _, err := s.db.Exec(query, args...); err != nil {
if _, err := s.db.ExecContext(context.Background(), query, args...); err != nil {
s.Logger().Debug("failed to update release in SQL database", slog.String("key", key), slog.Any("error", err))
return err
}
@ -638,6 +640,7 @@ 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)
}
defer transaction.Rollback()
selectQuery, args, err := s.statementBuilder.
Select(sqlReleaseTableBodyColumn).
@ -660,10 +663,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 transaction.Commit()
deleteQuery, args, err := s.statementBuilder.
Delete(sqlReleaseTableName).
@ -675,7 +676,7 @@ func (s *SQL) Delete(key string) (release.Releaser, error) {
return nil, err
}
_, err = transaction.Exec(deleteQuery, args...)
_, err = transaction.ExecContext(context.Background(), deleteQuery, args...)
if err != nil {
s.Logger().Debug("failed perform delete query", slog.Any("error", err))
return release, err
@ -699,8 +700,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.ExecContext(context.Background(), deleteCustomLabelsQuery, args...); err != nil {
s.Logger().Debug("failed to delete release custom labels", slog.String("key", key), slog.Any("error", err))
return release, err
}
if err := transaction.Commit(); err != nil {
s.Logger().Debug("failed to commit release deletion transaction", slog.Any("error", err))
return release, fmt.Errorf("error committing transaction: %w", err)
}
return release, nil
}
// Get release custom labels from database

156
pkg/storage/driver/sql_test.go
Unescape View File

@ -14,6 +14,7 @@ limitations under the License.
package driver
import (
"database/sql"
"database/sql/driver"
"errors"
"fmt"
@ -561,3 +562,158 @@ func TestSqlCheckAppliedMigrations(t *testing.T) {
assert.Equal(t, c.expectedResult, sqlDriver.checkAlreadyApplied(c.migrationsToApply), "Test case: %v, Expected: %v, Have: %v, Explanation: %v", i, c.expectedResult, !c.expectedResult, c.errorExplanation)
}
}
// TestSqlCreateCommitError verifies that a commit-time failure is surfaced to
// the caller. A deferred, error-discarding Commit would report success even
// though nothing was persisted.
func TestSqlCreateCommitError(t *testing.T) {
vers := 1
name := "smug-pigeon"
namespace := "default"
key := testKey(name, vers)
rel := releaseStub(name, vers, namespace, common.StatusDeployed)
sqlDriver, mock := newTestFixtureSQL(t)
body, _ := encodeRelease(rel)
query := fmt.Sprintf(
"INSERT INTO %s (%s,%s,%s,%s,%s,%s,%s,%s,%s) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9)",
sqlReleaseTableName,
sqlReleaseTableKeyColumn,
sqlReleaseTableTypeColumn,
sqlReleaseTableBodyColumn,
sqlReleaseTableNameColumn,
sqlReleaseTableNamespaceColumn,
sqlReleaseTableVersionColumn,
sqlReleaseTableStatusColumn,
sqlReleaseTableOwnerColumn,
sqlReleaseTableCreatedAtColumn,
)
mock.ExpectBegin()
mock.
ExpectExec(regexp.QuoteMeta(query)).
WithArgs(
key,
sqlReleaseDefaultType,
body,
rel.Name,
rel.Namespace,
int(rel.Version),
rel.Info.Status.String(),
sqlReleaseDefaultOwner,
recentUnixTimestamp(),
).
WillReturnResult(sqlmock.NewResult(1, 1))
labelsQuery := fmt.Sprintf(
"INSERT INTO %s (%s,%s,%s,%s) VALUES ($1,$2,$3,$4)",
sqlCustomLabelsTableName,
sqlCustomLabelsTableReleaseKeyColumn,
sqlCustomLabelsTableReleaseNamespaceColumn,
sqlCustomLabelsTableKeyColumn,
sqlCustomLabelsTableValueColumn,
)
mock.MatchExpectationsInOrder(false)
for k, v := range filterSystemLabels(rel.Labels) {
mock.
ExpectExec(regexp.QuoteMeta(labelsQuery)).
WithArgs(key, rel.Namespace, k, v).
WillReturnResult(sqlmock.NewResult(1, 1))
}
mock.ExpectCommit().WillReturnError(errors.New("transaction commit failed"))
err := sqlDriver.Create(key, rel)
require.Error(t, err, "expected Create to surface the commit error, got nil")
assert.NoErrorf(t, mock.ExpectationsWereMet(), "sql expectations weren't met")
}
// TestSqlDeleteCommitError verifies that a commit-time failure during Delete is
// surfaced to the caller instead of being silently discarded.
func TestSqlDeleteCommitError(t *testing.T) {
vers := 1
name := "smug-pigeon"
namespace := "default"
key := testKey(name, vers)
rel := releaseStub(name, vers, namespace, common.StatusDeployed)
body, _ := encodeRelease(rel)
sqlDriver, mock := newTestFixtureSQL(t)
selectQuery := fmt.Sprintf(
"SELECT %s FROM %s WHERE %s = $1 AND %s = $2",
sqlReleaseTableBodyColumn,
sqlReleaseTableName,
sqlReleaseTableKeyColumn,
sqlReleaseTableNamespaceColumn,
)
mock.ExpectBegin()
mock.
ExpectQuery(regexp.QuoteMeta(selectQuery)).
WithArgs(key, namespace).
WillReturnRows(
mock.NewRows([]string{sqlReleaseTableBodyColumn}).AddRow(body),
).RowsWillBeClosed()
deleteQuery := fmt.Sprintf(
"DELETE FROM %s WHERE %s = $1 AND %s = $2",
sqlReleaseTableName,
sqlReleaseTableKeyColumn,
sqlReleaseTableNamespaceColumn,
)
mock.
ExpectExec(regexp.QuoteMeta(deleteQuery)).
WithArgs(key, namespace).
WillReturnResult(sqlmock.NewResult(0, 1))
mockGetReleaseCustomLabels(mock, key, namespace, rel.Labels)
deleteLabelsQuery := fmt.Sprintf(
"DELETE FROM %s WHERE %s = $1 AND %s = $2",
sqlCustomLabelsTableName,
sqlCustomLabelsTableReleaseKeyColumn,
sqlCustomLabelsTableReleaseNamespaceColumn,
)
mock.
ExpectExec(regexp.QuoteMeta(deleteLabelsQuery)).
WithArgs(key, namespace).
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectCommit().WillReturnError(errors.New("transaction commit failed"))
_, err := sqlDriver.Delete(key)
require.Error(t, err, "expected Delete to surface the commit error, got nil")
assert.NoErrorf(t, mock.ExpectationsWereMet(), "sql expectations weren't met")
}
// TestSqlDeleteNotFoundReleasesTransaction verifies that the not-found path
// rolls back the transaction rather than leaking its pooled connection.
func TestSqlDeleteNotFoundReleasesTransaction(t *testing.T) {
vers := 1
name := "smug-pigeon"
namespace := "default"
key := testKey(name, vers)
sqlDriver, mock := newTestFixtureSQL(t)
selectQuery := fmt.Sprintf(
"SELECT %s FROM %s WHERE %s = $1 AND %s = $2",
sqlReleaseTableBodyColumn,
sqlReleaseTableName,
sqlReleaseTableKeyColumn,
sqlReleaseTableNamespaceColumn,
)
mock.ExpectBegin()
mock.
ExpectQuery(regexp.QuoteMeta(selectQuery)).
WithArgs(key, namespace).
WillReturnError(sql.ErrNoRows)
mock.ExpectRollback()
_, err := sqlDriver.Delete(key)
require.ErrorIs(t, err, ErrReleaseNotFound)
assert.NoErrorf(t, mock.ExpectationsWereMet(), "sql expectations weren't met")
}

Write Preview
Loading…
Cancel
Save