Open Adam0Brien opened 1 year ago
The file extension doesn't matter. There's no requirement that Ignition configs have an .ign
extension. And, in the case of the inline
directive, there isn't any file extension; the file contents are included directly in the parent config.
As #275 says, what we should do instead is validate the contents of the specified config using Ignition config validation.
As #275 says, what we should do instead is validate the contents of the specified config using Ignition config validation.
by ignition config validation do you mean https://github.com/coreos/butane/blob/main/vendor/github.com/coreos/ignition/v2/config/v3_5_experimental/types/resource.go#L34 ?
That's a validator that's invoked by the validation process. We need to run the validation process.
But thinking about it some more, we need to do more than just validation. The embedded Ignition config is in the form of text, not Go structs, so we also need to parse it. Ignition's config parsing function in config
handles both parsing and validation, so we can just call that, and merge its report into ours.
A few notes:
The parse function sometimes returns an error
alongside an empty report. We should probably AddOnError
the returned error to our validation report in addition to merging Ignition's report.
Merging Ignition's report is a bit tricky. We'd need to replace all the path fields with the path to the Butane field holding the config, which will be something like $.ignition.config.merge.0.inline
. That loses information but I'm not sure there's a good way around it. We could try replacing the Ignition error in the log entry with a new one that includes the original path and the original message.
The user is allowed to use older versions of Butane that don't understand the current Ignition spec, and is allowed to embed Ignition configs with versions newer than Butane understands. We should have special handling for that case: https://github.com/coreos/butane/issues/275#issuecomment-1620636053
This validation isn't FCOS-specific; it applies to all variants. So, it should be in base
. And since we're only going to be producing errors for configs that were already invalid (but would be rejected when the machine boots rather than when Butane runs) we should backport it to stable base
specs.
Let's split the vendor update into its own commit to make reviewing the change easier.
Let's split the vendor update into its own commit to make reviewing the change easier.
That's no problem!, but wont that break CI since validate.go wont be able to compile the parse function?
Fix #275
This PR introduces validation for local/inline fields when using the merge/replace feature in butane.
For this PR ignitions Parse function needs to be used so the vendor folder is has been updated to accommodate this