From 39fe63c4fdff569daf5622660c78e63f17755135 Mon Sep 17 00:00:00 2001 From: Andrew Minkin Date: Fri, 6 Oct 2023 16:17:49 +0600 Subject: [PATCH] EVEREST-482 Removed nolint:funlen,cyclop directives (#202) --- .golangci.yml | 3 + api/backup_storage.go | 71 ++++++++++--------- api/database_cluster.go | 36 +++++----- api/database_cluster_restore.go | 21 ++++-- api/monitoring_instance.go | 31 ++++---- pkg/kubernetes/backup_storage.go | 2 +- .../client/mock_kube_client_connector.go | 2 +- 7 files changed, 98 insertions(+), 68 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 67853412..4b468c54 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -2,6 +2,9 @@ # Almost all linters; some of them are optional. linters-settings: + cyclop: + max-complexity: 15 + depguard: rules: main: diff --git a/api/backup_storage.go b/api/backup_storage.go index ad13aba2..53b7492b 100644 --- a/api/backup_storage.go +++ b/api/backup_storage.go @@ -60,7 +60,7 @@ func (e *EverestServer) ListBackupStorages(ctx echo.Context) error { // CreateBackupStorage creates a new backup storage object. // Rollbacks are implemented without transactions bc the secrets storage is going to be moved out of pg. -func (e *EverestServer) CreateBackupStorage(ctx echo.Context) error { //nolint:funlen,cyclop +func (e *EverestServer) CreateBackupStorage(ctx echo.Context) error { params, err := validateCreateBackupStorageRequest(ctx, e.l) if err != nil { return ctx.JSON(http.StatusBadRequest, Error{Message: pointer.ToString(err.Error())}) @@ -88,27 +88,7 @@ func (e *EverestServer) CreateBackupStorage(ctx echo.Context) error { //nolint:f if err != nil { return ctx.JSON(http.StatusInternalServerError, Error{Message: pointer.ToString(err.Error())}) } - - var url string - if params.Url != nil { - url = *params.Url - } - - var description string - if params.Description != nil { - description = *params.Description - } - - s, err := e.storage.CreateBackupStorage(c, model.CreateBackupStorageParams{ - Name: params.Name, - Description: description, - Type: string(params.Type), - BucketName: params.BucketName, - URL: url, - Region: params.Region, - AccessKeyID: *accessKeyID, - SecretKeyID: *secretKeyID, - }) + s, err := e.createBackupStorage(c, params, accessKeyID, secretKeyID) if err != nil { var pgErr *pq.Error if errors.As(err, &pgErr) { @@ -137,8 +117,31 @@ func (e *EverestServer) CreateBackupStorage(ctx echo.Context) error { //nolint:f return ctx.JSON(http.StatusOK, result) } +func (e *EverestServer) createBackupStorage(c context.Context, params *CreateBackupStorageParams, accessKeyID, secretKeyID *string) (*model.BackupStorage, error) { + var url string + if params.Url != nil { + url = *params.Url + } + + var description string + if params.Description != nil { + description = *params.Description + } + + return e.storage.CreateBackupStorage(c, model.CreateBackupStorageParams{ + Name: params.Name, + Description: description, + Type: string(params.Type), + BucketName: params.BucketName, + URL: url, + Region: params.Region, + AccessKeyID: *accessKeyID, + SecretKeyID: *secretKeyID, + }) +} + // DeleteBackupStorage deletes the specified backup storage. -func (e *EverestServer) DeleteBackupStorage(ctx echo.Context, backupStorageName string) error { //nolint:cyclop,funlen +func (e *EverestServer) DeleteBackupStorage(ctx echo.Context, backupStorageName string) error { c := ctx.Request().Context() bs, err := e.storage.GetBackupStorage(c, nil, backupStorageName) if err != nil { @@ -176,9 +179,20 @@ func (e *EverestServer) DeleteBackupStorage(ctx echo.Context, backupStorageName } return ctx.JSON(http.StatusInternalServerError, Error{Message: pointer.ToString(err.Error())}) } + err = e.deleteBackupStorage(c, bs) + + if err != nil { + return ctx.JSON(http.StatusInternalServerError, Error{ + Message: pointer.ToString(err.Error()), + }) + } + + return ctx.NoContent(http.StatusNoContent) +} - err = e.storage.Transaction(func(tx *gorm.DB) error { - err := e.storage.DeleteBackupStorage(c, backupStorageName, tx) +func (e *EverestServer) deleteBackupStorage(c context.Context, bs *model.BackupStorage) error { + return e.storage.Transaction(func(tx *gorm.DB) error { + err := e.storage.DeleteBackupStorage(c, bs.Name, tx) if err != nil { e.l.Error(err) return errors.New("could not delete backup storage") @@ -193,13 +207,6 @@ func (e *EverestServer) DeleteBackupStorage(ctx echo.Context, backupStorageName return nil }) - if err != nil { - return ctx.JSON(http.StatusInternalServerError, Error{ - Message: pointer.ToString(err.Error()), - }) - } - - return ctx.NoContent(http.StatusNoContent) } // GetBackupStorage retrieves the specified backup storage. diff --git a/api/database_cluster.go b/api/database_cluster.go index 7c7741cf..f6606fa2 100644 --- a/api/database_cluster.go +++ b/api/database_cluster.go @@ -126,7 +126,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,cyclop +func (e *EverestServer) UpdateDatabaseCluster(ctx echo.Context, kubernetesID string, name string) error { dbc := &DatabaseCluster{} if err := e.getBodyFromContext(ctx, dbc); err != nil { e.l.Error(err) @@ -159,23 +159,11 @@ func (e *EverestServer) UpdateDatabaseCluster(ctx echo.Context, kubernetesID str } } - newBackupNames := backupStorageNamesFrom(dbc) - oldNames := withBackupStorageNamesFromDBCluster(make(map[string]struct{}), *oldDB) - err = e.createBackupStoragesOnUpdate(ctx.Request().Context(), kubeClient, oldNames, newBackupNames) - if err != nil { - e.l.Error(err) - return ctx.JSON(http.StatusInternalServerError, Error{ - Message: pointer.ToString("Could not create new BackupStorages in Kubernetes"), - }) - } - newMonitoringName := monitoringNameFrom(dbc) - err = e.createMonitoringInstanceOnUpdate(ctx.Request().Context(), kubeClient, oldDB, newMonitoringName) + newBackupNames := backupStorageNamesFrom(dbc) + err = e.createResources(ctx.Request().Context(), oldDB, kubeClient, newMonitoringName, newBackupNames) if err != nil { - e.l.Error(err) - return ctx.JSON(http.StatusInternalServerError, Error{ - Message: pointer.ToString("Could not create a new monitoring config in Kubernetes"), - }) + return ctx.JSON(http.StatusInternalServerError, Error{Message: pointer.ToString(err.Error())}) } proxyErr := e.proxyKubernetes(ctx, kubernetesID, name) @@ -196,6 +184,22 @@ func (e *EverestServer) UpdateDatabaseCluster(ctx echo.Context, kubernetesID str return nil } +func (e *EverestServer) createResources(c context.Context, oldDB *everestv1alpha1.DatabaseCluster, kubeClient *kubernetes.Kubernetes, newMonitoringName string, newBackupNames map[string]struct{}) error { + oldNames := withBackupStorageNamesFromDBCluster(make(map[string]struct{}), *oldDB) + err := e.createBackupStoragesOnUpdate(c, kubeClient, oldNames, newBackupNames) + if err != nil { + e.l.Error(err) + return errors.New("could not create new BackupStorages in Kubernetes") + } + + err = e.createMonitoringInstanceOnUpdate(c, kubeClient, oldDB, newMonitoringName) + if err != nil { + e.l.Error(err) + return errors.New("could not create new monitoring configs in Kubernetes") + } + return nil +} + // GetDatabaseClusterCredentials returns credentials for the specified database cluster on the specified kubernetes cluster. func (e *EverestServer) GetDatabaseClusterCredentials(ctx echo.Context, kubernetesID string, name string) error { k, kubeClient, code, err := e.initKubeClient(ctx.Request().Context(), kubernetesID) diff --git a/api/database_cluster_restore.go b/api/database_cluster_restore.go index 5736e364..4fedc94c 100644 --- a/api/database_cluster_restore.go +++ b/api/database_cluster_restore.go @@ -18,6 +18,7 @@ package api import ( "context" + "errors" "fmt" "net/http" "net/url" @@ -25,6 +26,9 @@ import ( "github.com/AlekSi/pointer" "github.com/labstack/echo/v4" + everestv1alpha1 "github.com/percona/everest-operator/api/v1alpha1" + + "github.com/percona/percona-everest-backend/pkg/kubernetes" ) // ListDatabaseClusterRestores List of the created database cluster restores on the specified kubernetes cluster. @@ -127,7 +131,7 @@ func (e *EverestServer) GetDatabaseClusterRestore(ctx echo.Context, kubernetesID } // UpdateDatabaseClusterRestore Replace the specified cluster restore on the specified kubernetes cluster. -func (e *EverestServer) UpdateDatabaseClusterRestore(ctx echo.Context, kubernetesID string, name string) error { //nolint:cyclop +func (e *EverestServer) UpdateDatabaseClusterRestore(ctx echo.Context, kubernetesID string, name string) error { _, kubeClient, code, err := e.initKubeClient(ctx.Request().Context(), kubernetesID) if err != nil { return ctx.JSON(code, Error{Message: pointer.ToString(err.Error())}) @@ -150,7 +154,7 @@ func (e *EverestServer) UpdateDatabaseClusterRestore(ctx echo.Context, kubernete } if newRestore.Spec == nil { - return ctx.JSON(http.StatusBadRequest, Error{Message: pointer.ToString("'Spec' field should not be empty")}) + return ctx.JSON(http.StatusBadRequest, Error{Message: pointer.ToString("'spec' field should not be empty")}) } if newRestore.Spec.DbClusterName != oldRestore.Spec.DBClusterName { @@ -169,16 +173,21 @@ func (e *EverestServer) UpdateDatabaseClusterRestore(ctx echo.Context, kubernete oldRestore.Spec.DataSource.BackupSource.BackupStorageName == newRestore.Spec.DataSource.BackupSource.BackupStorageName { return nil } + if err := e.syncBackupStorages(newRestore, oldRestore, kubeClient); err != nil { + return ctx.JSON(http.StatusInternalServerError, Error{Message: pointer.ToString(err.Error())}) + } + + return nil +} +func (e *EverestServer) syncBackupStorages(newRestore *DatabaseClusterRestore, oldRestore *everestv1alpha1.DatabaseClusterRestore, kubeClient *kubernetes.Kubernetes) error { // need to create the new BackupStorages CRs toCreateNames := map[string]struct{}{ newRestore.Spec.DataSource.BackupSource.BackupStorageName: {}, } - if err = e.createK8SBackupStorages(context.Background(), kubeClient, toCreateNames); err != nil { + if err := e.createK8SBackupStorages(context.Background(), kubeClient, toCreateNames); err != nil { e.l.Error(err) - return ctx.JSON(http.StatusInternalServerError, Error{ - Message: pointer.ToString("Could not create BackupStorage"), - }) + return errors.New("could not create BackupStorage") } // need to delete unused BackupStorages CRs diff --git a/api/monitoring_instance.go b/api/monitoring_instance.go index 5dbb4988..ac2bd062 100644 --- a/api/monitoring_instance.go +++ b/api/monitoring_instance.go @@ -147,7 +147,7 @@ func (e *EverestServer) UpdateMonitoringInstance(ctx echo.Context, name string) } // DeleteMonitoringInstance deletes a monitoring instance. -func (e *EverestServer) DeleteMonitoringInstance(ctx echo.Context, name string) error { //nolint:cyclop +func (e *EverestServer) DeleteMonitoringInstance(ctx echo.Context, name string) error { i, err := e.storage.GetMonitoringInstance(name) if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { e.l.Error(err) @@ -180,29 +180,36 @@ func (e *EverestServer) DeleteMonitoringInstance(ctx echo.Context, name string) }) if err != nil && !errors.Is(err, kubernetes.ErrConfigInUse) { e.l.Error(errors.Join(err, errors.New("could not delete monitoring config from kubernetes cluster"))) - return ctx.JSON(http.StatusInternalServerError, Error{Message: pointer.ToString("Could not delete monitoring config from the Kubernetes cluster")}) + if errors.Is(err, kubernetes.ErrConfigInUse) { + return ctx.JSON(http.StatusBadRequest, Error{Message: pointer.ToString("Could not delete monitoring config from the Kubernetes cluster")}) + } + return ctx.JSON(http.StatusInternalServerError, Error{Message: pointer.ToString(err.Error())}) } + err = e.deleteMonitoringConfig(ctx.Request().Context(), i) - err = e.storage.Transaction(func(tx *gorm.DB) error { + if err != nil { + return ctx.JSON(http.StatusInternalServerError, Error{ + Message: pointer.ToString(err.Error()), + }) + } + + return ctx.NoContent(http.StatusNoContent) +} + +func (e *EverestServer) deleteMonitoringConfig(c context.Context, i *model.MonitoringInstance) error { + return e.storage.Transaction(func(tx *gorm.DB) error { if err := e.storage.DeleteMonitoringInstance(i.Name, tx); err != nil { e.l.Error(err) return errors.New("could not delete monitoring instance") } - _, err = e.secretsStorage.DeleteSecret(context.Background(), i.APIKeySecretID) + _, err := e.secretsStorage.DeleteSecret(c, i.APIKeySecretID) if err != nil { return errors.Join(err, fmt.Errorf("could not delete monitoring instance API key secret %s", i.APIKeySecretID)) } return nil }) - if err != nil { - return ctx.JSON(http.StatusInternalServerError, Error{ - Message: pointer.ToString(err.Error()), - }) - } - - return ctx.NoContent(http.StatusNoContent) } // monitoringInstanceToAPIJson converts monitoring instance model to API JSON response. @@ -237,7 +244,7 @@ func (e *EverestServer) createAndStorePMMApiKey(ctx context.Context, name, url, return apiKeyID, nil } -func (e *EverestServer) performMonitoringInstanceUpdate( //nolint:cyclop +func (e *EverestServer) performMonitoringInstanceUpdate( ctx echo.Context, name string, apiKeyID *string, previousAPIKeyID string, params *UpdateMonitoringInstanceJSONRequestBody, ) error { diff --git a/pkg/kubernetes/backup_storage.go b/pkg/kubernetes/backup_storage.go index bb683b69..0084cd67 100644 --- a/pkg/kubernetes/backup_storage.go +++ b/pkg/kubernetes/backup_storage.go @@ -9,7 +9,7 @@ import ( // IsBackupStorageConfigInUse returns true if the backup storage is in use // by the provided Kubernetes cluster. -func IsBackupStorageConfigInUse(ctx context.Context, name string, kubeClient *Kubernetes) (bool, error) { //nolint:cyclop +func IsBackupStorageConfigInUse(ctx context.Context, name string, kubeClient *Kubernetes) (bool, error) { dbs, err := kubeClient.ListDatabaseClusters(ctx) if err != nil { return false, errors.Join(err, errors.New("could not list database clusters in Kubernetes")) diff --git a/pkg/kubernetes/client/mock_kube_client_connector.go b/pkg/kubernetes/client/mock_kube_client_connector.go index 59708262..72dc40f3 100644 --- a/pkg/kubernetes/client/mock_kube_client_connector.go +++ b/pkg/kubernetes/client/mock_kube_client_connector.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.34.0. DO NOT EDIT. +// Code generated by mockery v2.34.2. DO NOT EDIT. package client