Skip to content

Commit 8e4a840

Browse files
hongkailiuhongkailiu
committed
Ensure ServiceMonitor's endpoints are protected
1 parent 848143e commit 8e4a840

File tree

2 files changed

+119
-4
lines changed

2 files changed

+119
-4
lines changed

‎test/extended/prometheus/prometheus.go‎

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"context"
66
"encoding/json"
77
"fmt"
8+
"os"
89
"regexp"
910
"strings"
1011
"time"
@@ -14,6 +15,8 @@ import (
1415

1516
g "github.com/onsi/ginkgo/v2"
1617
o "github.com/onsi/gomega"
18+
prometheusoperatorv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
19+
prometheusoperatorv1client "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned/typed/monitoring/v1"
1720
promv1 "github.com/prometheus/client_golang/api/prometheus/v1"
1821
dto "github.com/prometheus/client_model/go"
1922
"github.com/prometheus/common/expfmt"
@@ -49,6 +52,111 @@ type TelemeterClientConfig struct {
4952
Enabled *bool `json:"enabled"`
5053
}
5154

55+
var componentNamespace = os.Getenv("COMPONENT_NAMESPACE")
56+
57+
var _ = g.Describe("[sig-instrumentation][Late] OpenShift service monitors [apigroup:image.openshift.io]", func() {
58+
defer g.GinkgoRecover()
59+
var (
60+
oc = exutil.NewCLIWithoutNamespace("prometheus")
61+
prometheusURL, bearerToken string
62+
63+
// TODO: remove the namespace when the bug is fixed.
64+
namespacesToSkip = sets.New[string]("openshift-monitoring", // https://issues.redhat.com/browse/MON-4304
65+
"openshift-marketplace", // https://issues.redhat.com/browse/OCPBUGS-59763
66+
"openshift-image-registry", // https://issues.redhat.com/browse/OCPBUGS-59767
67+
"openshift-operator-lifecycle-manager", // https://issues.redhat.com/browse/OCPBUGS-59768
68+
"openshift-cluster-samples-operator", // https://issues.redhat.com/browse/OCPBUGS-59769
69+
"openshift-cluster-version", // https://issues.redhat.com/browse/OCPBUGS-57585
70+
)
71+
)
72+
73+
g.BeforeEach(func(ctx g.SpecContext) {
74+
var err error
75+
prometheusURL, err = helper.PrometheusRouteURL(ctx, oc)
76+
o.Expect(err).NotTo(o.HaveOccurred(), "Get public url of prometheus")
77+
bearerToken, err = helper.RequestPrometheusServiceAccountAPIToken(ctx, oc)
78+
o.Expect(err).NotTo(o.HaveOccurred(), "Request prometheus service account API token")
79+
80+
if namespacesToSkip.Has(componentNamespace) {
81+
e2e.Logf("The namespace %s is not skipped because $COMPONENT_NAMESPACE is set to it", componentNamespace)
82+
namespacesToSkip.Delete(componentNamespace)
83+
}
84+
})
85+
86+
g.It("should not be accessible without authorization", func() {
87+
var errs []error
88+
89+
g.By("verifying all targets returns 401 or 403 without authorization")
90+
contents, err := helper.GetURLWithToken(helper.MustJoinUrlPath(prometheusURL, "api/v1/targets"), bearerToken)
91+
o.Expect(err).NotTo(o.HaveOccurred())
92+
93+
targets := &prometheusTargets{}
94+
err = json.Unmarshal([]byte(contents), targets)
95+
o.Expect(err).NotTo(o.HaveOccurred())
96+
97+
for _, target := range targets.Data.ActiveTargets {
98+
ns := target.Labels["namespace"]
99+
if !strings.HasPrefix(ns, "openshift-") {
100+
continue
101+
}
102+
if componentNamespace != "" && ns != componentNamespace {
103+
continue
104+
}
105+
// Error out only when the status code is obtained
106+
// Error on executing the cmd is skipped here because the endpoint does not expose any information.
107+
// Promethus may have troubles to scrape it but that is out of the scape of this test
108+
err = helper.ExpectURLStatusCodeExecViaPod("openshift-monitoring", "prometheus-k8s-0", target.ScrapeUrl, true, 401, 403)
109+
e2e.Logf("the scaple url %s for namespace %s without authorization returned error %v (skip=%t)", target.ScrapeUrl, ns, err, namespacesToSkip.Has(ns))
110+
if err != nil && !namespacesToSkip.Has(ns) {
111+
errs = append(errs, fmt.Errorf("the scaple url %s for namespace %s is accessible without authorization: %w", target.ScrapeUrl, ns, err))
112+
}
113+
}
114+
115+
g.By("verifying all service monitors are configured with authorization")
116+
client, err := prometheusoperatorv1client.NewForConfig(oc.AdminConfig())
117+
o.Expect(err).NotTo(o.HaveOccurred(), "Create monitoring client")
118+
119+
serviceMonitorList, err := client.ServiceMonitors("").List(context.Background(), metav1.ListOptions{})
120+
o.Expect(err).NotTo(o.HaveOccurred(), "List service monitors")
121+
122+
for _, sm := range serviceMonitorList.Items {
123+
// we do not check service monitors for user-workload-monitoring
124+
if !strings.HasPrefix(sm.Namespace, "openshift-") {
125+
continue
126+
}
127+
if componentNamespace != "" && sm.Namespace != componentNamespace {
128+
continue
129+
}
130+
ok := authorizationConfigured(sm)
131+
e2e.Logf("service monitor %s/%s has authorization: %t (skip=%t)", sm.Namespace, sm.Name, ok, namespacesToSkip.Has(sm.Namespace))
132+
if !ok && !namespacesToSkip.Has(sm.Namespace) {
133+
errs = append(errs, fmt.Errorf("service monitor %s/%s has no authorization", sm.Namespace, sm.Name))
134+
}
135+
}
136+
137+
o.Expect(errs).To(o.BeEmpty())
138+
})
139+
140+
})
141+
142+
// authorizationConfigured returns true if the service monitor is configured with authorization
143+
func authorizationConfigured(sm *prometheusoperatorv1.ServiceMonitor) bool {
144+
if sm == nil {
145+
return false
146+
}
147+
for _, e := range sm.Spec.Endpoints {
148+
if e.BasicAuth != nil ||
149+
e.OAuth2 != nil ||
150+
e.Authorization != nil ||
151+
e.BearerTokenFile != "" ||
152+
e.BearerTokenSecret != nil ||
153+
e.TLSConfig != nil {
154+
return true
155+
}
156+
}
157+
return false
158+
}
159+
52160
var _ = g.Describe("[sig-instrumentation][Late] OpenShift alerting rules [apigroup:image.openshift.io]", func() {
53161
defer g.GinkgoRecover()
54162

@@ -549,7 +657,7 @@ var _ = g.Describe("[sig-instrumentation] Prometheus [apigroup:image.openshift.i
549657
})).NotTo(o.HaveOccurred(), fmt.Sprintf("Did not find tsdb_samples_appended_total, tsdb_head_samples_appended_total, or prometheus_tsdb_head_samples_appended_total"))
550658

551659
g.By("verifying the Thanos querier service requires authentication")
552-
err := helper.ExpectURLStatusCodeExecViaPod(ns, execPod.Name, querySvcURL, 401, 403)
660+
err := helper.ExpectURLStatusCodeExecViaPod(ns, execPod.Name, querySvcURL, false, 401, 403)
553661
o.Expect(err).NotTo(o.HaveOccurred())
554662

555663
g.By("verifying a service account token is able to authenticate")

‎test/extended/util/prometheus/helpers.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,12 +283,19 @@ func ExpectHTTPStatusCode(url, bearerToken string, statusCodes ...int) error {
283283
}
284284

285285
// ExpectURLStatusCodeExecViaPod attempts connection to url via exec pod and returns an error
286-
// upon failure or if status return code is not equal to any of the statusCodes.
287-
func ExpectURLStatusCodeExecViaPod(ns, execPodName, url string, statusCodes ...int) error {
286+
// if status return code is not equal to any of the statusCodes, or
287+
// it is not tolerant to failures, i.e., allowFailure=false and some failure popped up when executing the command.
288+
// When allowFailure=false, it logs the error and the output and return nil.
289+
func ExpectURLStatusCodeExecViaPod(ns, execPodName, url string, allowFailure bool, statusCodes ...int) error {
288290
cmd := fmt.Sprintf("curl -k -s -o /dev/null -w '%%{http_code}' %q", url)
289291
output, err := e2eoutput.RunHostCmd(ns, execPodName, cmd)
290292
if err != nil {
291-
return fmt.Errorf("host command failed: %v\n%s", err, output)
293+
if !allowFailure {
294+
return fmt.Errorf("host command failed: %v\n%s", err, output)
295+
} else {
296+
framework.Logf("host command failed: %v\n%s", err, output)
297+
return nil
298+
}
292299
}
293300
for _, statusCode := range statusCodes {
294301
if output == strconv.Itoa(statusCode) {

0 commit comments

Comments
 (0)