From 0a1042ebfcf6ee7cf0f68029f8c1b1551ba680a0 Mon Sep 17 00:00:00 2001 From: QuantumEnigmaa Date: Wed, 22 May 2024 15:28:25 +0200 Subject: [PATCH] refactor code --- .../cluster_monitoring_controller.go | 18 ++--- main.go | 5 +- pkg/common/password/manager.go | 10 +++ pkg/common/secret/manager.go | 34 +++++++++ pkg/monitoring/mimir/ingress/ingress.go | 40 ----------- pkg/monitoring/mimir/service.go | 72 +++++++++++++++++-- pkg/monitoring/mimir/utils.go | 43 ----------- pkg/monitoring/prometheusagent/secret.go | 64 ++++++++++++----- pkg/monitoring/prometheusagent/service.go | 20 ++---- 9 files changed, 176 insertions(+), 130 deletions(-) create mode 100644 pkg/common/secret/manager.go delete mode 100644 pkg/monitoring/mimir/ingress/ingress.go delete mode 100644 pkg/monitoring/mimir/utils.go diff --git a/internal/controller/cluster_monitoring_controller.go b/internal/controller/cluster_monitoring_controller.go index 9670b8fe..7d779f36 100644 --- a/internal/controller/cluster_monitoring_controller.go +++ b/internal/controller/cluster_monitoring_controller.go @@ -120,16 +120,16 @@ func (r *ClusterMonitoringReconciler) reconcile(ctx context.Context, cluster *cl } } - // Create or update PrometheusAgent remote write configuration. - err := r.PrometheusAgentService.ReconcileRemoteWriteConfiguration(ctx, cluster) + err := r.MimirService.ConfigureMimir(ctx, r.ManagementCluster.Name) if err != nil { - logger.Error(err, "failed to create or update prometheus agent remote write config") + logger.Error(err, "failed to configure mimir") return ctrl.Result{RequeueAfter: 5 * time.Minute}, errors.WithStack(err) } - err = r.MimirService.ConfigureMimir(ctx, r.ManagementCluster.Name) + // Create or update PrometheusAgent remote write configuration. + err = r.PrometheusAgentService.ReconcileRemoteWriteConfiguration(ctx, cluster) if err != nil { - logger.Error(err, "failed to configure mimir") + logger.Error(err, "failed to create or update prometheus agent remote write config") return ctrl.Result{RequeueAfter: 5 * time.Minute}, errors.WithStack(err) } @@ -148,15 +148,15 @@ func (r *ClusterMonitoringReconciler) reconcileDelete(ctx context.Context, clust } } - err := r.PrometheusAgentService.DeleteRemoteWriteConfiguration(ctx, cluster) + err := r.MimirService.DeleteIngressSecret(ctx) if err != nil { - logger.Error(err, "failed to delete prometheus agent remote write config") + logger.Error(err, "failed to delete mimir ingress secret") return ctrl.Result{RequeueAfter: 5 * time.Minute}, errors.WithStack(err) } - err = r.MimirService.DeleteIngressSecret(ctx) + err = r.PrometheusAgentService.DeleteRemoteWriteConfiguration(ctx, cluster) if err != nil { - logger.Error(err, "failed to delete mimir ingress secret") + logger.Error(err, "failed to delete prometheus agent remote write config") return ctrl.Result{RequeueAfter: 5 * time.Minute}, errors.WithStack(err) } diff --git a/main.go b/main.go index b705ad8a..7cb64f9d 100644 --- a/main.go +++ b/main.go @@ -41,6 +41,7 @@ import ( "github.com/giantswarm/observability-operator/pkg/common" "github.com/giantswarm/observability-operator/pkg/common/organization" "github.com/giantswarm/observability-operator/pkg/common/password" + "github.com/giantswarm/observability-operator/pkg/common/secret" "github.com/giantswarm/observability-operator/pkg/monitoring/heartbeat" "github.com/giantswarm/observability-operator/pkg/monitoring/mimir" "github.com/giantswarm/observability-operator/pkg/monitoring/prometheusagent" @@ -197,7 +198,9 @@ func main() { } mimirService := mimir.MimirService{ - Client: mgr.GetClient(), + Client: mgr.GetClient(), + PasswordManager: password.SimpleManager{}, + SecretManager: secret.SimpleManager{}, } if err = (&controller.ClusterMonitoringReconciler{ diff --git a/pkg/common/password/manager.go b/pkg/common/password/manager.go index 80c2726b..5e91abfc 100644 --- a/pkg/common/password/manager.go +++ b/pkg/common/password/manager.go @@ -3,10 +3,12 @@ package password import ( "crypto/rand" "encoding/hex" + "os/exec" ) type Manager interface { GeneratePassword(length int) (string, error) + GenerateHtpasswd(username string, password string) (string, error) } type SimpleManager struct { @@ -19,3 +21,11 @@ func (m SimpleManager) GeneratePassword(length int) (string, error) { } return hex.EncodeToString(bytes), nil } + +func (m SimpleManager) GenerateHtpasswd(username string, password string) (string, error) { + htpasswd, err := exec.Command("htpasswd", "-bn", username, password).Output() + if err != nil { + return "", err + } + return string(htpasswd), nil +} diff --git a/pkg/common/secret/manager.go b/pkg/common/secret/manager.go new file mode 100644 index 00000000..9bef0007 --- /dev/null +++ b/pkg/common/secret/manager.go @@ -0,0 +1,34 @@ +package secret + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/giantswarm/observability-operator/pkg/monitoring" +) + +type Manager interface { + GenerateGenericSecret(secretName string, secretNamespace string, key string, value string) (*corev1.Secret, error) +} + +type SimpleManager struct { +} + +func (m SimpleManager) GenerateGenericSecret(secretName string, secretNamespace string, + key string, value string) (*corev1.Secret, error) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: secretNamespace, + Finalizers: []string{ + monitoring.MonitoringFinalizer, + }, + }, + Data: map[string][]byte{ + key: []byte(value), + }, + Type: "Opaque", + } + + return secret, nil +} diff --git a/pkg/monitoring/mimir/ingress/ingress.go b/pkg/monitoring/mimir/ingress/ingress.go deleted file mode 100644 index aa85ecce..00000000 --- a/pkg/monitoring/mimir/ingress/ingress.go +++ /dev/null @@ -1,40 +0,0 @@ -package ingress - -import ( - "os/exec" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/giantswarm/observability-operator/pkg/monitoring" -) - -const ( - secretName = "mimir-gateway-ingress" - secretNamespace = "mimir" -) - -func BuildIngressSecret(username string, password string) (*corev1.Secret, error) { - // Uses htpasswd to generate the password hash. - secretData, err := exec.Command("htpasswd", "-bn", username, password).Output() - if err != nil { - return nil, err - } - - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - // The secret name is hard coded so that it's easier to use on other places. - Name: secretName, - Namespace: secretNamespace, - Finalizers: []string{ - monitoring.MonitoringFinalizer, - }, - }, - Data: map[string][]byte{ - "auth": secretData, - }, - Type: "Opaque", - } - - return secret, nil -} diff --git a/pkg/monitoring/mimir/service.go b/pkg/monitoring/mimir/service.go index 15fe0603..e43a17f1 100644 --- a/pkg/monitoring/mimir/service.go +++ b/pkg/monitoring/mimir/service.go @@ -12,21 +12,38 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" + "github.com/giantswarm/observability-operator/pkg/common/password" + "github.com/giantswarm/observability-operator/pkg/common/secret" "github.com/giantswarm/observability-operator/pkg/monitoring" - "github.com/giantswarm/observability-operator/pkg/monitoring/mimir/ingress" + "github.com/giantswarm/observability-operator/pkg/monitoring/prometheusagent" +) + +const ( + ingressSecretName = "mimir-gateway-ingress" + ingressSecretNamespace = "mimir" + credsSecretName = "mimir-basic-auth" + credsSecretNamespace = "mimir" ) type MimirService struct { client.Client + PasswordManager password.Manager + SecretManager secret.Manager } func (ms *MimirService) ConfigureMimir(ctx context.Context, mc string) error { logger := log.FromContext(ctx).WithValues("cluster", mc) logger.Info("ensuring mimir config") - err := ms.CreateIngressSecret(ctx, mc, logger) + err := ms.CreateAuthSecret(ctx, logger, mc) + if err != nil { + logger.Error(err, "failed to create mimit auth secret") + return errors.WithStack(err) + } + + err = ms.CreateIngressSecret(ctx, mc, logger) if err != nil { - logger.Error(err, "failed to create or update mimir config") + logger.Error(err, "failed to create mimir ingress secret") return errors.WithStack(err) } @@ -35,6 +52,44 @@ func (ms *MimirService) ConfigureMimir(ctx context.Context, mc string) error { return nil } +func (ms *MimirService) CreateAuthSecret(ctx context.Context, logger logr.Logger, mc string) error { + objectKey := client.ObjectKey{ + Name: credsSecretName, + Namespace: credsSecretNamespace, + } + + current := &corev1.Secret{} + err := ms.Client.Get(ctx, objectKey, current) + if apierrors.IsNotFound(err) { + logger.Info("Building auth secret") + + password, err := ms.PasswordManager.GeneratePassword(32) + if err != nil { + return errors.WithStack(err) + } + + secretdata := mc + ":" + password + + secret, err := ms.SecretManager.GenerateGenericSecret(credsSecretName, credsSecretNamespace, "credentials", secretdata) + if err != nil { + return errors.WithStack(err) + } + + err = ms.Client.Create(ctx, secret) + if err != nil { + return errors.WithStack(err) + } + + logger.Info("Auth secret successfully created") + + return nil + } else if err != nil { + return errors.WithStack(err) + } + + return nil +} + func (ms *MimirService) CreateIngressSecret(ctx context.Context, mc string, logger logr.Logger) error { objectKey := client.ObjectKey{ Name: ingressSecretName, @@ -47,15 +102,21 @@ func (ms *MimirService) CreateIngressSecret(ctx context.Context, mc string, logg // CREATE SECRET logger.Info("building ingress secret") - password, err := GetMimirIngressPassword(ctx, mc) + password, err := prometheusagent.GetMimirIngressPassword(ctx) if err != nil { return errors.WithStack(err) } - secret, err := ingress.BuildIngressSecret(mc, password) + htpasswd, err := ms.PasswordManager.GenerateHtpasswd("whatever", password) if err != nil { return errors.WithStack(err) } + + secret, err := ms.SecretManager.GenerateGenericSecret(ingressSecretName, ingressSecretNamespace, "auth", htpasswd) + if err != nil { + return errors.WithStack(err) + } + err = ms.Client.Create(ctx, secret) if err != nil { return errors.WithStack(err) @@ -93,7 +154,6 @@ func (ms *MimirService) DeleteIngressSecret(ctx context.Context) error { controllerutil.RemoveFinalizer(desired, monitoring.MonitoringFinalizer) err = ms.Client.Patch(ctx, current, client.MergeFrom(desired)) if err != nil { - fmt.Println("ERROR REMOVING FINALIZER") return errors.WithStack(err) } diff --git a/pkg/monitoring/mimir/utils.go b/pkg/monitoring/mimir/utils.go deleted file mode 100644 index 7d415ca4..00000000 --- a/pkg/monitoring/mimir/utils.go +++ /dev/null @@ -1,43 +0,0 @@ -package mimir - -import ( - "context" - "fmt" - - corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/config" - - "github.com/giantswarm/observability-operator/pkg/monitoring/prometheusagent" -) - -const ( - ingressSecretName = "mimir-gateway-ingress" - ingressSecretNamespace = "mimir" -) - -func GetMimirIngressPassword(ctx context.Context, mc string) (string, error) { - cfg, err := config.GetConfig() - if err != nil { - return "", err - } - - c, err := client.New(cfg, client.Options{}) - if err != nil { - return "", err - } - - secret := &corev1.Secret{} - - err = c.Get(ctx, client.ObjectKey{ - Name: fmt.Sprintf("%s-remote-write-secret", mc), - Namespace: "org-giantswarm", - }, secret) - if err != nil { - return "", err - } - - mimirPassword, err := prometheusagent.ReadRemoteWritePasswordFromSecret(*secret) - - return mimirPassword, err -} diff --git a/pkg/monitoring/prometheusagent/secret.go b/pkg/monitoring/prometheusagent/secret.go index 063b312d..0ad70808 100644 --- a/pkg/monitoring/prometheusagent/secret.go +++ b/pkg/monitoring/prometheusagent/secret.go @@ -1,28 +1,66 @@ package prometheusagent import ( + "context" "fmt" + "strings" "github.com/pkg/errors" promv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/yaml" "github.com/giantswarm/observability-operator/pkg/monitoring" ) -const remoteWriteName = "mimir" +const ( + credsSecretName = "mimir-basic-auth" + credsSecretNamespace = "mimir" + remoteWriteName = "mimir" +) + +func GetMimirIngressPassword(ctx context.Context) (string, error) { + cfg, err := config.GetConfig() + if err != nil { + return "", err + } + + c, err := client.New(cfg, client.Options{}) + if err != nil { + return "", err + } + + secret := &corev1.Secret{} + + err = c.Get(ctx, client.ObjectKey{ + Name: credsSecretName, + Namespace: credsSecretNamespace, + }, secret) + if err != nil { + return "", err + } + + mimirPassword, err := readMimirAuthPasswordFromSecret(*secret) + + return mimirPassword, err +} func getPrometheusAgentRemoteWriteSecretName(cluster *clusterv1.Cluster) string { return fmt.Sprintf("%s-remote-write-secret", cluster.Name) } // buildRemoteWriteSecret builds the secret that contains the remote write configuration for the Prometheus agent. -func (pas PrometheusAgentService) buildRemoteWriteSecret( - cluster *clusterv1.Cluster, password string) (*corev1.Secret, error) { +func (pas PrometheusAgentService) buildRemoteWriteSecret(ctx context.Context, + cluster *clusterv1.Cluster) (*corev1.Secret, error) { url := fmt.Sprintf(remoteWriteEndpointTemplateURL, pas.ManagementCluster.BaseDomain, cluster.Name) + password, err := GetMimirIngressPassword(ctx) + if err != nil { + return nil, errors.WithStack(err) + } config := RemoteWriteConfig{ PrometheusAgentConfig: &PrometheusAgentConfig{ @@ -43,7 +81,7 @@ func (pas PrometheusAgentService) buildRemoteWriteSecret( }, }, }, - Username: cluster.Name, + Username: pas.ManagementCluster.Name, Password: password, }, }, @@ -70,21 +108,15 @@ func (pas PrometheusAgentService) buildRemoteWriteSecret( }, nil } -func ReadRemoteWritePasswordFromSecret(secret corev1.Secret) (string, error) { - remoteWriteConfig := RemoteWriteConfig{} - err := yaml.Unmarshal(secret.Data["values"], &remoteWriteConfig) +func readMimirAuthPasswordFromSecret(secret corev1.Secret) (string, error) { + var secretData string + + err := yaml.Unmarshal(secret.Data["credentials"], &secretData) if err != nil { return "", errors.WithStack(err) } - for _, rw := range remoteWriteConfig.PrometheusAgentConfig.RemoteWrite { - // We read the secret from the remote write configuration named `prometheus-meta-operator` only - // as this secret is generated per cluster. - // This will eventually be taken care of by the multi-tenancy contoller - if rw.Name == remoteWriteName { - return rw.Password, nil - } - } + password := strings.Split(secretData, ":")[1] - return "", errors.New("remote write password not found in secret") + return password, nil } diff --git a/pkg/monitoring/prometheusagent/service.go b/pkg/monitoring/prometheusagent/service.go index 443a3ff1..ba64568c 100644 --- a/pkg/monitoring/prometheusagent/service.go +++ b/pkg/monitoring/prometheusagent/service.go @@ -108,18 +108,14 @@ func (pas PrometheusAgentService) createOrUpdateSecret(ctx context.Context, // Get the current secret if it exists. err := pas.Client.Get(ctx, objectKey, current) if apierrors.IsNotFound(err) { - logger.Info("generating password for the prometheus agent") - password, err := pas.PasswordManager.GeneratePassword(32) + logger.Info("generating remote write secret for the prometheus agent") + secret, err := pas.buildRemoteWriteSecret(ctx, cluster) if err != nil { - logger.Error(err, "failed to generate the prometheus agent password") + logger.Error(err, "failed to generate the remote write secret for the prometheus agent") return errors.WithStack(err) } - logger.Info("generated password for the prometheus agent") + logger.Info("generated the remote write secret for the prometheus agent") - secret, err := pas.buildRemoteWriteSecret(cluster, password) - if err != nil { - return errors.WithStack(err) - } err = pas.Client.Create(ctx, secret) if err != nil { return errors.WithStack(err) @@ -128,14 +124,8 @@ func (pas PrometheusAgentService) createOrUpdateSecret(ctx context.Context, } else if err != nil { return errors.WithStack(err) } - // As it takes a long time to apply the new password to the agent due to a built-in delay in the app-platform, - // we keep the already generated remote write password. - password, err := ReadRemoteWritePasswordFromSecret(*current) - if err != nil { - return errors.WithStack(err) - } - desired, err := pas.buildRemoteWriteSecret(cluster, password) + desired, err := pas.buildRemoteWriteSecret(ctx, cluster) if err != nil { return errors.WithStack(err) }