Openapi schema inconsistencies #1422

UnityDG · 2022-02-25T13:12:06Z

Environment

Python version: 3.9
Nautobot version: 1.2.4

Found a few mismatches between the defined schema and the returned data when generating bindings for go with openapi-generator-cli

Sample URL: /api/dcim/interfaces/XXX/
Required data: Untaagged vlans applied to interface
Sample Definition: VmInterface + Interface:
Field: tagged_vlans
Schema type: string[] of uuids
Actually returned: NestedVLAN[]

  "tagged_vlans": [
    {
      "id": "89913e46-82b4-5ced-9b54-a459b8777b3a",
      "url": "http://localhost:8080/api/ipam/vlans/XX/",
      "vid": 10,
      "name": "XX",
      "display": "XX"
    },
    {
      "id": "ec495efc-1f11-586e-88b9-427ed1659717",
      "url": "http://localhost:8080/api/ipam/vlans/XX/",
      "vid": 20,
      "name": "XX",
      "display": "XX"
    }
  ],

Sample URL: /api/dcim/interfaces/XXX/
Required data: Connection between two interfaces
Sample Definition: Anywhere cable_peer / connected_endpoint is used
Field: cable_peer + connected_endpoint
Schema type: nullable string
Actually returned: NestedInterface?

  "cable_peer": {
    "id": "4ae220eb-9161-526e-8ecf-9a338b3c0d0c",
    "url": "http://localhost:8080/api/dcim/interfaces/4ae220eb-9161-526e-8ecf-9a338b3c0d0c/",
    "device": {
      "id": "a1a6fc59-8edb-52fd-8177-f08abc23bf7c",
      "url": "http://localhost:8080/api/dcim/devices/a1a6fc59-8edb-52fd-8177-f08abc23bf7c/",
      "name": "X",
      "display": "X"
    },
    "name": "swp51",
    "cable": "61d5e1e0-66a4-55b7-8209-001bbab4df1d",
    "display": "swp51"
  },
  "cable_peer_type": "dcim.interface",
  "connected_endpoint": {
    "id": "4ae220eb-9161-526e-8ecf-9a338b3c0d0c",
    "url": "http://localhost:8080/api/dcim/interfaces/4ae220eb-9161-526e-8ecf-9a338b3c0d0c/",
    "device": {
      "id": "a1a6fc59-8edb-52fd-8177-f08abc23bf7c",
      "url": "http://localhost:8080/api/dcim/devices/a1a6fc59-8edb-52fd-8177-f08abc23bf7c/",
      "name": "X",
      "display": "X"
    },
    "name": "swp51",
    "cable": "61d5e1e0-66a4-55b7-8209-001bbab4df1d",
    "display": "swp51"
  },
  "connected_endpoint_type": "dcim.interface",

Sample URL: /api/dcim/devices/X/
Required data: Device with a status defined.
Sample Definition: Anywhere status enum is used in a read state, DeviceWithConfigContext/status for example
Field: $X/status
Schema type: enum string
Actually returned:

  "status": {
    "value": "active",
    "label": "Active"
  },

Sample URL: /api/ipam/ip-addresses/X/
Required Data: IP Address assigned to an interface
Sample Definition: (Writable)IPAddress
Field: assigned_object
Schema type: nullable string
Actually returned: NestedInterface?

  "assigned_object_type": "dcim.interface",
  "assigned_object_id": "4086c7c8-4b12-52c3-8983-8e59d829d809",
  "assigned_object": {
    "id": "4086c7c8-4b12-52c3-8983-8e59d829d809",
    "url": "http://localhost:8080/api/dcim/interfaces/4086c7c8-4b12-52c3-8983-8e59d829d809/",
    "device": {
      "id": "1e13a364-32cd-5d68-9572-488853189f93",
      "url": "http://localhost:8080/api/dcim/devices/1e13a364-32cd-5d68-9572-488853189f93/",
      "name": "X",
      "display": "X"
    },
    "name": "eth0",
    "cable": "ba53d5df-01c9-5559-9aad-7e5eb317f09d",
    "display": "eth0"
  },

