Merge pull request #379 from sm43/fix-annotation-validation

openshift-pipelines · Dec 8, 2021 · b2ee645 · b2ee645
2 parents 8fadd2b + 913e03b
commit b2ee645
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 12 deletions.
diff --git a/docs/guide.md b/docs/guide.md
@@ -262,7 +262,11 @@ Pipeline or a PipelineSpec it will automatically inlines it.
 An annotation to a remote task looks like this :
 
   ```yaml
-  pipelinesascode.tekton.dev/task: "[git-clone]"
+  pipelinesascode.tekton.dev/task: "git-clone"
+  ```
+or multiple tasks via an array :
+  ```yaml
+  pipelinesascode.tekton.dev/task: ["git-clone", "pylint"]
   ```
 
 this installs the
@@ -279,9 +283,9 @@ You can have multiple lines if you add a `-NUMBER` suffix to the annotation, for
 example :
 
 ```yaml
-  pipelinesascode.tekton.dev/task: "[git-clone]"
-  pipelinesascode.tekton.dev/task-1: "[golang-test]"
-  pipelinesascode.tekton.dev/task-2: "[tkn]"
+  pipelinesascode.tekton.dev/task: "git-clone"
+  pipelinesascode.tekton.dev/task-1: "golang-test"
+  pipelinesascode.tekton.dev/task-2: "tkn" 
 ```
 
 By default `Pipelines as Code` will interpret the string as the `latest` task to

diff --git a/pkg/matcher/annotation_matcher.go b/pkg/matcher/annotation_matcher.go
@@ -19,8 +19,11 @@ const (
 	onEventAnnotation        = "on-event"
 	onTargetBranchAnnotation = "on-target-branch"
 	onTargetNamespace        = "target-namespace"
-	reValidateTag            = `^\[(.*)\]$`
 	maxKeepRuns              = "max-keep-runs"
+
+	// regex allows array of string or a single string
+	// eg. ["foo", "bar"], ["foo"] or "foo"
+	reValidateTag = `^\[(.*)\]$|^[^[\]\s]*$`
 )
 
 func branchMatch(prunBranch, baseBranch string) bool {
@@ -38,11 +41,17 @@ func branchMatch(prunBranch, baseBranch string) bool {
 // TODO: move to another file since it's common to all annotations_* files
 func getAnnotationValues(annotation string) ([]string, error) {
 	re := regexp.MustCompile(reValidateTag)
+	annotation = strings.TrimSpace(annotation)
 	match := re.Match([]byte(annotation))
 	if !match {
 		return nil, errors.New("annotations in pipeline are in wrong format")
 	}
 
+	// if it's not an array then it would be a single string
+	if !strings.HasPrefix(annotation, "[") {
+		return []string{annotation}, nil
+	}
+
 	// Split all tasks by comma and make sure to trim spaces in there
 	splitted := strings.Split(re.FindStringSubmatch(annotation)[1], ",")
 	for i := range splitted {

diff --git a/pkg/matcher/annotation_matcher_test.go b/pkg/matcher/annotation_matcher_test.go
@@ -241,13 +241,13 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			name: "bad-event-annotation",
+			name: "single-event-annotation",
 			args: args{
 				runevent: info.Event{EventType: "pull_request", BaseBranch: "main"},
 				pruns: []*tektonv1beta1.PipelineRun{
 					{
 						ObjectMeta: metav1.ObjectMeta{
-							Name: "bad-event-annotation",
+							Name: "single-event-annotation",
 							Annotations: map[string]string{
 								pipelinesascode.GroupName + "/" + onEventAnnotation:        "pull_request",
 								pipelinesascode.GroupName + "/" + onTargetBranchAnnotation: "[main]",
@@ -256,16 +256,16 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) {
 					},
 				},
 			},
-			wantErr: true,
+			wantErr: false,
 		},
 		{
-			name: "bad-target-branch-annotation",
+			name: "single-target-branch-annotation",
 			args: args{
 				runevent: info.Event{EventType: "pull_request", BaseBranch: "main"},
 				pruns: []*tektonv1beta1.PipelineRun{
 					{
 						ObjectMeta: metav1.ObjectMeta{
-							Name: "bad-target-branch-annotation",
+							Name: "single-target-branch-annotation",
 							Annotations: map[string]string{
 								pipelinesascode.GroupName + "/" + onEventAnnotation:        "[pull_request]",
 								pipelinesascode.GroupName + "/" + onTargetBranchAnnotation: "main",
@@ -274,7 +274,7 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) {
 					},
 				},
 			},
-			wantErr: true,
+			wantErr: false,
 		},
 		{
 			name: "empty-annotation",
@@ -389,6 +389,14 @@ func Test_getAnnotationValues(t *testing.T) {
 		want    []string
 		wantErr bool
 	}{
+		{
+			name: "get-annotation-string",
+			args: args{
+				annotation: "foo",
+			},
+			want:    []string{"foo"},
+			wantErr: false,
+		},
 		{
 			name: "get-annotation-simple",
 			args: args{
@@ -405,6 +413,13 @@ func Test_getAnnotationValues(t *testing.T) {
 			want:    []string{"foo", "bar"},
 			wantErr: false,
 		},
+		{
+			name: "get-annotation-multiple-string-bad-syntax",
+			args: args{
+				annotation: "foo, bar",
+			},
+			wantErr: true,
+		},
 		{
 			name: "get-annotation-bad-syntax",
 			args: args{

diff --git a/test/testdata/pipelinerun_remote_annotations.yaml b/test/testdata/pipelinerun_remote_annotations.yaml
@@ -6,8 +6,9 @@ metadata:
     pipelinesascode.tekton.dev/target-namespace: "%s"
     pipelinesascode.tekton.dev/on-target-branch: "[%s]"
     pipelinesascode.tekton.dev/on-event: "[%s]"
-    pipelinesascode.tekton.dev/task: "[pylint, .other-tasks/task-referenced-internally.yaml]"
+    pipelinesascode.tekton.dev/task: "[.other-tasks/task-referenced-internally.yaml]"
     pipelinesascode.tekton.dev/task-1: "[https://raw.githubusercontent.com/chmouel/scratchmyback/10c5ea559615c6783aa1a1aa9d93ea988b68dad7/.other-tasks/task-remote.yaml]"
+    pipelinesascode.tekton.dev/task-2: "pylint"
   name: pipelinerun-remote-annotations
 spec:
   pipelineRef: