Converting db size into pointer and improving the error message for v…

…alidation logic
nutanix-cloud-native · Jul 14, 2023 · ca00e83 · ca00e83
1 parent 9daf1ba
commit ca00e83
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 11 deletions.
diff --git a/api/v1alpha1/database_types.go b/api/v1alpha1/database_types.go
@@ -92,7 +92,7 @@ type Instance struct {
 	// Name of the secret holding the credentials for the database instance (password and ssh key)
 	CredentialSecret *string `json:"credentialSecret"`
 	// Size of the database instance, minimum 10 (GBs)
-	Size int `json:"size"`
+	Size *int `json:"size"`
 	// default UTC
 	// +optional
 	TimeZone *string `json:"timezone"`

diff --git a/api/v1alpha1/database_webhook.go b/api/v1alpha1/database_webhook.go
@@ -138,7 +138,7 @@ func ndbServerSpecValidatorForCreate(r *Database, allErrs field.ErrorList, ndbPa
 	}
 
 	if r.Spec.NDB.CredentialSecret == "" {
-		allErrs = append(allErrs, field.Invalid(ndbPath.Child("credentialSecret"), r.Spec.NDB.CredentialSecret, "CredentialSecret must be provided"))
+		allErrs = append(allErrs, field.Invalid(ndbPath.Child("credentialSecret"), r.Spec.NDB.CredentialSecret, "CredentialSecret must be provided in the NDB Server Spec"))
 	}
 
 	if err := util.ValidateURL(r.Spec.NDB.Server); err != nil {
@@ -156,12 +156,12 @@ func instanceSpecValidatorForCreate(r *Database, allErrs field.ErrorList, instan
 		allErrs = append(allErrs, field.Invalid(instancePath.Child("databaseInstanceName"), r.Spec.Instance.DatabaseInstanceName, "A unique Database Instance Name must be specified"))
 	}
 
-	if r.Spec.Instance.Size < 10 {
-		allErrs = append(allErrs, field.Invalid(instancePath.Child("size"), r.Spec.Instance.Size, "Initial Database size must be 10 GBs or more"))
+	if r.Spec.Instance.Size == nil || *r.Spec.Instance.Size < 10 {
+		allErrs = append(allErrs, field.Invalid(instancePath.Child("size"), r.Spec.Instance.Size, "Initial Database size must be specified with a value 10 GBs or more"))
 	}
 
 	if r.Spec.Instance.CredentialSecret == nil {
-		allErrs = append(allErrs, field.Invalid(instancePath.Child("credentialSecret"), r.Spec.Instance.CredentialSecret, "CredentialSecret must be provided"))
+		allErrs = append(allErrs, field.Invalid(instancePath.Child("credentialSecret"), r.Spec.Instance.CredentialSecret, "CredentialSecret must be provided in the Instance Spec"))
 	}
 
 	if _, isPresent := api.AllowedDatabaseTypes[*r.Spec.Instance.Type]; !isPresent {
@@ -190,20 +190,25 @@ func instanceSpecValidatorForCreate(r *Database, allErrs field.ErrorList, instan
 	}
 
 	if _, isPresent := api.AllowedLogCatchupIntervals[tmInfo.LogCatchUpFrequency]; !isPresent {
-		allErrs = append(allErrs, field.Invalid(tmPath.Child("logCatchUpFrequency"), tmInfo.LogCatchUpFrequency, "Log catchup frequency must have one of these values: {15, 30, 45, 60, 90, 120}"))
+		allErrs = append(allErrs, field.Invalid(tmPath.Child("logCatchUpFrequency"), tmInfo.LogCatchUpFrequency,
+			fmt.Sprintf("Log catchup frequency must be specified. Valid values are: %s", reflect.ValueOf(api.AllowedLogCatchupIntervals).MapKeys()),
+		))
 	}
 
-	// TODO: Does casing matter here?
 	if _, isPresent := api.AllowedWeeklySnapshotDays[tmInfo.WeeklySnapshotDay]; !isPresent {
-		allErrs = append(allErrs, field.Invalid(tmPath.Child("weeklySnapshotDay"), tmInfo.WeeklySnapshotDay, "Weekly snapshot day must have a valid value e.g. MONDAY"))
+		allErrs = append(allErrs, field.Invalid(tmPath.Child("weeklySnapshotDay"), tmInfo.WeeklySnapshotDay,
+			fmt.Sprintf("Weekly Snapshot day must be specified. Valid values are: %s", reflect.ValueOf(api.AllowedWeeklySnapshotDays).MapKeys()),
+		))
 	}
 
 	if tmInfo.MonthlySnapshotDay < 1 || tmInfo.MonthlySnapshotDay > 28 {
 		allErrs = append(allErrs, field.Invalid(tmPath.Child("monthlySnapshotDay"), tmInfo.MonthlySnapshotDay, "Monthly snapshot day value must be between 1 and 28"))
 	}
 
 	if _, isPresent := api.AllowedQuarterlySnapshotMonths[tmInfo.QuarterlySnapshotMonth]; !isPresent {
-		allErrs = append(allErrs, field.Invalid(tmPath.Child("quarterlySnapshotMonth"), tmInfo.QuarterlySnapshotMonth, "Quarterly snapshot month must be one of {Jan, Feb, Mar}"))
+		allErrs = append(allErrs, field.Invalid(tmPath.Child("quarterlySnapshotMonth"), tmInfo.QuarterlySnapshotMonth,
+			fmt.Sprintf("Quarterly snapshot month must be specified. Valid values are: %s", reflect.ValueOf(api.AllowedQuarterlySnapshotMonths).MapKeys()),
+		))
 	}
 
 	databaselog.Info("Exiting instanceSpecValidatorForCreate...")

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/controller_adapters/database.go b/controller_adapters/database.go
@@ -60,7 +60,7 @@ func (d *Database) GetDBInstanceTimeZone() string {
 }
 
 func (d *Database) GetDBInstanceSize() int {
-	return d.Spec.Instance.Size
+	return *d.Spec.Instance.Size
 }
 
 func (d *Database) GetNDBClusterId() string {

diff --git a/controllers/database_controller_test.go b/controllers/database_controller_test.go
@@ -44,6 +44,7 @@ var _ = Describe("Database controller", func() {
 		DatabaseName := "test-database-name"
 		timezoneUTC := "UTC"
 		typePostgres := "postgres"
+		databaseSize := 10
 		const namespaceName = "test-namespace"
 		const testNDBServer = "http://123.123.123.123:123"
 		const testDatabaseIP = "111.222.123.234"
@@ -92,7 +93,7 @@ var _ = Describe("Database controller", func() {
 						DatabaseInstanceName: &DatabaseName,
 						DatabaseNames:        []string{"database_1"},
 						CredentialSecret:     &instanceSecretName,
-						Size:                 10,
+						Size:                 &databaseSize,
 						TimeZone:             &timezoneUTC,
 						Type:                 &typePostgres,
 						TMInfo: &ndbv1alpha1.DBTimeMachineInfo{