From c922ac5f3809f5b1b6730e14bdb06290e1dbe61b Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 13 Aug 2025 06:52:44 +0200 Subject: [PATCH] fix: de-duplicate Forgejo Actions job names when needed (#8864) The status of two jobs by the same name shadow each other, they need to be distinct. If two jobs by the same name are found, they are made distinct by adding a `-` suffix. Resolves forgejo/forgejo#8648 For a given workflow, `jobparser.Parse` will generate one "single" (as opposed to a workflow that can be interpreted to generate multiple jobs) workflow for each job and then insert them (marshalled as yaml) in the database. https://codeberg.org/forgejo/forgejo/src/commit/e3bfa5133f40c5d4819cee402b772e0bb94affa9/models/actions/run.go#L237-L260 The name associated with this single workflow is what the runner will receive and it is what will be used to associate the job status with a commit. Resolves forgejo/forgejo#8648 ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. ### Documentation - [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [ ] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. ## Release notes - Bug fixes - [PR](https://codeberg.org/forgejo/forgejo/pulls/8864): de-duplicate Forgejo Actions job names when needed Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8864 Reviewed-by: Gusted Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- services/actions/job_parser.go | 31 ++++ services/actions/job_parser_test.go | 212 ++++++++++++++++++++++++++++ services/actions/notifier_helper.go | 2 +- services/actions/schedule_tasks.go | 2 +- services/actions/workflows.go | 2 +- 5 files changed, 246 insertions(+), 3 deletions(-) create mode 100644 services/actions/job_parser.go create mode 100644 services/actions/job_parser_test.go diff --git a/services/actions/job_parser.go b/services/actions/job_parser.go new file mode 100644 index 0000000000..294f1c4a0e --- /dev/null +++ b/services/actions/job_parser.go @@ -0,0 +1,31 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "fmt" + + "code.forgejo.org/forgejo/runner/v9/act/jobparser" +) + +func jobParser(workflow []byte, options ...jobparser.ParseOption) ([]*jobparser.SingleWorkflow, error) { + singleWorkflows, err := jobparser.Parse(workflow, false, options...) + if err != nil { + return nil, err + } + nameToSingleWorkflows := make(map[string][]*jobparser.SingleWorkflow, len(singleWorkflows)) + duplicates := make(map[string]int, len(singleWorkflows)) + for _, singleWorkflow := range singleWorkflows { + id, job := singleWorkflow.Job() + nameToSingleWorkflows[job.Name] = append(nameToSingleWorkflows[job.Name], singleWorkflow) + if len(nameToSingleWorkflows[job.Name]) > 1 { + duplicates[job.Name]++ + job.Name = fmt.Sprintf("%s-%d", job.Name, duplicates[job.Name]) + if err := singleWorkflow.SetJob(id, job); err != nil { + return nil, err + } + } + } + return singleWorkflows, nil +} diff --git a/services/actions/job_parser_test.go b/services/actions/job_parser_test.go new file mode 100644 index 0000000000..9c1361d74e --- /dev/null +++ b/services/actions/job_parser_test.go @@ -0,0 +1,212 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestServiceActions_jobParser(t *testing.T) { + for _, testCase := range []struct { + name string + workflow string + singleWorkflows []string + }{ + { + name: "OneJobNoDuplicate", + workflow: ` +jobs: + job1: + runs-on: docker + steps: + - run: echo OK +`, + singleWorkflows: []string{ + `jobs: + job1: + name: job1 + runs-on: docker + steps: + - run: echo OK +`, + }, + }, + { + name: "MatrixTwoJobsWithSameJobName", + workflow: ` +name: test +jobs: + job1: + name: shadowdefaultmatrixgeneratednames + strategy: + matrix: + version: [1.17, 1.19] + runs-on: docker + steps: + - run: echo OK +`, + singleWorkflows: []string{ + `name: test +jobs: + job1: + name: shadowdefaultmatrixgeneratednames + runs-on: docker + steps: + - run: echo OK + strategy: + matrix: + version: + - 1.17 +`, + `name: test +jobs: + job1: + name: shadowdefaultmatrixgeneratednames-1 + runs-on: docker + steps: + - run: echo OK + strategy: + matrix: + version: + - 1.19 +`, + }, + }, + { + name: "MatrixTwoJobsWithMatrixGeneratedNames", + workflow: ` +name: test +jobs: + job1: + strategy: + matrix: + version: [1.17, 1.19] + runs-on: docker + steps: + - run: echo OK +`, + singleWorkflows: []string{ + `name: test +jobs: + job1: + name: job1 (1.17) + runs-on: docker + steps: + - run: echo OK + strategy: + matrix: + version: + - 1.17 +`, + `name: test +jobs: + job1: + name: job1 (1.19) + runs-on: docker + steps: + - run: echo OK + strategy: + matrix: + version: + - 1.19 +`, + }, + }, + { + name: "MatrixTwoJobsWithDistinctInterpolatedNames", + workflow: ` +name: test +jobs: + job1: + name: myname-${{ matrix.version }} + strategy: + matrix: + version: [1.17, 1.19] + runs-on: docker + steps: + - run: echo OK +`, + singleWorkflows: []string{ + `name: test +jobs: + job1: + name: myname-1.17 + runs-on: docker + steps: + - run: echo OK + strategy: + matrix: + version: + - 1.17 +`, + `name: test +jobs: + job1: + name: myname-1.19 + runs-on: docker + steps: + - run: echo OK + strategy: + matrix: + version: + - 1.19 +`, + }, + }, + { + name: "MatrixTwoJobsWithIdenticalInterpolatedNames", + workflow: ` +name: test +jobs: + job1: + name: myname-${{ matrix.typo }} + strategy: + matrix: + version: [1.17, 1.19] + runs-on: docker + steps: + - run: echo OK +`, + singleWorkflows: []string{ + `name: test +jobs: + job1: + name: myname- + runs-on: docker + steps: + - run: echo OK + strategy: + matrix: + version: + - 1.17 +`, + `name: test +jobs: + job1: + name: myname--1 + runs-on: docker + steps: + - run: echo OK + strategy: + matrix: + version: + - 1.19 +`, + }, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + sw, err := jobParser([]byte(testCase.workflow)) + require.NoError(t, err) + for i, sw := range sw { + actual, err := sw.Marshal() + require.NoError(t, err) + assert.Equal(t, testCase.singleWorkflows[i], string(actual)) + } + }) + } +} diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 2b4273df31..e1df5776fb 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -372,7 +372,7 @@ func handleWorkflows( continue } - jobs, err := jobparser.Parse(dwf.Content, false, jobparser.WithVars(vars)) + jobs, err := jobParser(dwf.Content, jobparser.WithVars(vars)) if err != nil { run.Status = actions_model.StatusFailure log.Info("jobparser.Parse: invalid workflow, setting job status to failed: %v", err) diff --git a/services/actions/schedule_tasks.go b/services/actions/schedule_tasks.go index fb9c5094c5..ab8b330ba2 100644 --- a/services/actions/schedule_tasks.go +++ b/services/actions/schedule_tasks.go @@ -153,7 +153,7 @@ func CreateScheduleTask(ctx context.Context, cron *actions_model.ActionSchedule) run.NotifyEmail = notifications // Parse the workflow specification from the cron schedule - workflows, err := jobparser.Parse(cron.Content, false, jobparser.WithVars(vars)) + workflows, err := jobParser(cron.Content, jobparser.WithVars(vars)) if err != nil { return err } diff --git a/services/actions/workflows.go b/services/actions/workflows.go index b5875a7a50..1bc46bfadc 100644 --- a/services/actions/workflows.go +++ b/services/actions/workflows.go @@ -138,7 +138,7 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette return nil, nil, err } - jobs, err := jobparser.Parse(content, false, jobparser.WithVars(vars)) + jobs, err := jobParser(content, jobparser.WithVars(vars)) if err != nil { return nil, nil, err }