Add pipelineRef remote resolution #4596

ghost · 2022-02-17T21:40:42Z

Changes

Prior to this commit the API fields for remote resolution were just
stubs with no functionality.

This commit adds initial functionality: support in pipelineRef. You can now pull in pipelines directly from a git repo. This feature is in alpha. To actually see anything happening a dev also needs to deploy tektoncd/resolution and the gitresolver.

/kind feature

Submitter Checklist

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

Docs included if any changes are user facing
Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Release notes block below has been filled in or deleted (only if no user facing changes)

Release Notes

Initial integration with tektoncd/resolution project, allowing pipelineRefs to be resolved directly from public git repos.

tekton-robot · 2022-02-17T21:42:39Z

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/pipeline/v1beta1/pipeline_types.go	94.7%	94.9%	0.2
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.1%	-4.7
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.2
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.6%	-3.6
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

tekton-robot · 2022-02-17T21:45:13Z

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/pipeline/v1beta1/pipeline_types.go	94.7%	94.9%	0.2
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.1%	-4.7
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.2
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.6%	-3.6
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

tekton-robot · 2022-02-17T21:46:10Z

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/pipeline/v1beta1/pipeline_types.go	94.7%	94.9%	0.2
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.1%	-4.7
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.2
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.6%	-3.6
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

tekton-robot · 2022-02-17T21:46:54Z

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/pipeline/v1beta1/pipeline_types.go	94.7%	94.9%	0.2
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.1%	-4.7
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.1
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.6%	-3.6
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

tekton-robot · 2022-02-17T21:57:12Z

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/pipeline/v1beta1/pipeline_types.go	94.7%	94.9%	0.2
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.1%	-4.7
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.1
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.2%	-4.0
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

ghost · 2022-02-17T21:58:16Z

/hold

Holding for more documentation + more reconciler tests + better coverage numbers.

tekton-robot · 2022-02-17T22:20:21Z

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/pipeline/v1beta1/pipeline_types.go	94.7%	94.9%	0.2
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.1%	-4.7
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.1
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.2%	-4.0
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

ghost · 2022-02-17T22:20:28Z

Adding a short copy/pastable PipelineRun that can be used for manual testing:

apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  name: reqpr
spec:
  params:
  - name: name
    value: Scott
  pipelineRef:
    resolver: git
    resource:
    - name: url
      value: https://github.com/sbwsg/catalog.git
    - name: commit
      value: f0629ad5f9046bb8ff853f5303a5158e6dba409c
    - name: path
      value: /pipeline/simple/0.1/simple.yaml

tekton-robot · 2022-02-17T22:20: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/pipeline/v1beta1/pipeline_types.go	94.7%	94.9%	0.2
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.1%	-4.7
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.1
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.2%	-4.0
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

ghost · 2022-02-17T23:15:29Z

/test pull-tekton-pipeline-integration-tests

tekton-robot · 2022-02-18T01:20:12Z

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/pipeline/v1beta1/pipeline_types.go	94.7%	94.9%	0.2
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.1%	-4.7
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.1
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.2%	-4.0
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

tekton-robot · 2022-02-18T01:31:12Z

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/pipeline/v1beta1/pipeline_types.go	94.7%	94.9%	0.2
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.1%	-4.7
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.1
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.2%	-4.0
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

lbernick · 2022-02-18T14:48:58Z

pkg/reconciler/pipelinerun/pipelinerun.go

+	case errors.Is(err, resolution.ErrorRequestInProgress):
+		message := fmt.Sprintf("PipelineRun %s/%s awaiting remote resource", pr.Namespace, pr.Name)
+		pr.Status.MarkRunning(ReasonResolvingPipelineRef, message)
+		return nil


It seems like if a reconcile loop is triggered when the ref is being resolved, it'll stop reconciling, but what triggers another reconcile loop when the ref is done being resolved? It seems like (based on the tests) that the ref crd will be updated, and this will trigger another reconcile loop, but maybe worth adding a comment?

