Skip to content
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

Consider adding spec fields describing the config source and generator #696

Open
bgilbert opened this issue Dec 15, 2018 · 10 comments
Open

Comments

@bgilbert
Copy link
Contributor

Feature Request

Environment

Any

Desired Feature

Consider adding spec fields describing where a config came from: the corresponding human-readable source file and the tool used to convert it into an Ignition config.

The purpose of these fields is to assist users in determining the source of an Ignition config and what bugs may have existed in the tool used to generate it. They are explicitly not to be used by Ignition to work around bugs in generator tools.

Example spec fragment

The data model below is intentionally simplistic, e.g. it does not handle multiple source files or multiple generator tools. We could expand the model if that seems useful, but I didn't want to overdo it.

  • ignition (object): metadata about the configuration itself.
    • config (object): options related to the configuration.
      • origin (object): information about the human-readable source file for the configuration.
        • identifier (string): a URI referring to the source file. This is not resolved by Ignition.
        • hash (string): the hash of the source file, in the form <type>-<value> where type is sha512.
      • generator (object): information about the software that generated the configuration.
        • software (string): a URL referring to the software, such as a website or source code repository.
        • version (string): the version of the software. This does not need to be a semantic version number and has no semantics.
@ajeddeloh
Copy link
Contributor

I'm in favor.

Given that Ignition will just ignore this field (i.e. there is never a case where we want to alter behavior based on it) and that it's for humans only, why not just a string to put something like fcos-ct-0.1.0?

@lucab
Copy link
Contributor

lucab commented Dec 17, 2018

I'm pretty much of the opinion that this doesn't belong to the schema. Rationale is:

  • Ignition will not parse/react to this. If that is the case, I am not sure why it specifies a schema for it
  • proposed schema looks reasonable on the surface, but it feels completely arbitrary (and I can't judge whether it is just fine or overkill, I guess it depends on the consumer usecase)
  • AFAIK unknown fields are just disregarded by the JSON parser. As such, users needing something like this can directly add an arbitrary number of ad hoc fields (e.g. "_ua": "fcos-ct-0.1.0 (like Gecko)")

@ajeddeloh
Copy link
Contributor

Ignition warns on unused fields, so that would wrongfully generate warnings.

@jlebon
Copy link
Member

jlebon commented Dec 17, 2018

This sounds like a way to hack around JSON's lack of support for comments.

Ignition warns on unused fields, so that would wrongfully generate warnings.

Or maybe that itself should be refined so it allows e.g. fields starting with underscores without warnings. Then, one can add whatever __super_duper_key they want. At least there's some parallel with some programming languages that use this to denote a field as known unused. :)

@ajeddeloh
Copy link
Contributor

Hmm. that would also allow for passing data to other things in the initramfs (e.g. for package layering that gets set up on first boot, although I'm still unsure how exactly that would work).

@bgilbert
Copy link
Contributor Author

bgilbert commented Jan 3, 2019

I'm still concerned about even slightly encouraging slipstreaming of configuration for other initramfs tools. It seems like a giant attractive nuisance. (For distro developers, not directly for users.)

@jlebon's point about JSON comments rings true, though. What if we supported e.g. underscore keys but stripped them out before writing the cache file? We could always change that behavior later if desired.

@ajeddeloh
Copy link
Contributor

By "supported" I assume you mean "didn't warn on parsing them". I'd be in favor; it's a simple enough change to make too.

@ajeddeloh
Copy link
Contributor

Any reason this should have with 3.0.0 vs 3.1.0?

@bgilbert
Copy link
Contributor Author

It would allow FCOSCT to provide the additional metadata from day one. Other than that, no.

It's also somewhat less relevant now because of #726.

@ajeddeloh
Copy link
Contributor

Ok, dropping the spec 3.0.0 milestone then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants