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: separate nginx and django image tags #11054

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

corang
Copy link

@corang corang commented Oct 12, 2024

Having the tags combined keeps people from being able to use different versions of images. In my case I'm trying to use hardened containers from a government source that doesn't tag the nginx image in line with the django image so I can't deploy the chart with the hardened images.

@github-actions github-actions bot added the helm label Oct 12, 2024
Copy link

dryrunsecurity bot commented Oct 12, 2024

DryRun Security Summary

The pull request focuses on updating the Helm chart for the DefectDojo application, with a particular emphasis on improving the security and reliability of the deployment, including updating container image tags, configuring security-related settings, implementing secure management of sensitive data, improving the reliability of the application deployment, and providing options to enable TLS (HTTPS) for the Nginx component.

Expand for full summary

Summary:

The code changes in this pull request focus on updating the Helm chart for the DefectDojo application, with a particular emphasis on improving the security and reliability of the deployment. The key changes include:

  1. Updating the container image tags to use specific versions instead of the latest tag, which helps ensure the application is running a known, tested version of the components.
  2. Configuring security-related settings, such as security contexts, network policies, and volume mounts, to enhance the overall security posture of the application.
  3. Implementing secure management of sensitive data, such as database passwords and secret keys, by using Kubernetes secrets.
  4. Improving the reliability of the application deployment through the use of liveness and readiness probes, as well as database migration checks.
  5. Providing options to enable TLS (HTTPS) for the Nginx component, ensuring secure communication between the application and its clients.

These changes are generally positive from an application security perspective and demonstrate a focus on improving the security and reliability of the DefectDojo application deployment. While the code changes themselves do not introduce any obvious security concerns, it is essential to review the entire deployment configuration and the application's codebase to ensure there are no vulnerabilities that could be exploited.

Files Changed:

  1. helm/defectdojo/templates/celery-beat-deployment.yaml: The changes update the Celery container image tag to use the same tag as the Django container, which helps ensure version consistency.
  2. .github/workflows/release-x-manual-helm-chart.yml: The workflow responsible for releasing new versions of the DefectDojo Helm chart follows best practices for managing the release process, including versioning, packaging, and repository management.
  3. helm/defectdojo/templates/celery-worker-deployment.yaml: The changes update the Celery worker container image tag, and the deployment manifest includes several security-related configurations, such as secrets management, security context, and volume mounts.
  4. helm/defectdojo/templates/django-deployment.yaml: The changes update the image tags for the Django and Nginx components, and the deployment manifest includes various security-related configurations, such as security context, liveness and readiness probes, environment variables, and TLS configuration.
  5. helm/defectdojo/values.yaml: The changes update the image tags for the Django and Nginx components, and the Helm chart includes several security-related configurations, such as security context, network policies, and volume mounts.

Code Analysis

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

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

@kiblik
Copy link
Contributor

kiblik commented Oct 12, 2024

This tag is updated during the preparation of the helm chart.

yq -i '.tag="${{ github.event.inputs.release_number }}"' helm/defectdojo/values.yaml

This space will need to be adjusted accordingly as well.

@mtesauro
Copy link
Contributor

@corang We don't accept PRs against master. Please make this PR against the dev branch.

@corang corang changed the base branch from master to dev October 16, 2024 03:10
@corang
Copy link
Author

corang commented Oct 16, 2024

@mtesauro Fixed!
@kiblik Thanks for the heads up, think I changed it to what's appropriate

@kiblik
Copy link
Contributor

kiblik commented Oct 16, 2024

Thank you @corang,
"Unfortunately", I just noticed some other things.

  1. In the past, we have been consolidating naming conversation in the HELM chart based on official recommendations. I would like to ask you to follow it (skip -, use camelCase)
  2. .tag is used on much more places (celery-worker, celery-beat and maybe more?). Please, can you take a real deep dive into it?
  3. Similar to 1., this is change which redefines variables which somebody might use somewhere (redefine based on own needs). So this change needs to be announced in the Release notes.
  4. (Maybe it is bigger than this PR) If we are changing the way how tags are used in the HELM, it might be a good idea to start using a more common convention for it. I would go with adding bitnami/common as a dependency and use the "ImageRoot" syntax for it (similar to this and this). If would be helpful also for tools like Renovate.

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.

3 participants