-
Notifications
You must be signed in to change notification settings - Fork 21
CLOUDP-347194 - enable Pod Security Admission at restricted
level
#473
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
base: master
Are you sure you want to change the base?
Changes from all commits
2f9b4ee
158c53c
32c2593
99af9b3
697917e
195097a
65e9742
2d2bb43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
--- | ||
kind: fix | ||
date: 2025-09-24 | ||
--- | ||
|
||
* To follow the [Pod Security Standards](https://v1-32.docs.kubernetes.io/docs/concepts/security/pod-security-standards/) more secure default pod `securityContext` settings were added. | ||
Operator deployment `securityContext` settings that have changed: | ||
- `allowPrivilegeEscalation: false` | ||
- `capabilities.drop: [ ALL ]` | ||
- `seccompProfile.type: RuntimeDefault` | ||
|
||
Other workloads: | ||
- `capabilities.drop: [ ALL ]` - container level | ||
- `seccompProfile.type: RuntimeDefault` - pod level | ||
|
||
> **Note**: If you require less restrictive `securityContext` settings please use `template` or `podTemplate` overrides. | ||
> Detailed information about overrides can be found in [Modify Ops Manager or MongoDB Kubernetes Resource Containers](https://www.mongodb.com/docs/kubernetes/current/tutorial/modify-resource-image/). | ||
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -45,6 +45,9 @@ func Test_buildDatabaseInitContainer(t *testing.T) { | |||||||||||||||||||||||
SecurityContext: &corev1.SecurityContext{ | ||||||||||||||||||||||||
ReadOnlyRootFilesystem: ptr.To(true), | ||||||||||||||||||||||||
AllowPrivilegeEscalation: ptr.To(false), | ||||||||||||||||||||||||
Capabilities: &corev1.Capabilities{ | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there any potential that adding this default will break any customer's workload (or rather prevent the operator from deploying or the workload sts from restarting) and will require some manual intervention? Just thinking about our semver guarantees. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is our own deployment, that we manage. If the customer wants a managedSecurityContext they are allowed to, but otherwise we should be able to modify the one we provide. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is our defaults. Only problem I see with this is some customers now requiring explicitly setting capabilities. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this should be consider as a security fix? In that case we should be able to overwrite our defaults if they are not secure even if this forces customers to explicitly specify custom capabilities. What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still, do we force customers (which don't care about it) to do any manual fix when upgrading? If yes, we need to bump major. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand what will we break here. We are changing our default SecurityContext for operator and other pods created. If customer wants to have dedicated SecurityContext or PodSecurityContext they need to set Code that handles securityContext settings: mongodb-kubernetes/mongodb-community-operator/pkg/kube/podtemplatespec/podspec_template.go Lines 312 to 322 in 917723b
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have discussed with @lsierant that the change for the more strict Capabilities is only applied on the db/om containers, not the whole Pod. This will not affect other containers in the Pod i.e. security, istio sidecars that customer can have. The only change on the Pod level is adding
@mircea-cosbuc looking for guidance here how to proceed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's best to set it at pod level. I think based on @lsierant this needs clarity on what customers might need to change on upgrade (if anything), outlining those scenarios and deciding if it's a breaking change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've checked what are the consequences for using Based on what I have found in official Kubernetes docs:
Additionally on Red Hat OpenShift Container Platform To summarise it is unlikely that users of our Operator require more syscalls permissions in MongoDB workloads than what is allowed by @lsierant @mircea-cosbuc let me know if that justifies approving PR. I have already edited changelog. |
||||||||||||||||||||||||
Drop: []corev1.Capability{"ALL"}, | ||||||||||||||||||||||||
}, | ||||||||||||||||||||||||
}, | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
assert.Equal(t, expectedContainer, container) | ||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!