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

Rollout operator psp #3686

Merged
merged 4 commits into from
Dec 9, 2022
Merged

Rollout operator psp #3686

merged 4 commits into from
Dec 9, 2022

Conversation

breadly7
Copy link
Contributor

@breadly7 breadly7 commented Dec 9, 2022

What this PR does

This PR enables the rollout-operator serviceAccount to use podSecurityPolicies.

Which issue(s) this PR fixes or relates to

Currently mimir has the possibility to use podSecurityPolicies for all components with the exception of the rollout-operator,
which makes the deployment of the rollout-operator impossible in an environment, where the use of podSecurityPolicies are active.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@breadly7 breadly7 requested a review from a team as a code owner December 9, 2022 09:35
@CLAassistant
Copy link

CLAassistant commented Dec 9, 2022

CLA assistant check
All committers have signed the CLA.

@aknuds1 aknuds1 requested a review from pracucci December 9, 2022 09:50
@aknuds1 aknuds1 added the enhancement New feature or request label Dec 9, 2022
@@ -30,6 +30,7 @@ Entries should include a reference to the Pull Request that introduced the chang

* [ENHANCEMENT] Update the `rollout-operator` subchart to `0.2.0`. #3624
* [ENHANCEMENT] Add ability to manage PrometheusRule for metamonitoring with Prometheus operator from the Helm chart. The alerts are disabled by default but can be enabled with `prometheusRule.mimirAlerts` set to `true`. To enable the default rules, set `mimirRules` to `true`. #2134 #2609
* [BUGFIX] Enable `rollout-operator` to use PodSecurityPolicies if necessary
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this more of an enhancement rather than a bug fix?

Copy link
Contributor

Choose a reason for hiding this comment

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

On reading the changes (I'm unfamiliar with Helm), maybe it's a fix after all.

@aknuds1 aknuds1 requested a review from a team December 9, 2022 09:52
@aknuds1 aknuds1 added helm and removed enhancement New feature or request labels Dec 9, 2022
Copy link
Contributor

@krajorama krajorama left a comment

Choose a reason for hiding this comment

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

Please run make build-helm-tests as the failing CI suggest to generate the proper golden records.

In general I think this should be fixed in the rollout-operator, with adding the same kind of controls that the mimir helm chart has (values under rbac key) with additional validation in mimir's templates/validate.yaml to keep them in sync. (Or even better we should have common settings for such things in grafana helm charts. But that's long term.)

However, given that PSP is being deprecated and this seems like an easy enough fix, I think we can approve it.

@krajorama krajorama merged commit b1e834c into grafana:main Dec 9, 2022
@breadly7 breadly7 deleted the rollout-operator-psp branch December 10, 2022 08:25
masonmei pushed a commit to udmire/mimir that referenced this pull request Dec 16, 2022
* make rollout-operator able to use psp

* add new role-binding to generated templates

* add changelog entry

* add generated golden records
grobinson-grafana added a commit that referenced this pull request Jan 30, 2024
This commit updates Alertmanager to commit cab8ecb, which includes
pull request #3686. This pull request changes a number of metrics
related to UTF-8 support from gauges to counters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants