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

config-reloader: change to non-root user #4235

Merged
merged 7 commits into from
Nov 3, 2023
Merged

Conversation

rootmout
Copy link
Contributor

@rootmout rootmout commented Jun 22, 2023

PR Description

Force the config-reloader container to run as root is not practical in platform that are under rigorous security constraints (ex: Openshift).

Unless I am missing a specific situation, I suggest to setup the security context of this container, the same way the Prometheus Operator does. See prometheus-operator/prometheus-operator config_reloader.go#L256-L262

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

  • CHANGELOG updated
  • Documentation added (was not a part mentioned in doc)
  • Tests updated

@rootmout rootmout requested a review from a team as a code owner June 22, 2023 23:27
@CLAassistant
Copy link

CLAassistant commented Jun 22, 2023

CLA assistant check
All committers have signed the CLA.

operatorContainers := []core_v1.Container{
{
Name: "config-reloader",
Image: imagePathConfigReloader,
VolumeMounts: volumeMounts,
Env: envVars,
SecurityContext: &core_v1.SecurityContext{
RunAsUser: pointer.Int64(0),
Copy link
Contributor

Choose a reason for hiding this comment

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

@rfratto is there a specific reason why this was initially configured to run as root? do you see any problems with this?

Copy link
Member

Choose a reason for hiding this comment

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

Probably just me being aggressive with copy and paste, I think it's safe to remove.

Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

Sorry this took so long to merge. Thanks for the contribution!

@rfratto rfratto merged commit 7fe5023 into grafana:main Nov 3, 2023
8 checks passed
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 21, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants