Switch to Grafeas v1 API in grafeas storage by chuangw6 · Pull Request #453 · tektoncd/chains

chuangw6 · 2022-05-23T18:53:25Z

Background

Tekton Chains captures two types of artifacts: OCI artifacts, TaskRun itself.
For OCI artifacts: Tekton Chains generates simplesigning attestation for it.
For TaskRun artifacts: Tekton Chains generates intoto attestation with SLSA 0.2 predicate for it.
(Chains also supports tekton format for taskrun artifact, but we want grafeas backend storage to only accept intoto format)

Changes Summary:

Switch from Grafeas v1beta1 api to Grafeas v1 api.
Store simplesigning attestation in grafeas ATTESTATION occurrence. (see the illustration diagram below)
Store intoto attestation in BUILD occurrence. Prior to this, intoto attestation was also stored in the ATTESTATION occurrence. (see the illustration diagram below)
If a taskrun generates X images
- X number of ATTESTATION occurrences will be created to store the X simplesigning payload, and the resourceUri of the ATTESTATION occurrences will be set to the image identifier IMAGE_URL@IMAGE_DIGEST.
- X number of BUILD occurrence will be created in order to associate the taskrun intoto payload with every single image generated from the taskrun, and the resourceUri will be also set to IMAGE_URL@IMAGE_DIGEST.
Grafeas Note:

since a grafeas occurrence has to be linked to a grafeas note, we need to create two notes (attestation note and build note)
- create ATTESTATION note to link ATTESTATION occurrence.
- create BUILD note to link BUILD occurrence.
- user-configured note name will be added a suffix ('-simplesigning'
  or '-intoto') to differentiate the two note types.

tekton-robot · 2022-05-23T18:57:08Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	0.0%	-78.6

tekton-robot · 2022-05-25T06:05:07Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	0.0%	-78.6

tekton-robot · 2022-05-25T06:09:13Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	0.0%	-78.6

tekton-robot · 2022-05-25T18:41:35Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	0.0%	-78.6

tekton-robot · 2022-06-23T15:11:06Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	78.8%	0.1

tekton-robot · 2022-06-23T15:17:08Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	78.8%	0.1

tekton-robot · 2022-06-23T15:18:11Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	78.8%	0.1

tekton-robot · 2022-06-23T15:27:40Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	78.8%	0.1

chuangw6 · 2022-06-23T15:29:59Z

cc @wlynch

tekton-robot · 2022-06-23T19:46:11Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	78.8%	0.1

wlynch · 2022-06-23T21:02:12Z

-}
+// retrieveOCIURIs returns the uri of a specific OCI artifact and the uris of all the images generated by the taskrun
+// If it's taskrun payload & sig (judged from config.StorageOpts), the first returned value will be empty string.
+func (b *Backend) retrieveOCIURIs(tr *v1beta1.TaskRun, opts config.StorageOpts) (string, []string) {


I don't fully understand the difference between the 2 return values here - why not just return []string?

I was trying to simplify the way we get the specific image uri when OCI artifact payload (simplesigning) and signatures are passed in, and the way to get the URIs of all images built from a taskrun when Taskrun artifact payload (intoto) are passed in. Basically, unify the retrievals into a single function.

If OCI artifact payload is passed, the first returned value will be the image uri of that image, and the second returned value contains the URIs of all images built by the taskrun.
If taskrun artifact payload is passed, the first returned value will be empty string, and the second returned value also contains the URIs of all images built by the taskrun.

why not just return []string?

we can do this. But when we need to get uri for a specific image artifact, we still need a filtering process somewhere else (either another helper function or just inline).

Let me know what you think. I am happy with both.

Having trouble following why this behavior would change based on the attestation format - wouldn't the URIs be the same in either case?

We only use the attestation format to tell whether the payload&sig are for OCI artifact OR TaskRun artifact.

The second returned value - URIs will be the same in either case.

But for the first returned value, only the OCI artifact is applicable to have a specific URI associated. TaskRun artifact is not applicable to have a specific URI associated, which is why the first returned value is empty string for taskrun artifact.

Discussed with @wlynch offline. We decided to keep separating the uri retrieval for taskrun/oci into two functions for readability. In future, we might want to set storageOpts's key for OCI artifact to be the full representation so that we don't need to have an extra function to get the full digest for it. #476

wlynch · 2022-06-23T21:11:45Z

+
+func (b *Backend) createBuildOccurrence(ctx context.Context, tr *v1beta1.TaskRun, payload []byte, signature string, uri string) (*pb.Occurrence, error) {
+	in := intoto.ProvenanceStatement{}
+	if err := json.Unmarshal(payload, &in); err != nil {


It'd be interesting to just pass the intoto struct reference around rather than generating the intoto statement -> serializing it -> de-serializing it -> converting it to grafeas.

Might be worth looking into to simplify things (we can also do this in another PR if needed)

Yeah, agree that for grafeas backend, there are extra steps as you just described.

If we want to simplify that, we might need to change the function StorePayload's signature. But the sacrifice is that other backend options have to serialize when StorePayload is called, right?

Yeah... I think that's okay though? Seems reasonable to keep it as it's full spec and only serialize when we actually make the request. Fine for this now, but let's look into cleaning this up in another PR.

tekton-robot · 2022-06-24T14:22:57Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	78.8%	0.1

tekton-robot · 2022-07-04T15:57:25Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	78.6%	-0.0

tekton-robot · 2022-07-04T18:22:54Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	78.6%	-0.0

wlynch

Looking good! Just a few last cleanups.

Main Changes: 1. Grafeas Note - create ATTESTATION note to link ATTESTATION occurrence. - create BUILD note to link BUILD occurrence. - user-configured note name will be added a suffix ('-simplesigning' or '-intoto') to differentiate the two note types. 2. Grafeas Occurrence - For oci artifact - create ATTESTATION occurrence per OCI artifact. - the ATTESTATION occurrence contains simplesigning attestation. - For taskrun artifact - create BUILD occurrence per container image built by the taskrun. - the BUILD occurrence contains intoto statement with SLSA0.2 predicate.

tekton-robot · 2022-07-05T19:58:55Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/grafeas/grafeas.go	78.6%	78.1%	-0.6

wlynch · 2022-07-06T17:59:04Z

/lgtm
/approve

tekton-robot · 2022-07-06T17:59:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wlynch

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [wlynch]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 23, 2022

tekton-robot requested review from dlorenc and mattmoor May 23, 2022 18:53

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 23, 2022

chuangw6 force-pushed the grafeas-v1 branch from 0a4ea74 to 6be9d3e Compare May 25, 2022 06:03

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 2, 2022

chuangw6 force-pushed the grafeas-v1 branch from c678fac to 655626f Compare June 23, 2022 15:09

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 23, 2022

chuangw6 force-pushed the grafeas-v1 branch from 655626f to 5dfcdd9 Compare June 23, 2022 15:15

chuangw6 marked this pull request as ready for review June 23, 2022 15:15

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 23, 2022

chuangw6 force-pushed the grafeas-v1 branch from 5dfcdd9 to 7f16009 Compare June 23, 2022 15:16

chuangw6 force-pushed the grafeas-v1 branch from 7f16009 to 3a90ba0 Compare June 23, 2022 15:25

wlynch requested changes Jun 23, 2022

View reviewed changes

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 24, 2022

chuangw6 force-pushed the grafeas-v1 branch from 46c5f70 to 7e1e58d Compare June 24, 2022 14:20

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 24, 2022

chuangw6 requested a review from wlynch June 27, 2022 14:06

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2022

chuangw6 force-pushed the grafeas-v1 branch from 7e1e58d to 3792941 Compare July 4, 2022 14:20

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 4, 2022

chuangw6 force-pushed the grafeas-v1 branch from 3792941 to 98a4a82 Compare July 4, 2022 15:45

chuangw6 force-pushed the grafeas-v1 branch from 98a4a82 to e44deed Compare July 4, 2022 18:20

wlynch reviewed Jul 5, 2022

View reviewed changes

Comment thread pkg/chains/storage/grafeas/grafeas.go Outdated

Comment thread pkg/config/config.go Outdated

Comment thread config/100-rolebinding.yaml Outdated

Comment thread examples/kaniko/taskrun.yaml Outdated

chuangw6 force-pushed the grafeas-v1 branch from e44deed to 44ae423 Compare July 5, 2022 19:39

chuangw6 requested a review from wlynch July 5, 2022 19:53

tekton-robot assigned wlynch Jul 6, 2022

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 6, 2022

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 6, 2022

tekton-robot merged commit b1c8b5a into tektoncd:main Jul 6, 2022

chuangw6 mentioned this pull request Aug 23, 2022

Add Grafeas as a storage backend #358

Closed

Uh oh!

Conversation

chuangw6 commented May 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Changes Summary:

Uh oh!

tekton-robot commented May 23, 2022

Uh oh!

tekton-robot commented May 25, 2022

Uh oh!

tekton-robot commented May 25, 2022

Uh oh!

tekton-robot commented May 25, 2022

Uh oh!

tekton-robot commented Jun 23, 2022

Uh oh!

tekton-robot commented Jun 23, 2022

Uh oh!

tekton-robot commented Jun 23, 2022

Uh oh!

tekton-robot commented Jun 23, 2022

Uh oh!

chuangw6 commented Jun 23, 2022

Uh oh!

tekton-robot commented Jun 23, 2022

Uh oh!

wlynch Jun 23, 2022

Choose a reason for hiding this comment

Uh oh!

chuangw6 Jun 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wlynch Jun 28, 2022

Choose a reason for hiding this comment

Uh oh!

chuangw6 Jun 28, 2022

Choose a reason for hiding this comment

Uh oh!

chuangw6 Jul 5, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wlynch Jun 23, 2022

Choose a reason for hiding this comment

Uh oh!

chuangw6 Jun 24, 2022

Choose a reason for hiding this comment

Uh oh!

wlynch Jun 28, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tekton-robot commented Jun 24, 2022

Uh oh!

tekton-robot commented Jul 4, 2022

Uh oh!

tekton-robot commented Jul 4, 2022

Uh oh!

wlynch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tekton-robot commented Jul 5, 2022

Uh oh!

wlynch commented Jul 6, 2022

Uh oh!

tekton-robot commented Jul 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

chuangw6 commented May 23, 2022 •

edited

Loading

chuangw6 Jun 23, 2022 •

edited

Loading