Skip to content

Solution#63

Open
kkatkot wants to merge 8 commits into
mate-academy:mainfrom
kkatkot:main
Open

Solution#63
kkatkot wants to merge 8 commits into
mate-academy:mainfrom
kkatkot:main

Conversation

@kkatkot
Copy link
Copy Markdown

@kkatkot kkatkot commented May 2, 2026

No description provided.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Review Summary

This Terraform configuration has multiple critical blockers that prevent successful deployment. The most critical issues are: (1) backend.tf only defines Azure resources but is missing the actual terraform { backend "azurerm" {} } block required for remote state storage; (2) All modules reference azurerm_resource_group.example without defining it as a variable or declaring it locally, causing Terraform init/plan failures; (3) Storage container name is hardcoded as "vhds" instead of using var.storage_container_name which defaults to "task-artifacts"; (4) Variable naming mismatches between root and modules (e.g., vm_name vs virtual_machine_name, ssh_key_public vs ssh_public_key) will cause undefined variable errors; (5) Missing outputs.tf files in modules means data cannot flow between them. Additionally, main.tf is missing the required azurerm_resource_group resource declaration. These issues must be fixed before the configuration can be deployed.


✨ 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 modules/compute/variables.tf Outdated
Comment on lines +1 to +2
variable virtual_machine_name {
type = string
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 network interface uses ${var.virtual_machine_name}-nic but the task requires ${var.vm_name}-nic. The variable name doesn't match the requirement.

Comment thread modules/compute/variables.tf Outdated
Comment on lines +9 to +13
}
variable ssh_public_key {
type = string
default = "linuxboxsshkey"

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 variable is named ssh_public_key but the task specifies ssh_key_public. This naming mismatch should be corrected to match the requirement.

Comment thread backend.tf Outdated
Comment on lines +1 to +26
resource "azurerm_resource_group" "example" {
name = var.resource_group_name
location = var.location
}

resource "azurerm_storage_account" "example" {
name = "yourstorageaccount"
resource_group_name = azurerm_resource_group.example.name
location = azurerm_resource_group.example.location
account_tier = "Standard"
account_replication_type = "LRS"
}

resource "azurerm_storage_container" "example" {
name = var.vm_name
storage_account_id = azurerm_storage_account.example.id
container_access_type = "private"
}

resource "azurerm_storage_blob" "example" {
name = "my-awesome-content.zip"
storage_account_name = azurerm_storage_account.example.name
storage_container_name = azurerm_storage_container.example.name
type = "Block"
source = "some-local-file.zip"
} No newline at end of file
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 backend.tf file defines Azure resources instead of configuring a Terraform backend. According to the task requirements, this should be a terraform {} block with backend configuration pointing to Azure Blob Storage with container name 'tfstate'.

Comment thread backend.tf Outdated
Comment on lines +14 to +19
resource "azurerm_storage_container" "example" {
name = var.vm_name
storage_account_id = azurerm_storage_account.example.id
container_access_type = "private"
}

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 storage container name is 'vhds' but the requirements specify it must be named 'task-artifacts'.

Comment thread backend.tf Outdated
Comment on lines +8 to +13
resource_group_name = azurerm_resource_group.example.name
location = azurerm_resource_group.example.location
account_tier = "Standard"
account_replication_type = "LRS"
}

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 storage module has IP-based network rules that would block legitimate traffic. The storage account should be accessible for the task purposes.

Comment thread backend.tf Outdated
name = "yourstorageaccount"
resource_group_name = azurerm_resource_group.example.name
location = azurerm_resource_group.example.location
account_tier = "Standard"
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 network interface references azurerm_public_ip.example.id but the public IP resource is in the network module. This resource needs to be passed via module output/variable.

Comment thread backend.tf Outdated

