EVEREST-785: fix validation when creating a monitoring instance (#407)

api/monitoring_instance.go

@@ -16,6 +16,7 @@
 package api
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 
@@ -31,7 +32,7 @@ import (
 )
 
 // CreateMonitoringInstance creates a new monitoring instance.
-func (e *EverestServer) CreateMonitoringInstance(ctx echo.Context) error { //nolint:funlen,cyclop
+func (e *EverestServer) CreateMonitoringInstance(ctx echo.Context) error {
 	params, err := validateCreateMonitoringInstanceRequest(ctx)
 	if err != nil {
 		return ctx.JSON(http.StatusBadRequest, Error{Message: pointer.ToString(err.Error())})
@@ -51,23 +52,45 @@ func (e *EverestServer) CreateMonitoringInstance(ctx echo.Context) error { //nol
 		e.l.Error(err)
 		return ctx.JSON(http.StatusConflict, Error{Message: pointer.ToString(err.Error())})
 	}
-	var apiKey string
-	if params.Pmm != nil && params.Pmm.ApiKey != "" {
-		apiKey = params.Pmm.ApiKey
+
+	apiKey, err := e.getPMMApiKey(c, params)
+	if err != nil {
+		e.l.Error(err)
+		return ctx.JSON(http.StatusInternalServerError, Error{
+			Message: pointer.ToString("Could not create an API key in PMM"),
+		})
 	}
-	if params.Pmm != nil && params.Pmm.ApiKey == "" && params.Pmm.User != "" && params.Pmm.Password != "" {
-		e.l.Debug("Getting PMM API key by username and password")
-		apiKey, err = pmm.CreatePMMApiKey(
-			c, params.Url, fmt.Sprintf("everest-%s-%s", params.Name, uuid.NewString()),
-			params.Pmm.User, params.Pmm.Password,
-		)
-		if err != nil {
-			e.l.Error(err)
-			return ctx.JSON(http.StatusInternalServerError, Error{
-				Message: pointer.ToString("Could not create an API key in PMM"),
-			})
-		}
+
+	if err := e.createMonitoringK8sResources(c, params, apiKey); err != nil {
+		return ctx.JSON(http.StatusInternalServerError, Error{
+			Message: pointer.ToString(err.Error()),
+		})
+	}
+
+	result := MonitoringInstance{
+		Type: MonitoringInstanceBaseWithNameType(params.Type),
+		Name: params.Name,
+		Url:  params.Url,
+	}
+
+	return ctx.JSON(http.StatusOK, result)
+}
+
+func (e *EverestServer) getPMMApiKey(ctx context.Context, params *CreateMonitoringInstanceJSONRequestBody) (string, error) {
+	if params.Pmm != nil && params.Pmm.ApiKey != "" {
+		return params.Pmm.ApiKey, nil
 	}
+
+	e.l.Debug("Getting PMM API key by username and password")
+	return pmm.CreatePMMApiKey(
+		ctx, params.Url, fmt.Sprintf("everest-%s-%s", params.Name, uuid.NewString()),
+		params.Pmm.User, params.Pmm.Password,
+	)
+}
+
+func (e *EverestServer) createMonitoringK8sResources(
+	c context.Context, params *CreateMonitoringInstanceJSONRequestBody, apiKey string,
+) error {
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      params.Name,
@@ -76,24 +99,19 @@ func (e *EverestServer) CreateMonitoringInstance(ctx echo.Context) error { //nol
 		Type:       corev1.SecretTypeOpaque,
 		StringData: e.monitoringConfigSecretData(apiKey),
 	}
-	_, err = e.kubeClient.CreateSecret(c, secret)
-	if err != nil {
+	if _, err := e.kubeClient.CreateSecret(c, secret); err != nil {
 		if k8serrors.IsAlreadyExists(err) {
 			_, err = e.kubeClient.UpdateSecret(c, secret)
 			if err != nil {
 				e.l.Error(err)
-				return ctx.JSON(http.StatusInternalServerError, Error{
-					Message: pointer.ToString(fmt.Sprintf("Could not update k8s secret %s", params.Name)),
-				})
+				return fmt.Errorf("could not update k8s secret %s", params.Name)
 			}
 		} else {
 			e.l.Error(err)
-			return ctx.JSON(http.StatusInternalServerError, Error{
-				Message: pointer.ToString("Failed creating secret in the Kubernetes cluster"),
-			})
+			return fmt.Errorf("failed creating secret in the Kubernetes cluster")
 		}
 	}
-	err = e.kubeClient.CreateMonitoringConfig(c, &everestv1alpha1.MonitoringConfig{
+	err := e.kubeClient.CreateMonitoringConfig(c, &everestv1alpha1.MonitoringConfig{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      params.Name,
 			Namespace: e.kubeClient.Namespace(),
@@ -108,24 +126,13 @@ func (e *EverestServer) CreateMonitoringInstance(ctx echo.Context) error { //nol
 	})
 	if err != nil {
 		e.l.Error(err)
-		// TODO: Move this logic to the operator
-		dErr := e.kubeClient.DeleteSecret(c, params.Name)
-		if dErr != nil {
-			return ctx.JSON(http.StatusInternalServerError, Error{
-				Message: pointer.ToString("Failing cleaning up the secret because failed creating backup storage"),
-			})
+		if dErr := e.kubeClient.DeleteSecret(c, params.Name); dErr != nil {
+			return fmt.Errorf("failed cleaning up the secret because failed creating backup storage")
 		}
-		return ctx.JSON(http.StatusInternalServerError, Error{
-			Message: pointer.ToString("Failed creating monitoring instance"),
-		})
-	}
-	result := MonitoringInstance{
-		Type: MonitoringInstanceBaseWithNameType(params.Type),
-		Name: params.Name,
-		Url:  params.Url,
+		return fmt.Errorf("failed creating monitoring instance")
 	}
 
-	return ctx.JSON(http.StatusOK, result)
+	return nil
 }
 
 // ListMonitoringInstances lists all monitoring instances.

api/validation.go

@@ -350,8 +350,8 @@ func validateCreateMonitoringInstanceRequest(ctx echo.Context) (*CreateMonitorin
 			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")
+		if params.Pmm.ApiKey == "" && (params.Pmm.User == "" || params.Pmm.Password == "") {
+			return nil, errors.New("pmm.apiKey or pmm.user with pmm.password fields are required")
 		}
 	default:
 		return nil, fmt.Errorf("monitoring type %s is not supported", params.Type)

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/labstack/echo/v4 v4.11.3
 	github.com/oapi-codegen/echo-middleware v1.0.1
 	github.com/oapi-codegen/runtime v1.1.0
-	github.com/percona/everest-operator v0.6.0-dev1.0.20240125150540-298621412982
+	github.com/percona/everest-operator v0.6.0-dev1.0.20240125162053-e43000fbf0b8
 	github.com/stretchr/testify v1.8.4
 	go.uber.org/zap v1.26.0
 	golang.org/x/crypto v0.17.0

go.sum

@@ -422,8 +422,8 @@ github.com/onsi/gomega v1.30.0/go.mod h1:9sxs+SwGrKI0+PWe4Fxa9tFQQBG5xSsSbMXOI8P
 github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
 github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
 github.com/pelletier/go-toml v1.7.0/go.mod h1:vwGMzjaWMwyfHwgIBhI2YUM4fB6nL6lVAvS1LBMMhTE=
-github.com/percona/everest-operator v0.6.0-dev1.0.20240125150540-298621412982 h1:rb3XM3Ce544WoX1Z41E7R0sL4KFEuidn0fYRhHen6Lg=
-github.com/percona/everest-operator v0.6.0-dev1.0.20240125150540-298621412982/go.mod h1:o84NcJlAImYMpKK9+PIjS4V8SSREt1uZOqNhHt5qXMg=
+github.com/percona/everest-operator v0.6.0-dev1.0.20240125162053-e43000fbf0b8 h1:nXu+L8fCl+vb9i24zqBCA5AjOMeeEGBn6M/KqWlmrCM=
+github.com/percona/everest-operator v0.6.0-dev1.0.20240125162053-e43000fbf0b8/go.mod h1:o84NcJlAImYMpKK9+PIjS4V8SSREt1uZOqNhHt5qXMg=
 github.com/percona/percona-backup-mongodb v1.8.1-0.20230920143330-3b1c2e263901 h1:BDgsZRCjEuxl2/z4yWBqB0s8d20shuIDks7/RVdZiLs=
 github.com/percona/percona-backup-mongodb v1.8.1-0.20230920143330-3b1c2e263901/go.mod h1:fZRCMpUqkWlLVdRKqqaj001LoVP2eo6F0ZhoMPeXDng=
 github.com/percona/percona-postgresql-operator v0.0.0-20231220140959-ad5eef722609 h1:+UOK4gcHrRgqjo4smgfwT7/0apF6PhAJdQIdAV4ub/M=