-
Notifications
You must be signed in to change notification settings - Fork 135
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
[W.I.P] Add new model: Provenance #466
Conversation
Fixes in-toto#464 Signed-off-by: Furkan <furkan.turkal@trendyol.com>
Hi @Dentrax! This looks like it's going in a good direction. I wonder if we'd like to coallesce some elements of this predicate within a superclass that provides most of the utility functions. I'd assume we'll have more than this one predicate. What do you think? |
Is it ever going to be merged? Are there other known tools to generate provenance? |
Hi @hi-artem . Sorry this is in my review pile but I haven't gotten to it. I ack it's been a little while. Let me make sure some activity happens on this front soon... |
Actually, i was created this PR for learning purposes, i thought that it's a good opportunity to get into it, but not I'm not so sure whether it's makes sense to merge this since still w.i.p. What do you think? |
I'd have to take a close look at it. From skimming through it I see a lot of value added, so it'd be worthwhile to see this crystallize as a feature for in-toto-python in one way or the other 👍 |
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.
Hi @Dentrax! Thanks for this PR. I'm very sorry about how long this has taken. I'm now prioritizing this PR and related efforts to get them merged in.
|
||
|
||
@attr.s(repr=False, init=False) | ||
class Provenance(Signable): |
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.
As we did in in-toto-golang, let's explicitly version predicate models. Perhaps just putting them in separate directories (and so packages) is sufficient.
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.
So ITE-6 predicates require us to also implement attestation Statements. I wonder if that belongs in this PR or if we add support for that separately and rebase. We should also have DSSE land soon which the Statement would build off? cc @lukpueh
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.
Also note that Statement support is mandatory because we need subject
.
class Provenance(Signable): | ||
"""Evidence for a performed step or inspection of the supply chain. | ||
|
||
SPEC: https://github.com/in-toto/attestation/blob/main/spec/predicates/provenance.md |
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.
Link should be updated to slsa.dev.
|
||
""" | ||
_type = attr.ib() | ||
builder = attr.ib() |
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.
Strictly a side effect of the delay reviewing this PR (my bad!). We should either add this and mark it as v0.1 (and include v0.2 separately) or just update it to v0.2 and support from there onwards. Either way is fine but v0.2 is mandatory.
def __init__(self, **kwargs): | ||
super(Provenance, self).__init__() | ||
|
||
self._type = "provenance" |
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.
Nit: The type is incorrect.
Closing this, thanks for your work! We plan to add support for Statements and predicates like Provenance via https://pypi.org/project/in-toto-attestation/. |
Fixes #464
Signed-off-by: Furkan furkan.turkal@trendyol.com
For the
TODO
line:# TODO: securesystemslib.formats.ANY_NUMBER_SCHEMA.check_match(self.recipe.type)
I already created an issue: secure-systems-lab/securesystemslib#367
I created a small function called
_get_attribute
in order to pass(OPTIONAL)
variables. (I followed the spec)I implemented a bare model and tests, but couldn't figure it out how to make it work. It seems most
in_toto_*
entry points uses thelink
type for the running, generating and validating. Shouldn't we make thatrun
,sign
,verify
, etc. more generic in order to parse and read different predicate types? Should I create a new for parser for this? What is the way out? Thanks! (I just wanted to learn project's internal domain :) )Feed free to edit! Waiting your feedback!
Description of the changes being introduced by the pull request:
See the issue.
Please verify and check that the pull request fulfills the following
requirements: