-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Ensure Platform Prometheus targets are not scraped with insecure_skip_verify #31373
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -214,6 +214,52 @@ var _ = g.Describe("[sig-instrumentation][Late] Platform Prometheus targets", fu | |
| o.Expect(errs).To(o.BeEmpty()) | ||
| }) | ||
|
|
||
| g.It("should not skip TLS CA verification", func() { | ||
| // TODO: remove the job when the bug is fixed. | ||
| jobsToSkip := []string{ | ||
| "serviceMonitor/openshift-apiserver/openshift-apiserver-operator-check-endpoints/0", // https://issues.redhat.com/browse/OCPBUGS-98917 | ||
| "serviceMonitor/openshift-network-diagnostics/network-check-source/0", // https://issues.redhat.com/browse/OCPBUGS-98918 | ||
| } | ||
|
Comment on lines
+217
to
+222
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once the bugs are fixed, these entries become silent dead weight. Optional hardening: after the loop, assert each allowlisted job was observed with insecure_skip_verify: true, and fail if an exception is unused so the TODO list self-cleans. Same pattern as “exclude list should be reduced to 0” elsewhere in this file may be good ;)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, but I don't think TRT and the other teams would appreciate sudden, unannounced test failures. It will create unnecessary noise, require extra triage resources, and potentially block CI for teams in other timezones while we aren't online to fix it. I made a change to get logs like once the bugs are fixed. we can use those to update our tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree there, thanks for the explanation! lgtm! |
||
|
|
||
| g.By("fetching the Prometheus scrape configuration") | ||
| contents, err := helper.GetURLWithToken(helper.MustJoinUrlPath(prometheusURL, "api/v1/status/config"), bearerToken) | ||
| o.Expect(err).NotTo(o.HaveOccurred(), "fetch Prometheus scrape configuration") | ||
|
|
||
| var configResp struct { | ||
| Data struct { | ||
| YAML string `json:"yaml"` | ||
| } `json:"data"` | ||
| } | ||
| err = json.Unmarshal([]byte(contents), &configResp) | ||
| o.Expect(err).NotTo(o.HaveOccurred(), "unmarshal Prometheus config API response") | ||
|
|
||
| var promConfig struct { | ||
| ScrapeConfigs []struct { | ||
| JobName string `json:"job_name"` | ||
| TLSConfig *struct { | ||
| InsecureSkipVerify bool `json:"insecure_skip_verify"` | ||
| } `json:"tls_config"` | ||
| } `json:"scrape_configs"` | ||
| } | ||
| err = yaml.Unmarshal([]byte(configResp.Data.YAML), &promConfig) | ||
| o.Expect(err).NotTo(o.HaveOccurred(), "unmarshal Prometheus scrape configuration YAML") | ||
| // sanity check. | ||
| o.Expect(len(promConfig.ScrapeConfigs)).To(o.BeNumerically(">=", 5), fmt.Sprintf("only got %d scrape configs, something is wrong", len(promConfig.ScrapeConfigs))) | ||
|
|
||
| g.By("checking that no job has insecure_skip_verify set to true") | ||
| var errs []error | ||
| for _, sc := range promConfig.ScrapeConfigs { | ||
| if sc.TLSConfig == nil { | ||
| continue | ||
| } | ||
| e2e.Logf("job %q has insecure_skip_verify=%t (skip=%t)", sc.JobName, sc.TLSConfig.InsecureSkipVerify, slices.Contains(jobsToSkip, sc.JobName)) | ||
| if sc.TLSConfig.InsecureSkipVerify && !slices.Contains(jobsToSkip, sc.JobName) { | ||
| errs = append(errs, fmt.Errorf("scrape config %q has insecure_skip_verify set to true", sc.JobName)) | ||
|
machine424 marked this conversation as resolved.
|
||
| } | ||
| } | ||
| o.Expect(errs).To(o.BeEmpty()) | ||
| }) | ||
|
|
||
| }) | ||
|
|
||
| var _ = g.Describe("[sig-instrumentation][Late] OpenShift alerting rules [apigroup:image.openshift.io]", func() { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.