Skip to content

Conversation

chrkl
Copy link
Member

@chrkl chrkl commented Jun 5, 2025

How to categorize this PR?

/area monitoring
/kind enhancement

What this PR does / why we need it:
All Prometheus instances were bound to a ClusterRole that grants read access to all Nodes, Services, Endpoints, and Pods. The Shoot specific Prometheus instances do not need access to all namespaces but only the shoot specific and the garden namespace in the Seed cluster. This PR introduces a namespace scoped Role and RoleBinding for the Shoot Prometheus instances.

Which issue(s) this PR fixes:
Fixes gardener/monitoring#50

Special notes for your reviewer:
/cc @vicwicker @istvanballok @rickardsjp

Release note:

The Shoot Prometheus RBAC is now restricted to the control-plane and the garden namespace.

@gardener-prow gardener-prow bot added area/monitoring Monitoring (including availability monitoring and alerting) related kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 5, 2025
Copy link
Contributor

@Kostov6 Kostov6 left a comment

Choose a reason for hiding this comment

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

Nit otherwise lgtm

@chrkl
Copy link
Member Author

chrkl commented Jun 11, 2025

/hold

@gardener-prow gardener-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 11, 2025
@chrkl chrkl force-pushed the prometheus-rbac branch from 0de3644 to fc286a0 Compare June 17, 2025 12:55
@ialidzhikov
Copy link
Member

@chrkl , can we remove the hold now and review the PR again? Or is there a particular reason you hold the PR?

@chrkl
Copy link
Member Author

chrkl commented Jun 23, 2025

@ialidzhikov We identified that these changes break the alertmanager discovery mechanism. I am still working on the PR.

@chrkl chrkl force-pushed the prometheus-rbac branch from fc286a0 to 9af1071 Compare June 24, 2025 08:28
@gardener-prow gardener-prow bot added cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. and removed cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. labels Jun 24, 2025
@chrkl chrkl force-pushed the prometheus-rbac branch from 9af1071 to ce8adde Compare June 26, 2025 11:18
@gardener-prow gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 27, 2025
@chrkl chrkl force-pushed the prometheus-rbac branch from ce8adde to b50002f Compare June 30, 2025 13:16
@gardener-prow gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2025
@chrkl chrkl force-pushed the prometheus-rbac branch from b50002f to 9c7780a Compare July 3, 2025 16:03
chrkl added 5 commits July 14, 2025 13:49
All Prometheus instances were deployed with the recommended ClusterRole from the
Prometheus-Operator documentation
(https://prometheus-operator.dev/docs/platform/rbac/#prometheus-rbac). This
grants cluster wide access for Shoot Prometheus instances, which is not
required. This commit introduces a namespace scoped Role for Shoot Prometheus
instances.
@chrkl chrkl force-pushed the prometheus-rbac branch from 9c7780a to faeb894 Compare July 14, 2025 11:53
@chrkl
Copy link
Member Author

chrkl commented Jul 14, 2025

/unhold

@gardener-prow gardener-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 14, 2025
@vicwicker
Copy link
Member

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 15, 2025
Copy link
Contributor

gardener-prow bot commented Jul 15, 2025

LGTM label has been added.

Git tree hash: ec491cebaaf2b4f96ff06791976df64c7383917e

@chrkl
Copy link
Member Author

chrkl commented Jul 16, 2025

@ialidzhikov could you review again? Thanks!

@gardener-prow gardener-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2025
Copy link
Member

@ialidzhikov ialidzhikov left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2025
Copy link
Contributor

gardener-prow bot commented Jul 29, 2025

LGTM label has been added.

Git tree hash: 8dfb224bbdbc07ddaa61976e55f72dc96511a587

Copy link
Contributor

gardener-prow bot commented Jul 29, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ialidzhikov

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2025
@gardener-prow gardener-prow bot merged commit b54d7e6 into gardener:master Jul 29, 2025
19 checks passed
@chrkl chrkl deleted the prometheus-rbac branch July 29, 2025 15:45
Duciwuci pushed a commit to stackitcloud/gardener that referenced this pull request Sep 1, 2025
…2264)

* Restrict Shoot Prometheus permissions to namespace

All Prometheus instances were deployed with the recommended ClusterRole from the
Prometheus-Operator documentation
(https://prometheus-operator.dev/docs/platform/rbac/#prometheus-rbac). This
grants cluster wide access for Shoot Prometheus instances, which is not
required. This commit introduces a namespace scoped Role for Shoot Prometheus
instances.

* Apply reviewer suggestions

* Fixed condition to deploy shoot Role and RoleBinding

* Grant Shoot Prometheus read access to Services in garden namespace

* Clarify used namespace by code comment

* Avoid usage of constants in unit tests

---------

Co-authored-by: Ismail Alidzhikov <9372594+ialidzhikov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/monitoring Monitoring (including availability monitoring and alerting) related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restrict Shoot Prometheus RBAC Permissions
4 participants