From f9538e07a28e56d8f6bbff2dec4dd542db799397 Mon Sep 17 00:00:00 2001 From: Amin Salarkia Date: Mon, 30 Sep 2024 16:28:42 +0200 Subject: [PATCH] fix(cd-service): add applications column to the environments table, fix DeleteEnvFromApp (#1983) Ref: SRX-GJNPF1 --- ...269691278_environments_applications.up.sql | 6 + ...269691278_environments_applications.up.sql | 16 +++ pkg/db/db.go | 60 ++++++--- pkg/db/db_test.go | 19 +-- .../cd-service/pkg/repository/repository.go | 6 +- .../cd-service/pkg/repository/transformer.go | 52 +++++++- .../pkg/repository/transformer_db_test.go | 119 +++++++++++++++++- .../pkg/repository/transformer_test.go | 4 +- .../pkg/service/git_test.go | 2 +- 9 files changed, 246 insertions(+), 38 deletions(-) create mode 100644 database/migrations/postgres/1727264269691278_environments_applications.up.sql create mode 100644 database/migrations/sqlite/1727264269691278_environments_applications.up.sql diff --git a/database/migrations/postgres/1727264269691278_environments_applications.up.sql b/database/migrations/postgres/1727264269691278_environments_applications.up.sql new file mode 100644 index 000000000..991c37aa3 --- /dev/null +++ b/database/migrations/postgres/1727264269691278_environments_applications.up.sql @@ -0,0 +1,6 @@ +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'environments' AND column_name='applications') THEN + ALTER TABLE IF EXISTS environments ADD COLUMN applications VARCHAR; + END IF; +END $$; \ No newline at end of file diff --git a/database/migrations/sqlite/1727264269691278_environments_applications.up.sql b/database/migrations/sqlite/1727264269691278_environments_applications.up.sql new file mode 100644 index 000000000..dfdbedf86 --- /dev/null +++ b/database/migrations/sqlite/1727264269691278_environments_applications.up.sql @@ -0,0 +1,16 @@ +CREATE TABLE IF NOT EXISTS environments_new +( + created TIMESTAMP, + version BIGINT, + name VARCHAR(255), + json VARCHAR, + applications VARCHAR, + PRIMARY KEY(name, version) +); + +INSERT INTO environments_new(created, version, name, json) +SELECT created, version, name, json +FROM environments; + +DROP TABLE IF EXISTS environments; +ALTER TABLE environments_new RENAME TO environments; diff --git a/pkg/db/db.go b/pkg/db/db.go index b03b302df..3eaf74e4c 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -4674,17 +4674,19 @@ type DBAllEnvironmentsRow struct { } type DBEnvironment struct { - Created time.Time - Version int64 - Name string - Config config.EnvironmentConfig + Created time.Time + Version int64 + Name string + Config config.EnvironmentConfig + Applications []string } type DBEnvironmentRow struct { - Created time.Time - Version int64 - Name string - Config string + Created time.Time + Version int64 + Name string + Config string + Applications string } func EnvironmentFromRow(ctx context.Context, row *DBEnvironmentRow) (*DBEnvironment, error) { @@ -4696,11 +4698,17 @@ func EnvironmentFromRow(ctx context.Context, row *DBEnvironmentRow) (*DBEnvironm if err != nil { return nil, fmt.Errorf("unable to unmarshal the JSON in the database, JSON: %s, error: %w", row.Config, err) } + applications := []string{} + err = json.Unmarshal([]byte(row.Applications), &applications) + if err != nil { + return nil, fmt.Errorf("unable to unmarshal the JSON in the database, JSON: %s, error: %w", row.Applications, err) + } return &DBEnvironment{ - Created: row.Created, - Version: row.Version, - Name: row.Name, - Config: parsedConfig, + Created: row.Created, + Version: row.Version, + Name: row.Name, + Config: parsedConfig, + Applications: applications, }, nil } @@ -4710,7 +4718,7 @@ func (h *DBHandler) DBSelectEnvironment(ctx context.Context, tx *sql.Tx, environ selectQuery := h.AdaptQuery( ` -SELECT created, version, name, json +SELECT created, version, name, json, applications FROM environments WHERE name=? ORDER BY version DESC @@ -4739,7 +4747,7 @@ LIMIT 1; if rows.Next() { //exhaustruct:ignore row := DBEnvironmentRow{} - err := rows.Scan(&row.Created, &row.Version, &row.Name, &row.Config) + err := rows.Scan(&row.Created, &row.Version, &row.Name, &row.Config, &row.Applications) if err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, nil @@ -4771,7 +4779,8 @@ SELECT environments.created AS created, environments.version AS version, environments.name AS name, - environments.json AS json + environments.json AS json, + environments.applications AS applications FROM ( SELECT MAX(version) AS latest, @@ -4817,7 +4826,7 @@ LIMIT ? for rows.Next() { //exhaustruct:ignore row := DBEnvironmentRow{} - err := rows.Scan(&row.Created, &row.Version, &row.Name, &row.Config) + err := rows.Scan(&row.Created, &row.Version, &row.Name, &row.Config, &row.Applications) if err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, nil @@ -4833,8 +4842,8 @@ LIMIT ? return &envs, nil } -func (h *DBHandler) DBWriteEnvironment(ctx context.Context, tx *sql.Tx, environmentName string, environmentConfig config.EnvironmentConfig) error { - span, ctx := tracer.StartSpanFromContext(ctx, "DBWriteEnvironment") +func (h *DBHandler) DBWriteEnvironment(ctx context.Context, tx *sql.Tx, environmentName string, environmentConfig config.EnvironmentConfig, applications []string) error { + span, _ := tracer.StartSpanFromContext(ctx, "DBWriteEnvironment") defer span.Finish() if h == nil { @@ -4852,6 +4861,10 @@ func (h *DBHandler) DBWriteEnvironment(ctx context.Context, tx *sql.Tx, environm if err != nil { return fmt.Errorf("error while selecting environment %s from database, error: %w", environmentName, err) } + applicationsJson, err := json.Marshal(applications) + if err != nil { + return fmt.Errorf("could not marshal the application names list %v, error: %w", applicationsJson, err) + } var existingEnvironmentVersion int64 if existingEnvironment == nil { @@ -4861,7 +4874,7 @@ func (h *DBHandler) DBWriteEnvironment(ctx context.Context, tx *sql.Tx, environm } insertQuery := h.AdaptQuery( - "INSERT Into environments (created, version, name, json) VALUES (?, ?, ?, ?);", + "INSERT Into environments (created, version, name, json, applications) VALUES (?, ?, ?, ?, ?);", ) now, err := h.DBReadTransactionTimestamp(ctx, tx) if err != nil { @@ -4874,6 +4887,7 @@ func (h *DBHandler) DBWriteEnvironment(ctx context.Context, tx *sql.Tx, environm existingEnvironmentVersion+1, environmentName, jsonToInsert, + string(applicationsJson), ) if err != nil { return fmt.Errorf("could not write environment %s with config %v to environments table, error: %w", environmentName, environmentConfig, err) @@ -5060,10 +5074,16 @@ func (h *DBHandler) RunCustomMigrationEnvironments(ctx context.Context, getAllEn return fmt.Errorf("could not get environments, error: %w", err) } + allApplications, err := h.DBSelectAllApplications(ctx, transaction) + if err != nil { + return fmt.Errorf("could not get all applications, error: %w", err) + } + allEnvironmentNames := make([]string, 0) for envName, config := range allEnvironments { allEnvironmentNames = append(allEnvironmentNames, envName) - err = h.DBWriteEnvironment(ctx, transaction, envName, config) + + err = h.DBWriteEnvironment(ctx, transaction, envName, config, allApplications.Apps) if err != nil { return fmt.Errorf("unable to write manifest for environment %s to the database, error: %w", envName, err) } diff --git a/pkg/db/db_test.go b/pkg/db/db_test.go index 14e1516be..dab4bf1cc 100644 --- a/pkg/db/db_test.go +++ b/pkg/db/db_test.go @@ -1818,6 +1818,7 @@ func TestReadWriteEnvironment(t *testing.T) { type EnvAndConfig struct { EnvironmentName string EnvironmentConfig config.EnvironmentConfig + Applications []string } type TestCase struct { Name string @@ -1833,13 +1834,15 @@ func TestReadWriteEnvironment(t *testing.T) { { EnvironmentName: "development", EnvironmentConfig: testutil.MakeEnvConfigLatest(nil), + Applications: []string{"app1", "app2", "app3"}, }, }, EnvToQuery: "development", ExpectedEntry: &DBEnvironment{ - Version: 1, - Name: "development", - Config: testutil.MakeEnvConfigLatest(nil), + Version: 1, + Name: "development", + Config: testutil.MakeEnvConfigLatest(nil), + Applications: []string{"app1", "app2", "app3"}, }, }, { @@ -1848,13 +1851,15 @@ func TestReadWriteEnvironment(t *testing.T) { { EnvironmentName: "development", EnvironmentConfig: testutil.MakeEnvConfigLatestWithGroup(nil, conversion.FromString("development-group")), // "elaborate config" being the env group + Applications: []string{"app1"}, }, }, EnvToQuery: "development", ExpectedEntry: &DBEnvironment{ - Version: 1, - Name: "development", - Config: testutil.MakeEnvConfigLatestWithGroup(nil, conversion.FromString("development-group")), + Version: 1, + Name: "development", + Config: testutil.MakeEnvConfigLatestWithGroup(nil, conversion.FromString("development-group")), + Applications: []string{"app1"}, }, }, { @@ -1943,7 +1948,7 @@ func TestReadWriteEnvironment(t *testing.T) { for _, envToWrite := range tc.EnvsToWrite { err := dbHandler.WithTransaction(ctx, false, func(ctx context.Context, transaction *sql.Tx) error { - err := dbHandler.DBWriteEnvironment(ctx, transaction, envToWrite.EnvironmentName, envToWrite.EnvironmentConfig) + err := dbHandler.DBWriteEnvironment(ctx, transaction, envToWrite.EnvironmentName, envToWrite.EnvironmentConfig, envToWrite.Applications) if err != nil { return fmt.Errorf("error while writing environment, error: %w", err) } diff --git a/services/cd-service/pkg/repository/repository.go b/services/cd-service/pkg/repository/repository.go index e93f2b842..a1f0d1cbf 100644 --- a/services/cd-service/pkg/repository/repository.go +++ b/services/cd-service/pkg/repository/repository.go @@ -2070,14 +2070,14 @@ func (s *State) GetEnvironmentApplicationsFromManifest(environment string) ([]st } func (s *State) GetEnvironmentApplicationsFromDB(ctx context.Context, transaction *sql.Tx, environment string) ([]string, error) { - applications, err := s.DBHandler.DBSelectAllApplications(ctx, transaction) + envInfo, err := s.DBHandler.DBSelectEnvironment(ctx, transaction, environment) if err != nil { return nil, err } - if applications == nil { + if envInfo.Applications == nil { return make([]string, 0), nil } - return applications.Apps, nil + return envInfo.Applications, nil } // GetApplicationsFromFile returns all apps that exist in any env diff --git a/services/cd-service/pkg/repository/transformer.go b/services/cd-service/pkg/repository/transformer.go index 5698d4e02..4b58acf86 100644 --- a/services/cd-service/pkg/repository/transformer.go +++ b/services/cd-service/pkg/repository/transformer.go @@ -724,6 +724,28 @@ func (c *CreateApplicationVersion) Transform( for i := range sortedKeys { env := sortedKeys[i] man := c.Manifests[env] + // Add application to the environment if not exist + if state.DBHandler.ShouldUseOtherTables() { + envInfo, err := state.DBHandler.DBSelectEnvironment(ctx, transaction, env) + if err != nil { + return "", GetCreateReleaseGeneralFailure(err) + } + found := false + if envInfo != nil && envInfo.Applications != nil { + for _, app := range envInfo.Applications { + if app == c.Application { + found = true + break + } + } + } + if envInfo != nil && !found { + err = state.DBHandler.DBWriteEnvironment(ctx, transaction, env, envInfo.Config, append(envInfo.Applications, c.Application)) + if err != nil { + return "", GetCreateReleaseGeneralFailure(err) + } + } + } err := state.checkUserPermissions(ctx, transaction, env, c.Application, auth.PermissionCreateRelease, c.Team, c.RBACConfig, true) if err != nil { @@ -1697,6 +1719,26 @@ func (u *DeleteEnvFromApp) Transform( return "", err } } + + env, err := state.DBHandler.DBSelectEnvironment(ctx, transaction, u.Environment) + if err != nil { + return "", fmt.Errorf("Couldn't read environment: %s from environments table, error: %w", u.Environment, err) + } + if env == nil { + return "", fmt.Errorf("Attempting to delete an environment that doesn't exist in the environments table") + } + newApps := make([]string, 0) + if env.Applications != nil { + for _, app := range env.Applications { + if app != u.Application { + newApps = append(newApps, app) + } + } + } + err = state.DBHandler.DBWriteEnvironment(ctx, transaction, env.Name, env.Config, newApps) + if err != nil { + return "", fmt.Errorf("Couldn't write environment: %s into environments table, error: %w", u.Environment, err) + } } else { fs := state.Filesystem @@ -2673,7 +2715,15 @@ func (c *CreateEnvironment) Transform( } if state.DBHandler.ShouldUseOtherTables() { // write to environments table - err := state.DBHandler.DBWriteEnvironment(ctx, transaction, c.Environment, c.Config) + allApplications, err := state.DBHandler.DBSelectAllApplications(ctx, transaction) + if err != nil { + return "", fmt.Errorf("unable to read all applications, error: %w", err) + } + environmentApplications := make([]string, 0) + if allApplications != nil { + environmentApplications = allApplications.Apps + } + err = state.DBHandler.DBWriteEnvironment(ctx, transaction, c.Environment, c.Config, environmentApplications) if err != nil { return "", fmt.Errorf("unable to write to the environment table, error: %w", err) } diff --git a/services/cd-service/pkg/repository/transformer_db_test.go b/services/cd-service/pkg/repository/transformer_db_test.go index acc28f2a4..9f0afaac1 100644 --- a/services/cd-service/pkg/repository/transformer_db_test.go +++ b/services/cd-service/pkg/repository/transformer_db_test.go @@ -778,6 +778,13 @@ func TestCreateApplicationVersionDB(t *testing.T) { if diff := cmp.Diff(tc.expectedDbReleases, actualRelease, cmpopts.IgnoreFields(db.DBAllReleasesWithMetaData{}, "Created")); diff != "" { t.Errorf("error mismatch (-want, +got):\n%s", diff) } + environment, err4 := state.DBHandler.DBSelectEnvironment(ctx, transaction, "acceptance") + if err4 != nil { + return fmt.Errorf("error retrieving environment: %w", err) + } + if diff := cmp.Diff([]string{appName}, environment.Applications); diff != "" { + t.Errorf("environment applications list mismatch: (-want, +got):\n%s", diff) + } return nil }) if err3 != nil { @@ -1141,6 +1148,13 @@ func TestMinorFlag(t *testing.T) { if err != nil { return err } + _, state, _, err = repo.ApplyTransformersInternal(ctx, transaction, &CreateEnvironment{ + Environment: "new env", + Config: config.EnvironmentConfig{Upstream: &config.EnvironmentConfigUpstream{Environment: envAcceptance, Latest: false}}, + }) + if err != nil { + return err + } _, state, _, err = repo.ApplyTransformersInternal(ctx, transaction, tc.Transformers...) if err != nil { return err @@ -1682,6 +1696,14 @@ func TestEvents(t *testing.T) { { Name: "check if the number of events is equal to pageNumber plus pageSize", Transformers: []Transformer{ + &CreateEnvironment{ + Environment: "staging", + Config: config.EnvironmentConfig{ + Upstream: &config.EnvironmentConfigUpstream{ + Latest: true, + }, + }, + }, &CreateApplicationVersion{ Application: "app", SourceCommitId: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", @@ -1706,8 +1728,16 @@ func TestEvents(t *testing.T) { }, { Name: "Create a single application version and deploy it with DB", - // no need to bother with environments here Transformers: []Transformer{ + &CreateEnvironment{ + Authentication: Authentication{}, + Environment: "staging", + Config: config.EnvironmentConfig{ + Upstream: nil, + ArgoCd: nil, + EnvironmentGroup: conversion.FromString("mygroup"), + }, + }, &CreateApplicationVersion{ Application: "app", SourceCommitId: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", @@ -1844,7 +1874,7 @@ func TestEvents(t *testing.T) { t.Fatal(err) } if len(rows) != len(tc.expectedDBEvents) { - t.Fatalf("error event count mismatch expected '%d' events but got '%d'\n", len(tc.expectedDBEvents), len(rows)) + t.Fatalf("error event count mismatch expected '%d' events but got '%d' rows: %v\n", len(tc.expectedDBEvents), len(rows), rows) } dEvents, err := DBParseToEvents(rows) if err != nil { @@ -1873,6 +1903,40 @@ func TestEvents(t *testing.T) { } func TestDeleteEnvFromAppWithDB(t *testing.T) { + setupTransformers := []Transformer{ + &CreateEnvironment{ + Environment: "env", + Config: config.EnvironmentConfig{ + Upstream: nil, + ArgoCd: nil, + EnvironmentGroup: conversion.FromString("mygroup"), + }, + }, + &CreateEnvironment{ + Environment: "env1", + Config: config.EnvironmentConfig{ + Upstream: nil, + ArgoCd: nil, + EnvironmentGroup: conversion.FromString("mygroup"), + }, + }, + &CreateEnvironment{ + Environment: "env2", + Config: config.EnvironmentConfig{ + Upstream: nil, + ArgoCd: nil, + EnvironmentGroup: conversion.FromString("mygroup"), + }, + }, + &CreateEnvironment{ + Environment: "env3", + Config: config.EnvironmentConfig{ + Upstream: nil, + ArgoCd: nil, + EnvironmentGroup: conversion.FromString("mygroup"), + }, + }, + } firstRelease := db.DBReleaseWithMetaData{ EslVersion: 1, ReleaseNumber: 10, @@ -1958,6 +2022,10 @@ func TestDeleteEnvFromAppWithDB(t *testing.T) { ctxWithTime := time.WithTimeNow(testutil.MakeTestContext(), timeNowOld) repo := SetupRepositoryTestWithDB(t) err := repo.State().DBHandler.WithTransaction(ctxWithTime, false, func(ctx context.Context, transaction *sql.Tx) error { + _, _, _, err := repo.ApplyTransformersInternal(ctx, transaction, setupTransformers...) + if err != nil { + return err + } for _, release := range tc.PrevReleases { repo.State().DBHandler.DBInsertRelease(ctx, transaction, release, 0) } @@ -1979,6 +2047,17 @@ func TestDeleteEnvFromAppWithDB(t *testing.T) { } } } + environment, err2 := state.DBHandler.DBSelectEnvironment(ctx, transaction, tc.Transforms[0].(*DeleteEnvFromApp).Environment) + if err2 != nil { + return err2 + } + if environment != nil { + for _, envApp := range environment.Applications { + if envApp == firstRelease.App { + return fmt.Errorf("Expected app %s to be deleted from environment %s", firstRelease.App, environment.Name) + } + } + } return nil }) if err != nil { @@ -2231,6 +2310,14 @@ func TestUndeployApplicationDB(t *testing.T) { { Name: "Success", Transformers: []Transformer{ + &CreateEnvironment{ + Environment: envProduction, + Config: config.EnvironmentConfig{ + ArgoCd: nil, + Upstream: nil, + EnvironmentGroup: conversion.FromString("mygroup"), + }, + }, &CreateApplicationVersion{ Application: "app1", Manifests: map[string]string{ @@ -2251,6 +2338,14 @@ func TestUndeployApplicationDB(t *testing.T) { { Name: "Create un-deploy Version for un-deployed application should not work", Transformers: []Transformer{ + &CreateEnvironment{ + Environment: envProduction, + Config: config.EnvironmentConfig{ + ArgoCd: nil, + Upstream: nil, + EnvironmentGroup: conversion.FromString("mygroup"), + }, + }, &CreateApplicationVersion{ Application: "app1", Manifests: map[string]string{ @@ -2270,7 +2365,7 @@ func TestUndeployApplicationDB(t *testing.T) { }, }, expectedError: &TransformerBatchApplyError{ - Index: 3, + Index: 4, TransformerError: errMatcher{"cannot undeploy non-existing application 'app1'"}, }, expectedCommitMsg: "", @@ -2428,6 +2523,14 @@ func TestUndeployApplicationDB(t *testing.T) { { Name: "Undeploy application where the last release is not Undeploy shouldn't work", Transformers: []Transformer{ + &CreateEnvironment{ + Environment: envProduction, + Config: config.EnvironmentConfig{ + ArgoCd: nil, + Upstream: nil, + EnvironmentGroup: conversion.FromString("mygroup"), + }, + }, &CreateApplicationVersion{ Application: "app1", Manifests: map[string]string{ @@ -2453,7 +2556,7 @@ func TestUndeployApplicationDB(t *testing.T) { }, }, expectedError: &TransformerBatchApplyError{ - Index: 3, + Index: 4, TransformerError: errMatcher{"UndeployApplication: error last release is not un-deployed application version of 'app1'"}, }, expectedCommitMsg: "", @@ -2626,6 +2729,14 @@ func TestCreateUndeployDBState(t *testing.T) { { Name: "Success", Transformers: []Transformer{ + &CreateEnvironment{ + Environment: envProduction, + Config: config.EnvironmentConfig{ + ArgoCd: nil, + Upstream: nil, + EnvironmentGroup: conversion.FromString("mygroup"), + }, + }, &CreateApplicationVersion{ Application: appName, Manifests: map[string]string{ diff --git a/services/manifest-repo-export-service/pkg/repository/transformer_test.go b/services/manifest-repo-export-service/pkg/repository/transformer_test.go index d592e7bdc..8b752281a 100644 --- a/services/manifest-repo-export-service/pkg/repository/transformer_test.go +++ b/services/manifest-repo-export-service/pkg/repository/transformer_test.go @@ -826,7 +826,7 @@ func TestReleaseTrain(t *testing.T) { Environment: "staging", Latest: true, }, - }) + }, []string{appName}) if err != nil { return err } @@ -834,7 +834,7 @@ func TestReleaseTrain(t *testing.T) { Upstream: &config.EnvironmentConfigUpstream{ Environment: "staging", }, - }) + }, []string{appName}) if err != nil { return err } diff --git a/services/manifest-repo-export-service/pkg/service/git_test.go b/services/manifest-repo-export-service/pkg/service/git_test.go index 6b3344f01..f4fd50b88 100644 --- a/services/manifest-repo-export-service/pkg/service/git_test.go +++ b/services/manifest-repo-export-service/pkg/service/git_test.go @@ -113,7 +113,7 @@ func setupDBFixtures(ctx context.Context, dbHandler *db.DBHandler, transaction * Upstream: &config.EnvironmentConfigUpstream{ Latest: true, }, - }) + }, fixtureAppications) if err != nil { return err }