Validation is not declared-version-aware for IntelRdt fields

[ebendler]

On current main (35765bd41b50a86aa3919eb352bc90321e010e68), cmd/validate accepts CDI documents whose fields do not match the declared cdiVersion.

Repro:

cd cmd/validate
go run . -schema builtin - <<'EOF'
cdiVersion: "1.1.0"
kind: "vendor.com/device"
containerEdits:
  intelRdt:
    enableCMT: true
devices:
  - name: "gpu0"
    containerEdits:
      deviceNodes:
        - path: "/dev/null"
EOF

Actual:

<stdin>: document is valid.

Expected:
Validation should fail because enableCMT / enableMBM are legacy Intel RDT fields and are not valid for a document declaring
cdiVersion: "1.1.0"

The inverse also appears to be accepted:

  cdiVersion: "1.0.0"
  containerEdits:
    intelRdt:
      enableMonitoring: false

Expected: validation should fail because enableMonitoring is a v1.1.0 field.

There also seems to be a related library compatibility issue: pkg/cdi.ParseSpec uses strict unmarshalling, while current specs-go.IntelRdt no longer includes enableCMT / enableMBM. That means older valid CDI specs using those legacy fields may be rejected instead of parsed and processed according to their declared spec version.

Suggested behavior: validation/parsing should select field rules based on the document’s declared cdiVersion: accept and process legacy Intel RDT fields for versions where they existed, reject them for v1.1.0+, and reject v1.1.0-only fields in older declared versions.

[zvonkok]

/cc @zvonkok

@kad @mythi Is anyone taking up the work on CDI from the Intel side?

[elezar]

I believe the IntelRdt work was done by @marquiz.

[marquiz]

@ebendler thanks for reporting this. I found two generic gaps with the schema validation, addressed in #323 and #324

Regarding the version-specific validation, the current scheme doesn't work like that. The "builtin" schema always validates agains the current version (i.e. the cdiVersion is not inspected). Changing that would require bigger rework on the validator, also starting to store versioned validation schemas in the repo (with all the old versions).

All-in-all, this a generic schema validation issue/subject, not just intelRdt. Thoughts @elezar @kad @klihub @bart0sh ?

EDIT: Taking another look at this, I realize that cmd/cdi validate subcommand behaves entirely differently from cmd/validate. If I read it correctly, the former first unmarshals the data and the validates a re-marshalled version of it against the json validation schema (effectively a nop(?)). It also does the specs-go based version checks. On the contrary, the latter one (cmd/validate) validates the json schema only. Shouldn't the functionality of these two thingies be roughly equal? For example, putting integers or bools (unquoted 123 or true) in string fields seems to work fine for the json/yaml unmarshalling (and the cdi command is happy) whereas those fail the strict validation of json validation schema (the validate command).

[ebendler]

Yes, I agree that there is a general parsing change needed to validate CDI fields against the declared schema version. I didn't mean to raise this as an Intel-specific issue, it just so happened that these are the only fields that have been removed in later versions of the spec.

I did actually add a fully versioned validator to the cdi-rs crate as part of the PR to bring it up to parity with the 1.1 spec (cncf-tags/container-device-interface-rs#78) and the changes required are not especially gnarly. In there, I chose to just drop invalid fields with a warning.