Skip to content

Solution#47

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

Solution#47
dimonalek wants to merge 2 commits into
mate-academy:mainfrom
dimonalek:main

Conversation

@dimonalek

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

Your implementation correctly creates NSGs for all three subnets with appropriate Internet-facing rules, but it's missing a critical requirement: inter-subnet communication is not allowed. Each NSG needs an inbound security rule permitting traffic from the virtual network address space 10.20.30.0/24 with a lower priority (e.g., Priority 200) than existing Internet-facing rules. Add this rule to $webNsg, $mngNsg, and $dbNsg before calling New-AzNetworkSecurityGroup. Without these rules, subnets cannot communicate with each other, which violates the explicit requirement that "all subnets must allow traffic from other subnets within the virtual network."


✨ 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 task.ps1 Outdated
Comment on lines +17 to +33
Write-Host "Creating web network security group..."
# Write your code for creation of Web NSG here ->
$webNsgHttpHttpsRule = New-AzNetworkSecurityRuleConfig `
-Name "Allow-Http-Https" `
-Description "Allow HTTP and HTTPS from Internet" `
-Access Allow `
-Protocol Tcp `
-Direction Inbound `
-Priority 100 `
-SourceAddressPrefix "*" `
-SourcePortRange "*" `
-DestinationAddressPrefix "*" `
-DestinationPortRange @("80", "443")
$webNsg = New-AzNetworkSecurityGroup `
-Name $webSubnetName `
-ResourceGroupName $resourceGroupName `
-Location $location `
-SecurityRules $webNsgHttpHttpsRule

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 inter-subnet communication rule. Each NSG needs a rule allowing inbound traffic from the virtual network address space (10.20.30.0/24) so subnets can communicate with each other. Add a rule with lower priority (e.g., Priority 200) to $webNsg before creating it.

Comment thread task.ps1 Outdated
Comment on lines +34 to +51

Write-Host "Creating mngSubnet network security group..."
# Write your code for creation of management NSG here ->
$mngNsgSshRule = New-AzNetworkSecurityRuleConfig `
-Name "Allow-Ssh" `
-Description "Allow SSH from Internet" `
-Access Allow `
-Protocol Tcp `
-Direction Inbound `
-Priority 100 `
-SourceAddressPrefix "*" `
-SourcePortRange "*" `
-DestinationAddressPrefix "*" `
-DestinationPortRange "22"
$mngNsg = New-AzNetworkSecurityGroup `
-Name $mngSubnetName `
-ResourceGroupName $resourceGroupName `
-Location $location `
-SecurityRules $mngNsgSshRule

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 inter-subnet communication rule. Add a similar inbound rule from 10.20.30.0/24 to $mngNsg before creating it.

Comment thread task.ps1 Outdated
Comment on lines +52 to +57

Write-Host "Creating dbSubnet network security group..."
# Write your code for creation of management NSG here ->
$dbNsg = New-AzNetworkSecurityGroup `
-Name $dbSubnetName `
-ResourceGroupName $resourceGroupName `
-Location $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 inter-subnet communication rule. Add a similar inbound rule from 10.20.30.0/24 to $dbNsg before creating it.

@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

Your implementation successfully meets all requirements. The code correctly creates three Network Security Groups (webservers, database, management), attaches them to their respective subnets, and implements the proper security rules: HTTP/HTTPS access for the webservers subnet, SSH access for the management subnet, and no Internet access for the database subnet. The inter-subnet communication is properly enabled through VNet-level rules with address space 10.20.30.0/24 at Priority 200, allowing traffic between subnets while maintaining appropriate security boundaries. Great work addressing the previous feedback and ensuring all NSGs have the necessary inter-subnet communication rules in place.


✨ 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