Add securityContext to managed containers

[ArthurSens]

Description

Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request.
If it fixes a bug or resolves a feature request, be sure to link to that issue.

Following what has been done in prometheus-operator/kube-prometheus#1610, prometheus-operator/kube-prometheus#1600 and prometheus-operator/kube-prometheus#1593. This PR also sets the same securityContext to all managed containers (I believe I haven't missed any 😅)

Prometheus statefulset still doesn't have readonlyRootFilesystem: true, because of #4562

Type of change

What type of changes does your code introduce to the Prometheus operator? Put an x in the box that apply.

• CHANGE (fix or feature that would cause existing functionality to not work as expected)
• FEATURE (non-breaking change which adds functionality)
• BUGFIX (non-breaking change which fixes an issue)
• ENHANCEMENT (non-breaking change which improves existing functionality)
• NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Changelog entry

Please put a one-line changelog entry below. This will be copied to the changelog file during the release process.

Improve security with container Security Context

[simonpasquier]

I think you missed Thanos Ruler? I'll try to run it against the OpenShift CI to see if it triggers any issue there.

[simonpasquier]

openshift#156 fyi

[philipgough]

cc @ArthurSens - PTAL - openshift#156 (comment)

The reason the test is failing in OpenShift is because Prometheus can be configured to write out its query log to disk, stdout etc

This is the failing test - https://github.com/openshift/cluster-monitoring-operator/blob/5efe38173c590eb0dca89ff5934d2c81be2ec53d/test/e2e/config_test.go#L154 when the test writes the logs to disk.

I assume since other logs are being output we could successfully configure it to write to /dev/stdout without hitting this issue but perhaps we need to think about how to handle it. It is an edge case.

[simonpasquier]

It looks like the OpenShift e2e tests reveal an interesting edge case :)
In one of the tests, we configure the Prometheus object with spec.queryLogFile: /tmp/test.log which fails now that the root FS is read-only. I would argue that we should add an emptyDir volume mounted under /tmp but it might hit other users too.
I'm wondering if there are other ways that could trigger a similar problem. At least we need to emphasize the change and document it carefully. WDYT?

[ArthurSens]

I nice catch by openshift tests!

I would argue that we should add an emptyDir volume mounted under /tmp but it might hit other users too.

I'm not sure I understand how adding an emptyDir could affect other users, I believe this should be the way to move forward here 🤔

[simonpasquier]

sorry I wasn't clear. I meant that we could solve it in OpenShift with the monitoring operator (managing the Prometheus object) adding an emptyDir volume. But regardless of the OpenShift case, upstream users that specify queryLogFile today might be affected once/if we merge this PR.

[ArthurSens]

Would it be solved if I add an emptyDir volume to this PR?

[philipgough]

Would it be solved if I add an emptyDir volume to this PR?

I am not sure it would, since that would depend on where they write the log and where we would mount the dir. IIUC this could be a breaking change in that respect.

[ArthurSens]

Alright yeah makes sense

I'll remove the readonlyRootFilesystem from this PR and file an issue to remind us to tackle this later

[ArthurSens]

Issue created: #4562

[simonpasquier]

LGTM, maybe add a FIXME comment to the Prometheus statefulset's SecurityContext referring to #4562 so that we don't forget in the future and add it by accident?

[ArthurSens]

Trying to fix the Query-log-file issue in #4566, but I believe this one needs to be merged first. I'll leave the other one as a draft meanwhile

[philipgough]

lgtm

Yes, we can merge this first and follow up at that point.