Skip to content

Terraform: deploy ToDo app with network, compute and storage modules#50

Open
maximprysyazhnikov wants to merge 2 commits into
mate-academy:mainfrom
maximprysyazhnikov:main
Open

Terraform: deploy ToDo app with network, compute and storage modules#50
maximprysyazhnikov wants to merge 2 commits into
mate-academy:mainfrom
maximprysyazhnikov:main

Conversation

@maximprysyazhnikov

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

This is a great effort, and the overall structure of your Terraform project is well-organized. However, I'm requesting a few changes as some key resource parameters don't align with the task requirements. Once these are addressed, it should be good to go.

Here are the required changes:

  1. VM Size & Location: In terraform.tfvars, please update the vm_size to Standard_B1s and the location to uksouth.
  2. Public IP Allocation: In modules/network/main.tf, the public IP address must use a Dynamic allocation method. Remember to also set its sku to Basic to support dynamic allocation.
  3. Backend Storage Account: In backend.tf, the storage_account_name needs to be set to the required placeholder value, yourstorageaccount.

Keep up the good work!


✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread main.tf

provider "azurerm" {
features {}
subscription_id = "2beb95a1-e310-40c7-926c-e1fc7424d0e4"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hardcoding the subscription_id here can make your configuration less portable and is generally not recommended. It's better practice to let Terraform authenticate using the Azure CLI context or service principal environment variables, which will automatically select the correct subscription.

Comment thread backend.tf Outdated
terraform {
backend "azurerm" {
resource_group_name = "mate-azure-task-12"
storage_account_name = "tfstatemaks01"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task description requires the storage account name for the backend to be yourstorageaccount. Please update this value to match the requirement.

Comment thread modules/network/main.tf Outdated
location = var.location
resource_group_name = var.resource_group_name

allocation_method = "Static"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requirements specify that the public IP address should use a Dynamic allocation method. This is currently set to Static. Note that to use dynamic allocation, the sku for the public IP must be set to Basic, not Standard.

Comment thread terraform.tfvars Outdated
@@ -0,0 +1,18 @@
location = "northeurope"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requirements specify the location should be uksouth. Please update this value.

Comment thread terraform.tfvars Outdated
public_ip_address_name = "linuxboxpip"

vm_name = "matebox"
vm_size = "Standard_DS1_v2"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The required virtual machine size is Standard_B1s, but Standard_DS1_v2 is used here. Please update the vm_size to match the task requirements.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Excellent work on this task! You've successfully implemented all the required Azure resources and structured the Terraform modules perfectly, meeting all functional requirements.

The automated check failed due to a minor formatting issue in terraform.tfvars. This is a common style issue that can be easily fixed for future commits by running terraform fmt -recursive in your project directory.

Since this doesn't affect the functionality of your code, I am happy to approve your solution. Great job addressing all the previous feedback!


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

Footnotes

  1. Rate AI review example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants