-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(helm): Allow to keep initializer if requested #11257
Conversation
DryRun Security SummaryThe pull request updates the Helm chart for the DefectDojo vulnerability management tool, focusing on changes to the initialization job, which include security enhancements such as setting a time-to-live for finished jobs, allowing the specification of additional volumes for custom configuration or sensitive data, and securely connecting to the database using the Cloud SQL Proxy. Expand for full summarySummary: The code changes in this pull request are focused on updates to the Helm chart used to deploy the DefectDojo application, a popular open-source vulnerability management tool. The changes primarily involve modifications to the initialization job, which is responsible for performing initial setup or configuration tasks for the application. From an application security perspective, the changes do not directly introduce any obvious security vulnerabilities. However, it's important to ensure that the initialization job and its associated resources are properly secured and configured to mitigate potential security risks. This includes verifying the security context of the job and pod, reviewing the initialization scripts for any security issues, and monitoring the job's execution and logs for suspicious activities. Additionally, the changes introduce features that can improve the overall security of the deployment, such as setting a time-to-live (TTL) for finished jobs, allowing the specification of additional volumes for custom configuration or sensitive data, and securely connecting to the database using the Cloud SQL Proxy. These are all positive security enhancements that should be reviewed and validated to ensure they are properly implemented and configured. Files Changed:
Code AnalysisWe ran Riskiness🟢 Risk threshold not exceeded. |
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.
Approved
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.
@kiblik I am not very familiar with helm, but what would be the use case of keeping the init container around longer?
It is not connected to HELM itself. Quite often I see cases when migration was not executed during the upgrade or it fails for some reason. Keeping a container (hopefully with logs) should help with debugging - in case logs are needed as evidence. Yes, log collection to some other place is highly recommended in best practice but I suppose this is easier for users who miss this kind of solution. As far as I know, finished jobs do not consume additional resources (CPU, memory). Plus, right now user is forced to accept the setting (that the job will be removed). Yes, values can be adjusted to much higher but it is not possible to avoid removal (unless you set it to unreasonable value). This PR does not change the previous default behavior. |
Ahh that makes perfect sense |
If the user needed to keep the
initializer
in the stack, there was no way (only set an unreasonable large number).Now, any value that is not a positive integer will keep the
initializer
there.Tested with:
--set 'initializer.keepSeconds=[]'
- stayed--set 'initializer.keepSeconds=null'
- stayed--set 'initializer.keepSeconds=x'
- stayed--set 'initializer.keepSeconds=-1'
- stayed--set 'initializer.keepSeconds=0'
- stayed--set 'initializer.keepSeconds=1'
- removed after 1sContext: https://owasp.slack.com/archives/C2P5BA8MN/p1731520213600479?thread_ts=1724681809.122489&cid=C2P5BA8MN