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

TEP-0122 complete build instructions and parameters #820

Conversation

chitrangpatel
Copy link
Contributor

This proposal outlines what information is required to reproduce complete build instructions for taskruns. It also suggests where in the provenance to store this information.

@chitrangpatel chitrangpatel marked this pull request as draft September 15, 2022 15:46
@tekton-robot tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 15, 2022
@chitrangpatel chitrangpatel force-pushed the TEP-0122-reproducibility-of-complete-build-instructions branch 3 times, most recently from a94d563 to df8d557 Compare September 20, 2022 15:44
@tekton-robot tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 20, 2022
@chitrangpatel chitrangpatel force-pushed the TEP-0122-reproducibility-of-complete-build-instructions branch 7 times, most recently from 18d4a0f to 406d659 Compare September 20, 2022 16:22
@chitrangpatel chitrangpatel marked this pull request as ready for review September 20, 2022 16:22
@tekton-robot 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 Sep 20, 2022
@chitrangpatel
Copy link
Contributor Author

chitrangpatel commented Sep 20, 2022

/assign @wlynch
(@chuangw6 suggested that you might be interested in this PR 🙂 )

@chitrangpatel
Copy link
Contributor Author

/assign @wlynch

@chitrangpatel
Copy link
Contributor Author

cc @chuangw6

@lbernick
Copy link
Member

/kind tep

@tekton-robot tekton-robot added the kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). label Sep 26, 2022
@dibyom
Copy link
Member

dibyom commented Sep 26, 2022

/assign @vdemeester

@chitrangpatel chitrangpatel force-pushed the TEP-0122-reproducibility-of-complete-build-instructions branch 2 times, most recently from 3dd2f19 to 94de42b Compare November 21, 2022 14:56
@pritidesai
Copy link
Member

API WG - ready for review, please take a look 🙏

Copy link
Contributor

@bobcatfish bobcatfish left a comment

Choose a reason for hiding this comment

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

I had initially suggested we frame these changes around 'reproducibility' but I think that has caused some confusion (since the SLSA reproducible requirement requires very little from the build platform and I don't think we should try to go beyond that at least for now) and I think we are actually trying to meet two other SLSA provenance requirements:

  1. SLSA L1: Identifies build instructions
  2. SLSA L3: Includes all build parameters

I think it boils down to something like this:

  1. 'build instructions' for Tekton are the 'authoring time info' i.e. the Task/Pipeline defintion
  2. 'build parameters' for Tekton are the 'runtime info' that is provided (i.e. the info a TaskRun/PipelineRun provides in order to realize execution of at Task/Pipeline)

Our 'build instructions' are currently incomplete (or were 6 months ago anyway XD) b/c we just grab the steps and not the entire Task definition (and we need to now include Pipeline definitions for PipelineRun level provenance). If "build as code" is used, the SLSA requirement is just that we identify where the Task/Pipeline lives in version control. If it isn't, we need to reproduce the Task/Pipeline in the provenance (we can decide if we want to include it in the build as code case as well).

We also need to make sure we are including all of the param/runtime info as well. (You're probably right to include the invocation.environment info here as well BUT I think you would also be reasonable to declare that out of scope and tackle is separately)

So TL;DR, if you agree, for clarity I'd personally rename this TEP to something like "complete build instructions and parameters" - and maybe remove requirements around reproducibility as well.

@chitrangpatel
Copy link
Contributor Author

chitrangpatel commented Nov 29, 2022

I had initially suggested we frame these changes around 'reproducibility' but I think that has caused some confusion (since the SLSA reproducible requirement requires very little from the build platform and I don't think we should try to go beyond that at least for now) and I think we are actually trying to meet two other SLSA provenance requirements:

  1. SLSA L1: Identifies build instructions
  2. SLSA L3: Includes all build parameters

I think it boils down to something like this:

  1. 'build instructions' for Tekton are the 'authoring time info' i.e. the Task/Pipeline defintion
  2. 'build parameters' for Tekton are the 'runtime info' that is provided (i.e. the info a TaskRun/PipelineRun provides in order to realize execution of at Task/Pipeline)

Our 'build instructions' are currently incomplete (or were 6 months ago anyway XD) b/c we just grab the steps and not the entire Task definition (and we need to now include Pipeline definitions for PipelineRun level provenance). If "build as code" is used, the SLSA requirement is just that we identify where the Task/Pipeline lives in version control. If it isn't, we need to reproduce the Task/Pipeline in the provenance (we can decide if we want to include it in the build as code case as well).

We also need to make sure we are including all of the param/runtime info as well. (You're probably right to include the invocation.environment info here as well BUT I think you would also be reasonable to declare that out of scope and tackle is separately)

I can certainly do that. Things around that are creating a lot of confusion and raising questions that probably cannot be answered immediately.

So TL;DR, if you agree, for clarity I'd personally rename this TEP to something like "complete build instructions and parameters" - and maybe remove requirements around reproducibility as well.

Sounds good. Updated the TEP to reflect your suggestions. Let me know what you think.

@chitrangpatel chitrangpatel force-pushed the TEP-0122-reproducibility-of-complete-build-instructions branch 2 times, most recently from 47ab991 to ba5d586 Compare November 29, 2022 16:51
@chitrangpatel chitrangpatel changed the title TEP-0122 reproducibility of complete build instructions TEP-0122 complete build instructions and parameters Nov 29, 2022
@chitrangpatel chitrangpatel force-pushed the TEP-0122-reproducibility-of-complete-build-instructions branch from ba5d586 to b95cca4 Compare November 29, 2022 16:55
@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 29, 2022
- For completeness of build instructions, the provenance should contain all of the Task definition and runtime information.
- The shas of the images run should continue to be included regardless (as this is part of the dependencies / build instructions)
- It must be possible for users to construct policies based on the provenance that would allow them to determine if the build is ok - for example if they have policies around what pipeline tasks are acceptable to use, what parameters are acceptable, whether or not sidecars are allowed etc.
- According to [SLSA v0.2](https://slsa.dev/provenance/v0.2), if **invocation.configSource** is not available then **buildConfig** can be used to verify information about the build. If the policy requires validating the underlying task/pipeline spec that was not fetched from version control then the information from the spec required for validation must be embedded in the provenance’s buildConfig.
Copy link
Member

Choose a reason for hiding this comment

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

If I understand this correctly, when the resource definition is not coming from a versioned source, we use buildConfig to store it directly in the provenance, correct?
Would it make sense later on in the spec to specify the list of sources that are considered ok for configSource?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand this correctly, when the resource definition is not coming from a versioned source, we use buildConfig to store it directly in the provenance, correct?

Yes, that is correct.

Would it make sense later on in the spec to specify the list of sources that are considered ok for configSource?

My understanding is that SLSA needs the references need to be immutable. We support three: GitHub, OCI bundles and hub.

Copy link
Member

@afrittoli afrittoli 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 for this, it seems a fine proposal to me.
I have some questions / comments, but they could also be addressed in the next PR.

/approve

This proposal outlines what information is required in the provenance to reproduce complete build instructions for task runs. It also suggests where in the provenance to store this information.
@chitrangpatel chitrangpatel force-pushed the TEP-0122-reproducibility-of-complete-build-instructions branch from b95cca4 to 9aed990 Compare December 1, 2022 21:09
@bobcatfish
Copy link
Contributor

Looks great, thanks @chitrangpatel !

/approve

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli, bobcatfish, wlynch

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@afrittoli
Copy link
Member

From WG on Dec 5th
/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Status: UnAssigned
Development

Successfully merging this pull request may close these issues.