LOG-8978: Enhance log-file-metrics-exporter to only allow scraping of metrics from a secure endpoint

Author: Clee2691

internal/auth/rbac.go

@@ -2,6 +2,7 @@ package auth
 
 import (
 	"fmt"
+
 	"github.com/openshift/cluster-logging-operator/internal/reconcile"
 	"github.com/openshift/cluster-logging-operator/internal/runtime"
 	"github.com/openshift/cluster-logging-operator/internal/utils"
@@ -10,6 +11,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+const systemAuthDelegatorClusterRoleName = "system:auth-delegator"
+
 // ReconcileRBAC reconciles the RBAC specifically for the service account and SCC
 func ReconcileRBAC(k8sClient client.Client, rbacName, saNamespace, saName string, owner metav1.OwnerReference) error {
 	desiredCRB := NewMetaDataReaderClusterRoleBinding(saNamespace, saName, owner)
@@ -25,6 +28,30 @@ func ReconcileRBAC(k8sClient client.Client, rbacName, saNamespace, saName string
 	return reconcile.RoleBinding(k8sClient, desiredSCCRoleBinding)
 }
 
+// ReconcileMetricsAuthRBAC reconciles the ClusterRoleBinding that binds system:auth-delegator to the service account
+func ReconcileMetricsAuthRBAC(k8sClient client.Client, commonName, saNamespace, saName string) error {
+	name := fmt.Sprintf("%s-metrics-auth", commonName)
+	desiredMetricsAuthRoleBinding := NewMetricsAuthClusterRoleBinding(name, saNamespace, saName)
+	return reconcile.ClusterRoleBinding(k8sClient, desiredMetricsAuthRoleBinding.Name, func() *rbacv1.ClusterRoleBinding { return desiredMetricsAuthRoleBinding })
+}
+
+// NewMetricsAuthClusterRoleBinding binds the system:auth-delegator ClusterRole to the given service account.
+func NewMetricsAuthClusterRoleBinding(name, saNamespace, saName string) *rbacv1.ClusterRoleBinding {
+	return runtime.NewClusterRoleBinding(
+		name,
+		rbacv1.RoleRef{
+			APIGroup: rbacv1.GroupName,
+			Kind:     "ClusterRole",
+			Name:     systemAuthDelegatorClusterRoleName,
+		},
+		rbacv1.Subject{
+			Kind:      "ServiceAccount",
+			Name:      saName,
+			Namespace: saNamespace,
+		},
+	)
+}
+
 // NewMetaDataReaderClusterRoleBinding stubs a clusterrolebinding to allow reading of pod metadata (i.e. labels)
 func NewMetaDataReaderClusterRoleBinding(saNamespace, saName string, owner metav1.OwnerReference) *rbacv1.ClusterRoleBinding {
 	name := fmt.Sprintf("metadata-reader-%s-%s", saNamespace, saName)

internal/metrics/logfilemetricexporter/factory.go

@@ -116,9 +116,8 @@ func newLogMetricsExporterContainer(exporter loggingv1a1.LogFileMetricExporter,
 	// TODO: When openshift/api is updated with the Groups field in TLSProfileSpec,
 	// pass groups from the profile spec instead of nil.
 	exporterContainer.Args = []string{"-c",
-		"/usr/local/bin/log-file-metric-exporter -verbosity=2 -dir=/var/log/pods -http=:2112 -keyFile=/etc/logfilemetricexporter/metrics/tls.key -crtFile=/etc/logfilemetricexporter/metrics/tls.crt -tlsMinVersion=" +
-			tls.MinTLSVersion(tlsProfileSpec) + " -cipherSuites=" + strings.Join(tls.TLSCiphers(tlsProfileSpec), ",") +
-			" -groups=" + strings.Join(tls.TLSGroups(nil), ",")}
+		"/usr/local/bin/log-file-metric-exporter -verbosity=2 -dir=/var/log/pods -http=:2112 -keyFile=/etc/logfilemetricexporter/metrics/tls.key -crtFile=/etc/logfilemetricexporter/metrics/tls.crt -secureMetrics -tlsMinVersion=" +
+			tls.MinTLSVersion(tlsProfileSpec) + " -cipherSuites=" + strings.Join(tls.TLSCiphers(tlsProfileSpec), ",")}
 
 	exporterContainer.VolumeMounts = []v1.VolumeMount{
 		{Name: logContainers, ReadOnly: true, MountPath: logContainersValue},

internal/metrics/logfilemetricexporter/metric_exporter.go

@@ -51,6 +51,11 @@ func Reconcile(lfmeInstance *loggingv1alpha1.LogFileMetricExporter,
 		return err
 	}
 
+	if err := auth.ReconcileMetricsAuthRBAC(requestClient, resNames.CommonName, lfmeInstance.Namespace, resNames.ServiceAccount); err != nil {
+		log.Error(err, "logfilemetricexporter.ReconcileMetricsRBAC")
+		return err
+	}
+
 	if err := network.ReconcileService(requestClient, lfmeInstance.Namespace, resNames.CommonName, lfmeInstance.Name, constants.LogfilesmetricexporterName, constants.MetricsPortName, ExporterMetricsSecretName, exporterPort, owner, commonLabels); err != nil {
 		log.Error(err, "logfilemetricexporter.ReconcileService")
 		return err

internal/metrics/logfilemetricexporter/metric_exporter_test.go

@@ -17,6 +17,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -128,6 +129,17 @@ var _ = Describe("Reconcile LogFileMetricExporter", func() {
 		svcURL := fmt.Sprintf("%s.openshift-logging.svc", constants.LogfilesmetricexporterName)
 		Expect(smInstance.Spec.Endpoints[0].TLSConfig.SafeTLSConfig.ServerName).
 			To(Equal(svcURL))
+
+		Expect(smInstance.Spec.Endpoints[0].BearerTokenFile).
+			To(Equal("/var/run/secrets/kubernetes.io/serviceaccount/token"))
+
+		// Metrics Auth RBAC
+		// Verify the metrics auth ClusterRoleBinding exists and references system:auth-delegator
+		metricsAuthBinding := &rbacv1.ClusterRoleBinding{}
+		Expect(reqClient.Get(context.TODO(), types.NamespacedName{Name: fmt.Sprintf("%s-metrics-auth", constants.LogfilesmetricexporterName)}, metricsAuthBinding)).Should(Succeed())
+		Expect(metricsAuthBinding.RoleRef.Name).To(Equal("system:auth-delegator"))
+		Expect(metricsAuthBinding.Subjects).To(HaveLen(1))
+		Expect(metricsAuthBinding.Subjects[0].Name).To(Equal(constants.LogfilesmetricexporterName))
 	})
 
 	Context("when the logfilemetricexporter NetworkPolicy is reconciled", func() {

internal/metrics/service_monitor.go

@@ -13,15 +13,17 @@ import (
 )
 
 const (
-	prometheusCAFile = "/etc/prometheus/configmaps/serving-certs-ca-bundle/service-ca.crt"
+	prometheusCAFile          = "/etc/prometheus/configmaps/serving-certs-ca-bundle/service-ca.crt"
+	prometheusBearerTokenFile = "/var/run/secrets/kubernetes.io/serviceaccount/token"
 )
 
 func newServiceMonitor(namespace, name string, owner metav1.OwnerReference, selector map[string]string, portName string) *monitoringv1.ServiceMonitor {
 	var endpoint = []monitoringv1.Endpoint{
 		{
-			Port:   portName,
-			Path:   "/metrics",
-			Scheme: "https",
+			Port:            portName,
+			Path:            "/metrics",
+			Scheme:          "https",
+			BearerTokenFile: prometheusBearerTokenFile,
 			TLSConfig: &monitoringv1.TLSConfig{
 				CAFile: prometheusCAFile,
 				SafeTLSConfig: monitoringv1.SafeTLSConfig{

internal/metrics/service_monitor_test.go

@@ -69,5 +69,8 @@ var _ = Describe("Reconcile ServiceMonitor", func() {
 		svcURL := fmt.Sprintf("%s.openshift-logging.svc", serviceName)
 		Expect(smInstance.Spec.Endpoints[0].TLSConfig.SafeTLSConfig.ServerName).
 			To(Equal(svcURL))
+
+		Expect(smInstance.Spec.Endpoints[0].BearerTokenFile).
+			To(Equal("/var/run/secrets/kubernetes.io/serviceaccount/token"))
 	})
 })

internal/reconcile/rbac.go

@@ -7,6 +7,7 @@ import (
 	log "github.com/ViaQ/logerr/v2/log/static"
 	"github.com/openshift/cluster-logging-operator/internal/runtime"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
@@ -43,25 +44,29 @@ func RoleBinding(k8Client client.Client, desired *rbacv1.RoleBinding) error {
 }
 
 func ClusterRoleBinding(k8sClient client.Client, name string, generator func() *rbacv1.ClusterRoleBinding) error {
-	wantRoleBinding := generator()
-	crb := runtime.NewClusterRoleBinding(name, rbacv1.RoleRef{})
-	op, err := controllerutil.CreateOrUpdate(context.TODO(), k8sClient, crb, func() error {
-		// Update the clusterrolebinding with our desired state
-		crb.RoleRef = wantRoleBinding.RoleRef
-		crb.Subjects = wantRoleBinding.Subjects
-		return nil
-	})
+	desired := generator()
+	existing := runtime.NewClusterRoleBinding(name, rbacv1.RoleRef{})
+	err := k8sClient.Get(context.TODO(), client.ObjectKeyFromObject(existing), existing)
+	if apierrors.IsNotFound(err) {
+		log.V(3).Info("Creating clusterRoleBinding", "name", name)
+		return k8sClient.Create(context.TODO(), desired)
+	}
+	if err != nil {
+		return err
+	}
 
-	if err == nil {
-		log.V(3).Info(fmt.Sprintf("reconciled clusterRoleBinding - operation: %s", op))
+	if existing.RoleRef != desired.RoleRef {
+		log.V(3).Info("Deleting clusterRoleBinding due to roleRef change", "name", name)
+		if err := k8sClient.Delete(context.TODO(), existing); err != nil {
+			return err
+		}
+		log.V(3).Info("Recreating clusterRoleBinding", "name", name)
+		return k8sClient.Create(context.TODO(), desired)
 	}
-	return err
-}
 
-func DeleteClusterRole(k8sClient client.Client, name string) error {
-	object := runtime.NewClusterRole(name)
-	log.V(3).Info("Deleting", "object", object)
-	return k8sClient.Delete(context.TODO(), object)
+	existing.Subjects = desired.Subjects
+	log.V(3).Info("Updating clusterRoleBinding", "name", name)
+	return k8sClient.Update(context.TODO(), existing)
 }
 
 func DeleteClusterRoleBinding(k8sClient client.Client, name string) error {

internal/reconcile/service_account.go

@@ -16,6 +16,7 @@ func ServiceAccount(k8Client client.Client, desired *v1.ServiceAccount) (*v1.Ser
 	op, err := controllerutil.CreateOrUpdate(context.TODO(), k8Client, sa, func() error {
 		sa.Annotations = desired.Annotations
 		sa.OwnerReferences = desired.OwnerReferences
+		sa.Finalizers = desired.Finalizers
 		return nil
 	})
 

test/e2e/logfilesmetricexporter/lfme_test.go

@@ -4,6 +4,11 @@ import (
 	"context"
 	_ "embed"
 	"fmt"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"time"
+
 	log "github.com/ViaQ/logerr/v2/log/static"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -13,9 +18,6 @@ import (
 	framework "github.com/openshift/cluster-logging-operator/test/framework/e2e"
 	"github.com/openshift/cluster-logging-operator/test/helpers/oc"
 	"k8s.io/apimachinery/pkg/util/wait"
-	"regexp"
-	"strings"
-	"time"
 )
 
 //go:embed valid.yaml
@@ -47,24 +49,54 @@ var _ = Describe("[e2e][logfilemetricexporter] LogFileMetricsExporter", func() {
 		Expect(err.Error()).To(MatchRegexp("is invalid.*supported values.*instance"), "exp. the CR to be rejected because it is not THE singleton")
 	})
 
-	It("should be deployed by the operator and producing metrics", func() {
+	It("should serve metrics to authorized clients providing a valid bearer token", func() {
 		e2e.AddCleanup(func() error {
 			return oc.Literal().From("oc -n openshift-logging delete --ignore-not-found logfilemetricexporter instance").Output()
 		})
+		metricsReaderRoleName := fmt.Sprintf("%s-metrics-reader", constants.ClusterLoggingOperator)
+		metricsReaderBindingName := fmt.Sprintf("%s-metrics-reader", constants.LogfilesmetricexporterName)
+		metricsAuthRoleName := fmt.Sprintf("%s-metrics-auth", constants.LogfilesmetricexporterName)
+		e2e.AddCleanup(func() error {
+			return oc.Literal().From("oc delete --ignore-not-found clusterrole %s", metricsReaderRoleName).Output()
+		})
+		e2e.AddCleanup(func() error {
+			return oc.Literal().From("oc delete --ignore-not-found clusterrolebinding %s", metricsReaderBindingName).Output()
+		})
+		// Delete the metrics auth ClusterRoleBinding
+		// The LFME reconciles the ClusterRoleBinding and ClusterRole for metrics auth
+		e2e.AddCleanup(func() error {
+			return oc.Literal().From("oc delete --ignore-not-found clusterrolebinding %s", metricsAuthRoleName).Output()
+		})
+
 		err = createLFME(validCR)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(e2e.WaitForDaemonSet(constants.OpenshiftNS, constants.LogfilesmetricexporterName)).To(Succeed())
 
-		args := []string{"-k", "-s", fmt.Sprintf("https://%s.%s.svc:2112/metrics", constants.LogfilesmetricexporterName, constants.OpenshiftNS)}
+		By("creating the metrics-reader ClusterRole")
+		roleFilePath, err := filepath.Abs(filepath.Join("..", "..", "..", "config", "rbac", "metrics_reader_role.yaml"))
+		Expect(err).ToNot(HaveOccurred(), "Failed to construct role file path")
+		_, err = oc.Literal().From("oc apply -f %s", roleFilePath).Run()
+		Expect(err).ToNot(HaveOccurred(), "Failed to create metrics-reader ClusterRole")
+
+		By("creating a ClusterRoleBinding for the service account to allow access to metrics")
+		_, err = oc.Literal().From("oc create clusterrolebinding %s --clusterrole=%s --serviceaccount=%s:%s",
+			metricsReaderBindingName, metricsReaderRoleName, constants.OpenshiftNS, constants.LogfilesmetricexporterName).Run()
+		Expect(err).ToNot(HaveOccurred(), "Failed to create ClusterRoleBinding")
+
+		metricsURL := fmt.Sprintf("https://%s.%s.svc:2112/metrics", constants.LogfilesmetricexporterName, constants.OpenshiftNS)
+		curlCmd := fmt.Sprintf(`curl -s -k -H "Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)" %s`, metricsURL)
+
 		err = wait.PollUntilContextTimeout(context.TODO(), time.Second, time.Second*30, true, func(context.Context) (done bool, err error) {
-			out, err := oc.Exec().WithNamespace(constants.OpenshiftNS).Pod(fmt.Sprintf("ds/%s", constants.LogfilesmetricexporterName)).WithCmd("curl", args...).Run()
+			out, err := oc.Exec().WithNamespace(constants.OpenshiftNS).
+				Pod(fmt.Sprintf("ds/%s", constants.LogfilesmetricexporterName)).
+				WithCmd("sh", "-c", curlCmd).Run()
 			Expect(err).ToNot(HaveOccurred(), out)
-			log.V(5).Info("Polling metrics", "result", out)
+			log.V(5).Info("Polling secure metrics", "result", out)
 			if !strings.Contains(out, "log_logged_bytes_total") {
 				return false, nil
 			}
 			return regexp.MatchString(`log_logged_bytes_total{.*} [1-9][0-9]*`, out)
 		})
-		Expect(err).ToNot(HaveOccurred(), "Exp. to find log_logged_bytes_total being calculated")
+		Expect(err).ToNot(HaveOccurred(), "Exp. to scrape metrics with a bearer token")
 	})
 })