Sample URL: /api/dcim/devices/X/
Required data: Device/VM with config context data.
Sample Definition: (Writable)DeviceWithConfigContext, (Writable)VirtualMachineWithConfigContext
Field: config_context
Schema type: map[string]string
Actually returned: map[string]interface{}

Sample URL: /api/dcim/cables/X/
Required data: Connection between two interfaces
Sample Definition: Cable
Field: termination_a+b
Schema type: nullable string
Actually returned: NestedInterface?

  "termination_a_type": "dcim.consoleport",
  "termination_a_id": "063a2634-ccef-5452-8c8d-0c382ae09c09",
  "termination_a": {
    "id": "063a2634-ccef-5452-8c8d-0c382ae09c09",
    "url": "http://localhost:8080/api/dcim/console-ports/063a2634-ccef-5452-8c8d-0c382ae09c09/",
    "device": {
      "id": "4616541b-aeec-584f-b45e-d06a3a6b0ab3",
      "url": "http://localhost:8080/api/dcim/devices/4616541b-aeec-584f-b45e-d06a3a6b0ab3/",
      "name": "X",
      "display": "X"
    },
    "name": "X",
    "cable": "b09fb92e-bc81-55d9-a6e5-ff7e87c4996b",
    "display": "X"
  },
  "termination_b_type": "dcim.consoleserverport",
  "termination_b_id": "0cc4023f-602f-5568-a043-d19c17bee315",
  "termination_b": {
    "id": "0cc4023f-602f-5568-a043-d19c17bee315",
    "url": "http://localhost:8080/api/dcim/console-server-ports/0cc4023f-602f-5568-a043-d19c17bee315/",
    "device": {
      "id": "15f232bf-5021-5807-b555-99e6d9c117b2",
      "url": "http://localhost:8080/api/dcim/devices/15f232bf-5021-5807-b555-99e6d9c117b2/",
      "name": "X",
      "display": "X"
    },
    "name": "X",
    "cable": "b09fb92e-bc81-55d9-a6e5-ff7e87c4996b",
    "display": "X"
  },

The text was updated successfully, but these errors were encountered:

UnityDG · 2022-02-25T13:14:54Z

Currently I am applying these patches with jsonpatch to the schema before generating the bindings to get a working binding.
patches.zip (Likely not the best patchset!)

UnityDG · 2022-02-28T11:53:11Z

Similar issue with local_context_data being a string instead of a map[string]interface{}

json: cannot unmarshal object into Go struct field DeviceWithConfigContext.results.local_context_data of type string

bryanculver · 2022-03-15T19:48:28Z

Related to #595, #40

bryanculver · 2022-03-16T19:17:01Z

We’re run aground on a few other items that has us considering drf-spectactular which will require cleanup efforts of its own. This is blocked on the pending decision if that is a route we are going to take, potentially something else, or ultimately patch our way through our challenges with drf-yasg.

glennmatthews · 2022-03-30T21:15:27Z

FYI @UnityDG - from a quick review of the generated schema, I think that many of the specific issues reported here are likely addressed by #1544. If you have time and interest, it would be very helpful if you could give the updated code a try and let us know what remaining issues you see.

UnityDG · 2022-04-01T09:33:16Z

FYI @UnityDG - from a quick review of the generated schema, I think that many of the specific issues reported here are likely addressed by #1544. If you have time and interest, it would be very helpful if you could give the updated code a try and let us know what remaining issues you see.

I will try and take a look over the next week or so, quite busy lately however.

UnityDG · 2022-05-03T12:43:31Z

Hi @glennmatthews,

Attempted to generate from both 1.3.2 and 1.3.3's schema,
Both resulted in the following validation issue.

 | Error count: 1, Warning count: 0
