Azure / arm-ttk

Azure Resource Manager Template Toolkit
https://aka.ms/arm-ttk
MIT License
436 stars 184 forks source link

Managed app ARM templates failing while validation with latest version of ARM-TTK #703

Open sunnybhambhani opened 1 year ago

sunnybhambhani commented 1 year ago

Hi Team, Hope you are doing good.

Since the upgrade of ARM-TTK our template validation is failing. If I remember it correctly with v0.15 I never encountered this issue.

Also, I believe the reason is wrong or misleading since we already are using the practices which are shared over the documentation.

Ref: https://learn.microsoft.com/en-us/azure/azure-resource-manager/templates/template-test-cases Section: VM size uses parameter

Error:

  VM Size Should Be A Parameter                                                                                         
    [-] VM Size Should Be A Parameter (140 ms)                                                                          
        VMSize parameter must be declared for the parent template 

We are already using vmSize as a parameter, please find below snippets from our respective arm templates:

createUiDefinition.json [Ref: https://learn.microsoft.com/en-us/azure/azure-resource-manager/managed-applications/microsoft-compute-sizeselector]

            {
                "name": "nodeSize",
                "type": "Microsoft.Compute.SizeSelector",
                "label": "Size",
                "toolTip": "The size of VMs to be used as worker nodes in the AKS cluster",
                "recommendedSizes": [
                    "Standard_B4ms",
                    "Standard_DS3_v2",
                    "Standard_D4s_v3"
                ],
                "options": {
                    "hideDiskTypeFilter": false
                },
                "osPlatform": "Linux",
                "imageReference": {
                    "publisher": "Canonical",
                    "offer": "UbuntuServer",
                    "sku": "18.04"
                },
                "count": 1,
                "visible": true
            },

mainTemplate's parameters:

    "nodeSize": {
      "type": "string",
      "defaultValue": "Standard_B4ms",
      "metadata": {
        "description": "Node Size"
      }
    },

mainTemplate's resources (this is a part of AKS agent pool):

        "agentPoolProfiles": [
          {
            "name": "agentpool",
            "count": "[parameters('nodeCount')]",
            "vmSize": "[parameters('nodeSize')]",
            "enableAutoScaling": true,
            "minCount": "[parameters('nodeCount')]",
            "maxCount": "[parameters('nodeMaxCount')]",
            "osType": "Linux",
            "storageProfile": "ManagedDisks",
            "type": "VirtualMachineScaleSets",
            "mode": "System",
            "vnetSubnetID": "[variables('aksSubnetId')]",
            "maxPods": 50
          }
        ],

Could you please suggest OR correct me if I am doing something wrong?

Thank you, Sunny

sunnybhambhani commented 1 year ago

If a full copy of ARM is required, the sample can be found on below URL, since that too is failing and there as well we have vmSize as a parameter:

https://learn.microsoft.com/en-us/azure/aks/learn/quick-kubernetes-deploy-rm-template?tabs=azure-cli Section: Review the template

OR here: https://github.com/Azure/azure-quickstart-templates/blob/master/quickstarts/microsoft.kubernetes/aks/azuredeploy.json

Thanks, Sunny

bmoore-msft commented 1 year ago

Maybe another repro: https://github.com/Azure/azure-quickstart-templates/tree/master/application-workloads/darktrace-vsensor-standalone

StartAutomating commented 1 year ago

It appears we are not allowing the VMSize parameter to be anything other than VMSize:

https://github.com/Azure/arm-ttk/blob/master/arm-ttk/testcases/deploymentTemplate/VM-Size-Should-Be-A-Parameter.test.ps1#L36

I believe we should adjust this. But how? Should we require it to be named anything particular? If not, what heuristic do we use to determine if the top-level parameter is a VMSize?

bmoore-msft commented 1 year ago

It should have no naming constraints - the parameter can be named anything. We just need to verify that any resource property named vmSize has a property value that is a parameter (or a variable that references a parameter)... Keeping in mind that the param/var could be an object.

StartAutomating commented 1 year ago

@sunnybhambhani

This should be fixed by recent changes to the VMSize tests. Please confirm when you get the chance.