From ea9e50af0324f9fd9f79c733568a72bcb06c3c52 Mon Sep 17 00:00:00 2001 From: Piyush Garg Date: Mon, 2 Sep 2024 13:07:09 +0530 Subject: [PATCH] Refactor Resolver This will refactor resolver to work with multiple edge case scenarios --- pkg/matcher/annotation_tasks_install.go | 80 +++-- pkg/matcher/annotation_tasks_install_test.go | 307 +++++++++++------- pkg/resolve/remote.go | 202 ++++++++---- pkg/resolve/remote_test.go | 87 +++-- pkg/resolve/resolve.go | 106 ++---- ...peline-with-remote-task-from-pipeline.yaml | 17 + ...ine-with-remote-task-from-pipelinerun.yaml | 18 + ...pipelinerun-annotations-and-tektondir.yaml | 18 + ...n-annotations-and-pipeline-annotation.yaml | 19 ++ ...pipelinerun-annotations-and-tektondir.yaml | 18 + 10 files changed, 528 insertions(+), 344 deletions(-) create mode 100644 pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipeline.yaml create mode 100644 pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipelinerun.yaml create mode 100644 pkg/resolve/testdata/skip-fetching-multiple-pipelines-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml create mode 100644 pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-pipeline-annotation.yaml create mode 100644 pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml diff --git a/pkg/matcher/annotation_tasks_install.go b/pkg/matcher/annotation_tasks_install.go index 9640d43af..47be7237c 100644 --- a/pkg/matcher/annotation_tasks_install.go +++ b/pkg/matcher/annotation_tasks_install.go @@ -181,60 +181,54 @@ func grabValuesFromAnnotations(annotations map[string]string, annotationReg stri return ret, nil } -// GetTaskFromAnnotations Get task remotely if they are on Annotations. -func (rt RemoteTasks) GetTaskFromAnnotations(ctx context.Context, annotations map[string]string) ([]*tektonv1.Task, error) { - ret := []*tektonv1.Task{} - tasks, err := grabValuesFromAnnotations(annotations, taskAnnotationsRegexp) +func GrabTasksFromAnnotations(annotations map[string]string) ([]string, error) { + return grabValuesFromAnnotations(annotations, taskAnnotationsRegexp) +} + +func GrabPipelineFromAnnotations(annotations map[string]string) (string, error) { + pipelinesAnnotation, err := grabValuesFromAnnotations(annotations, pipelineAnnotationsRegexp) if err != nil { - return nil, err + return "", err } - for _, v := range tasks { - data, err := rt.getRemote(ctx, v, true, "task") - if err != nil { - return nil, fmt.Errorf("error getting remote task \"%s\": %w", v, err) - } - if data == "" { - return nil, fmt.Errorf("could not get remote task \"%s\": returning empty", v) - } - - task, err := rt.convertTotask(ctx, v, data) - if err != nil { - return nil, err - } - ret = append(ret, task) + if len(pipelinesAnnotation) > 1 { + return "", fmt.Errorf("only one pipeline is allowed on remote resolution, we have received multiple of them: %+v", pipelinesAnnotation) } - return ret, nil + if len(pipelinesAnnotation) == 0 { + return "", nil + } + return pipelinesAnnotation[0], nil } -// GetPipelineFromAnnotations Get pipeline remotely if they are on Annotations -// TODO: merge in a generic between the two. -func (rt RemoteTasks) GetPipelineFromAnnotations(ctx context.Context, annotations map[string]string) (*tektonv1.Pipeline, error) { - ret := []*tektonv1.Pipeline{} - pipelinesAnnotation, err := grabValuesFromAnnotations(annotations, pipelineAnnotationsRegexp) +func (rt RemoteTasks) GetTaskFromAnnotationName(ctx context.Context, name string) (*tektonv1.Task, error) { + data, err := rt.getRemote(ctx, name, true, "task") + if err != nil { + return nil, fmt.Errorf("error getting remote task \"%s\": %w", name, err) + } + if data == "" { + return nil, fmt.Errorf("could not get remote task \"%s\": returning empty", name) + } + + task, err := rt.convertTotask(ctx, name, data) if err != nil { return nil, err } - if len(pipelinesAnnotation) > 1 { - return nil, fmt.Errorf("only one pipeline is allowed on remote resolution, we have received multiple of them: %+v", pipelinesAnnotation) + return task, nil +} + +func (rt RemoteTasks) GetPipelineFromAnnotationName(ctx context.Context, name string) (*tektonv1.Pipeline, error) { + data, err := rt.getRemote(ctx, name, true, "pipeline") + if err != nil { + return nil, fmt.Errorf("error getting remote pipeline \"%s\": %w", name, err) } - if len(pipelinesAnnotation) == 0 { - return nil, nil + if data == "" { + return nil, fmt.Errorf("could not get remote pipeline \"%s\": returning empty", name) } - for _, v := range pipelinesAnnotation { - data, err := rt.getRemote(ctx, v, true, "pipeline") - if err != nil { - return nil, fmt.Errorf("error getting remote pipeline %s: %w", v, err) - } - if data == "" { - return nil, fmt.Errorf("could not get remote pipeline \"%s\": returning empty", v) - } - pipeline, err := rt.convertToPipeline(ctx, v, data) - if err != nil { - return nil, err - } - ret = append(ret, pipeline) + + pipeline, err := rt.convertToPipeline(ctx, name, data) + if err != nil { + return nil, err } - return ret[0], nil + return pipeline, nil } // getFileFromLocalFS get task locally if file exist diff --git a/pkg/matcher/annotation_tasks_install_test.go b/pkg/matcher/annotation_tasks_install_test.go index 1e395cf03..77ad07088 100644 --- a/pkg/matcher/annotation_tasks_install_test.go +++ b/pkg/matcher/annotation_tasks_install_test.go @@ -15,6 +15,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings" httptesthelper "github.com/openshift-pipelines/pipelines-as-code/pkg/test/http" "github.com/openshift-pipelines/pipelines-as-code/pkg/test/provider" + testifyassert "github.com/stretchr/testify/assert" tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "go.uber.org/zap" zapobserver "go.uber.org/zap/zaptest/observer" @@ -46,7 +47,138 @@ func readTDfile(t *testing.T, testname string) string { return string(data) } -func TestRemoteTasksGetTaskFromAnnotations(t *testing.T) { +func TestGrabTasksFromAnnotation(t *testing.T) { + tests := []struct { + annotations map[string]string + expected []string + name string + wantErr string + }{ + { + name: "single task", + annotations: map[string]string{ + keys.Task: "[http://remote.task]", + }, + expected: []string{"http://remote.task"}, + }, + { + name: "wrong key", + annotations: map[string]string{ + keys.Task: "[http://remote.task]", + pipelinesascode.GroupName + "/taskA": "[http://other.task]", // That's wrong this would be skipped + }, + expected: []string{"http://remote.task"}, + }, + { + name: "multiple tasks", + annotations: map[string]string{ + keys.Task: "[http://remote.task]", + keys.Task + "-1": "[http://other.task]", + }, + expected: []string{"http://other.task", "http://remote.task"}, + }, + { + name: "multiple tasks with random order", + annotations: map[string]string{ + keys.Task: "[http://remote.task]", + keys.Task + "-5": "[http://other.task]", + }, + expected: []string{"http://other.task", "http://remote.task"}, + }, + { + name: "multiple tasks with only orders", + annotations: map[string]string{ + keys.Task + "-5": "[http://remote.task]", + keys.Task + "-1": "[http://other.task]", + }, + expected: []string{"http://other.task", "http://remote.task"}, + }, + { + name: "multiple tasks with one annotation", + annotations: map[string]string{ + keys.Task + "-1": "[http://other.task, http://remote.task]", + }, + expected: []string{"http://other.task", "http://remote.task"}, + }, + { + name: "test-annotations-remote-http-bad-annotation", + annotations: map[string]string{ + keys.Task: "[http://remote.task", + }, + expected: []string{}, + wantErr: "annotations in pipeline are in wrong format: [http://remote.task", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output, err := GrabTasksFromAnnotations(tt.annotations) + testifyassert.ElementsMatch(t, tt.expected, output) + if tt.wantErr != "" { + assert.ErrorContains(t, err, tt.wantErr, "We should have get an error with %v but we didn't", tt.wantErr) + } + }) + } +} + +func TestGrabPipelineFromAnnotation(t *testing.T) { + tests := []struct { + annotations map[string]string + expected string + name string + wantErr string + }{ + { + name: "single pipeline", + annotations: map[string]string{ + keys.Pipeline: "[http://remote.task]", + }, + expected: "http://remote.task", + }, + { + name: "sing pipeline and a wrong key", + annotations: map[string]string{ + keys.Pipeline: "[http://remote.task]", + pipelinesascode.GroupName + "/pipelineA": "[http://other.task]", // That's wrong this would be skipped + }, + expected: "http://remote.task", + }, + { + name: "single pipeline with only wrong key", + annotations: map[string]string{ + keys.Pipeline + "-1": "[http://other.task]", + }, + expected: "", + }, + { + name: "multiple pipelines with one annotation", + annotations: map[string]string{ + keys.Pipeline: "[http://other.task, http://remote.task]", + }, + expected: "", + wantErr: "only one pipeline is allowed on remote resolution, we have received multiple of them: [http://other.task http://remote.task]", + }, + { + name: "test-annotations-remote-http-bad-annotation", + annotations: map[string]string{ + keys.Pipeline: "[http://remote.task", + }, + expected: "", + wantErr: "annotations in pipeline are in wrong format: [http://remote.task", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output, err := GrabPipelineFromAnnotations(tt.annotations) + assert.Equal(t, tt.expected, output) + if tt.wantErr != "" { + assert.ErrorContains(t, err, tt.wantErr, "We should have get an error with %v but we didn't", tt.wantErr) + return + } + }) + } +} + +func TestGetTaskFromAnnotationName(t *testing.T) { var hubCatalogs sync.Map hubCatalogs.Store( "default", settings.HubCatalog{ @@ -61,7 +193,7 @@ func TestRemoteTasksGetTaskFromAnnotations(t *testing.T) { Name: testCatalogHubName, }) tests := []struct { - annotations map[string]string + task string filesInsideRepo map[string]string gotTaskName string name string @@ -73,9 +205,7 @@ func TestRemoteTasksGetTaskFromAnnotations(t *testing.T) { }{ { name: "test-annotations-error-remote-http-not-k8", - annotations: map[string]string{ - keys.Task: "[http://remote.task]", - }, + task: "http://remote.task", remoteURLS: map[string]map[string]string{ "http://remote.task": { "body": "", @@ -85,26 +215,20 @@ func TestRemoteTasksGetTaskFromAnnotations(t *testing.T) { wantErr: "returning empty", }, { - name: "test-good-coming-from-provider", - annotations: map[string]string{ - keys.Task: "http://provider/remote.task", - }, + name: "test-good-coming-from-provider", + task: "http://provider/remote.task", wantProviderRemoteTask: true, wantErr: "returning empty", }, { - name: "test-bad-coming-from-provider", - annotations: map[string]string{ - keys.Task: "http://provider/remote.task", - }, + name: "test-bad-coming-from-provider", + task: "http://provider/remote.task", wantProviderRemoteTask: false, wantErr: "error getting remote task", }, { name: "test-annotations-remote-http", - annotations: map[string]string{ - keys.Task: "[http://remote.task]", - }, + task: "http://remote.task", remoteURLS: map[string]map[string]string{ "http://remote.task": { "body": readTDfile(t, "task-good"), @@ -128,10 +252,8 @@ func TestRemoteTasksGetTaskFromAnnotations(t *testing.T) { // wantErr: "cannot be validated properly", // }, { - name: "test-annotations-remote-https", - annotations: map[string]string{ - keys.Task: "[https://remote.task]", - }, + name: "test-annotations-remote-https", + task: "https://remote.task", gotTaskName: "task", remoteURLS: map[string]map[string]string{ "https://remote.task": { @@ -141,72 +263,52 @@ func TestRemoteTasksGetTaskFromAnnotations(t *testing.T) { }, }, { - name: "test-annotations-inside-repo", - annotations: map[string]string{ - keys.Task: "[be/healthy]", - }, - gotTaskName: "task", - filesInsideRepo: map[string]string{ - "be/healthy": readTDfile(t, "task-good"), - }, - runevent: info.Event{ - SHA: "007", - }, - }, - { - name: "test-annotations-remote-http-skipping-notmatching", - annotations: map[string]string{ - keys.Task: "[http://remote.task]", - pipelinesascode.GroupName + "/taskA": "[http://other.task]", // That's wrong this would be skipped - }, - gotTaskName: "task", + name: "bad/not a tasl", + task: "http://remote.task", remoteURLS: map[string]map[string]string{ "http://remote.task": { - "body": readTDfile(t, "task-good"), + "body": readTDfile(t, "pipeline-good"), "code": "200", }, }, + wantErr: "remote task from uri: http://remote.task has not been recognized as a tekton task", }, { - name: "test-annotations-remote-http-bad-annotation", - annotations: map[string]string{ - keys.Task: "[http://remote.task", + name: "test-annotations-inside-repo", + task: "be/healthy", + gotTaskName: "task", + filesInsideRepo: map[string]string{ + "be/healthy": readTDfile(t, "task-good"), + }, + runevent: info.Event{ + SHA: "007", }, - wantErr: "annotations in pipeline are in wrong format", }, { - name: "test-annotations-remote-inside-file-not-found", - annotations: map[string]string{ - keys.Task: "[pas/la]", - }, + name: "test-annotations-remote-inside-file-not-found", + task: "pas/la", wantErr: "could not find", runevent: info.Event{ SHA: "007", }, }, { - name: "test-annotations-remote-no-event-not-found-no-error", - annotations: map[string]string{ - keys.Task: "[not/here]", - }, + name: "test-annotations-remote-no-event-not-found-no-error", + task: "not/here", wantLog: "could not find remote file not/here", wantErr: "returning empty", }, { - name: "test-annotations-unknown-hub", - annotations: map[string]string{ - keys.Task: "[foo://bar]", - }, + name: "test-annotations-unknown-hub", + task: "foo://bar", wantLog: "custom catalog foo is not found", wantErr: "could not get remote task \"foo://bar\": returning empty", }, { name: "test-get-from-custom-hub", gotTaskName: "task", - annotations: map[string]string{ - keys.Task: "[anotherHub://chmouzie]", - }, - wantLog: "successfully fetched task chmouzie from custom catalog HUB anotherHub on URL https://mybelovedhub", + task: "anotherHub://chmouzie", + wantLog: "successfully fetched task chmouzie from custom catalog HUB anotherHub on URL https://mybelovedhub", remoteURLS: map[string]map[string]string{ testHubURL + "/resource/" + testCatalogHubName + "/task/chmouzie": { "body": `{"data": {"LatestVersion": {"version": "0.1"}}}`, @@ -221,9 +323,7 @@ func TestRemoteTasksGetTaskFromAnnotations(t *testing.T) { { name: "test-get-from-hub-latest", gotTaskName: "task", - annotations: map[string]string{ - keys.Task: "[chmouzie]", - }, + task: "chmouzie", remoteURLS: map[string]map[string]string{ testHubURL + "/resource/" + testCatalogHubName + "/task/chmouzie": { "body": `{"data": {"LatestVersion": {"version": "0.1"}}}`, @@ -238,9 +338,7 @@ func TestRemoteTasksGetTaskFromAnnotations(t *testing.T) { { name: "test-get-from-hub-specific-version", gotTaskName: "task", - annotations: map[string]string{ - keys.Task: "[chmouzie:0.2]", - }, + task: "chmouzie:0.2", remoteURLS: map[string]map[string]string{ testHubURL + "/resource/" + testCatalogHubName + "/task/chmouzie/0.2": { "body": `{}`, @@ -282,7 +380,7 @@ func TestRemoteTasksGetTaskFromAnnotations(t *testing.T) { Event: &tt.runevent, } - got, err := rt.GetTaskFromAnnotations(ctx, tt.annotations) + got, err := rt.GetTaskFromAnnotationName(ctx, tt.task) if tt.wantLog != "" { assert.Assert(t, len(fakelog.FilterMessageSnippet(tt.wantLog).TakeAll()) > 0, "could not find log message: got ", fakelog) } @@ -291,16 +389,16 @@ func TestRemoteTasksGetTaskFromAnnotations(t *testing.T) { return } assert.NilError(t, err, "GetTaskFromAnnotations() error = %v, wantErr %v", err, tt.wantErr) - assert.Assert(t, len(got) > 0, "GetTaskFromAnnotations() error no tasks has been processed") + assert.Assert(t, got != nil, "GetTaskFromAnnotations() error no tasks has been processed") if tt.gotTaskName != "" { - assert.Equal(t, tt.gotTaskName, got[0].GetName()) + assert.Equal(t, tt.gotTaskName, got.GetName()) } }) } } -func TestGetPipelineFromAnnotations(t *testing.T) { +func TestGetPipelineFromAnnotationName(t *testing.T) { var hubCatalogs sync.Map hubCatalogs.Store( "default", settings.HubCatalog{ @@ -315,7 +413,7 @@ func TestGetPipelineFromAnnotations(t *testing.T) { Name: testCatalogHubName, }) tests := []struct { - annotations map[string]string + pipeline string filesInsideRepo map[string]string gotPipelineName string name string @@ -327,9 +425,7 @@ func TestGetPipelineFromAnnotations(t *testing.T) { { name: "good/fetching from remote http", gotPipelineName: "pipeline", - annotations: map[string]string{ - keys.Pipeline: "[http://remote.pipeline]", - }, + pipeline: "http://remote.pipeline", remoteURLS: map[string]map[string]string{ "http://remote.pipeline": { "body": readTDfile(t, "pipeline-good"), @@ -340,9 +436,7 @@ func TestGetPipelineFromAnnotations(t *testing.T) { { name: "good/fetching with bundle", gotPipelineName: "pipeline", - annotations: map[string]string{ - keys.Pipeline: "[http://remote.pipeline]", - }, + pipeline: "http://remote.pipeline", remoteURLS: map[string]map[string]string{ "http://remote.pipeline": { "body": readTDfile(t, "pipeline-good-bundle"), @@ -378,10 +472,8 @@ func TestGetPipelineFromAnnotations(t *testing.T) { // wantErr: "emote pipeline from uri: http://remote.pipeline with name pipeline cannot be validated: expected at least one, got none:", // }, { - name: "bad/error getting pipeline", - annotations: map[string]string{ - keys.Pipeline: "[http://remote.pipeline]", - }, + name: "bad/error getting pipeline", + pipeline: "http://remote.pipeline", remoteURLS: map[string]map[string]string{ "http://remote.pipeline": { "code": "501", @@ -390,10 +482,8 @@ func TestGetPipelineFromAnnotations(t *testing.T) { wantErr: "error getting remote pipeline", }, { - name: "bad/not a pipeline", - annotations: map[string]string{ - keys.Pipeline: "[http://remote.pipeline]", - }, + name: "bad/not a pipeline", + pipeline: "http://remote.pipeline", remoteURLS: map[string]map[string]string{ "http://remote.pipeline": { "body": readTDfile(t, "task-good"), @@ -403,17 +493,13 @@ func TestGetPipelineFromAnnotations(t *testing.T) { wantErr: "remote pipeline from uri: http://remote.pipeline has not been recognized as a tekton pipeline", }, { - name: "bad/could not get remote", - annotations: map[string]string{ - keys.Pipeline: "[http://nowhere.pipeline]", - }, - wantErr: "error getting remote pipeline", + name: "bad/could not get remote", + pipeline: "http://nowhere.pipeline", + wantErr: "error getting remote pipeline", }, { - name: "bad/returning empty", - annotations: map[string]string{ - keys.Pipeline: "[http://remote.pipeline]", - }, + name: "bad/returning empty", + pipeline: "http://remote.pipeline", remoteURLS: map[string]map[string]string{ "http://remote.pipeline": { "body": "", @@ -423,27 +509,16 @@ func TestGetPipelineFromAnnotations(t *testing.T) { wantErr: "returning empty", }, { - name: "bad/more than one pipeline", - annotations: map[string]string{ - keys.Pipeline: "[http://foo.bar, http://remote.pipeline]", - }, - wantErr: "only one pipeline is allowed on remote", - }, - { - name: "test-annotations-unknown-hub", - annotations: map[string]string{ - keys.Pipeline: "[foo://bar]", - }, - wantLog: "custom catalog foo is not found", - wantErr: "could not get remote pipeline \"foo://bar\": returning empty", + name: "test-annotations-unknown-hub", + pipeline: "foo://bar", + wantLog: "custom catalog foo is not found", + wantErr: "could not get remote pipeline \"foo://bar\": returning empty", }, { name: "test-get-from-custom-hub", gotPipelineName: "pipeline", - annotations: map[string]string{ - keys.Pipeline: "[anotherHub://chmouzie]", - }, - wantLog: "successfully fetched pipeline chmouzie from custom catalog HUB anotherHub on URL https://mybelovedhub", + pipeline: "anotherHub://chmouzie", + wantLog: "successfully fetched pipeline chmouzie from custom catalog HUB anotherHub on URL https://mybelovedhub", remoteURLS: map[string]map[string]string{ testHubURL + "/resource/" + testCatalogHubName + "/pipeline/chmouzie": { "body": `{"data": {"LatestVersion": {"version": "0.1"}}}`, @@ -458,9 +533,7 @@ func TestGetPipelineFromAnnotations(t *testing.T) { { name: "test-get-from-hub-latest", gotPipelineName: "pipeline", - annotations: map[string]string{ - keys.Pipeline: "[chmouzie]", - }, + pipeline: "chmouzie", remoteURLS: map[string]map[string]string{ testHubURL + "/resource/" + testCatalogHubName + "/pipeline/chmouzie": { "body": `{"data": {"LatestVersion": {"version": "0.1"}}}`, @@ -475,9 +548,7 @@ func TestGetPipelineFromAnnotations(t *testing.T) { { name: "test-get-from-hub-specific-version", gotPipelineName: "pipeline", - annotations: map[string]string{ - keys.Pipeline: "[chmouzie:0.2]", - }, + pipeline: "chmouzie:0.2", remoteURLS: map[string]map[string]string{ testHubURL + "/resource/" + testCatalogHubName + "/pipeline/chmouzie/0.2": { "body": `{}`, @@ -519,7 +590,7 @@ func TestGetPipelineFromAnnotations(t *testing.T) { Event: &tt.runevent, } - got, err := rt.GetPipelineFromAnnotations(ctx, tt.annotations) + got, err := rt.GetPipelineFromAnnotationName(ctx, tt.pipeline) if tt.wantErr != "" { assert.ErrorContains(t, err, tt.wantErr, "We should have get an error with %v but we didn't", tt.wantErr) diff --git a/pkg/resolve/remote.go b/pkg/resolve/remote.go index 359dc74e5..7292df1af 100644 --- a/pkg/resolve/remote.go +++ b/pkg/resolve/remote.go @@ -5,22 +5,21 @@ import ( "fmt" "github.com/openshift-pipelines/pipelines-as-code/pkg/matcher" + tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" ) type NamedItem interface { GetName() string } -func alreadySeen[T NamedItem](items []T, item T) bool { - for _, value := range items { - if value.GetName() == item.GetName() { - return true - } +func alreadyFetchedResource[T NamedItem](resources map[string]T, resourceName string) bool { + if _, ok := resources[resourceName]; ok { + return true } return false } -// getRemotes will get remote tasks or Pipelines from annotations. +// resolveRemoteResources will get remote tasks or Pipelines from annotations. // // It already has some tasks or pipeline coming from the tekton directory stored in [types] // @@ -32,74 +31,161 @@ func alreadySeen[T NamedItem](items []T, item T) bool { // // The precedence logic for Pipeline is first from PipelineRun annotations and // then from Tekton directory. -func getRemotes(ctx context.Context, rt *matcher.RemoteTasks, types TektonTypes) (TektonTypes, error) { - remoteType := &TektonTypes{} +func resolveRemoteResources(ctx context.Context, rt *matcher.RemoteTasks, types TektonTypes, ropt *Opts) ([]*tektonv1.PipelineRun, error) { + // contain Resources fetched for the event + fetchedResourcesForEvent := FetchedResources{ + Tasks: map[string]*tektonv1.Task{}, + Pipelines: map[string]*tektonv1.Pipeline{}, + } + pipelineRuns := []*tektonv1.PipelineRun{} for _, pipelinerun := range types.PipelineRuns { - if len(pipelinerun.GetObjectMeta().GetAnnotations()) == 0 { - continue - } - - // get first all the tasks from the pipelinerun annotations - remoteTasks, err := rt.GetTaskFromAnnotations(ctx, pipelinerun.GetObjectMeta().GetAnnotations()) - if err != nil { - return TektonTypes{}, fmt.Errorf("error getting remote task from pipelinerun annotations: %w", err) + // contain Resources specific to run + fetchedResourcesForPipelineRun := FetchedResourcesForRun{ + Tasks: map[string]*tektonv1.Task{}, } + var pipeline *tektonv1.Pipeline + var err error + if ropt.RemoteTasks { + // no annotations on run, then skip + if pipelinerun.GetObjectMeta().GetAnnotations() == nil { + continue + } - for _, task := range remoteTasks { - if alreadySeen(remoteType.Tasks, task) { - rt.Logger.Debugf("skipping already fetched task %s in annotations on pipelinerun %s", task.GetName(), pipelinerun.GetName()) + if len(pipelinerun.GetObjectMeta().GetAnnotations()) == 0 { continue } - remoteType.Tasks = append(remoteType.Tasks, task) - } - // get the pipeline from the remote annotation if any - remotePipeline, err := rt.GetPipelineFromAnnotations(ctx, pipelinerun.GetObjectMeta().GetAnnotations()) - if err != nil { - return TektonTypes{}, fmt.Errorf("error getting remote pipeline from pipelinerun annotation: %w", err) - } + // get first all the pipeline from the pipelinerun annotations + remotePipeline, err := matcher.GrabPipelineFromAnnotations(pipelinerun.GetObjectMeta().GetAnnotations()) + if err != nil { + return []*tektonv1.PipelineRun{}, fmt.Errorf("error getting remote task from pipelinerun annotations: %w", err) + } - if remotePipeline != nil { - remoteType.Pipelines = append(remoteType.Pipelines, remotePipeline) + // if we got the pipeline name from annotation, we need to fetch the pipeline + if remotePipeline != "" { + // making sure that the pipeline with same annotation name is not fetched + if alreadyFetchedResource(fetchedResourcesForEvent.Pipelines, remotePipeline) { + rt.Logger.Debugf("skipping already fetched pipeline %s in annotations on pipelinerun %s", remotePipeline, pipelinerun.GetName()) + // already fetched, then just get the pipeline to add to run specific Resources + pipeline = fetchedResourcesForEvent.Pipelines[remotePipeline] + } else { + // seems like a new pipeline, fetch it based on name in annotation + pipeline, err = rt.GetPipelineFromAnnotationName(ctx, remotePipeline) + if err != nil { + return []*tektonv1.PipelineRun{}, fmt.Errorf("error getting remote pipeline from pipelinerun annotations: %w", err) + } + // add the pipeline to the Resources fetched for the Event + fetchedResourcesForEvent.Pipelines[remotePipeline] = pipeline + } + } } - } - - // grab the tasks from the remote pipeline - for _, pipeline := range remoteType.Pipelines { - if pipeline.GetObjectMeta().GetAnnotations() == nil { - continue + pipelineTasks := []string{} + // if run is referring to the pipelineRef and pipeline fetched from annotation have name equal to the pipelineRef + if pipelinerun.Spec.PipelineRef != nil && pipelinerun.Spec.PipelineRef.Resolver == "" { + if pipeline == nil || pipeline.Name != pipelinerun.Spec.PipelineRef.Name { + // if pipeline fetched from annotation is not having same name as PipelineRef, then we need to get a local pipeline if exist by same name + pipeline, err = getPipelineByName(pipelinerun.Spec.PipelineRef.Name, types.Pipelines) + if err != nil { + return []*tektonv1.PipelineRun{}, err + } + } + // add the pipeline to the run specific Resources + fetchedResourcesForPipelineRun.Pipeline = pipeline + // grab the tasks, that we may need to fetch for this pipeline from its annotations + if pipeline.GetObjectMeta().GetAnnotations() != nil { + // get all the tasks from the pipeline annotations + pipelineTasks, err = matcher.GrabTasksFromAnnotations(pipeline.GetObjectMeta().GetAnnotations()) + if err != nil { + return []*tektonv1.PipelineRun{}, fmt.Errorf("error getting remote task from pipeline annotations: %w", err) + } + } } - remoteTasks, err := rt.GetTaskFromAnnotations(ctx, pipeline.GetObjectMeta().GetAnnotations()) - if err != nil { - return TektonTypes{}, fmt.Errorf("error getting remote tasks from remote pipeline %s: %w", pipeline.GetName(), err) + + // now start fetching the tasks + if ropt.RemoteTasks { + // first get all the tasks from the pipelinerun annotations + remoteTasks, err := matcher.GrabTasksFromAnnotations(pipelinerun.GetObjectMeta().GetAnnotations()) + if err != nil { + return []*tektonv1.PipelineRun{}, fmt.Errorf("error getting remote task from pipelinerun annotations: %w", err) + } + + // now fetch all the tasks from pipelinerun and pipeline annotations, giving preference to pipelinerun annotation tasks + for _, remoteTask := range append(remoteTasks, pipelineTasks...) { + var task *tektonv1.Task + // if task is already fetched in the event, then just copy the task + if alreadyFetchedResource(fetchedResourcesForEvent.Tasks, remoteTask) { + rt.Logger.Debugf("skipping already fetched task %s in annotations on pipelinerun %s", remoteTask, pipelinerun.GetName()) + task = fetchedResourcesForEvent.Tasks[remoteTask] + } else { + // get the task from annotation name + task, err = rt.GetTaskFromAnnotationName(ctx, remoteTask) + if err != nil { + return []*tektonv1.PipelineRun{}, fmt.Errorf("error getting remote task from pipelinerun annotations: %w", err) + } + // add the newly fetched tasks to fetchedResourcesForEvent with key annotation value + fetchedResourcesForEvent.Tasks[remoteTask] = task + } + // now checking if run specific resources already contain a task with same name, then don't add it + // this is to give preference to the pipelinerun annotation then pipeline annotation + if !alreadyFetchedResource(fetchedResourcesForPipelineRun.Tasks, task.GetName()) { + rt.Logger.Infof("skipping remote task %s as already fetched task %s for pipelinerun %s", remoteTask, task.GetName(), pipelinerun.GetName()) + fetchedResourcesForPipelineRun.Tasks[task.GetName()] = task + } + } } - for _, remoteTask := range remoteTasks { - if alreadySeen(remoteType.Tasks, remoteTask) { - rt.Logger.Infof("skipping remote task %s from remote pipeline %s as already defined in pipelinerun", remoteTask.GetName(), pipeline.GetName()) + // now add all the tasks in .tekton directory to Tasks, as we add them by default if not found in annotation + // we will skip the ones which exist in run specific resources with same name + for _, task := range types.Tasks { + if alreadyFetchedResource(fetchedResourcesForPipelineRun.Tasks, task.GetName()) { + rt.Logger.Infof("overriding task %s coming from tekton directory by an annotation task for pipelinerun %s", task.GetName(), pipelinerun.GetName()) continue } - remoteType.Tasks = append(remoteType.Tasks, remoteTask) + fetchedResourcesForPipelineRun.Tasks[task.GetName()] = task } - } - ret := TektonTypes{ - PipelineRuns: types.PipelineRuns, - } - // first get the remote types and then the local ones so remote takes precedence - for _, task := range append(remoteType.Tasks, types.Tasks...) { - if alreadySeen(ret.Tasks, task) { - rt.Logger.Infof("overriding task %s coming from tekton directory by an annotation task on the pipeline or pipelinerun", task.GetName()) - continue + // if PipelineRef is used then, first resolve pipeline and replace all taskRef{Finally/Task} of Pipeline, then put inlinePipeline in PipelineRun + if pipelinerun.Spec.PipelineRef != nil && pipelinerun.Spec.PipelineRef.Resolver == "" { + pipelineResolved := fetchedResourcesForPipelineRun.Pipeline + turns, err := inlineTasks(pipelineResolved.Spec.Tasks, ropt, fetchedResourcesForPipelineRun) + if err != nil { + return nil, err + } + pipelineResolved.Spec.Tasks = turns + + fruns, err := inlineTasks(pipelineResolved.Spec.Finally, ropt, fetchedResourcesForPipelineRun) + if err != nil { + return nil, err + } + pipelineResolved.Spec.Finally = fruns + + pipelinerun.Spec.PipelineRef = nil + pipelinerun.Spec.PipelineSpec = &pipelineResolved.Spec } - ret.Tasks = append(ret.Tasks, task) - } - for _, remotePipeline := range append(remoteType.Pipelines, types.Pipelines...) { - if alreadySeen(ret.Pipelines, remotePipeline) { - rt.Logger.Infof("overriding pipeline %s coming from tekton directory by the annotation pipelinerun", remotePipeline.GetName()) - continue + + // if PipelineSpec is used then, now resolve the PipelineRun by replacing all taskRef{Finally/Task} + if pipelinerun.Spec.PipelineSpec != nil { + turns, err := inlineTasks(pipelinerun.Spec.PipelineSpec.Tasks, ropt, fetchedResourcesForPipelineRun) + if err != nil { + return nil, err + } + pipelinerun.Spec.PipelineSpec.Tasks = turns + + fruns, err := inlineTasks(pipelinerun.Spec.PipelineSpec.Finally, ropt, fetchedResourcesForPipelineRun) + if err != nil { + return nil, err + } + pipelinerun.Spec.PipelineSpec.Finally = fruns + } + + // Add a GenerateName based on the pipeline name and a "-" + // if we already have a GenerateName then just keep it like this + if ropt.GenerateName && pipelinerun.GenerateName == "" { + pipelinerun.GenerateName = pipelinerun.Name + "-" + pipelinerun.Name = "" } - ret.Pipelines = append(ret.Pipelines, remotePipeline) + pipelineRuns = append(pipelineRuns, pipelinerun) } - return ret, nil + // return all resolved PipelineRuns + return pipelineRuns, nil } diff --git a/pkg/resolve/remote_test.go b/pkg/resolve/remote_test.go index 5abe456b1..48e2dcb94 100644 --- a/pkg/resolve/remote_test.go +++ b/pkg/resolve/remote_test.go @@ -2,6 +2,7 @@ package resolve import ( "fmt" + "os" "strings" "testing" @@ -63,6 +64,9 @@ func TestRemote(t *testing.T) { pipelineTaskRef := []tektonv1.PipelineTask{ { Name: remoteTaskName, + TaskRef: &tektonv1.TaskRef{ + Name: remoteTaskName, + }, }, } pipelinewithTaskRef := ttkn.MakePipeline(remotePipelineName, pipelineTaskRef, map[string]string{ @@ -80,16 +84,16 @@ func TestRemote(t *testing.T) { assert.NilError(t, err) tests := []struct { - name string - pipelineruns []*tektonv1.PipelineRun - tasks []*tektonv1.Task - pipelines []*tektonv1.Pipeline - wantErrSnippet string - remoteURLS map[string]map[string]string - expectedLogsSnippets []string - expectedTaskSpec tektonv1.TaskSpec - expectedPipelinesFetch int - expectedTaskFetch int + name string + pipelineruns []*tektonv1.PipelineRun + tasks []*tektonv1.Task + pipelines []*tektonv1.Pipeline + wantErrSnippet string + remoteURLS map[string]map[string]string + expectedLogsSnippets []string + expectedTaskSpec tektonv1.TaskSpec + expectedPipelineRun string + noPipelineRun bool }{ { name: "remote pipeline with remote task from pipeline", @@ -123,8 +127,7 @@ func TestRemote(t *testing.T) { fmt.Sprintf("successfully fetched %s from remote https url", remotePipelineURL), fmt.Sprintf("successfully fetched %s from remote https url", remoteTaskURL), }, - expectedPipelinesFetch: 1, - expectedTaskFetch: 1, + expectedPipelineRun: "remote-pipeline-with-remote-task-from-pipeline.yaml", }, { name: "remote pipeline with remote task in pipeline overridden from pipelinerun", @@ -156,11 +159,10 @@ func TestRemote(t *testing.T) { }, }, expectedLogsSnippets: []string{ - fmt.Sprintf("successfully fetched %s from remote https url", taskFromPipelineRunURL), fmt.Sprintf("successfully fetched %s from remote https url", remotePipelineURL), + fmt.Sprintf("successfully fetched %s from remote https url", taskFromPipelineRunURL), }, - expectedPipelinesFetch: 1, - expectedTaskFetch: 1, + expectedPipelineRun: "remote-pipeline-with-remote-task-from-pipelinerun.yaml", }, { name: "remote pipelinerun no annotations", @@ -173,10 +175,11 @@ func TestRemote(t *testing.T) { }, ), }, + noPipelineRun: true, }, { name: "error/remote pipelinerun is 404", - wantErrSnippet: "error getting remote pipeline " + remotePipelineURL, + wantErrSnippet: "error getting remote pipeline \"" + remotePipelineURL + "\"", pipelineruns: []*tektonv1.PipelineRun{ ttkn.MakePR(randomPipelineRunName, map[string]string{ apipac.Pipeline: remotePipelineURL, @@ -190,7 +193,7 @@ func TestRemote(t *testing.T) { }, }, { - name: "skipping/multiple tasks of the same name from pipelinerun annotations and pipeline annotation", + name: "skip fetching multiple tasks of the same name from pipelinerun annotations and pipeline annotation", pipelineruns: []*tektonv1.PipelineRun{ ttkn.MakePR(randomPipelineRunName, map[string]string{ apipac.Pipeline: remotePipelineURL, @@ -216,16 +219,13 @@ func TestRemote(t *testing.T) { }, expectedTaskSpec: taskFromPipelineSpec, expectedLogsSnippets: []string{ - fmt.Sprintf("successfully fetched %s from remote https url", remoteTaskURL), - fmt.Sprintf("successfully fetched %s from remote https url", remoteTaskURL), fmt.Sprintf("successfully fetched %s from remote https url", remotePipelineURL), fmt.Sprintf("successfully fetched %s from remote https url", remoteTaskURL), }, - expectedPipelinesFetch: 1, - expectedTaskFetch: 1, + expectedPipelineRun: "skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-pipeline-annotation.yaml", }, { - name: "skipping/multiple tasks of the same name from pipelinerun annotations and tektondir", + name: "skip fetching multiple tasks of the same name from pipelinerun annotations and tektondir", pipelineruns: []*tektonv1.PipelineRun{ ttkn.MakePR(randomPipelineRunName, map[string]string{ apipac.Pipeline: remotePipelineURL, @@ -253,17 +253,15 @@ func TestRemote(t *testing.T) { }, expectedTaskSpec: taskFromPipelineSpec, expectedLogsSnippets: []string{ - fmt.Sprintf("successfully fetched %s from remote https url", remoteTaskURL), fmt.Sprintf("successfully fetched %s from remote https url", remotePipelineURL), fmt.Sprintf("successfully fetched %s from remote https url", remoteTaskURL), - fmt.Sprintf("skipping remote task %s from remote pipeline %s as already defined in pipelinerun", remoteTaskName, remotePipelineName), - fmt.Sprintf("overriding task %s coming from tekton directory by an annotation task on the pipeline or pipelinerun", remoteTaskName), + fmt.Sprintf("skipping remote task %s as already fetched task %s for pipelinerun %s", remoteTaskURL, remoteTaskName, randomPipelineRunName), + fmt.Sprintf("overriding task %s coming from tekton directory by an annotation task for pipelinerun %s", remoteTaskName, randomPipelineRunName), }, - expectedPipelinesFetch: 1, - expectedTaskFetch: 1, + expectedPipelineRun: "skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml", }, { - name: "skipping/multiple pipelines of the same name from pipelinerun annotations and tektondir", + name: "skip fetching multiple pipelines of the same name from pipelinerun annotations and tektondir", pipelineruns: []*tektonv1.PipelineRun{ ttkn.MakePR(randomPipelineRunName, map[string]string{ apipac.Pipeline: remotePipelineURL, @@ -291,14 +289,11 @@ func TestRemote(t *testing.T) { }, expectedTaskSpec: taskFromPipelineSpec, expectedLogsSnippets: []string{ - fmt.Sprintf("successfully fetched %s from remote https url", remoteTaskURL), fmt.Sprintf("successfully fetched %s from remote https url", remotePipelineURL), fmt.Sprintf("successfully fetched %s from remote https url", remoteTaskURL), - fmt.Sprintf("skipping remote task %s from remote pipeline %s as already defined in pipelinerun", remoteTaskName, remotePipelineName), - fmt.Sprintf("overriding pipeline %s coming from tekton directory by the annotation pipelinerun", remotePipelineName), + fmt.Sprintf("skipping remote task %s as already fetched task %s for pipelinerun %s", remoteTaskURL, remoteTaskName, randomPipelineRunName), }, - expectedPipelinesFetch: 1, - expectedTaskFetch: 1, + expectedPipelineRun: "skip-fetching-multiple-pipelines-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml", }, } for _, tt := range tests { @@ -323,32 +318,28 @@ func TestRemote(t *testing.T) { }, }, } - ret, err := getRemotes(ctx, rt, tktype) + ret, err := resolveRemoteResources(ctx, rt, tktype, &Opts{RemoteTasks: true, GenerateName: true}) if tt.wantErrSnippet != "" { assert.ErrorContains(t, err, tt.wantErrSnippet) return } assert.NilError(t, err) - allPipelinesNames := []string{} - for _, task := range ret.Pipelines { - allPipelinesNames = append(allPipelinesNames, task.GetName()) - } - assert.Equal(t, len(ret.Pipelines), tt.expectedPipelinesFetch, allPipelinesNames) - - allTasksNames := []string{} - for _, task := range ret.Tasks { - allTasksNames = append(allTasksNames, task.GetName()) - } - assert.Equal(t, len(ret.Tasks), tt.expectedTaskFetch, allTasksNames) - for k, snippet := range tt.expectedLogsSnippets { logmsg := log.AllUntimed()[k].Message assert.Assert(t, strings.Contains(logmsg, snippet), "\n on index: %d\n we want: %s\n we got: %s", k, snippet, logmsg) } - if tt.expectedTaskFetch > 0 { - assert.DeepEqual(t, tt.expectedTaskSpec, ret.Tasks[0].Spec) + + if tt.noPipelineRun { + assert.Assert(t, len(ret) == 0, "not expecting any pipelinerun") + return } + expectedData, err := os.ReadFile("testdata/" + tt.expectedPipelineRun) + assert.NilError(t, err) + pipelineRun := &tektonv1.PipelineRun{} + err = yaml.Unmarshal(expectedData, pipelineRun) + assert.NilError(t, err) + assert.DeepEqual(t, pipelineRun, ret[0]) }) } } diff --git a/pkg/resolve/resolve.go b/pkg/resolve/resolve.go index 30cfb903e..7546bc17b 100644 --- a/pkg/resolve/resolve.go +++ b/pkg/resolve/resolve.go @@ -20,6 +20,7 @@ import ( yaml "sigs.k8s.io/yaml/goyaml.v2" ) +// Contains Resources Fetched from tektondir. type TektonTypes struct { PipelineRuns []*tektonv1.PipelineRun Pipelines []*tektonv1.Pipeline @@ -28,6 +29,18 @@ type TektonTypes struct { ValidationErrors map[string]string } +// Contains Fetched Resources for Event, with key equals to annotation value. +type FetchedResources struct { + Tasks map[string]*tektonv1.Task + Pipelines map[string]*tektonv1.Pipeline +} + +// Contains Fetched Resources for Run, with key equals to resource name from metadata.name field. +type FetchedResourcesForRun struct { + Tasks map[string]*tektonv1.Task + Pipeline *tektonv1.Pipeline +} + func NewTektonTypes() TektonTypes { return TektonTypes{ ValidationErrors: map[string]string{}, @@ -57,19 +70,10 @@ func detectAtleastNameOrGenerateNameFromPipelineRun(data string) string { return "unknown" } -// getTaskRunByName returns the taskrun with the given name the first one found +// getPipelineByName returns the Pipeline with the given name the first one found // will be matched. It does not handle conflicts so user has fetched multiple -// taskruns with the same name it will just pick up the first one. -// if the taskrun is not found it returns an error. -func getTaskByName(name string, tasks []*tektonv1.Task) (*tektonv1.Task, error) { - for _, value := range tasks { - if value.Name == name { - return value, nil - } - } - return &tektonv1.Task{}, fmt.Errorf("cannot find referenced task %s. if it's a remote task make sure to add it in the annotations", name) -} - +// pipeline with the same name it will just pick up the first one. +// if the pipeline is not found it returns an error. func getPipelineByName(name string, tasks []*tektonv1.Pipeline) (*tektonv1.Pipeline, error) { for _, value := range tasks { if value.Name == name { @@ -115,7 +119,7 @@ func isTektonAPIVersion(apiVersion string) bool { return strings.HasPrefix(apiVersion, "tekton.dev/") || apiVersion == "" } -func inlineTasks(tasks []tektonv1.PipelineTask, ropt *Opts, types TektonTypes) ([]tektonv1.PipelineTask, error) { +func inlineTasks(tasks []tektonv1.PipelineTask, ropt *Opts, remoteResource FetchedResourcesForRun) ([]tektonv1.PipelineTask, error) { pipelineTasks := []tektonv1.PipelineTask{} for _, task := range tasks { if task.TaskRef != nil && @@ -123,9 +127,9 @@ func inlineTasks(tasks []tektonv1.PipelineTask, ropt *Opts, types TektonTypes) ( isTektonAPIVersion(task.TaskRef.APIVersion) && string(task.TaskRef.Kind) != "ClusterTask" && !skippingTask(task.TaskRef.Name, ropt.SkipInlining) { - taskResolved, err := getTaskByName(task.TaskRef.Name, types.Tasks) - if err != nil { - return nil, err + taskResolved, ok := remoteResource.Tasks[task.TaskRef.Name] + if !ok { + return nil, fmt.Errorf("cannot find referenced task %s. if it's a remote task make sure to add it in the annotations", task.TaskRef.Name) } tmd := tektonv1.PipelineTaskMetadata{ Annotations: taskResolved.GetObjectMeta().GetAnnotations(), @@ -209,70 +213,18 @@ func Resolve(ctx context.Context, cs *params.Run, logger *zap.SugaredLogger, pro return []*tektonv1.PipelineRun{}, err } - // Resolve remote annotations on remote task or remote pipeline or tasks - // inside remote pipeline - if ropt.RemoteTasks { - rt := &matcher.RemoteTasks{ - Run: cs, - Event: event, - ProviderInterface: providerintf, - Logger: logger, - } - var err error - if types, err = getRemotes(ctx, rt, types); err != nil { - return []*tektonv1.PipelineRun{}, err - } + rt := &matcher.RemoteTasks{ + Run: cs, + Event: event, + ProviderInterface: providerintf, + Logger: logger, } - // Resolve {Finally/Task}Ref inside Pipeline - for _, pipeline := range types.Pipelines { - pipelineTasks, err := inlineTasks(pipeline.Spec.Tasks, ropt, types) - if err != nil { - return nil, err - } - pipeline.Spec.Tasks = pipelineTasks - - finallyTasks, err := inlineTasks(pipeline.Spec.Finally, ropt, types) - if err != nil { - return nil, err - } - pipeline.Spec.Finally = finallyTasks - } - - for _, pipelinerun := range types.PipelineRuns { - // Resolve {Finally/Task}Ref inside PipelineSpec inside PipelineRun - if pipelinerun.Spec.PipelineSpec != nil { - turns, err := inlineTasks(pipelinerun.Spec.PipelineSpec.Tasks, ropt, types) - if err != nil { - return nil, err - } - pipelinerun.Spec.PipelineSpec.Tasks = turns - - fruns, err := inlineTasks(pipelinerun.Spec.PipelineSpec.Finally, ropt, types) - if err != nil { - return nil, err - } - pipelinerun.Spec.PipelineSpec.Finally = fruns - } - - // Resolve PipelineRef inside PipelineRef - if pipelinerun.Spec.PipelineRef != nil && pipelinerun.Spec.PipelineRef.Resolver == "" { - pipelineResolved, err := getPipelineByName(pipelinerun.Spec.PipelineRef.Name, types.Pipelines) - if err != nil { - return []*tektonv1.PipelineRun{}, err - } - pipelinerun.Spec.PipelineRef = nil - pipelinerun.Spec.PipelineSpec = &pipelineResolved.Spec - } - - // Add a GenerateName based on the pipeline name and a "-" - // if we already have a GenerateName then just keep it like this - if ropt.GenerateName && pipelinerun.GenerateName == "" { - pipelinerun.GenerateName = pipelinerun.Name + "-" - pipelinerun.Name = "" - } + fetchedResources, err := resolveRemoteResources(ctx, rt, types, ropt) + if err != nil { + return []*tektonv1.PipelineRun{}, err } - return types.PipelineRuns, nil + return fetchedResources, nil } func MetadataResolve(prs []*tektonv1.PipelineRun) ([]*tektonv1.PipelineRun, error) { diff --git a/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipeline.yaml b/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipeline.yaml new file mode 100644 index 000000000..84ccf0a6b --- /dev/null +++ b/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipeline.yaml @@ -0,0 +1,17 @@ +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + annotations: + pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline + generateName: pipelinerun-abc- +spec: + pipelineSpec: + tasks: + - name: remote-task + taskSpec: + steps: + - name: step1 + image: scratch + command: + - "true" + finally: [] diff --git a/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipelinerun.yaml b/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipelinerun.yaml new file mode 100644 index 000000000..4ebb11c56 --- /dev/null +++ b/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipelinerun.yaml @@ -0,0 +1,18 @@ +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + annotations: + pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline + pipelinesascode.tekton.dev/task: http://remote/task-from-pipelinerun + generateName: pipelinerun-abc- +spec: + pipelineSpec: + tasks: + - name: remote-task + taskSpec: + steps: + - name: frompipelinerun + image: scratch + command: + - "false" + finally: [] diff --git a/pkg/resolve/testdata/skip-fetching-multiple-pipelines-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml b/pkg/resolve/testdata/skip-fetching-multiple-pipelines-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml new file mode 100644 index 000000000..91561ba87 --- /dev/null +++ b/pkg/resolve/testdata/skip-fetching-multiple-pipelines-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml @@ -0,0 +1,18 @@ +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + annotations: + pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline + pipelinesascode.tekton.dev/task: http://remote/remote-task + generateName: pipelinerun-abc- +spec: + pipelineSpec: + tasks: + - name: remote-task + taskSpec: + steps: + - name: step1 + image: scratch + command: + - "true" + finally: [] diff --git a/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-pipeline-annotation.yaml b/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-pipeline-annotation.yaml new file mode 100644 index 000000000..e7b6edd44 --- /dev/null +++ b/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-pipeline-annotation.yaml @@ -0,0 +1,19 @@ +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + annotations: + pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline + pipelinesascode.tekton.dev/task: http://remote/remote-task + pipelinesascode.tekton.dev/task-1: http://remote/remote-task + generateName: pipelinerun-abc- +spec: + pipelineSpec: + tasks: + - name: remote-task + taskSpec: + steps: + - name: step1 + image: scratch + command: + - "true" + finally: [] diff --git a/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml b/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml new file mode 100644 index 000000000..91561ba87 --- /dev/null +++ b/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml @@ -0,0 +1,18 @@ +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + annotations: + pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline + pipelinesascode.tekton.dev/task: http://remote/remote-task + generateName: pipelinerun-abc- +spec: + pipelineSpec: + tasks: + - name: remote-task + taskSpec: + steps: + - name: step1 + image: scratch + command: + - "true" + finally: []