EVEREST-482 Removed nolint:funlen,cyclop directives (#202)

.golangci.yml

@@ -2,6 +2,9 @@
 # Almost all linters; some of them are optional.
 
 linters-settings:
+  cyclop:
+    max-complexity: 15
+
   depguard:
     rules:
       main:

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.

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)

api/database_cluster_restore.go

@@ -18,13 +18,17 @@ package api
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
 	"strings"
 
 	"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

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 {

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"))