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

Make it possible to run jmx init container as non root in common chart #667

Merged
merged 3 commits into from
Sep 18, 2023

Conversation

bianchi2
Copy link
Collaborator

@bianchi2 bianchi2 commented Sep 14, 2023

Addresses #666

When runAsRoot is forbidden in the cluster, the pod can't be scheduled. It does make sense to be able to disable securityContent for the init container, as well as let users define their own securityContext (can't think of cases when container securityContext will differ from the one set in pod spec though).

This PR brings changes to the common chart which defines jmx init container. Once merged, a new version will be released, and a new PR to helm charts will be raised with the 2nd portion of changes.

Checklist

  • I have added unit tests
  • I have applied the change to all applicable products
  • The E2E test has passed (use e2e label)

@@ -30,8 +30,15 @@ Jmx init container
image: {{ .Values.monitoring.jmxExporterImageRepo}}:{{ .Values.monitoring.jmxExporterImageTag}}
command: ["cp"]
args: ["/opt/bitnami/jmx-exporter/jmx_prometheus_javaagent.jar", "{{ .Values.volumes.sharedHome.mountPath }}"]
{{- if .Values.monitoring.jmxExporterInitContainer.runAsRoot }}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe I am just blind but I can't see the monitoring.jmxExporterInitContainer.runAsRoot and monitoring.jmxExporterInitContainer.customSecurityContext stanza in any of the values.yaml files. Was this just omitted in the PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@nanux the idea is to release common first and then raise a PR with changes to helm charts + unit tests

Copy link
Member

Choose a reason for hiding this comment

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

I see, that does make sense. Might be just good to indicate in PR description.

@bianchi2 bianchi2 changed the title Make it possible to run jmx init container as non root Make it possible to run jmx init container as non root in common chart Sep 17, 2023
@bianchi2 bianchi2 merged commit 349275d into main Sep 18, 2023
2 checks passed
@bianchi2 bianchi2 deleted the jmx-init-container-security-context branch September 18, 2023 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants