Independently evaluate status filter and support on workflows (#6183)

fix https://github.com/woodpecker-ci/woodpecker/issues/6161

And also allow `when.status` at workflow level. `runs_on` is deprecated then.


Co-authored-by: 6543 <6543@obermui.de>

docs/docs/20-usage/20-workflow-syntax.md

@@ -350,6 +350,18 @@ There are use cases for executing steps on failure, such as sending notification
 +      - status: [ success, failure ]
 ```
 
+The filter is aware of the other filters. If you want to run on failures if the event is `tag`, but if it's a `pull_request`, run it on both success and failure:
+
+```diff
+ when:
++  - event: tag
++    status: [ failure ]
++  - event: pull_request
++    status: [ success, failure ]
+```
+
+If there's no matching filter at all or all matching filters don't have set `status`, it will use the default, which means it runs on success only. In the example above this will happen if the event is neither `tag` nor `pull_request`.
+
 #### `platform`
 
 :::note
@@ -761,10 +773,6 @@ The workflow now triggers on `main`, but also if the target branch of a pull req
 
 Woodpecker supports to define multiple workflows for a repository. Those workflows will run independent from each other. To depend them on each other you can use the [`depends_on`](./25-workflows.md#flow-control) keyword.
 
-## `runs_on`
-
-Workflows that should run even on failure should set the `runs_on` tag. See [here](./25-workflows.md#flow-control) for an example.
-
 ## Advanced network options for steps
 
 :::warning

docs/docs/20-usage/25-workflows.md

@@ -97,7 +97,7 @@ The name for a `depends_on` entry is the filename without the path, leading dots
 +  - test
 ```
 
-Workflows that need to run even on failures should set the `runs_on` tag.
+Workflows that need to run even on failures should set the `status` filter.
 
 ```diff
  steps:
@@ -109,9 +109,12 @@ Workflows that need to run even on failures should set the `runs_on` tag.
  depends_on:
    - deploy
 
-+runs_on: [ success, failure ]
++when:
++  - status: [ success, failure ]
 ```
 
+This works just like the [`status` filter for steps](./20-workflow-syntax.md#status).
+
 :::info
 Some workflows don't need the source code, like creating a notification on failure.
 Read more about `skip_clone` at [pipeline syntax](./20-workflow-syntax.md#skip_clone)

docs/src/pages/migrations.md

@@ -8,6 +8,7 @@ To enhance the usability of Woodpecker and meet evolving security standards, occ
 
 - (Kubernetes) Deprecated `step` label on pod in favor of new namespaced label `woodpecker-ci.org/step`. The `step` label will be removed in a future update.
 - deprecated `CI_COMMIT_AUTHOR_AVATAR` and `CI_PREV_COMMIT_AUTHOR_AVATAR` env vars in favor of `CI_PIPELINE_AVATAR` and `CI_PREV_PIPELINE_AVATAR`
+- deprecated `runs_on` workflow property in favor of `when.status`.
 
 ### Admin-facing migrations
 

pipeline/frontend/yaml/compiler/convert.go

@@ -147,9 +147,9 @@ func (c *Compiler) createProcess(container *yaml_types.Container, workflow *yaml
 	}
 
 	// at least one constraint contain status success, or all constraints have no status set
-	onSuccess := container.When.IncludesStatusSuccess()
+	onSuccess := container.When.IncludesStatusSuccess(c.metadata, false, c.env)
 	// at least one constraint must include the status failure.
-	onFailure := container.When.IncludesStatusFailure()
+	onFailure := container.When.IncludesStatusFailure(c.metadata, false, c.env)
 
 	failure := container.Failure
 	if container.Failure == "" {

pipeline/frontend/yaml/constraint/constraint.go

@@ -80,17 +80,19 @@ func (when *When) Match(metadata metadata.Metadata, global bool, env map[string]
 	return false, nil
 }
 
-func (when *When) IncludesStatusFailure() bool {
+func (when *When) IncludesStatusFailure(metadata metadata.Metadata, global bool, env map[string]string) bool {
 	for _, c := range when.Constraints {
-		if slices.Contains(c.Status, statusFailure) {
-			return true
+		if matches, err := c.Match(metadata, global, env); err == nil && matches {
+			if slices.Contains(c.Status, statusFailure) {
+				return true
+			}
 		}
 	}
 
 	return false
 }
 
-func (when *When) IncludesStatusSuccess() bool {
+func (when *When) IncludesStatusSuccess(metadata metadata.Metadata, global bool, env map[string]string) bool {
 	// "success" acts differently than "failure" in that it's
 	// presumed to be included unless it's specifically not part
 	// of the list
@@ -98,11 +100,15 @@ func (when *When) IncludesStatusSuccess() bool {
 		return true
 	}
 	for _, c := range when.Constraints {
-		if len(c.Status) == 0 || slices.Contains(c.Status, statusSuccess) {
-			return true
+		matches, err := c.Match(metadata, global, env)
+		fmt.Println("mat", matches, err, c.Status)
+		if matches, err := c.Match(metadata, global, env); err == nil && matches {
+			if len(c.Status) > 0 && !slices.Contains(c.Status, statusSuccess) {
+				return false
+			}
 		}
 	}
-	return false
+	return true
 }
 
 // False if (any) non local.

pipeline/frontend/yaml/constraint/constraint_test.go

@@ -23,19 +23,25 @@ import (
 	"go.woodpecker-ci.org/woodpecker/v3/pipeline/frontend/metadata"
 )
 
-func TestConstraintStatusSuccess(t *testing.T) {
+func TestConstraintStatusSuccessFailure(t *testing.T) {
 	testdata := []struct {
-		conf string
-		want bool
+		conf        string
+		wantSuccess bool
+		wantFail    bool
 	}{
-		{conf: "", want: true},
-		{conf: "{status: [failure]}", want: false},
-		{conf: "{status: [success]}", want: true},
-		{conf: "{status: [failure, success]}", want: true},
+		{conf: "", wantSuccess: true, wantFail: false},
+		{conf: "{status: [failure]}", wantSuccess: false, wantFail: true},
+		{conf: "{status: [success]}", wantSuccess: true, wantFail: false},
+		{conf: "{status: [failure, success]}", wantSuccess: true, wantFail: true},
+		{conf: "{event: push, status: [failure, success]}", wantSuccess: true, wantFail: false},
+		{conf: "{event: pull_request, status: [failure, success]}", wantSuccess: true, wantFail: true},
+		{conf: "{event: push, status: [failure]}", wantSuccess: true, wantFail: false},
+		{conf: "{event: pull_request, status: [failure]}", wantSuccess: false, wantFail: true},
 	}
 	for _, test := range testdata {
 		c := parseConstraints(t, test.conf)
-		assert.Equal(t, test.want, c.IncludesStatusSuccess(), "when: '%s'", test.conf)
+		assert.Equal(t, test.wantSuccess, c.IncludesStatusSuccess(metadata.Metadata{Curr: metadata.Pipeline{Event: metadata.EventPull}}, true, map[string]string{}), "when: '%s'", test.conf)
+		assert.Equal(t, test.wantFail, c.IncludesStatusFailure(metadata.Metadata{Curr: metadata.Pipeline{Event: metadata.EventPull}}, true, map[string]string{}), "when: '%s'", test.conf)
 	}
 }
 

pipeline/frontend/yaml/linter/linter.go

@@ -301,14 +301,26 @@ func (l *Linter) lintSchema(config *WorkflowConfig) error {
 	return linterErr
 }
 
-func (l *Linter) lintDeprecations(config *WorkflowConfig) (err error) {
+func (l *Linter) lintDeprecations(config *WorkflowConfig) error {
 	parsed := new(types.Workflow)
-	err = xyaml.Unmarshal([]byte(config.RawConfig), parsed)
+	err := xyaml.Unmarshal([]byte(config.RawConfig), parsed)
 	if err != nil {
 		return err
 	}
 
-	return nil
+	if len(parsed.RunsOn) > 0 { //nolint:staticcheck
+		err = multierr.Append(err, &pipeline_errors.PipelineError{
+			Type:    pipeline_errors.PipelineErrorTypeDeprecation,
+			Message: "Usage of `runs_on` is deprecated, use `when.status`",
+			Data: pipeline_errors.DeprecationErrorData{
+				File:  config.File,
+				Field: fmt.Sprintf("%s.runs_on", config.File),
+				Docs:  "https://woodpecker-ci.org/docs/usage/workflow-syntax#status",
+			},
+		})
+	}
+
+	return err
 }
 
 func (l *Linter) lintBadHabits(config *WorkflowConfig) (err error) {

pipeline/frontend/yaml/linter/schema/.woodpecker/test-when.yaml

@@ -1,3 +1,6 @@
+when:
+  status: [success, failure]
+
 steps:
   when-branch:
     image: alpine

pipeline/frontend/yaml/linter/schema/schema.json

@@ -47,6 +47,7 @@
     },
     "runs_on": {
       "type": "array",
+      "description": "Deprecated: use `when.status` instead. Read more: https://woodpecker-ci.org/docs/usage/workflows#flow-control",
       "minLength": 1,
       "items": {
         "type": "string"
@@ -187,6 +188,23 @@
           "description": "filter cron by title. Read more: https://woodpecker-ci.org/docs/usage/workflow-syntax#cron",
           "$ref": "#/definitions/constraint_list"
         },
+        "status": {
+          "description": "Read more: https://woodpecker-ci.org/docs/usage/workflows#flow-control",
+          "oneOf": [
+            {
+              "type": "array",
+              "minLength": 1,
+              "items": {
+                "type": "string",
+                "enum": ["success", "failure"]
+              }
+            },
+            {
+              "type": "string",
+              "enum": ["success", "failure"]
+            }
+          ]
+        },
         "platform": {
           "description": "Execute a step only on a specific platform. Read more: https://woodpecker-ci.org/docs/usage/workflow-syntax#platform",
           "$ref": "#/definitions/constraint_list"

pipeline/frontend/yaml/parse_test.go

@@ -48,8 +48,8 @@ func TestParse(t *testing.T) {
 		assert.Equal(t, "build", out.Labels["com.example.type"])
 		assert.Equal(t, "lint", out.DependsOn[0])
 		assert.Equal(t, "test", out.DependsOn[1])
-		assert.Equal(t, ("success"), out.RunsOn[0])
-		assert.Equal(t, ("failure"), out.RunsOn[1])
+		assert.Equal(t, ("success"), out.RunsOn[0]) //nolint:staticcheck
+		assert.Equal(t, ("failure"), out.RunsOn[1]) //nolint:staticcheck
 		assert.False(t, out.SkipClone)
 	})
 
@@ -322,10 +322,10 @@ labels:
 depends_on:
     - lint
     - test
+skip_clone: false
 runs_on:
     - success
     - failure
-skip_clone: false
 `, string(workBin2))
 }
 

pipeline/frontend/yaml/types/workflow.go

@@ -28,8 +28,9 @@ type (
 		Services  ContainerList     `yaml:"services,omitempty"`
 		Labels    map[string]string `yaml:"labels,omitempty"`
 		DependsOn []string          `yaml:"depends_on,omitempty"`
-		RunsOn    []string          `yaml:"runs_on,omitempty"`
 		SkipClone bool              `yaml:"skip_clone"`
+		// Deprecated: use when.status. TODO remove in next major.
+		RunsOn []string `yaml:"runs_on,omitempty"`
 	}
 
 	// Workspace defines a pipeline workspace.

server/pipeline/stepbuilder/step_builder.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"maps"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -181,7 +182,7 @@ func (b *StepBuilder) genItemForWorkflow(workflow *model.Workflow, axis matrix.A
 		Config:    ir,
 		Labels:    parsed.Labels,
 		DependsOn: parsed.DependsOn,
-		RunsOn:    parsed.RunsOn,
+		RunsOn:    parsed.RunsOn, //nolint:staticcheck // TODO: remove in next major.
 	}
 	if len(item.Labels) == 0 {
 		item.Labels = make(map[string]string, len(b.DefaultLabels))
@@ -189,6 +190,13 @@ func (b *StepBuilder) genItemForWorkflow(workflow *model.Workflow, axis matrix.A
 		maps.Copy(item.Labels, b.DefaultLabels)
 	}
 
+	if !slices.Contains(item.RunsOn, "failure") && parsed.When.IncludesStatusFailure(workflowMetadata, true, environ) {
+		item.RunsOn = append(item.RunsOn, "failure")
+	}
+	if !slices.Contains(item.RunsOn, "success") && parsed.When.IncludesStatusFailure(workflowMetadata, true, environ) {
+		item.RunsOn = append(item.RunsOn, "success")
+	}
+
 	// "woodpecker-ci.org" namespace is reserved for internal use
 	for key := range item.Labels {
 		if strings.HasPrefix(key, pipeline.InternalLabelPrefix) {

server/pipeline/stepbuilder/step_builder_test.go

@@ -245,13 +245,11 @@ func TestRunsOn(t *testing.T) {
 			{Data: []byte(`
 when:
   event: push
+  status: [ success, failure ]
+
 steps:
   - name: deploy
     image: scratch
-
-runs_on:
-  - success
-  - failure
 `)},
 		},
 	}
@@ -259,7 +257,7 @@ runs_on:
 	items, err := b.Build()
 	assert.NoError(t, err)
 	assert.Len(t, items[0].RunsOn, 2, "Should run on success and failure")
-	assert.Equal(t, "failure", items[0].RunsOn[1], "Should run on failure")
+	assert.ElementsMatchf(t, []string{"success", "failure"}, items[0].RunsOn, "Should run on failure")
 }
 
 func TestPipelineName(t *testing.T) {