Errors:
        -attribute components.schemas.JobInput.default is not of type `object`

Ignoring the validation error allows it to generate (Although its not happy)
https://gist.github.com/UnityDG/5a1cb88c613e742a7cde61d06186b531

From a quick look it appears that at least some of the issues still apply, will go through it in more depth soon.

chadell · 2022-05-03T13:30:28Z

Hi @UnityDG ,
How are you generating it?

I was trying this successfully:

wget --tries=5  https://demo.nautobot.com/api/swagger.yaml?api_version=1.3 -O swagger.yaml
oapi-codegen -generate client -o nautobot.go -package nautobot swagger.yaml
oapi-codegen -generate types -o types.go -package nautobot swagger.yaml

UnityDG · 2022-05-03T13:34:42Z

Generating a full client with the following (With 1.3.3 running locally)

curl -o swagger.json http://localhost:8080/api/swagger.json
docker pull openapitools/openapi-generator-cli:latest
docker run --rm -v "$PWD:/local" openapitools/openapi-generator-cli:latest \
	generate \
	-i /local/swagger.json \
	-g go \
	-o /local/nautobot/ \
	--package-name=nautobot

UnityDG · 2022-05-03T13:53:14Z

As discussed on slack,

Changing JobInput.properties.data.default = {}
resolves the validation issue

glennmatthews · 2022-05-03T13:54:47Z

Corresponding code change here would be:

diff --git a/nautobot/extras/api/serializers.py b/nautobot/extras/api/serializers.py
index 9ccb18124..2c5393c4f 100644
--- a/nautobot/extras/api/serializers.py
+++ b/nautobot/extras/api/serializers.py
@@ -786,7 +786,7 @@ class JobClassDetailSerializer(JobClassSerializer):  # pylint: disable=abstract-
 
 
 class JobInputSerializer(serializers.Serializer):  # pylint: disable=abstract-method
-    data = serializers.JSONField(required=False, default="")
+    data = serializers.JSONField(required=False, default={})
     commit = serializers.BooleanField(required=False, default=None)
     schedule = NestedScheduledJobSerializer(required=False)

glennmatthews · 2022-05-03T15:38:25Z

It might also be interesting to test setting SPECTACULAR_SETTINGS["COMPONENT_SPLIT_REQUEST"] = True or SPECTACULAR_SETTINGS["COMPONENT_NO_READ_ONLY_REQUIRED"] = True in nautobot_config.py and see whether those improve the usability of the generated schema - from some quick evaluation on my end both look like improvements, specifically around the handling of fields like id and url which are required=True but also read_only=True.

glennmatthews · 2022-09-09T14:59:31Z

#2398 adds COMPONENT_SPLIT_REQUEST and COMPONENT_SPLIT_PATCH to the default drf-spectacular settings as well as a number of other fixes/enhancements to bulk-operation schema representation.

glennmatthews · 2022-09-09T15:01:10Z

#2092 corrected the issue with JobInputSerializer.data's default value.

bryanculver · 2023-02-14T20:41:52Z

@nautobot/core Do we know how many of these issues remain?

glennmatthews · 2023-02-14T21:00:22Z

This should be greatly improved after #2092 and #2398 (and #3215 for 2.0). My preference would be to have someone spend a few minutes to review the specific issues reported in the original submission, and if still present, get individual issues opened for each such item.

Related issues:

glennmatthews · 2023-02-24T15:44:10Z

Tested with https://demo.nautobot.com/api/docs/?api_version=1.5 (current version: 1.5.7)

Sample URL: /api/dcim/interfaces/XXX/
Required data: Untagged vlans applied to interface
Sample Definition: VmInterface + Interface:
Field: tagged_vlans
Schema type: string[] of uuids
Actually returned: NestedVLAN[]
  "tagged_vlans": [
    {
      "id": "89913e46-82b4-5ced-9b54-a459b8777b3a",
      "url": "http://localhost:8080/api/ipam/vlans/XX/",
      "vid": 10,
      "name": "XX",
      "display": "XX"
    },
    {
      "id": "ec495efc-1f11-586e-88b9-427ed1659717",
      "url": "http://localhost:8080/api/ipam/vlans/XX/",
      "vid": 20,
      "name": "XX",
      "display": "XX"
    }
  ],

