Create Network Security Group by vladyslavgusakovskiy · Pull Request #36 · mate-academy/azure_task_16_create_network_security_group

vladyslavgusakovskiy · 2025-10-20T16:44:31Z

No description provided.

mateacademy-ai-mentor

Review Summary

I’m requesting changes because there are critical blockers that will prevent successful deployment and fail core requirements. Specifically:

Bug in web NSG rule (task.ps1, around the rule for HTTP/HTTPS): using -DestinationPortRange 80, 443 will cause a parameter binding/runtime error. Use -DestinationPortRanges @('80','443') or split into two rules (one for 80 and one for 443) with distinct priorities.
Missing explicit intra‑VNet allow rules: for each NSG (web, management, database), add an inbound rule with SourceAddressPrefix = 'VirtualNetwork' (and DestinationAddressPrefix = 'VirtualNetwork' if applicable) to permit subnet-to-subnet traffic, as required. Ensure these have higher precedence (lower numeric priority) than Internet allow rules.
Priority ordering: adjust priorities so the VirtualNetwork allow comes before Internet rules (e.g., VirtualNetwork at 100, Internet rules at 200+), avoiding unintended blocks.
Resource group naming: align with the prerequisite mate-resources RG name to match validation expectations.
After fixes, run scripts/generate-artifacts.ps1 and scripts/validate-artifacts.ps1, and commit the updated task.ps1 and the produced result.json.

You did well creating and attaching NSGs using the correct Az cmdlets—once the above are addressed, this should meet the requirements. Keep going; you’re close.

✨ 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-10-20T16:52:52Z

+    -SourceAddressPrefix Internet `
+    -SourcePortRange * `
+    -DestinationAddressPrefix $webSubnetIpRange `
+    -DestinationPortRange 80, 443


Bug: -DestinationPortRange 80, 443 will likely fail because -DestinationPortRange expects a single value (string). Use -DestinationPortRanges @('80','443') or create two separate rules (one for 80 and one for 443).

mateacademy-ai-mentor · 2025-10-20T16:52:53Z


 Write-Host "Creating web network security group..."
-# Write your code for creation of Web NSG here -> 
+$web_rule_http = New-AzNetworkSecurityRuleConfig `


Missing requirement: The web NSG block needs an explicit inbound rule that allows intra‑VNet traffic using SourceAddressPrefix = 'VirtualNetwork'. Also ensure rule priorities are set so this intra‑VNet allow has higher priority (lower numeric value) than Internet rules.

mateacademy-ai-mentor · 2025-10-20T16:52:53Z


 Write-Host "Creating mngSubnet network security group..."
