From 51d74ae9a612341322337b9c274f1f88013a7021 Mon Sep 17 00:00:00 2001 From: Diogo Nogueira Date: Mon, 21 Oct 2024 10:18:22 +0100 Subject: [PATCH] fix(db): fixed select query that ignored eslversions with deleted flag (#2053) In the DeleteEnvFromApp a query was made which selected the most recent eslversions for each release of the app. This query ignored rows that has the deleted flag set to true. When the transformer later updated each release if the lated eslversion was deleted then an error would occur. This query was fixed and, in the process, altered to be more efficient. Ref: SRX-9MM6SC --- pkg/db/db.go | 48 +++++++++++++++---- pkg/db/db_test.go | 31 +++++++++++- .../cd-service/pkg/repository/transformer.go | 4 +- .../pkg/repository/transformer_db_test.go | 7 +-- tests/integration-tests/release_test.go | 2 +- 5 files changed, 75 insertions(+), 17 deletions(-) diff --git a/pkg/db/db.go b/pkg/db/db.go index 734723c04..e2246b9a2 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -789,27 +789,55 @@ func (h *DBHandler) processReleaseManifestRows(ctx context.Context, err error, r return result, nil } -func (h *DBHandler) DBSelectReleasesByApp(ctx context.Context, tx *sql.Tx, app string, deleted bool, ignorePrepublishes bool) ([]*DBReleaseWithMetaData, error) { - span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByApp") +// DBSelectReleasesByAppLatestEslVersion returns the latest eslversion +// for each release of an app. It includes deleted releases and loads manifests. +func (h *DBHandler) DBSelectReleasesByAppLatestEslVersion(ctx context.Context, tx *sql.Tx, app string, ignorePrepublishes bool) ([]*DBReleaseWithMetaData, error) { + span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByAppLatestEslVersion") defer span.Finish() - selectQuery := h.AdaptQuery(fmt.Sprintf( - "SELECT eslVersion, created, appName, metadata, manifests, releaseVersion, deleted, environments " + - " FROM releases " + - " WHERE appName=? AND deleted=?" + - " ORDER BY releaseVersion DESC, eslVersion DESC, created DESC;")) + selectQuery := h.AdaptQuery( + `SELECT + releases.eslVersion, + releases.created, + releases.appName, + releases.metadata, + releases.manifests, + releases.releaseVersion, + releases.deleted, + releases.environments + FROM ( + SELECT + MAX(eslVersion) AS latestEslVersion, + appname, + releaseversion + FROM + releases + WHERE + appname=? + GROUP BY + appname, releaseversion + ) as currentEslReleases + JOIN + releases + ON + currentEslReleases.appname = releases.appname + AND + currentEslReleases.latesteslversion = releases.eslversion + AND + currentEslReleases.releaseversion = releases.releaseversion + ORDER BY currentEslReleases.releaseversion DESC;`, + ) span.SetTag("query", selectQuery) rows, err := tx.QueryContext( ctx, selectQuery, app, - deleted, ) return h.processReleaseRows(ctx, err, rows, ignorePrepublishes, true) } -func (h *DBHandler) DBSelectReleasesByAppLatestEslVersion(ctx context.Context, tx *sql.Tx, app string, deleted bool, ignorePrepublishes bool) ([]*DBReleaseWithMetaData, error) { - span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByApp") +func (h *DBHandler) DBSelectReleasesByAppOrderedByEslVersion(ctx context.Context, tx *sql.Tx, app string, deleted bool, ignorePrepublishes bool) ([]*DBReleaseWithMetaData, error) { + span, ctx := tracer.StartSpanFromContext(ctx, "DBSelectReleasesByAppOrderedByEslVersion") defer span.Finish() selectQuery := h.AdaptQuery(fmt.Sprintf( "SELECT eslVersion, created, appName, metadata, releaseVersion, deleted, environments " + diff --git a/pkg/db/db_test.go b/pkg/db/db_test.go index 7221dffef..35563e9e6 100644 --- a/pkg/db/db_test.go +++ b/pkg/db/db_test.go @@ -2311,6 +2311,35 @@ func TestReadReleasesByApp(t *testing.T) { }, }, }, + { + Name: "Retrieve deleted release", + Releases: []DBReleaseWithMetaData{ + { + EslVersion: 1, + ReleaseNumber: 10, + App: "app1", + Manifests: DBReleaseManifests{Manifests: map[string]string{"dev": "manifest1"}}, + }, + { + EslVersion: 2, + ReleaseNumber: 10, + App: "app1", + Deleted: true, + Manifests: DBReleaseManifests{Manifests: map[string]string{"dev": "manifest1"}}, + }, + }, + AppName: "app1", + Expected: []*DBReleaseWithMetaData{ + { + EslVersion: 2, + ReleaseNumber: 10, + Deleted: true, + App: "app1", + Manifests: DBReleaseManifests{Manifests: map[string]string{"dev": "manifest1"}}, + Environments: []string{"dev"}, + }, + }, + }, { Name: "Retrieve multiple releases", Releases: []DBReleaseWithMetaData{ @@ -2525,7 +2554,7 @@ func TestReadReleasesByApp(t *testing.T) { return fmt.Errorf("error while writing release, error: %w", err) } } - releases, err := dbHandler.DBSelectReleasesByApp(ctx, transaction, tc.AppName, false, !tc.RetrievePrepublishes) + releases, err := dbHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, tc.AppName, !tc.RetrievePrepublishes) if err != nil { return fmt.Errorf("error while selecting release, error: %w", err) } diff --git a/services/cd-service/pkg/repository/transformer.go b/services/cd-service/pkg/repository/transformer.go index c308054ab..c684bb02c 100644 --- a/services/cd-service/pkg/repository/transformer.go +++ b/services/cd-service/pkg/repository/transformer.go @@ -672,7 +672,7 @@ func (c *CreateApplicationVersion) Transform( sortedKeys := sorting.SortKeys(c.Manifests) if state.DBHandler.ShouldUseOtherTables() { - prevRelease, err := state.DBHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, c.Application, false, false) + prevRelease, err := state.DBHandler.DBSelectReleasesByAppOrderedByEslVersion(ctx, transaction, c.Application, false, false) if err != nil { return "", err } @@ -1711,7 +1711,7 @@ func (u *DeleteEnvFromApp) Transform( return "", err } if state.DBHandler.ShouldUseOtherTables() { - releases, err := state.DBHandler.DBSelectReleasesByApp(ctx, transaction, u.Application, false, true) + releases, err := state.DBHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, u.Application, true) if err != nil { return "", err } diff --git a/services/cd-service/pkg/repository/transformer_db_test.go b/services/cd-service/pkg/repository/transformer_db_test.go index 16b87918d..973c987e8 100644 --- a/services/cd-service/pkg/repository/transformer_db_test.go +++ b/services/cd-service/pkg/repository/transformer_db_test.go @@ -22,11 +22,12 @@ import ( "encoding/json" "errors" "fmt" - "github.com/lib/pq" "regexp" "testing" gotime "time" + "github.com/lib/pq" + "github.com/freiheit-com/kuberpult/pkg/api/v1" "github.com/freiheit-com/kuberpult/pkg/event" @@ -2264,7 +2265,7 @@ func TestDeleteEnvFromAppWithDB(t *testing.T) { if err != nil { return fmt.Errorf("error: %v", err) } - releases, err2 := state.DBHandler.DBSelectReleasesByApp(ctx, transaction, appName, false, true) + releases, err2 := state.DBHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, appName, true) if err2 != nil { return fmt.Errorf("error retrieving release: %v", err2) } @@ -3649,7 +3650,7 @@ func TestTimestampConsistency(t *testing.T) { t.Fatalf("error mismatch on envAcceptance(-want, +got):\n%s", diff) } //Release - releases, err := state.DBHandler.DBSelectReleasesByApp(ctx, transaction, testAppName, false, true) + releases, err := state.DBHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, testAppName, true) if err != nil { return err } diff --git a/tests/integration-tests/release_test.go b/tests/integration-tests/release_test.go index 81809fe43..513dcb3a6 100644 --- a/tests/integration-tests/release_test.go +++ b/tests/integration-tests/release_test.go @@ -472,7 +472,7 @@ func callDBForLock(t *testing.T, dbHandler *db.DBHandler, ctx context.Context, e func callDBForReleases(t *testing.T, dbHandler *db.DBHandler, ctx context.Context, appName string) []*db.DBReleaseWithMetaData { release, err := db.WithTransactionMultipleEntriesT(dbHandler, ctx, true, func(ctx context.Context, transaction *sql.Tx) ([]*db.DBReleaseWithMetaData, error) { - return dbHandler.DBSelectReleasesByApp(ctx, transaction, appName, false, true) + return dbHandler.DBSelectReleasesByAppLatestEslVersion(ctx, transaction, appName, true) }) if err != nil { t.Fatalf("DBSelectReleasesByApp failed %s", err)