Skip to content

fix(prometheus.operator.servicemonitors)!: Sync ServiceMonitor permission semantics with upstream - #6885

Open
jharvey10 wants to merge 8 commits into
mainfrom
jdh/fix-prom-operator
Open

fix(prometheus.operator.servicemonitors)!: Sync ServiceMonitor permission semantics with upstream#6885
jharvey10 wants to merge 8 commits into
mainfrom
jdh/fix-prom-operator

Conversation

@jharvey10

@jharvey10 jharvey10 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Brief description of Pull Request

Sync prometheus.operator.servicemonitors with Prometheus Operator's arbitrary filesystem access guard for ServiceMonitor endpoints. ServiceMonitor endpoints now reject bearerTokenFile, tlsConfig.caFile, tlsConfig.certFile, and tlsConfig.keyFile by default, while allowing trusted deployments to opt out with disallow_arbitrary_file_access = false.

BREAKING CHANGE: prometheus.operator.servicemonitors now rejects ServiceMonitor endpoints that reference local files through bearerTokenFile, tlsConfig.caFile, tlsConfig.certFile, or tlsConfig.keyFile by default. Set disallow_arbitrary_file_access = false to preserve the previous behavior for trusted ServiceMonitor authors.

PR Checklist

  • Documentation added
  • Tests updated
  • This pull request was substantially generated with AI assistance (see the GenAI policy)

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

💻 Deploy preview available (fix(prometheus.operator): Sync ServiceMonitor permission semantics with upstream):

Copilot AI left a comment

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.

Pull request overview

Aligns ServiceMonitor filesystem-access safeguards with Prometheus Operator semantics.

Changes:

  • Adds a default-on arbitrary file access guard.
  • Rejects unsafe ServiceMonitor endpoint fields and emits warnings.
  • Adds tests and security documentation.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/component/prometheus/operator/types.go Adds the configuration argument and default.
internal/component/prometheus/operator/configgen/config_gen.go Threads the guard into config generation.
internal/component/prometheus/operator/configgen/config_gen_servicemonitor.go Detects and rejects file references.
internal/component/prometheus/operator/configgen/config_gen_servicemonitor_test.go Tests allowed and rejected endpoint fields.
internal/component/prometheus/operator/common/crdmanager.go Applies the guard and records warnings atomically.
internal/component/prometheus/operator/common/crdmanager_test.go Tests warnings, rejection, and partial-config handling.
docs/sources/reference/components/prometheus/prometheus.operator.servicemonitors.md Documents security behavior and configuration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/component/prometheus/operator/types.go Outdated
Comment thread internal/component/prometheus/operator/common/crdmanager.go

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (2)

docs/sources/reference/components/prometheus/prometheus.operator.servicemonitors.md:50

  • Keep the required forward_to row before the new optional argument. The component documentation convention requires required arguments first and optional arguments afterward (docs/developer/writing-component-documentation.md:184).
| `disallow_arbitrary_file_access`   | `bool`                  | Disallow ServiceMonitor endpoints that reference arbitrary files on the {{< param "PRODUCT_NAME" >}} filesystem. | `true`        | no       |
| `forward_to`                       | `list(MetricsReceiver)` | List of receivers to send scraped metrics to.                                                             |               | yes      |

internal/component/prometheus/operator/common/crdmanager.go:597

  • The staged maps prevent partial endpoints from entering the manager state, but the error path still returns without applying that state. During an update, onUpdateServiceMonitor clears the previous entries first, so the discovery and scrape managers continue running the old configuration until an unrelated reconciliation calls apply(), even though this ServiceMonitor is reported as rejected. Apply the post-clear state before returning so rejection takes effect immediately.
	discoveryConfigs := map[string]discovery.Configs{}
	scrapeConfigs := map[string]*config.ScrapeConfig{}

@jharvey10 jharvey10 changed the title fix(prometheus.operator): Sync ServiceMonitor permission semantics with upstream fix(prometheus.operator.servicemonitors): Sync ServiceMonitor permission semantics with upstream Aug 13, 2026
@jharvey10 jharvey10 changed the title fix(prometheus.operator.servicemonitors): Sync ServiceMonitor permission semantics with upstream fix(prometheus.operator.servicemonitors)!: Sync ServiceMonitor permission semantics with upstream Aug 13, 2026
@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Aug 13, 2026
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

💻 Deploy preview available (fix(prometheus.operator.servicemonitors)!: Sync ServiceMonitor permission semantics with upstream):

@jharvey10
jharvey10 marked this pull request as ready for review August 14, 2026 14:49
@jharvey10
jharvey10 requested a review from a team as a code owner August 14, 2026 14:49
@clayton-cornell

Copy link
Copy Markdown
Contributor

Doc part looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/docs Docs Squad label across all Grafana Labs repos

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants