mirror of
https://github.com/go-gitea/gitea.git
synced 2026-05-06 04:01:05 +09:00
f94b476c45
## Problem
Workflow-level concurrency groups were evaluated — and jobs were parsed
— before the run was persisted, so `run.ID` was `0` and `github.run_id`
in the expression context resolved to an empty string. Expressions like:
```yaml
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true
```
collapsed to `<workflow>-` on every push event (`head_ref` is empty on
push), so `cancel-in-progress` cancelled in-progress runs across
**unrelated branches**, not just the current one.
Reproduced on a 1.26 instance:
- push to `master` → `ci` run starts
- push to `feature-branch` → the `master` run gets cancelled
GitHub Actions' documented semantic: on push events `github.run_id` is
unique per run, so the group is unique → no cancellation; on PR events
`github.head_ref` is the source branch → cancellation is per-PR.
## Fix
Insert the run **before** parsing jobs or evaluating workflow-level
concurrency, so `run.ID` is populated in time for every expression that
reads `github.run_id` — not just the concurrency group, but also
`run-name`, job names, and `runs-on`.
`jobparser.Parse` now runs inside the `InsertRun` transaction, after
`db.Insert(ctx, run)`. Workflow-level concurrency evaluation runs next
and only mutates `run` in memory. All concurrency-derived fields
(`raw_concurrency`, `concurrency_group`, `concurrency_cancel`) plus
`status` and `title` are persisted in a single final `UpdateRun` at
end-of-transaction — one `INSERT` + one `UPDATE` per run in both the
concurrency and non-concurrency paths (matches pre-branch parity, one
fewer `UpdateRepoRunsNumbers` `COUNT` than the interim state).
`GenerateGiteaContext` now sets `run_id` from `run.ID` unconditionally;
every caller passes a persisted run.
**Verification**: tested end-to-end on a 1.26 deployment. Before the
patch, two successive `ci` pushes (one to master, one to a feature
branch) cross-cancelled each other. After the patch, the same pushes —
in both orders (master→branch, branch→master) — run to completion
simultaneously across 15+ runs with zero cancellations.
**Regression tests** in `services/actions/context_test.go`:
- `TestEvaluateRunConcurrency_RunIDFallback` — unit check that
`EvaluateRunConcurrencyFillModel` resolves `github.run_id` from
`run.ID`.
- `TestPrepareRunAndInsert_ExpressionsSeeRunID` — full-flow check: calls
`PrepareRunAndInsert` with `${{ github.run_id }}` in both `run-name` and
the concurrency group, then asserts the persisted `Title`,
`ConcurrencyGroup`, and `RawConcurrency` contain / survive the run's ID.
Re-ordering `db.Insert` relative to either parse or concurrency eval
fails this test.
## Relation to #37119
[#37119](https://github.com/go-gitea/gitea/pull/37119) also moves
concurrency evaluation into `InsertRun` but keeps it **before**
`db.Insert`, then tries to populate `run_id` only when `run.ID > 0` —
which is still `0` at that call site, so the cross-branch leak would
survive that PR as written. This PR fixes the ordering so that `run.ID`
is actually populated at eval time, and broadens it to cover parse-time
expression interpolation too.
Co-authored-by: Claude (Opus 4.7) <noreply@anthropic.com>
101 lines
3.3 KiB
Go
101 lines
3.3 KiB
Go
// Copyright 2024 The Gitea Authors. All rights reserved.
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
package actions
|
|
|
|
import (
|
|
"strconv"
|
|
"testing"
|
|
|
|
actions_model "code.gitea.io/gitea/models/actions"
|
|
"code.gitea.io/gitea/models/unittest"
|
|
|
|
act_model "github.com/nektos/act/pkg/model"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
func TestEvaluateRunConcurrency_RunIDFallback(t *testing.T) {
|
|
// Unit-level check that EvaluateRunConcurrencyFillModel resolves
|
|
// github.run_id from run.ID. The full-flow regression — that run.ID is
|
|
// non-zero by the time evaluation happens — is in
|
|
// TestPrepareRunAndInsert_ExpressionsSeeRunID.
|
|
assert.NoError(t, unittest.PrepareTestDatabase())
|
|
ctx := t.Context()
|
|
|
|
runA := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: 791})
|
|
runB := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: 792})
|
|
|
|
expr := &act_model.RawConcurrency{
|
|
Group: "${{ github.workflow }}-${{ github.head_ref || github.run_id }}",
|
|
CancelInProgress: "true",
|
|
}
|
|
|
|
assert.NoError(t, EvaluateRunConcurrencyFillModel(ctx, runA, expr, nil, nil))
|
|
assert.NoError(t, EvaluateRunConcurrencyFillModel(ctx, runB, expr, nil, nil))
|
|
|
|
assert.Contains(t, runA.ConcurrencyGroup, "791")
|
|
assert.Contains(t, runB.ConcurrencyGroup, "792")
|
|
assert.NotEqual(t, runA.ConcurrencyGroup, runB.ConcurrencyGroup)
|
|
}
|
|
|
|
func TestPrepareRunAndInsert_ExpressionsSeeRunID(t *testing.T) {
|
|
// Regression for the cross-branch concurrency leak: github.run_id must
|
|
// be available during BOTH jobparser.Parse (run-name) and workflow-level
|
|
// concurrency evaluation. Re-ordering db.Insert relative to either step
|
|
// would leave run.ID at 0 and break this test.
|
|
assert.NoError(t, unittest.PrepareTestDatabase())
|
|
ctx := t.Context()
|
|
|
|
content := []byte(`name: cross-branch
|
|
run-name: "Run ${{ github.run_id }}"
|
|
on: push
|
|
concurrency:
|
|
group: group-${{ github.run_id }}
|
|
cancel-in-progress: true
|
|
jobs:
|
|
hello:
|
|
runs-on: ubuntu-latest
|
|
steps:
|
|
- run: echo hi
|
|
`)
|
|
|
|
run := &actions_model.ActionRun{
|
|
Title: "before parse",
|
|
RepoID: 4,
|
|
OwnerID: 1,
|
|
WorkflowID: "expr-runid.yaml",
|
|
TriggerUserID: 1,
|
|
Ref: "refs/heads/master",
|
|
CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0",
|
|
Event: "push",
|
|
TriggerEvent: "push",
|
|
EventPayload: "{}",
|
|
}
|
|
require.NoError(t, PrepareRunAndInsert(ctx, content, run, nil))
|
|
require.Positive(t, run.ID)
|
|
|
|
persisted := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: run.ID})
|
|
runIDStr := strconv.FormatInt(run.ID, 10)
|
|
assert.Equal(t, "Run "+runIDStr, persisted.Title)
|
|
assert.Equal(t, "group-"+runIDStr, persisted.ConcurrencyGroup)
|
|
// Rerun reads raw_concurrency from the DB to re-evaluate the group;
|
|
// see services/actions/rerun.go. Must survive the insert.
|
|
assert.NotEmpty(t, persisted.RawConcurrency)
|
|
}
|
|
|
|
func TestFindTaskNeeds(t *testing.T) {
|
|
assert.NoError(t, unittest.PrepareTestDatabase())
|
|
|
|
task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 51})
|
|
job := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: task.JobID})
|
|
|
|
ret, err := FindTaskNeeds(t.Context(), job)
|
|
assert.NoError(t, err)
|
|
assert.Len(t, ret, 1)
|
|
assert.Contains(t, ret, "job1")
|
|
assert.Len(t, ret["job1"].Outputs, 2)
|
|
assert.Equal(t, "abc", ret["job1"].Outputs["output_a"])
|
|
assert.Equal(t, "bbb", ret["job1"].Outputs["output_b"])
|
|
}
|