Terraform todo app by VitaliySemeniv · Pull Request #47 · mate-academy/devops_todolist_terraform_task

VitaliySemeniv · 2025-11-16T14:04:17Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because the submission is missing the core Terraform infrastructure code, which is the primary requirement of this task. While the Django application code is present, the main goal is to provision the Azure resources needed to deploy it.

Here are the critical issues that need to be addressed:

Missing Terraform Configuration: The project lacks all Terraform (.tf) files. You will need to create the specified directory structure and implement the modules for networking, compute, and storage as described in the task.
Incorrect GitHub Workflow: The workflow in .github/workflows/main.yml is set up for Docker Compose, but it should be validating the Terraform code. Please update it to include steps for terraform init, terraform validate, and terraform plan.
Application Configuration Errors: For the application to run once deployed, you must update todolist/settings.py. The ALLOWED_HOSTS list cannot be empty when DEBUG is False, and the database HOST should be set to 'localhost' to connect to the database on the VM.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-11-16T14:08:15Z

+
+jobs:
+
+  docker-compose:


This workflow job is set up to test the application using Docker Compose, but the main goal of this task is to manage infrastructure with Terraform. To make this workflow more relevant, you should add steps to validate your Terraform configuration. Consider adding steps for terraform init, terraform validate, and terraform plan.

mateacademy-ai-mentor · 2025-11-16T14:08:15Z

+    - uses: actions/checkout@v4
+
+    - uses: hoverkraft-tech/compose-action@v2.0.0
+      name: Compose Up


The indentation for the name key is incorrect. In GitHub Actions workflow syntax, name should be at the same indentation level as uses because they are both properties of a step.

mateacademy-ai-mentor · 2025-11-16T14:08:15Z

+RUN pip install --upgrade pip && \
+    pip install -r requirements.txt
+
+RUN python manage.py migrate


Running database migrations during the image build process is not a good practice. The migrate command requires a live database connection, which is typically not available during the docker build step. This will likely cause the build to fail. Consider running migrations as part of the container's startup command or in a separate script that runs when the application is deployed.

mateacademy-ai-mentor · 2025-11-16T14:08:15Z

+def todolist(request, todolist_id):
+    todolist = get_object_or_404(TodoList, pk=todolist_id)
+    if request.method == "POST":
+        redirect("lists:add_todo", todolist_id=todolist_id)


The redirect() function in Django returns an HttpResponseRedirect object. To actually perform the redirect, you need to return this object from your view. Without the return keyword here, the function will continue executing and will eventually hit the render() function below, which is likely not the intended behavior for a POST request in this view.

mateacademy-ai-mentor · 2025-11-16T14:08:16Z

+# SECURITY WARNING: don't run with debug turned on in production!
+DEBUG = True
+
+ALLOWED_HOSTS = []


When DEBUG is False, an empty ALLOWED_HOSTS list will cause Django to block all incoming requests. To allow access to your deployed application, you need to add the VM's public IP address or DNS name here. For this project, you could use ['*'] to allow all hosts, though using environment variables is a more secure practice for production.

mateacademy-ai-mentor · 2025-11-16T14:08:16Z

+        'NAME': 'app_db',
+        'USER': 'app_user',
+        'PASSWORD': '1234',
+        'HOST': '172.17.0.2',  # You can use a different host if your MySQL server is on a remote machine.


This hardcoded IP address is specific to a local development setup and will not work on the deployed Azure VM. The installation script sets up a local database on the VM, so this host should be changed to '127.0.0.1' or 'localhost'. It's also recommended to manage all database settings (user, password, etc.) through environment variables instead of hardcoding them.

mateacademy-ai-mentor · 2025-11-16T14:08:16Z

+  <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
+  <!-- FONT
+  –––––––––––––––––––––––––––––––––––––––––––––––––– -->
+  <link href='http://fonts.googleapis.com/css?family=Raleway:400,300,600' rel='stylesheet' type='text/css'>


