search

in-toto / attestation

in-toto Attestation Framework
Other
233 stars 59 forks source link

Add proto definition for vuln predicate type #345

Open hectorj2f opened 6 months ago

hectorj2f commented 6 months ago

This PR adds a proto definition for the vuln predicate type. This was pending since we added the vuln predicate type and it is needed to update the cosign vuln predicate types.

hectorj2f commented 6 months ago

related to #268

marcelamelara commented 2 months ago

Friendly ping on this PR. @hectorj2f Could you please remove the generated Go/Python/Java files and add an entry for this proto here? then I think your PR will be pretty much ready to go.

hectorj2f commented 2 months ago

@marcelamelara Yes, I'll do it.

TomHennen commented 1 month ago

I think something might have gotten a bit screwy with the git commits (I've done this myself for sure). When I look at 'Files Changed' it doesn't look like anything has changed since our earlier comments?

hectorj2f commented 1 month ago

I'm confused now. I followed what @marcelamelara commented above remove all the generated go/python and java code and leave the entry in protos.

marcelamelara commented 1 month ago

If I understand correctly, having only the .proto file is what Tom had originally requested. But there may have been some edits to that proto file that were lost when the auto-generated files were removed? I only see 1 commit in this PR right now.

Separately, I also had requested a minor documentation update to the /protos/README.md file. This way this new proto is included in that list.

marcelamelara commented 1 week ago

One final question (which can be addressed in a separate PR, I think) is about the predicate type URI for this predicate. Right now, in the vuln predicate spec it's listed as https"//in-toto.io/attestation/vuln_s_ rather than /vuln which is the name we use everywhere else. Does the spec have a typo? We usually want the name to match everywhere, but I'm also worried that tooling might already be using the pluralized type URL.

@hectorj2f What do you think? Is this a concern?