Set ConfigSource in bundleresolver #5684

chuangw6 · 2022-10-25T14:49:03Z

Changes

/kind feature

Related to #5522

Prior, a field named Source was introduced to ResolutionRequest status to record the source where the remote resource came from. And the individual resolvers need to implement the Source function to set the correct source value. But the method in ociresolver returns a nil value.

Now, we return correct source value with the 3 subfields: url, digest and entrypoint

url: The image repository URI
digest: The map of the algorithm portion -> the hex encoded portion of the image digest.
entrypoint: The resource name in the OCI bundle image.

Signed-off-by: Chuang Wang chuangw@google.com

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs included if any changes are user facing
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

ociresolver captures correct source information about where remote image came from.

chuangw6 · 2022-10-25T20:22:22Z

Hey @jagathprakash @wlynch,

Please take a look at this PR when you have a minute. This PR aims to set a correct source value for the OCI bundle image. Thank you!!

/assign @wlynch
/assign @jagathprakash

tekton-robot · 2022-10-25T20:22:24Z

@chuangw6: GitHub didn't allow me to assign the following users: jagathprakash.

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.
For more information please see the contributor guide

In response to this:

Hey @jagathprakash @wlynch,

Please take a look at this PR when you have a minute. This PR aims to set a correct source value for the OCI bundle image. Thank you!!

/assign @wlynch
/assign @jagathprakash

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.

JeromeJu · 2022-10-28T16:04:03Z

pkg/resolution/resolver/bundle/bundle.go

 			}, nil
 		}
 	}
 	return nil, fmt.Errorf("could not find object in image with kind: %s and name: %s", opts.Kind, opts.EntryName)
 }

 // retrieveImage will fetch the image's contents and manifest.
-func retrieveImage(ctx context.Context, keychain authn.Keychain, ref string) (v1.Image, error) {
+func retrieveImage(ctx context.Context, keychain authn.Keychain, ref string) (string, v1.Image, error) {


Thanks Chuang!
Can we also update the comment above for the return values?

Good catch! Thanks @JeromeJu . Updated!

wlynch

Looks good! Can we add tests?

See https://github.com/tektoncd/pipeline/blob/76e40b5a7b11262bfaa96ae31f28db2009002115/test/remote_test.go for an example of how to setup a local test registry

wlynch · 2022-10-28T16:57:41Z

pkg/resolution/resolver/bundle/bundle.go

-	img, err := retrieveImage(ctx, keychain, opts.Bundle)
+	uri, img, err := retrieveImage(ctx, keychain, opts.Bundle)
+	if err != nil {
+		return nil, err


These errors may be a bit difficult to know where they come from on their own - you may want to wrap them with a fmt.Errorf

Sounds good! Changed.

chuangw6 · 2022-11-01T13:52:58Z

Looks good! Can we add tests?

See https://github.com/tektoncd/pipeline/blob/76e40b5a7b11262bfaa96ae31f28db2009002115/test/remote_test.go for an example of how to setup a local test registry

Sure. That's indeed missed. I opened a new PR to add the test for easier review. PTAL #5704 @wlynch . Thank you!!

abayer · 2022-11-03T17:10:55Z

/approve

tekton-robot · 2022-11-10T01:02:41Z

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

File	Old Coverage	New Coverage	Delta
pkg/resolution/resolver/bundle/bundle.go	72.1%	72.2%	0.2

wlynch

/lgtm

JeromeJu · 2022-11-10T14:31:30Z

/retest

chuangw6 · 2022-11-10T14:40:35Z

Just updated the unit test a bit to compare the source field in resolutionrequest status. The reason is that we need to test the digest can be successfully resolved no matter the given bundle param has actual digest or a tag name. PTAL. Let me know if you have any feedback. Thanks @wlynch

tekton-robot · 2022-11-10T14:41:02Z

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

File	Old Coverage	New Coverage	Delta
pkg/resolution/resolver/bundle/bundle.go	72.1%	72.2%	0.2

tekton-robot · 2022-11-10T14:48:44Z

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

File	Old Coverage	New Coverage	Delta
pkg/resolution/resolver/bundle/bundle.go	72.1%	72.2%	0.2

tekton-robot · 2022-11-10T14:59:49Z

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

File	Old Coverage	New Coverage	Delta
pkg/resolution/resolver/bundle/bundle.go	72.1%	72.2%	0.2

Prior, a field named Source was introduced to `ResolutionRequest` status to record the source where the remote resource came from. And the individual resolvers need to implement the Source function to set the correct source value. But the method in ociresolver returns a nil value. Now, we return correct source value with the 3 subfields: url, digest and entrypoint - `uri`: The image repository URI - `digest`: The map of the algorithm portion -> the hex encoded portion of the image digest. - `entrypoint`: The resource name in the OCI bundle image. Signed-off-by: Chuang Wang <chuangw@google.com>

tekton-robot · 2022-11-10T19:43:16Z

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

File	Old Coverage	New Coverage	Delta
pkg/resolution/resolver/bundle/bundle.go	72.1%	72.2%	0.2

wlynch

/lgtm

tekton-robot · 2022-11-10T21:03:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abayer, 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:

~~OWNERS~~ [abayer]

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 release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Oct 25, 2022

tekton-robot requested review from bobcatfish and vdemeester October 25, 2022 14:49

tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 25, 2022

chuangw6 force-pushed the source-in-ociresolver branch from a2c7b52 to d47c22d Compare October 25, 2022 14:51

chuangw6 mentioned this pull request Oct 25, 2022

Set Source information in ResolutionRequest #5522

Closed

tekton-robot assigned wlynch Oct 25, 2022

JeromeJu reviewed Oct 28, 2022

View reviewed changes

wlynch requested changes Oct 28, 2022

View reviewed changes

chuangw6 force-pushed the source-in-ociresolver branch from d47c22d to 66ef330 Compare November 1, 2022 13:51

chuangw6 force-pushed the source-in-ociresolver branch from 66ef330 to 53a1058 Compare November 1, 2022 13:54

chuangw6 changed the title ~~Set ConfigSource in ociresolver~~ Set ConfigSource in bundleresolver Nov 2, 2022

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

chuangw6 requested review from wlynch and removed request for vdemeester and bobcatfish November 4, 2022 18:54

chuangw6 force-pushed the source-in-ociresolver branch from 53a1058 to 2e63aaf Compare November 10, 2022 00:58

wlynch reviewed Nov 10, 2022

View reviewed changes

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

chuangw6 force-pushed the source-in-ociresolver branch from 2e63aaf to 4a1f6cd Compare November 10, 2022 14:36

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 10, 2022

chuangw6 requested a review from wlynch November 10, 2022 14:40

chuangw6 force-pushed the source-in-ociresolver branch from 4a1f6cd to 9bc5f7f Compare November 10, 2022 14:42

chuangw6 force-pushed the source-in-ociresolver branch from 9bc5f7f to bec38a2 Compare November 10, 2022 14:54

chuangw6 requested review from JeromeJu and wlynch and removed request for wlynch and JeromeJu November 10, 2022 17:25

chuangw6 force-pushed the source-in-ociresolver branch from bec38a2 to 44ca742 Compare November 10, 2022 19:38

wlynch approved these changes Nov 10, 2022

View reviewed changes

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

tekton-robot merged commit c3bd8a3 into tektoncd:main Nov 10, 2022

chuangw6 mentioned this pull request Nov 16, 2022

Chains needs to be able to "Identify entry point" - SLSA L3 requirement. tektoncd/chains#521

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set ConfigSource in bundleresolver #5684

Set ConfigSource in bundleresolver #5684

chuangw6 commented Oct 25, 2022 •

edited

Loading

chuangw6 commented Oct 25, 2022

tekton-robot commented Oct 25, 2022

JeromeJu Oct 28, 2022

chuangw6 Nov 1, 2022

wlynch left a comment

wlynch Oct 28, 2022

chuangw6 Nov 1, 2022

chuangw6 commented Nov 1, 2022

abayer commented Nov 3, 2022

tekton-robot commented Nov 10, 2022

wlynch left a comment

JeromeJu commented Nov 10, 2022

chuangw6 commented Nov 10, 2022 •

edited

Loading

tekton-robot commented Nov 10, 2022

tekton-robot commented Nov 10, 2022

tekton-robot commented Nov 10, 2022

tekton-robot commented Nov 10, 2022

wlynch left a comment

tekton-robot commented Nov 10, 2022

Set ConfigSource in bundleresolver #5684

Set ConfigSource in bundleresolver #5684

Conversation

chuangw6 commented Oct 25, 2022 • edited Loading

Changes

Submitter Checklist

Release Notes

chuangw6 commented Oct 25, 2022

tekton-robot commented Oct 25, 2022

JeromeJu Oct 28, 2022

Choose a reason for hiding this comment

chuangw6 Nov 1, 2022

Choose a reason for hiding this comment

wlynch left a comment

Choose a reason for hiding this comment

wlynch Oct 28, 2022

Choose a reason for hiding this comment

chuangw6 Nov 1, 2022

Choose a reason for hiding this comment

chuangw6 commented Nov 1, 2022

abayer commented Nov 3, 2022

tekton-robot commented Nov 10, 2022

wlynch left a comment

Choose a reason for hiding this comment

JeromeJu commented Nov 10, 2022

chuangw6 commented Nov 10, 2022 • edited Loading

tekton-robot commented Nov 10, 2022

tekton-robot commented Nov 10, 2022

tekton-robot commented Nov 10, 2022

tekton-robot commented Nov 10, 2022

wlynch left a comment

Choose a reason for hiding this comment

tekton-robot commented Nov 10, 2022

chuangw6 commented Oct 25, 2022 •

edited

Loading

chuangw6 commented Nov 10, 2022 •

edited

Loading