From 6bc1803c70280b09af3e62bfc4000755faa36b42 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Mon, 11 Aug 2025 18:56:12 +0200 Subject: [PATCH] fix: de-duplicate Forgejo Actions job names when needed 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 --- 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 }