Solution

[zave52]

No description provided.

[mateacademy-ai-mentor]

Review Summary

Decision: REQUEST_CHANGES — several core requirements are missing and block successful completion.

Critical issues to fix (blocking):

1. Attach NSGs to existing subnets in an existing VNet (do not create a new VNet/subnets).

   • Why: The task requires updating the existing VNet. Creating a new VNet breaks validation and does not secure the intended environment.
   • How: Use Get-AzVirtualNetwork -Name <existingVNet> -ResourceGroupName <rg> to retrieve the VNet, get subnets via $vnet.Subnets | Where-Object Name -eq 'webservers' (and others), set NetworkSecurityGroup on each subnet, then Set-AzVirtualNetwork to save.
   • Tip: Do not call New-AzVirtualNetwork or recreate subnets.

2. Add intra-VNet allow rules on all NSGs (webservers, management, database).

   • Why: Required to allow inter-subnet traffic; without it, expected VNet communication is blocked.
   • How: Add an inbound rule with -SourceAddressPrefix VirtualNetwork -DestinationAddressPrefix VirtualNetwork -Access Allow -Direction Inbound -Priority <e.g., 100> for each NSG.

3. Implement idempotency.

   • Why: Script must be safe to rerun without failing or duplicating resources.
   • How: Use Get-AzNetworkSecurityGroup -Name <nsg> -ErrorAction SilentlyContinue to check existence; create if missing, otherwise update rules if not present. When associating NSGs to subnets, check current association before setting.

4. Database NSG: add an explicit deny for Internet-sourced inbound traffic.

   • Why: Requirement suggests explicit deny for clarity and validation, not just relying on default deny.
   • How: Add a rule like -Name 'Deny-Internet-Inbound' -SourceAddressPrefix Internet -Access Deny -Direction Inbound -Priority <higher than allow rules, e.g., 200>.

Line-level guidance (task.ps1):

• Anywhere you call New-AzVirtualNetwork and Add-AzVirtualNetworkSubnetConfig: replace with retrieving the existing VNet using Get-AzVirtualNetwork and updating its subnets.
• Where you create rules for webservers (HTTP/HTTPS) and management (SSH): insert an additional VirtualNetwork allow rule above more specific rules (lower priority number, e.g., 100) for each NSG.
• After NSG creation, before associating: check for existing NSGs with Get-AzNetworkSecurityGroup and reuse/update instead of recreating.
• When associating NSGs: set ($subnet).NetworkSecurityGroup = $nsg and then run Set-AzVirtualNetwork -VirtualNetwork $vnet once per VNet.

What you did well:

• Correct use of Az cmdlets and appropriate HTTP/HTTPS and SSH rules on the right NSGs.
• Clear naming aligned with subnet roles.

Please address the above blockers and resubmit. You’re close—once you target the existing VNet, add the intra-VNet allow rules, ensure idempotency, and add the explicit deny on the database NSG, the solution should meet all 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.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

Review Summary

Decision: Request changes

Your script has good foundations for NSG creation and rule definition, but there are critical blockers that prevent meeting the task requirements.

Blocking issues to fix

1. Incorrect VNet/subnet handling (critical): You are recreating the VNet and subnets using New-AzVirtualNetwork and New-AzVirtualNetworkSubnetConfig instead of updating the existing ones. This violates the requirement to update the existing network and will fail validation.
   • Why: The task requires associating NSGs to the existing subnets.
   • How to fix: Use Get-AzVirtualNetwork -Name <vnetName> -ResourceGroupName <rg> to retrieve the existing VNet. Update each target subnet’s .NetworkSecurityGroup property (e.g., $vnet.Subnets | Where-Object Name -eq 'webservers') to the corresponding NSG, then Set-AzVirtualNetwork -VirtualNetwork $vnet to persist. Do not call New-AzVirtualNetwork or New-AzVirtualNetworkSubnetConfig.
   • Example: $subnet = $vnet.Subnets | Where-Object Name -eq 'webservers'; $subnet.NetworkSecurityGroup = $webNsg; Set-AzVirtualNetwork -VirtualNetwork $vnet.
2. Missing intra-VNet allow rules on all NSGs (critical): No inbound rule allowing VirtualNetwork to VirtualNetwork traffic.
   • Why: Required to permit subnet-to-subnet communication and part of the checklist.
   • How to fix: Add an inbound allow rule on each NSG with -SourceAddressPrefix 'VirtualNetwork' -DestinationAddressPrefix 'VirtualNetwork' -Access Allow -Direction Inbound -Protocol * -Priority 100 (ensure priority is lower number than Internet rules so it’s evaluated first as needed).
3. Database NSG missing explicit Internet deny (critical): No explicit inbound deny rule for source Internet.
   • Why: The requirement calls for an explicit deny to pass validation (not relying on default deny).
   • How to fix: Add an inbound deny rule on the database NSG with -SourceAddressPrefix 'Internet' -Access Deny -Direction Inbound -Protocol * -Priority 200 (priority higher number than the intra-VNet allow, but before any other broader allows if present).
4. Idempotency for subnet association (critical): Because you recreate the VNet/subnets, reruns are unsafe.
   • How to fix: When updating the existing VNet, check if each subnet already references the correct NSG before setting it. Only call Set-AzVirtualNetwork when changes are needed.

What you did well

• Clear NSG creation and naming; correct Internet allow rules for web (80/443) and management (22). Artifacts are generated properly.

Please address the above items. Once you switch to updating the existing VNet via Get-AzVirtualNetwork/Set-AzVirtualNetwork, add the intra-VNet allow rules, and include the database Internet deny, your solution should meet all 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.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

Review Summary

Educational approval: You’ve made solid progress—NSGs are correctly created, named after the subnets, associated to the right subnets, and your rule creation is idempotent. Nice work using the proper Az cmdlets and keeping PowerShell 7 compatibility.

Critical fixes to address:

• Missing intra-VNet allow rules (task.ps1): Add an inbound allow with SourceAddressPrefix VirtualNetwork, DestinationAddressPrefix *, Protocol *. Suggest priority 100 and name like Allow-IntraVNet. This enables subnet-to-subnet traffic as required.
• Database NSG lacks explicit Internet deny (task.ps1): Add an inbound SourceAddressPrefix Internet, Access Deny, name Deny-Internet, priority 300. This meets the explicit deny requirement.
• Rule priorities are all 100 (task.ps1): Each rule must have a unique priority. Suggested pattern: 100 (intra-VNet allow), 200/210/220 (service allows), 300 (explicit denies). This ensures correct evaluation order.

Improvements for clarity/validation:

• Split web HTTP/HTTPS into two rules (task.ps1): Use Allow-HTTP (80) and Allow-HTTPS (443) with distinct priorities (e.g., 200 and 210) for clearer intent and validation.
• Use clear, deterministic names: Allow-IntraVNet, Allow-HTTP, Allow-HTTPS, Allow-SSH, Deny-Internet.
• Resource group name: Replace mate-azure-task-16 with mate-resources per requirements.
• Idempotent subnet association: Before calling Set-AzVirtualNetwork, check if the subnet’s NetworkSecurityGroup.Id already matches the NSG to avoid unnecessary updates.

You’re very close—addressing these targeted changes will bring your script to full compliance. Great job so far; keep going!

---

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

Footnotes

1. Rate AI review example ↩