Ensure ServiceMonitor's endpoints are protected

Author: hongkailiu

test/extended/prometheus/prometheus.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"os"
 	"regexp"
 	"strings"
 	"time"
@@ -14,6 +15,8 @@ import (
 
 	g "github.com/onsi/ginkgo/v2"
 	o "github.com/onsi/gomega"
+	prometheusoperatorv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
+	prometheusoperatorv1client "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned/typed/monitoring/v1"
 	promv1 "github.com/prometheus/client_golang/api/prometheus/v1"
 	dto "github.com/prometheus/client_model/go"
 	"github.com/prometheus/common/expfmt"
@@ -49,6 +52,97 @@ type TelemeterClientConfig struct {
 	Enabled *bool `json:"enabled"`
 }
 
+var componentNamespace = os.Getenv("COMPONENT_NAMESPACE")
+
+var _ = g.Describe("[sig-instrumentation][Late] OpenShift service monitors [apigroup:image.openshift.io]", func() {
+	defer g.GinkgoRecover()
+	var (
+		oc                         = exutil.NewCLIWithoutNamespace("prometheus")
+		prometheusURL, bearerToken string
+	)
+
+	g.BeforeEach(func(ctx g.SpecContext) {
+		var err error
+		prometheusURL, err = helper.PrometheusRouteURL(ctx, oc)
+		o.Expect(err).NotTo(o.HaveOccurred(), "Get public url of prometheus")
+		bearerToken, err = helper.RequestPrometheusServiceAccountAPIToken(ctx, oc)
+		o.Expect(err).NotTo(o.HaveOccurred(), "Request prometheus service account API token")
+	})
+
+	g.It("should not be accessible without authorization", func() {
+		var errs []error
+
+		g.By("verifying all targets returns 401 or 403 without authorization")
+		contents, err := helper.GetURLWithToken(helper.MustJoinUrlPath(prometheusURL, "api/v1/targets"), bearerToken)
+		o.Expect(err).NotTo(o.HaveOccurred())
+
+		targets := &prometheusTargets{}
+		err = json.Unmarshal([]byte(contents), targets)
+		o.Expect(err).NotTo(o.HaveOccurred())
+
+		for _, target := range targets.Data.ActiveTargets {
+			ns := target.Labels["namespace"]
+			if !strings.HasPrefix(ns, "openshift-") {
+				continue
+			}
+			if componentNamespace != "" && ns != componentNamespace {
+				continue
+			}
+			// Error out only when the status code is obtained
+			// Error on executing the cmd is skipped here because the endpoint does not expose any information.
+			// Promethus may have troubles to scrape it but that is out of the scape of this test
+			err = helper.ExpectURLStatusCodeExecViaPod("openshift-monitoring", "prometheus-k8s-0", target.ScrapeUrl, true, 401, 403)
+			if err != nil {
+				errs = append(errs, fmt.Errorf("the scaple url %s for namespace %s is accessible without authorization: %w", target.ScrapeUrl, ns, err))
+			} else {
+				e2e.Logf("the scaple url %s for namespace %s is not accessible without authorization", target.ScrapeUrl, ns)
+			}
+		}
+
+		g.By("verifying all service monitors are configured with authorization")
+		client, err := prometheusoperatorv1client.NewForConfig(oc.AdminConfig())
+		o.Expect(err).NotTo(o.HaveOccurred(), "Create monitoring client")
+
+		serviceMonitorList, err := client.ServiceMonitors("").List(context.Background(), metav1.ListOptions{})
+		o.Expect(err).NotTo(o.HaveOccurred(), "List service monitors")
+
+		for _, sm := range serviceMonitorList.Items {
+			// we do not check service monitors for user-workload-monitoring
+			if !strings.HasPrefix(sm.Namespace, "openshift-") {
+				continue
+			}
+			if componentNamespace != "" && sm.Namespace != componentNamespace {
+				continue
+			}
+			if !authorizationConfigured(sm) {
+				errs = append(errs, fmt.Errorf("service monitor %s/%s has no authorization", sm.Namespace, sm.Name))
+			} else {
+				e2e.Logf("service monitor %s/%s has authorization", sm.Namespace, sm.Name)
+			}
+		}
+
+		o.Expect(errs).To(o.BeEmpty())
+	})
+
+})
+
+// authorizationConfigured returns true if the service monitor is configured with authorization
+func authorizationConfigured(sm *prometheusoperatorv1.ServiceMonitor) bool {
+	if sm == nil {
+		return false
+	}
+	for _, e := range sm.Spec.Endpoints {
+		if e.BasicAuth != nil ||
+			e.OAuth2 != nil ||
+			e.Authorization != nil ||
+			e.BearerTokenFile != "" ||
+			e.BearerTokenSecret != nil {
+			return true
+		}
+	}
+	return false
+}
+
 var _ = g.Describe("[sig-instrumentation][Late] OpenShift alerting rules [apigroup:image.openshift.io]", func() {
 	defer g.GinkgoRecover()
 
@@ -549,7 +643,7 @@ var _ = g.Describe("[sig-instrumentation] Prometheus [apigroup:image.openshift.i
 			})).NotTo(o.HaveOccurred(), fmt.Sprintf("Did not find tsdb_samples_appended_total, tsdb_head_samples_appended_total, or prometheus_tsdb_head_samples_appended_total"))
 
 			g.By("verifying the Thanos querier service requires authentication")
-			err := helper.ExpectURLStatusCodeExecViaPod(ns, execPod.Name, querySvcURL, 401, 403)
+			err := helper.ExpectURLStatusCodeExecViaPod(ns, execPod.Name, querySvcURL, false, 401, 403)
 			o.Expect(err).NotTo(o.HaveOccurred())
 
 			g.By("verifying a service account token is able to authenticate")

test/extended/util/prometheus/helpers.go

@@ -283,12 +283,19 @@ func ExpectHTTPStatusCode(url, bearerToken string, statusCodes ...int) error {
 }
 
 // ExpectURLStatusCodeExecViaPod attempts connection to url via exec pod and returns an error
-// upon failure or if status return code is not equal to any of the statusCodes.
-func ExpectURLStatusCodeExecViaPod(ns, execPodName, url string, statusCodes ...int) error {
+// if status return code is not equal to any of the statusCodes, or
+// it is not tolerant to failures, i.e., allowFailure=false and some failure popped up when executing the command.
+// When allowFailure=false, it logs the error and the output and return nil.
+func ExpectURLStatusCodeExecViaPod(ns, execPodName, url string, allowFailure bool, statusCodes ...int) error {
 	cmd := fmt.Sprintf("curl -k -s -o /dev/null -w '%%{http_code}' %q", url)
 	output, err := e2eoutput.RunHostCmd(ns, execPodName, cmd)
 	if err != nil {
-		return fmt.Errorf("host command failed: %v\n%s", err, output)
+		if !allowFailure {
+			return fmt.Errorf("host command failed: %v\n%s", err, output)
+		} else {
+			framework.Logf("host command failed: %v\n%s", err, output)
+			return nil
+		}
 	}
 	for _, statusCode := range statusCodes {
 		if output == strconv.Itoa(statusCode) {