Add cluster remote resolver by abayer · Pull Request #5405 · tektoncd/pipeline

abayer · 2022-08-31T15:40:54Z

Changes

Relates to #4476 and https://github.com/tektoncd/community/blob/main/teps/0096-pipelines-v1-api.md

This resolver replaces ClusterTasks going forward.

/kind feature

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

Add a cluster resolver for referencing Pipeline and Task resources in other namespaces

tekton-robot · 2022-08-31T15:45:32Z

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/apis/config/feature_flags.go	79.0%	78.1%	-0.9
pkg/resolution/resolver/cluster/resolver.go	Do not exist	80.6%

abayer · 2022-08-31T15:49:42Z

/retest

abayer · 2022-08-31T15:57:43Z

/retest

abayer · 2022-08-31T16:34:57Z

/assign @vdemeester @jerop @lbernick
cc @chuangw6

lbernick · 2022-08-31T18:56:49Z

docs/cluster-resolver.md

+
+| Option Name    | Description                                                        | Example Values     |
+|----------------|--------------------------------------------------------------------|--------------------|
+| `default-kind` | The default resource kind to fetch if not specified in parameters. | `task`, `pipeline` |


maybe worth having a default namespace?

Yeah, seems reasonable. Adding that now.

docs/cluster-resolver.md

pkg/resolution/resolver/cluster/resolver_test.go

tekton-robot · 2022-08-31T20:23:46Z

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/apis/config/feature_flags.go	79.0%	78.1%	-0.9
pkg/resolution/resolver/cluster/resolver.go	Do not exist	82.8%

abayer · 2022-08-31T20:32:12Z

/retest

vdemeester

Does the resolver have access to all namespace (by default, in what we ship / will ship) ? It might not have (or shouldn't ?), and thus allowing the user to specify the namespace "might" be prohibited. I don't think that's a problem, but we might want to document that.

vdemeester · 2022-09-01T14:33:35Z

config/resolvers/cluster-resolver-config.yaml

+    app.kubernetes.io/part-of: tekton-pipelines
+data:
+  # The default kind to fetch.
+  default-kind: "task"


What is this used for ? 🤔 if we do not pass kind param right ?

#5408 would handle this for "from pipeline usage" I guess, but when creating the request aside from that, I think it make sense 👼🏼

Yeah, I'm torn as to whether to include this default or not. We do need to know the kind so we know whether to use the Task or Pipeline lookup, but the git resolver doesn't actually need to know, nor will the coming go-scm-based one. The bundle and hub ones also do need to know, so they can look in the right bundle layer/directory respectively.

abayer · 2022-09-01T14:54:33Z

Does the resolver have access to all namespace (by default, in what we ship / will ship) ? It might not have (or shouldn't ?), and thus allowing the user to specify the namespace "might" be prohibited. I don't think that's a problem, but we might want to document that.

Good question? With https://github.com/tektoncd/pipeline/pull/5405/files#diff-ce83974e71e344e3684dc4c1d40b14800038482fa264e3111f08c83827d60fc6R28-R30, the resolver should, I think?

vdemeester · 2022-09-01T15:10:38Z

Good question? With https://github.com/tektoncd/pipeline/pull/5405/files#diff-ce83974e71e344e3684dc4c1d40b14800038482fa264e3111f08c83827d60fc6R28-R30, the resolver should, I think?

Indeed 👼🏼

abayer · 2022-09-01T15:24:42Z

Ok, so that's a really good question: should we allow access to all namespaces? Or perhaps more accurately, should we have a way of only allowing and/or blocking access to a specified list of namespaces? I'm leaning towards yes...

abayer · 2022-09-01T15:49:13Z

Alright, I'm adding new allowed-namespaces and blocked-namespaces config fields. They're both optional comma-separated lists of namespaces. If they're empty, all namespaces can be used. If allowed-namespaces contains namespaces, then the requested namespace must be in that list, and if blocked-namespaces contains namespaces, then the requested namespace must not be in that list.

Relates to tektoncd#4476 and https://github.com/tektoncd/community/blob/main/teps/0096-pipelines-v1-api.md This resolver replaces `ClusterTask`s going forward. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

tekton-robot · 2022-09-01T15:55:08Z

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/apis/config/feature_flags.go	79.0%	78.1%	-0.9
pkg/resolution/resolver/cluster/resolver.go	Do not exist	84.7%

vdemeester

code LGTM 🐯

tekton-robot · 2022-09-05T08:19:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lbernick, vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [lbernick,vdemeester]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

abayer · 2022-09-05T13:53:31Z

/test check-pr-has-kind-label

tekton-robot · 2022-09-05T13:53:34Z

@abayer: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-tekton-pipeline-alpha-integration-tests
/test pull-tekton-pipeline-build-tests
/test pull-tekton-pipeline-integration-tests
/test tekton-pipeline-unit-tests

The following commands are available to trigger optional jobs:

/test pull-tekton-pipeline-go-coverage

Use /test all to run all jobs.

Details

In response to this:

/test check-pr-has-kind-label

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.

abayer · 2022-09-06T16:23:28Z

ping @lbernick @jerop - looking for an lgtm. =)

lbernick · 2022-09-06T19:43:28Z

/lgtm

abayer added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 31, 2022

tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Aug 31, 2022

tekton-robot requested review from imjasonh and pritidesai August 31, 2022 15:41

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 31, 2022

abayer mentioned this pull request Aug 31, 2022

Populate the *Run.Status.Provenance.ConfigSource field #5397

Merged

7 tasks

tekton-robot assigned jerop, lbernick and vdemeester Aug 31, 2022

lbernick approved these changes Aug 31, 2022

View reviewed changes

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

abayer force-pushed the cluster-resolver branch 2 times, most recently from 4323579 to e7cc98f Compare August 31, 2022 20:18

This was referenced Aug 31, 2022

Default kind in pipelineRef and taskRef resolver params if not specified #5408

Closed

ResolutionRequest error getting resource is kind of ugly #5409

Closed

vdemeester reviewed Sep 1, 2022

View reviewed changes

Add cluster resolver

98879a4

Relates to tektoncd#4476 and https://github.com/tektoncd/community/blob/main/teps/0096-pipelines-v1-api.md This resolver replaces `ClusterTask`s going forward. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

abayer force-pushed the cluster-resolver branch from e7cc98f to 98879a4 Compare September 1, 2022 15:49

tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 1, 2022

abayer mentioned this pull request Sep 1, 2022

Doc about migrating from v1beta1 to v1 #5415

Merged

6 tasks

vdemeester approved these changes Sep 5, 2022

View reviewed changes

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

tekton-robot merged commit 6074fa4 into tektoncd:main Sep 6, 2022

Conversation

abayer commented Aug 31, 2022

Changes

Submitter Checklist

Release Notes

Uh oh!

tekton-robot commented Aug 31, 2022

Uh oh!

abayer commented Aug 31, 2022

Uh oh!

abayer commented Aug 31, 2022

Uh oh!

abayer commented Aug 31, 2022

Uh oh!

lbernick Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

abayer Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tekton-robot commented Aug 31, 2022

Uh oh!

abayer commented Aug 31, 2022

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

vdemeester Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

vdemeester Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

abayer Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

abayer commented Sep 1, 2022

Uh oh!

vdemeester commented Sep 1, 2022

Uh oh!

abayer commented Sep 1, 2022

Uh oh!

abayer commented Sep 1, 2022

Uh oh!

tekton-robot commented Sep 1, 2022

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

tekton-robot commented Sep 5, 2022

Uh oh!

abayer commented Sep 5, 2022

Uh oh!

tekton-robot commented Sep 5, 2022

Uh oh!

abayer commented Sep 6, 2022

Uh oh!

lbernick commented Sep 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants