ynkmr/gitea
1
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2026-10-23 19:43:59 +09:00
Code Issues Projects Releases Wiki Activity

fix(actions): bound requested artifact retention (#39894) (#39946)

Browse Source 
`actions/upload-artifact` v4 and later send an absolute `expires_at`
computed with the runner's clock, which Gitea stored unchecked. A runner
clock running behind made artifacts expire at the next cleanup, and
workflows could keep artifacts longer than `ARTIFACT_RETENTION_DAYS`.

- Requested expiries are bounded by Gitea's clock, at least 1 day and at
most `ARTIFACT_RETENTION_DAYS` when it is not `0`. Longer
`retention-days` requests now get the instance limit.
- `github.retention_days` now carries `ARTIFACT_RETENTION_DAYS`, so
upload actions cap `retention-days` themselves and log a warning, like
on GitHub.

Co-authored-by: bircni <bircni@icloud.com>
This commit is contained in:
silverwind
2026-10-22 23:21:37 +00:00
committed by GitHub
co-authored by bircni
parent 9141a6386f
commit 9d1d5ceddb
6 changed files with 45 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
custom/conf/app.example.ini
+1 -1
View File
@@ -3083,7 +3083,7 @@ LEVEL = Info
;LOG_RETENTION_DAYS = 365
;; Days to keep artifacts. Old artifacts will be deleted after this period. 0 means keep forever.
;; Changes only apply to newly uploaded artifacts, existing ones keep the expiry stored when they were uploaded.
;; Artifacts could have their own retention periods by setting the `retention-days` option in `actions/upload-artifact` step.
;; Workflows can request a shorter retention period, down to 1 day.
;ARTIFACT_RETENTION_DAYS = 90
;; Max artifact size in bytes that can be browsed or previewed in the web UI. Set to 0 to disable artifact previews, or -1 for no limit.
;ARTIFACT_PREVIEW_MAX_SIZE = 10485760
models/actions/artifact.go
+6 -1
View File
@@ -93,7 +93,12 @@ const artifactKeepForever timeutil.TimeStamp = 0
func artifactExpiry(requested optional.Option[timeutil.TimeStamp]) timeutil.TimeStamp {
if requested.Has() {
return max(requested.Value(), artifactKeepForever+1)
now := timeutil.TimeStampNow()
expiry := max(requested.Value(), now.Add(timeutil.Day)) // v4 clients use the runner's clock
if setting.Actions.ArtifactRetentionDays > 0 {
expiry = min(expiry, now.Add(timeutil.Day*setting.Actions.ArtifactRetentionDays))
}
return expiry
}
if setting.Actions.ArtifactRetentionDays <= 0 {
return artifactKeepForever
models/actions/artifact_test.go
+24
View File
@@ -5,6 +5,12 @@ package actions
import (
"testing"
"time"
"gitea.dev/modules/optional"
"gitea.dev/modules/setting"
"gitea.dev/modules/test"
"gitea.dev/modules/timeutil"
"github.com/stretchr/testify/assert"
)
@@ -25,3 +31,21 @@ func TestKeepLatestAttemptArtifacts(t *testing.T) {
}
assert.Equal(t, []int64{1, 4, 5}, ids)
}
func TestArtifactExpiry(t *testing.T) {
defer timeutil.MockSet(time.Unix(1_000_000_000, 0))()
now := timeutil.TimeStampNow()
days := func(n int64) timeutil.TimeStamp { return now.Add(n * timeutil.Day) }
requested := optional.Some[timeutil.TimeStamp]
defer test.MockVariableValue(&setting.Actions.ArtifactRetentionDays, 90)()
assert.Equal(t, days(90), artifactExpiry(optional.None[timeutil.TimeStamp]()))
assert.Equal(t, days(5), artifactExpiry(requested(days(5))))
assert.Equal(t, days(1), artifactExpiry(requested(days(-2))), "runner clock behind must not expire the artifact at once")
assert.Equal(t, days(90), artifactExpiry(requested(days(1000))), "runner must not exceed the instance retention")
setting.Actions.ArtifactRetentionDays = 0
assert.Equal(t, artifactKeepForever, artifactExpiry(optional.None[timeutil.TimeStamp]()))
assert.Equal(t, days(1000), artifactExpiry(requested(days(1000))))
assert.Equal(t, days(1), artifactExpiry(requested(now)))
}
services/actions/cleanup_test.go
+2 -1
View File
@@ -123,7 +123,7 @@ func TestCleanupRetentionZeroKeepsForever(t *testing.T) {
require.NoError(t, err)
assert.Zero(t, art.ExpiredUnix)
// a workflow-requested expiry is honored, only the instance default may mean never
// a workflow-requested expiry is bounded, only the instance default may mean never
asked, err := actions_model.CreateArtifact(t.Context(), task, "client-asked", "b.txt", optional.Some(timeutil.TimeStampNow()))
require.NoError(t, err)
assert.Positive(t, asked.ExpiredUnix)
@@ -141,6 +141,7 @@ func TestCleanupRetentionZeroKeepsForever(t *testing.T) {
_, err = db.GetEngine(t.Context()).In("id", art.ID, past.ID).Cols("status").
Update(&actions_model.ActionArtifact{Status: actions_model.ArtifactStatusUploadConfirmed})
require.NoError(t, err)
require.NoError(t, actions_model.UpdateArtifact(t.Context(), &actions_model.ActionArtifact{ID: past.ID, ExpiredUnix: 1}, "expired_unix"))
expiring, err := actions_model.ListNeedExpiredArtifacts(t.Context())
require.NoError(t, err)
ids := container.FilterSlice(expiring, func(a *actions_model.ActionArtifact) (int64, bool) { return a.ID, true })
services/actions/context.go
+6 -1
View File
@@ -64,6 +64,11 @@ func GenerateGiteaContext(ctx context.Context, run *actions_model.ActionRun, att
refProtected = false
}
retentionDays := "" // not "0", v3 artifact uploads would read it as a 0-day limit
if setting.Actions.ArtifactRetentionDays > 0 {
retentionDays = strconv.FormatInt(setting.Actions.ArtifactRetentionDays, 10)
}
gitContext := GiteaContext{
// standard contexts, see https://docs.github.com/en/actions/learn-github-actions/contexts#github-context
"action": "", // string, The name of the action currently running, or the id of a step. GitHub removes special characters, and uses the name __run when the current step runs a script without an id. If you use the same action more than once in the same job, the name will include a suffix with the sequence number with underscore before it. For example, the first script you run will have the name __run, and the second script will be named __run_2. Similarly, the second invocation of actions/checkout will be actionscheckout2.
@@ -89,7 +94,7 @@ func GenerateGiteaContext(ctx context.Context, run *actions_model.ActionRun, att
"repository": run.Repo.OwnerName + "/" + run.Repo.Name, // string, The owner and repository name. For example, Codertocat/Hello-World.
"repository_owner": run.Repo.OwnerName, // string, The repository owner's name. For example, Codertocat.
"repositoryUrl": run.Repo.HTMLURL(), // string, The Git URL to the repository. For example, git://github.com/codertocat/hello-world.git.
"retention_days": "", // string, The number of days that workflow run logs and artifacts are kept.
"retention_days": retentionDays, // string, The number of days that artifacts are kept, empty when kept forever.
"run_id": strconv.FormatInt(run.ID, 10), // string, A unique number for each workflow run within a repository. This number does not change if you re-run the workflow run.
"run_number": strconv.FormatInt(run.Index, 10), // string, A unique number for each run of a particular workflow in a repository. This number begins at 1 for the workflow's first run, and increments with each new run. This number does not change if you re-run the workflow run.
"run_attempt": "", // string, A unique number for each attempt of a particular workflow run in a repository. This number begins at 1 for the workflow run's first attempt, and increments with each re-run.
services/actions/context_test.go
+6
View File
@@ -16,6 +16,7 @@ import (
user_model "gitea.dev/models/user"
actions_module "gitea.dev/modules/actions"
"gitea.dev/modules/json"
"gitea.dev/modules/setting"
api "gitea.dev/modules/structs"
"gitea.dev/modules/test"
webhook_module "gitea.dev/modules/webhook"
@@ -408,6 +409,11 @@ func TestGenerateGiteaContextPullRequestTarget(t *testing.T) {
assert.Equal(t, "refs/heads/main", giteaCtx["ref"])
assert.Equal(t, "main", giteaCtx["ref_name"])
defer test.MockVariableValue(&setting.Actions.ArtifactRetentionDays, 7)()
assert.Equal(t, "7", GenerateGiteaContext(t.Context(), run, nil, nil)["retention_days"])
setting.Actions.ArtifactRetentionDays = 0
assert.Empty(t, GenerateGiteaContext(t.Context(), run, nil, nil)["retention_days"])
}
func TestGenerateGiteaContextRefProtected(t *testing.T) {