[etirelli]

What

Follow-up to #16, fixing the three issues from its last review (comment). All three predate #16.

• [P2] matchPath was not validated. matchPath: null or 1 crashed both commands with a TypeError and no report. matchPath: "[" crashed check with re.error.
• [P2] flattenAllOf was not checked as a boolean. null and "false" passed, and null silently turned off --flatten-allof.
• [P3] The contract id, type and declaredArtifact were not checked, and the parity table covered only selected fields.

Approach

#16's hand-written validator covered only the fields the commands happened to read, so each review round found another gap. This PR enforces the schema itself instead.

• modules/openapi-compat/descriptor.py interprets schemas/contract-set.schema.json at run time. The workflows use the runner's python3 with no packages, so it doesn't depend on jsonschema.
  • It implements exactly the keywords the schema uses: type, required, properties, items, additionalProperties, $ref, allOf, if/then, const, enum, pattern, minLength and minItems.
  • It refuses to run if the schema gains a keyword it doesn't implement, so a schema change can't go silently unenforced.
  • One rule is stricter than the schema: matchPath must compile as a regular expression, because both commands use it as one.
• validate_descriptor() now delegates to it, so check and coverage enforce every schema rule before doing anything else. Errors name the field's path, for example spec.contracts[0].policy.flattenAllOf must be a boolean (true or false), not str 'false'. check also catches re.error as an integrity error, as a second safeguard.
• Schema: oasdiffVersion and yqVersion now require at least one character (minLength: 1). An empty pin was already treated as unpinned.
• Cleanup: coverage's warning for contracts that aren't openapi type is removed; the schema now rejects such contracts up front.

Tests

• modules/coverage/tests/test_schema.py now generates its cases from the schema. It walks every property, including array items, additionalProperties, and both generator if/then branches. Each property is omitted, given wrong-type values, and given values that break its const/enum/pattern/minLength/minItems rule. jsonschema and descriptor.py must agree on all 285 generated cases.
  • Every property is covered by construction. A test fails if the valid fixture stops using a property the schema declares, or if the schema uses a keyword the validator doesn't implement.
• CLI tests through both commands for each reviewed case:
  • matchPath null, int or bad regex
  • flattenAllOf null or a string
  • id missing, a list or null; type missing or protobuf; declaredArtifact a list
  • apiVersion, metadata and severityOverrides values
  • For check, each one also writes report.json.

Verification

All 655 Python tests in the CI directories pass, mypy is clean, and the action-pin check and actionlint pass. I also validated the descriptors in the onboarded repositories I could find (opendatahub-io/model-registry, opendatahub-io/odh-dashboard, red-hat-data-services/model-registry and red-hat-data-services/odh-dashboard). All pass both jsonschema and the new validator. model-registry-operator runs coverage without a descriptor (CRD-only), which this change doesn't touch.

Compatibility

No change for schema-valid descriptors. Descriptors the schema already rejects now fail both commands with exit 2 and a report. In v2.2.0, these passed or crashed:

• matchPath that isn't a string, or isn't a valid regex. These crashed, except a bad regex in coverage, which already exited 2.
• flattenAllOf that isn't a boolean, which passed.
• A contract missing id, type or declaredArtifact, or with a non-string value. These passed or reported gaps.
• A contract type other than openapi. Coverage warned; check already rejected it.
• A missing or wrong apiVersion or metadata.name, which passed.
• A severityOverrides level outside info/warn/error. These were passed through to oasdiff.
• An empty sourceGlobs, a generator missing a field its adapter requires, or an empty version pin.

Some error messages change for cases that were already rejected. A missing reason is now spec.exclusions[0].reason is required: record why, for reviewers; ..., and an unregistered adapter is now rejected by the schema's enum before reaching the adapter registry, which still fails closed on its own.

🤖 Generated with Claude Code

[coderabbitai]

Important

• Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: a71cb4d1-8a28-45aa-93df-5d8f4348885d