-
Notifications
You must be signed in to change notification settings - Fork 59
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
vdk-pipelines-control-service: don't include needless service token. #1260
vdk-pipelines-control-service: don't include needless service token. #1260
Conversation
Signed-off-by: murphp15 <[email protected]>
That is also very useful and it's a security improvement at no cost. (see https://cheatsheetseries.owasp.org/cheatsheets/Kubernetes_Security_Cheat_Sheet.html - "Do not mount the service account credentials in a container if it does not need to access the Kubernetes API.") So I urge you to finish it and push it. We should do this (preferably separate PR) also for data job started pods. |
I have recommended some talks on a conference I was here - relevant one for this PR is https://youtu.be/jRrCi1IS7gk?t=14313 (Hardening against Kubernetes Hacks) You can see how this vulnerability (among others) is being exploited . It's very fun talk and very educative about how Kubernetes works |
@@ -1234,6 +1234,7 @@ public void createJob( | |||
if (StringUtils.isNotEmpty(serviceAccountName)) { | |||
podSpecBuilder.withServiceAccountName(serviceAccountName); | |||
} | |||
podSpecBuilder.withAutomountServiceAccountToken(false); |
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.
Consider adding this property through the k8s-data-job-template.yaml rather than the source code.
This property has been added to one of our internal deployments without any issues. |
Closing this as this is being handled in #1679 |
Why?
We don't need the token so we shouldn't include it by default
What?
Set the property so the token is not included
How has this been tested?
Integration and unit tests are passing
Signed-off-by: murphp15 [email protected]