Solution#34
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I’m requesting changes because there are critical blockers that will break artifact generation/validation and violate explicit requirements.
- Artifacts container/name mismatch (blocker):
artifacts.jsonpoints totask-artifacts, buttask.ps1createstask-atrifacts(typo). This prevents upload/download ofexported-template.json. Fix the container name intask.ps1(or updateartifacts.jsonto the actual path) and ensure the SAS URL is valid and the file exists atartifacts.json.resourcesTemplate. - NSG naming requirement (blocker): The task requires each NSG name to exactly match its subnet name. In
task.ps1, NSGs are created with a-nsgsuffix; change to usewebservers,management,databaseexactly (e.g.,-Name $webSubnetName). Also ensure deployment parameters inresult.jsonset those exact names. - Validation script inaccuracies (blocker for reliable grading): In
scripts/validate-artifacts.ps1, success messages don’t match the checks (webservers expects 3 rules; management 2; database 1). Update messages to match counts, re-enable the webservers port checks (80/443 fromInternet), and addsourceAddressPrefix -eq 'Internet'to the SSH rule check.
Minor/cleanup:
- In
result.json, remove the misspelled container resourcetask-atrifacts(lines 496–511) and keep only the correcttask-artifacts(lines 479–494). Consider consolidating duplicatesecurityRules(inline vs child resources) to avoid confusion.
Once fixed, re-deploy to the required resource group, regenerate artifacts, and re-run validation. The rule logic itself looks solid—great work there. Keep going; you’re 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
| @@ -1,3 +1,3 @@ | |||
| { | |||
| "resourcesTemplate": "" | |||
| "resourcesTemplate": "https://storageac88.blob.core.windows.net/task-artifacts/task16/exported-template.json?sv=2025-05-05&se=2025-10-31T00%3A21%3A42Z&sr=b&sp=r&sig=PWWf7gJLK%2BJzmSlo1%2Bh8WjCp%2B09mDWxfdwEgkLpyE9A%3D" | |||
There was a problem hiding this comment.
The URL on this line points at the container task-artifacts and the exported template location. Confirm that this exact path exists and that the SAS URL remains valid. The validation script downloads the template from artifacts.json.resourcesTemplate, so if the file is not present at this location the validation will fail (scripts/validate-artifacts.ps1 reads this value and invokes Invoke-WebRequest) . If you intend to keep the current URL, ensure your deployment creates the container and uploads exported-template.json to this path.
| { | ||
| "type": "Microsoft.Storage/storageAccounts/blobServices/containers", | ||
| "apiVersion": "2025-01-01", | ||
| "name": "[concat(parameters('storageAccounts_storageac88_name'), '/default/task-atrifacts')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Storage/storageAccounts/blobServices', parameters('storageAccounts_storageac88_name'), 'default')]", | ||
| "[resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccounts_storageac88_name'))]" | ||
| ], | ||
| "properties": { | ||
| "immutableStorageWithVersioning": { | ||
| "enabled": false | ||
| }, | ||
| "defaultEncryptionScope": "$account-encryption-key", | ||
| "denyEncryptionScopeOverride": false, | ||
| "publicAccess": "None" | ||
| } |
There was a problem hiding this comment.
There is a misspelled storage container resource named task-atrifacts. The artifacts generation expects task-artifacts — the typo will prevent artifact upload and later validation. Remove the incorrect container resource or rename it to task-artifacts so only the correct container exists. See the correct container resource at lines 479-494 and the typo at lines 496-511 in this file.
| "networkSecurityGroups_database_nsg_name": { | ||
| "type": "String" | ||
| }, | ||
| "networkSecurityGroups_management_nsg_name": { | ||
| "type": "String" | ||
| }, | ||
| "networkSecurityGroups_webservers_nsg_name": { | ||
| "type": "String" |
There was a problem hiding this comment.
The template defines parameters for NSG names (e.g. networkSecurityGroups_webservers_nsg_name) but does not enforce concrete values here. The task requirement states each NSG must have the same name as its corresponding subnet (webservers, database, management). When you deploy/generate artifacts ensure the parameter values are set to those exact subnet names (no extra suffixes such as -nsg). If the parameters are populated incorrectly at deployment time, validation may fail. Relevant parameter block is shown here.
| "securityRules": [ | ||
| { | ||
| "name": "Allow-VNet", | ||
| "id": "[resourceId('Microsoft.Network/networkSecurityGroups/securityRules', parameters('networkSecurityGroups_webservers_nsg_name'), 'Allow-VNet')]", | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "properties": { | ||
| "description": "Allow all VNet traffic", | ||
| "protocol": "*", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "*", | ||
| "sourceAddressPrefix": "VirtualNetwork", | ||
| "destinationAddressPrefix": "VirtualNetwork", | ||
| "access": "Allow", | ||
| "priority": 100, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } | ||
| }, | ||
| { | ||
| "name": "Allow-HTTP", | ||
| "id": "[resourceId('Microsoft.Network/networkSecurityGroups/securityRules', parameters('networkSecurityGroups_webservers_nsg_name'), 'Allow-HTTP')]", | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "properties": { | ||
| "description": "Allow HTTP", | ||
| "protocol": "Tcp", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "80", | ||
| "sourceAddressPrefix": "Internet", | ||
| "destinationAddressPrefix": "*", | ||
| "access": "Allow", | ||
| "priority": 200, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } | ||
| }, | ||
| { | ||
| "name": "Allow-HTTPS", | ||
| "id": "[resourceId('Microsoft.Network/networkSecurityGroups/securityRules', parameters('networkSecurityGroups_webservers_nsg_name'), 'Allow-HTTPS')]", | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "properties": { | ||
| "description": "Allow HTTPS", | ||
| "protocol": "Tcp", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "443", | ||
| "sourceAddressPrefix": "Internet", | ||
| "destinationAddressPrefix": "*", | ||
| "access": "Allow", | ||
| "priority": 210, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
The NSG rules for each NSG look correct and comply with requirements: Allow-VNet priority 100 (intra-VNet allow), webservers NSG includes inbound TCP 80 and 443 from Internet (priorities 200 and 210), management NSG includes inbound TCP 22 from Internet (priority 200), and database NSG only has the Allow-VNet rule. These are the expected rule semantics (source prefixes, directions and priorities). See webservers rules and management rules in the template.
| { | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "apiVersion": "2024-07-01", | ||
| "name": "[concat(parameters('networkSecurityGroups_webservers_nsg_name'), '/Allow-HTTP')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_webservers_nsg_name'))]" | ||
| ], | ||
| "properties": { | ||
| "description": "Allow HTTP", | ||
| "protocol": "Tcp", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "80", | ||
| "sourceAddressPrefix": "Internet", | ||
| "destinationAddressPrefix": "*", | ||
| "access": "Allow", | ||
| "priority": 200, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } | ||
| }, | ||
| { | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "apiVersion": "2024-07-01", | ||
| "name": "[concat(parameters('networkSecurityGroups_webservers_nsg_name'), '/Allow-HTTPS')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_webservers_nsg_name'))]" | ||
| ], | ||
| "properties": { | ||
| "description": "Allow HTTPS", | ||
| "protocol": "Tcp", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "443", | ||
| "sourceAddressPrefix": "Internet", | ||
| "destinationAddressPrefix": "*", | ||
| "access": "Allow", | ||
| "priority": 210, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } | ||
| }, | ||
| { | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "apiVersion": "2024-07-01", | ||
| "name": "[concat(parameters('networkSecurityGroups_management_nsg_name'), '/Allow-SSH')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_management_nsg_name'))]" | ||
| ], | ||
| "properties": { | ||
| "description": "Allow SSH", | ||
| "protocol": "Tcp", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "22", | ||
| "sourceAddressPrefix": "Internet", | ||
| "destinationAddressPrefix": "*", | ||
| "access": "Allow", | ||
| "priority": 200, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } |
There was a problem hiding this comment.
There are duplicated securityRules entries: security rules exist inline within NSG resources and also as separate Microsoft.Network/networkSecurityGroups/securityRules resources (for example the separate Allow-HTTP/Allow-HTTPS/Allow-SSH resources starting around line 213). While ARM exports sometimes produce both forms, having both can be confusing and could lead to management complexity. Consider consolidating (either inline or separate child resources) so the template is clearer. If you keep both, ensure they remain consistent.
|
|
||
| $databaseNSG = ( $TemplateObject.resources | Where-Object -Property type -EQ "Microsoft.Network/networkSecurityGroups" | Where-Object {$_.name.Contains("database")}) | ||
| if ($databaseNSG.properties.securityRules.Count -eq 0) { | ||
| if ($databaseNSG.properties.securityRules.Count -eq 1) { |
There was a problem hiding this comment.
The database NSG check asserts Count -eq 1 (which corresponds to the Allow-VNet rule), but the success message says "Checked if database NSG has no rules - OK." This is contradictory. Update the message to reflect the actual expectation (1 rule: Allow-VNet) or change the test if the intended expectation differs. The current mismatch is confusing and may hide failures.
| -SourceAddressPrefix Internet -SourcePortRange * ` | ||
| -DestinationAddressPrefix * -DestinationPortRange 443 | ||
|
|
||
| $webnsg = New-AzNetworkSecurityGroup -ResourceGroupName $resourceGroupName -Location $location -Name "$webSubnetName-nsg" -SecurityRules $allowVnet,$allowHttp,$allowHttps |
There was a problem hiding this comment.
NSG name uses a -nsg suffix. The task requires each NSG to have the same name as its corresponding subnet (i.e. webservers, database, management). Change the -Name parameter to use the subnet name variable directly, e.g. -Name $webSubnetName instead of -Name "$webSubnetName-nsg". Also ensure the resource group used is the required one (see other comment). This impacts validation which expects NSG names to match subnets. Requirement reference: each NSG must have the same name as its corresponding subnet.
| -SourceAddressPrefix Internet -SourcePortRange * ` | ||
| -DestinationAddressPrefix * -DestinationPortRange 22 | ||
|
|
||
| $mngnsg = New-AzNetworkSecurityGroup -ResourceGroupName $resourceGroupName -Location $location -Name "$mngSubnetName-nsg" -SecurityRules $allowVnet,$allowSsh |
There was a problem hiding this comment.
Same issue here for the management NSG. Use -Name $mngSubnetName (i.e. management) rather than -Name "$mngSubnetName-nsg". Also confirm the resource group variable is set to the required mate-resources before deployment so artifacts/validation target the correct RG.
| -SourceAddressPrefix VirtualNetwork -SourcePortRange * ` | ||
| -DestinationAddressPrefix VirtualNetwork -DestinationPortRange * | ||
|
|
||
| $dbnsg = New-AzNetworkSecurityGroup -ResourceGroupName $resourceGroupName -Location $location -Name "$dbSubnetName-nsg" -SecurityRules $allowVnet |
There was a problem hiding this comment.
Same issue for the database NSG. Use -Name $dbSubnetName (i.e. database) instead of -Name "$dbSubnetName-nsg". The validate script expects NSGs attached and named after subnets. After renaming, re-deploy and re-run artifact generation/validation.
|
|
||
| $storageKey = (Get-AzStorageAccountKey -ResourceGroupName $resourceGroupName -Name $storageName)[0].Value | ||
| $ctx = New-AzStorageContext -StorageAccountName $storageName -StorageAccountKey $storageKey | ||
| New-AzStorageContainer -Name "task-atrifacts" -Context $ctx -Permission Off No newline at end of file |
There was a problem hiding this comment.
Storage container name is misspelled: you create task-atrifacts (missing a second 'c'). The artifacts generation/validation pipeline expects the container task-artifacts. Rename this to task-artifacts so scripts/generate-artifacts.ps1 can upload artifacts and scripts/validate-artifacts.ps1 can retrieve them. This typo will break the verification workflow.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Decision: REQUEST_CHANGES — there are critical blockers that will break validation and task completion.
-
Critical blockers to fix:
- In
result.json, remove the duplicate/misspelled storage container resourcetask-atrifacts(lines noted in comments). Keep only the correcttask-artifactsresource to prevent artifact upload/download failures. - In
task.ps1, NSG names must exactly match subnet names (no “-nsg” suffix). UpdateNew-AzNetworkSecurityGroup -Nameto usewebservers,management, anddatabaseexactly. This is required by the checklist and the validator depends on exact names. - In
scripts/validate-artifacts.ps1, re-enable the HTTP/HTTPS checks for webservers and ensure the SSH check verifiessourceAddressPrefix -eq 'Internet'. Also fix the error messages to match the expected rule counts (webservers=3, management=2, database=1) to avoid confusing failures.
- In
-
Additional guidance:
- In
result.json, remove duplicated childsecurityRulesresources or keep only inline rules to avoid ambiguity. - Ensure
artifacts.jsonSAS URL is valid and thatexported-template.jsonis uploaded totask-artifactsafter runningscripts/generate-artifacts.ps1.
- In
You’ve correctly implemented the NSG rule logic and subnet associations—great work. Address the naming, container, and validator issues above, regenerate artifacts, and the solution should pass validation.
✨ 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,3 +1,3 @@ | |||
| { | |||
| "resourcesTemplate": "" | |||
| "resourcesTemplate": "https://storageac88.blob.core.windows.net/task-artifacts/task16/exported-template.json?sv=2025-05-05&se=2025-10-31T00%3A21%3A42Z&sr=b&sp=r&sig=PWWf7gJLK%2BJzmSlo1%2Bh8WjCp%2B09mDWxfdwEgkLpyE9A%3D" | |||
There was a problem hiding this comment.
The resourcesTemplate URL uses the task-artifacts container — this is correct and matches the expected artifacts container. Before submitting, verify: 1) the SAS URL is valid and not expired; 2) the file exported-template.json exists at this exact path after you run scripts/generate-artifacts.ps1; 3) there are no typos in other files (e.g., task-atrifacts) that would cause uploads/downloads to go to a different container. If any typos exist elsewhere, fix them so all references use task-artifacts.
| { | ||
| "type": "Microsoft.Storage/storageAccounts/blobServices/containers", | ||
| "apiVersion": "2025-01-01", | ||
| "name": "[concat(parameters('storageAccounts_storageac88_name'), '/default/task-atrifacts')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Storage/storageAccounts/blobServices', parameters('storageAccounts_storageac88_name'), 'default')]", | ||
| "[resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccounts_storageac88_name'))]" | ||
| ], | ||
| "properties": { | ||
| "immutableStorageWithVersioning": { | ||
| "enabled": false | ||
| }, | ||
| "defaultEncryptionScope": "$account-encryption-key", | ||
| "denyEncryptionScopeOverride": false, | ||
| "publicAccess": "None" | ||
| } |
There was a problem hiding this comment.
Critical: The template contains a duplicate and misspelled storage container resource named "task-atrifacts". This will conflict with the artifacts workflow (artifacts.json expects task-artifacts) and break upload/download of exported-template.json. Remove this resource (the entire object at these lines) so only the correct "task-artifacts" container remains.
| { | ||
| "type": "Microsoft.Storage/storageAccounts/blobServices/containers", | ||
| "apiVersion": "2025-01-01", | ||
| "name": "[concat(parameters('storageAccounts_storageac88_name'), '/default/task-artifacts')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Storage/storageAccounts/blobServices', parameters('storageAccounts_storageac88_name'), 'default')]", | ||
| "[resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccounts_storageac88_name'))]" | ||
| ], | ||
| "properties": { | ||
| "immutableStorageWithVersioning": { | ||
| "enabled": false | ||
| }, | ||
| "defaultEncryptionScope": "$account-encryption-key", | ||
| "denyEncryptionScopeOverride": false, | ||
| "publicAccess": "None" | ||
| } |
There was a problem hiding this comment.
The correct storage container resource appears earlier at lines 479–494 as "task-artifacts" — keep that one and delete the misspelled duplicate. Ensure the SAS URL in artifacts.json points to this container and is valid after you regenerate artifacts.
| "networkSecurityGroups_database_nsg_name": { | ||
| "type": "String" | ||
| }, | ||
| "networkSecurityGroups_management_nsg_name": { | ||
| "type": "String" | ||
| }, | ||
| "networkSecurityGroups_webservers_nsg_name": { | ||
| "type": "String" |
There was a problem hiding this comment.
The template defines parameters for NSG names (networkSecurityGroups_webservers_nsg_name, networkSecurityGroups_management_nsg_name, networkSecurityGroups_database_nsg_name) but does not set explicit values here. The validator and task requirements expect NSG names to exactly match subnet names (webservers, management, database). Make sure the exported template (or the parameters passed during deployment/artifacts generation) populates these parameters with exactly those names (no '-nsg' suffix). You can provide defaultValue fields here or ensure task.ps1 creates NSGs with these exact names so the exported template contains them.
| { | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "apiVersion": "2024-07-01", | ||
| "name": "[concat(parameters('networkSecurityGroups_webservers_nsg_name'), '/Allow-HTTP')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_webservers_nsg_name'))]" | ||
| ], | ||
| "properties": { | ||
| "description": "Allow HTTP", | ||
| "protocol": "Tcp", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "80", | ||
| "sourceAddressPrefix": "Internet", | ||
| "destinationAddressPrefix": "*", | ||
| "access": "Allow", | ||
| "priority": 200, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } | ||
| }, | ||
| { | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "apiVersion": "2024-07-01", | ||
| "name": "[concat(parameters('networkSecurityGroups_webservers_nsg_name'), '/Allow-HTTPS')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_webservers_nsg_name'))]" | ||
| ], | ||
| "properties": { | ||
| "description": "Allow HTTPS", | ||
| "protocol": "Tcp", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "443", | ||
| "sourceAddressPrefix": "Internet", | ||
| "destinationAddressPrefix": "*", | ||
| "access": "Allow", | ||
| "priority": 210, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } | ||
| }, | ||
| { | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "apiVersion": "2024-07-01", | ||
| "name": "[concat(parameters('networkSecurityGroups_management_nsg_name'), '/Allow-SSH')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_management_nsg_name'))]" | ||
| ], | ||
| "properties": { | ||
| "description": "Allow SSH", | ||
| "protocol": "Tcp", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "22", | ||
| "sourceAddressPrefix": "Internet", | ||
| "destinationAddressPrefix": "*", | ||
| "access": "Allow", | ||
| "priority": 200, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } | ||
| }, | ||
| { | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "apiVersion": "2024-07-01", | ||
| "name": "[concat(parameters('networkSecurityGroups_database_nsg_name'), '/Allow-VNet')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_database_nsg_name'))]" | ||
| ], | ||
| "properties": { | ||
| "description": "Allow all VNet traffic", | ||
| "protocol": "*", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "*", | ||
| "sourceAddressPrefix": "VirtualNetwork", | ||
| "destinationAddressPrefix": "VirtualNetwork", | ||
| "access": "Allow", | ||
| "priority": 100, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } | ||
| }, | ||
| { | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "apiVersion": "2024-07-01", | ||
| "name": "[concat(parameters('networkSecurityGroups_management_nsg_name'), '/Allow-VNet')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_management_nsg_name'))]" | ||
| ], | ||
| "properties": { | ||
| "description": "Allow all VNet traffic", | ||
| "protocol": "*", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "*", | ||
| "sourceAddressPrefix": "VirtualNetwork", | ||
| "destinationAddressPrefix": "VirtualNetwork", | ||
| "access": "Allow", | ||
| "priority": 100, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } | ||
| }, | ||
| { | ||
| "type": "Microsoft.Network/networkSecurityGroups/securityRules", | ||
| "apiVersion": "2024-07-01", | ||
| "name": "[concat(parameters('networkSecurityGroups_webservers_nsg_name'), '/Allow-VNet')]", | ||
| "dependsOn": [ | ||
| "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_webservers_nsg_name'))]" | ||
| ], | ||
| "properties": { | ||
| "description": "Allow all VNet traffic", | ||
| "protocol": "*", | ||
| "sourcePortRange": "*", | ||
| "destinationPortRange": "*", | ||
| "sourceAddressPrefix": "VirtualNetwork", | ||
| "destinationAddressPrefix": "VirtualNetwork", | ||
| "access": "Allow", | ||
| "priority": 100, | ||
| "direction": "Inbound", | ||
| "sourcePortRanges": [], | ||
| "destinationPortRanges": [], | ||
| "sourceAddressPrefixes": [], | ||
| "destinationAddressPrefixes": [] | ||
| } |
There was a problem hiding this comment.
There are duplicated security rule entries: the NSG resources include inline securityRules (e.g., webservers NSG at lines 112–173 includes Allow-HTTP and Allow-HTTPS), and later the template also contains separate Microsoft.Network/networkSecurityGroups/securityRules child resources (starting at line 213 and onward). This duplication is confusing and may lead to unexpected template behavior. Consider keeping rules inline in the NSG resource OR as child resources — not both — to avoid ambiguity and simplify validation.
| -DestinationAddressPrefix VirtualNetwork -DestinationPortRange * | ||
|
|
||
| # Allow HTTP from Internet | ||
| $allowHttp = New-AzNetworkSecurityRuleConfig -Name "Allow-HTTP" -Description "Allow HTTP" ` |
There was a problem hiding this comment.
Positive: HTTP allow rule is defined for webservers with SourceAddressPrefix Internet and DestinationPortRange 80 — this satisfies the requirement to allow HTTP from the Internet. Ensure the exported template includes this as a separate security rule so the validator can find it.
| -DestinationAddressPrefix * -DestinationPortRange 80 | ||
|
|
||
| # Allow HTTPS from Internet | ||
| $allowHttps = New-AzNetworkSecurityRuleConfig -Name "Allow-HTTPS" -Description "Allow HTTPS" ` |
There was a problem hiding this comment.
Positive: HTTPS allow rule is defined for webservers with SourceAddressPrefix Internet and DestinationPortRange 443 — this satisfies the requirement to allow HTTPS from the Internet. Together with the Allow-VNet rule, webservers NSG should have three inbound rules.
| -DestinationAddressPrefix VirtualNetwork -DestinationPortRange * | ||
|
|
||
| # Allow SSH from Internet | ||
| $allowSsh = New-AzNetworkSecurityRuleConfig -Name "Allow-SSH" -Description "Allow SSH" ` |
There was a problem hiding this comment.
Positive: SSH allow rule is defined for management with SourceAddressPrefix Internet and DestinationPortRange 22. This meets the requirement that management accepts SSH from the Internet. Ensure the validation script checks the sourceAddressPrefix equals 'Internet' (validator in repo needs that check enabled).
| $webSubnet = New-AzVirtualNetworkSubnetConfig -Name $webSubnetName -AddressPrefix $webSubnetIpRange -NetworkSecurityGroup $webnsg | ||
| $dbSubnet = New-AzVirtualNetworkSubnetConfig -Name $dbSubnetName -AddressPrefix $dbSubnetIpRange -NetworkSecurityGroup $dbnsg | ||
| $mngSubnet = New-AzVirtualNetworkSubnetConfig -Name $mngSubnetName -AddressPrefix $mngSubnetIpRange -NetworkSecurityGroup $mngnsg |
There was a problem hiding this comment.
Subnet association: NSG objects are attached to the subnet configs via the -NetworkSecurityGroup parameter when creating the subnets ($webSubnet, $dbSubnet, $mngSubnet) and then New-AzVirtualNetwork is called with those subnets. This correctly associates NSGs with subnets. After fixing NSG names above, this association should remain valid.
|
|
||
| $storageKey = (Get-AzStorageAccountKey -ResourceGroupName $resourceGroupName -Name $storageName)[0].Value | ||
| $ctx = New-AzStorageContext -StorageAccountName $storageName -StorageAccountKey $storageKey | ||
| New-AzStorageContainer -Name "task-artifacts" -Context $ctx -Permission Off No newline at end of file |
There was a problem hiding this comment.
Artifacts container: the script creates the storage container task-artifacts (New-AzStorageContainer -Name "task-artifacts"). This matches the expected container name in artifacts.json — ensure the SAS URL in artifacts.json references the same container and is valid. If other files/templates include a misspelled container (task-atrifacts), remove that misspelling so artifact generation/validation works.
2 participants
No description provided.