From 39a7ebda75b7a9a033ba2a9640689e1bd7f9476b Mon Sep 17 00:00:00 2001 From: qwerty287 <80460567+qwerty287@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:37:37 +0100 Subject: [PATCH] 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 | 16 ++++++++++---- docs/docs/20-usage/25-workflows.md | 7 ++++-- docs/src/pages/migrations.md | 1 + pipeline/frontend/yaml/compiler/convert.go | 4 ++-- .../frontend/yaml/constraint/constraint.go | 20 +++++++++++------ .../yaml/constraint/constraint_test.go | 22 ++++++++++++------- pipeline/frontend/yaml/linter/linter.go | 18 ++++++++++++--- .../linter/schema/.woodpecker/test-when.yaml | 3 +++ .../frontend/yaml/linter/schema/schema.json | 18 +++++++++++++++ pipeline/frontend/yaml/parse_test.go | 6 ++--- pipeline/frontend/yaml/types/workflow.go | 3 ++- server/pipeline/stepbuilder/step_builder.go | 10 ++++++++- .../pipeline/stepbuilder/step_builder_test.go | 8 +++---- 13 files changed, 100 insertions(+), 36 deletions(-) diff --git a/docs/docs/20-usage/20-workflow-syntax.md b/docs/docs/20-usage/20-workflow-syntax.md index f97d87e4f..12a02200a 100644 --- a/docs/docs/20-usage/20-workflow-syntax.md +++ b/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 diff --git a/docs/docs/20-usage/25-workflows.md b/docs/docs/20-usage/25-workflows.md index ef09d485e..2483d07eb 100644 --- a/docs/docs/20-usage/25-workflows.md +++ b/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) diff --git a/docs/src/pages/migrations.md b/docs/src/pages/migrations.md index 34fe95477..cbfef9af1 100644 --- a/docs/src/pages/migrations.md +++ b/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 diff --git a/pipeline/frontend/yaml/compiler/convert.go b/pipeline/frontend/yaml/compiler/convert.go index 7a72c509c..3a0331860 100644 --- a/pipeline/frontend/yaml/compiler/convert.go +++ b/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 == "" { diff --git a/pipeline/frontend/yaml/constraint/constraint.go b/pipeline/frontend/yaml/constraint/constraint.go index 7f77193f0..8e07c7859 100644 --- a/pipeline/frontend/yaml/constraint/constraint.go +++ b/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. diff --git a/pipeline/frontend/yaml/constraint/constraint_test.go b/pipeline/frontend/yaml/constraint/constraint_test.go index 4812390c8..9c0003199 100644 --- a/pipeline/frontend/yaml/constraint/constraint_test.go +++ b/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) } } diff --git a/pipeline/frontend/yaml/linter/linter.go b/pipeline/frontend/yaml/linter/linter.go index 0a560dfd3..4951974da 100644 --- a/pipeline/frontend/yaml/linter/linter.go +++ b/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) { diff --git a/pipeline/frontend/yaml/linter/schema/.woodpecker/test-when.yaml b/pipeline/frontend/yaml/linter/schema/.woodpecker/test-when.yaml index 390420956..a9799a637 100644 --- a/pipeline/frontend/yaml/linter/schema/.woodpecker/test-when.yaml +++ b/pipeline/frontend/yaml/linter/schema/.woodpecker/test-when.yaml @@ -1,3 +1,6 @@ +when: + status: [success, failure] + steps: when-branch: image: alpine diff --git a/pipeline/frontend/yaml/linter/schema/schema.json b/pipeline/frontend/yaml/linter/schema/schema.json index f3cf824a5..c4903c846 100644 --- a/pipeline/frontend/yaml/linter/schema/schema.json +++ b/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" diff --git a/pipeline/frontend/yaml/parse_test.go b/pipeline/frontend/yaml/parse_test.go index 962d99bcb..4c4638972 100644 --- a/pipeline/frontend/yaml/parse_test.go +++ b/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)) } diff --git a/pipeline/frontend/yaml/types/workflow.go b/pipeline/frontend/yaml/types/workflow.go index 134aa47c6..c0c72ec20 100644 --- a/pipeline/frontend/yaml/types/workflow.go +++ b/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. diff --git a/server/pipeline/stepbuilder/step_builder.go b/server/pipeline/stepbuilder/step_builder.go index 42b0b3bc7..96b161d21 100644 --- a/server/pipeline/stepbuilder/step_builder.go +++ b/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) { diff --git a/server/pipeline/stepbuilder/step_builder_test.go b/server/pipeline/stepbuilder/step_builder_test.go index f468894dd..e8079bf48 100644 --- a/server/pipeline/stepbuilder/step_builder_test.go +++ b/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) {