From b0148c09b72487ea5cfe221020a33778711b6f4b Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Wed, 19 Aug 2026 14:09:52 -0400 Subject: [PATCH 1/4] Support YAML for the portable schema export/preview/apply feature Adds YAML as an accepted and documented-preferred interchange format for Custom Object Type schema documents, alongside JSON. - Add PyYAML as an explicit dependency (previously only an incidental transitive one via NetBox core). - Add YAMLParser/YAMLRenderer (netbox_custom_objects/api/{parsers,renderers}.py), built directly on PyYAML rather than pulling in djangorestframework-yaml. - SchemaPreviewView and SchemaApplyView now accept either Content-Type: application/yaml or application/json, and render YAML when Accept: application/yaml is sent. A request with no Accept header still gets JSON, so existing callers are unaffected. - export_cot()/export_cots() are unchanged -- they already returned a plain dict and left serialization to the caller; only the documented examples and the two API endpoints change. - Update docs/portable-schema.md examples to YAML, and clarify that cot_schema_v1.json is a format-agnostic JSON Schema validator (it validates the parsed document, not a specific wire encoding). Closes: #665 --- docs/portable-schema.md | 217 ++++++++---------- netbox_custom_objects/api/parsers.py | 17 ++ netbox_custom_objects/api/renderers.py | 16 ++ netbox_custom_objects/api/views.py | 92 ++++---- .../tests/schema/test_schema_api.py | 77 +++++++ .../tests/schema/test_yaml_codec.py | 51 ++++ pyproject.toml | 1 + 7 files changed, 308 insertions(+), 163 deletions(-) create mode 100644 netbox_custom_objects/api/parsers.py create mode 100644 netbox_custom_objects/api/renderers.py create mode 100644 netbox_custom_objects/tests/schema/test_yaml_codec.py diff --git a/docs/portable-schema.md b/docs/portable-schema.md index 5222f981..8941d07c 100644 --- a/docs/portable-schema.md +++ b/docs/portable-schema.md @@ -1,36 +1,31 @@ # Portable Schema The portable schema feature allows Custom Object Type (COT) definitions to be exported as -structured JSON documents, versioned in source control, and applied to other NetBox instances. +structured YAML documents, versioned in source control, and applied to other NetBox instances. This makes COT schemas shareable, auditable, and deployable across environments in a consistent -and repeatable way. +and repeatable way. YAML is the recommended format; JSON is also accepted (see +[Schema Document Format](#schema-document-format)). ## Concepts ### Schema Documents -A schema document is a JSON object that fully describes one or more Custom Object Types — -their names, metadata, and all field definitions. The document is self-contained: a reader -does not need access to the originating NetBox instance to understand or validate it. - -```json -{ - "schema_version": "1", - "types": [ - { - "name": "circuit", - "slug": "circuit", - "verbose_name": "Circuit", - "verbose_name_plural": "Circuits", - "description": "WAN circuit inventory", - "fields": [ - { "id": 1, "name": "carrier", "type": "text", "required": true }, - { "id": 2, "name": "bandwidth_mbps", "type": "integer", "validation_minimum": 0 } - ], - "removed_fields": [] - } - ] -} +A schema document is a mapping that fully describes one or more Custom Object Types — their +names, metadata, and all field definitions. The document is self-contained: a reader does not +need access to the originating NetBox instance to understand or validate it. + +```yaml +schema_version: "1" +types: + - name: circuit + slug: circuit + verbose_name: Circuit + verbose_name_plural: Circuits + description: WAN circuit inventory + fields: + - { id: 1, name: carrier, type: text, required: true } + - { id: 2, name: bandwidth_mbps, type: integer, validation_minimum: 0 } + removed_fields: [] ``` ### Schema IDs @@ -58,10 +53,9 @@ When a field is removed from a COT, the comparator needs to distinguish "this fi intentionally deleted" from "this field is not in the schema yet." Tombstone entries in `removed_fields` provide that signal: -```json -"removed_fields": [ - { "id": 4, "name": "legacy_carrier_code", "type": "text", "removed_in": "2.0.0" } -] +```yaml +removed_fields: + - { id: 4, name: legacy_carrier_code, type: text, removed_in: "2.0.0" } ``` A tombstone records the field's last-known `id`, `name`, `type`, and the version string when @@ -139,9 +133,12 @@ The pattern rejects: ## Schema Document Format -The JSON Schema validator for schema documents lives at -`netbox_custom_objects/schema/cot_schema_v1.json` and is used by the API endpoints to -validate incoming documents before any DB access. +Schema documents may be written in YAML (recommended) or JSON — both parse to the same +structure described below, and the [preview](#previewing-a-schema-api) and +[apply](#applying-a-schema-api) endpoints accept either. The validator for this structure lives +at `netbox_custom_objects/schema/cot_schema_v1.json`, a [JSON Schema](https://json-schema.org/) +definition — it validates the parsed document itself, independent of whether it arrived as +YAML or JSON, and is used by the API endpoints before any DB access. ### Top-Level Structure @@ -219,13 +216,11 @@ Attributes that match their defaults are omitted from exported documents to keep ### Tombstone Record -```json -{ - "id": 4, - "name": "legacy_carrier_code", - "type": "text", - "removed_in": "2.0.0" -} +```yaml +id: 4 +name: legacy_carrier_code +type: text +removed_in: "2.0.0" ``` `removed_in` is optional but recommended. The `id` value must match the original field's @@ -261,11 +256,11 @@ Then inside the shell, export specific COTs by slug: ```python from netbox_custom_objects.schema.exporter import export_cots from netbox_custom_objects.models import CustomObjectType -import json +import yaml cots = CustomObjectType.objects.filter(slug__in=["circuit", "device-profile"]) document = export_cots(cots) -print(json.dumps(document, indent=2)) +print(yaml.safe_dump(document, sort_keys=False)) ``` Or export **all** COTs at once: @@ -273,11 +268,11 @@ Or export **all** COTs at once: ```python from netbox_custom_objects.schema.exporter import export_cots from netbox_custom_objects.models import CustomObjectType -import json +import yaml cots = CustomObjectType.objects.all() document = export_cots(cots) -print(json.dumps(document, indent=2)) +print(yaml.safe_dump(document, sort_keys=False)) ``` To run a script file non-interactively, pipe it in: @@ -306,11 +301,11 @@ django.setup() # must be called before any model or app imports from netbox_custom_objects.schema.exporter import export_cots from netbox_custom_objects.models import CustomObjectType -import json +import yaml cots = CustomObjectType.objects.filter(slug__in=["circuit", "device-profile"]) document = export_cots(cots) -print(json.dumps(document, indent=2)) +print(yaml.safe_dump(document, sort_keys=False)) ``` !!! warning "Missing `django.setup()` causes `AppRegistryNotReady`" @@ -332,62 +327,49 @@ document wrapper, use `export_cot(cot)`. `POST /api/plugins/custom-objects/schema/preview/` Submit a schema document and receive a structured diff showing what would change, **without -modifying the database**: +modifying the database**. The request body and response may be YAML or JSON — YAML is +recommended and is what's shown here; send `Content-Type: application/json` (and, if you want +a JSON response, `Accept: application/json`) to use JSON instead. A request with no `Accept` +header gets a JSON response. ```http POST /api/plugins/custom-objects/schema/preview/ -Content-Type: application/json +Content-Type: application/yaml +Accept: application/yaml Authorization: Token -{ - "schema_version": "1", - "types": [ - { - "name": "circuit", - "slug": "circuit", - "verbose_name_plural": "Circuits", - "fields": [ - { "id": 1, "name": "carrier", "type": "text", "required": true }, - { "id": 3, "name": "contract_ref", "type": "text" } - ], - "removed_fields": [ - { "id": 2, "name": "bandwidth_mbps", "type": "integer", "removed_in": "2.0.0" } - ] - } - ] -} +schema_version: "1" +types: + - name: circuit + slug: circuit + verbose_name_plural: Circuits + fields: + - { id: 1, name: carrier, type: text, required: true } + - { id: 3, name: contract_ref, type: text } + removed_fields: + - { id: 2, name: bandwidth_mbps, type: integer, removed_in: "2.0.0" } ``` Response `200`: -```json -{ - "diffs": [ - { - "slug": "circuit", - "name": "circuit", - "is_new": false, - "has_changes": true, - "has_destructive_changes": true, - "cot_changes": {}, - "field_changes": [ - { - "op": "add", - "schema_id": 3, - "db_name": null, - "schema_def": { "id": 3, "name": "contract_ref", "type": "text" } - }, - { - "op": "remove", - "schema_id": 2, - "db_name": "bandwidth_mbps", - "schema_def": { "id": 2, "name": "bandwidth_mbps", "type": "integer", "removed_in": "2.0.0" } - } - ], - "warnings": [] - } - ] -} +```yaml +diffs: + - slug: circuit + name: circuit + is_new: false + has_changes: true + has_destructive_changes: true + cot_changes: {} + field_changes: + - op: add + schema_id: 3 + db_name: null + schema_def: { id: 3, name: contract_ref, type: text } + - op: remove + schema_id: 2 + db_name: bandwidth_mbps + schema_def: { id: 2, name: bandwidth_mbps, type: integer, removed_in: "2.0.0" } + warnings: [] ``` `has_destructive_changes: true` indicates that applying this schema would drop at least one @@ -399,13 +381,12 @@ column. The preview endpoint never returns `409` — it is safe to call at any t ```http POST /api/plugins/custom-objects/schema/apply/ -Content-Type: application/json +Content-Type: application/yaml +Accept: application/yaml Authorization: Token -{ - "allow_destructive": false, - "schema": { ... } -} +allow_destructive: false +schema: { ... } ``` - **`allow_destructive`** (default `false`): must be `true` for the apply to proceed when the @@ -421,37 +402,31 @@ Authorization: Token Response `200`: -```json -{ - "applied": true, - "diffs": [ ... ] -} +```yaml +applied: true +diffs: [ ... ] ``` Response `409 Conflict`: -```json -{ - "error": "destructive_changes", - "detail": "Schema contains destructive field removals for COT(s): circuit.", - "destructive_slugs": ["circuit"] -} +```yaml +error: destructive_changes +detail: "Schema contains destructive field removals for COT(s): circuit." +destructive_slugs: [circuit] ``` Response `400 Bad Request` (invalid schema, unresolvable reference, or circular COT dependency): -```json -{ - "error": "unresolvable_reference", - "detail": "..." -} +```yaml +error: unresolvable_reference +detail: "..." ``` ### Typical End-to-End Workflow 1. **Define and iterate** on COT schemas in a development environment using the NetBox UI or API. -2. **Export** the schemas to a JSON file and commit to version control. +2. **Export** the schemas to a YAML file and commit to version control. 3. **Review** the diff in the PR — because IDs are stable integers and defaults are elided, the diff is human-readable. 4. **Preview** the schema on a staging instance using the preview endpoint to confirm the diff @@ -465,15 +440,13 @@ Response `400 Bad Request` (invalid schema, unresolvable reference, or circular Fields can be marked deprecated without being removed, allowing a grace period before deletion: -```json -{ - "id": 5, - "name": "old_carrier_name", - "type": "text", - "deprecated": true, - "deprecated_since": "2.1.0", - "scheduled_removal": "3.0.0" -} +```yaml +id: 5 +name: old_carrier_name +type: text +deprecated: true +deprecated_since: "2.1.0" +scheduled_removal: "3.0.0" ``` - `deprecated: true` marks the field as read-only in the UI; no new values can be entered. diff --git a/netbox_custom_objects/api/parsers.py b/netbox_custom_objects/api/parsers.py new file mode 100644 index 00000000..3df50b29 --- /dev/null +++ b/netbox_custom_objects/api/parsers.py @@ -0,0 +1,17 @@ +import yaml + +from rest_framework.exceptions import ParseError +from rest_framework.parsers import BaseParser + + +class YAMLParser(BaseParser): + """Parses YAML request bodies for the schema preview/apply endpoints (#665).""" + + media_type = 'application/yaml' + + def parse(self, stream, media_type=None, parser_context=None): + try: + data = yaml.safe_load(stream) + except yaml.YAMLError as exc: + raise ParseError(f"YAML parse error: {exc}") + return data if data is not None else {} diff --git a/netbox_custom_objects/api/renderers.py b/netbox_custom_objects/api/renderers.py new file mode 100644 index 00000000..bf066226 --- /dev/null +++ b/netbox_custom_objects/api/renderers.py @@ -0,0 +1,16 @@ +import yaml + +from rest_framework.renderers import BaseRenderer + + +class YAMLRenderer(BaseRenderer): + """Renders responses as YAML for the schema preview/apply endpoints (#665).""" + + media_type = 'application/yaml' + format = 'yaml' + charset = 'utf-8' + + def render(self, data, accepted_media_type=None, renderer_context=None): + if data is None: + return '' + return yaml.safe_dump(data, sort_keys=False, default_flow_style=False) diff --git a/netbox_custom_objects/api/views.py b/netbox_custom_objects/api/views.py index 31b53cf8..0ac00ebf 100644 --- a/netbox_custom_objects/api/views.py +++ b/netbox_custom_objects/api/views.py @@ -18,6 +18,8 @@ class ETagMixin: # pragma: no cover – NetBox < 4.6 shim """No-op shim for NetBox versions that don't provide ETagMixin.""" pass +from rest_framework.parsers import JSONParser +from rest_framework.renderers import JSONRenderer from rest_framework.response import Response from rest_framework.routers import APIRootView from rest_framework.views import APIView @@ -25,6 +27,7 @@ class ETagMixin: # pragma: no cover – NetBox < 4.6 shim from rest_framework.exceptions import PermissionDenied, ValidationError from netbox.api.authentication import IsAuthenticatedOrLoginNotRequired, TokenWritePermission +from netbox.api.renderers import FormlessBrowsableAPIRenderer from netbox_custom_objects.constants import APP_LABEL @@ -40,6 +43,8 @@ class ETagMixin: # pragma: no cover – NetBox < 4.6 shim UnknownObjectTypeError, ) from . import serializers +from .parsers import YAMLParser +from .renderers import YAMLRenderer logger = logging.getLogger(__name__) @@ -49,6 +54,13 @@ class ETagMixin: # pragma: no cover – NetBox < 4.6 shim _SCHEMA_FILE = Path(__file__).parent.parent / "schema" / "cot_schema_v1.json" +# Schema preview/apply accept and can return either format (#665); YAML is the +# documented default for new integrations. JSONRenderer stays first so that a +# request with no explicit Accept header (existing callers) keeps getting JSON +# back; send `Accept: application/yaml` to get YAML instead. +_SCHEMA_PARSER_CLASSES = [YAMLParser, JSONParser] +_SCHEMA_RENDERER_CLASSES = [JSONRenderer, YAMLRenderer, FormlessBrowsableAPIRenderer] + @functools.lru_cache(maxsize=1) def _get_validator(): @@ -311,38 +323,36 @@ class SchemaPreviewView(APIView): ``cot_schema_v1.json``. Returns a structured diff for every COT in the document without making any DB changes. + Accepts and returns either YAML (``application/yaml``, the recommended + format) or JSON (``application/json``), selected via the ``Content-Type`` + and ``Accept`` headers respectively. A request with no ``Accept`` header + gets a JSON response. + ## Request body - { - "schema_version": "1", - "types": [ ... ] - } + schema_version: "1" + types: [ ... ] ## Response (200) - { - "diffs": [ - { - "slug": "my-cot", - "name": "my_cot", - "is_new": false, - "has_changes": true, - "has_destructive_changes": false, - "cot_changes": {"description": ["old", "new"]}, - "field_changes": [ - { - "op": "add", - "schema_id": 5, - "db_name": null, - "schema_def": { ... } - } - ], - "warnings": [] - } - ] - } + diffs: + - slug: my-cot + name: my_cot + is_new: false + has_changes: true + has_destructive_changes: false + cot_changes: + description: [old, new] + field_changes: + - op: add + schema_id: 5 + db_name: null + schema_def: { ... } + warnings: [] """ + parser_classes = _SCHEMA_PARSER_CLASSES + renderer_classes = _SCHEMA_RENDERER_CLASSES permission_classes = [IsAuthenticatedOrLoginNotRequired] def post(self, request, *args, **kwargs): @@ -361,36 +371,34 @@ class SchemaApplyView(APIView): current DB state and all changes are applied atomically. The applied diffs are returned in the response. + Accepts and returns either YAML (``application/yaml``, the recommended + format) or JSON (``application/json``), selected via the ``Content-Type`` + and ``Accept`` headers respectively. A request with no ``Accept`` header + gets a JSON response. + ## Request body - { - "allow_destructive": false, - "schema": { - "schema_version": "1", - "types": [ ... ] - } - } + allow_destructive: false + schema: + schema_version: "1" + types: [ ... ] ``allow_destructive`` defaults to ``false``. Set it to ``true`` to permit ``REMOVE`` field operations (which drop DB columns). ## Response (200) - { - "applied": true, - "diffs": [ ... ] - } + applied: true + diffs: [ ... ] ## Error responses **409 Conflict** — the document contains ``REMOVE`` operations and ``allow_destructive`` was not set: - { - "error": "destructive_changes", - "detail": "Schema contains destructive ...", - "destructive_slugs": ["my-cot"] - } + error: destructive_changes + detail: "Schema contains destructive ..." + destructive_slugs: [my-cot] **400 Bad Request** — circular COT dependency, unresolvable FK target, or invalid schema document structure. @@ -401,6 +409,8 @@ class SchemaApplyView(APIView): ``transaction.atomic()``, so any such failure leaves the DB unchanged. """ + parser_classes = _SCHEMA_PARSER_CLASSES + renderer_classes = _SCHEMA_RENDERER_CLASSES permission_classes = [IsAuthenticatedOrLoginNotRequired, TokenWritePermission] def post(self, request, *args, **kwargs): diff --git a/netbox_custom_objects/tests/schema/test_schema_api.py b/netbox_custom_objects/tests/schema/test_schema_api.py index 78b52afa..fd223fe7 100644 --- a/netbox_custom_objects/tests/schema/test_schema_api.py +++ b/netbox_custom_objects/tests/schema/test_schema_api.py @@ -10,8 +10,10 @@ - Unresolvable FK reference error (400) - Missing / malformed 'schema' key (400) - Authentication enforced (401 for unauthenticated requests) +- YAML request/response support (issue #665) """ +import yaml from django.urls import reverse from django.test import TransactionTestCase @@ -388,3 +390,78 @@ def test_apply_allow_destructive_string_returns_400(self): ) self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) self.assertIn("allow_destructive", resp.data) + + +# --------------------------------------------------------------------------- +# YAML request/response support (issue #665) +# --------------------------------------------------------------------------- + +class SchemaYAMLFormatTestCase(_SchemaAPIBase): + """POST /schema/preview/ and /schema/apply/ also accept and can return YAML.""" + + def setUp(self): + super().setUp() + self.cot = self.create_custom_object_type(name='yamlcot', slug='yaml-cot') + self.create_custom_object_type_field(self.cot, name='alpha', type='text') + perm = ObjectPermission(name='schema_yaml_cot_perm', actions=['add', 'change']) + perm.save() + perm.users.add(self.user) + perm.object_types.add(ObjectType.objects.get_for_model(CustomObjectType)) + + def test_preview_accepts_yaml_request_body(self): + type_def = export_cot(self.cot) + body = yaml.safe_dump({"schema_version": "1", "types": [type_def]}) + resp = self.client.post(self.preview_url, data=body, content_type="application/yaml") + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertIn("diffs", resp.data) + + def test_preview_response_defaults_to_json_content_type(self): + type_def = export_cot(self.cot) + resp = self.client.post( + self.preview_url, + data={"schema_version": "1", "types": [type_def]}, + format="json", + ) + self.assertIn("application/json", resp["Content-Type"]) + + def test_preview_returns_yaml_when_accepted(self): + type_def = export_cot(self.cot) + resp = self.client.post( + self.preview_url, + data={"schema_version": "1", "types": [type_def]}, + format="json", + HTTP_ACCEPT="application/yaml", + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertIn("application/yaml", resp["Content-Type"]) + parsed = yaml.safe_load(resp.content) + self.assertIn("diffs", parsed) + + def test_preview_malformed_yaml_body_returns_400(self): + resp = self.client.post(self.preview_url, data="types: [\n", content_type="application/yaml") + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + + def test_apply_accepts_yaml_request_body_and_creates_cot(self): + body = yaml.safe_dump({ + "allow_destructive": False, + "schema": {"schema_version": "1", "types": [{"name": "yamlapplied", "slug": "yaml-applied"}]}, + }) + resp = self.client.post(self.apply_url, data=body, content_type="application/yaml") + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertTrue(CustomObjectType.objects.filter(slug="yaml-applied").exists()) + + def test_apply_returns_yaml_when_accepted(self): + body = yaml.safe_dump({ + "allow_destructive": False, + "schema": {"schema_version": "1", "types": [{"name": "yamlapplied2", "slug": "yaml-applied-2"}]}, + }) + resp = self.client.post( + self.apply_url, + data=body, + content_type="application/yaml", + HTTP_ACCEPT="application/yaml", + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertIn("application/yaml", resp["Content-Type"]) + parsed = yaml.safe_load(resp.content) + self.assertTrue(parsed["applied"]) diff --git a/netbox_custom_objects/tests/schema/test_yaml_codec.py b/netbox_custom_objects/tests/schema/test_yaml_codec.py new file mode 100644 index 00000000..da266cb2 --- /dev/null +++ b/netbox_custom_objects/tests/schema/test_yaml_codec.py @@ -0,0 +1,51 @@ +""" +Unit tests for the YAML parser/renderer used by the schema preview/apply +endpoints (#665). +""" + +import io + +from django.test import SimpleTestCase +from rest_framework.exceptions import ParseError + +from netbox_custom_objects.api.parsers import YAMLParser +from netbox_custom_objects.api.renderers import YAMLRenderer + + +class YAMLParserTestCase(SimpleTestCase): + + def test_parses_valid_yaml_to_dict(self): + stream = io.BytesIO(b"schema_version: '1'\ntypes: []\n") + data = YAMLParser().parse(stream) + self.assertEqual(data, {"schema_version": "1", "types": []}) + + def test_empty_body_returns_empty_dict(self): + data = YAMLParser().parse(io.BytesIO(b"")) + self.assertEqual(data, {}) + + def test_malformed_yaml_raises_parse_error(self): + stream = io.BytesIO(b"types: [\n") + with self.assertRaises(ParseError): + YAMLParser().parse(stream) + + +class YAMLRendererTestCase(SimpleTestCase): + + def test_renders_dict_to_yaml(self): + output = YAMLRenderer().render({"diffs": [{"slug": "circuit"}]}) + self.assertEqual( + output, + "diffs:\n- slug: circuit\n", + ) + + def test_renders_none_to_empty_string(self): + self.assertEqual(YAMLRenderer().render(None), '') + + def test_round_trips_through_parser(self): + import yaml + original = {"schema_version": "1", "types": [{"name": "circuit", "slug": "circuit"}]} + rendered = YAMLRenderer().render(original) + parsed = YAMLParser().parse(io.BytesIO(rendered.encode("utf-8"))) + self.assertEqual(parsed, original) + # Sanity-check against a plain yaml.safe_load too. + self.assertEqual(yaml.safe_load(rendered), original) diff --git a/pyproject.toml b/pyproject.toml index f4e45ca9..79712882 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,6 +27,7 @@ dependencies = [ "Django", "jsonschema", "packaging", + "PyYAML", ] [project.optional-dependencies] From db18a84354fea7844ba8dbf0d2d51f95cf6a7c96 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Wed, 19 Aug 2026 18:30:29 -0400 Subject: [PATCH 2/4] Fix three issues from code review of the YAML schema-interchange endpoints - SchemaApplyView.post() called request.data.get(...) before checking request.data was a dict, so a non-dict top-level body (e.g. a YAML/JSON list) crashed with an unhandled AttributeError (500) instead of the documented 400. Now validated up front. - YAMLParser silently turned an empty body into {}, while JSONParser (used on the same endpoints) raises ParseError (400) for an empty body. YAMLParser now raises ParseError for an empty or null document too, so both formats fail the same way for the same input. - yaml.safe_load uses PyYAML's default YAML 1.1 boolean resolver, which coerces unquoted no/yes/on/off (any case) to booleans -- the "Norway problem" -- so a schema field named/valued "no" would silently become False. YAMLParser now uses a _StrictBoolLoader that only resolves true/false as booleans. Added tests for all three in test_yaml_codec.py and test_schema_api.py. --- netbox_custom_objects/api/parsers.py | 35 +++++++++++++++++-- netbox_custom_objects/api/views.py | 3 ++ .../tests/schema/test_schema_api.py | 20 +++++++++++ .../tests/schema/test_yaml_codec.py | 24 +++++++++++-- 4 files changed, 76 insertions(+), 6 deletions(-) diff --git a/netbox_custom_objects/api/parsers.py b/netbox_custom_objects/api/parsers.py index 3df50b29..3d94295f 100644 --- a/netbox_custom_objects/api/parsers.py +++ b/netbox_custom_objects/api/parsers.py @@ -1,8 +1,33 @@ -import yaml +import copy +import re +import yaml from rest_framework.exceptions import ParseError from rest_framework.parsers import BaseParser +_BOOL_TAG = 'tag:yaml.org,2002:bool' + + +class _StrictBoolLoader(yaml.SafeLoader): + """ + SafeLoader variant that only resolves the literal tokens ``true``/``false`` + (any case) as booleans, instead of PyYAML's default YAML 1.1 behaviour of + also treating ``yes``/``no``/``on``/``off`` as booleans (the "Norway + problem"). Without this, an unquoted schema value like ``no`` would be + silently coerced to ``False`` rather than kept as the string "no". + """ + + +_StrictBoolLoader.yaml_implicit_resolvers = { + key: [item for item in resolvers if item[0] != _BOOL_TAG] + for key, resolvers in copy.deepcopy(yaml.SafeLoader.yaml_implicit_resolvers).items() +} +_StrictBoolLoader.add_implicit_resolver( + _BOOL_TAG, + re.compile(r'^(?:true|True|TRUE|false|False|FALSE)$'), + list('tTfF'), +) + class YAMLParser(BaseParser): """Parses YAML request bodies for the schema preview/apply endpoints (#665).""" @@ -11,7 +36,11 @@ class YAMLParser(BaseParser): def parse(self, stream, media_type=None, parser_context=None): try: - data = yaml.safe_load(stream) + data = yaml.load(stream, Loader=_StrictBoolLoader) except yaml.YAMLError as exc: raise ParseError(f"YAML parse error: {exc}") - return data if data is not None else {} + if data is None: + # Matches JSONParser, which raises ParseError (400) for an empty + # body rather than silently treating it as an empty document. + raise ParseError("YAML parse error: request body is empty.") + return data diff --git a/netbox_custom_objects/api/views.py b/netbox_custom_objects/api/views.py index 0ac00ebf..c5a53a4a 100644 --- a/netbox_custom_objects/api/views.py +++ b/netbox_custom_objects/api/views.py @@ -431,6 +431,9 @@ def post(self, request, *args, **kwargs): "Both add and change permissions on CustomObjectType are required." ) + if not isinstance(request.data, dict): + raise ValidationError({"non_field_errors": [_("Request body must be a JSON/YAML object.")]}) + allow_destructive = request.data.get("allow_destructive", False) if not isinstance(allow_destructive, bool): raise ValidationError({"allow_destructive": _("'allow_destructive' must be a boolean.")}) diff --git a/netbox_custom_objects/tests/schema/test_schema_api.py b/netbox_custom_objects/tests/schema/test_schema_api.py index fd223fe7..0c9bca5f 100644 --- a/netbox_custom_objects/tests/schema/test_schema_api.py +++ b/netbox_custom_objects/tests/schema/test_schema_api.py @@ -450,6 +450,26 @@ def test_apply_accepts_yaml_request_body_and_creates_cot(self): self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertTrue(CustomObjectType.objects.filter(slug="yaml-applied").exists()) + def test_apply_non_dict_json_body_returns_400(self): + resp = self.client.post(self.apply_url, data=[1, 2, 3], format="json") + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + + def test_apply_non_dict_yaml_body_returns_400(self): + resp = self.client.post( + self.apply_url, + data=yaml.safe_dump([1, 2, 3]), + content_type="application/yaml", + ) + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + + def test_preview_empty_yaml_body_returns_400(self): + resp = self.client.post(self.preview_url, data="", content_type="application/yaml") + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + + def test_apply_empty_yaml_body_returns_400(self): + resp = self.client.post(self.apply_url, data="", content_type="application/yaml") + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + def test_apply_returns_yaml_when_accepted(self): body = yaml.safe_dump({ "allow_destructive": False, diff --git a/netbox_custom_objects/tests/schema/test_yaml_codec.py b/netbox_custom_objects/tests/schema/test_yaml_codec.py index da266cb2..445d6fe0 100644 --- a/netbox_custom_objects/tests/schema/test_yaml_codec.py +++ b/netbox_custom_objects/tests/schema/test_yaml_codec.py @@ -19,15 +19,33 @@ def test_parses_valid_yaml_to_dict(self): data = YAMLParser().parse(stream) self.assertEqual(data, {"schema_version": "1", "types": []}) - def test_empty_body_returns_empty_dict(self): - data = YAMLParser().parse(io.BytesIO(b"")) - self.assertEqual(data, {}) + def test_empty_body_raises_parse_error(self): + # Matches JSONParser, which also raises ParseError (400) for an empty body. + with self.assertRaises(ParseError): + YAMLParser().parse(io.BytesIO(b"")) + + def test_null_document_raises_parse_error(self): + with self.assertRaises(ParseError): + YAMLParser().parse(io.BytesIO(b"null\n")) def test_malformed_yaml_raises_parse_error(self): stream = io.BytesIO(b"types: [\n") with self.assertRaises(ParseError): YAMLParser().parse(stream) + def test_unquoted_no_yes_on_off_preserved_as_strings(self): + # PyYAML's default YAML 1.1 resolver treats these as booleans (the + # "Norway problem"); the schema parser must not, since field + # names/values could legitimately be these words. + stream = io.BytesIO(b"a: no\nb: yes\nc: On\nd: OFF\n") + data = YAMLParser().parse(stream) + self.assertEqual(data, {"a": "no", "b": "yes", "c": "On", "d": "OFF"}) + + def test_unquoted_true_false_still_resolve_to_booleans(self): + stream = io.BytesIO(b"a: true\nb: False\n") + data = YAMLParser().parse(stream) + self.assertEqual(data, {"a": True, "b": False}) + class YAMLRendererTestCase(SimpleTestCase): From 54fb012d82bcb28be1c8c68d256479e5392f8d75 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Wed, 19 Aug 2026 19:18:00 -0400 Subject: [PATCH 3/4] Address automated PR review comment on #668 - YAMLRenderer.render() now returns bytes (encoded with self.charset) instead of str, matching DRF's renderer convention (JSONRenderer does the same). DRF's Response.render() happened to paper over the str case, but middleware inspecting the raw render output before encoding would not have. - YAMLParser's ParseError for a None-parsed document no longer claims the body is "empty" -- yaml.load() can't distinguish an empty body from an explicit `null` document, and the old message was misleading for the latter. Message now says the body must be a mapping, not null or empty. - SchemaPreviewView.post() now guards against a non-dict top-level body the same way SchemaApplyView already did, so both endpoints return the same non_field_errors shape for a non-dict body instead of preview's YAML-list/scalar case falling through to jsonschema's schema_errors format. Updated tests accordingly (renderer output is bytes now; added non-dict-body coverage for SchemaPreviewView). Full suite verified in a clean venv: 1227 tests, 0 failures/errors, 8 skipped. --- netbox_custom_objects/api/parsers.py | 8 +++++--- netbox_custom_objects/api/renderers.py | 4 ++-- netbox_custom_objects/api/views.py | 3 +++ .../tests/schema/test_schema_api.py | 15 +++++++++++++++ .../tests/schema/test_yaml_codec.py | 16 +++++++++++----- 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/netbox_custom_objects/api/parsers.py b/netbox_custom_objects/api/parsers.py index 3d94295f..7c911654 100644 --- a/netbox_custom_objects/api/parsers.py +++ b/netbox_custom_objects/api/parsers.py @@ -40,7 +40,9 @@ def parse(self, stream, media_type=None, parser_context=None): except yaml.YAMLError as exc: raise ParseError(f"YAML parse error: {exc}") if data is None: - # Matches JSONParser, which raises ParseError (400) for an empty - # body rather than silently treating it as an empty document. - raise ParseError("YAML parse error: request body is empty.") + # Covers both an empty body and an explicit YAML `null` document -- + # yaml.load() can't tell them apart, and neither is ever a valid + # schema/apply body (both endpoints require a mapping). Matches + # JSONParser, which also raises ParseError (400) for an empty body. + raise ParseError("YAML parse error: request body must be a mapping (object), not null or empty.") return data diff --git a/netbox_custom_objects/api/renderers.py b/netbox_custom_objects/api/renderers.py index bf066226..7acbf0f0 100644 --- a/netbox_custom_objects/api/renderers.py +++ b/netbox_custom_objects/api/renderers.py @@ -12,5 +12,5 @@ class YAMLRenderer(BaseRenderer): def render(self, data, accepted_media_type=None, renderer_context=None): if data is None: - return '' - return yaml.safe_dump(data, sort_keys=False, default_flow_style=False) + return b'' + return yaml.safe_dump(data, sort_keys=False, default_flow_style=False).encode(self.charset) diff --git a/netbox_custom_objects/api/views.py b/netbox_custom_objects/api/views.py index c5a53a4a..45b0fdf7 100644 --- a/netbox_custom_objects/api/views.py +++ b/netbox_custom_objects/api/views.py @@ -356,6 +356,9 @@ class SchemaPreviewView(APIView): permission_classes = [IsAuthenticatedOrLoginNotRequired] def post(self, request, *args, **kwargs): + if not isinstance(request.data, dict): + raise ValidationError({"non_field_errors": [_("Request body must be a JSON/YAML object.")]}) + schema_doc = request.data _validate_schema_doc(schema_doc) diffs = diff_document(schema_doc) diff --git a/netbox_custom_objects/tests/schema/test_schema_api.py b/netbox_custom_objects/tests/schema/test_schema_api.py index 0c9bca5f..2323d444 100644 --- a/netbox_custom_objects/tests/schema/test_schema_api.py +++ b/netbox_custom_objects/tests/schema/test_schema_api.py @@ -450,9 +450,24 @@ def test_apply_accepts_yaml_request_body_and_creates_cot(self): self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertTrue(CustomObjectType.objects.filter(slug="yaml-applied").exists()) + def test_preview_non_dict_json_body_returns_400(self): + resp = self.client.post(self.preview_url, data=[1, 2, 3], format="json") + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("non_field_errors", resp.data) + + def test_preview_non_dict_yaml_body_returns_400(self): + resp = self.client.post( + self.preview_url, + data=yaml.safe_dump([1, 2, 3]), + content_type="application/yaml", + ) + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("non_field_errors", resp.data) + def test_apply_non_dict_json_body_returns_400(self): resp = self.client.post(self.apply_url, data=[1, 2, 3], format="json") self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("non_field_errors", resp.data) def test_apply_non_dict_yaml_body_returns_400(self): resp = self.client.post( diff --git a/netbox_custom_objects/tests/schema/test_yaml_codec.py b/netbox_custom_objects/tests/schema/test_yaml_codec.py index 445d6fe0..165a7713 100644 --- a/netbox_custom_objects/tests/schema/test_yaml_codec.py +++ b/netbox_custom_objects/tests/schema/test_yaml_codec.py @@ -49,21 +49,27 @@ def test_unquoted_true_false_still_resolve_to_booleans(self): class YAMLRendererTestCase(SimpleTestCase): - def test_renders_dict_to_yaml(self): + def test_renders_dict_to_yaml_bytes(self): + # DRF renderer convention: render() returns bytes, not str (matches + # JSONRenderer; DRF's Response.render() would otherwise have to + # re-encode a str result itself). output = YAMLRenderer().render({"diffs": [{"slug": "circuit"}]}) + self.assertIsInstance(output, bytes) self.assertEqual( output, - "diffs:\n- slug: circuit\n", + b"diffs:\n- slug: circuit\n", ) - def test_renders_none_to_empty_string(self): - self.assertEqual(YAMLRenderer().render(None), '') + def test_renders_none_to_empty_bytes(self): + output = YAMLRenderer().render(None) + self.assertIsInstance(output, bytes) + self.assertEqual(output, b'') def test_round_trips_through_parser(self): import yaml original = {"schema_version": "1", "types": [{"name": "circuit", "slug": "circuit"}]} rendered = YAMLRenderer().render(original) - parsed = YAMLParser().parse(io.BytesIO(rendered.encode("utf-8"))) + parsed = YAMLParser().parse(io.BytesIO(rendered)) self.assertEqual(parsed, original) # Sanity-check against a plain yaml.safe_load too. self.assertEqual(yaml.safe_load(rendered), original) From 8fa2405f4e795be5e1c8bcf502d933caa2411023 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Wed, 26 Aug 2026 11:07:04 -0400 Subject: [PATCH 4/4] Reject YAML anchors/aliases in schema documents Per Jason Novinger's review suggestion (PR #668), _StrictBoolLoader now overrides compose_node() to reject any AliasEvent, raising ComposerError (already caught by YAMLParser's existing except yaml.YAMLError, so it becomes a clean 400 same as any other malformed document). Schema documents have no legitimate use for anchors/aliases, and without this an anchor/alias expansion bomb could grow arbitrarily large in memory at parse time, before _validate_schema_doc() ever sees the result. Adds a regression test confirming an anchor/alias document is rejected. Full plugin suite verified in a clean venv: 1228 tests, 0 failures/errors, 8 skipped. https://github.com/netboxlabs/netbox-custom-objects/pull/668#pullrequestreview-5031808627 --- netbox_custom_objects/api/parsers.py | 12 ++++++++++++ .../tests/schema/test_yaml_codec.py | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/netbox_custom_objects/api/parsers.py b/netbox_custom_objects/api/parsers.py index 7c911654..33b27812 100644 --- a/netbox_custom_objects/api/parsers.py +++ b/netbox_custom_objects/api/parsers.py @@ -15,8 +15,20 @@ class _StrictBoolLoader(yaml.SafeLoader): also treating ``yes``/``no``/``on``/``off`` as booleans (the "Norway problem"). Without this, an unquoted schema value like ``no`` would be silently coerced to ``False`` rather than kept as the string "no". + + Also rejects anchors/aliases outright: schema documents have no legitimate + use for them, and without this an anchor/alias bomb could expand in memory + at parse time, before _validate_schema_doc() ever sees the result. """ + def compose_node(self, parent, index): + if self.check_event(yaml.events.AliasEvent): + event = self.peek_event() + raise yaml.composer.ComposerError( + None, None, "YAML anchors/aliases are not permitted in schema documents", event.start_mark + ) + return super().compose_node(parent, index) + _StrictBoolLoader.yaml_implicit_resolvers = { key: [item for item in resolvers if item[0] != _BOOL_TAG] diff --git a/netbox_custom_objects/tests/schema/test_yaml_codec.py b/netbox_custom_objects/tests/schema/test_yaml_codec.py index 165a7713..45c20ecf 100644 --- a/netbox_custom_objects/tests/schema/test_yaml_codec.py +++ b/netbox_custom_objects/tests/schema/test_yaml_codec.py @@ -46,6 +46,13 @@ def test_unquoted_true_false_still_resolve_to_booleans(self): data = YAMLParser().parse(stream) self.assertEqual(data, {"a": True, "b": False}) + def test_anchors_and_aliases_rejected(self): + # Schema documents have no legitimate use for anchors/aliases; rejecting + # them outright avoids an anchor/alias expansion bomb at parse time. + stream = io.BytesIO(b"a: &anchor [1, 2, 3]\nb: *anchor\n") + with self.assertRaises(ParseError): + YAMLParser().parse(stream) + class YAMLRendererTestCase(SimpleTestCase):