-
Notifications
You must be signed in to change notification settings - Fork 210
adding enumeration to the namespace field; adding enums for deb/rpm #678
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
base: main
Are you sure you want to change the base?
adding enumeration to the namespace field; adding enums for deb/rpm #678
Conversation
- adding the ability to have an enumeration given to specify valid values - adding those values to RPM and DEB types
|
This is a follow up from |
| }, | ||
| "valid_values": { | ||
| "title": "Valid values", | ||
| "description": "Optional set of allowed values for this namespace. If provided, the namespace value MUST be one of these.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MUST is a problem as I cannot enumerate ALL the known RPM-based distros at all times, so SHOULD would be better. We can also ensure in validation tests that we can return a warning as this would round up the thing nicely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another aspect to consider here are custom packages you add to an OS image/installation, say you build and install your own, patched version of the kernel. In my eyes, an ideal SBOM scanner should detect that the vendor for this package is not debian/opensuse/... and set the namespace accordingly to gernot-patched or unknown. Not sure if this should explicitly be mentioned in the spec or we simply leave it at SHOULD which would allow this degree of freedom?
| ] | ||
| }, | ||
| "valid_values": { | ||
| "title": "Valid values", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we cannot really be exclusive, I would rather could this "known_values"
etc/scripts/purl_type_definition.py
Outdated
| valid_values: Optional[list[str]] = Field( | ||
| None, | ||
| description=( | ||
| "Optional set of allowed values for this namespace. If provided, the namespace value" | ||
| " MUST be one of these." | ||
| ), | ||
| title="Valid values", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need a set of known values rather than a set of valid values. If implementations are validating that they are receiving only the expected values then adding new values (eg adding Linux Mint) is a breaking change.
|
This PR is our first proposed change to the I have created
Beyond that detail task I think that we need to talk a more formal process for any changes to |
|
@pombredanne and @mjherzog anything else I need to do for this PR or is it just process/approval stuff now ? |
|
@jacobcalvert We discussed this PR during the PURL community call this morning and there are some relevant updates from the call:
I apologize for stacking up so many issues on top of this PR, but thank you for being the catalyst for raising them. This will be a topic for further discussion at the next PURL community meeting on October 1 (https://meet.google.com/ryq-aimn-ghd). |
|
@jacobcalvert
|
…om:jacobcalvert/purl-spec into jacobcalvert/add-rpm-and-deb-clarification
77311a5 to
5305e70
Compare
PR Changes
adding the ability to have an enumeration given to specify valid values in the namespace field
This adds a
valid_valuesoptional key to thenamespace_definitionschema so that we can restrict thenamespacefield to a valid set of valueadding those values to RPM and DEB types
I have added these specific enum values to the DEB and RPM types.
For Discussion
Non-descriptive Namespaces
It appears that some SBOM generators read the
IDfield from/etc/os-releasewhich can work well sometimes (e.g., for Ubuntu it reportsubuntu) but in other cases it is nonsensically short (e.g., OracleLinux reportsol). In these cases I just opted to specify the namespace by following the convention of the others in its category, so Oracle Linux becameoraclelinuxfollowing the form ofalmalinuxand others.Edit:
I have run:
And it seems
make gendocsreformatted the CPAN markdown. I don't know why. I can revert that change if needed.