search

coreos / ignition-config-rs

Data structures for reading/writing Ignition configs
https://crates.io/crates/ignition-config
Apache License 2.0
3 stars 5 forks source link

Support parsing Ignition configs of unknown versions #15

Closed bgilbert closed 2 years ago

bgilbert commented 2 years ago

Add a two-phase parse, similar to what Ignition itself does. Return a vector of warnings, currently only for unknown fields that were ignored by the parser.

bgilbert commented 2 years ago

Updated, made some small tweaks, and rebased onto #16 (which I should have done in a separate push, sorry).

bgilbert commented 2 years ago

Added a commit to fail parsing on unrecognized fields.

bgilbert commented 2 years ago

It turns out that JSON Schema has an additionalProperties keyword that, when false, disallows unknown properties. The Ignition schema doesn't set it, and to do so would be a breaking change. Since a schema-level mechanism exists, the second commit feels hackier now, and I'm thinking I should drop it. Interested in thoughts though.

jlebon commented 2 years ago

It turns out that JSON Schema has an additionalProperties keyword that, when false, disallows unknown properties. The Ignition schema doesn't set it, and to do so would be a breaking change. Since a schema-level mechanism exists, the second commit feels hackier now, and I'm thinking I should drop it. Interested in thoughts though.

Tricky. I think ideally, we should be able to parse whatever Ignition can, which would argue for not failing on unknown fields. Though the round-trip issue is a legitimate concern for an API (while it isn't for Ignition), and unlike Ignition, we don't have a good way to report problems.

Hmm, maybe we should make it easy to let the client choose? E.g. we could keep the base types themselves as is (not denying) but provide a function which can parse in "strict mode". I found https://docs.rs/serde_ignored/latest/serde_ignored/ which would fit the bill. (We could also have a second function which doesn't error but returns that report.)

jlebon commented 2 years ago

Related: https://github.com/coreos/ignition/issues/696

bgilbert commented 2 years ago

Ah, serde_ignored is a good find! I've reworked the API to return a vector of warnings for unknown fields. The caller can ignore them, report them, or fail, as it wishes.