Solution

[Muskam09]

No description provided.

[mateacademy-ai-mentor]

Review Summary

Decision: REQUEST_CHANGES — critical blockers remain that prevent meeting core requirements and likely cause validation failure.

Why: The script misses required rules and targets the wrong resources, violating checklist items 1.3, 1.6, and 2.3. These are critical blockers, not minor issues.

Critical issues to fix:

1. Intra-VNet allow rules missing (Requirement 1.3)

   • Add an inbound allow rule on each NSG (webservers, management, database) with:
     • Source: VirtualNetwork (service tag)
     • Destination: VirtualNetwork or *
     • Protocol: *
     • DestinationPortRange: *
     • Access: Allow
     • Priority: lower (e.g., 100-200) than any deny rules.
   • Example: New-AzNetworkSecurityRuleConfig -Name Allow-IntraVNet -Access Allow -Protocol * -Direction Inbound -Priority 110 -SourceAddressPrefix VirtualNetwork -SourcePortRange * -DestinationAddressPrefix * -DestinationPortRange *
   • Point of failure: in task.ps1, there is no rule using the VirtualNetwork tag for any NSG.

2. Database NSG must deny all inbound Internet traffic (Requirement 1.6)

   • Add an explicit inbound deny rule on the database NSG:
     • Source: Internet
     • Protocol: *
     • DestinationPortRange: *
     • Access: Deny
     • Priority: higher number than allows (e.g., 300).
   • Example: New-AzNetworkSecurityRuleConfig -Name Deny-Internet-All -Access Deny -Protocol * -Direction Inbound -Priority 300 -SourceAddressPrefix Internet -SourcePortRange * -DestinationAddressPrefix * -DestinationPortRange *
   • Point of failure: task.ps1 defines no explicit deny on the database NSG.

3. Update existing VNet and subnets rather than creating new ones (Requirement 2.3)

   • Replace VNet/subnet creation with retrieval and update:
     • $vnet = Get-AzVirtualNetwork -Name <existingVnetName> -ResourceGroupName <rg>
     • Get existing subnets: $subnet = $vnet.Subnets | Where-Object Name -eq 'webservers' (repeat for management, database).
     • Associate NSGs: $subnet.NetworkSecurityGroup = $webNsg; Set-AzVirtualNetwork -VirtualNetwork $vnet.
   • Point of failure: current script creates a new VNet and subnets and attaches NSGs to those new subnets.

4. Set explicit priorities for all rules (Requirement 2.6)

   • Ensure ordering: intra-VNet allow (e.g., 110), specific Internet allows (web: 120/130; mgmt: 120), and denies (database Internet deny at 300). Avoid overlaps and duplicate priorities.

5. Idempotency (Requirement 3.7)

   • Add checks before creating resources and rules:
     • Use Get-AzNetworkSecurityGroup to reuse existing NSGs if they exist.
     • Before adding a rule, check SecurityRules.Name and update if needed.
     • When associating NSGs to subnets, only set/commit if the association differs.

6. Artifacts completeness

   • Generate and include result.json if required by your pipeline/validation scripts.

Minor improvements (optional):

• Consider explicit Internet deny rules on webservers/management for non-HTTP(S)/SSH if validation expects them.
• Add basic error handling (try/catch) and parameterize names/locations.

You’re close—your use of Az cmdlets and the basic NSG setup is solid. Address the above items and you should pass validation. If you share the existing VNet/subnet names, I can help sketch the exact cmdlet sequence to update them safely.

---

✨ 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]

You are missing an intra-VNet allow rule for the webservers NSG. Each NSG must have a rule that allows traffic from the 'VirtualNetwork' service tag (intra-VNet traffic) as required by checklist item 1.3.

[mateacademy-ai-mentor]

You are missing an intra-VNet allow rule for the management NSG. Each NSG must have a rule that allows traffic from the 'VirtualNetwork' service tag (intra-VNet traffic) as required by checklist item 1.3.

[mateacademy-ai-mentor]

You are missing both an intra-VNet allow rule and an explicit deny rule for Internet-sourced traffic for the database NSG. The database NSG must deny all inbound Internet traffic and allow intra-VNet traffic as per requirements 1.3 and 1.6.

[Alkaponees]

LGTM!