fix(storage/sql): honor namespace argument in getReleaseCustomLabels

getReleaseCustomLabels took a namespace parameter but discarded it,
querying with s.namespace instead. In all-namespaces mode (s.namespace
== ""), this meant the custom-labels lookup matched nothing, so every
release listed or queried across namespaces came back with empty
custom labels.

Also fixed TestSQLList and TestSqlQuery, whose mocked rows only
returned a body column, leaving record.Key/record.Namespace as empty
strings during scanning - a gap the old buggy getReleaseCustomLabels
masked by ignoring its namespace argument entirely. Both tests now
mock the full key/namespace/body row shape and assert against it.

Fixes item 7 of #32394, reported by @Mukuwul during review of this PR.

Signed-off-by: ahmedfawzy21 <ahmed.fawzy21@gmail.com>
pull/32492/head
ahmedfawzy21 17 hours ago
parent 10782cacf6
commit 95eb1a66db

@ -254,6 +254,13 @@ func (mock *MockSecretsInterface) Delete(_ context.Context, name string, _ metav
// newTestFixtureSQL mocks the SQL database (for testing purposes)
func newTestFixtureSQL(t *testing.T, _ ...*rspb.Release) (*SQL, sqlmock.Sqlmock) {
t.Helper()
return newTestFixtureSQLWithNamespace(t, "default")
}
// newTestFixtureSQLWithNamespace is like newTestFixtureSQL but lets the caller configure the
// driver's namespace - in particular, pass "" to simulate all-namespaces mode.
func newTestFixtureSQLWithNamespace(t *testing.T, namespace string) (*SQL, sqlmock.Sqlmock) {
t.Helper()
sqlDB, mock, err := sqlmock.New()
require.NoError(t, err, "error when opening stub database connection")
@ -261,7 +268,7 @@ func newTestFixtureSQL(t *testing.T, _ ...*rspb.Release) (*SQL, sqlmock.Sqlmock)
sqlxDB := sqlx.NewDb(sqlDB, "sqlmock")
return &SQL{
db: sqlxDB,
namespace: "default",
namespace: namespace,
statementBuilder: sq.StatementBuilder.PlaceholderFormat(sq.Dollar),
}, mock
}

@ -688,13 +688,13 @@ func (s *SQL) Delete(key string) (release.Releaser, error) {
}
// Get release custom labels from database
func (s *SQL) getReleaseCustomLabels(key string, _ string) (map[string]string, error) {
func (s *SQL) getReleaseCustomLabels(key string, namespace string) (map[string]string, error) {
query, args, err := s.statementBuilder.
Select(sqlCustomLabelsTableKeyColumn, sqlCustomLabelsTableValueColumn).
From(sqlCustomLabelsTableName).
Where(sq.Eq{
sqlCustomLabelsTableReleaseKeyColumn: key,
sqlCustomLabelsTableReleaseNamespaceColumn: s.namespace,
sqlCustomLabelsTableReleaseNamespaceColumn: namespace,
}).
ToSql()
if err != nil {

@ -128,11 +128,13 @@ func TestSQLList(t *testing.T) {
)
rows := mock.NewRows([]string{
sqlReleaseTableKeyColumn,
sqlReleaseTableNamespaceColumn,
sqlReleaseTableBodyColumn,
})
for _, r := range releases {
body, _ := encodeRelease(r)
rows.AddRow(body)
rows.AddRow(testKey(r.Name, r.Version), r.Namespace, body)
}
mock.
ExpectQuery(regexp.QuoteMeta(query)).
@ -140,7 +142,7 @@ func TestSQLList(t *testing.T) {
WillReturnRows(rows).RowsWillBeClosed()
for _, r := range releases {
mockGetReleaseCustomLabels(mock, "", r.Namespace, r.Labels)
mockGetReleaseCustomLabels(mock, testKey(r.Name, r.Version), r.Namespace, r.Labels)
}
}
@ -178,6 +180,53 @@ func TestSQLList(t *testing.T) {
require.Contains(t, rls.Labels, "key1", "Expected 'key1' label in results, actual %v", rls.Labels)
}
// TestSqlListAllNamespacesReturnsCustomLabels is a regression test for helm/helm#32394 (item 7):
// getReleaseCustomLabels ignored the namespace argument passed to it by List/Query and used
// s.namespace instead. In all-namespaces mode (s.namespace == ""), this meant the custom-labels
// query filtered on an empty namespace and matched nothing, so every release listed across
// namespaces silently came back with empty custom labels.
func TestSqlListAllNamespacesReturnsCustomLabels(t *testing.T) {
sqlDriver, mock := newTestFixtureSQLWithNamespace(t, "")
rel := releaseStub("smug-pigeon", 1, "team-a", common.StatusDeployed)
key := testKey(rel.Name, 1)
body, _ := encodeRelease(rel)
listQuery := fmt.Sprintf(
"SELECT %s, %s, %s FROM %s WHERE %s = $1",
sqlReleaseTableKeyColumn,
sqlReleaseTableNamespaceColumn,
sqlReleaseTableBodyColumn,
sqlReleaseTableName,
sqlReleaseTableOwnerColumn,
)
mock.
ExpectQuery(regexp.QuoteMeta(listQuery)).
WithArgs(sqlReleaseDefaultOwner).
WillReturnRows(
mock.NewRows([]string{
sqlReleaseTableKeyColumn,
sqlReleaseTableNamespaceColumn,
sqlReleaseTableBodyColumn,
}).AddRow(key, rel.Namespace, body),
)
// The custom-labels lookup must be scoped to this release's own namespace ("team-a"),
// not the driver's all-namespaces configuration (""). mockGetReleaseCustomLabels asserts
// the query args match exactly, so this expectation fails if the fix regresses.
mockGetReleaseCustomLabels(mock, key, rel.Namespace, rel.Labels)
releases, err := sqlDriver.List(func(release.Releaser) bool { return true })
require.NoError(t, err)
require.Len(t, releases, 1)
got := convertReleaserToV1(t, releases[0])
for k, v := range filterSystemLabels(rel.Labels) {
assert.Equal(t, v, got.Labels[k], "custom label %q should be present when listing across all namespaces", k)
}
assert.NoErrorf(t, mock.ExpectationsWereMet(), "sql expectations weren't met")
}
func TestSqlCreate(t *testing.T) {
vers := 1
name := "smug-pigeon"
@ -540,6 +589,8 @@ func TestSqlQuery(t *testing.T) {
WithArgs("smug-pigeon", sqlReleaseDefaultOwner, "unknown", "default").
WillReturnRows(
mock.NewRows([]string{
sqlReleaseTableKeyColumn,
sqlReleaseTableNamespaceColumn,
sqlReleaseTableBodyColumn,
}),
).RowsWillBeClosed()
@ -549,13 +600,15 @@ func TestSqlQuery(t *testing.T) {
WithArgs("smug-pigeon", sqlReleaseDefaultOwner, "deployed", "default").
WillReturnRows(
mock.NewRows([]string{
sqlReleaseTableKeyColumn,
sqlReleaseTableNamespaceColumn,
sqlReleaseTableBodyColumn,
}).AddRow(
deployedReleaseBody,
testKey(deployedRelease.Name, deployedRelease.Version), deployedRelease.Namespace, deployedReleaseBody,
),
).RowsWillBeClosed()
mockGetReleaseCustomLabels(mock, "", deployedRelease.Namespace, deployedRelease.Labels)
mockGetReleaseCustomLabels(mock, testKey(deployedRelease.Name, deployedRelease.Version), deployedRelease.Namespace, deployedRelease.Labels)
query = fmt.Sprintf(
"SELECT %s, %s, %s FROM %s WHERE %s = $1 AND %s = $2 AND %s = $3",
@ -573,16 +626,18 @@ func TestSqlQuery(t *testing.T) {
WithArgs("smug-pigeon", sqlReleaseDefaultOwner, "default").
WillReturnRows(
mock.NewRows([]string{
sqlReleaseTableKeyColumn,
sqlReleaseTableNamespaceColumn,
sqlReleaseTableBodyColumn,
}).AddRow(
supersededReleaseBody,
testKey(supersededRelease.Name, supersededRelease.Version), supersededRelease.Namespace, supersededReleaseBody,
).AddRow(
deployedReleaseBody,
testKey(deployedRelease.Name, deployedRelease.Version), deployedRelease.Namespace, deployedReleaseBody,
),
).RowsWillBeClosed()
mockGetReleaseCustomLabels(mock, "", supersededRelease.Namespace, supersededRelease.Labels)
mockGetReleaseCustomLabels(mock, "", deployedRelease.Namespace, deployedRelease.Labels)
mockGetReleaseCustomLabels(mock, testKey(supersededRelease.Name, supersededRelease.Version), supersededRelease.Namespace, supersededRelease.Labels)
mockGetReleaseCustomLabels(mock, testKey(deployedRelease.Name, deployedRelease.Version), deployedRelease.Namespace, deployedRelease.Labels)
_, err := sqlDriver.Query(labelSetUnknown)
require.Errorf(t, err, "Expected error {%v}, got nil", ErrReleaseNotFound)

Loading…
Cancel
Save