resource "azurerm_storage_account" "example" {
name = "yourstorageaccount"
resource_group_name = azurerm_resource_group.example.name
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 network interface references azurerm_subnet.example.id but the subnet resource is in the network module. This resource needs to be passed via module output/variable.

Comment thread backend.tf Outdated
Comment on lines +1 to +4
resource "azurerm_resource_group" "example" {
name = var.resource_group_name
location = var.location
}
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 network interface references resources from other modules without proper variable declarations. The compute module should receive subnet_id and public_ip_address_id as input variables.

Comment thread backend.tf Outdated
Comment on lines +25 to +26
source = "some-local-file.zip"
} No newline at end of file
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 admin_ssh_key variable name doesn't match the root variables.tf which defines 'ssh_key_public'. The variable reference should be var.ssh_key_public.

Comment thread modules/compute/main.tf Outdated
Comment on lines +14 to +17
resource "azurerm_linux_virtual_machine" "example" {
name = var.virtual_machine_name
resource_group_name = azurerm_resource_group.example.name
location = azurerm_resource_group.example.location
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 container name should be tfstate as specified in the task requirements for the remote state backend, not var.vm_name

Comment thread modules/compute/main.tf
Comment on lines +1 to +25
resource "azurerm_network_interface" "example" {
name = "${var.virtual_machine_name}-nic"
location = azurerm_resource_group.example.location
resource_group_name = azurerm_resource_group.example.name

ip_configuration {
name = "internal"
subnet_id = azurerm_subnet.example.id
private_ip_address_allocation = "Dynamic"
public_ip_address_id = azurerm_public_ip.example.id
}
}

resource "azurerm_linux_virtual_machine" "example" {
name = var.virtual_machine_name
resource_group_name = azurerm_resource_group.example.name
location = azurerm_resource_group.example.location
size = var.vm_size
admin_username = "adminuser"
network_interface_ids = [
azurerm_network_interface.example.id,
]

admin_ssh_key {
username = "adminuser"
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 Terraform backend configuration is missing. This file defines resources but doesn't configure the backend. You need to add a terraform { backend "azurerm" { ... } } block to enable remote state storage

Comment thread modules/compute/main.tf Outdated
}

resource "azurerm_linux_virtual_machine" "example" {
name = var.virtual_machine_name
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable vm_name is not defined in this file's variables.tf. The compute module variables.tf defines virtual_machine_name, causing a reference mismatch

Comment thread modules/compute/main.tf Outdated

settings = <<SETTINGS
{
"commandToExecute": "bash cd .. && cd .. && install-app.sh"
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 cd commands are invalid - you cannot cd to parent directories from the root. The command should just be bash install-app.sh to execute the script

Comment thread .gitignore
@@ -0,0 +1,2 @@
.terraform/
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 backend.tf file is missing the actual Terraform backend configuration block. It only creates Azure resources but doesn't configure Terraform to use them for state storage. You need to add a terraform {} block with backend "azurerm" configuration at the top of this file or in a separate file.

Comment thread .gitignore
@@ -0,0 +1,2 @@
.terraform/
.terraform.lock.hcl No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable naming inconsistency: root variables.tf defines vm_name, but the compute module uses virtual_machine_name. These should match. Update compute module's variables.tf and main.tf to use vm_name consistently.

Comment thread modules/network/variables.tf Outdated
Comment on lines +3 to +7
default = "vnet"
}

variable virtual_network_address_prefix {
type = list(string)
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 network module references azurerm_resource_group.example but the resource group should be passed as a variable, not hardcoded. Add a resource_group_name variable.

Comment thread modules/network/variables.tf Outdated
Comment on lines +6 to +8
variable virtual_network_address_prefix {
type = list(string)
default = ["10.0.0.0/16"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable type mismatch: network module uses list(string) for address_prefix but root variables.tf uses string. The types should be consistent across modules.

Comment thread modules/network/variables.tf Outdated
Comment on lines +8 to +12
default = ["10.0.0.0/16"]
}

variable subnet_name {
type = string
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Storage module references azurerm_subnet.example.id which doesn't exist in this module. Remove the network_rules block or pass the subnet_id as a variable.

Comment thread modules/network/variables.tf Outdated
Comment on lines +20 to +23

variable azurerm_network_security_group_name {
type = string
default = "defaultnsg"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Storage container name is hardcoded as 'vhds' instead of using var.storage_container_name. Use the variable to match the requirement of naming it 'task-artifacts'.

Comment thread modules/storage/variables.tf Outdated
Comment on lines +1 to +2
variable storage_account_name {
type = string
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 default storage account name "storageaccountname" may not be valid as Azure storage account names must be 3-24 characters, lowercase, and unique. Consider making this more unique or passing it as a variable from terraform.tfvars.

Comment thread modules/storage/variables.tf Outdated
@@ -0,0 +1,9 @@
variable storage_account_name {
type = string
default = "storageaccountname"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Both the network and storage modules reference azurerm_resource_group.example without defining it. This resource should be passed as a variable (e.g., var.resource_group_name) or declared locally with a data source.

Comment thread modules/storage/main.tf
}
}

resource "azurerm_storage_container" "example" {
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 storage container is hardcoded as 'vhds' instead of using var.storage_container_name which defaults to 'task-artifacts' as required.

Comment thread modules/storage/main.tf
Comment on lines +8 to +11

network_rules {
default_action = "Deny"
ip_rules = ["100.0.0.1"]
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 network_rules block references azurerm_subnet.example.id but this resource doesn't exist in the storage module. This will cause a Terraform error. Either remove the network_rules or pass the subnet_id as a variable.

Comment thread modules/storage/main.tf
virtual_network_subnet_ids = [azurerm_subnet.example.id]
}

tags = {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable naming inconsistency: root variables.tf defines subnet_address_prefix but network module variables.tf defines subnet_name_address_prefix. These should match for consistent variable passing.

Comment thread modules/network/main.tf Outdated
Comment on lines +6 to +7
location = azurerm_resource_group.example.location
resource_group_name = azurerm_resource_group.example.name
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 network module references azurerm_resource_group.example but this resource is not defined in this module. The resource group should be passed as a variable (e.g., var.resource_group_name) so it can be provided when the module is called from main.tf.

Comment thread modules/network/main.tf Outdated
Comment on lines +15 to +19

# delegation {
# name = "delegation"

# service_delegation {
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 subnet variable is named subnet_name_address_prefix in this module, but the root variables.tf defines subnet_address_prefix. These names should be consistent. Consider renaming to match the root variable name.

Comment thread modules/network/main.tf Outdated
Comment on lines +8 to +12
}

resource "azurerm_subnet" "example" {
name = var.subnet_name
resource_group_name = azurerm_resource_group.example.name
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 storage module references azurerm_subnet.example.id but this resource doesn't exist in the storage module. This will cause a Terraform error. Either remove the network_rules block or pass subnet_id as a variable.

Comment thread modules/network/main.tf Outdated
Comment on lines +20 to +23
# name = "Microsoft.ContainerInstance/containerGroups"
# actions = ["Microsoft.Network/virtualNetworks/subnets/join/action", "Microsoft.Network/virtualNetworks/subnets/prepareNetworkPolicies/action"]
# }
# }
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 storage container name is hardcoded as 'vhds' instead of using var.storage_container_name. The variable exists and defaults to 'task-artifacts' but isn't being used.

Comment thread modules/network/main.tf Outdated
Comment on lines +2 to +5

resource "azurerm_virtual_network" "example" {
name = var.virtual_network_name
address_space = var.virtual_network_address_prefix
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 storage account references azurerm_resource_group.example without defining the resource group as a variable in this module. This resource must be passed in from the calling configuration.

Comment thread variables.tf Outdated
Comment on lines +20 to +23

variable subnet_address_prefix {
type = string
default = "subnet_address_prefix"
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 subnet_address_prefix variable has an incorrect default value of "subnet_address_prefix" instead of "10.0.0.0/24" as required by the task specification.

Comment thread variables.tf Outdated
}

variable subnet_address_prefix {
type = string
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable naming inconsistency: the root variable is named subnet_address_prefix but the network module's variables.tf uses subnet_name_address_prefix. These should be renamed to match consistently for proper variable passing.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Review Summary

The request meets the required criteria for approval.


✨ 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 Outdated
Comment on lines +1 to +12
module "network" {
source = "./modules/network"

subnet_address_prefix = var.subnet_address_prefix
dns_label = var.dns_label
public_ip_address_name = var.public_ip_address_name
virtual_network_name = var.virtual_network_name
subnet_name = var.subnet_name
network_security_group_name = var.network_security_group_name
vnet_address_prefix = var.vnet_address_prefix
resource_group_name = var.resource_group_name
location = var.location
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

MISSING CRITICAL BLOCK: This file only defines Azure resources but is missing the required terraform { backend "azurerm" {} } block for remote state storage. The backend configuration with resource_group, storage_account, container_name, and key must be defined here.

Comment thread main.tf Outdated
Comment on lines +11 to +13
resource_group_name = var.resource_group_name
location = var.location
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL ISSUE: The storage container is hardcoded as "vhds" instead of using var.storage_container_name. Per requirements, it should be named task-artifacts.

Comment thread main.tf Outdated
Comment on lines +3 to +4

subnet_address_prefix = var.subnet_address_prefix
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TYPO IN OUTPUT NAME: This output is misspelled as storage_comtainer_name but backend.tf line 9 references module.storage.storage_container_name (correct spelling). This will cause a Terraform error.

Comment thread modules/compute/main.tf
Comment on lines +1 to +12
resource "azurerm_network_interface" "example" {
name = "${var.vm_name}-nic"
location = var.location
resource_group_name = var.resource_group_name

ip_configuration {
name = "internal"
subnet_id = var.subnet_id
private_ip_address_allocation = "Dynamic"
public_ip_address_id = var.public_ip
}
}
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 backend.tf file still only contains resource definitions. It must include the actual terraform { backend "azurerm" {} } block to configure remote state storage in Azure Blob Storage.

Comment thread modules/compute/main.tf
Comment on lines +3 to +6
location = var.location
resource_group_name = var.resource_group_name

ip_configuration {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Duplicate variable declaration for location. Terraform will fail to initialize with this error.

Comment thread modules/compute/main.tf
private_ip_address_allocation = "Dynamic"
public_ip_address_id = var.public_ip
}
}
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 storage container name is hardcoded as "vhds" but the requirement specifies it should be task-artifacts. Use var.storage_container_name which already defaults to "task-artifacts".

Comment thread modules/compute/main.tf
resource "azurerm_network_interface" "example" {
name = "${var.vm_name}-nic"
location = var.location
resource_group_name = var.resource_group_name
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typo in output name: storage_comtainer_name should be storage_container_name. This will cause errors when modules/compute or other modules try to reference this output.

Comment thread modules/compute/main.tf
Comment on lines +7 to +9
name = "internal"
subnet_id = var.subnet_id
private_ip_address_allocation = "Dynamic"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Network rule restricts access to only IP 100.0.0.1, which will block Terraform's access to the storage account during deployments. Consider using default_action = "Allow" for initial deployment.

Comment thread modules/compute/main.tf
resource_group_name = var.resource_group_name

ip_configuration {
name = "internal"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incorrect network prefix. Should be 10.0.0.0/16 as specified in the requirements.

Comment thread modules/compute/main.tf

ip_configuration {
name = "internal"
subnet_id = var.subnet_id
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incorrect subnet prefix. Should be 10.0.0.0/24 as specified in the requirements.

Comment thread backend.tf Outdated
Comment on lines +1 to +12
resource "azurerm_resource_group" "example" {
name = var.resource_group_name
location = var.location
}

resource "azurerm_storage_blob" "example" {
name = "my-awesome-content.zip"
storage_account_name = module.storage.storage_account_name
storage_container_name = module.storage.storage_container_name
type = "Block"
source = "some-local-file.zip"
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL: This file is missing the required terraform { backend "azurerm" {} } block. According to the task requirements, backend.tf must configure Azure Blob Storage for remote state with: Resource Group mate-azure-task-12, Storage Account yourstorageaccount, Container Name tfstate, and Key terraform.tfstate. Without this block, Terraform will use local state instead of remote state.

Comment on lines +4 to +7

variable location {}
variable vm_name {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL: Duplicate variable declaration for location. Terraform will fail with an error like 'Duplicate variable definition'. Remove one of these declarations (lines 4 and 7).

Comment thread modules/network/main.tf
Comment on lines +5 to +7
address_space = var.vnet_address_prefix
location = var.location
resource_group_name = var.resource_group_name
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

DUPLICATE VARIABLE DECLARATION: location is declared twice (lines 5 and 7). Terraform will fail with an error about duplicate variable names. Remove one of these declarations.

Comment on lines +4 to +8

variable subnet_name {}

variable subnet_address_prefix {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL - DUPLICATE VARIABLE: The variable location is declared twice (lines 4 and 7). Terraform will fail with error: 'Variable "location" was already declared at line 4'. Remove one of these declarations.

Comment thread modules/storage/main.tf
}
}

resource "azurerm_storage_container" "example" {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL - HARDCODED CONTAINER NAME: The container should use var.storage_container_name (which defaults to "task-artifacts" per requirements) instead of hardcoded "vhds".

Comment thread modules/storage/main.tf
resource "azurerm_storage_account" "example" {
name = var.storage_account_name
resource_group_name = var.resource_group_name

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL - TYPO IN OUTPUT: The output name is storage_comtainer_name (typo - 'tain' instead of 'tainer'). Since backend.tf references module.storage.storage_container_name, this typo will cause a Terraform error when trying to access this output.

Comment thread modules/storage/main.tf Outdated
Comment on lines +8 to +12

network_rules {
default_action = "Deny"
ip_rules = ["100.0.0.1"]
virtual_network_subnet_ids = [azurerm_subnet.example.id]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Network rules restrict access to only IP 100.0.0.1 and one subnet. This may block Terraform provider access during operations. Consider using default_action = "Allow" for initial deployment or ensuring your IP is in the allowed list.

Comment thread modules/storage/main.tf
resource "azurerm_storage_account" "example" {
name = var.storage_account_name
resource_group_name = var.resource_group_name

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

INCORRECT VALUE: The VNet address prefix should be "10.0.0.0/16" per task requirements, not "10000/16".

Comment thread modules/storage/main.tf
resource_group_name = var.resource_group_name

location = var.location
account_tier = "Standard"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

INCORRECT VALUE: The subnet address prefix should be "10.0.0.0/24" per task requirements, not "10000/24".

Comment thread modules/storage/variables.tf Outdated
Comment on lines +8 to +12
default = "task-artifacts"
}
variable resource_group_name {}

variable location {} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL ERROR: azurerm_subnet.example is not defined in this module. Subnet resources are created in the network module, not storage. This will cause Terraform to fail with 'Resource 'azurerm_subnet.example' not found'. Either remove this line or use a data source to reference the subnet.

Comment thread modules/storage/variables.tf Outdated
Comment on lines +3 to +4
default = "mate-storage-account"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TYPO: Output name is misspelled as storage_comtainer_name. If other modules reference module.storage.storage_container_name (correct spelling), this will fail.

variable storage_account_name {
type = string
default = "mate-storage-account"
}
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 network prefix is incorrect. Should be "10.0.0.0/16" per requirements, not "10000/16".

Comment thread modules/storage/variables.tf Outdated
default = "mate-storage-account"
}

variable storage_container_name {
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 subnet prefix is incorrect. Should be "10.0.0.0/24" per requirements, not "10000/24".

@@ -0,0 +1,6 @@
output "storage_account_name" {
value = azurerm_storage_account.example.name
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TYPO IN OUTPUT NAME: storage_comtainer_name is misspelled. Should be storage_container_name to match what backend.tf and other modules will reference.

Comment thread modules/storage/outputs.tf Outdated
output "storage_account_name" {
value = azurerm_storage_account.example.name
}
output "storage_comtainer_name" {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

INCORRECT NETWORK PREFIX: Should be "10.0.0.0/16" as specified in the task requirements.

}
output "storage_comtainer_name" {
value = azurerm_storage_container.example.name
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

INCORRECT SUBNET PREFIX: Should be "10.0.0.0/24" as specified in the task requirements.

Comment thread terraform.tfvars Outdated
Comment on lines +11 to +12
ssh_key_public = "your-public-key-content"
dns_label = "matetask" No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL: This references azurerm_subnet.example.id but the subnet resource is in the network module, not the storage module. This will cause a Terraform error. Either remove this reference or create a variable for subnet_id input.

Comment thread terraform.tfvars Outdated
location = "uksouth"
resource_group_name = "mate-azure-task-12"
virtual_network_name = "vnet"
vnet_address_prefix = "10000/16"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TYPO: storage_comtainer_name should be storage_container_name (missing 'a'). Other modules expecting storage_container_name will fail.

Comment thread terraform.tfvars Outdated
location = "uksouth"
resource_group_name = "mate-azure-task-12"
virtual_network_name = "vnet"
vnet_address_prefix = "10000/16"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

INCORRECT VALUE: The vnet address prefix should be "10.0.0.0/16" as required by the task, not "10000/16".

Comment thread terraform.tfvars Outdated
virtual_network_name = "vnet"
vnet_address_prefix = "10000/16"
subnet_name = "default"
subnet_address_prefix = "10000/24"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

INCORRECT VALUE: The subnet address prefix should be "10.0.0.0/24" as required by the task, not "10000/24".

Comment thread variables.tf Outdated
Comment on lines +20 to +23

variable subnet_address_prefix {
type = string
default = "subnet_address_prefix"
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 default value is a placeholder string "subnet_address_prefix" instead of the required CIDR notation "10.0.0.0/24". While this may be overridden by terraform.tfvars, the default should match the task requirement.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Review Summary

Your Terraform project has good structure and organization with proper module separation for network, compute, and storage. I can see you attempted to configure the backend and implement all required resources. However, there are several important fixes needed for your code to work properly: the backend.tf should use terraform { backend "azurerm" { ... } } syntax instead of resource blocks; the network address prefixes need to be corrected from 10000/16 and 10000/24 to 10.0.0.0/16 and 10.0.0.0/24; remove the duplicate variable location {} declarations in modules/compute/variables.tf; and fix the cross-module reference to azurerm_subnet.example.id in the storage module by either removing it or passing the subnet ID as a variable. These are common learning points when working with Terraform modules and Azure backends. Keep iterating on these fixes and your infrastructure code will be production-ready!


✨ 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
Comment on lines +1 to +12
terraform {
required_providers {
azurerm = {
source = "hashicorp/azurerm"
version = "4.72.0"
}
}
}

provider "azurerm" {
# Configuration options
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This file should configure the Terraform remote state backend using Azure Blob Storage, not create Azure resources. The backend configuration should be inside a terraform {} block with backend "azurerm" { ... }. Remove the azurerm_resource_group and azurerm_storage_blob resources and replace with proper backend configuration as specified in the requirements.

Comment thread main.tf Outdated
Comment on lines +8 to +21
}

provider "azurerm" {
# Configuration options
}


module "network" {
source = "./modules/network"

subnet_address_prefix = var.subnet_address_prefix
dns_label = var.dns_label
public_ip_address_name = var.public_ip_address_name
virtual_network_name = var.virtual_network_name
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 storage module's main.tf references azurerm_subnet.example.id on line 8, but the storage module doesn't have access to this resource. Either remove the network_rules block entirely, or pass the subnet ID as a variable to the storage module.

Comment thread main.tf
source = "hashicorp/azurerm"
version = "4.72.0"
}
}
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 vnet_address_prefix is set to 10000/16 which is invalid CIDR notation. According to requirements, it should be 10.0.0.0/16.

Comment thread main.tf
version = "4.72.0"
}
}
}
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 subnet_address_prefix is set to 10000/24 which is invalid CIDR notation and doesn't match the required 10.0.0.0/24 from the task description.

Comment thread modules/compute/main.tf
Comment on lines +1 to +12
resource "azurerm_network_interface" "example" {
name = "${var.vm_name}-nic"
location = var.location
resource_group_name = var.resource_group_name

ip_configuration {
name = "internal"
subnet_id = var.subnet_id
private_ip_address_allocation = "Dynamic"
public_ip_address_id = var.public_ip
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

backend.tf should contain the Terraform backend configuration for Azure Blob Storage, not resource definitions. The backend block should be:

terraform {
  backend "azurerm" {
    resource_group_name  = "mate-azure-task-12"
    storage_account_name = "yourstorageaccount"
    container_name       = "tfstate"
    key                  = "terraform.tfstate"
  }
}

Remove the azurerm_resource_group and azurerm_storage_blob resources from this file.

Comment thread modules/compute/main.tf
name = "${var.vm_name}-nic"
location = var.location
resource_group_name = var.resource_group_name

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

vnet_address_prefix is set to '10000/16' but the task requirements specify '10.0.0.0/16'.

Comment thread modules/compute/main.tf
resource_group_name = var.resource_group_name

ip_configuration {
name = "internal"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

subnet_address_prefix is set to '10000/24' but the task requirements specify '10.0.0.0/24'.

Comment thread modules/compute/main.tf
}

resource "azurerm_linux_virtual_machine" "example" {
name = var.vm_name
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 storage module references azurerm_subnet.example.id but this resource doesn't exist in the storage module's context. This will cause a Terraform error. Either remove the network_rules block or ensure the subnet is properly referenced.

Comment thread backend.tf Outdated
Comment on lines +1 to +12
resource "azurerm_resource_group" "example" {
name = var.resource_group_name
location = var.location
}

resource "azurerm_storage_blob" "example" {
name = "my-awesome-content.zip"
storage_account_name = module.storage.storage_account_name
storage_container_name = module.storage.storage_container_name
type = "Block"
source = "some-local-file.zip"
} No newline at end of file
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 backend.tf file is missing the required Terraform backend configuration. It should contain a terraform block with backend "azurerm" that specifies the resource group, storage account, container name, and key for Azure Blob Storage remote state backend.

Comment thread backend.tf Outdated
location = var.location
}

resource "azurerm_storage_blob" "example" {
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 vnet_address_prefix is set to 10000/16 but should be 10.0.0.0/16 according to the task requirements.

Comment thread backend.tf Outdated

resource "azurerm_storage_blob" "example" {
name = "my-awesome-content.zip"
storage_account_name = module.storage.storage_account_name
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 subnet_address_prefix is set to 10000/24 but should be 10.0.0.0/24 according to the task requirements.

Comment on lines +4 to +8

variable subnet_name {}

variable subnet_address_prefix {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Duplicate variable declaration for location on lines 4 and 7. Terraform requires unique variable names within a module. Remove one of these duplicate declarations.

Comment on lines +4 to +7

variable location {}
variable vm_name {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Duplicate variable declaration - variable location {} is declared twice (lines 4 and 7). Terraform will fail with an error about duplicate variable names. Remove one of the declarations.

Comment thread modules/network/main.tf
Comment on lines +5 to +8
address_space = var.vnet_address_prefix
location = var.location
resource_group_name = var.resource_group_name
}
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 location variable is declared twice (lines 5 and 8). This will cause a Terraform parsing error. Remove one of the duplicate declarations.

output "storage_account_name" {
value = azurerm_storage_account.example.name
}
output "storage_container_name" {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

vnet_address_prefix is set to 10000/16 but requirements specify 10.0.0.0/16.

}
output "storage_container_name" {
value = azurerm_storage_container.example.name
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

subnet_address_prefix is set to 10000/24 but requirements specify 10.0.0.0/24.

Comment thread modules/storage/variables.tf Outdated
Comment on lines +11 to +12

variable location {} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Line 11 references azurerm_subnet.example.id, but the storage module doesn't have access to this resource (it's defined in the network module). This will cause a Terraform error. Options: 1) Remove the virtual_network_subnet_ids block entirely, or 2) Pass the subnet ID as a variable to the storage module, or 3) Change default_action to "Allow" and remove the subnet reference.

variable storage_account_name {
type = string
default = "mate-storage-account"
}
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 vnet_address_prefix is set to 10000/16 but the task requirements specify 10.0.0.0/16.

Comment thread modules/storage/variables.tf Outdated
default = "mate-storage-account"
}

variable storage_container_name {
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 subnet_address_prefix is set to 10000/24 but the task requirements specify 10.0.0.0/24.

Comment thread modules/storage/main.tf Outdated
Comment on lines +8 to +12

network_rules {
default_action = "Deny"
ip_rules = ["100.0.0.1"]
virtual_network_subnet_ids = [azurerm_subnet.example.id]
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 network_rules block restricts access to only IP 100.0.0.1. The VM will not be able to access this storage account. Either set default_action = "Allow" or ensure the VM's public IP is included in ip_rules. Consider removing these restrictions for simplicity.

Comment thread modules/storage/main.tf Outdated
network_rules {
default_action = "Deny"
ip_rules = ["100.0.0.1"]
virtual_network_subnet_ids = [azurerm_subnet.example.id]
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 virtual_network_subnet_ids references azurerm_subnet.example.id but this resource is not defined in the storage module. This will cause a Terraform error. Either remove this line or pass the subnet ID as a variable.

Comment thread modules/storage/main.tf
resource "azurerm_storage_account" "example" {
name = var.storage_account_name
resource_group_name = var.resource_group_name

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

vnet_address_prefix is set to 10000/16 but should be 10.0.0.0/16 according to task requirements.

Comment thread modules/storage/main.tf
resource_group_name = var.resource_group_name

location = var.location
account_tier = "Standard"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

subnet_address_prefix is set to 10000/24 but should be 10.0.0.0/24 according to task requirements.

Comment thread terraform.tfvars Outdated
Comment on lines +8 to +12
public_ip_address_name = "linuxboxpip"
vm_name = "matebox"
vm_size = "Standard_B1s"
ssh_key_public = "your-public-key-content"
dns_label = "matetask" No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reference to undefined resource - azurerm_subnet.example.id is referenced here, but azurerm_subnet.example is not defined in the storage module. This will cause a Terraform error. Options: 1) Remove the virtual_network_subnet_ids block entirely, 2) Pass the subnet ID as a variable to this module, or 3) Change default_action to "Allow" and remove the subnet reference.

Comment thread terraform.tfvars Outdated
location = "uksouth"
resource_group_name = "mate-azure-task-12"
virtual_network_name = "vnet"
vnet_address_prefix = "10000/16"
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 vnet_address_prefix is set to 10000/16 but the requirements specify 10.0.0.0/16.

Comment thread terraform.tfvars Outdated
virtual_network_name = "vnet"
vnet_address_prefix = "10000/16"
subnet_name = "default"
subnet_address_prefix = "10000/24"
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 subnet_address_prefix is set to 10000/24 but the requirements specify 10.0.0.0/24.

Comment thread variables.tf Outdated
Comment on lines +20 to +23

variable subnet_address_prefix {
type = string
default = "subnet_address_prefix"
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 subnet_address_prefix default value is "subnet_address_prefix" which is invalid CIDR notation. According to task requirements, this should be "10.0.0.0/24". This variable is used in the network module to create the subnet, so an incorrect value will cause deployment issues.

Copy link
Copy Markdown

@linubah linubah left a comment

Choose a reason for hiding this comment

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

@kkatkot kkatkot requested a review from linubah May 14, 2026 20:34
Copy link
Copy Markdown

@Losiev Losiev left a comment

Choose a reason for hiding this comment

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

Well done!👏

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.

4 participants