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

feat: Finalize the SLSA v1.0 provenance format #221

Merged
merged 2 commits into from
Apr 21, 2023

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Apr 14, 2023

Fixes issue:
Fixes #197

Description:
Finalizes the SLSA v1.0 provenance format.

@adityasaky it would be great to merge the next branch into main soon and do a minor release with the SLSA v1 feature!

Please verify and check that the pull request fulfills the following
requirements:

  • [ x ] Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

cc @MarkLodato @laurentsimon @chuangw6

@asraa asraa changed the base branch from master to next April 14, 2023 17:14
Copy link
Contributor

@chuangw6 chuangw6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @asraa !!
Can we add a provenance statement struct in model.go like these?

@chuangw6
Copy link
Contributor

Also want to loop in @sherzberg-1 as fyi. Just want to make sure the intoto provenance statement can be serialized and deserialized to grafeas proto.

Signed-off-by: Asra Ali <[email protected]>
@sherzberg-1
Copy link

Thanks @chuangw6 , will make sure they can serialize/deserialize back and forth.

@MarkLodato
Copy link

LGTM

@chuangw6
Copy link
Contributor

Hi @adityasaky ,
SLSA v1 is released. And I think this PR is up to date.
Can you please TAL at this PR and see if we can get the branch into main and cut a release?
Thanks!

Copy link
Member

@adityasaky adityasaky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @asraa! I think we can merge this overall.

However, I do want to hear from @in-toto/attestation-maintainers re using the models generated for ResourceDescriptor (and possibly Statement etc as well).

// ResourceDescriptor describes a particular software artifact or resource
// (mutable or immutable).
// See https://github.com/in-toto/attestation/blob/main/spec/v1.0/resource_descriptor.md
type ResourceDescriptor struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's a good idea. But given the proto only defines ResourceDescriptor and misses many other fields of SLSA v1 predicate, should we move on with current approach and cut a release first, and later switch over to models generated from proto once all v1 predicate fields are well defined in attestation repo?

Copy link
Contributor

@marcelamelara marcelamelara Apr 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my understanding, SLSA already defines full protos for SLSA provenance v1: https://github.com/slsa-framework/slsa/tree/main/docs/provenance/schema/v1/provenance.proto

We (in-toto attestation maintainers) were operating under the assumption that SLSA will maintain all SLSA-related predicate spec and proto versions. Based on your comment here, it sounds like there's an expectation that the in-toto attestation repo should maintain (a separate copy) of SLSA provenance v1 protos?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I was only looking at https://github.com/in-toto/attestation/tree/main/protos/in_toto_attestation/v1. I thought intoto will define its own proto again.

If intoto can reuse the proto from SLSA, that would be great. I have no option on this :D I'd defer intoto maintainers to decide this.

I am looking forward to seeing an intoto-golang release for SLSA v1!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it :) At least my preference would be to use the SLSA-defined protos for generating the language bindings for the predicate, so this integration is something we should definitely discuss on our end.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me! Is there a target date we can expect a release with the slsa v1 support? Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm okay to merge this PR and then next into main and cut a release to unblock other tools. My inclination is to switch it out behind the scenes later as we want to do that for multiple models, not just SLSA Provenance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 with @adityasaky can merge for now and switch behind the scenes in another release.

Copy link
Member

@adityasaky adityasaky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I do have a question about the package directory.

in_toto/slsa_provenance/v1.0/provenance.go Show resolved Hide resolved
// ResourceDescriptor describes a particular software artifact or resource
// (mutable or immutable).
// See https://github.com/in-toto/attestation/blob/main/spec/v1.0/resource_descriptor.md
type ResourceDescriptor struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm okay to merge this PR and then next into main and cut a release to unblock other tools. My inclination is to switch it out behind the scenes later as we want to do that for multiple models, not just SLSA Provenance.

Copy link
Member

@pxp928 pxp928 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

// ResourceDescriptor describes a particular software artifact or resource
// (mutable or immutable).
// See https://github.com/in-toto/attestation/blob/main/spec/v1.0/resource_descriptor.md
type ResourceDescriptor struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 with @adityasaky can merge for now and switch behind the scenes in another release.

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

Successfully merging this pull request may close these issues.

Support v1.0 SLSA provenance format
7 participants