Skip to content

Implement schema patching (RFC 7386) #573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

Gaspi
Copy link
Contributor

@Gaspi Gaspi commented Mar 18, 2025

Implements Schema patching

This PR introduces schema patching, allowing fine-grained modifications to JSON schemas
via a new patchSchemaWith key under onyxia-api, similar to overwriting.

Schema resolution now supports JSON Merge Patch (RFC 7386)
in addition to the existing overwriteSchemaWith and $ref substitution mechanisms.

Motivation

The existing overwriting mechanism completely replaces the JSON structure, which has several drawbacks:

  • Requires redefining the entire schema section, even for minor modifications (e.g., updating a "default" value)
  • Risks breaking functionality if critical sections are not properly reimplemented
  • Prevents updates to overwritable schema sections in charts

By introducing a targeted JSON patching mechanism, users can specify only the values that need updating,
improving both readability and maintainability.

Corner cases

  • When multiple substitution mechanisms ($ref, rewriteSchemaWith, patchSchemaWith) are present,
    only the first available, in that order, is applied.
  • A rewriteSchemaWith nested within a patched object is processed first, followed by patching on the resolved JSON
  • Nested patchSchemaWith entries are applied from the innermost to the outermost level.
  • As specified in RFC 7386:
    • Arrays are replaced entirely rather than patched item-by-item
    • In case of type conflicts, the patch takes precedence
    • Keys can be removed using "key": null (only valid in patches, not in schema files)
      Does not fail if the "key" is missing from the patched object

Implementation Notes

  • Avoids unnecessary deepCopy() by reconstructing a fresh JsonNode
  • Does not detect self-referencing loops, which may lead to stack overflows, for instance if a patch references itself
  • Onyxia-specific keys (rewriteSchemaWith, patchSchemaWith) are removed from the final resolved schema

Example

The input.json schema below is resolved as output.json when
the ingress.json, enabled.json, hostname.json, and ignored.json files
can be retrieved from the JsonSchemaRegistryService.

input.json
{
  "title": "Original Ingress title",
  "type": "Original Ingress type",
  "x-onyxia": {
    "patchSchemaWith": "ingress.json"
  },
  "properties": {
    "enabled": {
      "description": "Original enabled description",
      "type": "Original enabled type",
      "x-onyxia": {
        "overwriteSchemaWith": "enabled.json",
        "patchSchemaWith": "ignored.json"
      }
    },
    "useTlsSecret": {
      "title" : "Original title not overriden since missing-schema.json doesn't exist",
      "x-onyxia": {
        "overwriteSchemaWith": "missing-schema.json"
      }
    },
    "hostname": {
      "title": "Original hostname title",
      "type": "Original hostname type",
      "x-onyxia": {
        "patchSchemaWith": "hostname.json"
      }
    },
    "useCertManager": {
      "$ref": "#/definitions/useCertManagerRef"
    }
  },
  "definitions": {
    "useCertManagerRef": {
      "type": "Use Cert Manager type from ref",
      "description": "Use Cert Manager description from ref"
    }
  }
}
ingress.json
{
  "title": "Title edited by by ingress.json",
  "description": "Description added by ingress.json",
  "properties": {
    "ingressClassName": {
      "default": "New ingressClassName object created by ingress.json"
    },
    "enabled": {
      "title": "Title created by enabled.json and then overriden by ingress.json"
    },
    "hostname": {
      "title": "Title overriden by ingress.json",
      "description": null,
      "type": "Type deleted by hostname.json, recreated by ingress.json",
      "nonexistent-field-deleted-without-error": null
    },
    "useCertManager": {
      "description": "Use Cert Manager description edited by ingress.json"
    }
  }
}
enabled.json
{
  "title": "Title created by enabled.json",
  "description": "Description overwritten by enabled.json",
  "type": "type overwritten by enabled.json",
  "new_field": "New field created by enabled.json"
}
hostname.json
{
  "title": "Title edited by hostname.json",
  "description": "New description created by hostname.json",
  "type": null
}
ignored.json
"I replace stuff with this string if I'm used anywhere"
output.json
{
  "title": "Title edited by by ingress.json",
  "type": "Original Ingress type",
  "properties": {
    "enabled": {
      "title": "Title created by enabled.json and then overriden by ingress.json",
      "description": "Description overwritten by enabled.json",
      "type": "type overwritten by enabled.json",
      "new_field": "New field created by enabled.json"
    },
    "useTlsSecret": {
      "title": "Original title not overriden since missing-schema.json doesn't exist"
    },
    "hostname": {
      "title": "Title overriden by ingress.json",
      "type": "Type deleted by hostname.json, recreated by ingress.json"
    },
    "useCertManager": {
      "type": "Use Cert Manager type from ref",
      "description": "Use Cert Manager description edited by ingress.json"
    },
    "ingressClassName": {
      "default": "New ingressClassName object created by ingress.json"
    }
  },
  "definitions": {
    "useCertManagerRef": {
      "type": "Use Cert Manager type from ref",
      "description": "Use Cert Manager description from ref"
    }
  },
  "description": "Description added by ingress.json"
}

@Gaspi Gaspi requested review from olevitt and fcomte March 18, 2025 13:53
@Gaspi Gaspi self-assigned this Mar 18, 2025
Copy link

Copy link

@Gaspi Gaspi added enhancement New feature or request help wanted Extra attention is needed labels Mar 18, 2025
@garronej
Copy link
Contributor

For reference, I pushed this a while a go:
InseeFrLab/onyxia#798 (comment)

I think this proposal is quite aligned

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants