Dockerfile: merge nginx into django

[fopina]

#11181 is a "lighter" simplification but I believe this one is much better

By merging nginx as a target of Dockerfile.django:

• No risk of the build stage going out of sync (and wasting precious cache)
• We can actually re-use the django stage for collectstatic, saving one stage

I've only updated the dockerfiles and docker-compose. No docs or any mentions of the files were updated. I can do so, if this PR gets a 👍

[dryrunsecurity]

DryRun Security Summary

The pull request focuses on improving the build process and deployment of the DefectDojo application, a web-based open-source vulnerability management tool, by updating the Dockerfile configurations, the Docker Compose file, and the Debian-based Dockerfile, while ensuring ongoing security through dependency version management, environment variable security, static file handling, and Nginx configuration review.

Expand for full summary

Summary:

The code changes in this pull request are focused on improving the build process and deployment of the DefectDojo application, a web-based open-source vulnerability management tool. The changes include updates to the Dockerfile configurations, the Docker Compose file, and the Debian-based Dockerfile.

From an application security perspective, the changes do not appear to introduce any obvious security concerns. However, there are a few areas that should be reviewed and monitored to ensure the ongoing security of the application:

1. Dependency Versions: The Dockerfiles are using specific versions of Python, Node.js, and NGINX. It's important to ensure that these versions are kept up-to-date and that any known security vulnerabilities are addressed in a timely manner.
2. Environment Variables: The Dockerfiles and Docker Compose file set various environment variables that are used to configure the application. It's crucial to ensure that these variables are properly secured and that any sensitive information (e.g., passwords) is not exposed.
3. Static File Handling: The changes related to the "collectstatic" stage in the Dockerfiles ensure that the static files are properly collected and served by the NGINX container. This is an important aspect of web application security, as improper handling of static files can lead to vulnerabilities.
4. Nginx Configuration: The changes to the Nginx configuration files should be reviewed to ensure that the web server is properly secured and configured to mitigate potential security risks.

Overall, the changes in this pull request appear to be focused on improving the build process and deployment of the DefectDojo application, and they do not seem to introduce any obvious security concerns. However, it's always important to review the entire codebase and deployment process to ensure that the application is secure and compliant with best practices.

Files Changed:

1. Dockerfile.alpine:

   • Removal of a comment about the build image code being identical to the Dockerfile.nginx
   • Addition of a new "yarn" stage to install a specific version of Yarn
   • Addition of a new "collectstatic" stage to collect and copy static files
   • Modifications to the "nginx" stage to copy static files, add OpenSSL, and set various environment variables

2. docker-compose.yml:

   • Changes to the Dockerfile references for the nginx and uwsgi services
   • Addition of the DD_DJANGO_METRICS_ENABLED and DD_ALLOWED_HOSTS environment variables
   • Changes to the volume mounts for the defectdojo_media volume

3. Dockerfile.debian:

   • Removal of a redundant comment about the build image code
   • Upgrade to Python 3.11.9-slim-bookworm
   • Addition of a "yarn" stage to install a specific version of Yarn
   • Addition of a "collectstatic" stage to collect and copy static files
   • Updates to the Nginx configuration files and environment variables

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]

I like the idea of simplification.

By the way, different images are not mentioned only in the docker-compose file. They are pushed to docker-hub and used in helm charts. Maybe even more. Can you check other places as well, please?

[fopina]

Hi @kiblik yeah, I did add that note to the description, that many places were missing but I didn’t want to waste the time if the idea of reusing same dockerfile was not welcome.
Can I assume it is then?

[kiblik]

Sorry, I missed last paragraph of description.

I'm not core maintainer of project. My voice might be supportive but I have no formal power over decision.

[dsever]

I don't think we should go to that direction, as you going away from the cloud native principles.

[fopina]

@dsever honestly curious as I'm not familiar with their principles, would you be able to quote or link which principle it goes against?

[mtesauro]

@fopina One reason to keep Nginx as a separate container is that it allows users of DefectDojo to choose other options if they want to front DefectDojo with something else. This could be something like Caddy or maybe a LB from their cloud provider.

Yes, I understand that the static assets are built into the Nginx container and someone who chooses to use something different to front DefectDojo would need to also handle that situation but that would mean they'd only need to:

1. Create a new container (maybe) to replace nginx + the static assets
2. Use all the other upstream containers from DefectDojo without modification

So, if for no other reason, I like the fact that keeping Nginx separate gives the community flexibility to change out Nginx without having to modify the other containers needed to run DefectDojo.

[fopina]

@mtesauro I totally understand that as, for my projects, I don't "bake" static assets into Nginx. I use upstream nginx image with a volume where configuration and assets are placed

That said, there might be a confusion: this PR does NOT add nginx to dojo container.
It adds it to the same Dockerfile, as a separate target.

Resulting containers are the same.

[mtesauro]

@fopina Please see the update I posted earlier today - the project is about to do a fairly significant change/upgrade to the containers for this project. Given that's about to happen, merging two dockerfiles into one doesn't really add much value IMHO.

It does break our existing GHA tests.

I realize we'll have to update those once the new containers/dockerfiles are in place but why do that work twice when this PR doesn't change any real functionality - rather it create a different style of dockerfile.

I'd also prefer to slow roll changes to our containers given the size of our user base. (10+ million pulls). For new hardened container images, we can produce a new image for a period of time to allow people to transition to the new images before we deprecate and eventually stop building the older images.

I appreciate and can understand the desire to potentially optimize 2 dockerfiles into a single file but given the potential side effects, the optimization isn't better than the costs.

[fopina]

Understood and agreed.

Impact (known or unknown) was an expected argument against the change and the reason I didn't fully complete it (updating docs and references).
Also why I mention the other PR I created.

I was just confused by which principle this would be against so I would avoid PRs like that again, but I see now it was a misunderstanding.

Thanks, I'll close the PR

[mtesauro]

@fopina Thanks for being understanding.

FWIW, I'm trying to find time today to do a proper review the other PR you did - #11181 but at a quick glace, that one looks nice. 👍