-# Write your code for creation of management NSG here -> 
+$mng_rule = New-AzNetworkSecurityRuleConfig `


Missing requirement: The management NSG currently allows SSH from the Internet but lacks an explicit VirtualNetwork allow rule. Add an inbound rule with SourceAddressPrefix = 'VirtualNetwork' to permit subnet-to-subnet traffic (and set priorities so it takes precedence as needed).

mateacademy-ai-mentor · 2025-10-20T16:52:53Z


 Write-Host "Creating dbSubnet network security group..."
-# Write your code for creation of management NSG here -> 
+$db_nsg = New-AzNetworkSecurityGroup -ResourceGroupName $resourceGroupName -Location $location -Name $dbSubnetName


Requirement detail: The database NSG has no custom allow rules which blocks Internet—this meets the "no Internet ingress" requirement — but you still must add an explicit VirtualNetwork allow rule to permit intra‑VNet traffic as required by the task.

…fic; corrected web_rule_http -DestinationPortRange

mateacademy-ai-mentor

Review Summary

I am requesting changes because several core requirements are not met and will block correct functionality. In task.ps1, the web and management NSGs are missing explicit intra‑VNet allow rules (use -SourceAddressPrefix 'VirtualNetwork' and -DestinationAddressPrefix 'VirtualNetwork') with higher precedence than Internet rules; this violates the requirement to allow traffic from other subnets. The database NSG’s VNet rule uses -Protocol Tcp; per the requirement to allow full intra‑VNet traffic, set -Protocol '*' to avoid unintentionally blocking non‑TCP. Priorities conflict: multiple rules use -Priority 100 (e.g., your web Internet rule) causing ambiguity; set VNet allows to a lower numeric value (e.g., 100) and Internet allows to higher (e.g., 200+), ensuring the VNet rule takes precedence. Also, align the resource group to mate-resources [CHECKLIST ITEM], and commit the generated result.json after running scripts/generate-artifacts.ps1 and scripts/validate-artifacts.ps1 so validation can pass. Minor: for clarity and consistency, prefer DestinationAddressPrefix 'VirtualNetwork' on VNet rules across all NSGs. Great work getting the NSGs created and attached—once you address these items, the solution should meet the task requirements.

✨ 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-10-20T17:22:37Z

+$web_rule_http = New-AzNetworkSecurityRuleConfig `
+    -Name "HTTP_Rule" `
+    -Description "Allow HTTP and HTTPS" `
+    -Access Allow `
+    -Protocol Tcp `
+    -Direction Inbound `
+    -Priority 100 `
+    -SourceAddressPrefix Internet `
+    -SourcePortRange * `
+    -DestinationAddressPrefix $webSubnetIpRange `
+    -DestinationPortRanges @('80','443')
+
+$web_nsg = New-AzNetworkSecurityGroup -ResourceGroupName $resourceGroupName -Location $location -Name $webSubnetName -SecurityRules $web_rule_http


Web NSG only contains an Internet allow for HTTP/HTTPS. The task requires allowing traffic from other subnets in the VNet — add an explicit inbound rule with -SourceAddressPrefix 'VirtualNetwork' (and appropriate destination) to permit intra‑VNet traffic, and ensure its priority is lower (higher precedence) than the Internet rule.

mateacademy-ai-mentor · 2025-10-20T17:22:37Z

+    -Access Allow `
+    -Protocol Tcp `
+    -Direction Inbound `
+    -Priority 100 `


You used -Priority 100 here for the web Internet rule; several other rules also use 100 which creates priority conflicts. Make the VirtualNetwork allow rules have a lower numeric priority (e.g. 100) and set Internet allow rules to a higher numeric value (e.g. 200+).

mateacademy-ai-mentor · 2025-10-20T17:22:37Z

+$mng_rule = New-AzNetworkSecurityRuleConfig `
+    -Name "Allow_SSH" `
+    -Description "Allow SSH" `
+    -Access Allow `
+    -Protocol Tcp `
+    -Direction Inbound `
+    -Priority 100 `
+    -SourceAddressPrefix Internet `
+    -SourcePortRange * `
+    -DestinationAddressPrefix $mngSubnetIpRange `
+    -DestinationPortRange 22
+
+$mng_nsg = New-AzNetworkSecurityGroup -ResourceGroupName $resourceGroupName -Location $location -Name $mngSubnetName -SecurityRules $mng_rule


The management NSG defines only an Internet SSH allow. Add a VirtualNetwork inbound allow rule for intra‑VNet traffic and set priorities so the VNet rule has precedence (lower number) than the Internet SSH allow.

mateacademy-ai-mentor · 2025-10-20T17:22:37Z

+$db_Vnet_rule = New-AzNetworkSecurityRuleConfig `
+    -Name "Allow_Traffic_From_VNet" `
+    -Description "Allow traffic from other subnets within the same VNet" `
+    -Access Allow `
+    -Protocol Tcp `
+    -Direction Inbound `
+    -Priority 100 `
+    -SourceAddressPrefix VirtualNetwork `
+    -SourcePortRange * `
+    -DestinationAddressPrefix $dbSubnetIpRange `
+    -DestinationPortRange *


The database NSG currently has a VNet rule but uses -Protocol Tcp. To fully allow subnet-to-subnet traffic (as required), set -Protocol '*' (or equivalent) so all protocols/ports between subnets are permitted. Also verify you intentionally omitted any Internet allow rules for the database NSG (this is correct per requirements).

mateacademy-ai-mentor · 2025-10-20T17:22:38Z

+    -SourceAddressPrefix VirtualNetwork `
+    -SourcePortRange * `
+    -DestinationAddressPrefix $dbSubnetIpRange `


The -DestinationAddressPrefix for VNet rules can be VirtualNetwork to clearly express VNet-to-VNet allowance. Using the specific subnet IP range is acceptable, but ensure consistency across NSGs and that SourceAddressPrefix is VirtualNetwork as required by the task.

…fic the web and management NSGs

mateacademy-ai-mentor

Review Summary

Here is my approval to help you keep progressing as you learn. You’ve correctly created and attached NSGs per subnet, used Protocol '*' for intra‑VNet rules, and ensured the database NSG blocks Internet traffic — great job aligning with the core requirements.

Critical fixes to address next:

In task.ps1: Make the intra‑VNet allow rules higher precedence than Internet rules. For example, set VNet allow to Priority 100 and Internet rules to Priority 200+ for both web and management (see current Internet rules at Priority 100 and VNet rules at Priority 200).
In task.ps1: Target the required resource group mate-resources (replace the current $resourceGroupName value) so validation runs against the expected environment.
In artifacts.json: After running scripts/generate-artifacts.ps1, commit the generated result.json and update artifacts.json to include its storage URL (ensure SAS token/container match the prerequisites). Then run scripts/validate-artifacts.ps1 and commit outputs if required.

