From a5ef372190409cc14ecfc9676bf9a1ea409bf8cf Mon Sep 17 00:00:00 2001
From: 6543 <6543@obermui.de>
Date: Thu, 17 Aug 2023 15:52:43 +0200
Subject: [PATCH] Move "skip ci" logic into global pipeline conditions (#2216)

... and make custom errors follow std err conventions

this fix a 500 response if the whole pipeline is filtered out
---
 .vscode/settings.json                         |  1 +
 Makefile                                      |  2 +-
 .../frontend/yaml/constraint/constraint.go    | 14 +++++++++++
 server/api/hook.go                            | 18 --------------
 server/pipeline/create.go                     |  2 +-
 server/pipeline/errors.go                     | 24 +++++++++++++++++++
 6 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/.vscode/settings.json b/.vscode/settings.json
index 4315da366..abf274e2b 100644
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -16,6 +16,7 @@
   "prettier.configPath": "./web/.prettierrc.js",
   "prettier.ignorePath": "./web/.prettierignore",
   "cSpell.words": [
+    "Curr",
     "doublestar",
     "multierr"
   ]
diff --git a/Makefile b/Makefile
index 685f23604..a9656a045 100644
--- a/Makefile
+++ b/Makefile
@@ -168,7 +168,7 @@ test-server-datastore-coverage: ## Test server datastore with coverage report
 
 test-ui: ui-dependencies ## Test UI code
 	(cd web/; pnpm run lint)
-	(cd web/; pnpm run formatcheck)
+	(cd web/; pnpm run format:check)
 	(cd web/; pnpm run typecheck)
 	(cd web/; pnpm run test)
 
diff --git a/pipeline/frontend/yaml/constraint/constraint.go b/pipeline/frontend/yaml/constraint/constraint.go
index 76a039a8e..80bdf96bf 100644
--- a/pipeline/frontend/yaml/constraint/constraint.go
+++ b/pipeline/frontend/yaml/constraint/constraint.go
@@ -18,10 +18,12 @@ import (
 	"errors"
 	"fmt"
 	"path"
+	"regexp"
 	"strings"
 
 	"github.com/antonmedv/expr"
 	"github.com/bmatcuk/doublestar/v4"
+	"github.com/rs/zerolog/log"
 	"golang.org/x/exp/maps"
 	"gopkg.in/yaml.v3"
 
@@ -29,6 +31,8 @@ import (
 	yaml_base_types "github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/types/base"
 )
 
+var skipRe = regexp.MustCompile(`\[(?i:ci *skip|skip *ci)\]`)
+
 type (
 	// When defines a set of runtime constraints.
 	When struct {
@@ -78,6 +82,16 @@ func (when *When) IsEmpty() bool {
 
 // Returns true if at least one of the internal constraints is true.
 func (when *When) Match(metadata metadata.Metadata, global bool, env map[string]string) (bool, error) {
+	if global {
+		// skip the whole workflow if any case-insensitive combination of the words "skip" and "ci"
+		// wrapped in square brackets appear in the commit message
+		skipMatch := skipRe.FindString(metadata.Curr.Commit.Message)
+		if len(skipMatch) > 0 {
+			log.Debug().Msgf("skip workflow as keyword to do so was detected in commit message '%s'", metadata.Curr.Commit.Message)
+			return false, nil
+		}
+	}
+
 	for _, c := range when.Constraints {
 		match, err := c.Match(metadata, global, env)
 		if err != nil {
diff --git a/server/api/hook.go b/server/api/hook.go
index c201c96ec..1f664e3af 100644
--- a/server/api/hook.go
+++ b/server/api/hook.go
@@ -22,7 +22,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"regexp"
 
 	"github.com/gin-gonic/gin"
 	"github.com/rs/zerolog/log"
@@ -35,8 +34,6 @@ import (
 	"github.com/woodpecker-ci/woodpecker/shared/token"
 )
 
-var skipRe = regexp.MustCompile(`\[(?i:ci *skip|skip *ci)\]`)
-
 // GetQueueInfo
 //
 //	@Summary	Get pipeline queue information
@@ -140,21 +137,6 @@ func PostHook(c *gin.Context) {
 		return
 	}
 
-	//
-	// Skip if commit message contains skip-ci
-	// TODO: move into global pipeline conditions logic
-	//
-
-	// skip the tmpPipeline if any case-insensitive combination of the words "skip" and "ci"
-	// wrapped in square brackets appear in the commit message
-	skipMatch := skipRe.FindString(tmpPipeline.Message)
-	if len(skipMatch) > 0 {
-		msg := fmt.Sprintf("ignoring hook: %s found in %s", skipMatch, tmpPipeline.Commit)
-		log.Debug().Msg(msg)
-		c.String(http.StatusNoContent, msg)
-		return
-	}
-
 	//
 	// 2. Get related repo from store and take repo renaming into account
 	//
diff --git a/server/pipeline/create.go b/server/pipeline/create.go
index a99f7669e..f4ac0864d 100644
--- a/server/pipeline/create.go
+++ b/server/pipeline/create.go
@@ -66,7 +66,7 @@ func Create(ctx context.Context, _store store.Store, repo *model.Repo, pipeline
 		filtered, parseErr = checkIfFiltered(repo, pipeline, forgeYamlConfigs)
 		if parseErr == nil {
 			if filtered {
-				err := ErrFiltered{Msg: "branch does not match restrictions defined in yaml"}
+				err := ErrFiltered{Msg: "global when filter of all workflows do skip this pipeline"}
 				log.Debug().Str("repo", repo.FullName).Msgf("%v", err)
 				return nil, err
 			}
diff --git a/server/pipeline/errors.go b/server/pipeline/errors.go
index fc403dff8..e9f2826f5 100644
--- a/server/pipeline/errors.go
+++ b/server/pipeline/errors.go
@@ -22,6 +22,14 @@ func (e ErrNotFound) Error() string {
 	return e.Msg
 }
 
+func (e ErrNotFound) Is(target error) bool {
+	_, ok := target.(ErrNotFound) //nolint:errorlint
+	if !ok {
+		_, ok = target.(*ErrNotFound) //nolint:errorlint
+	}
+	return ok
+}
+
 type ErrBadRequest struct {
 	Msg string
 }
@@ -30,6 +38,14 @@ func (e ErrBadRequest) Error() string {
 	return e.Msg
 }
 
+func (e ErrBadRequest) Is(target error) bool {
+	_, ok := target.(ErrBadRequest) //nolint:errorlint
+	if !ok {
+		_, ok = target.(*ErrBadRequest) //nolint:errorlint
+	}
+	return ok
+}
+
 type ErrFiltered struct {
 	Msg string
 }
@@ -37,3 +53,11 @@ type ErrFiltered struct {
 func (e ErrFiltered) Error() string {
 	return "ignoring hook: " + e.Msg
 }
+
+func (e *ErrFiltered) Is(target error) bool {
+	_, ok := target.(ErrFiltered) //nolint:errorlint
+	if !ok {
+		_, ok = target.(*ErrFiltered) //nolint:errorlint
+	}
+	return ok
+}