For the GET case, the schema is now showing nested VLANs for both untagged_vlan and tagged_vlans:

For the POST case, the request schema shows UUIDs (a limitation of the current nested-serializer implementation) but the response schema shows nested VLANs:

Sample URL: /api/dcim/interfaces/XXX/
Required data: Connection between two interfaces
Sample Definition: Anywhere cable_peer / connected_endpoint is used
Field: cable_peer + connected_endpoint
Schema type: nullable string Actually returned: NestedInterface?

  "cable_peer": {
    "id": "4ae220eb-9161-526e-8ecf-9a338b3c0d0c",
    "url": "http://localhost:8080/api/dcim/interfaces/4ae220eb-9161-526e-8ecf-9a338b3c0d0c/",
    "device": {
      "id": "a1a6fc59-8edb-52fd-8177-f08abc23bf7c",
      "url": "http://localhost:8080/api/dcim/devices/a1a6fc59-8edb-52fd-8177-f08abc23bf7c/",
      "name": "X",
      "display": "X"
    },
    "name": "swp51",
    "cable": "61d5e1e0-66a4-55b7-8209-001bbab4df1d",
    "display": "swp51"
  },
  "cable_peer_type": "dcim.interface",
  "connected_endpoint": {
    "id": "4ae220eb-9161-526e-8ecf-9a338b3c0d0c",
    "url": "http://localhost:8080/api/dcim/interfaces/4ae220eb-9161-526e-8ecf-9a338b3c0d0c/",
    "device": {
      "id": "a1a6fc59-8edb-52fd-8177-f08abc23bf7c",
      "url": "http://localhost:8080/api/dcim/devices/a1a6fc59-8edb-52fd-8177-f08abc23bf7c/",
      "name": "X",
      "display": "X"
    },
    "name": "swp51",
    "cable": "61d5e1e0-66a4-55b7-8209-001bbab4df1d",
    "display": "swp51"
  },
  "connected_endpoint_type": "dcim.interface",

cable_peer and connected_endpoint are currently showing in the UI as empty dicts. Checking the schema YAML I see:

        cable_peer:
          type: object
          additionalProperties: {}
          nullable: true
          readOnly: true
        connected_endpoint:
          type: object
          additionalProperties: {}
          nullable: true
          readOnly: true

This probably needs work (TODO), though this may be tricky to represent in the schema since these are GenericForeignKeys in the data model and so could be one of several different data types and serializers.

Sample URL: /api/dcim/devices/X/
Required data: Device with a status defined.
Sample Definition: Anywhere status enum is used in a read state, DeviceWithConfigContext/status for example
Field: $X/status Schema type: enum string
Actually returned:
  "status": {
    "value": "active",
    "label": "Active"
  },

This is fixed in 2.0 by #3225

Sample URL: /api/ipam/ip-addresses/X/
Required Data: IP Address assigned to an interface
Sample Definition: (Writable)IPAddress Field: assigned_object
Schema type: nullable string
Actually returned: NestedInterface?

  "assigned_object_type": "dcim.interface",
  "assigned_object_id": "4086c7c8-4b12-52c3-8983-8e59d829d809",
  "assigned_object": {
    "id": "4086c7c8-4b12-52c3-8983-8e59d829d809",
    "url": "http://localhost:8080/api/dcim/interfaces/4086c7c8-4b12-52c3-8983-8e59d829d809/",
    "device": {
      "id": "1e13a364-32cd-5d68-9572-488853189f93",
      "url": "http://localhost:8080/api/dcim/devices/1e13a364-32cd-5d68-9572-488853189f93/",
      "name": "X",
      "display": "X"
    },
    "name": "eth0",
    "cable": "ba53d5df-01c9-5559-9aad-7e5eb317f09d",
    "display": "eth0"
  },

