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 `-<occurence number>` 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.

e3bfa5133f/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/<pull request number>.md` to be be used for the release notes instead of the title.

<!--start release-notes-assistant-->

## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Bug fixes
  - [PR](https://codeberg.org/forgejo/forgejo/pulls/8864): <!--number 8864 --><!--line 0 --><!--description ZGUtZHVwbGljYXRlIEZvcmdlam8gQWN0aW9ucyBqb2IgbmFtZXMgd2hlbiBuZWVkZWQ=-->de-duplicate Forgejo Actions job names when needed<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8864
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
This commit is contained in:
Earl Warren 2025-08-13 06:52:44 +02:00 committed by Earl Warren
commit c922ac5f38
5 changed files with 246 additions and 3 deletions

View file

@ -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
}

View file

@ -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))
}
})
}
}

View file

@ -372,7 +372,7 @@ func handleWorkflows(
continue continue
} }
jobs, err := jobparser.Parse(dwf.Content, false, jobparser.WithVars(vars)) jobs, err := jobParser(dwf.Content, jobparser.WithVars(vars))
if err != nil { if err != nil {
run.Status = actions_model.StatusFailure run.Status = actions_model.StatusFailure
log.Info("jobparser.Parse: invalid workflow, setting job status to failed: %v", err) log.Info("jobparser.Parse: invalid workflow, setting job status to failed: %v", err)

View file

@ -153,7 +153,7 @@ func CreateScheduleTask(ctx context.Context, cron *actions_model.ActionSchedule)
run.NotifyEmail = notifications run.NotifyEmail = notifications
// Parse the workflow specification from the cron schedule // 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 { if err != nil {
return err return err
} }

View file

@ -138,7 +138,7 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette
return nil, nil, err return nil, nil, err
} }
jobs, err := jobparser.Parse(content, false, jobparser.WithVars(vars)) jobs, err := jobParser(content, jobparser.WithVars(vars))
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }