From 9ae7a7f6efc790c44e98da0c77f076ba5238b89c Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Mon, 18 Sep 2023 20:11:43 +0600 Subject: [PATCH 01/22] EVEREST-256 Improved validation of database cluster CR --- api/database_cluster.go | 4 + api/validation.go | 80 ++++++++++++++-- .../client/customresources/databaseengines.go | 95 +++++++++++++++++++ pkg/kubernetes/client/database_engine.go | 18 ++++ pkg/kubernetes/client/gen.go | 2 +- pkg/kubernetes/client/kubeclient_interface.go | 4 + .../client/mock_kube_client_connector.go | 52 ++++++++++ pkg/kubernetes/database_engine.go | 33 +++++++ 8 files changed, 278 insertions(+), 10 deletions(-) create mode 100644 pkg/kubernetes/client/customresources/databaseengines.go create mode 100644 pkg/kubernetes/client/database_engine.go create mode 100644 pkg/kubernetes/database_engine.go diff --git a/api/database_cluster.go b/api/database_cluster.go index 4798e76d..e4f2d994 100644 --- a/api/database_cluster.go +++ b/api/database_cluster.go @@ -42,6 +42,10 @@ func (e *EverestServer) CreateDatabaseCluster(ctx echo.Context, kubernetesID str return ctx.JSON(http.StatusBadRequest, Error{Message: pointer.ToString(err.Error())}) } + if err := e.validateDatabaseClusterCR(ctx, kubernetesID, dbc); err != nil { + return ctx.JSON(http.StatusBadRequest, Error{Message: pointer.ToString(err.Error())}) + } + _, kubeClient, code, err := e.initKubeClient(ctx.Request().Context(), kubernetesID) if err != nil { return ctx.JSON(code, Error{Message: pointer.ToString(err.Error())}) diff --git a/api/validation.go b/api/validation.go index 3c1f07a5..115b9e0a 100644 --- a/api/validation.go +++ b/api/validation.go @@ -17,6 +17,7 @@ package api import ( + "errors" "fmt" "net/http" "net/url" @@ -28,7 +29,6 @@ import ( "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/s3" "github.com/labstack/echo/v4" - "github.com/pkg/errors" "go.uber.org/zap" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -36,37 +36,51 @@ import ( "github.com/percona/percona-everest-backend/model" ) +const ( + pxcDeploymentName = "percona-xtradb-cluster-operator" + psmdbDeploymentName = "percona-server-mongodb-operator" + pgDeploymentName = "percona-postgresql-operator" + engineTypePXC = "pxc" + engineTypePSMDB = "psmdb" + engineTypePG = "postgresql" +) + var ( errDBCEmptyMetadata = errors.New("DatabaseCluster's Metadata should not be empty") errDBCNameEmpty = errors.New("DatabaseCluster's metadata.name should not be empty") errDBCNameWrongFormat = errors.New("DatabaseCluster's metadata.name should be a string") + operatorEngine = map[string]string{ + engineTypePXC: pxcDeploymentName, + engineTypePSMDB: psmdbDeploymentName, + engineTypePG: pgDeploymentName, + } ) // ErrNameNotRFC1035Compatible when the given fieldName doesn't contain RFC 1035 compatible string. func ErrNameNotRFC1035Compatible(fieldName string) error { - return errors.Errorf(`'%s' is not RFC 1035 compatible. The name should contain only lowercase alphanumeric characters or '-', start with an alphabetic character, end with an alphanumeric character`, + return fmt.Errorf(`'%s' is not RFC 1035 compatible. The name should contain only lowercase alphanumeric characters or '-', start with an alphabetic character, end with an alphanumeric character`, fieldName, ) } // ErrNameTooLong when the given fieldName is longer than expected. func ErrNameTooLong(fieldName string) error { - return errors.Errorf("'%s' can be at most 22 characters long", fieldName) + return fmt.Errorf("'%s' can be at most 22 characters long", fieldName) } // ErrCreateStorageNotSupported appears when trying to create a storage of a type that is not supported. func ErrCreateStorageNotSupported(storageType string) error { - return errors.Errorf("Creating storage is not implemented for '%s'", storageType) + return fmt.Errorf("Creating storage is not implemented for '%s'", storageType) } // ErrUpdateStorageNotSupported appears when trying to update a storage of a type that is not supported. func ErrUpdateStorageNotSupported(storageType string) error { - return errors.Errorf("Updating storage is not implemented for '%s'", storageType) + return fmt.Errorf("Updating storage is not implemented for '%s'", storageType) } // ErrInvalidURL when the given fieldName contains invalid URL. func ErrInvalidURL(fieldName string) error { - return errors.Errorf("'%s' is an invalid URL", fieldName) + return fmt.Errorf("'%s' is an invalid URL", fieldName) } // validates names to be RFC-1035 compatible https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names @@ -231,14 +245,14 @@ func validateCreateMonitoringInstanceRequest(ctx echo.Context) (*CreateMonitorin switch params.Type { case MonitoringInstanceCreateParamsTypePmm: if params.Pmm == nil { - return nil, errors.Errorf("pmm key is required for type %s", params.Type) + return nil, fmt.Errorf("pmm key is required for type %s", params.Type) } if params.Pmm.ApiKey == "" && params.Pmm.User == "" && params.Pmm.Password == "" { return nil, errors.New("one of pmm.apiKey, pmm.user or pmm.password fields is required") } default: - return nil, errors.Errorf("monitoring type %s is not supported", params.Type) + return nil, fmt.Errorf("monitoring type %s is not supported", params.Type) } return ¶ms, nil @@ -274,7 +288,7 @@ func validateUpdateMonitoringInstanceType(params UpdateMonitoringInstanceJSONReq return nil case MonitoringInstanceUpdateParamsTypePmm: if params.Pmm == nil { - return errors.Errorf("pmm key is required for type %s", params.Type) + return fmt.Errorf("pmm key is required for type %s", params.Type) } default: return errors.New("this monitoring type is not supported") @@ -319,3 +333,51 @@ func (e *EverestServer) validateDBClusterAccess(ctx echo.Context, kubernetesID, return nil } + +func (e *EverestServer) validateDatabaseClusterCR(ctx echo.Context, kubernetesID string, databaseCluster *DatabaseCluster) error { + _, kubeClient, _, err := e.initKubeClient(ctx.Request().Context(), kubernetesID) + if err != nil { + return err + } + engineName, ok := operatorEngine[databaseCluster.Spec.Engine.Type] + if !ok { + return errors.New("Unsupported database engine") + } + engine, err := kubeClient.GetDatabaseEngine(ctx.Request().Context(), engineName) + if err != nil { + return err + } + if databaseCluster.Spec.Engine.Version != nil { + if len(engine.Spec.AllowedVersions) != 0 && !containsVersion(*databaseCluster.Spec.Engine.Version, engine.Spec.AllowedVersions) { + return fmt.Errorf("Using %s version for %s is not allowed", databaseCluster.Spec.Engine.Version, databaseCluster.Spec.Engine.Type) + } + if _, ok := engine.Status.AvailableVersions.Engine[*databaseCluster.Spec.Engine.Version]; !ok { + return fmt.Errorf("%s is not in available versions list", *databaseCluster.Spec.Engine.Version) + } + } + if databaseCluster.Spec.Proxy.Type != nil { + if databaseCluster.Spec.Engine.Type == engineTypePXC && (*databaseCluster.Spec.Proxy.Type != "proxysql" || *databaseCluster.Spec.Proxy.Type != "haproxy") { + return errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") + } + + if databaseCluster.Spec.Engine.Type == engineTypePG && *databaseCluster.Spec.Proxy.Type != "pgbouncer" { + return errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") + } + if databaseCluster.Spec.Engine.Type == engineTypePSMDB && *databaseCluster.Spec.Proxy.Type != "mongos" { + return errors.New("You can use only Mongos as a proxy type for MongoDB clusters") + } + } + return nil +} + +func containsVersion(version string, versions []string) bool { + if version == "" { + return true + } + for _, allowedVersion := range versions { + if version == allowedVersion { + return true + } + } + return false +} diff --git a/pkg/kubernetes/client/customresources/databaseengines.go b/pkg/kubernetes/client/customresources/databaseengines.go new file mode 100644 index 00000000..b2fa5b5e --- /dev/null +++ b/pkg/kubernetes/client/customresources/databaseengines.go @@ -0,0 +1,95 @@ +// percona-everest-backend +// Copyright (C) 2023 Percona LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package customresources + +import ( + "context" + + everestv1alpha1 "github.com/percona/everest-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" +) + +const ( + dbEnginesAPIKind = "databaseengines" +) + +// DBEngines returns a db engine. +func (c *Client) DBEngines(namespace string) DBEngineInterface { //nolint:ireturn + return &dbEngineClient{ + restClient: c.restClient, + namespace: namespace, + } +} + +type dbEngineClient struct { + restClient rest.Interface + namespace string +} + +// DBEngineInterface supports list, get and watch methods. +type DBEngineInterface interface { + List(ctx context.Context, opts metav1.ListOptions) (*everestv1alpha1.DatabaseEngineList, error) + Get(ctx context.Context, name string, options metav1.GetOptions) (*everestv1alpha1.DatabaseEngine, error) + Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) +} + +// List lists database clusters based on opts. +func (c *dbEngineClient) List(ctx context.Context, opts metav1.ListOptions) (*everestv1alpha1.DatabaseEngineList, error) { + result := &everestv1alpha1.DatabaseEngineList{} + err := c.restClient. + Get(). + Namespace(c.namespace). + Resource(dbEnginesAPIKind). + VersionedParams(&opts, scheme.ParameterCodec). + Do(ctx). + Into(result) + return result, err +} + +// Get retrieves database cluster based on opts. +func (c *dbEngineClient) Get( + ctx context.Context, + name string, + opts metav1.GetOptions, +) (*everestv1alpha1.DatabaseEngine, error) { + result := &everestv1alpha1.DatabaseEngine{} + err := c.restClient. + Get(). + Namespace(c.namespace). + Resource(dbEnginesAPIKind). + VersionedParams(&opts, scheme.ParameterCodec). + Name(name). + Do(ctx). + Into(result) + return result, err +} + +// Watch starts a watch based on opts. +func (c *dbEngineClient) Watch( //nolint:ireturn + ctx context.Context, + opts metav1.ListOptions, +) (watch.Interface, error) { + opts.Watch = true + return c.restClient. + Get(). + Namespace(c.namespace). + Resource(dbEnginesAPIKind). + VersionedParams(&opts, scheme.ParameterCodec). + Watch(ctx) +} diff --git a/pkg/kubernetes/client/database_engine.go b/pkg/kubernetes/client/database_engine.go new file mode 100644 index 00000000..452598df --- /dev/null +++ b/pkg/kubernetes/client/database_engine.go @@ -0,0 +1,18 @@ +package client + +import ( + "context" + + everestv1alpha1 "github.com/percona/everest-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// ListDatabaseEngines returns list of managed database clusters. +func (c *Client) ListDatabaseEngines(ctx context.Context) (*everestv1alpha1.DatabaseEngineList, error) { + return c.customClientSet.DBEngines(c.namespace).List(ctx, metav1.ListOptions{}) +} + +// GetDatabaseEngine returns database clusters by provided name. +func (c *Client) GetDatabaseEngine(ctx context.Context, name string) (*everestv1alpha1.DatabaseEngine, error) { + return c.customClientSet.DBEngines(c.namespace).Get(ctx, name, metav1.GetOptions{}) +} diff --git a/pkg/kubernetes/client/gen.go b/pkg/kubernetes/client/gen.go index b9082a79..4eeedab8 100644 --- a/pkg/kubernetes/client/gen.go +++ b/pkg/kubernetes/client/gen.go @@ -15,5 +15,5 @@ package client -//go:generate ../../../bin/ifacemaker -f backup_storage.go -f client.go -f database_cluster.go -f monitoring_config.go -f namespace.go -f node.go -f pod.go -f resource.go -f secret.go -f storage.go -s Client -i KubeClientConnector -p client -o kubeclient_interface.go +//go:generate ../../../bin/ifacemaker -f backup_storage.go -f client.go -f database_cluster.go -f database_engine.go -f monitoring_config.go -f namespace.go -f node.go -f pod.go -f resource.go -f secret.go -f storage.go -s Client -i KubeClientConnector -p client -o kubeclient_interface.go //go:generate ../../../bin/mockery --name=KubeClientConnector --case=snake --inpackage diff --git a/pkg/kubernetes/client/kubeclient_interface.go b/pkg/kubernetes/client/kubeclient_interface.go index f915d960..6b6d0399 100644 --- a/pkg/kubernetes/client/kubeclient_interface.go +++ b/pkg/kubernetes/client/kubeclient_interface.go @@ -41,6 +41,10 @@ type KubeClientConnector interface { ListDatabaseClusters(ctx context.Context) (*everestv1alpha1.DatabaseClusterList, error) // GetDatabaseCluster returns database clusters by provided name. GetDatabaseCluster(ctx context.Context, name string) (*everestv1alpha1.DatabaseCluster, error) + // ListDatabaseEngines returns list of managed database clusters. + ListDatabaseEngines(ctx context.Context) (*everestv1alpha1.DatabaseEngineList, error) + // GetDatabaseEngine returns database clusters by provided name. + GetDatabaseEngine(ctx context.Context, name string) (*everestv1alpha1.DatabaseEngine, error) // CreateMonitoringConfig creates an MonitoringConfig. CreateMonitoringConfig(ctx context.Context, mc *everestv1alpha1.MonitoringConfig) error // GetMonitoringConfig returns the MonitoringConfig. diff --git a/pkg/kubernetes/client/mock_kube_client_connector.go b/pkg/kubernetes/client/mock_kube_client_connector.go index a94914bd..cefd0896 100644 --- a/pkg/kubernetes/client/mock_kube_client_connector.go +++ b/pkg/kubernetes/client/mock_kube_client_connector.go @@ -238,6 +238,32 @@ func (_m *MockKubeClientConnector) GetDatabaseCluster(ctx context.Context, name return r0, r1 } +// GetDatabaseEngine provides a mock function with given fields: ctx, name +func (_m *MockKubeClientConnector) GetDatabaseEngine(ctx context.Context, name string) (*v1alpha1.DatabaseEngine, error) { + ret := _m.Called(ctx, name) + + var r0 *v1alpha1.DatabaseEngine + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) (*v1alpha1.DatabaseEngine, error)); ok { + return rf(ctx, name) + } + if rf, ok := ret.Get(0).(func(context.Context, string) *v1alpha1.DatabaseEngine); ok { + r0 = rf(ctx, name) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*v1alpha1.DatabaseEngine) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, name) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetMonitoringConfig provides a mock function with given fields: ctx, name func (_m *MockKubeClientConnector) GetMonitoringConfig(ctx context.Context, name string) (*v1alpha1.MonitoringConfig, error) { ret := _m.Called(ctx, name) @@ -500,6 +526,32 @@ func (_m *MockKubeClientConnector) ListDatabaseClusters(ctx context.Context) (*v return r0, r1 } +// ListDatabaseEngines provides a mock function with given fields: ctx +func (_m *MockKubeClientConnector) ListDatabaseEngines(ctx context.Context) (*v1alpha1.DatabaseEngineList, error) { + ret := _m.Called(ctx) + + var r0 *v1alpha1.DatabaseEngineList + var r1 error + if rf, ok := ret.Get(0).(func(context.Context) (*v1alpha1.DatabaseEngineList, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(context.Context) *v1alpha1.DatabaseEngineList); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*v1alpha1.DatabaseEngineList) + } + } + + if rf, ok := ret.Get(1).(func(context.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // ListMonitoringConfigs provides a mock function with given fields: ctx func (_m *MockKubeClientConnector) ListMonitoringConfigs(ctx context.Context) (*v1alpha1.MonitoringConfigList, error) { ret := _m.Called(ctx) diff --git a/pkg/kubernetes/database_engine.go b/pkg/kubernetes/database_engine.go new file mode 100644 index 00000000..b14ee9be --- /dev/null +++ b/pkg/kubernetes/database_engine.go @@ -0,0 +1,33 @@ +// percona-everest-backend +// Copyright (C) 2023 Percona LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package kubernetes ... +package kubernetes + +import ( + "context" + + everestv1alpha1 "github.com/percona/everest-operator/api/v1alpha1" +) + +// ListDatabaseEngines returns list of managed database clusters. +func (k *Kubernetes) ListDatabaseEngines(ctx context.Context) (*everestv1alpha1.DatabaseEngineList, error) { + return k.client.ListDatabaseEngines(ctx) +} + +// GetDatabaseEngine returns database clusters by provided name. +func (k *Kubernetes) GetDatabaseEngine(ctx context.Context, name string) (*everestv1alpha1.DatabaseEngine, error) { + return k.client.GetDatabaseEngine(ctx, name) +} From 6298452d15a9a1d69719a40dcb84414784acbbe4 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Mon, 18 Sep 2023 21:18:05 +0600 Subject: [PATCH 02/22] EVEREST-256 Small fixes --- .golangci.yml | 2 -- api/validation.go | 25 ++++++++++++++++--------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 7b3c9ba6..c190e70d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,8 +9,6 @@ linters-settings: - $all - "!$test" deny: - - pkg: errors - desc: use "github.com/pkg/errors" instead - pkg: github.com/gogo/protobuf/proto desc: use "github.com/golang/protobuf/proto" instead diff --git a/api/validation.go b/api/validation.go index 115b9e0a..6354b6db 100644 --- a/api/validation.go +++ b/api/validation.go @@ -356,15 +356,8 @@ func (e *EverestServer) validateDatabaseClusterCR(ctx echo.Context, kubernetesID } } if databaseCluster.Spec.Proxy.Type != nil { - if databaseCluster.Spec.Engine.Type == engineTypePXC && (*databaseCluster.Spec.Proxy.Type != "proxysql" || *databaseCluster.Spec.Proxy.Type != "haproxy") { - return errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") - } - - if databaseCluster.Spec.Engine.Type == engineTypePG && *databaseCluster.Spec.Proxy.Type != "pgbouncer" { - return errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") - } - if databaseCluster.Spec.Engine.Type == engineTypePSMDB && *databaseCluster.Spec.Proxy.Type != "mongos" { - return errors.New("You can use only Mongos as a proxy type for MongoDB clusters") + if err := validateProxy(databaseCluster.Spec.Engine.Type, string(*databaseCluster.Spec.Proxy.Type)); err != nil { + return err } } return nil @@ -381,3 +374,17 @@ func containsVersion(version string, versions []string) bool { } return false } + +func validateProxy(engineType, proxyType string) error { + if engineType == engineTypePXC && (proxyType != "proxysql" || proxyType != "haproxy") { + return errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") + } + + if engineType == engineTypePG && proxyType != "pgbouncer" { + return errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") + } + if engineType == engineTypePSMDB && proxyType != "mongos" { + return errors.New("You can use only Mongos as a proxy type for MongoDB clusters") + } + return nil +} From 62ddc5aa0624aa8fa2e2a207e90bb198004210bd Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Tue, 19 Sep 2023 13:08:49 +0600 Subject: [PATCH 03/22] EVEREST-256 build fixes --- api/database_cluster.go | 4 ---- api/validation.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/api/database_cluster.go b/api/database_cluster.go index e4f2d994..6f0e1787 100644 --- a/api/database_cluster.go +++ b/api/database_cluster.go @@ -38,10 +38,6 @@ func (e *EverestServer) CreateDatabaseCluster(ctx echo.Context, kubernetesID str }) } - if err := validateCreateDatabaseClusterRequest(*dbc); err != nil { - return ctx.JSON(http.StatusBadRequest, Error{Message: pointer.ToString(err.Error())}) - } - if err := e.validateDatabaseClusterCR(ctx, kubernetesID, dbc); err != nil { return ctx.JSON(http.StatusBadRequest, Error{Message: pointer.ToString(err.Error())}) } diff --git a/api/validation.go b/api/validation.go index 6354b6db..984c2b5e 100644 --- a/api/validation.go +++ b/api/validation.go @@ -31,6 +31,7 @@ import ( "github.com/labstack/echo/v4" "go.uber.org/zap" k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" "github.com/percona/percona-everest-backend/cmd/config" "github.com/percona/percona-everest-backend/model" @@ -54,6 +55,9 @@ var ( engineTypePSMDB: psmdbDeploymentName, engineTypePG: pgDeploymentName, } + minStorageQuantity = resource.MustParse("1G") + minCPUQuantity = resource.MustParse("600m") + minMemQuantity = resource.MustParse("512m") ) // ErrNameNotRFC1035Compatible when the given fieldName doesn't contain RFC 1035 compatible string. @@ -335,6 +339,10 @@ func (e *EverestServer) validateDBClusterAccess(ctx echo.Context, kubernetesID, } func (e *EverestServer) validateDatabaseClusterCR(ctx echo.Context, kubernetesID string, databaseCluster *DatabaseCluster) error { + if err := validateCreateDatabaseClusterRequest(*databaseCluster); err != nil { + return err + } + _, kubeClient, _, err := e.initKubeClient(ctx.Request().Context(), kubernetesID) if err != nil { return err @@ -360,6 +368,16 @@ func (e *EverestServer) validateDatabaseClusterCR(ctx echo.Context, kubernetesID return err } } + if err := validateBackupSpec(databaseCluster); err != nil { + return err + } + if err := validateResourceLimits(databaseCluster); err != nil { + return err + } + if err := validateStorageLimits(databaseCluster); err != nil { + return err + } + return nil } @@ -388,3 +406,13 @@ func validateProxy(engineType, proxyType string) error { } return nil } + +func validateBackupSpec(cluster *DatabaseCluster) error { + return nil +} +func validateResourceLimits(cluster *DatabaseCluster) error { + return nil +} +func validateStorageLimits(cluster *DatabaseCluster) error { + return nil +} From 21b03d9222ad486f6519bea27110f7490e9b9b7d Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Tue, 19 Sep 2023 13:43:08 +0600 Subject: [PATCH 04/22] EVEREST-256 Added resources validation --- api/validation.go | 61 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 53 insertions(+), 8 deletions(-) diff --git a/api/validation.go b/api/validation.go index 984c2b5e..f11de593 100644 --- a/api/validation.go +++ b/api/validation.go @@ -57,7 +57,7 @@ var ( } minStorageQuantity = resource.MustParse("1G") minCPUQuantity = resource.MustParse("600m") - minMemQuantity = resource.MustParse("512m") + minMemQuantity = resource.MustParse("512M") ) // ErrNameNotRFC1035Compatible when the given fieldName doesn't contain RFC 1035 compatible string. @@ -374,10 +374,6 @@ func (e *EverestServer) validateDatabaseClusterCR(ctx echo.Context, kubernetesID if err := validateResourceLimits(databaseCluster); err != nil { return err } - if err := validateStorageLimits(databaseCluster); err != nil { - return err - } - return nil } @@ -411,8 +407,57 @@ func validateBackupSpec(cluster *DatabaseCluster) error { return nil } func validateResourceLimits(cluster *DatabaseCluster) error { - return nil -} -func validateStorageLimits(cluster *DatabaseCluster) error { + if cluster.Spec.Engine.Resources == nil { + return errors.New("Please specify resource limits for the cluster") + } + if cluster.Spec.Engine.Resources.Cpu == nil { + return errors.New("CPU limits should be above 600m and cannot be empty") + } + if cluster.Spec.Engine.Resources.Memory == nil { + return errors.New("Memory limits should be above 512M and cannot be empty") + } + cpuStr, err := cluster.Spec.Engine.Resources.Cpu.AsDatabaseClusterSpecEngineResourcesCpu1() + if err == nil { + cpu, err := resource.ParseQuantity(cpuStr) + if err != nil { + return err + } + if cpu.Cmp(minCPUQuantity) == -1 { + return errors.New("CPU limits should be above 600m") + } + } + _, err = cluster.Spec.Engine.Resources.Cpu.AsDatabaseClusterSpecEngineResourcesCpu0() + if err == nil { + return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") + } + _, err = cluster.Spec.Engine.Resources.Memory.AsDatabaseClusterSpecEngineResourcesMemory0() + if err == nil { + return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") + } + memStr, err := cluster.Spec.Engine.Resources.Memory.AsDatabaseClusterSpecEngineResourcesMemory1() + if err == nil { + mem, err := resource.ParseQuantity(memStr) + if err != nil { + return err + } + if mem.Cmp(minMemQuantity) == -1 { + return errors.New("Memory limits should be above 512M") + } + } + _, err = cluster.Spec.Engine.Storage.Size.AsDatabaseClusterSpecEngineStorageSize0() + if err == nil { + return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") + } + sizeStr, err := cluster.Spec.Engine.Storage.Size.AsDatabaseClusterSpecEngineStorageSize1() + if err == nil { + + size, err := resource.ParseQuantity(sizeStr) + if err != nil { + return err + } + if size.Cmp(minStorageQuantity) == -1 { + return errors.New("Storage size should be above 1G") + } + } return nil } From 04b14cfdf8da6e4c40b7a3999a5e21a61b549e6a Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Tue, 19 Sep 2023 13:47:09 +0600 Subject: [PATCH 05/22] EVEREST-256 Fixed linter --- api/validation.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/api/validation.go b/api/validation.go index f11de593..37daff34 100644 --- a/api/validation.go +++ b/api/validation.go @@ -357,7 +357,7 @@ func (e *EverestServer) validateDatabaseClusterCR(ctx echo.Context, kubernetesID } if databaseCluster.Spec.Engine.Version != nil { if len(engine.Spec.AllowedVersions) != 0 && !containsVersion(*databaseCluster.Spec.Engine.Version, engine.Spec.AllowedVersions) { - return fmt.Errorf("Using %s version for %s is not allowed", databaseCluster.Spec.Engine.Version, databaseCluster.Spec.Engine.Type) + return fmt.Errorf("Using %s version for %s is not allowed", *databaseCluster.Spec.Engine.Version, databaseCluster.Spec.Engine.Type) } if _, ok := engine.Status.AvailableVersions.Engine[*databaseCluster.Spec.Engine.Version]; !ok { return fmt.Errorf("%s is not in available versions list", *databaseCluster.Spec.Engine.Version) @@ -390,8 +390,10 @@ func containsVersion(version string, versions []string) bool { } func validateProxy(engineType, proxyType string) error { - if engineType == engineTypePXC && (proxyType != "proxysql" || proxyType != "haproxy") { - return errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") + if engineType == engineTypePXC { + if proxyType != "proxysql" || proxyType != "haproxy" { + return errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") + } } if engineType == engineTypePG && proxyType != "pgbouncer" { From ee49b10438ab57ffb54726a6cdde9a02c494796a Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Tue, 19 Sep 2023 13:55:29 +0600 Subject: [PATCH 06/22] EVEREST-256 Backup spec validation --- api/validation.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/api/validation.go b/api/validation.go index 37daff34..9c22961e 100644 --- a/api/validation.go +++ b/api/validation.go @@ -406,6 +406,24 @@ func validateProxy(engineType, proxyType string) error { } func validateBackupSpec(cluster *DatabaseCluster) error { + if cluster.Spec.Backup == nil { + return nil + } + if !cluster.Spec.Backup.Enabled { + return nil + } + if cluster.Spec.Backup.Schedules == nil { + return errors.New("Please specify at lease one backup schedule") + } + + for _, schedule := range *cluster.Spec.Backup.Schedules { + if schedule.Name == "" { + return errors.New("'name' field cannot be empty") + } + if schedule.Enabled && schedule.BackupStorageName == "" { + return errors.New("'backupStorageName' field cannot be empty when schedule is enabled") + } + } return nil } func validateResourceLimits(cluster *DatabaseCluster) error { From b360eb69038a41c41c3a6cbbf25e2e57b70e22fc Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Tue, 19 Sep 2023 14:10:50 +0600 Subject: [PATCH 07/22] EVEREST-256 Fixed logic for proxy validation --- api/validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/validation.go b/api/validation.go index 9c22961e..398292a7 100644 --- a/api/validation.go +++ b/api/validation.go @@ -391,7 +391,7 @@ func containsVersion(version string, versions []string) bool { func validateProxy(engineType, proxyType string) error { if engineType == engineTypePXC { - if proxyType != "proxysql" || proxyType != "haproxy" { + if proxyType != "proxysql" && proxyType != "haproxy" { return errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") } } From 22fe77dd7531ff9f468d9878f392541187818687 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Tue, 19 Sep 2023 15:36:15 +0600 Subject: [PATCH 08/22] EVEREST-256 Fixed linter --- .golangci.yml | 1 + api/validation.go | 94 +++++++++++++------ .../customresources/databaseclusters.go | 4 +- .../client/customresources/databaseengines.go | 4 +- 4 files changed, 70 insertions(+), 33 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index c190e70d..00da60a2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -31,6 +31,7 @@ linters-settings: linters: enable-all: true disable: + - goerr113 # not useful after migration to the standard errors - exhaustruct # not useful - exhaustivestruct # annoying and duplicates exhaustruct - godox # fails to be nolint-ed when necessary diff --git a/api/validation.go b/api/validation.go index 398292a7..09305a90 100644 --- a/api/validation.go +++ b/api/validation.go @@ -29,6 +29,7 @@ import ( "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/s3" "github.com/labstack/echo/v4" + everestv1alpha1 "github.com/percona/everest-operator/api/v1alpha1" "go.uber.org/zap" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" @@ -50,14 +51,15 @@ var ( errDBCEmptyMetadata = errors.New("DatabaseCluster's Metadata should not be empty") errDBCNameEmpty = errors.New("DatabaseCluster's metadata.name should not be empty") errDBCNameWrongFormat = errors.New("DatabaseCluster's metadata.name should be a string") - operatorEngine = map[string]string{ + //nolint:gochecknoglobals + operatorEngine = map[string]string{ engineTypePXC: pxcDeploymentName, engineTypePSMDB: psmdbDeploymentName, engineTypePG: pgDeploymentName, } - minStorageQuantity = resource.MustParse("1G") - minCPUQuantity = resource.MustParse("600m") - minMemQuantity = resource.MustParse("512M") + minStorageQuantity = resource.MustParse("1G") //nolint:gochecknoglobals + minCPUQuantity = resource.MustParse("600m") //nolint:gochecknoglobals + minMemQuantity = resource.MustParse("512M") //nolint:gochecknoglobals ) // ErrNameNotRFC1035Compatible when the given fieldName doesn't contain RFC 1035 compatible string. @@ -74,12 +76,12 @@ func ErrNameTooLong(fieldName string) error { // ErrCreateStorageNotSupported appears when trying to create a storage of a type that is not supported. func ErrCreateStorageNotSupported(storageType string) error { - return fmt.Errorf("Creating storage is not implemented for '%s'", storageType) + return fmt.Errorf("Creating storage is not implemented for '%s'", storageType) //nolint:stylecheck } // ErrUpdateStorageNotSupported appears when trying to update a storage of a type that is not supported. func ErrUpdateStorageNotSupported(storageType string) error { - return fmt.Errorf("Updating storage is not implemented for '%s'", storageType) + return fmt.Errorf("Updating storage is not implemented for '%s'", storageType) //nolint:stylecheck } // ErrInvalidURL when the given fieldName contains invalid URL. @@ -226,7 +228,7 @@ func validateCreateBackupStorageRequest(ctx echo.Context, l *zap.SugaredLogger) // check data access if err := validateStorageAccessByCreate(params); err != nil { l.Error(err) - return nil, errors.New("Could not connect to the backup storage, please check the new credentials are correct") + return nil, errors.New("Could not connect to the backup storage, please check the new credentials are correct") //nolint:stylecheck } return ¶ms, nil @@ -349,19 +351,14 @@ func (e *EverestServer) validateDatabaseClusterCR(ctx echo.Context, kubernetesID } engineName, ok := operatorEngine[databaseCluster.Spec.Engine.Type] if !ok { - return errors.New("Unsupported database engine") + return errors.New("Unsupported database engine") //nolint:stylecheck } engine, err := kubeClient.GetDatabaseEngine(ctx.Request().Context(), engineName) if err != nil { return err } - if databaseCluster.Spec.Engine.Version != nil { - if len(engine.Spec.AllowedVersions) != 0 && !containsVersion(*databaseCluster.Spec.Engine.Version, engine.Spec.AllowedVersions) { - return fmt.Errorf("Using %s version for %s is not allowed", *databaseCluster.Spec.Engine.Version, databaseCluster.Spec.Engine.Type) - } - if _, ok := engine.Status.AvailableVersions.Engine[*databaseCluster.Spec.Engine.Version]; !ok { - return fmt.Errorf("%s is not in available versions list", *databaseCluster.Spec.Engine.Version) - } + if err := validateVersion(databaseCluster.Spec.Engine.Version, engine); err != nil { + return err } if databaseCluster.Spec.Proxy.Type != nil { if err := validateProxy(databaseCluster.Spec.Engine.Type, string(*databaseCluster.Spec.Proxy.Type)); err != nil { @@ -371,8 +368,17 @@ func (e *EverestServer) validateDatabaseClusterCR(ctx echo.Context, kubernetesID if err := validateBackupSpec(databaseCluster); err != nil { return err } - if err := validateResourceLimits(databaseCluster); err != nil { - return err + return validateResourceLimits(databaseCluster) +} + +func validateVersion(version *string, engine *everestv1alpha1.DatabaseEngine) error { + if version != nil { + if len(engine.Spec.AllowedVersions) != 0 && !containsVersion(*version, engine.Spec.AllowedVersions) { + return fmt.Errorf("Using %s version for %s is not allowed", *version, engine.Spec.Type) //nolint:stylecheck + } + if _, ok := engine.Status.AvailableVersions.Engine[*version]; !ok { + return fmt.Errorf("%s is not in available versions list", *version) + } } return nil } @@ -392,15 +398,15 @@ func containsVersion(version string, versions []string) bool { func validateProxy(engineType, proxyType string) error { if engineType == engineTypePXC { if proxyType != "proxysql" && proxyType != "haproxy" { - return errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") + return errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") //nolint:stylecheck } } if engineType == engineTypePG && proxyType != "pgbouncer" { - return errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") + return errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") //nolint:stylecheck } if engineType == engineTypePSMDB && proxyType != "mongos" { - return errors.New("You can use only Mongos as a proxy type for MongoDB clusters") + return errors.New("You can use only Mongos as a proxy type for MongoDB clusters") //nolint:stylecheck } return nil } @@ -413,7 +419,7 @@ func validateBackupSpec(cluster *DatabaseCluster) error { return nil } if cluster.Spec.Backup.Schedules == nil { - return errors.New("Please specify at lease one backup schedule") + return errors.New("Please specify at lease one backup schedule") //nolint:stylecheck } for _, schedule := range *cluster.Spec.Backup.Schedules { @@ -426,16 +432,34 @@ func validateBackupSpec(cluster *DatabaseCluster) error { } return nil } + func validateResourceLimits(cluster *DatabaseCluster) error { + if err := ensureNotEmptySpec(cluster); err != nil { + return err + } + if err := validateCPU(cluster); err != nil { + return err + } + if err := validateMemory(cluster); err != nil { + return err + } + return validateStorageSize(cluster) +} + +func ensureNotEmptySpec(cluster *DatabaseCluster) error { if cluster.Spec.Engine.Resources == nil { - return errors.New("Please specify resource limits for the cluster") + return errors.New("Please specify resource limits for the cluster") //nolint:stylecheck } if cluster.Spec.Engine.Resources.Cpu == nil { return errors.New("CPU limits should be above 600m and cannot be empty") } if cluster.Spec.Engine.Resources.Memory == nil { - return errors.New("Memory limits should be above 512M and cannot be empty") + return errors.New("Memory limits should be above 512M and cannot be empty") //nolint:stylecheck } + return nil +} + +func validateCPU(cluster *DatabaseCluster) error { cpuStr, err := cluster.Spec.Engine.Resources.Cpu.AsDatabaseClusterSpecEngineResourcesCpu1() if err == nil { cpu, err := resource.ParseQuantity(cpuStr) @@ -448,11 +472,15 @@ func validateResourceLimits(cluster *DatabaseCluster) error { } _, err = cluster.Spec.Engine.Resources.Cpu.AsDatabaseClusterSpecEngineResourcesCpu0() if err == nil { - return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") + return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") //nolint:stylecheck } - _, err = cluster.Spec.Engine.Resources.Memory.AsDatabaseClusterSpecEngineResourcesMemory0() + return nil +} + +func validateMemory(cluster *DatabaseCluster) error { + _, err := cluster.Spec.Engine.Resources.Memory.AsDatabaseClusterSpecEngineResourcesMemory0() if err == nil { - return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") + return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") //nolint:stylecheck } memStr, err := cluster.Spec.Engine.Resources.Memory.AsDatabaseClusterSpecEngineResourcesMemory1() if err == nil { @@ -461,22 +489,26 @@ func validateResourceLimits(cluster *DatabaseCluster) error { return err } if mem.Cmp(minMemQuantity) == -1 { - return errors.New("Memory limits should be above 512M") + return errors.New("Memory limits should be above 512M") //nolint:stylecheck } } - _, err = cluster.Spec.Engine.Storage.Size.AsDatabaseClusterSpecEngineStorageSize0() + return nil +} + +func validateStorageSize(cluster *DatabaseCluster) error { + _, err := cluster.Spec.Engine.Storage.Size.AsDatabaseClusterSpecEngineStorageSize0() if err == nil { - return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") + return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") //nolint:stylecheck } sizeStr, err := cluster.Spec.Engine.Storage.Size.AsDatabaseClusterSpecEngineStorageSize1() - if err == nil { + if err == nil { size, err := resource.ParseQuantity(sizeStr) if err != nil { return err } if size.Cmp(minStorageQuantity) == -1 { - return errors.New("Storage size should be above 1G") + return errors.New("Storage size should be above 1G") //nolint:stylecheck } } return nil diff --git a/pkg/kubernetes/client/customresources/databaseclusters.go b/pkg/kubernetes/client/customresources/databaseclusters.go index 9db7e9c0..2ddac12d 100644 --- a/pkg/kubernetes/client/customresources/databaseclusters.go +++ b/pkg/kubernetes/client/customresources/databaseclusters.go @@ -1,3 +1,4 @@ +// Package customresources ... // percona-everest-backend // Copyright (C) 2023 Percona LLC // @@ -12,7 +13,8 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. - +// +//nolint:dupl package customresources import ( diff --git a/pkg/kubernetes/client/customresources/databaseengines.go b/pkg/kubernetes/client/customresources/databaseengines.go index b2fa5b5e..77be0dd3 100644 --- a/pkg/kubernetes/client/customresources/databaseengines.go +++ b/pkg/kubernetes/client/customresources/databaseengines.go @@ -1,3 +1,4 @@ +// Package customresources ... // percona-everest-backend // Copyright (C) 2023 Percona LLC // @@ -12,7 +13,8 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. - +// +//nolint:dupl package customresources import ( From b9d670a06469cdaf09d44bedf98036669ac1d9bb Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Tue, 19 Sep 2023 16:00:55 +0600 Subject: [PATCH 09/22] Update api/validation.go Co-authored-by: Diogo Recharte --- api/validation.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/validation.go b/api/validation.go index 09305a90..bd2b1ecf 100644 --- a/api/validation.go +++ b/api/validation.go @@ -396,16 +396,16 @@ func containsVersion(version string, versions []string) bool { } func validateProxy(engineType, proxyType string) error { - if engineType == engineTypePXC { - if proxyType != "proxysql" && proxyType != "haproxy" { + if engineType == everestv1alpha1.DatabaseEnginePXC { + if proxyType != everestv1alpha1.ProxyTypeProxySQL && proxyType != everestv1alpha1.ProxyTypeHAProxy { return errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") //nolint:stylecheck } } - if engineType == engineTypePG && proxyType != "pgbouncer" { + if engineType == everestv1alpha1.DatabaseEnginePostgresql && proxyType != everestv1alpha1.ProxyTypePGBouncer { return errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") //nolint:stylecheck } - if engineType == engineTypePSMDB && proxyType != "mongos" { + if engineType == everestv1alpha1.DatabaseEnginePSMDB && proxyType != everestv1alpha1.ProxyTypeMongos { return errors.New("You can use only Mongos as a proxy type for MongoDB clusters") //nolint:stylecheck } return nil From 221efec7e56a8106196964683270cc98725ba83d Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Tue, 19 Sep 2023 17:25:54 +0600 Subject: [PATCH 10/22] EVEREST-256 Small refactoring --- api/validation.go | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/api/validation.go b/api/validation.go index bd2b1ecf..d2ba8d44 100644 --- a/api/validation.go +++ b/api/validation.go @@ -42,20 +42,21 @@ const ( pxcDeploymentName = "percona-xtradb-cluster-operator" psmdbDeploymentName = "percona-server-mongodb-operator" pgDeploymentName = "percona-postgresql-operator" - engineTypePXC = "pxc" - engineTypePSMDB = "psmdb" - engineTypePG = "postgresql" ) var ( errDBCEmptyMetadata = errors.New("DatabaseCluster's Metadata should not be empty") errDBCNameEmpty = errors.New("DatabaseCluster's metadata.name should not be empty") errDBCNameWrongFormat = errors.New("DatabaseCluster's metadata.name should be a string") + errNotEnoughMemory = errors.New("Memory limits should be above 512M") //nolint:stylecheck + errInt64NotSupported = errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") //nolint:stylecheck + errNotEnoughCPU = errors.New("CPU limits should be above 600m") //nolint:stylecheck + errNotEnoughDiskSize = errors.New("Storage size should be above 1G") //nolint:stylecheck //nolint:gochecknoglobals - operatorEngine = map[string]string{ - engineTypePXC: pxcDeploymentName, - engineTypePSMDB: psmdbDeploymentName, - engineTypePG: pgDeploymentName, + operatorEngine = map[everestv1alpha1.EngineType]string{ + everestv1alpha1.DatabaseEnginePXC: pxcDeploymentName, + everestv1alpha1.DatabaseEnginePSMDB: psmdbDeploymentName, + everestv1alpha1.DatabaseEnginePostgresql: pgDeploymentName, } minStorageQuantity = resource.MustParse("1G") //nolint:gochecknoglobals minCPUQuantity = resource.MustParse("600m") //nolint:gochecknoglobals @@ -349,7 +350,7 @@ func (e *EverestServer) validateDatabaseClusterCR(ctx echo.Context, kubernetesID if err != nil { return err } - engineName, ok := operatorEngine[databaseCluster.Spec.Engine.Type] + engineName, ok := operatorEngine[everestv1alpha1.EngineType(databaseCluster.Spec.Engine.Type)] if !ok { return errors.New("Unsupported database engine") //nolint:stylecheck } @@ -396,16 +397,16 @@ func containsVersion(version string, versions []string) bool { } func validateProxy(engineType, proxyType string) error { - if engineType == everestv1alpha1.DatabaseEnginePXC { - if proxyType != everestv1alpha1.ProxyTypeProxySQL && proxyType != everestv1alpha1.ProxyTypeHAProxy { + if engineType == string(everestv1alpha1.DatabaseEnginePXC) { + if proxyType != string(everestv1alpha1.ProxyTypeProxySQL) && proxyType != string(everestv1alpha1.ProxyTypeHAProxy) { return errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") //nolint:stylecheck } } - if engineType == everestv1alpha1.DatabaseEnginePostgresql && proxyType != everestv1alpha1.ProxyTypePGBouncer { + if engineType == string(everestv1alpha1.DatabaseEnginePostgresql) && proxyType != string(everestv1alpha1.ProxyTypePGBouncer) { return errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") //nolint:stylecheck } - if engineType == everestv1alpha1.DatabaseEnginePSMDB && proxyType != everestv1alpha1.ProxyTypeMongos { + if engineType == string(everestv1alpha1.DatabaseEnginePSMDB) && proxyType != string(everestv1alpha1.ProxyTypeMongos) { return errors.New("You can use only Mongos as a proxy type for MongoDB clusters") //nolint:stylecheck } return nil @@ -451,10 +452,10 @@ func ensureNotEmptySpec(cluster *DatabaseCluster) error { return errors.New("Please specify resource limits for the cluster") //nolint:stylecheck } if cluster.Spec.Engine.Resources.Cpu == nil { - return errors.New("CPU limits should be above 600m and cannot be empty") + return errNotEnoughCPU } if cluster.Spec.Engine.Resources.Memory == nil { - return errors.New("Memory limits should be above 512M and cannot be empty") //nolint:stylecheck + return errNotEnoughMemory } return nil } @@ -467,12 +468,12 @@ func validateCPU(cluster *DatabaseCluster) error { return err } if cpu.Cmp(minCPUQuantity) == -1 { - return errors.New("CPU limits should be above 600m") + return errNotEnoughCPU } } _, err = cluster.Spec.Engine.Resources.Cpu.AsDatabaseClusterSpecEngineResourcesCpu0() if err == nil { - return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") //nolint:stylecheck + return errInt64NotSupported } return nil } @@ -480,7 +481,7 @@ func validateCPU(cluster *DatabaseCluster) error { func validateMemory(cluster *DatabaseCluster) error { _, err := cluster.Spec.Engine.Resources.Memory.AsDatabaseClusterSpecEngineResourcesMemory0() if err == nil { - return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") //nolint:stylecheck + return errInt64NotSupported } memStr, err := cluster.Spec.Engine.Resources.Memory.AsDatabaseClusterSpecEngineResourcesMemory1() if err == nil { @@ -489,7 +490,7 @@ func validateMemory(cluster *DatabaseCluster) error { return err } if mem.Cmp(minMemQuantity) == -1 { - return errors.New("Memory limits should be above 512M") //nolint:stylecheck + return errNotEnoughMemory } } return nil @@ -498,7 +499,7 @@ func validateMemory(cluster *DatabaseCluster) error { func validateStorageSize(cluster *DatabaseCluster) error { _, err := cluster.Spec.Engine.Storage.Size.AsDatabaseClusterSpecEngineStorageSize0() if err == nil { - return errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") //nolint:stylecheck + return errInt64NotSupported } sizeStr, err := cluster.Spec.Engine.Storage.Size.AsDatabaseClusterSpecEngineStorageSize1() @@ -508,7 +509,7 @@ func validateStorageSize(cluster *DatabaseCluster) error { return err } if size.Cmp(minStorageQuantity) == -1 { - return errors.New("Storage size should be above 1G") //nolint:stylecheck + return errNotEnoughDiskSize } } return nil From c50121732e696c24ea26b2e6d8af7d9499da1fc3 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Tue, 19 Sep 2023 18:05:11 +0600 Subject: [PATCH 11/22] EVEREST-256 Unittests for proxy validation --- api/validation.go | 23 +++++----- api/validation_test.go | 96 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 10 deletions(-) diff --git a/api/validation.go b/api/validation.go index d2ba8d44..343ea076 100644 --- a/api/validation.go +++ b/api/validation.go @@ -45,13 +45,16 @@ const ( ) var ( - errDBCEmptyMetadata = errors.New("DatabaseCluster's Metadata should not be empty") - errDBCNameEmpty = errors.New("DatabaseCluster's metadata.name should not be empty") - errDBCNameWrongFormat = errors.New("DatabaseCluster's metadata.name should be a string") - errNotEnoughMemory = errors.New("Memory limits should be above 512M") //nolint:stylecheck - errInt64NotSupported = errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") //nolint:stylecheck - errNotEnoughCPU = errors.New("CPU limits should be above 600m") //nolint:stylecheck - errNotEnoughDiskSize = errors.New("Storage size should be above 1G") //nolint:stylecheck + errDBCEmptyMetadata = errors.New("DatabaseCluster's Metadata should not be empty") + errDBCNameEmpty = errors.New("DatabaseCluster's metadata.name should not be empty") + errDBCNameWrongFormat = errors.New("DatabaseCluster's metadata.name should be a string") + errNotEnoughMemory = errors.New("Memory limits should be above 512M") //nolint:stylecheck + errInt64NotSupported = errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") //nolint:stylecheck + errNotEnoughCPU = errors.New("CPU limits should be above 600m") //nolint:stylecheck + errNotEnoughDiskSize = errors.New("Storage size should be above 1G") //nolint:stylecheck + errUnsupportedPXCProxy = errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") //nolint:stylecheck + errUnsupportedPGProxy = errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") //nolint:stylecheck + errUnsupportedPSMDBProxy = errors.New("You can use only Mongos as a proxy type for MongoDB clusters") //nolint:stylecheck //nolint:gochecknoglobals operatorEngine = map[everestv1alpha1.EngineType]string{ everestv1alpha1.DatabaseEnginePXC: pxcDeploymentName, @@ -399,15 +402,15 @@ func containsVersion(version string, versions []string) bool { func validateProxy(engineType, proxyType string) error { if engineType == string(everestv1alpha1.DatabaseEnginePXC) { if proxyType != string(everestv1alpha1.ProxyTypeProxySQL) && proxyType != string(everestv1alpha1.ProxyTypeHAProxy) { - return errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") //nolint:stylecheck + return errUnsupportedPXCProxy } } if engineType == string(everestv1alpha1.DatabaseEnginePostgresql) && proxyType != string(everestv1alpha1.ProxyTypePGBouncer) { - return errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") //nolint:stylecheck + return errUnsupportedPGProxy } if engineType == string(everestv1alpha1.DatabaseEnginePSMDB) && proxyType != string(everestv1alpha1.ProxyTypeMongos) { - return errors.New("You can use only Mongos as a proxy type for MongoDB clusters") //nolint:stylecheck + return errUnsupportedPSMDBProxy } return nil } diff --git a/api/validation_test.go b/api/validation_test.go index 388fe713..c47d3caa 100644 --- a/api/validation_test.go +++ b/api/validation_test.go @@ -17,6 +17,7 @@ package api import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -166,3 +167,98 @@ func TestValidateCreateDatabaseClusterRequest(t *testing.T) { }) } } + +func TestValidateProxy(t *testing.T) { + t.Parallel() + cases := []struct { + name string + engineType string + proxyType string + err error + }{ + { + name: "PXC with mongos", + engineType: "pxc", + proxyType: "mongos", + err: errUnsupportedPXCProxy, + }, + { + name: "PXC with pgbouncer", + engineType: "pxc", + proxyType: "pgbouncer", + err: errUnsupportedPXCProxy, + }, + { + name: "PXC with haproxy", + engineType: "pxc", + proxyType: "haproxy", + err: nil, + }, + { + name: "PXC with proxysql", + engineType: "pxc", + proxyType: "proxysql", + err: nil, + }, + { + name: "psmdb with mongos", + engineType: "psmdb", + proxyType: "mongos", + err: nil, + }, + { + name: "psmdb with pgbouncer", + engineType: "psmdb", + proxyType: "pgbouncer", + err: errUnsupportedPSMDBProxy, + }, + { + name: "psmdb with haproxy", + engineType: "psmdb", + proxyType: "haproxy", + err: errUnsupportedPSMDBProxy, + }, + { + name: "psmdb with proxysql", + engineType: "psmdb", + proxyType: "proxysql", + err: errUnsupportedPSMDBProxy, + }, + { + name: "postgresql with mongos", + engineType: "postgresql", + proxyType: "mongos", + err: errUnsupportedPGProxy, + }, + { + name: "postgresql with pgbouncer", + engineType: "postgresql", + proxyType: "pgbouncer", + err: nil, + }, + { + name: "postgresql with haproxy", + engineType: "postgresql", + proxyType: "haproxy", + err: errUnsupportedPGProxy, + }, + { + name: "postgresql with proxysql", + engineType: "postgresql", + proxyType: "proxysql", + err: errUnsupportedPGProxy, + }, + } + for _, tc := range cases { + c := tc + t.Run(c.name, func(t *testing.T) { + t.Parallel() + err := validateProxy(c.engineType, c.proxyType) + if c.err == nil { + require.Nil(t, err) + return + } + assert.Equal(t, c.err.Error(), err.Error()) + }) + } +} From 46e89641ee6817d4cfd5b6fbe0a89c7d5f84495a Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Wed, 20 Sep 2023 00:34:54 +0600 Subject: [PATCH 12/22] EVEREST-256 Added draft of tests for the rest of stuff --- api/validation_test.go | 129 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/api/validation_test.go b/api/validation_test.go index c47d3caa..2d774a10 100644 --- a/api/validation_test.go +++ b/api/validation_test.go @@ -15,8 +15,10 @@ package api import ( + "encoding/json" "testing" + everestv1alpha1 "github.com/percona/everest-operator/api/v1alpha1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -262,3 +264,130 @@ func TestValidateProxy(t *testing.T) { }) } } + +func TestContainsVersion(t *testing.T) { + t.Parallel() + cases := []struct { + version string + versions []string + result bool + }{ + { + version: "1", + versions: []string{}, + result: false, + }, + { + version: "1", + versions: []string{"1", "2"}, + result: true, + }, + { + version: "1", + versions: []string{"1"}, + result: true, + }, + { + version: "1", + versions: []string{"12", "23"}, + result: false, + }, + } + for _, tc := range cases { + + tc := tc + t.Run(tc.version, func(t *testing.T) { + t.Parallel() + res := containsVersion(tc.version, tc.versions) + assert.Equal(t, res, tc.result) + }) + } + +} + +func TestValidateVersion(t *testing.T) { + t.Parallel() + cases := []struct { + name string + version *string + engine *everestv1alpha1.DatabaseEngine + err error + }{ + { + name: "empty version is allowed", + version: nil, + engine: nil, + err: nil, + }, + } + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + err := validateVersion(tc.version, tc.engine) + if tc.err == nil { + require.Nil(t, err) + return + } + assert.Equal(t, err.Error(), tc.err.Error()) + }) + } +} +func TestValidateBackupSpec(t *testing.T) { + t.Parallel() + cases := []struct { + name string + cluster []byte + err error + }{ + { + name: "empty version is allowed", + cluster: []byte(`{"spec": {"backup": {"enabled": false}}}`), + err: nil, + }, + } + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cluster := &DatabaseCluster{} + err := json.Unmarshal(tc.cluster, cluster) + require.NoError(t, err) + err = validateBackupSpec(cluster) + if tc.err == nil { + require.Nil(t, err) + return + } + assert.Equal(t, err.Error(), tc.err.Error()) + }) + } +} +func TestValidateResourceLimits(t *testing.T) { + t.Parallel() + cases := []struct { + name string + cluster []byte + err error + }{ + { + name: "success", + cluster: []byte(`{"spec": {"engine": {"resources": {"cpu": "600m", "memory":"1G"}, "storage": {"size": "2G"}}}}`), + err: nil, + }, + } + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cluster := &DatabaseCluster{} + err := json.Unmarshal(tc.cluster, cluster) + require.NoError(t, err) + err = validateResourceLimits(cluster) + if tc.err == nil { + require.Nil(t, err) + return + } + assert.Equal(t, err.Error(), tc.err.Error()) + }) + } +} From 3dd841fda51eefd8e276de854645fe0483d2eeba Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Wed, 20 Sep 2023 00:36:36 +0600 Subject: [PATCH 13/22] Update api/validation.go Co-authored-by: Michal Kralik --- api/validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/validation.go b/api/validation.go index 343ea076..ee1dfd21 100644 --- a/api/validation.go +++ b/api/validation.go @@ -52,7 +52,7 @@ var ( errInt64NotSupported = errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") //nolint:stylecheck errNotEnoughCPU = errors.New("CPU limits should be above 600m") //nolint:stylecheck errNotEnoughDiskSize = errors.New("Storage size should be above 1G") //nolint:stylecheck - errUnsupportedPXCProxy = errors.New("You can use only either HAProxy or Proxy SQL for PXC clusters") //nolint:stylecheck + errUnsupportedPXCProxy = errors.New("You can use either HAProxy or Proxy SQL for PXC clusters") //nolint:stylecheck errUnsupportedPGProxy = errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") //nolint:stylecheck errUnsupportedPSMDBProxy = errors.New("You can use only Mongos as a proxy type for MongoDB clusters") //nolint:stylecheck //nolint:gochecknoglobals From 3eb8e344d3d8b96ccdf37b7544fc4df4a4134ce2 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Wed, 20 Sep 2023 00:40:48 +0600 Subject: [PATCH 14/22] EVEREST-256 u --- api/validation.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/validation.go b/api/validation.go index ee1dfd21..a6fdeef4 100644 --- a/api/validation.go +++ b/api/validation.go @@ -364,7 +364,7 @@ func (e *EverestServer) validateDatabaseClusterCR(ctx echo.Context, kubernetesID if err := validateVersion(databaseCluster.Spec.Engine.Version, engine); err != nil { return err } - if databaseCluster.Spec.Proxy.Type != nil { + if databaseCluster.Spec.Proxy != nil && databaseCluster.Spec.Proxy.Type != nil { if err := validateProxy(databaseCluster.Spec.Engine.Type, string(*databaseCluster.Spec.Proxy.Type)); err != nil { return err } @@ -428,7 +428,7 @@ func validateBackupSpec(cluster *DatabaseCluster) error { for _, schedule := range *cluster.Spec.Backup.Schedules { if schedule.Name == "" { - return errors.New("'name' field cannot be empty") + return errors.New("'name' field for the schedules cannot be empty") } if schedule.Enabled && schedule.BackupStorageName == "" { return errors.New("'backupStorageName' field cannot be empty when schedule is enabled") From d63da10e28bcf9cc81c5fa5a267a821adfacd92e Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Wed, 20 Sep 2023 00:48:20 +0600 Subject: [PATCH 15/22] EVEREST-256 u --- api/validation.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/api/validation.go b/api/validation.go index a6fdeef4..b1a1308c 100644 --- a/api/validation.go +++ b/api/validation.go @@ -45,14 +45,18 @@ const ( ) var ( + minStorageQuantity = resource.MustParse("1G") //nolint:gochecknoglobals + minCPUQuantity = resource.MustParse("600m") //nolint:gochecknoglobals + minMemQuantity = resource.MustParse("512M") //nolint:gochecknoglobals + errDBCEmptyMetadata = errors.New("DatabaseCluster's Metadata should not be empty") errDBCNameEmpty = errors.New("DatabaseCluster's metadata.name should not be empty") errDBCNameWrongFormat = errors.New("DatabaseCluster's metadata.name should be a string") - errNotEnoughMemory = errors.New("Memory limits should be above 512M") //nolint:stylecheck + errNotEnoughMemory = fmt.Errorf("Memory limits should be above %s", minMemQuantity.String()) //nolint:stylecheck errInt64NotSupported = errors.New("Specifying resources using int64 data type is not supported. Please use string format for that") //nolint:stylecheck - errNotEnoughCPU = errors.New("CPU limits should be above 600m") //nolint:stylecheck - errNotEnoughDiskSize = errors.New("Storage size should be above 1G") //nolint:stylecheck - errUnsupportedPXCProxy = errors.New("You can use either HAProxy or Proxy SQL for PXC clusters") //nolint:stylecheck + errNotEnoughCPU = fmt.Errorf("CPU limits should be above %s", minCPUQuantity.String()) //nolint:stylecheck + errNotEnoughDiskSize = fmt.Errorf("Storage size should be above %s", minStorageQuantity.String()) //nolint:stylecheck + errUnsupportedPXCProxy = errors.New("You can use either HAProxy or Proxy SQL for PXC clusters") //nolint:stylecheck errUnsupportedPGProxy = errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") //nolint:stylecheck errUnsupportedPSMDBProxy = errors.New("You can use only Mongos as a proxy type for MongoDB clusters") //nolint:stylecheck //nolint:gochecknoglobals @@ -61,9 +65,6 @@ var ( everestv1alpha1.DatabaseEnginePSMDB: psmdbDeploymentName, everestv1alpha1.DatabaseEnginePostgresql: pgDeploymentName, } - minStorageQuantity = resource.MustParse("1G") //nolint:gochecknoglobals - minCPUQuantity = resource.MustParse("600m") //nolint:gochecknoglobals - minMemQuantity = resource.MustParse("512M") //nolint:gochecknoglobals ) // ErrNameNotRFC1035Compatible when the given fieldName doesn't contain RFC 1035 compatible string. From d5834a17c0fc85de6d65233d6f1b1a604d829ff8 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Wed, 20 Sep 2023 01:21:20 +0600 Subject: [PATCH 16/22] EVEREST-256 tests --- api/validation.go | 19 +++++-- api/validation_test.go | 125 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 8 deletions(-) diff --git a/api/validation.go b/api/validation.go index b1a1308c..0714b308 100644 --- a/api/validation.go +++ b/api/validation.go @@ -59,6 +59,10 @@ var ( errUnsupportedPXCProxy = errors.New("You can use either HAProxy or Proxy SQL for PXC clusters") //nolint:stylecheck errUnsupportedPGProxy = errors.New("You can use only PGBouncer as a proxy type for Postgres clusters") //nolint:stylecheck errUnsupportedPSMDBProxy = errors.New("You can use only Mongos as a proxy type for MongoDB clusters") //nolint:stylecheck + errNoSchedules = errors.New("Please specify at least one backup schedule") //nolint:stylecheck + errNoNameInSchedule = errors.New("'name' field for the backup schedules cannot be empty") + errNoBackupStorageName = errors.New("'backupStorageName' field cannot be empty when schedule is enabled") + errNoResourceDefined = errors.New("Please specify resource limits for the cluster") //nolint:stylecheck //nolint:gochecknoglobals operatorEngine = map[everestv1alpha1.EngineType]string{ everestv1alpha1.DatabaseEnginePXC: pxcDeploymentName, @@ -378,8 +382,11 @@ func (e *EverestServer) validateDatabaseClusterCR(ctx echo.Context, kubernetesID func validateVersion(version *string, engine *everestv1alpha1.DatabaseEngine) error { if version != nil { - if len(engine.Spec.AllowedVersions) != 0 && !containsVersion(*version, engine.Spec.AllowedVersions) { - return fmt.Errorf("Using %s version for %s is not allowed", *version, engine.Spec.Type) //nolint:stylecheck + if len(engine.Spec.AllowedVersions) != 0 { + if !containsVersion(*version, engine.Spec.AllowedVersions) { + return fmt.Errorf("Using %s version for %s is not allowed", *version, engine.Spec.Type) //nolint:stylecheck + } + return nil } if _, ok := engine.Status.AvailableVersions.Engine[*version]; !ok { return fmt.Errorf("%s is not in available versions list", *version) @@ -424,15 +431,15 @@ func validateBackupSpec(cluster *DatabaseCluster) error { return nil } if cluster.Spec.Backup.Schedules == nil { - return errors.New("Please specify at lease one backup schedule") //nolint:stylecheck + return errNoSchedules } for _, schedule := range *cluster.Spec.Backup.Schedules { if schedule.Name == "" { - return errors.New("'name' field for the schedules cannot be empty") + return errNoNameInSchedule } if schedule.Enabled && schedule.BackupStorageName == "" { - return errors.New("'backupStorageName' field cannot be empty when schedule is enabled") + return errNoBackupStorageName } } return nil @@ -453,7 +460,7 @@ func validateResourceLimits(cluster *DatabaseCluster) error { func ensureNotEmptySpec(cluster *DatabaseCluster) error { if cluster.Spec.Engine.Resources == nil { - return errors.New("Please specify resource limits for the cluster") //nolint:stylecheck + return errNoResourceDefined } if cluster.Spec.Engine.Resources.Cpu == nil { return errNotEnoughCPU diff --git a/api/validation_test.go b/api/validation_test.go index 2d774a10..18233763 100644 --- a/api/validation_test.go +++ b/api/validation_test.go @@ -18,7 +18,9 @@ import ( "encoding/json" "testing" + "github.com/AlekSi/pointer" everestv1alpha1 "github.com/percona/everest-operator/api/v1alpha1" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -294,7 +296,6 @@ func TestContainsVersion(t *testing.T) { }, } for _, tc := range cases { - tc := tc t.Run(tc.version, func(t *testing.T) { t.Parallel() @@ -319,6 +320,56 @@ func TestValidateVersion(t *testing.T) { engine: nil, err: nil, }, + { + name: "shall exist in availableVersions", + version: pointer.ToString("8.0.32"), + engine: &everestv1alpha1.DatabaseEngine{ + Status: everestv1alpha1.DatabaseEngineStatus{ + AvailableVersions: everestv1alpha1.Versions{ + Engine: everestv1alpha1.ComponentsMap{ + "8.0.32": &everestv1alpha1.Component{}, + }, + }, + }, + }, + err: nil, + }, + { + name: "shall not exist in availableVersions", + version: pointer.ToString("8.0.32"), + engine: &everestv1alpha1.DatabaseEngine{ + Status: everestv1alpha1.DatabaseEngineStatus{ + AvailableVersions: everestv1alpha1.Versions{ + Engine: everestv1alpha1.ComponentsMap{ + "8.0.31": &everestv1alpha1.Component{}, + }, + }, + }, + }, + err: errors.New("8.0.32 is not in available versions list"), + }, + { + name: "shall exist in allowedVersions", + version: pointer.ToString("8.0.32"), + engine: &everestv1alpha1.DatabaseEngine{ + Spec: everestv1alpha1.DatabaseEngineSpec{ + Type: "pxc", + AllowedVersions: []string{"8.0.32"}, + }, + }, + err: nil, + }, + { + name: "shall not exist in allowedVersions", + version: pointer.ToString("8.0.32"), + engine: &everestv1alpha1.DatabaseEngine{ + Spec: everestv1alpha1.DatabaseEngineSpec{ + Type: "pxc", + AllowedVersions: []string{"8.0.31"}, + }, + }, + err: errors.New("Using 8.0.32 version for pxc is not allowed"), + }, } for _, tc := range cases { tc := tc @@ -341,10 +392,35 @@ func TestValidateBackupSpec(t *testing.T) { err error }{ { - name: "empty version is allowed", + name: "empty backup is allowed", + cluster: []byte(`{"spec": {"backup": null}}`), + err: nil, + }, + { + name: "disabled backup is allowed", cluster: []byte(`{"spec": {"backup": {"enabled": false}}}`), err: nil, }, + { + name: "errNoSchedules", + cluster: []byte(`{"spec": {"backup": {"enabled": true}}}`), + err: errNoSchedules, + }, + { + name: "errNoNameInSchedule", + cluster: []byte(`{"spec": {"backup": {"enabled": true, "schedules": [{"enabled": true}]}}}`), + err: errNoNameInSchedule, + }, + { + name: "errNoBackupStorageName", + cluster: []byte(`{"spec": {"backup": {"enabled": true, "schedules": [{"enabled": true, "name": "name"}]}}}`), + err: errNoBackupStorageName, + }, + { + name: "errNoBackupStorageName", + cluster: []byte(`{"spec": {"backup": {"enabled": true, "schedules": [{"enabled": true, "name": "name", "backupStorageName": "some"}]}}}`), + err: nil, + }, } for _, tc := range cases { tc := tc @@ -374,6 +450,51 @@ func TestValidateResourceLimits(t *testing.T) { cluster: []byte(`{"spec": {"engine": {"resources": {"cpu": "600m", "memory":"1G"}, "storage": {"size": "2G"}}}}`), err: nil, }, + { + name: "errNoResourceDefined", + cluster: []byte(`{"spec": {"engine": {"resources":null, "storage": {"size": "2G"}}}}`), + err: errNoResourceDefined, + }, + { + name: "Not enough CPU", + cluster: []byte(`{"spec": {"engine": {"resources": {"cpu": null, "memory":"1G"}, "storage": {"size": "2G"}}}}`), + err: errNotEnoughCPU, + }, + { + name: "Not enough memory", + cluster: []byte(`{"spec": {"engine": {"resources": {"cpu": "600m", "memory":null}, "storage": {"size": "2G"}}}}`), + err: errNotEnoughMemory, + }, + { + name: "No int64 for CPU", + cluster: []byte(`{"spec": {"engine": {"resources": {"cpu": 6000, "memory": "1G"}, "storage": {"size": "2G"}}}}`), + err: errInt64NotSupported, + }, + { + name: "No int64 for Memory", + cluster: []byte(`{"spec": {"engine": {"resources": {"cpu": "600m", "memory": 1000000}, "storage": {"size": "2G"}}}}`), + err: errInt64NotSupported, + }, + { + name: "No int64 for storage", + cluster: []byte(`{"spec": {"engine": {"resources": {"cpu": "600m", "memory": "1G"}, "storage": {"size": 20000}}}}`), + err: errInt64NotSupported, + }, + { + name: "not enough disk size", + cluster: []byte(`{"spec": {"engine": {"resources": {"cpu": "600m", "memory": "1G"}, "storage": {"size": "512M"}}}}`), + err: errNotEnoughDiskSize, + }, + { + name: "not enough CPU", + cluster: []byte(`{"spec": {"engine": {"resources": {"cpu": "200m", "memory": "1G"}, "storage": {"size": "2G"}}}}`), + err: errNotEnoughCPU, + }, + { + name: "not enough Mem", + cluster: []byte(`{"spec": {"engine": {"resources": {"cpu": "600m", "memory": "400M"}, "storage": {"size": "2G"}}}}`), + err: errNotEnoughMemory, + }, } for _, tc := range cases { tc := tc From b42d759aee5f831dc0db395e33c770ce3613f917 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Wed, 20 Sep 2023 01:22:20 +0600 Subject: [PATCH 17/22] EVEREST-256 tests --- api/validation_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/validation_test.go b/api/validation_test.go index 18233763..b347b37a 100644 --- a/api/validation_test.go +++ b/api/validation_test.go @@ -303,7 +303,6 @@ func TestContainsVersion(t *testing.T) { assert.Equal(t, res, tc.result) }) } - } func TestValidateVersion(t *testing.T) { @@ -384,6 +383,7 @@ func TestValidateVersion(t *testing.T) { }) } } + func TestValidateBackupSpec(t *testing.T) { t.Parallel() cases := []struct { @@ -438,6 +438,7 @@ func TestValidateBackupSpec(t *testing.T) { }) } } + func TestValidateResourceLimits(t *testing.T) { t.Parallel() cases := []struct { From 269d056f3c8ed58c3ab793b2177863c5a2ecb8a6 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Wed, 20 Sep 2023 01:23:09 +0600 Subject: [PATCH 18/22] EVEREST-256 tests --- api/validation_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/validation_test.go b/api/validation_test.go index b347b37a..3fa202cf 100644 --- a/api/validation_test.go +++ b/api/validation_test.go @@ -303,6 +303,7 @@ func TestContainsVersion(t *testing.T) { assert.Equal(t, res, tc.result) }) } + } func TestValidateVersion(t *testing.T) { @@ -383,7 +384,6 @@ func TestValidateVersion(t *testing.T) { }) } } - func TestValidateBackupSpec(t *testing.T) { t.Parallel() cases := []struct { @@ -417,7 +417,7 @@ func TestValidateBackupSpec(t *testing.T) { err: errNoBackupStorageName, }, { - name: "errNoBackupStorageName", + name: "valid spec", cluster: []byte(`{"spec": {"backup": {"enabled": true, "schedules": [{"enabled": true, "name": "name", "backupStorageName": "some"}]}}}`), err: nil, }, @@ -438,7 +438,6 @@ func TestValidateBackupSpec(t *testing.T) { }) } } - func TestValidateResourceLimits(t *testing.T) { t.Parallel() cases := []struct { From e31d6fae1a1b4ad815cb188429bbf57c1c6fbb90 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Wed, 20 Sep 2023 01:25:49 +0600 Subject: [PATCH 19/22] EVEREST-256 validate on update --- api/database_cluster.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api/database_cluster.go b/api/database_cluster.go index 6f0e1787..8f13ace1 100644 --- a/api/database_cluster.go +++ b/api/database_cluster.go @@ -134,6 +134,10 @@ func (e *EverestServer) UpdateDatabaseCluster(ctx echo.Context, kubernetesID str }) } + if err := e.validateDatabaseClusterCR(ctx, kubernetesID, dbc); err != nil { + return ctx.JSON(http.StatusBadRequest, Error{Message: pointer.ToString(err.Error())}) + } + _, kubeClient, code, err := e.initKubeClient(ctx.Request().Context(), kubernetesID) if err != nil { return ctx.JSON(code, Error{Message: pointer.ToString(err.Error())}) From 756122bd40f3799a4a8ad153854280b9c9b87dc2 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Wed, 20 Sep 2023 01:26:53 +0600 Subject: [PATCH 20/22] EVEREST-256 format --- api/validation_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/validation_test.go b/api/validation_test.go index 3fa202cf..fd9b255a 100644 --- a/api/validation_test.go +++ b/api/validation_test.go @@ -303,7 +303,6 @@ func TestContainsVersion(t *testing.T) { assert.Equal(t, res, tc.result) }) } - } func TestValidateVersion(t *testing.T) { @@ -384,6 +383,7 @@ func TestValidateVersion(t *testing.T) { }) } } + func TestValidateBackupSpec(t *testing.T) { t.Parallel() cases := []struct { @@ -438,6 +438,7 @@ func TestValidateBackupSpec(t *testing.T) { }) } } + func TestValidateResourceLimits(t *testing.T) { t.Parallel() cases := []struct { From ff8dff36237f1160cdaae7f47814f2843e18b383 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Wed, 20 Sep 2023 01:31:01 +0600 Subject: [PATCH 21/22] EVEREST-256 format --- api/database_cluster.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/database_cluster.go b/api/database_cluster.go index 8f13ace1..89eb04a9 100644 --- a/api/database_cluster.go +++ b/api/database_cluster.go @@ -125,7 +125,7 @@ func (e *EverestServer) GetDatabaseCluster(ctx echo.Context, kubernetesID string } // UpdateDatabaseCluster replaces the specified database cluster on the specified kubernetes cluster. -func (e *EverestServer) UpdateDatabaseCluster(ctx echo.Context, kubernetesID string, name string) error { //nolint:funlen +func (e *EverestServer) UpdateDatabaseCluster(ctx echo.Context, kubernetesID string, name string) error { //nolint:funlen,cyclop dbc := &DatabaseCluster{} if err := e.getBodyFromContext(ctx, dbc); err != nil { e.l.Error(err) From 3fb2eef132193d900c819542e113bf4be640ee57 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Wed, 20 Sep 2023 21:37:41 +0600 Subject: [PATCH 22/22] EVEREST-256 rename func --- api/validation.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/validation.go b/api/validation.go index 0714b308..d381ec83 100644 --- a/api/validation.go +++ b/api/validation.go @@ -446,7 +446,7 @@ func validateBackupSpec(cluster *DatabaseCluster) error { } func validateResourceLimits(cluster *DatabaseCluster) error { - if err := ensureNotEmptySpec(cluster); err != nil { + if err := ensureNonEmptyResources(cluster); err != nil { return err } if err := validateCPU(cluster); err != nil { @@ -458,7 +458,7 @@ func validateResourceLimits(cluster *DatabaseCluster) error { return validateStorageSize(cluster) } -func ensureNotEmptySpec(cluster *DatabaseCluster) error { +func ensureNonEmptyResources(cluster *DatabaseCluster) error { if cluster.Spec.Engine.Resources == nil { return errNoResourceDefined }