Skip to content
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

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

kiblik
Copy link
Contributor

@kiblik kiblik commented Nov 13, 2024

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 1s

Context: https://owasp.slack.com/archives/C2P5BA8MN/p1731520213600479?thread_ts=1724681809.122489&cid=C2P5BA8MN

@github-actions github-actions bot added the helm label Nov 13, 2024
Copy link

DryRun Security Summary

The 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 summary

Summary:

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:

  1. helm/defectdojo/values.yaml:

    • The changes update the keepSeconds value in the initializer section, which controls the duration for which the initializer job and pod will be kept deployed after they have completed their tasks. This is a deployment-related setting that does not have a direct impact on the application's security posture, but it's important to ensure that the job and pod are properly secured.
  2. helm/defectdojo/templates/initializer-job.yaml:

    • The changes introduce a conditional check to set the ttlSecondsAfterFinished field for the initialization job, which can help to automatically clean up finished jobs, a good security practice.
    • The code allows for the specification of additional volumes that can be mounted into the initialization job's containers, which can be useful for providing custom configuration files or sensitive data, but it's important to ensure that these volumes are properly secured.
    • The initialization job includes a container that waits for the database to be available before running the initialization script, and if the application is configured to use Google Cloud SQL, it includes a container that runs the Cloud SQL Proxy to securely connect to the database.
    • The initialization job uses environment variables to configure the application, including the database connection details, which should be properly secured to prevent sensitive information exposure.

Code Analysis

We ran 9 analyzers against 2 files and 0 analyzers had findings. 9 analyzers had no findings.

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

Copy link
Contributor

@Maffooch Maffooch left a 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?

@kiblik
Copy link
Contributor Author

kiblik commented Nov 14, 2024

@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.

@Maffooch
Copy link
Contributor

Ahh that makes perfect sense

@mtesauro mtesauro merged commit cdf7b82 into DefectDojo:bugfix Nov 15, 2024
72 checks passed
@kiblik kiblik deleted the helm_keep_init branch November 15, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants