Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Test for Web-hooks and converting the top-level Instance Spec fields to pointers #129

Merged
merged 23 commits into from
Jul 20, 2023
Merged

Test for Web-hooks and converting the top-level Instance Spec fields to pointers #129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
3070027
Converting TMInfo field to pointer
akshmish Jul 13, 2023
d0e8562
Converting Profiles spec into pointer
akshmish Jul 13, 2023
9daf1ba
Converting more primitive spec fields to pointers
akshmish Jul 14, 2023
ca00e83
Converting db size into pointer and improving the error message for v…
akshmish Jul 14, 2023
959ac74
Converting databaseNames field into a pointer
akshmish Jul 14, 2023
1e2ef15
Improving error messages
akshmish Jul 14, 2023
5097eb5
Handle empty database names case
akshmish Jul 14, 2023
b3e53b8
Fixing typos
akshmish Jul 17, 2023
8c311dd
Addressing the comments
akshmish Jul 17, 2023
a95382b
Refactoring method signature for validate and create
akshmish Jul 17, 2023
241435e
Merge branch 'main' into pointers
manavrajvanshi Jul 18, 2023
34acc95
Reverting the temporary change in docker repo
akshmish Jul 18, 2023
f6a8511
Fix the empty database name use case
akshmish Jul 18, 2023
6072dd4
Add a gingko test
akshmish Jul 18, 2023
c5b82f0
Adding functional tests for the Webhooks
akshmish Jul 19, 2023
1904a1a
Adding FT for TM and profiles
akshmish Jul 19, 2023
5e2debc
Fix tests
akshmish Jul 19, 2023
d9b0cb5
Adding TM missing test case
akshmish Jul 19, 2023
1a0608e
Update description
akshmish Jul 19, 2023
f9ac824
Reverting databaseNames pointers change
akshmish Jul 19, 2023
98189a4
Cleaning up test env before and after every test
akshmish Jul 20, 2023
ed9de0b
Revert artifactory url change
akshmish Jul 20, 2023
213440c
Improving comments in the code
akshmish Jul 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,9 @@ help: ## Display this help.

##@ Development

## Explanation of the statement: sed -i.bak "s/'{emptytminfo}'/{}/g" config/crd/bases/*.yaml && rm config/crd/bases/*.bak
## This is an open issue on operator-sdk: https://github.com/operator-framework/operator-sdk/issues/6346
## DBTimeMachineInfo is an optional nested struct under spec section of the database CRD. However, it does not get an empty struct value '{}'
## after running code-gen via `make manifests` in the database CRD yaml manifest. Hence we've added a marker -'{emptytminfo}' in the kubebuilder
## default anotation as the default value of an empty time machine info struct and we replace is with '{}' using sed once the manifests are generated.
## More info - https://github.com/operator-framework/operator-sdk/issues/6346#issuecomment-1482995285
.PHONY: manifests
manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
sed -i.bak "s/'{emptytminfo}'/{}/g" config/crd/bases/*.yaml && rm config/crd/bases/*.bak

.PHONY: generate
generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
Expand Down
5 changes: 2 additions & 3 deletions api/v1alpha1/database_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,10 @@ type Instance struct {
TimeZone string `json:"timezone"`
Type string `json:"type"`
// +optional
Profiles Profiles `json:"profiles"`
Profiles *Profiles `json:"profiles"`
// +optional
// +kubebuilder:default:="{emptytminfo}"
// Information related to time machine that is to be associated with this database
TMInfo DBTimeMachineInfo `json:"timeMachine"`
TMInfo *DBTimeMachineInfo `json:"timeMachine"`
}

// Time Machine details
Expand Down
147 changes: 81 additions & 66 deletions api/v1alpha1/database_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package v1alpha1

import (
"fmt"
"reflect"
"regexp"

"github.com/nutanix-cloud-native/ndb-operator/api"
Expand All @@ -42,65 +44,69 @@ func (r *Database) SetupWebhookWithManager(mgr ctrl.Manager) error {

var _ webhook.Defaulter = &Database{}

func instanceSpecDefaulterForCreate(r *Database) {
func instanceSpecDefaulterForCreate(instance *Instance) {
akshmish marked this conversation as resolved.
Show resolved Hide resolved

if len(r.Spec.Instance.DatabaseNames) == 0 {
r.Spec.Instance.DatabaseNames = api.DefaultDatabaseNames
if len(instance.DatabaseNames) == 0 {
instance.DatabaseNames = api.DefaultDatabaseNames
}

if r.Spec.Instance.TimeZone == "" {
r.Spec.Instance.TimeZone = "UTC"
if instance.TimeZone == "" {
instance.TimeZone = common.TIMEZONE_UTC
}

if r.Spec.Instance.Profiles.Compute.Id == "" && r.Spec.Instance.Profiles.Compute.Name == "" {
r.Spec.Instance.Profiles.Compute.Name = common.PROFILE_DEFAULT_OOB_SMALL_COMPUTE
}
// initialize Profiles field, if it's nil

// time machine defaulter logic
if instance.Profiles == nil {
databaselog.Info("Initialzing empty Profiles ...")
instance.Profiles = &(Profiles{})
}

tmInfo := r.Spec.Instance.TMInfo
if tmInfo.Name == "" {
tmInfo.Name = r.Spec.Instance.DatabaseInstanceName + "_TM"
// Profiles defaulting logic
if instance.Profiles.Compute.Id == "" && instance.Profiles.Compute.Name == "" {
instance.Profiles.Compute.Name = common.PROFILE_DEFAULT_OOB_SMALL_COMPUTE
}

if tmInfo.Description == "" {
tmInfo.Description = "Time Machine for " + r.Spec.Instance.DatabaseInstanceName
// initialize TM field, if it's nil
if instance.TMInfo == nil {
databaselog.Info("Initialzing empty TMInfo...")
instance.TMInfo = &(DBTimeMachineInfo{})
}

if tmInfo.SnapshotsPerDay == 0 {
tmInfo.SnapshotsPerDay = 1
// TM defaulting logic
if instance.TMInfo.SnapshotsPerDay == 0 {
instance.TMInfo.SnapshotsPerDay = 1
}

if tmInfo.SLAName == "" {
tmInfo.SLAName = common.SLA_NAME_NONE
if instance.TMInfo.SLAName == "" {
instance.TMInfo.SLAName = common.SLA_NAME_NONE
}

if tmInfo.DailySnapshotTime == "" {
tmInfo.DailySnapshotTime = "04:00:00"
if instance.TMInfo.DailySnapshotTime == "" {
instance.TMInfo.DailySnapshotTime = "04:00:00"
}

if tmInfo.LogCatchUpFrequency == 0 {
tmInfo.LogCatchUpFrequency = 30
if instance.TMInfo.LogCatchUpFrequency == 0 {
instance.TMInfo.LogCatchUpFrequency = 30
}

if tmInfo.WeeklySnapshotDay == "" {
tmInfo.WeeklySnapshotDay = "FRIDAY"
if instance.TMInfo.WeeklySnapshotDay == "" {
instance.TMInfo.WeeklySnapshotDay = "FRIDAY"
}

if tmInfo.MonthlySnapshotDay == 0 {
tmInfo.MonthlySnapshotDay = 15
if instance.TMInfo.MonthlySnapshotDay == 0 {
instance.TMInfo.MonthlySnapshotDay = 15
}

if tmInfo.QuarterlySnapshotMonth == "" {
tmInfo.QuarterlySnapshotMonth = "Jan"
if instance.TMInfo.QuarterlySnapshotMonth == "" {
instance.TMInfo.QuarterlySnapshotMonth = "Jan"
akshmish marked this conversation as resolved.
Show resolved Hide resolved
}

}

// Default implements webhook.Defaulter so a webhook will be registered for the type
func (r *Database) Default() {
databaselog.Info("Entering Defaulter logic...")
instanceSpecDefaulterForCreate(r)
instanceSpecDefaulterForCreate(&r.Spec.Instance)
databaselog.Info("Exiting Defaulter logic...")
}

Expand All @@ -109,82 +115,91 @@ func (r *Database) Default() {

var _ webhook.Validator = &Database{}

func ndbServerSpecValidatorForCreate(r *Database, allErrs field.ErrorList, ndbPath *field.Path) field.ErrorList {
func ndbServerSpecValidatorForCreate(ndb *NDB, allErrs field.ErrorList, ndbPath *field.Path) field.ErrorList {
databaselog.Info("Entering ndbServerSpecValidatorForCreate...")

if r.Spec.NDB == (NDB{}) {
allErrs = append(allErrs, field.Invalid(ndbPath, r.Spec.NDB, "NDB spec field must not be empty"))
if *ndb == (NDB{}) {
allErrs = append(allErrs, field.Invalid(ndbPath, ndb, "NDB spec field must not be empty"))
}

if err := util.ValidateUUID(r.Spec.NDB.ClusterId); err != nil {
allErrs = append(allErrs, field.Invalid(ndbPath.Child("clusterId"), r.Spec.NDB.ClusterId, "ClusterId field must be a valid UUID"))
if err := util.ValidateUUID(ndb.ClusterId); err != nil {
allErrs = append(allErrs, field.Invalid(ndbPath.Child("clusterId"), ndb.ClusterId, "ClusterId field must be a valid UUID"))
}

if r.Spec.NDB.CredentialSecret == "" {
allErrs = append(allErrs, field.Invalid(ndbPath.Child("credentialSecret"), r.Spec.NDB.CredentialSecret, "CredentialSecret must be provided"))
if ndb.CredentialSecret == "" {
allErrs = append(allErrs, field.Invalid(ndbPath.Child("credentialSecret"), ndb.CredentialSecret, "CredentialSecret must be provided in the NDB Server Spec"))
}

if err := util.ValidateURL(r.Spec.NDB.Server); err != nil {
allErrs = append(allErrs, field.Invalid(ndbPath.Child("server"), r.Spec.NDB.Server, "Server must be a valid URL"))
if err := util.ValidateURL(ndb.Server); err != nil {
allErrs = append(allErrs, field.Invalid(ndbPath.Child("server"), ndb.Server, "Server must be a valid URL"))
}

databaselog.Info("Exiting ndbServerSpecValidatorForCreate...")
return allErrs
}

func instanceSpecValidatorForCreate(r *Database, allErrs field.ErrorList, instancePath *field.Path) field.ErrorList {
func instanceSpecValidatorForCreate(instance *Instance, allErrs field.ErrorList, instancePath *field.Path) field.ErrorList {
databaselog.Info("Entering instanceSpecValidatorForCreate...")

if r.Spec.Instance.DatabaseInstanceName == "" {
allErrs = append(allErrs, field.Invalid(instancePath.Child("databaseInstanceName"), r.Spec.Instance.DatabaseInstanceName, "A unique Database Instance Name must be specified"))
databaselog.Info("Logging the Instance details inside validator method", "databaseInstance", instance)

akshmish marked this conversation as resolved.
Show resolved Hide resolved
// need to assert using a regex
if instance.DatabaseInstanceName == "" {
allErrs = append(allErrs, field.Invalid(instancePath.Child("databaseInstanceName"), instance.DatabaseInstanceName, "A valid 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 instance.Size < 10 {
allErrs = append(allErrs, field.Invalid(instancePath.Child("size"), instance.Size, "Initial Database size must be specified with a value 10 GBs or more"))
}

if r.Spec.Instance.CredentialSecret == "" {
allErrs = append(allErrs, field.Invalid(instancePath.Child("credentialSecret"), r.Spec.Instance.CredentialSecret, "CredentialSecret must be provided"))
if instance.CredentialSecret == "" {
allErrs = append(allErrs, field.Invalid(instancePath.Child("credentialSecret"), instance.CredentialSecret, "CredentialSecret must be provided in the Instance Spec"))
}

if _, isPresent := api.AllowedDatabaseTypes[r.Spec.Instance.Type]; !isPresent {
allErrs = append(allErrs, field.Invalid(instancePath.Child("type"), r.Spec.Instance.Type, "A valid database type must be specified"))
if _, isPresent := api.AllowedDatabaseTypes[instance.Type]; !isPresent {
allErrs = append(allErrs, field.Invalid(instancePath.Child("type"), instance.Type,
fmt.Sprintf("A valid database type must be specified. Valid values are: %s", reflect.ValueOf(api.AllowedDatabaseTypes).MapKeys()),
))
}

if _, isPresent := api.ClosedSourceDatabaseTypes[r.Spec.Instance.Type]; isPresent {
if r.Spec.Instance.Profiles == (Profiles{}) || r.Spec.Instance.Profiles.Software == (Profile{}) {
allErrs = append(allErrs, field.Invalid(instancePath.Child("profiles"), r.Spec.Instance.Profiles, "Software Profile must be provided for the closed-source database engines"))
if _, isPresent := api.ClosedSourceDatabaseTypes[instance.Type]; isPresent {
if instance.Profiles == &(Profiles{}) || instance.Profiles.Software == (Profile{}) {
allErrs = append(allErrs, field.Invalid(instancePath.Child("profiles").Child("software"), instance.Profiles.Software, "Software Profile must be provided for the closed-source database engines"))
}
}

// validating time machine info
tmPath := instancePath.Child("timeMachine")
tmInfo := r.Spec.Instance.TMInfo

dailySnapshotTimeRegex := regexp.MustCompile(`^(2[0-3]|[01][0-9]):[0-5][0-9]:[0-5][0-9]$`)
if isMatch := dailySnapshotTimeRegex.MatchString(tmInfo.DailySnapshotTime); !isMatch {
allErrs = append(allErrs, field.Invalid(tmPath.Child("dailySnapshotTime"), tmInfo.DailySnapshotTime, "Invalid time format for the daily snapshot time. Use the 24-hour format (HH:MM:SS)."))
if isMatch := dailySnapshotTimeRegex.MatchString(instance.TMInfo.DailySnapshotTime); !isMatch {
allErrs = append(allErrs, field.Invalid(tmPath.Child("dailySnapshotTime"), instance.TMInfo.DailySnapshotTime, "Invalid time format for the daily snapshot time. Use the 24-hour format (HH:MM:SS)."))
}

if tmInfo.SnapshotsPerDay < 1 || tmInfo.SnapshotsPerDay > 6 {
allErrs = append(allErrs, field.Invalid(tmPath.Child("snapshotsPerDay"), tmInfo.SnapshotsPerDay, "Number of snapshots per day should be within 1 to 6"))
if instance.TMInfo.SnapshotsPerDay < 1 || instance.TMInfo.SnapshotsPerDay > 6 {
allErrs = append(allErrs, field.Invalid(tmPath.Child("snapshotsPerDay"), instance.TMInfo.SnapshotsPerDay, "Number of snapshots per day should be within 1 to 6"))
}

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}"))
if _, isPresent := api.AllowedLogCatchupFrequencyInMinutes[instance.TMInfo.LogCatchUpFrequency]; !isPresent {
allErrs = append(allErrs, field.Invalid(tmPath.Child("logCatchUpFrequency"), instance.TMInfo.LogCatchUpFrequency,
fmt.Sprintf("Log catchup frequency must be specified. Valid values are: %s", reflect.ValueOf(api.AllowedLogCatchupFrequencyInMinutes).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"))
if _, isPresent := api.AllowedWeeklySnapshotDays[instance.TMInfo.WeeklySnapshotDay]; !isPresent {
allErrs = append(allErrs, field.Invalid(tmPath.Child("weeklySnapshotDay"), instance.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 instance.TMInfo.MonthlySnapshotDay < 1 || instance.TMInfo.MonthlySnapshotDay > 28 {
allErrs = append(allErrs, field.Invalid(tmPath.Child("monthlySnapshotDay"), instance.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}"))
if _, isPresent := api.AllowedQuarterlySnapshotMonths[instance.TMInfo.QuarterlySnapshotMonth]; !isPresent {
allErrs = append(allErrs, field.Invalid(tmPath.Child("quarterlySnapshotMonth"), instance.TMInfo.QuarterlySnapshotMonth,
fmt.Sprintf("Quarterly snapshot month must be specified. Valid values are: %s", reflect.ValueOf(api.AllowedQuarterlySnapshotMonths).MapKeys()),
akshmish marked this conversation as resolved.
Show resolved Hide resolved
))
}

databaselog.Info("Exiting instanceSpecValidatorForCreate...")
Expand All @@ -195,8 +210,8 @@ func instanceSpecValidatorForCreate(r *Database, allErrs field.ErrorList, instan
func (r *Database) ValidateCreate() error {
databaselog.Info("Entering ValidateCreate...")

ndbSpecErrors := ndbServerSpecValidatorForCreate(r, field.ErrorList{}, field.NewPath("spec").Child("ndb"))
dbSpecErrors := instanceSpecValidatorForCreate(r, field.ErrorList{}, field.NewPath("spec").Child("instance"))
ndbSpecErrors := ndbServerSpecValidatorForCreate(&r.Spec.NDB, field.ErrorList{}, field.NewPath("spec").Child("ndb"))
dbSpecErrors := instanceSpecValidatorForCreate(&r.Spec.Instance, field.ErrorList{}, field.NewPath("spec").Child("databaseInstance"))

allErrs := append(ndbSpecErrors, dbSpecErrors...)

Expand Down
Loading