Fixes #655 ansible mods#656
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis change introduces a comprehensive Ansible-based deployment setup for a Django project. It adds configuration templates for systemd, Nginx (HTTP and HTTPS), environment variables, and deployment/update scripts. The Ansible playbook automates OS bootstrapping, application deployment, and server/service hardening, supporting dynamic environment and integration settings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DeployScript as deploy.sh
participant Ansible
participant TargetHost
User->>DeployScript: Run deploy.sh with VPS details
DeployScript->>Ansible: Execute ansible-playbook playbook.yml
Ansible->>TargetHost: 1️⃣ Bootstrap OS (install packages, reboot, setup swap)
Ansible->>TargetHost: 2️⃣ Deploy application (clone repo, setup venv, env, migrate, collectstatic)
Ansible->>TargetHost: 3️⃣ Harden & run (template Nginx, systemd, open firewall, start services)
TargetHost-->>Ansible: Reports status
Ansible-->>DeployScript: Playbook complete
DeployScript-->>User: Deployment finished
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @ishaan-arora-1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request delivers a robust, automated deployment solution for Django applications using Ansible. The primary goal is to streamline the setup process for a Django web application, particularly for 'classroom deployment' scenarios, by providing a complete playbook that handles server provisioning, application setup, and critical configurations like WebSocket support via Nginx and Uvicorn. It significantly reduces manual intervention and ensures a consistent deployment environment.
Highlights
- Comprehensive Ansible Playbook: This pull request introduces a complete Ansible playbook (
playbook.yml) designed for end-to-end deployment of a Django application. It handles OS bootstrapping (package installation, swapfile creation), application deployment (git cloning, virtual environment setup, dependency installation, Django migrations, static file collection), and server configuration. - Django ASGI/WebSocket Support: The deployment now fully supports Django applications using ASGI (Asynchronous Server Gateway Interface) for features like WebSockets. This is achieved by configuring Gunicorn to use Uvicorn workers via a new systemd service (
education-website.service.j2) and setting up Nginx (nginx-http.conf.j2) with appropriate proxy headers (Upgrade,Connection: upgrade) to correctly handle WebSocket traffic. - Automated Environment Management: The playbook includes logic to read environment variables from a local
.envfile and dynamically generate the server-side.envfile (env.j2) based on these variables and Ansible defaults. This streamlines configuration management for various deployment environments, including sensitive data like API keys and database credentials. - Deployment Utility Scripts: New utility scripts are added:
deploy.shsimplifies running the Ansible playbook by handling VPS connection details and password prompting, whileupdate.sh.j2provides a templated script for quick post-deployment updates (git pull, pip install, migrate, collectstatic, service restart). - Nginx and Systemd Configuration: Dedicated Jinja2 templates are provided for Nginx (
nginx-http.conf.j2) and systemd service files (education-website.service.j2), ensuring consistent and automated server setup. The Nginx configuration includes serving static/media files and proxying to the Django application, while the systemd service manages the Gunicorn/Uvicorn process.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of Ansible files for deploying a Django application, including a playbook, configuration templates, and deployment scripts. The setup covers bootstrapping the server, deploying the application code, and configuring Nginx, systemd, and other services.
My review has identified several critical security and correctness issues that should be addressed before merging. These include insecure handling of passwords, missing environment variables in the application service, and weak default credentials. I've also provided suggestions to improve the robustness and maintainability of the Nginx configuration and Ansible playbook logic. Addressing these points will significantly improve the security and reliability of the deployment process.
| if [[ -n "$VPS_PASSWORD" ]]; then | ||
| ANSIBLE_CMD+=(--extra-vars "ansible_password=${VPS_PASSWORD}") | ||
| else | ||
| ANSIBLE_CMD+=(--ask-pass) | ||
| fi |
There was a problem hiding this comment.
Passing a password on the command line via --extra-vars is a major security vulnerability. The password will be visible in the system's process list to other users on the same machine.
It's highly recommended to use SSH key-based authentication instead of passwords. If passwords are required, the existing --ask-pass is a much safer alternative as it uses a secure prompt.
| if [[ -n "$VPS_PASSWORD" ]]; then | |
| ANSIBLE_CMD+=(--extra-vars "ansible_password=${VPS_PASSWORD}") | |
| else | |
| ANSIBLE_CMD+=(--ask-pass) | |
| fi | |
| ANSIBLE_CMD+=(--ask-pass) |
| User={{ vps_user }} | ||
| Group={{ vps_user }} | ||
| WorkingDirectory=/home/{{ vps_user }}/{{ project_name }} | ||
| Environment="PATH=/home/{{ vps_user }}/{{ project_name }}/venv/bin" |
There was a problem hiding this comment.
The Django application requires environment variables from the .env file to function correctly (e.g., SECRET_KEY, database settings). Systemd does not load this file by default, which will likely cause the application to fail at startup.
You should use the EnvironmentFile directive to instruct systemd to load variables from your project's .env file.
Environment="PATH=/home/{{ vps_user }}/{{ project_name }}/venv/bin"
EnvironmentFile=/home/{{ vps_user }}/{{ project_name }}/.env
|
|
||
| - name: Map essential env vars to lowercase variables expected by templates | ||
| set_fact: | ||
| secret_key: "{{ env_vars.SECRET_KEY }}" |
There was a problem hiding this comment.
The SECRET_KEY is essential for Django's security. If it's missing from the .env file, this task will set it to an empty string, which will cause the application to crash at runtime. It's much safer to make this variable required so that the playbook fails immediately with a clear error message if it's not set.
secret_key: "{{ env_vars.SECRET_KEY | mandatory }}"
| vps_user: "{{ env_vars.VPS_USER | default('django') }}" | ||
| db_name: "{{ env_vars.DB_NAME | default('education_website') }}" | ||
| db_user: "{{ env_vars.DB_USER | default('root') }}" | ||
| db_password: "{{ env_vars.DB_PASSWORD | default('change_me') }}" |
There was a problem hiding this comment.
Using a weak default password like 'change_me' is a major security vulnerability. If DB_PASSWORD is not provided in the .env file, the application will be deployed with a publicly known, insecure password.
To prevent this, the playbook should fail if a required secret is missing. You can achieve this by using the mandatory filter, which will stop execution and alert the user that the variable is required.
db_password: "{{ env_vars.DB_PASSWORD | mandatory }}"
| proxy_http_version 1.1; | ||
| proxy_set_header Upgrade $http_upgrade; | ||
| proxy_set_header Connection "upgrade"; | ||
| proxy_set_header Host {{ domain_name | default('alphaonelabs.com') }}; |
There was a problem hiding this comment.
Hardcoding the Host header with {{ domain_name | default('...') }} is not robust. It can lead to incorrect behavior if the server is accessed via other valid hostnames (e.g., www subdomains) that are also configured in server_name.
Using the Nginx variable $host is the standard and more flexible approach, as it dynamically passes the hostname from the original request. This is already done correctly in other parts of the configuration (e.g., line 68).
The same issue is present on line 131.
proxy_set_header Host $host;
| tasks: | ||
| - name: Configure nginx | ||
| template: | ||
| src: nginx-http.conf.j2 |
There was a problem hiding this comment.
This task is hardcoded to use nginx-http.conf.j2, which sets up a basic HTTP-only server. There is a much more feature-rich template, nginx.conf.j2, available that includes logic for HTTPS, redirects, and special handling for SSE.
To enable these production-ready features, you should use nginx.conf.j2 here. Its internal logic can already handle cases where HTTPS is not enabled, making nginx-http.conf.j2 redundant.
src: nginx.conf.j2
| WorkingDirectory=/home/{{ vps_user }}/{{ project_name }} | ||
| Environment="PATH=/home/{{ vps_user }}/{{ project_name }}/venv/bin" | ||
| ExecStart=/home/{{ vps_user }}/{{ project_name }}/venv/bin/gunicorn \ | ||
| web.asgi:application -w 4 -k uvicorn.workers.UvicornWorker \ |
There was a problem hiding this comment.
The number of gunicorn workers is hardcoded to 4. This may not be optimal for different server specifications. It's better to make this configurable via an Ansible variable to allow for easier tuning.
A common formula for the number of workers is (2 * number_of_cpu_cores) + 1.
web.asgi:application -w {{ gunicorn_workers | default(4) }} -k uvicorn.workers.UvicornWorker \
| python3.11 -m venv venv | ||
| source venv/bin/activate | ||
| pip install --upgrade pip wheel | ||
| pip install -r requirements.txt uvicorn gunicorn psycopg2-binary |
There was a problem hiding this comment.
For better dependency management and reproducibility, all Python packages should be listed in requirements.txt. Currently, uvicorn, gunicorn, and psycopg2-binary are installed directly via this shell command.
Please add these packages to your requirements.txt file and remove them from this pip install command. This ensures you have a single, reliable source of truth for your project's dependencies.
pip install -r requirements.txt
| @@ -0,0 +1,8 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
It's good practice to add set -e to the beginning of your shell scripts. This ensures that the script will exit immediately if any command fails, which prevents potentially harmful subsequent commands from running. For example, you wouldn't want to restart the application if the pip install or migrate steps failed.
#!/bin/bash
set -e
| pip install -r requirements.txt | ||
| python manage.py migrate | ||
| python manage.py collectstatic --noinput | ||
| sudo systemctl restart {{ project_name }} |
There was a problem hiding this comment.
The systemd service file is named education-website.service, but this script uses {{ project_name }} to restart it. While the default value for project_name matches, this will fail if PROJECT_NAME is ever customized in the .env file, as the service file name will not change with it.
For consistency, you should either hardcode the service name here to match the service file, or better yet, parameterize the service file name in the Ansible playbook (e.g., {{ project_name }}.service.j2) so it always matches.
sudo systemctl restart education-website
1 participant
Fixes #655 ansible mods for django websockets and classroom deployment
Summary by CodeRabbit