From 74d7782e6002486f12425a8a78565d53afca0e8a Mon Sep 17 00:00:00 2001 From: Michal Kralik Date: Mon, 29 Jan 2024 10:28:03 +0100 Subject: [PATCH] EVEREST-785: fix validation when creating a monitoring instance (#407) --- api/monitoring_instance.go | 87 ++++++++++++++++++++------------------ api/validation.go | 4 +- go.mod | 2 +- go.sum | 4 +- 4 files changed, 52 insertions(+), 45 deletions(-) diff --git a/api/monitoring_instance.go b/api/monitoring_instance.go index 77a4e3ce..93d3b6a3 100644 --- a/api/monitoring_instance.go +++ b/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. diff --git a/api/validation.go b/api/validation.go index f8b3cc8c..06ef5afb 100644 --- a/api/validation.go +++ b/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) diff --git a/go.mod b/go.mod index c4396a80..66710231 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 787cab16..51780c77 100644 --- a/go.sum +++ b/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=