Another case (similar to 2. above) where assigned_object is a GenericForeignKey and will need special handling (TODO). Currently the schema type is just object:

        assigned_object:
          type: object
          additionalProperties: {}
          nullable: true
          readOnly: true

Sample URL: /api/dcim/devices/X/
Required data: Device/VM with config context data.
Sample Definition: (Writable)DeviceWithConfigContext, (Writable)VirtualMachineWithConfigContext
Field: config_context
Schema type: map[string]string
Actually returned: map[string]interface{}

This seems incorrect to me - it should be just an arbitrary object (dict) of JSON data. The schema is currently generic:

        config_context:
          type: object
          additionalProperties: {}
          readOnly: true

Can anyone provide sample "Actually returned" data showing the issue here?

Sample URL: /api/dcim/cables/X/
Required data: Connection between two interfaces
Sample Definition: Cable
Field: termination_a+b
Schema type: nullable string
Actually returned: NestedInterface?

  "termination_a_type": "dcim.consoleport",
  "termination_a_id": "063a2634-ccef-5452-8c8d-0c382ae09c09",
  "termination_a": {
    "id": "063a2634-ccef-5452-8c8d-0c382ae09c09",
    "url": "http://localhost:8080/api/dcim/console-ports/063a2634-ccef-5452-8c8d-0c382ae09c09/",
    "device": {
      "id": "4616541b-aeec-584f-b45e-d06a3a6b0ab3",
      "url": "http://localhost:8080/api/dcim/devices/4616541b-aeec-584f-b45e-d06a3a6b0ab3/",
      "name": "X",
      "display": "X"
    },
    "name": "X",
    "cable": "b09fb92e-bc81-55d9-a6e5-ff7e87c4996b",
    "display": "X"
  },
  "termination_b_type": "dcim.consoleserverport",
  "termination_b_id": "0cc4023f-602f-5568-a043-d19c17bee315",
  "termination_b": {
    "id": "0cc4023f-602f-5568-a043-d19c17bee315",
    "url": "http://localhost:8080/api/dcim/console-server-ports/0cc4023f-602f-5568-a043-d19c17bee315/",
    "device": {
      "id": "15f232bf-5021-5807-b555-99e6d9c117b2",
      "url": "http://localhost:8080/api/dcim/devices/15f232bf-5021-5807-b555-99e6d9c117b2/",
      "name": "X",
      "display": "X"
    },
    "name": "X",
    "cable": "b09fb92e-bc81-55d9-a6e5-ff7e87c4996b",
    "display": "X"
  },

Another GenericForeignKey currently represented as a generic object in the schema. TODO.

Summary