Yeah you're totally right about how this works. It's the event handler added to the ResourceRequest informer in pkg/reconciler/pipelinerun/controller.go that binds PipelineRun's reconciler to updates on ResourceRequests.

I am not totally sure this is the right spot for a comment on that specific mechanism though, since it's pretty far removed from it. Commenting the AddEventHandler( call in pkg/reconciler/pipelinerun/controller.go in turn feels like it would be restating what the code there already says.

What info and/or location in the code would be most helpful to document this d'you think?

I think what is specifically confusing is that the remote resolution is returning an error, but the pipelinerun resolution is not returning an error. For taskruns contained in pipelineruns, if a taskrun is still running, we treat this as status "pending", not an error. It would make more sense to me to handle a ref that is still resolving the same way. would it be possible to treat the ResourceRequest as "pending" rather than an error?

Ah I see, so if we updated the signature of resources.GetPipelineData to something like this:

pipelineMeta, pipelineSpec, pending, err := resources.GetPipelineData(ctx, pr, getPipelineFunc) switch { case pending: // The code here to mark the PR as pending case err != nil: // The code here to fail the PipelineRun default: // The code to store the pipeline meta and spec on pr.status }

Does that read clearer than the sentinel check we have here atm?

After discussing offline and spending a bit of time trying to rejig this bit of the code I think I'm going to leave it as-is for the time being.

There's scope here for a tidier interface between the reconcilers and pipelineref resolution but the codebase is in a bit of an in-between state because we have resolution logic in (a) the reconcilers themselves (b) the resources subpackage which abstracts some resolution mechanics and (c) the remote package which hides away the built-in OCI bundles resolution (and now the new remoteresolution functionality).

A holistic change to the way this all works makes sense, and is def something we can look at in another pr, but for this one I'm trying to avoid changing how tekton's existing resolution mechanics function to limit blast radius of the new code.

SGTM! Would you be able to create an issue or something to describe a refactoring that would make more sense?

We do have an issue around this, #3305, so I've added a comment describing the remaining work to do here: #3305 (comment)

sounds good, thanks!

lbernick · 2022-02-18T14:50:08Z

pkg/reconciler/pipelinerun/controller.go

@@ -85,6 +90,10 @@ func NewController(opts *pipeline.Options, clock clock.Clock) func(context.Conte
 			FilterFunc: controller.FilterController(&v1beta1.PipelineRun{}),
 			Handler:    controller.HandleAll(impl.EnqueueControllerOf),
 		})
