From f9f7997f26001c9ab6b3a3dd4e3dba2c6d5a6091 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 +- 2 files changed, 49 insertions(+), 42 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 c94ce34d..d130fe98 100644 --- a/api/validation.go +++ b/api/validation.go @@ -341,8 +341,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)