[TEP-0091] Trusted resources alpha add sigstore packages #5552
Conversation
|
Skipping CI for Draft Pull Request. |
tekton-robot
added
do-not-merge/work-in-progress
tekton-robot
added
the
size/XL
Yongxuanzhang
changed the title
tekton-robot
removed
the
do-not-merge/work-in-progress
|
What I'm worried about is the huge added lines of code into vendor. 😟 |
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // clientsetscheme "k8s.io/client-go/kubernetes/scheme" | ||
| // aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" | ||
| // ) | ||
| // import ( |
There was a problem hiding this comment.
Looks like these are unnecessary tabs introduced.
Yongxuanzhang
force-pushed
the
trusted-resources-alpha-sign-verify
branch
from
97fd530
to
628943d
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.
|
| ) | ||
|
|
||
| // signInterface returns the encoded signature for the given object. | ||
| func signInterface(signer signature.Signer, i interface{}) (string, error) { |
There was a problem hiding this comment.
This function is not used in other packages at this commit, I guess we should make them unexported?
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
| "os" | ||
| "path/filepath" | ||
|
|
||
| "github.com/sigstore/cosign/pkg/cosign" |
There was a problem hiding this comment.
I'd drop the signing from this PR, since the pipeline controller will primarily be interested in verification only. (signing will likely be more relevant for CLI / CLI plugin).
In generaly, I'd avoid taking on the cosign dependency unless it's absolutely necessary - as you can see from the vendor directory, there's a ton of dependencies.
There was a problem hiding this comment.
I'm remembering now - you're using this for testing.
It looks like you're using cosign mostly for the key generation - you can drop this in favor of generating + loading the key directly via one of the load functions in https://pkg.go.dev/github.com/sigstore/sigstore/pkg/signature
e.g. you could replace most of this with something like:
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err ...
sv, err := signature.LoadECDSASignerVerifier(key, crypto.SHA256)
if err ...
return svThere was a problem hiding this comment.
if/when you need the multi-algo sv creator, use https://pkg.go.dev/github.com/sigstore/sigstore/pkg/signature#LoadSignerVerifier
There was a problem hiding this comment.
Sure! Thanks! I will try it out!
| "os" | ||
| "path/filepath" | ||
|
|
||
| "github.com/sigstore/cosign/pkg/cosign" |
There was a problem hiding this comment.
I'm remembering now - you're using this for testing.
It looks like you're using cosign mostly for the key generation - you can drop this in favor of generating + loading the key directly via one of the load functions in https://pkg.go.dev/github.com/sigstore/sigstore/pkg/signature
e.g. you could replace most of this with something like:
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err ...
sv, err := signature.LoadECDSASignerVerifier(key, crypto.SHA256)
if err ...
return sv| ) | ||
|
|
||
| func init() { | ||
| os.Setenv("SYSTEM_NAMESPACE", namespace) |
There was a problem hiding this comment.
I think the preferred way of doing this is is to import knative.dev/pkg/testing.
I'm also curious why you need this if we're not verifying any controller functionality yet 🤔
There was a problem hiding this comment.
This should be the piece of code I forgot to remove 🥲
| "os" | ||
| "path/filepath" | ||
|
|
||
| "github.com/sigstore/cosign/pkg/cosign" |
There was a problem hiding this comment.
if/when you need the multi-algo sv creator, use https://pkg.go.dev/github.com/sigstore/sigstore/pkg/signature#LoadSignerVerifier
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
tekton-robot
added
lgtm
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
Yongxuanzhang
force-pushed
the
trusted-resources-alpha-sign-verify
branch
from
a0574bf
to
3322d72
Compare
|
The following is the coverage report on the affected files.
|
Yongxuanzhang
force-pushed
the
trusted-resources-alpha-sign-verify
branch
2 times, most recently
from
775abbb
to
fec4d52
Compare
|
The following is the coverage report on the affected files.
|
Yongxuanzhang
force-pushed
the
trusted-resources-alpha-sign-verify
branch
from
fec4d52
to
287ddcb
Compare
|
The following is the coverage report on the affected files.
|
Yongxuanzhang
force-pushed
the
trusted-resources-alpha-sign-verify
branch
from
287ddcb
to
edde682
Compare
|
The following is the coverage report on the affected files.
|
This commit is part of the work of trusted resources, it introduces the dependency of sigstore by adding signing and verifying functions to the code base. Those functions are not invoked in this commit but will be used in the following work. update
Yongxuanzhang
force-pushed
the
trusted-resources-alpha-sign-verify
branch
from
edde682
to
99f6580
Compare
|
The following is the coverage report on the affected files.
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dibyom, 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 |
tekton-robot
added
the
approved
|
/lgtm |
Changes
This commit is part of the work of trusted resources, it introduces the
dependency of sigstore/sigstore by adding signing and verifying
functions to the code base. Those functions are not invoked in this
commit but will be used in the following work.
/kind feature
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