Warn user when on cel-expression is used along side on-target branch and on-event

This patch warns users in both the `user events namespaces` and `controller logs`
when using `on-event` and `on-target-branch` annotations along side
`on-cel-expression`

Hence warning users would avoid further confusion

Fixes: openshift-pipelines#1974

Signed-off-by: PuneetPunamiya <[email protected]>

Author: PuneetPunamiya

pkg/matcher/annotation_matcher.go

@@ -10,7 +10,9 @@ import (
 	"github.com/google/cel-go/common/types"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
 	apipac "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/events"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
@@ -149,7 +151,7 @@ func getName(prun *tektonv1.PipelineRun) string {
 	return name
 }
 
-func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger, pruns []*tektonv1.PipelineRun, cs *params.Run, event *info.Event, vcx provider.Interface) ([]Match, error) {
+func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger, pruns []*tektonv1.PipelineRun, cs *params.Run, event *info.Event, vcx provider.Interface, eventEmitter *events.EventEmitter, repo *v1alpha1.Repository) ([]Match, error) {
 	matchedPRs := []Match{}
 	infomsg := fmt.Sprintf("matching pipelineruns to event: URL=%s, target-branch=%s, source-branch=%s, target-event=%s",
 		event.URL,
@@ -226,6 +228,23 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger
 		if event.EventType == opscomments.NoOpsCommentEventType.String() || event.EventType == opscomments.OnCommentEventType.String() {
 			continue
 		}
+
+		if prun.GetObjectMeta().GetAnnotations()[keys.OnCelExpression] != "" {
+			if prun.GetObjectMeta().GetAnnotations()[keys.OnEvent] != "" && prun.GetObjectMeta().GetAnnotations()[keys.OnTargetBranch] != "" {
+				msg := fmt.Sprintf("Warning: The PipelineRun '%s' has both `on-cel-expression` and `on-event`/`on-target-branch` annotations. The `on-cel-expression` will take precedence, and the other annotations will be ignored.", prun.Name)
+				logger.Warnf(msg)
+				eventEmitter.EmitMessage(repo, zap.WarnLevel, "RepositoryCannotLocatePipelineRun", msg)
+			} else if prun.GetObjectMeta().GetAnnotations()[keys.OnEvent] != "" {
+				msg := fmt.Sprintf("Warning: The PipelineRun '%s' has both `on-cel-expression` and `on-event` annotations. The `on-cel-expression` will take precedence, and the `on-event` annotation will be ignored.", prun.Name)
+				logger.Warnf(msg)
+				eventEmitter.EmitMessage(repo, zap.WarnLevel, "RespositoryTakesOnCelExpressionPrecedence", msg)
+			} else if prun.GetObjectMeta().GetAnnotations()[keys.OnTargetBranch] != "" {
+				msg := fmt.Sprintf("Warning: The PipelineRun '%s' has both `on-cel-expression` and `on-target-branch` annotations. The `on-cel-expression` will take precedence, and the `on-target-branch` annotation will be ignored.", prun.Name)
+				logger.Warnf(msg)
+				eventEmitter.EmitMessage(repo, zap.WarnLevel, "RespositoryTakesOnCelExpressionPrecedence", msg)
+			}
+		}
+
 		if celExpr, ok := prun.GetObjectMeta().GetAnnotations()[keys.OnCelExpression]; ok {
 			out, err := celEvaluate(ctx, celExpr, event, vcx)
 			if err != nil {

pkg/matcher/annotation_matcher_test.go

@@ -1416,7 +1416,7 @@ func runTest(ctx context.Context, t *testing.T, tt annotationTest, vcx provider.
 
 	matches, err := MatchPipelinerunByAnnotation(ctx, logger,
 		tt.args.pruns,
-		client, &tt.args.runevent, vcx,
+		client, &tt.args.runevent, vcx, nil, nil,
 	)
 
 	if tt.wantLog != "" {
@@ -1883,7 +1883,7 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) {
 				Clients: clients.Clients{},
 				Info:    info.Info{},
 			}
-			matches, err := MatchPipelinerunByAnnotation(ctx, logger, tt.args.pruns, cs, &tt.args.runevent, &ghprovider.Provider{})
+			matches, err := MatchPipelinerunByAnnotation(ctx, logger, tt.args.pruns, cs, &tt.args.runevent, &ghprovider.Provider{}, nil, nil)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("MatchPipelinerunByAnnotation() error = %v, wantErr %v", err, tt.wantErr)
 				return

pkg/pipelineascode/match.go

@@ -217,7 +217,7 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
 			return nil, err
 		}
 		// Don't fail or do anything if we don't have a match yet, we will do it properly later in this function
-		_, _ = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, rtypes.PipelineRuns, p.run, p.event, p.vcx)
+		_, _ = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, rtypes.PipelineRuns, p.run, p.event, p.vcx, p.eventEmitter, repo)
 	}
 	// Replace those {{var}} placeholders user has in her template to the run.Info variable
 	allTemplates := p.makeTemplate(ctx, repo, rawTemplates)
@@ -248,7 +248,7 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
 	// Match the PipelineRun with annotation
 	var matchedPRs []matcher.Match
 	if p.event.TargetTestPipelineRun == "" {
-		if matchedPRs, err = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx); err != nil {
+		if matchedPRs, err = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx, p.eventEmitter, repo); err != nil {
 			// Don't fail when you don't have a match between pipeline and annotations
 			p.eventEmitter.EmitMessage(nil, zap.WarnLevel, "RepositoryNoMatch", err.Error())
 			// In a scenario where an external user submits a pull request and the repository owner uses the
@@ -336,7 +336,7 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
 		}}, nil
 	}
 
-	matchedPRs, err = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx)
+	matchedPRs, err = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx, p.eventEmitter, repo)
 	if err != nil {
 		// Don't fail when you don't have a match between pipeline and annotations
 		p.eventEmitter.EmitMessage(nil, zap.WarnLevel, "RepositoryNoMatch", err.Error())