It's a best practice to load external resources like fonts over HTTPS to prevent potential mixed content warnings and improve security. You can simply change http:// to https:// in the URL.

mateacademy-ai-mentor · 2025-11-16T14:08:16Z

+  {% block css %}{% endblock %}
+  <!-- Scripts
+  –––––––––––––––––––––––––––––––––––––––––––––––––– -->
+  <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/2.1.3/jquery.min.js"></script>


To maintain security and avoid mixed content issues, it's recommended to load external scripts like jQuery over HTTPS. Please update the src attribute to use https://.

mateacademy-ai-mentor

Review Summary

Excellent progress on adding the Terraform configuration! However, I am requesting changes because several critical issues are preventing the infrastructure from deploying and the application from running correctly.

Here are the key areas to address:

GitHub Workflow: The workflow in .github/workflows/main.yml is still set up for Docker Compose. It needs to be updated to validate your Terraform code with terraform init, validate, and plan.
Deployment Script: The install-app.sh script will fail because it uses an incorrect path to copy application files and references a todoapp.service file that is missing from the project. Additionally, the VM extension in modules/compute/main.tf hardcodes a repository URL, which needs to be updated to deploy the code from your pull request.
Application Settings: In todolist/settings.py, the ALLOWED_HOSTS list must be updated (e.g., to ['*']) and the database HOST should be set to 'localhost' to work on the VM.
Terraform Configuration: The public IP in modules/network/main.tf needs its allocation_method changed to Dynamic and the domain_name_label argument added. Also, please add the required default values to the variables in the root variables.tf file.

Once these issues are resolved, your deployment should be successful. Keep up the great work!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-11-16T14:37:31Z

+name: Run Docker Compose
+on:
+  pull_request:
+    branches: [ "main" ]
+
+  workflow_dispatch:
+
+jobs:
+
+  docker-compose:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v4
+
+    - uses: hoverkraft-tech/compose-action@v2.0.0
+      name: Compose Up
+


This workflow is configured to run Docker Compose, which doesn't align with the task requirements. The goal is to validate your Terraform code. Please update this file to include steps for terraform init, terraform validate, and terraform plan.

mateacademy-ai-mentor · 2025-11-16T14:37:31Z

+RUN pip install --upgrade pip && \
+    pip install -r requirements.txt
+
+RUN python manage.py migrate


Running database migrations during the image build process is generally not recommended. The database might not be available at build time, and this couples the image to a specific database state. It's better to run migrations as a separate step during application startup or deployment.

mateacademy-ai-mentor · 2025-11-16T14:37:31Z

+EXPOSE 8080
+
+# Run database migrations and start the Django application
+ENTRYPOINT ["python", "manage.py", "runserver", "0.0.0.0:8080"]


The Django development server (runserver) is not suitable for production use due to performance and security limitations. For deploying an application, you should use a production-ready WSGI server like Gunicorn or uWSGI.

mateacademy-ai-mentor · 2025-11-16T14:37:32Z

+def todolist(request, todolist_id):
+    todolist = get_object_or_404(TodoList, pk=todolist_id)
+    if request.method == "POST":
+        redirect("lists:add_todo", todolist_id=todolist_id)


The redirect function returns an HttpResponseRedirect object, but you are not returning it from the view. This line should be return redirect(...). However, it's worth noting that the form in the todolist.html template posts to the add_todo view, so this block of code is currently unreachable.

mateacademy-ai-mentor · 2025-11-16T14:37:32Z

+mkdir /app 
+# make sure to uncomment the line bellow and update the link with your GitHub username
+# git clone https://github.com/<your-gh-username>/azure_task_12_deploy_app_with_vm_extention.git
+cp -r devops_todolist_terraform_task/app/* /app


This command attempts to copy files from devops_todolist_terraform_task/app/, but based on the project structure you've submitted, the Django application files are at the root of the repository, not inside an app directory. This command will likely fail. Please adjust the path to correctly copy the application files.

mateacademy-ai-mentor · 2025-11-16T14:37:32Z

+variable "location" { type = string }
+variable "resource_group_name" { type = string }
+variable "virtual_network_name" { type = string }
+variable "vnet_address_prefix" { type = string }
+variable "subnet_name" { type = string }
+variable "subnet_address_prefix" { type = string }
+variable "network_security_group" { type = string }
+variable "public_ip_name" { type = string }
+variable "dns_label" { type = string } # базова частина, наприклад "matetask"
+
+variable "vm_name" { type = string }
+variable "vm_size" { type = string }
+variable "ssh_key_public" { type = string }


This file is missing the required default values for several variables, and some variable names do not match the task specifications. According to the requirements, you should define variables with the following parameters:

location: default "uksouth"

resource_group_name: default "mate-azure-task-12"

virtual_network_name: default "vnet"

vnet_address_prefix: default "10.0.0.0/16"

subnet_name: default "default"

subnet_address_prefix: default "10.0.0.0/24"

network_security_group_name: default "defaultnsg" (You have network_security_group)

public_ip_address_name: default "linuxboxpip" (You have public_ip_name)

vm_name: default "matebox"

vm_size: default "Standard_B1s"

dns_label: default "matetask"

Please update the variable names and add the default attribute to each of these variable blocks.

mateacademy-ai-mentor · 2025-11-16T14:37:32Z

+# SECURITY WARNING: don't run with debug turned on in production!
+DEBUG = True
+
+ALLOWED_HOSTS = []


When deployed to the Azure VM, the application will be accessed via its public IP or FQDN. An empty ALLOWED_HOSTS list will cause Django to raise a DisallowedHost error and block the request. For this deployment, you should add '*' to this list to allow all hosts. It is also a security best practice to set DEBUG = False for any deployed application.

mateacademy-ai-mentor · 2025-11-16T14:37:33Z

+        'NAME': 'app_db',
+        'USER': 'app_user',
+        'PASSWORD': '1234',
+        'HOST': '172.17.0.2',  # You can use a different host if your MySQL server is on a remote machine.


The database HOST is hardcoded to an IP address (172.17.0.2) that will not be available in the Azure VM. Since the application and database will run on the same machine in this deployment, this should be changed to 'localhost'. Please also ensure your install-app.sh script installs and configures a MySQL server on the VM, as it's required by this configuration.

mateacademy-ai-mentor · 2025-11-16T14:37:33Z

+  <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
+  <!-- FONT
+  –––––––––––––––––––––––––––––––––––––––––––––––––– -->
+  <link href='http://fonts.googleapis.com/css?family=Raleway:400,300,600' rel='stylesheet' type='text/css'>


It's a best practice to load external resources over HTTPS to prevent mixed-content warnings when your application is served over a secure connection. Please change http:// to https:// for this font import.

mateacademy-ai-mentor · 2025-11-16T14:37:33Z

+  {% block css %}{% endblock %}
+  <!-- Scripts
+  –––––––––––––––––––––––––––––––––––––––––––––––––– -->
+  <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/2.1.3/jquery.min.js"></script>


Similar to the font import, this script should be loaded over HTTPS to ensure your site remains secure and avoids mixed-content issues. Please update the URL to use https://.

mateacademy-ai-mentor

Review Summary

Great job on setting up the Terraform modules and creating the validation workflow! I'm approving this pull request as an educational override to help you progress, but there are several critical issues that need your attention for the deployment to succeed.

Please focus on these key areas:

GitHub Workflow: Your new Terraform workflow is correct, but it's in the wrong directory. Move it from mate_academy/terraform/terraform_todo_app/.github/workflows/ to the root .github/workflows/ and replace the old Docker workflow.
Deployment Script: The install-app.sh script has incorrect paths and is missing steps like installing Python dependencies and a systemd service file.
Application Settings: In todolist/settings.py, ALLOWED_HOSTS must be set to ['*'] and the database HOST should be 'localhost'.
Terraform Configuration: The public IP in mate_academy/terraform/terraform_todo_app/modules/network/main.tf needs its allocation_method set to Dynamic and the domain_name_label added.

Addressing these points will get your infrastructure and application running correctly. Keep up the great work!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2025-11-16T20:31:46Z

+name: Run Docker Compose
+on:
+  pull_request:
+    branches: [ "main" ]
+
+  workflow_dispatch:
+
+jobs:
+
+  docker-compose:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v4
+
+    - uses: hoverkraft-tech/compose-action@v2.0.0
+      name: Compose Up


This workflow is still configured for Docker Compose, which doesn't meet the task requirement to validate the Terraform code. The workflow should use terraform init, validate, and plan. It looks like you have the correct workflow implementation in mate_academy/terraform/terraform_todo_app/.github/workflows/main.yml, but it needs to be located here at the root of the repository in .github/workflows/.

mateacademy-ai-mentor · 2025-11-16T20:31:46Z

+RUN pip install --upgrade pip && \
+    pip install -r requirements.txt
+
+RUN python manage.py migrate


Running database migrations during the Docker build (RUN) is generally not a good practice. The build process often lacks connectivity to the production database, which can cause the build to fail. It's better to run migrations when the container starts, for example, using an entrypoint script.

mateacademy-ai-mentor · 2025-11-16T20:31:47Z

+              <li><input type="checkbox" id="checkbox" data-todo-id="{{ todo.id }}"> {{ todo.description }}</li>
+            {% endif %}
+          {% endfor %}
+        </ul>
+      </div>
+      <div class="one-half column finished-todos">
+        <h6 class="docs-header finished-todos">{{ todolist.count_finished }} finished</h6>
+        <ul>
+          {% for todo in todolist.todos.all %}
+            {% if todo.is_finished %}
+            <li><input type="checkbox" id="checkbox" data-todo-id="{{ todo.id }}" checked="checked"> {{ todo.description }}</li>


Using a static id="checkbox" inside a for loop will result in multiple HTML elements with the same ID, which is invalid. IDs must be unique within a document. While this may not break the current functionality, it's best practice to use classes for shared styling or behavior, or to generate a unique ID for each element, like id="checkbox-{{ todo.id }}".

mateacademy-ai-mentor · 2025-11-16T20:31:47Z

+def todolist(request, todolist_id):
+    todolist = get_object_or_404(TodoList, pk=todolist_id)
+    if request.method == "POST":
+        redirect("lists:add_todo", todolist_id=todolist_id)


The redirect() function returns an HttpResponseRedirect object, but you are not returning it from the view. This means that even if this if block were executed, the function would continue and execute the render() call at the end, which is not the intended behavior. To fix this, you should add return before redirect().

mateacademy-ai-mentor · 2025-11-16T20:31:47Z

+mkdir /app 
+# make sure to uncomment the line bellow and update the link with your GitHub username
+# git clone https://github.com/<your-gh-username>/azure_task_12_deploy_app_with_vm_extention.git
+cp -r devops_todolist_terraform_task/app/* /app


This copy command is likely to fail because the application files are located at the root of the devops_todolist_terraform_task directory, not within an app subdirectory. You should adjust the source path to copy all necessary application files to the /app directory.

mateacademy-ai-mentor · 2025-11-16T20:31:47Z

+resource "azurerm_public_ip" "pip" {
+  name                = var.public_ip_name
+  location            = var.location
+  resource_group_name = var.resource_group_name
+
+  allocation_method = "Static"
+  sku               = "Standard"
+  sku_tier          = "Regional"
+}


This public IP resource is missing the required domain_name_label argument. You've correctly generated the value in the locals block, but it needs to be assigned to the resource here.

mateacademy-ai-mentor · 2025-11-16T20:31:47Z

+variable "location" { type = string }
+variable "resource_group_name" { type = string }
+variable "virtual_network_name" { type = string }
+variable "vnet_address_prefix" { type = string }
+variable "subnet_name" { type = string }
+variable "subnet_address_prefix" { type = string }
+variable "network_security_group" { type = string }
+variable "public_ip_name" { type = string }
+variable "dns_label" { type = string } # базова частина, наприклад "matetask"
+
+variable "vm_name" { type = string }
+variable "vm_size" { type = string }
+variable "ssh_key_public" { type = string }


The task requirements specify default values for several variables in this file (e.g., location = "uksouth", resource_group_name = "mate-azure-task-12", etc.). Please add the default attribute to all the required variables as listed in the task description. This was also mentioned in the previous review.

mateacademy-ai-mentor · 2025-11-16T20:31:47Z

+# SECURITY WARNING: don't run with debug turned on in production!
+DEBUG = True
+
+ALLOWED_HOSTS = []


As noted in the previous review, the ALLOWED_HOSTS list cannot be empty for a deployed application. To allow access from any host, which is acceptable for this task, please change this to ['*'].

mateacademy-ai-mentor · 2025-11-16T20:31:48Z

+        'NAME': 'app_db',
+        'USER': 'app_user',
+        'PASSWORD': '1234',
+        'HOST': '172.17.0.2',  # You can use a different host if your MySQL server is on a remote machine.


The database HOST is hardcoded to an IP that is unlikely to be correct on the deployed VM. Since the database will be running on the same machine as the application, this should be set to 'localhost' or '127.0.0.1'. This was also a high-priority issue from the previous review.

mateacademy-ai-mentor · 2025-11-16T20:31:48Z

+  <link href='http://fonts.googleapis.com/css?family=Raleway:400,300,600' rel='stylesheet' type='text/css'>
+  {% load static %}
+  <!-- CSS
+  –––––––––––––––––––––––––––––––––––––––––––––––––– -->
+  <link rel="stylesheet" type='text/css' href="https://cdnjs.cloudflare.com/ajax/libs/normalize/3.0.2/normalize.min.css">
+  <link rel="stylesheet" type='text/css' href="https://cdnjs.cloudflare.com/ajax/libs/skeleton/2.0.4/skeleton.min.css">
+  <link rel="stylesheet" type='text/css' href="{% static 'css/custom.css' %}">
+  {% block css %}{% endblock %}
+  <!-- Scripts
+  –––––––––––––––––––––––––––––––––––––––––––––––––– -->
+  <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/2.1.3/jquery.min.js"></script>


It's a best practice to load external resources like fonts and scripts over https:// instead of http://. This prevents mixed-content issues if your site is ever served over HTTPS and is generally more secure. Please update the href and src attributes to use https://.

ykulyk404 and others added 8 commits November 15, 2025 23:44

initial commit

fc6d6a6

Create main.yml

fd4a93e

Update main.yml

0fe0be0

Update main.yml

f4873c0

test compose

7e743e0

fix

602c986

fix

b5a680e

Add Terraform config for Azure VM with custom script extension

c6d22c0

mateacademy-ai-mentor suggested changes Nov 16, 2025

View reviewed changes

Add Terraform infrastructure code for Azure VM deployment

b67e2f0

VitaliySemeniv requested a review from mateacademy-ai-mentor November 16, 2025 14:31

mateacademy-ai-mentor suggested changes Nov 16, 2025

View reviewed changes

Fix GitHub Actions workflow

573c9c6

VitaliySemeniv requested a review from mateacademy-ai-mentor November 16, 2025 20:26

mateacademy-ai-mentor approved these changes Nov 16, 2025

View reviewed changes


		jobs:

		docker-compose:

Uh oh!

Conversation

VitaliySemeniv commented Nov 16, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone