From 81ad492c7efef79ef80bbffe29216f4b3ce59f94 Mon Sep 17 00:00:00 2001 From: Katie Date: Thu, 16 Nov 2023 13:37:25 -0500 Subject: [PATCH] DDO-3269: Environment V3 SQL Migrations (#356) * environment sql migrations * fixes * environment sql migrations * fixes * delete after logic * didnt add idk * whered all my changes go * environment sql migrations * fixes * delete after logic * environment sql migrations * fixes * didnt add idk * whered all my changes go * remove fc ref * merge conflict * delete prevent deletion lifecycle constraint from sql and v2 * merge main * tests * fall through :/ --- .../migrations/000062_environment_v3.down.sql | 29 ++++ .../migrations/000062_environment_v3.up.sql | 58 +++++++ .../deprecated_models/v2models/environment.go | 3 - sherlock/internal/models/environment_test.go | 149 ++++++++++++++++++ sherlock/internal/models/test_data.go | 2 +- 5 files changed, 237 insertions(+), 4 deletions(-) create mode 100644 sherlock/db/migrations/000062_environment_v3.down.sql create mode 100644 sherlock/db/migrations/000062_environment_v3.up.sql diff --git a/sherlock/db/migrations/000062_environment_v3.down.sql b/sherlock/db/migrations/000062_environment_v3.down.sql new file mode 100644 index 000000000..4dbe02671 --- /dev/null +++ b/sherlock/db/migrations/000062_environment_v3.down.sql @@ -0,0 +1,29 @@ +alter table environments + drop constraint if exists name_valid; + +alter table environments + drop constraint if exists owner_id_present; + +alter table environments + drop constraint if exists lifecycle_valid; + +alter table environments + drop constraint if exists default_namespace_present; + +alter table environments + drop constraint if exists helmfile_ref_present; + +alter table environments + drop constraint if exists unique_resource_prefix_present; + +alter table environments + drop constraint if exists delete_after_valid; + +alter table environments + drop constraint if exists offline_valid; + +alter table environments + drop constraint if exists offline_schedule_begin_time_present; + +alter table environments + drop constraint if exists offline_schedule_end_time_present; diff --git a/sherlock/db/migrations/000062_environment_v3.up.sql b/sherlock/db/migrations/000062_environment_v3.up.sql new file mode 100644 index 000000000..8c1ce0ce6 --- /dev/null +++ b/sherlock/db/migrations/000062_environment_v3.up.sql @@ -0,0 +1,58 @@ +alter table environments + add constraint name_valid + check (name is not null and name != '' and name similar to '[a-z0-9]([-a-z0-9]*[a-z0-9])?'); + +alter table environments + add constraint owner_id_present + check (owner_id is not null or + (legacy_owner is not null and legacy_owner != '')); + +alter table environments + add constraint lifecycle_valid + check ((lifecycle = 'template' and template_environment_id is null) or + (lifecycle = 'dynamic' and + template_environment_id is not null and + base is not null and base != '' and + default_cluster_id is not null and + requires_suitability is not null) or + (lifecycle = 'static' and + base is not null and base != '' and + default_cluster_id is not null and + requires_suitability is not null)); + +alter table environments + add constraint default_namespace_present + check (default_namespace is not null and default_namespace != ''); + +alter table environments + add constraint helmfile_ref_present + check (helmfile_ref is not null and helmfile_ref != ''); + +alter table environments + add constraint unique_resource_prefix_present + check (unique_resource_prefix is not null and unique_resource_prefix != ''); + +alter table environments + add constraint delete_after_valid + check (delete_after is null or + (lifecycle = 'dynamic' and + (prevent_deletion is null or prevent_deletion is false))); + +alter table environments + add constraint offline_valid + check (lifecycle = 'dynamic' or + ((offline is null or offline is false) and + (offline_schedule_begin_enabled is null or offline_schedule_begin_enabled is false) and + (offline_schedule_end_enabled is null or offline_schedule_end_enabled is false))); + +alter table environments + add constraint offline_schedule_begin_time_present + check (offline_schedule_begin_enabled is null or + offline_schedule_begin_enabled is false or + offline_schedule_begin_time is not null); + +alter table environments + add constraint offline_schedule_end_time_present + check (offline_schedule_end_enabled is null or + offline_schedule_end_enabled is false or + offline_schedule_end_time is not null); diff --git a/sherlock/internal/deprecated_models/v2models/environment.go b/sherlock/internal/deprecated_models/v2models/environment.go index f45ceb751..aac017087 100644 --- a/sherlock/internal/deprecated_models/v2models/environment.go +++ b/sherlock/internal/deprecated_models/v2models/environment.go @@ -217,9 +217,6 @@ func validateEnvironment(environment *Environment) error { return fmt.Errorf("a %T must have a non-empty unique resource prefix", environment) } - if environment.PreventDeletion != nil && *environment.PreventDeletion && environment.Lifecycle != "dynamic" { - return fmt.Errorf("preventDeletion is only valid for dynamic environments") - } if environment.AutoDelete != nil { if err := environment.AutoDelete.Validate(); err != nil { return err diff --git a/sherlock/internal/models/environment_test.go b/sherlock/internal/models/environment_test.go index f9ad1f9d9..303945f93 100644 --- a/sherlock/internal/models/environment_test.go +++ b/sherlock/internal/models/environment_test.go @@ -1,8 +1,10 @@ package models import ( + "database/sql" "github.com/broadinstitute/sherlock/go-shared/pkg/utils" "github.com/broadinstitute/sherlock/sherlock/internal/errors" + "time" ) func (s *modelSuite) TestEnvironmentUniqueResourcePrefixAssigning() { @@ -218,3 +220,150 @@ func (s *modelSuite) TestEnvironmentDeleteForbidden() { Delete(&environment). Error, errors.Forbidden) } + +func (s *modelSuite) TestEnvironmentValidationSqlNameInvalid() { + s.SetNonSuitableTestUserForDB() + environment := s.TestData.Environment_Swatomation_TestBee() + err := s.DB.Model(&environment).Select("Name").Updates(&Environment{Name: "prod_env"}).Error + s.ErrorContains(err, "violates check constraint \"name_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlNameEmpty() { + s.SetNonSuitableTestUserForDB() + environment := s.TestData.Environment_Swatomation_TestBee() + err := s.DB.Model(&environment).Select("Name").Updates(&Environment{Name: ""}).Error + s.ErrorContains(err, "violates check constraint \"name_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlOwnerIDEmpty() { + s.SetSuitableTestUserForDB() + environment := s.TestData.Environment_Prod() + err := s.DB.Model(&environment).Select("OwnerID", "LegacyOwner").Updates(&Environment{OwnerID: nil, LegacyOwner: utils.PointerTo("")}).Error + s.ErrorContains(err, "violates check constraint \"owner_id_present\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlOwnerIDNull() { + s.SetSuitableTestUserForDB() + environment := s.TestData.Environment_Prod() + err := s.DB.Model(&environment).Select("OwnerID", "LegacyOwner").Updates(&Environment{OwnerID: nil, LegacyOwner: nil}).Error + s.ErrorContains(err, "violates check constraint \"owner_id_present\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlTemplateLifecycle() { + s.SetSuitableTestUserForDB() + templateEnv := s.TestData.Environment_Swatomation() + err := s.DB.Model(&templateEnv).Updates(&Environment{TemplateEnvironmentID: utils.PointerTo(uint(1))}).Error + s.ErrorContains(err, "violates check constraint \"lifecycle_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlDynamicLifecycle() { + s.SetSuitableTestUserForDB() + dynamicEnv := s.TestData.Environment_Swatomation_TestBee() + err := s.DB.Model(&dynamicEnv).Select("TemplateEnvironmentID").Updates(&Environment{TemplateEnvironmentID: nil}).Error + s.ErrorContains(err, "violates check constraint \"lifecycle_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlDynamicLifecycleBase() { + s.SetSuitableTestUserForDB() + staticEnv := s.TestData.Environment_Swatomation_LongBee() + err := s.DB.Model(&staticEnv).Select("Base").Updates(&Environment{Base: ""}).Error + s.ErrorContains(err, "violates check constraint \"lifecycle_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlDynamicLifecycleDefaultClusterID() { + s.SetSuitableTestUserForDB() + staticEnv := s.TestData.Environment_Swatomation_DevBee() + err := s.DB.Model(&staticEnv).Select("DefaultClusterID").Updates(&Environment{DefaultClusterID: nil}).Error + s.ErrorContains(err, "violates check constraint \"lifecycle_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlDynamicLifecycleRequiresSuitability() { + s.SetSuitableTestUserForDB() + staticEnv := s.TestData.Environment_Swatomation_TestBee() + err := s.DB.Model(&staticEnv).Select("RequiresSuitability").Updates(&Environment{RequiresSuitability: nil}).Error + s.ErrorContains(err, "violates check constraint \"lifecycle_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlStaticLifecycleBase() { + s.SetSuitableTestUserForDB() + staticEnv := s.TestData.Environment_Prod() + err := s.DB.Model(&staticEnv).Select("Base").Updates(&Environment{Base: ""}).Error + s.ErrorContains(err, "violates check constraint \"lifecycle_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlStaticLifecycleDefaultClusterID() { + s.SetSuitableTestUserForDB() + staticEnv := s.TestData.Environment_Prod() + err := s.DB.Model(&staticEnv).Select("DefaultClusterID").Updates(&Environment{DefaultClusterID: nil}).Error + s.ErrorContains(err, "violates check constraint \"lifecycle_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlStaticLifecycleRequiresSuitability() { + s.SetSuitableTestUserForDB() + staticEnv := s.TestData.Environment_Prod() + err := s.DB.Model(&staticEnv).Select("RequiresSuitability").Updates(&Environment{RequiresSuitability: nil}).Error + s.ErrorContains(err, "violates check constraint \"lifecycle_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlDefaultNamespace() { + s.SetSuitableTestUserForDB() + environment := s.TestData.Environment_DdpAzureDev() + err := s.DB.Model(&environment).Select("DefaultNamespace").Updates(&Environment{DefaultNamespace: ""}).Error + s.ErrorContains(err, "violates check constraint \"default_namespace_present\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlUniqueResourcePrefix() { + s.SetSuitableTestUserForDB() + environment := s.TestData.Environment_DdpAzureProd() + err := s.DB.Model(&environment).Select("UniqueResourcePrefix").Updates(&Environment{UniqueResourcePrefix: ""}).Error + s.ErrorContains(err, "violates check constraint \"unique_resource_prefix_present\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlDeleteAfterNotDynamic() { + s.SetSuitableTestUserForDB() + environment := s.TestData.Environment_Staging() + err := s.DB.Model(&environment).Select("DeleteAfter").Updates(&Environment{DeleteAfter: sql.NullTime{Time: time.Now().Add(6 * time.Hour), Valid: true}}).Error + s.ErrorContains(err, "violates check constraint \"delete_after_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlDeleteAfterPreventDeletion() { + s.SetSuitableTestUserForDB() + environment := s.TestData.Environment_Swatomation_TestBee() + err := s.DB.Model(&environment).Select("PreventDeletion").Updates(&Environment{PreventDeletion: utils.PointerTo(true)}).Error + s.ErrorContains(err, "violates check constraint \"delete_after_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlOfflineNotDynamic() { + s.SetSuitableTestUserForDB() + environment := s.TestData.Environment_Prod() + err := s.DB.Model(&environment).Select("Offline").Updates(&Environment{Offline: utils.PointerTo(true)}).Error + s.ErrorContains(err, "violates check constraint \"offline_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlOfflineBegin() { + s.SetSuitableTestUserForDB() + environment := s.TestData.Environment_Dev() + err := s.DB.Model(&environment).Select("OfflineScheduleBeginEnabled", "OfflineScheduleBeginTime").Updates(&Environment{OfflineScheduleBeginEnabled: utils.PointerTo(true), OfflineScheduleBeginTime: utils.PointerTo("begin time")}).Error + s.ErrorContains(err, "violates check constraint \"offline_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlOfflineEnd() { + s.SetSuitableTestUserForDB() + environment := s.TestData.Environment_Staging() + err := s.DB.Model(&environment).Select("OfflineScheduleEndEnabled", "OfflineScheduleEndTime").Updates(&Environment{OfflineScheduleEndEnabled: utils.PointerTo(true), OfflineScheduleEndTime: utils.PointerTo("end time")}).Error + s.ErrorContains(err, "violates check constraint \"offline_valid\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlOfflineBeginPresent() { + s.SetSuitableTestUserForDB() + environment := s.TestData.Environment_Swatomation_DevBee() + err := s.DB.Model(&environment).Select("OfflineScheduleBeginTime").Updates(&Environment{OfflineScheduleBeginTime: nil}).Error + s.ErrorContains(err, "violates check constraint \"offline_schedule_begin_time_present\"") +} + +func (s *modelSuite) TestEnvironmentValidationSqlOfflineEndPresent() { + s.SetSuitableTestUserForDB() + environment := s.TestData.Environment_Swatomation_DevBee() + err := s.DB.Model(&environment).Select("OfflineScheduleEndTime").Updates(&Environment{OfflineScheduleEndTime: nil}).Error + s.ErrorContains(err, "violates check constraint \"offline_schedule_end_time_present\"") +} diff --git a/sherlock/internal/models/test_data.go b/sherlock/internal/models/test_data.go index 0579b79ee..c419a3e0d 100644 --- a/sherlock/internal/models/test_data.go +++ b/sherlock/internal/models/test_data.go @@ -630,7 +630,7 @@ func (td *testDataImpl) Environment_Swatomation_TestBee() Environment { NamePrefixesDomain: utils.PointerTo(true), HelmfileRef: utils.PointerTo("HEAD"), PreventDeletion: utils.PointerTo(false), - DeleteAfter: sql.NullTime{Time: time.Now().Add(6 * time.Hour)}, + DeleteAfter: sql.NullTime{Time: time.Now().Add(6 * time.Hour), Valid: true}, Offline: utils.PointerTo(false), } td.h.SetSuitableTestUserForDB()