-
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
Set ConfigSource in clusterresolver #5687
Set ConfigSource in clusterresolver #5687
Conversation
The following is the coverage report on the affected files.
|
2dc2d1d
to
426e02a
Compare
The following is the coverage report on the affected files.
|
pipeline.Kind = "Pipeline" | ||
pipeline.APIVersion = "tekton.dev/v1beta1" | ||
pipeline.APIVersion = groupVersion |
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.
Out of curiosity, are we also going to support v1
in the remote resolution after V1 release?
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.
Yeah, we will. Basically, this should always be whatever version is used in the PipelineRun
reconciler by default, but we should probably add a param for apiVersion
to allow overriding that...
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.
Out of curiosity, are we also going to support
v1
in the remote resolution after V1 release?
Yeah agreed. We will need to support v1 in remote resolution. Thanks for asking @XinruZhang .
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.
+1 for a new param for apiVersion, it looks like a future refactor work XD
/hold Since resources in cluster are not in version control which is desired for slsa provenance, the source value might be meaningless. We need more discussions around this. |
The following is the coverage report on the affected files.
|
a4c57fd
to
d96e214
Compare
The following is the coverage report on the affected files.
|
d96e214
to
86fef92
Compare
3c9a646
to
00dd28a
Compare
00dd28a
to
6e74588
Compare
The following is the coverage report on the affected files.
|
/retest |
/approve |
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.
/lgtm
[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:
Approvers can indicate their approval by writing |
Related to tektoncd#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 clusterresolver returns a nil value. Now, we return correct source value with the 3 subfields: url, digest and entrypoint - url: [Kubernetes CRD namespace-scoped resource URI](https://kubernetes.io/docs/reference/using-api/api-concepts/#resource-uris) appended with UID. Example: /apis/GROUP/VERSION/namespaces/NAMESPACE/RESOURCETYPE/NAME@UID. - digest: hex-encoded sha256 checksum of the in-cluster resource - entrypoint: ***empty*** because the path is already available in url field. Signed-off-by: Chuang Wang <[email protected]>
6e74588
to
765bf8a
Compare
Thanks for the lgtm @wlynch. |
The following is the coverage report on the affected files.
|
/retest |
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.
/lgtm
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 clusterresolver returns a nil value.
Now, we return correct source value with the 3 subfields: url, digest and entrypoint
Example: /apis/GROUP/VERSION/namespaces/NAMESPACE/RESOURCETYPE/NAME@UID.
Signed-off-by: Chuang Wang [email protected]
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