-
Notifications
You must be signed in to change notification settings - Fork 15
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
chore: enforce pod security standards #895
Conversation
e3cda96
to
90e8b1e
Compare
/e2e --tests sdk |
It seems to miss a permission for accessing substra namespace
|
The |
90e8b1e
to
0f2e2c1
Compare
c4971f1
to
9564fe3
Compare
460b854
to
2dfbea3
Compare
/e2e --tests=sdk,doc |
/e2e --tests=sdk,doc |
End to end tests: ✔️ SUCCESS Aw yeah! |
2dfbea3
to
95e6542
Compare
/e2e --tests=sdk,doc |
End to end tests: ✔️ SUCCESS |
podSecurityContext: | ||
runAsNonRoot: true | ||
seccompProfile: | ||
type: RuntimeDefault | ||
containerSecurityContext: | ||
allowPrivilegeEscalation: false | ||
capabilities: | ||
drop: | ||
- ALL |
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.
Was it not working there?
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.
Maybe related to the chart version update?
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.
Indeed, this has been lost in the various trials / rebase
The new containerSecurityContext
is more restrictive, and since it's the only container in the pod, the podSecurityContext
should not matter, but I'll add it again to be explicit.
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.
Are we?
- Testing the charts in a
alpha
version before merging (once dev is available) - Merging then testing the charts + code in
alpha
on dev
I would recommend going for (1) as the code update is very minor.
95e6542
to
c5bedc3
Compare
/e2e --tests=sdk,doc |
End to end tests: ✔️ SUCCESS Congratulations! |
I would also be in favour of 1 (and the whole thing will be tested at the next release anyway) |
c5bedc3
to
34ce564
Compare
Signed-off-by: SdgJlbl <[email protected]>
Signed-off-by: SdgJlbl <[email protected]>
Signed-off-by: SdgJlbl <[email protected]>
34ce564
to
327ddb0
Compare
Fixes FL-1453
Description
This PR introduces the following changes to the security context:
spec.containers[*].securityContext.allowPrivilegeEscalation=false
spec.containers[*].securityContext.runAsNonRoot=true
spec.containers[*].securityContext.seccompProfile.type=RuntimeDefault
spec.containers[*].securityContext.capabilities.drop=["ALL"]
The exceptions are :
add-cert
initContainer in several pods (if the local CA feature is used),wait-registry
initContainer andkaniko
initContainer in the registry-prepopulate pod.spec.securityContext.runAsNonRoot=true
spec.securityContext.seccompProfile.type= RuntimeDefault
The exceptions are the kaniko pods spawned by the builder.
How to test:
Deploy locally (with skaffold), and then apply labels:
or
The baseline profile should yield no warning.
The restricted profile will show a few warnings, we are trying to remove.