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 :/
broadinstitute · Nov 16, 2023 · 81ad492 · 81ad492
1 parent dd5fb7d
commit 81ad492
Show file tree

Hide file tree

Showing 5 changed files with 237 additions and 4 deletions.
diff --git a/sherlock/db/migrations/000062_environment_v3.down.sql 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
@@ -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
@@ -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
@@ -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
@@ -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()