Skip to content
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

Limit the Prometheus role to two namespaces by default #720

Merged
merged 2 commits into from
Dec 5, 2020

Conversation

alexellis
Copy link
Member

@alexellis alexellis commented Nov 29, 2020

Signed-off-by: Alex Ellis (OpenFaaS Ltd) alexellis2@gmail.com

Description

Limit the Prometheus role to two namespaces by default

Motivation and Context

Limit the Prometheus role to two namespaces

Unless clusterRole is specified, the Prometheus role will be
restricted to scraping from only a single namespace.

This fixes issue: #717 where I user complained that they did
not want to create a ClusterRole in their cluster.

How Has This Been Tested?

It has been tested with k3d and K8s 1.19 with and without the
--set clusterRole=true flag passed into the faas-netes helm
chart.

The second Role and RoleBinding needed a different name to the
ones in the primary namespace in order for the RBAC error to
go away in Prometheus.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

This is likely to break for any users wishing to upgrade, due to the existing ClusterRole which they
will have in their cluster and its mapping.

The upgrade procedure is to delete the roles and bindings and then to run a helm upgrade.

Unless clusterRole is specified, the Prometheus role will be
restricted to scraping from only a single namespace.

This fixes issue: #717 where I user complained that they did
not want to create a ClusterRole in their cluster.

It has been tested with k3d and K8s 1.19 with and without the
--set clusterRole=true flag passed into the faas-netes helm
chart.

The second Role and RoleBinding needed a different name to the
ones in the primary namespace in order for the RBAC error to
go away in Prometheus.

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
When the namespace was the same as the function namespace,
the Prometheus configuration was set to scrape openfaas twice.

I assume that this was either not a problem for users, or
it was de-duplicated by Prometheus.

Tested by looking at the output of the configmap with and
without differing namespaces set.

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
@alexellis
Copy link
Member Author

Thank you to @meyskens for suggesting that two Roles may be necessary. I tried two roles and bindings with the same name (but differing namespaces) which failed, but with unique names, it worked as expected.

@alexellis
Copy link
Member Author

@LucasRoesler just wondering when you could get to this?

Copy link
Member

@LucasRoesler LucasRoesler left a comment

Choose a reason for hiding this comment

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

Looks good and is working for me

@alexellis alexellis merged commit d998e05 into master Dec 5, 2020
@alexellis alexellis deleted the openfaasltd/limit-prometheus-role branch December 5, 2020 15:29
@alexellis
Copy link
Member Author

Thank you

@LucasRoesler LucasRoesler restored the openfaasltd/limit-prometheus-role branch May 22, 2021 07:15
@alexellis alexellis deleted the openfaasltd/limit-prometheus-role branch May 23, 2022 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants