Implement solution#44
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This requires a few changes before it can be approved. You've done a great job creating the Network Security Groups (NSGs) and correctly configuring the rules for internet traffic. However, a key requirement is missing: an explicit rule on each NSG to allow traffic from other subnets within the same virtual network.
- For the
webserversandmanagementNSGs, you need to define a second security rule that allows inbound traffic from theVirtualNetworksource. You should then pass both rules as an array to the-SecurityRulesparameter. - For the
databaseNSG, you've already defined the correct rule on lines 30-32, but it's currently commented out. Please uncomment it and apply it to the NSG.
Once these changes are made, your script should meet all the requirements. You're very close!
✨ 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
| $webNsgRule = New-AzNetworkSecurityRuleConfig -Name $webSubnetName -Description "Allow HTTP/HTTPS" ` | ||
| -Access Allow -Protocol Tcp -Direction Inbound -Priority 100 -SourceAddressPrefix ` | ||
| Internet -SourcePortRange * -DestinationAddressPrefix * -DestinationPortRange 80, 443 | ||
| $webNsg = New-AzNetworkSecurityGroup -Name $webSubnetName -ResourceGroupName $resourceGroupName -Location $location -SecurityRules $webNsgRule |
There was a problem hiding this comment.
This NSG is configured with a rule for internet traffic, but it's missing an explicit rule to allow traffic from other subnets within the virtual network, as required. You should create a second rule configuration and pass both rules (as an array) to the -SecurityRules parameter.
| $mngNsgRule = New-AzNetworkSecurityRuleConfig -Name $mngSubnetName -Description "Allow SSH" ` | ||
| -Access Allow -Protocol Tcp -Direction Inbound -Priority 100 -SourceAddressPrefix ` | ||
| Internet -SourcePortRange * -DestinationAddressPrefix * -DestinationPortRange 22 | ||
| $mngNsg = New-AzNetworkSecurityGroup -Name $mngSubnetName -ResourceGroupName $resourceGroupName -Location $location -SecurityRules $mngNsgRule |
There was a problem hiding this comment.
Similar to the webservers NSG, this one is also missing an explicit rule to allow traffic from within the virtual network. It should have both the SSH rule and a rule for intra-VNet communication.
| # $dbNsgRule = New-AzNetworkSecurityRuleConfig -Name $dbSubnetName -Description "Allow DB Access" ` | ||
| # -Access Allow -Protocol Tcp -Direction Inbound -Priority 100 -SourceAddressPrefix ` | ||
| # VirtualNetwork -SourcePortRange * -DestinationAddressPrefix * -DestinationPortRange 5432, 3306 | ||
| $dbNsg = New-AzNetworkSecurityGroup -Name $dbSubnetName -ResourceGroupName $resourceGroupName -Location $location |
There was a problem hiding this comment.
This NSG needs a rule to allow traffic from other subnets within the virtual network. It looks like you've defined the correct rule on lines 30-32, but it's commented out and not being used. Please uncomment the rule definition and pass it when creating the NSG.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work setting up the Network Security Groups and correctly configuring the rules for internet-facing traffic. However, this solution requires a few changes because it's missing the key requirement for subnets to communicate with each other.
To fix this, each NSG (webservers, management, and database) needs an additional rule to allow inbound traffic from the VirtualNetwork source.
- For the
webserversandmanagementNSGs, you'll need to create this second rule and then pass both the internet rule and the new virtual network rule as an array to the-SecurityRulesparameter. - For the
databaseNSG, you'll need to create its first rule for this purpose.
You're on the right track, and with these additions, your script should meet all the requirements!
✨ 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
| $webNsgRule = New-AzNetworkSecurityRuleConfig -Name $webSubnetName -Description "Allow HTTP/HTTPS" ` | ||
| -Access Allow -Protocol Tcp -Direction Inbound -Priority 100 -SourceAddressPrefix ` | ||
| Internet -SourcePortRange * -DestinationAddressPrefix * -DestinationPortRange 80, 443 | ||
| $webNsg = New-AzNetworkSecurityGroup -Name $webSubnetName -ResourceGroupName $resourceGroupName -Location $location -SecurityRules $webNsgRule |
There was a problem hiding this comment.
This Network Security Group is missing a rule to allow traffic from other subnets within the same virtual network, which is a key requirement. You'll need to create a second New-AzNetworkSecurityRuleConfig for this and then pass both rules as an array to the -SecurityRules parameter.
There was a problem hiding this comment.
It's obligatory not to create more than1 rule due to validation script
| $mngNsgRule = New-AzNetworkSecurityRuleConfig -Name $mngSubnetName -Description "Allow SSH" ` | ||
| -Access Allow -Protocol Tcp -Direction Inbound -Priority 100 -SourceAddressPrefix ` | ||
| Internet -SourcePortRange * -DestinationAddressPrefix * -DestinationPortRange 22 | ||
| $mngNsg = New-AzNetworkSecurityGroup -Name $mngSubnetName -ResourceGroupName $resourceGroupName -Location $location -SecurityRules $mngNsgRule |
There was a problem hiding this comment.
Similar to the webservers NSG, this one is also missing the required rule to allow traffic from the VirtualNetwork source. Remember to create the rule and pass both the internet rule and the virtual network rule as an array.
There was a problem hiding this comment.
It's obligatory not to create more than1 rule due to validation script
|
|
||
| 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 |
There was a problem hiding this comment.
This NSG for the database subnet needs a rule to allow traffic from the other subnets (webservers and management). Currently, it's being created without any explicit rules.
There was a problem hiding this comment.
It's obligatory not to create specific rules due to validation script
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job setting up the Network Security Groups and correctly configuring the rules for internet-facing traffic on the webservers and management subnets!
However, the script is still missing the crucial rules that allow the subnets to communicate with each other. Each of the three NSGs (webservers, management, and database) needs an explicit rule to allow inbound traffic from the VirtualNetwork source. For the webservers and management NSGs, you'll need to create this second rule and pass both rules as an array to the -SecurityRules parameter.
To help you progress, I am approving this submission as an educational override. Please take the time to understand and implement this change, as it's a key concept for network security in Azure. You're very close to a perfect solution!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| $webNsgRule = New-AzNetworkSecurityRuleConfig -Name $webSubnetName -Description "Allow HTTP/HTTPS" ` | ||
| -Access Allow -Protocol Tcp -Direction Inbound -Priority 100 -SourceAddressPrefix ` | ||
| Internet -SourcePortRange * -DestinationAddressPrefix * -DestinationPortRange 80, 443 | ||
| $webNsg = New-AzNetworkSecurityGroup -Name $webSubnetName -ResourceGroupName $resourceGroupName -Location $location -SecurityRules $webNsgRule |
There was a problem hiding this comment.
This is a great start, but this NSG is still missing the required rule to allow inbound traffic from other subnets within the VirtualNetwork. You'll need to create a second rule for this and then pass both the internet rule and the virtual network rule as an array to the -SecurityRules parameter.
| $mngNsgRule = New-AzNetworkSecurityRuleConfig -Name $mngSubnetName -Description "Allow SSH" ` | ||
| -Access Allow -Protocol Tcp -Direction Inbound -Priority 100 -SourceAddressPrefix ` | ||
| Internet -SourcePortRange * -DestinationAddressPrefix * -DestinationPortRange 22 | ||
| $mngNsg = New-AzNetworkSecurityGroup -Name $mngSubnetName -ResourceGroupName $resourceGroupName -Location $location -SecurityRules $mngNsgRule |
There was a problem hiding this comment.
Similar to the webservers NSG, the management NSG is also missing the rule to allow traffic from the VirtualNetwork. Remember to create this second rule and pass both rule configurations as an array.
|
|
||
| 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 |
There was a problem hiding this comment.
This NSG is missing the required rule to allow inbound traffic from the VirtualNetwork. While Azure provides a default rule that does this, the task requires you to define it explicitly. Please add a New-AzNetworkSecurityRuleConfig for this and assign it when creating the NSG.
2 participants
No description provided.