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

custom/conf/app.example.ini

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

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

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

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

@@ -68,6 +68,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.
@@ -93,7 +98,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

@@ -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"
@@ -385,7 +386,7 @@ func TestFindTaskNeeds(t *testing.T) {
 	assert.Equal(t, "bbb", ret["job1"].Outputs["output_b"])
 }
 
-func TestGenerateGiteaContextPullRequestRefs(t *testing.T) {
+func TestGenerateGiteaContext(t *testing.T) {
 	payload := api.PullRequestPayload{
 		PullRequest: &api.PullRequest{
 			Base: &api.PRBranchInfo{
@@ -418,6 +419,11 @@ func TestGenerateGiteaContextPullRequestRefs(t *testing.T) {
 
 	run.TriggerEvent, run.Ref = string(actions_module.GithubEventPullRequest), "refs/pull/1/head"
 	assert.Equal(t, "branch", GenerateGiteaContext(t.Context(), run, nil, nil)["ref_type"])
+
+	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) {