Skip to content

Commit

Permalink
Fixing VMSize Test: No Longer Checking for top-level parameter (#710)
Browse files Browse the repository at this point in the history
* Fixing VMSize Test:  No Longer Checking for top-level parameter (Fixes #695)

* Allowing VMSize to be in any parameter, not just top-level parameters

* Adding VMSize-With-Alternate-Parameter-Name passing test

* Updating VMSize test:  Not considering resource parameters as valid values

* Removing NestedVMSize failure test

* Update VMSize-With-Alternate-Parameter-Name.json

Co-authored-by: James Brundage <@github.com>
Co-authored-by: Brian Moore <[email protected]>
  • Loading branch information
StartAutomating and bmoore-msft authored Nov 11, 2022
1 parent 7f568a1 commit 4da6d95
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,26 @@ foreach ($vmSizeObject in $vmSizes) {

$vmSize = $vmSizeObject.vmSize
# If the vmSize was in a parameter, we will want to continue
if ($vmSizeObject.JSONPath -match '^parameters\.vmsize$') {
if ($vmSizeObject.JSONPath -match '^parameters' -or
$vmSizeObject.JSONPath -match '\.parameters\.') {
# but not before we check if it was an inner template
if ($vmSizeObject.ParentObject[0].expressionEvaluationOptions.scope -eq 'inner') {
# If it was an inner template, check to make sure that the inner template contains a vmSize
if (-not $vmSize.Value | ?<ARM_Parameter> -Parameter vmSize) {
Write-Error "Nested template parameter vmSize does not map to vmSize parameter" -TargetObject $vmSizeObject
if (-not ($vmSize.Value | ?<ARM_Parameter>)) {
Write-Error "Nested template parameter vmSize does not map to a parameter" -TargetObject $vmSizeObject
continue
}
} else {

# Otherwise, make note of the fact that we have a parameter called VMSize
$vmSizeTopLevelParameterDeclared = $true
}
continue

# If the VMSize was in the properties passed to a template, it is not declaring the parameter.
# Therefore, if we determine that we are examining properties, we should validate this as any other VMSize parameter.
if ($vmSizeObject.JSONPath -notmatch 'resources\[\d+\]\.properties\.parameters') {
continue
}
}

# The only other places we should find VMSizes are in resources.
Expand All @@ -55,7 +62,7 @@ foreach ($vmSizeObject in $vmSizes) {
$resourceName = $vmSizeObject.ParentObject.name


if (-not $vmSize | ?<ARM_Parameter>) { # If the VMSize does not have a parameter reference
if (-not ($vmSize | ?<ARM_Parameter>)) { # If the VMSize does not have a parameter reference
if ($vmSize | ?<ARM_Variable>) { # but does have a variable reference
# try expanding the variable
$resolvedVmSize = Expand-AzTemplate -Expression $vmSize -InputObject $OriginalTemplateObject
Expand All @@ -70,9 +77,4 @@ foreach ($vmSizeObject in $vmSizes) {
Write-Error "VM Size for resourceType '$resourceType' named '$resourceName' must be a parameter" -TargetObject $vm
}
}
}

if ($vmSizes -and -not $vmSizeTopLevelParameterDeclared) {
Write-Error "VMSize parameter must be declared for the parent template"
return
}
102 changes: 0 additions & 102 deletions unit-tests/VM-Size-Should-Be-A-Parameter/Fail/NestedVMSize.json

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"parameters": {
"nodeSku": {}
},
"resources": [
{
"type": "Microsoft.MachineLearningServices/workspaces/computes",
"apiVersion": "2018-11-19",
"name": "[concat(parameters('workspaceName'), '/gpu-cluster')]",
"location": "[parameters('location')]",
"properties": {
"computeType": "AmlCompute",
"computeLocation": "[parameters('location')]",
"description": "gpu nodes",
"properties": {
"remoteLoginPortPublicAccess": "Enabled",
"scaleSettings": {
"maxNodeCount": 4,
"minNodeCount": 0
},
"subnet": {
"id": "[variables('subnet')]"
},
"vmSize": "[parameters('nodeSku')]"
}
}
}
]
}

0 comments on commit 4da6d95

Please sign in to comment.