search

Azure / azure-resource-manager-schemas

Schemas used to author and validate Resource Manager Templates. These schemas power the intellisense and syntax completion in our ARM Tools VSCode extension, as well as the Export Template API
MIT License
609 stars 516 forks source link

Not all resources should inherit from ResourceBase #1850

Open matthchr opened 3 years ago

matthchr commented 3 years ago

Currently, all resources are joined with ResourceBase, see for example the beginning of the autogenerated spec:

  "allOf": [
    {
      "$ref": "https://schema.management.azure.com/schemas/common/definitions.json#/definitions/resourceBase"
    },
    {
      "oneOf": [
        {
          "$ref": "https://schema.management.azure.com/schemas/2017-01-01/Microsoft.AAD.json#/resourceDefinitions/domainServices"
        },
        {
          "$ref": "https://schema.management.azure.com/schemas/2017-06-01/Microsoft.AAD.json#/resourceDefinitions/domainServices"
        },
        ....

This assumes that every resource supports every property defined in ResourceBase. That has always seemed like a reasonable assumption to me, but I recently learned that it isn't correct.

For example, according to ARM tracked vs proxy resources, proxy resources do not support tags. Yet ResourceBase defines tags. There are Proxy resources in the list of resources, for example: https://schema.management.azure.com/schemas/2018-12-01/Microsoft.Batch.json#/resourceDefinitions/batchAccounts_pools is a proxy resource that doesn't support tags. Same for https://schema.management.azure.com/schemas/2020-12-01/Microsoft.ContainerService.json#/resourceDefinitions/managedClusters_agentPools

The result is that these resources look like they support tags, but it's either rejected or ignored.

Should ResourceBase only include fields that are guaranteed to be on all resources (which doesn't include tags?)

matthchr commented 3 years ago

@anthony-c-martin curious as to your thoughts on this

anthony-c-martin commented 3 years ago

@anthony-c-martin curious as to your thoughts on this

I agree - the vast majority of schemas are being automatically generated today; this generation process will pick up base resource properties as well. Generally if tags are supported by a resource type, they should be declared in the swagger, which is used to drive this generation. I don't see a huge amount of value in a shared base class which may not always be accurate in certain cases.

matthchr commented 3 years ago

Is there an easy path to changing this in the specs today (at least for autogenerated resources?)

anthony-c-martin commented 2 years ago

Implementation notes - need to collapse the allOf in the following places:

https://github.com/Azure/azure-resource-manager-schemas/blob/6cbf2a9e2485a6eca89ca7801daa5f7aafe5c390/schemas/common/autogeneratedResources.json#L8-L10 https://github.com/Azure/azure-resource-manager-schemas/blob/6cbf2a9e2485a6eca89ca7801daa5f7aafe5c390/schemas/2018-05-01/subscriptionDeploymentTemplate.json#L498-L500 https://github.com/Azure/azure-resource-manager-schemas/blob/6cbf2a9e2485a6eca89ca7801daa5f7aafe5c390/schemas/2019-08-01/managementGroupDeploymentTemplate.json#L498-L500 https://github.com/Azure/azure-resource-manager-schemas/blob/6cbf2a9e2485a6eca89ca7801daa5f7aafe5c390/schemas/2019-08-01/tenantDeploymentTemplate.json#L500-L502