-
Notifications
You must be signed in to change notification settings - Fork 594
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
Add convention for JSON metadata type names and port existing values to the new convention #1844
Comments
Note that any changes we make should still be compatible in our syft decoders. That is, even if we change these names, we should still be able to convert old syft documents and their package metadata without issue. |
When writing this issue originally it was really from the viewpoint of "there are a few metadata types which names are not consistent with the current naming convention". However, while digging I came to a realization that there is really not a strong enough convention other than "use the struct type name + add 'Metadata' to the end" which didn't seem grounded enough. I want to take a step back and try and define these types in a way that makes sense for what they need to be used for. So! What is the Something interesting, these metadata type names are coupled to the json schema. Changing these names means there is a schema change, even if it's just the definition name. Does this mean if we rename a type definition that it's a breaking schema change? Using the struct name as the metadata type leaks what is essentially an internal detail into the json output. We want to be resilient to struct renames not causing parsing issues for json consumers. I want to propose two things:
By convention the
Furthermore I think we should not overload Metadata to represent multiple use cases. They should represent the same thing no matter what instance of that metadata you are referring to . So today:
This ensures when a new field needs to be added to the Metadata for one use case but not another that we don't need to do things like conditionally populate fields in syft or ignore consuming fields downstream. I'll add another comment with specific impacts to the metadata type names shortly... to be continued! 🍿 |
Here are the proposed updates to the metadata type names, as they would appear in JSON output:
|
I've incorporated comments from a team sync and updated: By convention the
Regarding breaking up metadata types by use case, I found a one exception:
Here are the proposed updates to the metadata type names, as they would appear in JSON output:
|
The
pkg.MetadataType
constants will be removed from the core API in syft 1.0, however, the JSON format will still be outputting the string representation. There are inconsistencies with how these are crafted:Some of these end in
Type
and others do not. The correct answer is that they should not end in type. Why? These string constants represent type hints for the JSON format as to how to interpret thepkg.Metadata
field. They describe the type names, not a "type of types".The metadata type names should always be the same names as the structs. So for instance:
is not correct since the struct this represents is called
CargoPackageMetadata
.The text was updated successfully, but these errors were encountered: