SQL storage driver: transaction and error-contract correctness issues
What happened?
While reading pkg/storage/driver/sql.go I found a cluster of transaction- and error-contract defects in the SQL storage backend (HELM_DRIVER=sql). Filing them together since they're all in one file and some fixes overlap; happy to split if preferred. PRs incoming, smallest first.
1. Create/Delete commit via defer and discard the commit error — silent data loss
Create ends with defer transaction.Commit() and Delete registers the same defer before its DELETE statements run. Consequences:
- If COMMIT fails (connection drop, server error),
Createreturnsnilwhile the release record was never persisted;Deletereturns the release as deleted while nothing was deleted. - In
Delete, error paths after the defer (custom-labels lookup or delete failing) still commit a partial deletion while reporting failure — release row gone, labels orphaned.
2. ErrReleaseExists detection can never work on PostgreSQL
After a failed INSERT inside the transaction, Create runs a SELECT on the same transaction to decide between ErrReleaseExists and the raw error. PostgreSQL aborts a transaction after any statement error ("current transaction is aborted, commands ignored until end of transaction block"), so that SELECT always errors and the ErrReleaseExists branch is unreachable. The SQL backend therefore surfaces a raw pq: duplicate key value… where the Secrets/ConfigMaps drivers return Helm's "release already exists" — a storage-contract break (postgres is the only supported dialect, so this is the only path).
3. Every read error mapped to ErrReleaseNotFound
Get and the existence check in Delete map any DB error (connection refused, RLS permission denied, timeout) to ErrReleaseNotFound, with the real cause only at debug level. Callers make control-flow decisions on that sentinel — e.g. treating a transient DB outage as "release absent". Only sql.ErrNoRows should map to not-found.
4. Invalid SQL in the custom_labels down migration
The Down migration is DELETE TABLE …; — not valid SQL (should be DROP TABLE). Latent today since only migrate.Up runs, but wrong nonetheless.
5. Create/Update mutate the driver's namespace — stateful and racy
s.namespace = namespace in both write paths overwrites the driver instance's namespace with whichever release was last written. Any SDK program sharing one configuration across namespaces gets Get/List/Query scoped to the namespace of the most recent write, and it's a data race under concurrency (List reads s.namespace while Create writes it). The Secrets driver derives scope per call instead.
6. Update never syncs custom labels
Update rewrites body/name/version/status but never touches the custom_labels_v1 table, so label changes on a stored release are silently dropped for the SQL backend (the K8s drivers re-persist labels with the whole object).
7. getReleaseCustomLabels ignores the namespace it is given, so all-namespaces listing returns no custom labels
sql.go:694 declares func (s *SQL) getReleaseCustomLabels(key string, _ string) — the namespace parameter is discarded, and the query filters on s.namespace instead (sql.go:698-699). But List and Query both support all-namespaces mode (if s.namespace != "" { ...filter... }) and call it per row as s.getReleaseCustomLabels(record.Key, record.Namespace), clearly intending per-record scoping. With s.namespace == "" the label lookup becomes WHERE release_namespace = '', which matches nothing, so every release listed across namespaces comes back with empty custom labels on the SQL backend. Honouring the argument already passed at both call sites is the fix; it overlaps heavily with item 5, so the two are best done together.
What did you expect to happen?
Commit errors surface to the caller; duplicate creates return ErrReleaseExists like the other drivers; only "no rows" maps to ErrReleaseNotFound; valid SQL in migrations; per-operation namespace scoping; label updates persisted.
How can we reproduce it (as minimally and precisely as possible)?
Each item is visible from the code paths cited above; the commit-handling one is reproducible by killing the DB connection between the statements and COMMIT (or via sqlmock, as the regression tests in the first PR do).
Helm version
$ helm version
main @ ccb8f59Kubernetes version
Not applicable — SQL storage backend, no cluster involved.
Source: helm/helm