Skip to content

NO-JIRA: chore(extended/prometheus): reduce targets-auth skip list and handle mTLS #31372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
machine424 wants to merge 1 commit into
base: main
Choose a base branch
from
+43 −51
Open

NO-JIRA: chore(extended/prometheus): reduce targets-auth skip list and handle mTLS #31372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 27 additions & 39 deletions test/extended/prometheus/prometheus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"encoding/json"
"fmt"
"net/http"
"os"
"regexp"
"runtime"
"slices"
Expand Down Expand Up @@ -56,24 +55,11 @@ type TelemeterClientConfig struct {
Enabled *bool `json:"enabled"`
}

// Set $MONITORING_AUTH_TEST_NAMESPACE to focus on the targets from a single namespace
var namespaceUnderTest = os.Getenv("MONITORING_AUTH_TEST_NAMESPACE")

@jan--f jan--f Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a short comment why this is no longer needed can be helpful to future readers.

@machine424 machine424 Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can leave the comment here:
this is no longer needed and was making the logic hard to read.


var _ = g.Describe("[sig-instrumentation][Late] Platform Prometheus targets", func() {
defer g.GinkgoRecover()
var (
oc = exutil.NewCLIWithPodSecurityLevel("prometheus", admissionapi.LevelBaseline)
prometheusURL, bearerToken string

// TODO: remove the namespace when the bug is fixed.
namespacesToSkip = sets.New[string]("openshift-marketplace", // https://issues.redhat.com/browse/OCPBUGS-59763
"openshift-image-registry", // https://issues.redhat.com/browse/OCPBUGS-59767
"openshift-operator-lifecycle-manager", // https://issues.redhat.com/browse/OCPBUGS-59768
"openshift-cluster-samples-operator", // https://issues.redhat.com/browse/OCPBUGS-59769
"openshift-cluster-csi-drivers", // https://issues.redhat.com/browse/OCPBUGS-60159
"openshift-cluster-node-tuning-operator", // https://issues.redhat.com/browse/OCPBUGS-60258
"openshift-etcd", // https://issues.redhat.com/browse/OCPBUGS-60263
)
)

g.BeforeEach(func(ctx g.SpecContext) {
Expand All @@ -91,33 +77,27 @@ var _ = g.Describe("[sig-instrumentation][Late] Platform Prometheus targets", fu
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")

if namespacesToSkip.Has(namespaceUnderTest) {
e2e.Logf("The namespace %s is not skipped because $MONITORING_AUTH_TEST_NAMESPACE is set to it", namespaceUnderTest)
namespacesToSkip.Delete(namespaceUnderTest)
}
})

g.It("should not be accessible without auth [Serial]", func() {
expectedStatusCodes := sets.New(http.StatusUnauthorized, http.StatusForbidden)
// TODO: remove the namespace when the bug is fixed.
namespacesToSkip := []string{
"openshift-image-registry", // https://issues.redhat.com/browse/OCPBUGS-59767
"openshift-cluster-samples-operator", // https://issues.redhat.com/browse/OCPBUGS-59769
}

g.By("checking that targets reject the requests with 401 or 403")
expectedStatusCodes := []int{http.StatusUnauthorized, http.StatusForbidden}

// With network policies bypassed, targets should still reject unauthenticated requests.
g.By("deploying network policies to allow test pods through existing ones")
ports := []networkPolicyTarget{
{Namespace: "openshift-dns", Port: 9154},
{Namespace: "openshift-dns-operator", Port: 9393},
{Namespace: "openshift-ingress-operator", Port: 9393},
}
networkPolicies := BuildNetworkPolicies(oc.Namespace(), ports)
for _, networkPolicy := range networkPolicies {
policies, err := oc.AdminKubeClient().NetworkingV1().NetworkPolicies(networkPolicy.Namespace).List(context.Background(), metav1.ListOptions{})

@machine424 machine424 Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the goal is to ensure auth is present even if NPs are bypassed.
deploying unneeded allow NP is not a problem.

o.Expect(err).NotTo(o.HaveOccurred(), fmt.Sprintf("List networkpolicies in %s", networkPolicy.Namespace))
// If there are no network policies in the target namespace, the
// test pods should already have access. No need to install
// additional network policies.
if len(policies.Items) == 0 {
continue
}
_, err = oc.AdminKubeClient().NetworkingV1().NetworkPolicies(networkPolicy.Namespace).Create(context.Background(), &networkPolicy, metav1.CreateOptions{})
_, err := oc.AdminKubeClient().NetworkingV1().NetworkPolicies(networkPolicy.Namespace).Create(context.Background(), &networkPolicy, metav1.CreateOptions{})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
o.Expect(err).NotTo(o.HaveOccurred(), fmt.Sprintf("Create networkpolicy %s/%s", networkPolicy.Namespace, networkPolicy.Name))
}
defer func() {
Expand All @@ -129,6 +109,8 @@ var _ = g.Describe("[sig-instrumentation][Late] Platform Prometheus targets", fu
o.Expect(err).NotTo(o.HaveOccurred(), fmt.Sprintf("Delete networkpolicy %s/%s", networkPolicy.Namespace, networkPolicy.Name))
}
}()

g.By("checking that targets reject the requests with 401 or 403")
execPod := exutil.CreateExecPodOrFail(oc.AdminKubeClient(), oc.Namespace(), "execpod-targets-authorization")
defer func() {
err := oc.AdminKubeClient().CoreV1().Pods(execPod.Namespace).Delete(context.Background(), execPod.Name, *metav1.NewDeleteOptions(1))
Expand Down Expand Up @@ -161,13 +143,19 @@ var _ = g.Describe("[sig-instrumentation][Late] Platform Prometheus targets", fu
eg.Go(func() error {
targetNs, targetJob, targetPod, targetScrapeURL := target.Labels["namespace"], target.Labels["job"], target.Labels["pod"], target.ScrapeUrl
o.Expect(targetNs).NotTo(o.BeEmpty())
if namespaceUnderTest != "" && targetNs != namespaceUnderTest {
return nil
}

scrapeErr := wait.PollUntilContextTimeout(context.Background(), 10*time.Second, 5*time.Minute, true, func(context.Context) (bool, error) {
statusCode, err := helper.URLStatusCodeExecViaPod(execPod.Namespace, execPod.Name, targetScrapeURL)
e2e.Logf("scraping target %s of pod %s/%s/%s without auth returned %d, err: %v (skip=%t)", targetScrapeURL, targetNs, targetJob, targetPod, statusCode, err, namespacesToSkip.Has(targetNs))
if expectedStatusCodes.Has(statusCode) {
statusCode, curlErrMsg, err := helper.CurlExecViaPod(execPod.Namespace, execPod.Name, targetScrapeURL)
e2e.Logf("scraping target %s of pod %s/%s/%s without auth returned %d, curlErrMsg: %q, err: %v (skip=%t)", targetScrapeURL, targetNs, targetJob, targetPod, statusCode, curlErrMsg, err, slices.Contains(namespacesToSkip, targetNs))
if slices.Contains(expectedStatusCodes, statusCode) {
return true, nil
}

// Some targets enforce auth via mTLS (client certificates).
// We assume bad certificates will be rejected.
// https://issues.redhat.com/browse/OCPBUGS-60263 (etcd)
// https://issues.redhat.com/browse/OCPBUGS-59763 (marketplace)
if err != nil && statusCode == 0 && strings.Contains(curlErrMsg, "certificate required") {
return true, nil
}

Expand All @@ -178,16 +166,16 @@ var _ = g.Describe("[sig-instrumentation][Late] Platform Prometheus targets", fu
statusCode == http.StatusTooManyRequests {
return false, nil
}
return false, fmt.Errorf("expecting status code %v but returned %d", expectedStatusCodes.UnsortedList(), statusCode)
return false, fmt.Errorf("expecting status code %v but returned %d (curlErrMsg: %q)", expectedStatusCodes, statusCode, curlErrMsg)
})

// Ignoring targets that Prometheus no longer scrapes or fails to scrape.
// These may be leftovers from earlier tests.
// Reference: https://issues.redhat.com/browse/OCPBUGS-61193
if scrapeErr != nil && !namespacesToSkip.Has(targetNs) {
if scrapeErr != nil && !slices.Contains(namespacesToSkip, targetNs) {
targets, err := promTargets()
if err != nil {
e2e.Logf("refreshing state of target %s of pod %s/%s/%s failed, err: %v (skip=%t)", targetScrapeURL, targetNs, targetJob, targetPod, err, namespacesToSkip.Has(targetNs))
e2e.Logf("refreshing state of target %s of pod %s/%s/%s failed, err: %v (skip=%t)", targetScrapeURL, targetNs, targetJob, targetPod, err, slices.Contains(namespacesToSkip, targetNs))
targets = initialPromTargets
}
idx := slices.IndexFunc(targets.Data.ActiveTargets, func(t prometheusTarget) bool {
Expand Down
28 changes: 16 additions & 12 deletions test/extended/util/prometheus/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,19 +299,23 @@ func ExpectURLStatusCodeExecViaPod(ns, execPodName, url string, statusCodes ...i
return fmt.Errorf("last response from server was not in %v: %s", statusCodes, output)
}

// URLStatusCodeExecViaPod attempts connection to url via exec pod and returns the status code
// or an error if any errors happens during the process.
func URLStatusCodeExecViaPod(ns, name, url string) (int, error) {
cmd := fmt.Sprintf("curl -k -s -o /dev/null -w '%%{http_code}' %q", url)
output, err := e2eoutput.RunHostCmd(ns, name, cmd)
if err != nil {
return 0, fmt.Errorf("host command failed: %v\n%s", err, output)
}
ret, err := strconv.Atoi(output)
if err != nil {
return 0, fmt.Errorf("unable to parse status code out of the command's ouput: %v\n%s", err, output)
// CurlExecViaPod attempts connection to url via exec pod and returns the HTTP
// status code and the error message reported by curl.
func CurlExecViaPod(ns, name, url string) (int, string, error) {
cmd := fmt.Sprintf("curl -k -s -o /dev/null -w '%%{http_code} %%{errormsg}' %q", url)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
output, execErr := e2eoutput.RunHostCmd(ns, name, cmd)
// Parse output before checking execErr: curl's -w output is in stdout even
// when curl exits non-zero (e.g. TLS rejection), and RunHostCmd returns that
// output alongside the error.
statusStr, errMsg, _ := strings.Cut(output, " ")
statusCode, parseErr := strconv.Atoi(statusStr)
if parseErr != nil {
if execErr != nil {
return 0, "", fmt.Errorf("host command failed: %v\n%s", execErr, output)
}
return 0, output, fmt.Errorf("unable to parse command output: %v\n%s", parseErr, output)
}
return ret, nil
return statusCode, errMsg, execErr
}

// ExpectPrometheusEndpoint attempts to connect to the metrics endpoint with
Expand Down