Minor improvement: Keep rule priorities consistent and clearly commented to avoid ambiguity. You’re very close — these adjustments will align everything with the checklist and strengthen your deployment.

✨ 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-10-20T17:37:08Z

@@ -1,3 +1,3 @@
 {
-  "resourcesTemplate": ""
+  "resourcesTemplate": "https://matestorage584.blob.core.windows.net/task-artifacts/task16/exported-template.json?sv=2025-05-05&se=2025-11-19T18%3A32%3A41Z&sr=b&sp=r&sig=U3iHf3XRQMUcXhNPUjo9cI8VnkhYNKoiZRuA%2FrcRu6M%3D"


This file only contains the resourcesTemplate URL. The task's checklist requires that the artifacts produced by scripts/generate-artifacts.ps1 (notably the generated result.json) be committed and referenced so validation can run. After running scripts/generate-artifacts.ps1 you should commit the generated result.json and update artifacts.json to include a reference/URL to that result (ensure the SAS token and container match the prerequisites).

mateacademy-ai-mentor · 2025-10-20T17:37:08Z

+    -Access Allow `
+    -Protocol Tcp `
+    -Direction Inbound `
+    -Priority 100 `


This Internet HTTP/HTTPS rule has Priority 100 which currently takes precedence over the VNet allow rule (priority 200). Per the requirements the intra‑VNet allow must take precedence. Make the VNet allow priority lower (e.g., 100) and this Internet rule a higher number (e.g., 200+).

mateacademy-ai-mentor · 2025-10-20T17:37:08Z

+    -Access Allow `
+    -Protocol * `
+    -Direction Inbound `
+    -Priority 200 `


The intra‑VNet rule here uses Priority 200. To satisfy the checklist the VNet allow must have a lower numeric priority than Internet rules so it takes precedence — set this to a lower number (e.g., 100) and move the Internet rule to a higher priority number.

mateacademy-ai-mentor · 2025-10-20T17:37:08Z

+    -Access Allow `
+    -Protocol Tcp `
+    -Direction Inbound `
+    -Priority 100 `


The management SSH Internet allow is Priority 100 which currently will take precedence over the VNet allow. Per the requirements the VNet allow must take precedence — change this Internet rule to a higher numeric priority (e.g., 200+).

mateacademy-ai-mentor · 2025-10-20T17:37:08Z

+    -Access Allow `
+    -Protocol * `
+    -Direction Inbound `
+    -Priority 200 `


The management intra‑VNet rule is Priority 200; it should be a lower numeric value than the Internet allow so intra‑VNet traffic is allowed first. Use a lower number here (e.g., 100) and raise the Internet rule priority.

mateacademy-ai-mentor · 2025-10-20T17:37:08Z

+    -DestinationAddressPrefix VirtualNetwork `
+    -DestinationPortRange *
+
+$web_nsg = New-AzNetworkSecurityGroup -ResourceGroupName $resourceGroupName -Location $location -Name $webSubnetName -SecurityRules $web_rule_http, $web_Vnet_rule


Each NSG is being created in the resource group referenced by $resourceGroupName. The task prerequisites require the resources to be in the resource group named mate-resources. Ensure $resourceGroupName equals mate-resources (or otherwise target that resource group) so the deployment and artifact validations use the expected resource group.

mateacademy-ai-mentor · 2025-10-20T17:37:08Z

+    -DestinationAddressPrefix VirtualNetwork `
+    -DestinationPortRange *
+
+$db_nsg = New-AzNetworkSecurityGroup -ResourceGroupName $resourceGroupName -Location $location -Name $dbSubnetName -SecurityRules $db_Vnet_rule


The database NSG contains only the intra‑VNet allow rule (Protocol '*' and no Internet allow) which matches the requirement to block Internet traffic. After fixing the web/management priorities, run scripts/generate-artifacts.ps1 and scripts/validate-artifacts.ps1 and commit the produced result.json/artifact outputs to the repo as required.

Created NSGs for subnets in the VNet

2b9e47b

mateacademy-ai-mentor suggested changes Oct 20, 2025

View reviewed changes

added an explicit VirtualNetwork allow rule to permit intra‑VNet traf…

1dac43d

…fic; corrected web_rule_http -DestinationPortRange

vladyslavgusakovskiy requested a review from mateacademy-ai-mentor October 20, 2025 17:18

mateacademy-ai-mentor suggested changes Oct 20, 2025

View reviewed changes

added an explicit VirtualNetwork allow rule to permit intra‑VNet traf…

ce18a95

…fic the web and management NSGs

vladyslavgusakovskiy requested a review from mateacademy-ai-mentor October 20, 2025 17:34

mateacademy-ai-mentor approved these changes Oct 20, 2025

View reviewed changes

Uh oh!

Conversation

vladyslavgusakovskiy commented Oct 20, 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants