Verification helpers #72
Conversation
Signed-off-by: rgnote <[email protected]>
Signed-off-by: rgnote <[email protected]>
Signed-off-by: rgnote <[email protected]>
| // TrustStores this policy statement uses | ||
| TrustStores []string `json:"trustStores,omitempty"` |
There was a problem hiding this comment.
Shouldn't we update the spec first then the code?
There was a problem hiding this comment.
We discussed this change in yesterday's call. Made the code update as it is minor. If the spec deviates, I can come back and make those changes.
There was a problem hiding this comment.
@shizhMSFT I'll be submitting a spec PR for this, we discussed the change and use case in Thursday call . Rakesh has been working on verification workflow logic and it was simpler to update it in a single pass.
verification/helpers.go
Outdated
| func getArtifactDigestFromUri(artifactUri string) (string, error) { | ||
| i := strings.LastIndex(artifactUri, ":") | ||
| if i < 0 { | ||
| return "", fmt.Errorf("artifact URI %q could not be parsed, make sure it is the fully qualified OCI artifact URI without the scheme/protocol. e.g domain.com:80/my/repository:digest", artifactUri) |
There was a problem hiding this comment.
digest is digest like sha256:xxx or a tag like v1?
There was a problem hiding this comment.
Spec is not supporting tags yet. We can come back and update this code when we support tags.
There was a problem hiding this comment.
domain.com:80/my/repository:digest this reference string makes me confused. If it is for digest, it should be in the form of domain.com:80/my/repository@digest. That is, @ instead of :.
Signed-off-by: rgnote <[email protected]>
Signed-off-by: rgnote <[email protected]>
| // TrustStores this policy statement uses | ||
| TrustStores []string `json:"trustStores,omitempty"` |
There was a problem hiding this comment.
@shizhMSFT I'll be submitting a spec PR for this, we discussed the change and use case in Thursday call . Rakesh has been working on verification workflow logic and it was simpler to update it in a single pass.
Signed-off-by: rgnote <[email protected]>
Signed-off-by: rgnote <[email protected]>
|
lgtm |
verification/helpers.go
Outdated
| func getArtifactDigestFromUri(artifactUri string) (string, error) { | ||
| i := strings.LastIndex(artifactUri, ":") | ||
| if i < 0 { | ||
| return "", fmt.Errorf("artifact URI %q could not be parsed, make sure it is the fully qualified OCI artifact URI without the scheme/protocol. e.g domain.com:80/my/repository:digest", artifactUri) |
There was a problem hiding this comment.
domain.com:80/my/repository:digest this reference string makes me confused. If it is for digest, it should be in the form of domain.com:80/my/repository@digest. That is, @ instead of :.
verification/helpers_test.go
Outdated
| digest string | ||
| wantErr bool | ||
| }{ | ||
| {"domain.com:80/repository:digest", "digest", false}, |
There was a problem hiding this comment.
We should set wantErr: true for this case.
There was a problem hiding this comment.
Updated the code to accept domain.com:80/repository@<alg>:<digest>
My understanding is that <alg> is not included when querying manifests from the registry, is that correct? Or the entire thing <alg>:<digest> is considered a digest and we need to use that when resolving?
There was a problem hiding this comment.
NVM. Alg is needed to query with a digest. I addressed it in the latest revision
Signed-off-by: rgnote <[email protected]>
Signed-off-by: rgnote <[email protected]>
| func getArtifactDigestFromUri(artifactUri string) (string, error) { | ||
| invalidUriErr := fmt.Errorf("artifact URI %q could not be parsed, make sure it is the fully qualified OCI artifact URI without the scheme/protocol. e.g domain.com:80/my/repository@sha256:digest", artifactUri) | ||
| i := strings.LastIndex(artifactUri, "@") | ||
| if i < 0 || i+1 == len(artifactUri) { | ||
| return "", invalidUriErr | ||
| } | ||
|
|
||
| j := strings.LastIndex(artifactUri[i+1:], ":") | ||
| if j < 0 || j+1 == len(artifactUri[i+1:]) { | ||
| return "", invalidUriErr | ||
| } | ||
|
|
||
| return artifactUri[i+1:], nil | ||
| } |
There was a problem hiding this comment.
This block of code is workable but fragile. Try registry.ParseReference().
There was a problem hiding this comment.
Note: A digest is usually with the algorithm, not just the hash value.
* Add verification helpers * fix build Signed-off-by: rgnote <[email protected]> Signed-off-by: Junjie Gao <[email protected]>
Changes:
trustPolicy.TrustStoretotrustPolicy.TrustStoresTypefield to X509TrustStore to be able to filter trust stores based on their typeVerificationResultandSignatureVerificationOutsometypes to encapsulate verification results.