Azure / oav

Tools for validating OpenAPI (Swagger) files.
MIT License
96 stars 55 forks source link

Handle duplicate models from different files #280

Open lmazuel opened 6 years ago

lmazuel commented 6 years ago

This raise an OAV problem: https://github.com/Azure/azure-rest-api-specs/pull/3495

Swagger

    "Zone": {
      "properties": {
        "etag": {
          "type": "string",
          "description": "The etag of the zone."
        },
        "properties": {
          "x-ms-client-flatten": true,
          "$ref": "#/definitions/ZoneProperties",
          "description": "The properties of the zone."
        }
      },
      "allOf": [
        {
          "$ref": "#/definitions/Resource"
        }
      ],
      "description": "Describes a DNS zone."
    },
    "Resource": {
      "description": "The Resource model definition.",
      "properties": {
        "id": {
          "readOnly": true,
          "type": "string",
          "description": "Resource Id"
        },
        "name": {
          "readOnly": true,
          "type": "string",
          "description": "Resource name"
        },
        "type": {
          "readOnly": true,
          "type": "string",
          "description": "Resource type"
        },
        "location": {
          "type": "string",
          "description": "Resource location"
        },
        "tags": {
          "type": "object",
          "additionalProperties": {
            "type": "string"
          },
          "description": "Resource tags"
        }
      },
      "required": [
        "location"
      ],
      "x-ms-azure-resource": true
    },

JSON to match:

      {
        "id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Network/dnsZones/zone1",
        "etag": "00000000-0000-0000-0000-000000000000",
        "location": "global",
        "name": "zone1",
        "type": "Microsoft.Network/dnsZones",
        "properties": {
          "maxNumberOfRecordSets": 5000,
          "numberOfRecordSets": 1,
          "nameServers": [],
          "zoneType": "Private",
          "registrationVirtualNetworks": [
            {
              "id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Network/virtualNetworks/vnet1"
            }
          ],
          "resolutionVirtualNetworks": [
            {
              "id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Network/virtualNetworks/vnet2"
            }
          ]
        },
        "tags": {
          "key1": "value1"
        }
      }

Creates:

 error: 
operationId: Zones_CreateOrUpdate
scenario: Create zone
source: request
responseCode: ALL
severity: 0
errorCode: OBJECT_ADDITIONAL_PROPERTIES
errorDetails:
  code: OBJECT_ADDITIONAL_PROPERTIES
  params:
    - - tags
      - location
  message: 'Additional properties not allowed: tags,location'
  path: ''
  description: Describes a DNS zone.
vladbarosan commented 6 years ago

Had a look at this and the issue has nothing to do with the allOf and the properties being together or not ( although they should ).

The issue comes form the fact that "Resource" is defined twice. Once in the spec itself and once in the https://github.com/Azure/azure-rest-api-specs/blob/master/specification/common-types/resource-management/v1/types.json file. Since the second one is loaded when resolving references it overwrites the first one and it looses the 2 extra fields.

Solution is to reference the "Resource" from types and remove it from this spec. This is to be noted for any spec that refers types.json, they should not overwrite any of the params there.

lmazuel commented 6 years ago

@vladbarosan should we close this as invalid then?

vladbarosan commented 6 years ago

@lmazuel actually lets keep it open, ill rename to reflect that we might need to either fail on duplicate models or have a way to handle them