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

Remove makemigrations from scripts #11638

Merged
merged 3 commits into from
Jan 29, 2025

Conversation

valentijnscholten
Copy link
Member

@valentijnscholten valentijnscholten commented Jan 24, 2025

Description
Historically the initializer and other scripts automatically made new migration files for any changes made to the database model. This is practical for (first time) users making modifications to the Defect Dojo codebase.

But when these migrations end up in production or even staging environments, it may prevent these user from ever being able to upgrade to new upstream releases.

It's possible to merge custom migration with upstream migrations and keep upgrading to newer upstream Defect Dojo releases. But this requires a bit of Django knowledge, which users might not realize until they're stuck in production with an out of sync data model.

This PR (suggests to) remove(s) the makemigrations step and instead warns the users if the datamodel was changed without proper/versioned migrations. This makes users aware of the risks and make a more conscious decision about having custom migrations which could diverge from official Defect Dojo releases.

The PR also adds a bit of documentation to explain how to generate new migrations.

Test results
It just works ;-)

Documentation

Please update any documentation when needed in the documentation folder)

Checklist

This checklist is for your information.

  • Make sure to rebase your PR against the very latest dev.
  • Features/Changes should be submitted against the dev.
  • Bugfixes should be submitted against the bugfix branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is flake8 compliant.
  • Your code is python 3.11 compliant.
  • If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

Extra information

Possibly we could go one step further and let the defect dojo startup script fail if there are changes detected in the datamodel that have no migrations.

@valentijnscholten valentijnscholten changed the title Remove makemigrations Remove makemigrations from scripts Jan 24, 2025
Copy link

DryRun Security Summary

The pull request focuses on enhancing the Defect Dojo project's security, maintainability, and development processes through improvements in database migrations, password generation, API schema validation, and comprehensive testing.

Expand for full summary

Summary:

The code changes in this pull request cover various aspects of the Defect Dojo project, with a strong focus on improving the application's security, maintainability, and development processes. The changes include updates to the contribution guidelines, enhancements to the Docker-based application initialization and testing setup, and improvements to the overall application integrity.

From an application security perspective, the key highlights are:

  1. Database Migrations: The changes emphasize the importance of creating database migrations when making changes to the data model, which helps maintain the integrity of the database schema and prevent potential issues during deployment or upgrades.

  2. Random Password and Webhook Secret Generation: The Docker entrypoint script generates random, strong passwords for the initial admin user and a random UUID for the JIRA webhook secret, improving the overall security posture of the application.

  3. API Schema Validation: The unit testing setup includes checks to validate the Django REST Framework (DRF) API schema, ensuring that the API is properly documented and secure.

  4. Comprehensive Testing: The changes focus on improving the reliability and stability of the unit test suite, which is crucial for maintaining the overall security and integrity of the application.

Files Changed:

  1. readme-docs/CONTRIBUTING.md: The changes provide guidance on database migrations and emphasize the need for Python 3.11 compliance, improving the project's maintainability and long-term stability.

  2. docker/entrypoint-initializer.sh: The changes include checks for existing admin users, random password and webhook secret generation, and database migration validation, all of which contribute to the application's overall security.

  3. docker/entrypoint-unit-tests.sh: The changes ensure the proper loading of secret environment variables, correct database configuration, and validation of the OpenAPI schema and database migrations, enhancing the security and reliability of the unit testing setup.

  4. docker/entrypoint-unit-tests-devDocker.sh: The changes focus on maintaining the integrity of the database schema, validating the API schema, and running comprehensive unit tests, which are crucial for the security and reliability of the application.

Overall, the code changes in this pull request demonstrate a strong commitment to improving the security, maintainability, and development processes of the Defect Dojo project, which is a positive step towards ensuring the long-term stability and security of the application.

Code Analysis

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

View PR in the DryRun Dashboard.

@valentijnscholten valentijnscholten marked this pull request as draft January 24, 2025 18:27
@Maffooch
Copy link
Contributor

This is a good move. I'll add an approval once you're happy with the PR and take it out of draft 😄

@valentijnscholten valentijnscholten marked this pull request as ready for review January 24, 2025 19:53
@valentijnscholten
Copy link
Member Author

Ah, forgot to undraft it 😀

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

@Maffooch Maffooch merged commit 80a29ce into DefectDojo:dev Jan 29, 2025
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants