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(db-checker): Extension of "db reachable" #11651

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

kiblik
Copy link
Contributor

@kiblik kiblik commented Jan 25, 2025

Reopen #10497

Extend wait_for_database_to_be_reachable. Not only for simple operation but check that DB is compatible.

Added based on #10490

This PR also contains changes from #11650 because they are needed for proper testing of this functionality.

@kiblik kiblik force-pushed the db_checker branch 4 times, most recently from 1cf37e0 to 3f36a4e Compare January 26, 2025 22:22
@kiblik kiblik marked this pull request as ready for review January 26, 2025 22:27
@kiblik kiblik requested review from mtesauro, Maffooch and cneill and removed request for mtesauro January 28, 2025 15:40
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

@@ -85,6 +85,7 @@ jobs:
./helm/defectdojo \
--set django.ingress.enabled=true \
--set imagePullPolicy=Never \
--set initializer.keepSeconds="x" \
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this value intentional? Docs suggest this ttlSecondsAfterFinished should be an integer - I'm not sure what it would do with a value of "x"?

ttlSecondsAfterFinished: {{ .Values.initializer.keepSeconds }}

Screenshot 2025-02-03 at 17 50 12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You would be right if the value were passed to ttlSecondsAfterFinished. However thanks to the previous line whole ttlSecondsAfterFinished is dropped if you put their value other than a positive integer. This functionality was added in #11257

{{- if and (int .Values.initializer.keepSeconds) (gt (int .Values.initializer.keepSeconds) 0) }}

I might change it to keep me, 0, -1, ... or anything more descriptive here. I'm open to any recommendation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I should have read up one line! 😂 I think -1 is a pretty common pattern for "ignore this".

Copy link

dryrunsecurity bot commented Feb 4, 2025

DryRun Security Summary

The pull request implements comprehensive security and reliability improvements across DefectDojo's deployment and testing infrastructure, including secure configuration management, enhanced error handling, database connectivity checks, and secure deployment practices through updates to Docker entrypoint scripts, GitHub Actions workflows, and testing infrastructure.

Expand for full summary

Summary:

The code changes in this pull request cover various aspects of the DefectDojo application's deployment and testing infrastructure, with a focus on improving the security and reliability of the application. The changes include updates to the Docker entrypoint scripts, GitHub Actions workflows, and unit test scripts.

Key security-related improvements include:

  1. Secure Configuration Management: The scripts now use the /secret-file-loader.sh script to load sensitive configuration data, such as database credentials and API keys, in a more secure manner.
  2. Error Handling and Reliability: The addition of the set -e command in various scripts ensures that the scripts exit immediately if any command returns a non-zero exit status, improving the overall reliability and error handling.
  3. Database Connectivity Checks: The scripts now include checks to ensure that the database is reachable and able to execute basic SQL commands before proceeding with further operations.
  4. Django Application Checks: The scripts run the Django manage.py check command to validate the application's configuration and identify potential issues before deployment.
  5. Secure Deployment Configuration: The GitHub Actions workflow avoids using hardcoded credentials and instead relies on GitHub secrets, and it uses a local image pull policy to reduce the attack surface.

While the changes generally demonstrate a security-conscious approach, it's important to continue reviewing the application's codebase, dependencies, and overall deployment configuration to ensure the ongoing security and integrity of the DefectDojo application.

Files Changed:

  1. docker/entrypoint-celery-beat.sh: This file has been updated to include the set -e command, source the /secret-file-loader.sh script, and handle the overriding of the local_settings.py file with custom settings files.
  2. docker/entrypoint-celery-worker.sh: The changes in this file include the set -e command, handling of the /app/docker/extra_settings/ directory, and the loading of configuration files using the /secret-file-loader.sh and /reach_database.sh scripts.
  3. docker/entrypoint-initializer.sh: The updates to this file focus on error handling, the overriding of settings.py files, and the handling of various initialization tasks, such as admin user creation and JIRA Webhook Secret generation.
  4. .github/workflows/k8s-tests.yml: The changes in this GitHub Actions workflow file include the configuration of the Helm chart deployment, with a focus on the initializer job lifetime and the use of a local image pull policy.
  5. docker/entrypoint-integration-tests.sh: The updates to this file involve the addition of the set -e command, the secure loading of sensitive information, and the execution of comprehensive integration tests.
  6. docker/entrypoint-nginx.sh: This file has been updated to handle the generation of self-signed TLS certificates, the configuration of Nginx based on environment variables, and the enabling of Nginx metrics with basic authentication.
  7. docker/entrypoint-uwsgi-dev.sh: The changes in this file include the addition of the set -e command, the inclusion of the /reach_database.sh script, and the configuration of the uWSGI server for the development environment.
  8. docker/entrypoint-unit-tests.sh: The updates to this file focus on improving the reliability and security of the unit testing process, including the addition of the set -e command, database connectivity checks, and the execution of the Django manage.py commands.
  9. docker/reach_database.sh: The changes in this file involve the addition of an extra database connectivity test using the Django database connection.
  10. docker/entrypoint-uwsgi.sh: The updates to this file include the addition of the set -e command, the inclusion of the /reach_database.sh script, and the execution of the Django manage.py check command.
  11. docker/unit-tests.sh: The changes in this file involve the addition of the set -e command and the execution of the Django makemigrations and migrate commands, followed by the running of the unit tests.

Code Analysis

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

View PR in the DryRun Dashboard.

@mtesauro mtesauro merged commit e7d3d05 into DefectDojo:dev Feb 5, 2025
73 checks passed
@kiblik kiblik deleted the db_checker branch February 5, 2025 23:34
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