Appears resolved now (likely by Fixes to OpenAPI schema for bulk API operations #2398)
Schema needs to account for GenericForeignKey if possible
Fixed in 2.0 by Revamp REST API "status" fields #3225
Schema needs to account for GenericForeignKey if possible
More information required, current schema appears correct
Schema needs to account for GenericForeignKey if possible

* Fix #1422: add polymorphic types for serializer GenericForeignKeys * Fix linting and unit-test * Update nautobot/extras/api/nested_serializers.py * Cable.termination_a and .termination_b are never null * Remove some code that turned out to be unneeded for the schema * More schema corrections, disambiguate component names * Add framework for schema testing and some sample tests for dcim schemas * Rework get_all_concrete_foo implementation for reliability * Change lookup functions to return a list rather than yielding individual items * Extract common helper methods from schema tests, add tests for remaining poly endpoints, fix schema for cablepath.path * Add validate_polymorphic_property test method * Remove callable serializer support as it was moved upstream. --------- Co-authored-by: Bryan Culver <bryan.culver@networktocode.com>

glennmatthews · 2023-03-08T14:48:38Z

Resolved for 2.0 by #3368. Please feel free to open new issues for any other specific gaps in the OpenAPI schema.

glennmatthews added type: bug Something isn't working as expected workflow: under review labels Feb 25, 2022

bryanculver added workflow: future status: blocked Another issue or external requirement is preventing implementation and removed workflow: under review labels Mar 15, 2022

glennmatthews mentioned this issue Mar 24, 2022

Upgrade OpenAPI/Swagger support to OpenAPI 3.0 #595

Closed

5 tasks

glennmatthews removed status: blocked Another issue or external requirement is preventing implementation workflow: future labels Apr 5, 2022

glennmatthews added the question Further information is requested label Apr 20, 2022

glennmatthews mentioned this issue Aug 10, 2022

Closes #2131: Fix various issues with the available-ips/prefixes API … #2172

Closed

2 tasks

glennmatthews self-assigned this Feb 24, 2023

glennmatthews added a commit that referenced this issue Feb 24, 2023

Fix #1422: add polymorphic types for serializer GenericForeignKeys

857caa9

glennmatthews mentioned this issue Feb 24, 2023

Add polymorphic types for serializer GenericForeignKeys #3368

Merged

6 tasks

glennmatthews linked a pull request Feb 24, 2023 that will close this issue

Add polymorphic types for serializer GenericForeignKeys #3368

Merged

6 tasks

glennmatthews removed the question Further information is requested label Feb 27, 2023

bryanculver added this to the v2.0.0 milestone Mar 3, 2023

bryanculver added impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release and removed impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release labels Mar 3, 2023

glennmatthews closed this as completed Mar 8, 2023

github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Openapi schema inconsistencies #1422

Openapi schema inconsistencies #1422

UnityDG commented Feb 25, 2022 •

edited

Loading

UnityDG commented Feb 25, 2022

UnityDG commented Feb 28, 2022 •

edited

Loading

bryanculver commented Mar 15, 2022

bryanculver commented Mar 16, 2022

glennmatthews commented Mar 30, 2022

UnityDG commented Apr 1, 2022

UnityDG commented May 3, 2022 •

edited

Loading

chadell commented May 3, 2022

UnityDG commented May 3, 2022

UnityDG commented May 3, 2022 •

edited

Loading

glennmatthews commented May 3, 2022

glennmatthews commented May 3, 2022

glennmatthews commented Sep 9, 2022

glennmatthews commented Sep 9, 2022

bryanculver commented Feb 14, 2023

glennmatthews commented Feb 14, 2023

glennmatthews commented Feb 24, 2023

glennmatthews commented Mar 8, 2023

Openapi schema inconsistencies #1422

Openapi schema inconsistencies #1422

Comments

UnityDG commented Feb 25, 2022 • edited Loading

Environment

UnityDG commented Feb 25, 2022

UnityDG commented Feb 28, 2022 • edited Loading

bryanculver commented Mar 15, 2022

bryanculver commented Mar 16, 2022

glennmatthews commented Mar 30, 2022

UnityDG commented Apr 1, 2022

UnityDG commented May 3, 2022 • edited Loading

chadell commented May 3, 2022

UnityDG commented May 3, 2022

UnityDG commented May 3, 2022 • edited Loading

glennmatthews commented May 3, 2022

glennmatthews commented May 3, 2022

glennmatthews commented Sep 9, 2022

glennmatthews commented Sep 9, 2022

bryanculver commented Feb 14, 2023

glennmatthews commented Feb 14, 2023

glennmatthews commented Feb 24, 2023

Summary

glennmatthews commented Mar 8, 2023

UnityDG commented Feb 25, 2022 •

edited

Loading

UnityDG commented Feb 28, 2022 •

edited

Loading

UnityDG commented May 3, 2022 •

edited

Loading

UnityDG commented May 3, 2022 •

edited

Loading