From 0edaa504f38e46e4be548731a41569598774b9f6 Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Wed, 8 Nov 2023 15:14:26 +0100 Subject: [PATCH] Add support of remote task on remote Pipeline We now support remote tasks on remote Pipeline, allowing to share a remote Pipeline across multiple repositories. User can override tasks from the remote pipeline by adding a task with the same name. Signed-off-by: Chmouel Boudjnah --- docs/content/docs/guide/resolver.md | 82 ++++- pkg/action/patch_test.go | 2 +- pkg/matcher/annotation_tasks_install.go | 7 +- pkg/matcher/annotation_tasks_install_test.go | 4 +- pkg/pipelineascode/pipelineascode_test.go | 2 +- pkg/reconciler/controller_test.go | 2 +- pkg/resolve/remote.go | 105 ++++++ pkg/resolve/remote_test.go | 356 +++++++++++++++++++ pkg/resolve/resolve.go | 173 +++++---- pkg/test/tekton/genz.go | 43 ++- 10 files changed, 666 insertions(+), 110 deletions(-) create mode 100644 pkg/resolve/remote.go create mode 100644 pkg/resolve/remote_test.go diff --git a/docs/content/docs/guide/resolver.md b/docs/content/docs/guide/resolver.md index ef214772f..1a18fd105 100644 --- a/docs/content/docs/guide/resolver.md +++ b/docs/content/docs/guide/resolver.md @@ -46,7 +46,7 @@ command to learn on how to use it. location with annotations on PipelineRun. If the resolver sees a PipelineRun referencing a remote task or a Pipeline in -a PipelineRun or a PipelineSpec it will automatically inlines them. +a PipelineRun or a PipelineSpec it will automatically inline them. If multiple annotations reference the same task name the resolver will pick the first one fetched from the annotations. @@ -128,10 +128,10 @@ will fetch the task directly from that remote URL : ### Remote HTTP URL from a private GitHub repository If you are using `GitHub` and If the remote task URL uses the same host as where -the repo CRD is, PAC will use the GitHub token and fetch the URL using the +the repository CRD is, PAC will use the GitHub token and fetch the URL using the GitHub API. -For example if you have a repo URL looking like this : +For example if you have a repository URL looking like this : @@ -139,7 +139,8 @@ and the remote HTTP URLs is a referenced GitHub "blob" URL: -if the remote HTTP url has a slash (/) in the branch name you will need to html encode with the `%2F` character, eg: +if the remote HTTP URL has a slash (/) in the branch name you will need to HTML +encode with the `%2F` character, example: @@ -150,14 +151,14 @@ GitHub app token are scoped to the owner or organization where the repository is If you are using the GitHub webhook method you are able to fetch any private or public repositories on any organization where the personal token is allowed. -There is settings you can set in the pac `Configmap` to control that behavior, see the +There is settings you can set in the pac `Configmap` to control that behaviour, see the `secret-github-app-token-scoped` and `secret-github-app-scope-extra-repos` settings in the [settings documentation](/docs/install/settings). ### Tasks or Pipelines inside the repository Additionally, you can as well have a reference to a task or pipeline from a YAML file inside -your repo if you specify the relative path to it, for example : +your repository if you specify the relative path to it, for example : ```yaml pipelinesascode.tekton.dev/task: "[share/tasks/git-clone.yaml]" @@ -174,18 +175,77 @@ If the object fetched cannot be parsed as a Tekton `Task` it will error out. ## Remote Pipeline annotations -Remote Pipeline can be referenced by annotation, this allows you to share your Pipeline definition across. +Remote Pipeline can be referenced by annotation, allowing you to share a Pipeline across multiple repositories. -Only one Pipeline is allowed in annotation. +Only one Pipeline is allowed on the `PipelineRun` annotation. -An annotation to a remote pipeline looks like this : +An annotation to a remote pipeline looks like this, using a remote URL: ```yaml pipelinesascode.tekton.dev/pipeline: "https://git.provider/raw/pipeline.yaml ``` -It supports remote URL and files inside the same Git repository. +or from a relative path inside the repository: + +```yaml +pipelinesascode.tekton.dev/pipeline: "./tasks/pipeline.yaml +``` + +Fetching `Pipelines` from the [Tekton Hub](https://hub.tekton.dev) is not currently supported. + +### Overriding tasks from a remote pipeline on a PipelineRun + +Remote task annotations on the remote pipeline are supported. No other +annotations like `on-target-branch`, `on-event` or `on-cel-expression` are +supported. + +If a user wants to override one of the tasks from the remote pipeline, they can do +so by adding a task in the annotations that has the same name In their `PipelineRun` annotations. + +For example if the user PipelineRun contains those annotations: + +```yaml +kind: PipelineRun +metadata: + annotations: + pipelinesascode.tekton.dev/pipeline: "https://git.provider/raw/pipeline.yaml + pipelinesascode.tekton.dev/task: "./my-git-clone-task.yaml +``` + +and the Pipeline referenced by the `pipelinesascode.tekton.dev/pipeline` annotation +in "" contains those annotations: + +```yaml +kind: Pipeline +metadata: + annotations: + pipelinesascode.tekton.dev/task: "git-clone" +``` + +In this case if the `my-git-clone-task.yaml` file in the root directory is a +task named `git-clone` it will be used instead of the `git-clone` on the remote +pipeline that is coming from the Tekon Hub. {{< hint info >}} -[Tekton Hub](https://hub.tekton.dev) doesn't currently have support for `Pipeline`. +Task overriding is only supported for tasks that are referenced by a `taskRef` +to a `Name`, no override is done on `Tasks` embedded with a `taskSpec`. See +[Tekton documentation](https://tekton.dev/docs/pipelines/pipelines/#adding-tasks-to-the-pipeline) for the differences between `taskRef` and `taskSpec`: {{< /hint >}} + +### Tasks or Pipelines Precedence + +From where tasks or pipelines of the same name takes precedence? + +for remote Tasks, when you have a `taskRef` on a task name, pac will try to find the task in this order: + +1. A task matched from the PipelineRun annotations +2. A task matched from the remote Pipeline annotations +3. A task matched fetched from the Tekton directory + (the tasks from the `.tekton` directory and its subdirs are automatically included) + +for remote Pipelines referenced on a `pipelineRef`, pac will try to match a +pipeline in this order: + +1. Pipeline from the PipelineRun annotations +2. Pipeline from the Tekton directory (pipelines are automatically fetched from + the `.tekton` directory and subdirs) diff --git a/pkg/action/patch_test.go b/pkg/action/patch_test.go index 8fe46772d..5dd091c3f 100644 --- a/pkg/action/patch_test.go +++ b/pkg/action/patch_test.go @@ -22,7 +22,7 @@ func TestPatchPipelineRun(t *testing.T) { observer, _ := zapobserver.New(zap.InfoLevel) logger := zap.New(observer).Sugar() - testPR := tektontest.MakePR("namespace", "force-me", []pipelinev1.ChildStatusReference{ + testPR := tektontest.MakePRStatus("namespace", "force-me", []pipelinev1.ChildStatusReference{ tektontest.MakeChildStatusReference("first"), tektontest.MakeChildStatusReference("last"), tektontest.MakeChildStatusReference("middle"), diff --git a/pkg/matcher/annotation_tasks_install.go b/pkg/matcher/annotation_tasks_install.go index 315147c04..57b148fe5 100644 --- a/pkg/matcher/annotation_tasks_install.go +++ b/pkg/matcher/annotation_tasks_install.go @@ -216,7 +216,7 @@ func (rt RemoteTasks) GetTaskFromAnnotations(ctx context.Context, annotations ma // 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) { +func (rt RemoteTasks) GetPipelineFromAnnotations(ctx context.Context, annotations map[string]string) (*tektonv1.Pipeline, error) { ret := []*tektonv1.Pipeline{} pipelinesAnnotation, err := grabValuesFromAnnotations(annotations, pipelineAnnotationsRegexp) if err != nil { @@ -225,6 +225,9 @@ func (rt RemoteTasks) GetPipelineFromAnnotations(ctx context.Context, annotation if len(pipelinesAnnotation) > 1 { return nil, fmt.Errorf("only one pipeline is allowed on remote resolution, we have received multiple of them: %+v", pipelinesAnnotation) } + if len(pipelinesAnnotation) == 0 { + return nil, nil + } for _, v := range pipelinesAnnotation { data, err := rt.getRemote(ctx, v, false) if err != nil { @@ -239,7 +242,7 @@ func (rt RemoteTasks) GetPipelineFromAnnotations(ctx context.Context, annotation } ret = append(ret, pipeline) } - return ret, nil + return ret[0], nil } // getTaskFromLocalFS 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 cccde93f8..191850c5d 100644 --- a/pkg/matcher/annotation_tasks_install_test.go +++ b/pkg/matcher/annotation_tasks_install_test.go @@ -463,10 +463,10 @@ func TestGetPipelineFromAnnotations(t *testing.T) { assert.Assert(t, len(fakelog.FilterMessageSnippet(tt.wantLog).TakeAll()) > 0, "could not find log message: got ", fakelog) } assert.NilError(t, err) - assert.Assert(t, len(got) > 0, "GetPipelineFromAnnotations() error no pipelines has been processed") + assert.Assert(t, got != nil, "GetPipelineFromAnnotations() error no pipelines has been processed") if tt.gotPipelineName != "" { - assert.Equal(t, tt.gotPipelineName, got[0].GetName()) + assert.Equal(t, tt.gotPipelineName, got.GetName()) } }) } diff --git a/pkg/pipelineascode/pipelineascode_test.go b/pkg/pipelineascode/pipelineascode_test.go index 01710ec1a..5616ce52d 100644 --- a/pkg/pipelineascode/pipelineascode_test.go +++ b/pkg/pipelineascode/pipelineascode_test.go @@ -517,7 +517,7 @@ func TestRun(t *testing.T) { }, Repositories: tt.repositories, PipelineRuns: []*pipelinev1.PipelineRun{ - tektontest.MakePR("namespace", "force-me", []pipelinev1.ChildStatusReference{ + tektontest.MakePRStatus("namespace", "force-me", []pipelinev1.ChildStatusReference{ tektontest.MakeChildStatusReference("first"), tektontest.MakeChildStatusReference("last"), tektontest.MakeChildStatusReference("middle"), diff --git a/pkg/reconciler/controller_test.go b/pkg/reconciler/controller_test.go index c9cd24f33..7c64de671 100644 --- a/pkg/reconciler/controller_test.go +++ b/pkg/reconciler/controller_test.go @@ -33,7 +33,7 @@ func TestCheckStateAndEnqueue(t *testing.T) { }) // Create a new PipelineRun object with the "started" state label. - testPR := tektontest.MakePR("namespace", "force-me", []pipelinev1.ChildStatusReference{ + testPR := tektontest.MakePRStatus("namespace", "force-me", []pipelinev1.ChildStatusReference{ tektontest.MakeChildStatusReference("first"), tektontest.MakeChildStatusReference("last"), tektontest.MakeChildStatusReference("middle"), diff --git a/pkg/resolve/remote.go b/pkg/resolve/remote.go new file mode 100644 index 000000000..975673300 --- /dev/null +++ b/pkg/resolve/remote.go @@ -0,0 +1,105 @@ +package resolve + +import ( + "context" + "fmt" + + "github.com/openshift-pipelines/pipelines-as-code/pkg/matcher" +) + +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 + } + } + return false +} + +// getRemotes will get remote tasks or Pipelines from annotations. +// +// It already has some tasks or pipeline coming from the tekton directory stored in [types] +// +// The precedence logic for tasks is in this order: +// +// * Tasks from the PipelineRun annotations +// * Tasks from the Pipeline annotations +// * Tasks from the Tekton directory +// +// 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{} + 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) + } + + for _, task := range remoteTasks { + if alreadySeen(remoteType.Tasks, task) { + rt.Logger.Infof("skipping duplicated task %s in annotations on pipelinerun %s", task.GetName(), pipelinerun.GetName()) + 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) + } + + if remotePipeline != nil { + remoteType.Pipelines = append(remoteType.Pipelines, remotePipeline) + } + } + + // grab the tasks from the remote pipeline + for _, pipeline := range remoteType.Pipelines { + if pipeline.GetObjectMeta().GetAnnotations() == nil { + continue + } + 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) + } + + 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()) + continue + } + remoteType.Tasks = append(remoteType.Tasks, remoteTask) + } + } + + 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 + } + 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 + } + ret.Pipelines = append(ret.Pipelines, remotePipeline) + } + return ret, nil +} diff --git a/pkg/resolve/remote_test.go b/pkg/resolve/remote_test.go new file mode 100644 index 000000000..d1fd56116 --- /dev/null +++ b/pkg/resolve/remote_test.go @@ -0,0 +1,356 @@ +package resolve + +import ( + "fmt" + "strings" + "testing" + + apipac "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" + "github.com/openshift-pipelines/pipelines-as-code/pkg/matcher" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients" + httptesthelper "github.com/openshift-pipelines/pipelines-as-code/pkg/test/http" + testprovider "github.com/openshift-pipelines/pipelines-as-code/pkg/test/provider" + ttkn "github.com/openshift-pipelines/pipelines-as-code/pkg/test/tekton" + tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "go.uber.org/zap" + zapobserver "go.uber.org/zap/zaptest/observer" + "gotest.tools/v3/assert" + rtesting "knative.dev/pkg/reconciler/testing" + "sigs.k8s.io/yaml" +) + +func TestRemote(t *testing.T) { + randomPipelineRunName := "pipelinerun-abc" + remotePipelineName := "remote-pipeline" + remotePipelineURL := "http://remote/" + remotePipelineName + + taskFromPipelineRunName := "task-from-pipelinerun" + taskFromPipelineRunURL := "http://remote/" + taskFromPipelineRunName + + remoteTaskName := "remote-task" + remoteTaskURL := "http://remote/" + remoteTaskName + taskFromPipelineSpec := tektonv1.TaskSpec{ + Steps: []tektonv1.Step{ + { + Name: "step1", + Image: "scratch", + Command: []string{"true"}, + }, + }, + } + taskFromPipelineRunSpec := tektonv1.TaskSpec{ + Steps: []tektonv1.Step{ + { + Name: "frompipelinerun", + Image: "scratch", + Command: []string{"false"}, + }, + }, + } + pipelineTaskEmbedded := []tektonv1.PipelineTask{ + { + Name: "embedded", + TaskSpec: &tektonv1.EmbeddedTask{ + TaskSpec: taskFromPipelineSpec, + }, + }, + } + pipelinewithTaskEmbedded := ttkn.MakePipeline(remotePipelineName, pipelineTaskEmbedded, nil) + pipelinewithTaskEmbeddedB, err := yaml.Marshal(pipelinewithTaskEmbedded) + assert.NilError(t, err) + + pipelineTaskRef := []tektonv1.PipelineTask{ + { + Name: remoteTaskName, + }, + } + pipelinewithTaskRef := ttkn.MakePipeline(remotePipelineName, pipelineTaskRef, map[string]string{ + apipac.Task: remoteTaskURL, + }) + pipelinewithTaskRefYamlB, err := yaml.Marshal(pipelinewithTaskRef) + assert.NilError(t, err) + + singleTask := ttkn.MakeTask(remoteTaskName, taskFromPipelineSpec) + singleTaskB, err := yaml.Marshal(singleTask) + assert.NilError(t, err) + + taskFromPipelineRun := ttkn.MakeTask(remoteTaskName, taskFromPipelineRunSpec) + taskFromPipelineRunB, err := yaml.Marshal(taskFromPipelineRun) + 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: "remote pipeline with remote task from pipeline", + pipelineruns: []*tektonv1.PipelineRun{ + ttkn.MakePR(randomPipelineRunName, map[string]string{ + apipac.Pipeline: remotePipelineURL, + }, + tektonv1.PipelineRunSpec{ + PipelineRef: &tektonv1.PipelineRef{ + Name: "remote-pipeline", + }, + }, + ), + }, + remoteURLS: map[string]map[string]string{ + "http://remote/embedpipeline": { + "body": string(pipelinewithTaskEmbeddedB), + "code": "200", + }, + remotePipelineURL: { + "body": string(pipelinewithTaskRefYamlB), + "code": "200", + }, + remoteTaskURL: { + "body": string(singleTaskB), + "code": "200", + }, + }, + expectedTaskSpec: taskFromPipelineSpec, + expectedLogsSnippets: []string{ + fmt.Sprintf("successfully fetched %s from remote https url", remotePipelineURL), + fmt.Sprintf("successfully fetched %s from remote https url", remoteTaskURL), + }, + expectedPipelinesFetch: 1, + expectedTaskFetch: 1, + }, + { + name: "remote pipeline with remote task in pipeline overridden from pipelinerun", + pipelineruns: []*tektonv1.PipelineRun{ + ttkn.MakePR(randomPipelineRunName, map[string]string{ + apipac.Pipeline: remotePipelineURL, + apipac.Task: taskFromPipelineRunURL, + }, + tektonv1.PipelineRunSpec{ + PipelineRef: &tektonv1.PipelineRef{ + Name: "remote-pipeline", + }, + }, + ), + }, + expectedTaskSpec: taskFromPipelineRunSpec, + remoteURLS: map[string]map[string]string{ + remotePipelineURL: { + "body": string(pipelinewithTaskRefYamlB), + "code": "200", + }, + remoteTaskURL: { + "body": string(singleTaskB), + "code": "200", + }, + taskFromPipelineRunURL: { + "body": string(taskFromPipelineRunB), + "code": "200", + }, + }, + expectedLogsSnippets: []string{ + fmt.Sprintf("successfully fetched %s from remote https url", taskFromPipelineRunURL), + fmt.Sprintf("successfully fetched %s from remote https url", remotePipelineURL), + }, + expectedPipelinesFetch: 1, + expectedTaskFetch: 1, + }, + { + name: "remote pipelinerun no annotations", + pipelineruns: []*tektonv1.PipelineRun{ + ttkn.MakePR(randomPipelineRunName, map[string]string{}, + tektonv1.PipelineRunSpec{ + PipelineRef: &tektonv1.PipelineRef{ + Name: "remote-pipeline", + }, + }, + ), + }, + }, + { + name: "error/remote pipelinerun is 404", + wantErrSnippet: "error getting remote pipeline " + remotePipelineURL, + pipelineruns: []*tektonv1.PipelineRun{ + ttkn.MakePR(randomPipelineRunName, map[string]string{ + apipac.Pipeline: remotePipelineURL, + }, + tektonv1.PipelineRunSpec{ + PipelineRef: &tektonv1.PipelineRef{ + Name: "remote-pipeline", + }, + }, + ), + }, + }, + { + name: "skipping/multiple tasks of the same name from pipelinerun annotations and pipeline annotation", + pipelineruns: []*tektonv1.PipelineRun{ + ttkn.MakePR(randomPipelineRunName, map[string]string{ + apipac.Pipeline: remotePipelineURL, + apipac.Task: remoteTaskURL, + apipac.Task + "-1": remoteTaskURL, + }, + tektonv1.PipelineRunSpec{ + PipelineRef: &tektonv1.PipelineRef{ + Name: "remote-pipeline", + }, + }, + ), + }, + remoteURLS: map[string]map[string]string{ + remotePipelineURL: { + "body": string(pipelinewithTaskRefYamlB), + "code": "200", + }, + remoteTaskURL: { + "body": string(singleTaskB), + "code": "200", + }, + }, + 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("skipping duplicated task %s in annotations on pipelinerun %s", remoteTaskName, randomPipelineRunName), + 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), + }, + expectedPipelinesFetch: 1, + expectedTaskFetch: 1, + }, + { + name: "skipping/multiple tasks of the same name from pipelinerun annotations and tektondir", + pipelineruns: []*tektonv1.PipelineRun{ + ttkn.MakePR(randomPipelineRunName, map[string]string{ + apipac.Pipeline: remotePipelineURL, + apipac.Task: remoteTaskURL, + }, + tektonv1.PipelineRunSpec{ + PipelineRef: &tektonv1.PipelineRef{ + Name: "remote-pipeline", + }, + }, + ), + }, + tasks: []*tektonv1.Task{ + singleTask, + }, + remoteURLS: map[string]map[string]string{ + remotePipelineURL: { + "body": string(pipelinewithTaskRefYamlB), + "code": "200", + }, + remoteTaskURL: { + "body": string(singleTaskB), + "code": "200", + }, + }, + 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), + }, + expectedPipelinesFetch: 1, + expectedTaskFetch: 1, + }, + { + name: "skipping/multiple pipelines of the same name from pipelinerun annotations and tektondir", + pipelineruns: []*tektonv1.PipelineRun{ + ttkn.MakePR(randomPipelineRunName, map[string]string{ + apipac.Pipeline: remotePipelineURL, + apipac.Task: remoteTaskURL, + }, + tektonv1.PipelineRunSpec{ + PipelineRef: &tektonv1.PipelineRef{ + Name: "remote-pipeline", + }, + }, + ), + }, + pipelines: []*tektonv1.Pipeline{ + pipelinewithTaskEmbedded, + }, + remoteURLS: map[string]map[string]string{ + remotePipelineURL: { + "body": string(pipelinewithTaskRefYamlB), + "code": "200", + }, + remoteTaskURL: { + "body": string(singleTaskB), + "code": "200", + }, + }, + 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), + }, + expectedPipelinesFetch: 1, + expectedTaskFetch: 1, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + observer, log := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() + ctx, _ := rtesting.SetupFakeContext(t) + tktype := TektonTypes{ + Pipelines: tt.pipelines, + Tasks: tt.tasks, + PipelineRuns: tt.pipelineruns, + } + + tprovider := &testprovider.TestProviderImp{} + httpTestClient := httptesthelper.MakeHTTPTestClient(tt.remoteURLS) + rt := &matcher.RemoteTasks{ + ProviderInterface: tprovider, + Logger: logger, + Run: ¶ms.Run{ + Clients: clients.Clients{ + HTTP: *httpTestClient, + }, + }, + } + ret, err := getRemotes(ctx, rt, tktype) + 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) + } + }) + } +} diff --git a/pkg/resolve/resolve.go b/pkg/resolve/resolve.go index 51055cae1..10541f551 100644 --- a/pkg/resolve/resolve.go +++ b/pkg/resolve/resolve.go @@ -27,53 +27,6 @@ type TektonTypes struct { var yamlDocSeparatorRe = regexp.MustCompile(`(?m)^---\s*$`) -func ReadTektonTypes(ctx context.Context, log *zap.SugaredLogger, data string) (TektonTypes, error) { - types := TektonTypes{} - decoder := k8scheme.Codecs.UniversalDeserializer() - - for _, doc := range yamlDocSeparatorRe.Split(data, -1) { - if strings.TrimSpace(doc) == "" { - continue - } - - obj, _, err := decoder.Decode([]byte(doc), nil, nil) - if err != nil { - log.Infof("Skipping document not looking like a kubernetes resources: %v", err) - continue - } - switch o := obj.(type) { - case *tektonv1beta1.Pipeline: //nolint: staticcheck // we need to support v1beta1 - c := &tektonv1.Pipeline{} - if err := o.ConvertTo(ctx, c); err != nil { - return types, fmt.Errorf("pipeline v1beta1 %s cannot be converted as v1: err: %w", o.GetName(), err) - } - types.Pipelines = append(types.Pipelines, c) - case *tektonv1beta1.PipelineRun: //nolint: staticcheck // we need to support v1beta1 - c := &tektonv1.PipelineRun{} - if err := o.ConvertTo(ctx, c); err != nil { - return types, fmt.Errorf("pipelinerun v1beta1 %s cannot be converted as v1: err: %w", o.GetName(), err) - } - types.PipelineRuns = append(types.PipelineRuns, c) - case *tektonv1beta1.Task: //nolint: staticcheck // we need to support v1beta1 - c := &tektonv1.Task{} - if err := o.ConvertTo(ctx, c); err != nil { - return types, fmt.Errorf("task v1beta1 %s cannot be converted as v1: err: %w", o.GetName(), err) - } - types.Tasks = append(types.Tasks, c) - case *tektonv1.PipelineRun: - types.PipelineRuns = append(types.PipelineRuns, o) - case *tektonv1.Pipeline: - types.Pipelines = append(types.Pipelines, o) - case *tektonv1.Task: - types.Tasks = append(types.Tasks, o) - default: - log.Info("Skipping document not looking like a tekton resource we can Resolve.") - } - } - - return types, nil -} - // getTaskRunByName returns the taskrun 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. @@ -96,6 +49,29 @@ func getPipelineByName(name string, tasks []*tektonv1.Pipeline) (*tektonv1.Pipel return &tektonv1.Pipeline{}, fmt.Errorf("cannot find referenced pipeline %s. for a remote pipeline make sure to add it in the annotation", name) } +func pipelineRunsWithSameName(prs []*tektonv1.PipelineRun) error { + prNames := map[string]bool{} + for _, pr := range prs { + name := pr.GetName() + generateName := pr.GetGenerateName() + + if name != "" { + if _, exist := prNames[name]; exist { + return fmt.Errorf("found multiple pipelinerun in .tekton with the same name: %v, please update", name) + } + prNames[name] = true + } + + if generateName != "" { + if _, exist := prNames[generateName]; exist { + return fmt.Errorf("found multiple pipelinerun in .tekton with the same generateName: %v, please update", generateName) + } + prNames[generateName] = true + } + } + return nil +} + func skippingTask(taskName string, skippedTasks []string) bool { for _, value := range skippedTasks { if value == taskName { @@ -136,6 +112,53 @@ type Opts struct { ProviderToken string } +func ReadTektonTypes(ctx context.Context, log *zap.SugaredLogger, data string) (TektonTypes, error) { + types := TektonTypes{} + decoder := k8scheme.Codecs.UniversalDeserializer() + + for _, doc := range yamlDocSeparatorRe.Split(data, -1) { + if strings.TrimSpace(doc) == "" { + continue + } + + obj, _, err := decoder.Decode([]byte(doc), nil, nil) + if err != nil { + log.Infof("Skipping document not looking like a kubernetes resources: %v", err) + continue + } + switch o := obj.(type) { + case *tektonv1beta1.Pipeline: //nolint: staticcheck // we need to support v1beta1 + c := &tektonv1.Pipeline{} + if err := o.ConvertTo(ctx, c); err != nil { + return types, fmt.Errorf("pipeline v1beta1 %s cannot be converted as v1: err: %w", o.GetName(), err) + } + types.Pipelines = append(types.Pipelines, c) + case *tektonv1beta1.PipelineRun: //nolint: staticcheck // we need to support v1beta1 + c := &tektonv1.PipelineRun{} + if err := o.ConvertTo(ctx, c); err != nil { + return types, fmt.Errorf("pipelinerun v1beta1 %s cannot be converted as v1: err: %w", o.GetName(), err) + } + types.PipelineRuns = append(types.PipelineRuns, c) + case *tektonv1beta1.Task: //nolint: staticcheck // we need to support v1beta1 + c := &tektonv1.Task{} + if err := o.ConvertTo(ctx, c); err != nil { + return types, fmt.Errorf("task v1beta1 %s cannot be converted as v1: err: %w", o.GetName(), err) + } + types.Tasks = append(types.Tasks, c) + case *tektonv1.PipelineRun: + types.PipelineRuns = append(types.PipelineRuns, o) + case *tektonv1.Pipeline: + types.Pipelines = append(types.Pipelines, o) + case *tektonv1.Task: + types.Tasks = append(types.Tasks, o) + default: + log.Info("Skipping document not looking like a tekton resource we can Resolve.") + } + } + + return types, nil +} + // Resolve gets a large string which is a yaml multi documents containing // Pipeline/PipelineRuns/Tasks and resolve them inline as a single PipelineRun // generateName can be set as True to set the name as a generateName + "-" for @@ -149,27 +172,18 @@ func Resolve(ctx context.Context, cs *params.Run, logger *zap.SugaredLogger, pro return []*tektonv1.PipelineRun{}, err } - // First resolve Annotations Tasks - for _, pipelinerun := range types.PipelineRuns { - if ropt.RemoteTasks && pipelinerun.GetObjectMeta().GetAnnotations() != nil { - rt := matcher.RemoteTasks{ - Run: cs, - Event: event, - ProviderInterface: providerintf, - Logger: logger, - } - remoteTasks, err := rt.GetTaskFromAnnotations(ctx, pipelinerun.GetObjectMeta().GetAnnotations()) - if err != nil { - return []*tektonv1.PipelineRun{}, err - } - // Merge remote tasks with local tasks - types.Tasks = append(types.Tasks, remoteTasks...) - - remotePipelines, err := rt.GetPipelineFromAnnotations(ctx, pipelinerun.GetObjectMeta().GetAnnotations()) - if err != nil { - return []*tektonv1.PipelineRun{}, err - } - types.Pipelines = append(types.Pipelines, remotePipelines...) + // 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 } } @@ -224,29 +238,6 @@ func Resolve(ctx context.Context, cs *params.Run, logger *zap.SugaredLogger, pro return types.PipelineRuns, nil } -func pipelineRunsWithSameName(prs []*tektonv1.PipelineRun) error { - prNames := map[string]bool{} - for _, pr := range prs { - name := pr.GetName() - generateName := pr.GetGenerateName() - - if name != "" { - if _, exist := prNames[name]; exist { - return fmt.Errorf("found multiple pipelinerun in .tekton with the same name: %v, please update", name) - } - prNames[name] = true - } - - if generateName != "" { - if _, exist := prNames[generateName]; exist { - return fmt.Errorf("found multiple pipelinerun in .tekton with the same generateName: %v, please update", generateName) - } - prNames[generateName] = true - } - } - return nil -} - func MetadataResolve(prs []*tektonv1.PipelineRun) ([]*tektonv1.PipelineRun, error) { if err := pipelineRunsWithSameName(prs); err != nil { return []*tektonv1.PipelineRun{}, err diff --git a/pkg/test/tekton/genz.go b/pkg/test/tekton/genz.go index e345231d2..5124db2bd 100644 --- a/pkg/test/tekton/genz.go +++ b/pkg/test/tekton/genz.go @@ -50,7 +50,7 @@ func MakeChildStatusReference(name string) tektonv1.ChildStatusReference { } } -func MakePR(namespace, name string, childStatus []tektonv1.ChildStatusReference, status *knativeduckv1.Status) *tektonv1.PipelineRun { +func MakePRStatus(namespace, name string, childStatus []tektonv1.ChildStatusReference, status *knativeduckv1.Status) *tektonv1.PipelineRun { if status == nil { status = &knativeduckv1.Status{} } @@ -68,6 +68,20 @@ func MakePR(namespace, name string, childStatus []tektonv1.ChildStatusReference, } } +func MakePR(name string, annotations map[string]string, spec tektonv1.PipelineRunSpec) *tektonv1.PipelineRun { + return &tektonv1.PipelineRun{ + TypeMeta: metav1.TypeMeta{ + Kind: "PipelineRun", + APIVersion: "tekton.dev/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Annotations: annotations, + }, + Spec: spec, + } +} + func MakePRCompletion(clock clockwork.FakeClock, name, namespace, runstatus string, annotations, labels map[string]string, timeshift int) *tektonv1.PipelineRun { // fakeing time logic give me headache // this will make the pr finish 5mn ago, starting 5-5mn ago @@ -110,6 +124,33 @@ func MakePRCompletion(clock clockwork.FakeClock, name, namespace, runstatus stri } } +func MakePipeline(name string, tasks []tektonv1.PipelineTask, annotations map[string]string) *tektonv1.Pipeline { + return &tektonv1.Pipeline{ + TypeMeta: metav1.TypeMeta{ + Kind: "Pipeline", + APIVersion: "tekton.dev/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Annotations: annotations, + }, + Spec: tektonv1.PipelineSpec{ + Tasks: tasks, + }, + } +} + +func MakeTask(name string, taskSpec tektonv1.TaskSpec) *tektonv1.Task { + return &tektonv1.Task{ + TypeMeta: metav1.TypeMeta{ + Kind: "Task", + APIVersion: "tekton.dev/v1", + }, + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: taskSpec, + } +} + func MakeTaskRunCompletion(clock clockwork.FakeClock, name, namespace, runstatus string, annotation map[string]string, taskStatus tektonv1.TaskRunStatusFields, conditions knativeduckv1.Conditions, timeshift int) *tektonv1.TaskRun { starttime := time.Duration((timeshift - 5*-1) * int(time.Minute)) endtime := time.Duration((timeshift * -1) * int(time.Minute))