+		resourceRequestInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{


does this require someone to have the resourcerequest CRD defined in their cluster? (genuinely asking)

Yes you're absolutely right, and this is why the integration tests are all failing badly.

The only workaround I can think of right now is to put this initialization behind an environment variable that the user must explicitly enable.

OK, unfortunately it doesn't look like I can optionally start informers like that, I'll have to think of something else.

I saw the slack conversation you opened on pipeline-dev; I agree with Matt Moor that it would be OK to import the ResourceRequest CRD as part of the Tekton pipelines config.

I've brought the CRD over into pipelines. One unfortunate downside is that this means the CRD is duplicated both in resolution and here. I tried vendoring the CRD yaml in resolution but go mod doesn't pull anything except go code into pipelines.

ack -- that's too bad but it doesn't sound like there's a good way around this duplication.

I've removed the CRD from resolution. Now the definition in Pipelines is the only one and Tekton Resolution will lean on that rather than the other way round.

lbernick · 2022-02-18T14:54:41Z

pkg/apis/pipeline/v1beta1/taskref_validation.go

@@ -71,18 +71,25 @@ func (ref *TaskRef) validateInTreeRef(ctx context.Context) (errs *apis.FieldErro
 // valid remote resource reference or a valid in-tree resource reference,
 // but not both.
 func (ref *TaskRef) validateAlphaRef(ctx context.Context) (errs *apis.FieldError) {
-	switch {
-	case ref.Resolver == "" && ref.Resource != nil:
+	hasResolver := ref.Resolver != ""


some readability nits: maybe also define hasName and hasBundle, and I think some of the if statements might actually be more readable if they were nested (esp the last two).
Also, it looks like there are two new branches here (resource + name and resource + bundle), seems worth adding test cases for them? The changes to this file could be split into a separate pr if needed.

I've had a go at tackling these readability nits and added a unit test, should be ready for another look!

lbernick · 2022-02-18T15:02:15Z

pkg/reconciler/pipelinerun/pipelinerun_test.go

+	prt := newPipelineRunTest(d, t)
+	defer prt.Cancel()
+
+	pipelinerun, _ := prt.reconcileRun("default", "pr", nil, false)


nit: can you comment these params inline here and below, e.g. reconcileRun("default", "pr", /* wantEvents=*/ nil, /*permanentErr */ = false)

Added named variables for these, cheers!

lbernick · 2022-02-18T15:04:16Z

pkg/reconciler/pipelinerun/pipelinerun_test.go

+                  script: |
+                    echo "hello world!"
+        `)
+	resreq.Status.ResourceRequestStatusFields.Data = base64.StdEncoding.Strict().EncodeToString(pipelineBytes)


possibly out of scope for this PR, but this might benefit from some testing helper functions for mocking returned data.

Good point, this isn't something I've tackled as part of this PR but I am also starting to think about how the resolution package could provide test helpers / mocks for this so that pipelines doesn't have to own them.

consider adding a TODO comment here to move out of this test file

I created an issue in resolution to capture this: tektoncd/resolution#10

lbernick · 2022-02-18T15:08:48Z

pkg/reconciler/pipelinerun/resources/pipelineref.go

@@ -107,3 +120,19 @@ func (l *LocalPipelineRefResolver) GetPipeline(ctx context.Context, name string)
 	}
 	return l.Tektonclient.TektonV1beta1().Pipelines(l.Namespace).Get(ctx, name, metav1.GetOptions{})
 }
+
+func readRuntimeObjectAsPipeline(ctx context.Context, obj runtime.Object) (v1beta1.PipelineObject, error) {


can you please add a comment for this function?

Added a comment.

lbernick · 2022-02-18T15:12:54Z

pkg/reconciler/pipelinerun/resources/pipelineref_test.go

@@ -291,6 +293,46 @@ func TestGetPipelineFuncSpecAlreadyFetched(t *testing.T) {
 	}
 }

+func TestGetPipelineFunc_RemoteResolution(t *testing.T) {


might also be worth adding a test case where the bytes returned are invalid

Added a UT for invalid bytes.

lbernick · 2022-02-18T15:17:00Z

pkg/remote/resolution/error.go

@@ -0,0 +1,64 @@
+package resolution


missing a copyright

Good catch, cheers, fixed!

tekton-robot · 2022-03-02T17:10: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/apis/pipeline/v1beta1/pipeline_types.go	96.1%	96.2%	0.1
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.1%	-4.7
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.1
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.2%	-4.0
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

tekton-robot · 2022-03-02T17:27:43Z

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/pipeline/v1beta1/pipeline_types.go	96.1%	96.2%	0.1
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.1%	-4.7
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.1
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.2%	-4.0
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

ghost · 2022-03-02T17:56:23Z

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

ghost · 2022-03-02T18:25:24Z

Alpha integration failed first on workspace-in-sidecar and this time on run-steps-as-non-root. The failures don't appear to be directly related to the content of the pull request.

ghost · 2022-03-02T18:25:31Z

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

ghost · 2022-03-02T18:59:02Z

workspace-in-sidecar failed again.

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

tekton-robot · 2022-03-02T21:12:43Z

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/pipeline/v1beta1/pipeline_types.go	96.1%	96.2%	0.1
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	92.9%	-3.9
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.1
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	82.2%	-4.0
pkg/reconciler/pipelinerun/resources/pipelinespec.go	100.0%	89.5%	-10.5
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

ghost · 2022-03-22T18:37:51Z

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

/test pull-tekton-pipeline-integration-tests

ghost · 2022-03-22T19:11:44Z

workspace-in-sidecar and another example failed. neither related to the PR.

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

ghost · 2022-03-22T20:09:32Z

@tektoncd/core-maintainers Feedback from Pipelines WG has been addressed and this PR is ready for another look. Cheers!

lbernick · 2022-03-22T20:48:01Z

/lgtm

ghost · 2022-03-28T12:05:05Z

Rebased on main.

tekton-robot · 2022-03-28T12:06:43Z

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/pipeline/v1beta1/pipeline_types.go	96.4%	96.5%	0.1
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	97.6%	0.8
pkg/reconciler/pipelinerun/pipelinerun.go	83.9%	84.0%	0.1
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	87.5%	1.3
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

dibyom · 2022-03-29T19:48:33Z

config/300-resolutionrequest.yaml

+  name: resolutionrequests.resolution.tekton.dev
+  labels:
+    resolution.tekton.dev/release: devel
+    knative.dev/crd-install: "true"


what does this label do?

Ah, good question :S

I think it's used to filter which resources to install when a user executes kubectl apply. This isn't used by any of the other pipeline crds so I'll remove it. Cheers!

tekton-robot · 2022-03-31T15:18:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dibyom

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~~ [dibyom]

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

Prior to this commit the API fields for remote resolution were just stubs with no functionality. This commit adds support for resolving pipelineRefs from public git repos using the tektoncd/resolution project. This feature is in alpha and relies on a new CRD called a ResourceRequest.

tekton-robot · 2022-03-31T20:56:39Z

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/pipeline/v1beta1/pipeline_types.go	96.4%	96.5%	0.1
pkg/apis/pipeline/v1beta1/taskref_validation.go	96.8%	97.6%	0.8
pkg/reconciler/pipelinerun/pipelinerun.go	83.7%	83.9%	0.1
pkg/reconciler/pipelinerun/resources/pipelineref.go	86.2%	87.5%	1.3
pkg/remote/resolution/error.go	Do not exist	50.0%
pkg/remote/resolution/request.go	Do not exist	0.0%
pkg/remote/resolution/resolver.go	Do not exist	85.7%
test/controller.go	24.5%	24.0%	-0.5
test/resolution.go	Do not exist	0.0%

ghost · 2022-03-31T20:59:20Z

Updated resolution to be based on latest knative + go1.17.

Should be ready for one final look + (hopefully 🤞) lgtm!

ghost · 2022-03-31T21:00:14Z

Also we have a first draft of a Bundles resolver: https://github.com/tektoncd/resolution/tree/main/bundleresolver

ghost · 2022-03-31T21:29:46Z

Workspace-in-sidecar failed; known flake

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

ghost · 2022-03-31T22:01:23Z

TestPipelineTaskTimeout failed with

reason: TaskRunTimeout
status: "False"
type: Succeeded

I can't reproduce this locally so let's see if it repeats.

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

lbernick · 2022-04-01T13:20:53Z

/lgtm

Followup to tektoncd#4596, needed for tektoncd#4710. remote resolution, both in explicitly created `TaskRun`s and in `PipelineRun`s' `PipelineTask`s, from public git repositories using tektoncd/resolution. This is still in alpha. Signed-off-by: Andrew Bayer <[email protected]>

Followup to tektoncd#4596, needed for tektoncd#4710. Enables remote resolution of `Task`s, both in explicitly created `TaskRun`s and in `PipelineRun`s' `PipelineTask`s, from public git repositories using tektoncd/resolution. Also modifies `resolution.Resolver` to take both owner and optional name/namespace. This is needed because we don't necessarily have a `TaskRun` yet when calling `GetTaskFunc` in the `PipelineRun` reconciler, but still need to ensure that we only make one remote resolution request for a `PipelineTask` via the `PipelineRun` and `TaskRun` reconcilers. Therefore, we must have the same deterministically-generated resolution request name in both places, using the predictable eventual `TaskRun` name and namespace. We also want to make sure that the created `ResolutionRequest` has the appropriate owner reference, hence still passing a `kmeta.OwnerRefable` to `NewResolver` as well as the name and namespace. This is still in alpha. Signed-off-by: Andrew Bayer <[email protected]>

Followup to #4596, needed for #4710. Enables remote resolution of `Task`s, both in explicitly created `TaskRun`s and in `PipelineRun`s' `PipelineTask`s, from public git repositories using tektoncd/resolution. Also modifies `resolution.Resolver` to take both owner and optional name/namespace. This is needed because we don't necessarily have a `TaskRun` yet when calling `GetTaskFunc` in the `PipelineRun` reconciler, but still need to ensure that we only make one remote resolution request for a `PipelineTask` via the `PipelineRun` and `TaskRun` reconcilers. Therefore, we must have the same deterministically-generated resolution request name in both places, using the predictable eventual `TaskRun` name and namespace. We also want to make sure that the created `ResolutionRequest` has the appropriate owner reference, hence still passing a `kmeta.OwnerRefable` to `NewResolver` as well as the name and namespace. This is still in alpha. Signed-off-by: Andrew Bayer <[email protected]>

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. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 17, 2022

tekton-robot requested review from imjasonh and jerop February 17, 2022 21:40

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 17, 2022

lbernick reviewed Feb 18, 2022

View reviewed changes

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 22, 2022

afrittoli added this to the Pipelines v0.34 milestone Feb 22, 2022

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 2, 2022

ghost mentioned this pull request Mar 2, 2022

workspace-in-sidecar example test is very flakey #4169

Closed

tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 22, 2022

tekton-robot added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels Mar 22, 2022

tekton-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 31, 2022

dibyom approved these changes Mar 31, 2022

View reviewed changes

tekton-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 31, 2022

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 31, 2022

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

tekton-robot merged commit adc127a into tektoncd:main Apr 1, 2022

abayer mentioned this pull request May 11, 2022

Add taskRef remote resolution support #4859

Merged

5 tasks

Add pipelineRef remote resolution #4596

Add pipelineRef remote resolution #4596

Conversation

ghost commented Feb 17, 2022 • edited by ghost Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Feb 17, 2022

tekton-robot commented Feb 17, 2022

tekton-robot commented Feb 17, 2022

tekton-robot commented Feb 17, 2022

tekton-robot commented Feb 17, 2022

ghost commented Feb 17, 2022

tekton-robot commented Feb 17, 2022

ghost commented Feb 17, 2022

tekton-robot commented Feb 17, 2022

ghost commented Feb 17, 2022

tekton-robot commented Feb 18, 2022

tekton-robot commented Feb 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tekton-robot commented Mar 2, 2022

tekton-robot commented Mar 2, 2022

ghost commented Mar 2, 2022

ghost commented Mar 2, 2022

ghost commented Mar 2, 2022

ghost commented Mar 2, 2022

tekton-robot commented Mar 2, 2022

ghost commented Mar 22, 2022

ghost commented Mar 22, 2022

ghost commented Mar 22, 2022

lbernick commented Mar 22, 2022

ghost commented Mar 28, 2022

tekton-robot commented Mar 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tekton-robot commented Mar 31, 2022

tekton-robot commented Mar 31, 2022

ghost commented Mar 31, 2022

ghost commented Mar 31, 2022

ghost commented Mar 31, 2022

ghost commented Mar 31, 2022

lbernick commented Apr 1, 2022

ghost commented Feb 17, 2022 •

edited by ghost

Loading