Merge remote-tracking branch 'giteaofficial/main'

* giteaofficial/main:
  Clear up old Actions logs (go-gitea#31735)
  Fix createElementFromAttrs bug (go-gitea#31751)
  bump vue-bar-graph (go-gitea#31705)
  Use UTC as default timezone when schedule Actions cron tasks (go-gitea#31742)
  Add permission description for API to add repo collaborator (go-gitea#31744)
  Clarify Actions resources ownership (go-gitea#31724)
  Exclude protected branches from recently pushed (go-gitea#31748)
  [skip ci] Updated translations via Crowdin
  Distinguish LFS object errors to ignore missing objects during migration (go-gitea#31702)

Author: zjjhot

custom/conf/app.example.ini

@@ -2684,6 +2684,8 @@ LEVEL = Info
 ;;
 ;; Default platform to get action plugins, `github` for `https://github.com`, `self` for the current Gitea instance.
 ;DEFAULT_ACTIONS_URL = github
+;; Logs retention time in days. Old logs will be deleted after this period.
+;LOG_RETENTION_DAYS = 365
 ;; Default artifact retention time in days. Artifacts could have their own retention periods by setting the `retention-days` option in `actions/upload-artifact` step.
 ;ARTIFACT_RETENTION_DAYS = 90
 ;; Timeout to stop the task which have running status, but haven't been updated for a long time

models/actions/runner.go

@@ -23,14 +23,25 @@ import (
 )
 
 // ActionRunner represents runner machines
+//
+// It can be:
+//  1. global runner, OwnerID is 0 and RepoID is 0
+//  2. org/user level runner, OwnerID is org/user ID and RepoID is 0
+//  3. repo level runner, OwnerID is 0 and RepoID is repo ID
+//
+// Please note that it's not acceptable to have both OwnerID and RepoID to be non-zero,
+// or it will be complicated to find runners belonging to a specific owner.
+// For example, conditions like `OwnerID = 1` will also return runner {OwnerID: 1, RepoID: 1},
+// but it's a repo level runner, not an org/user level runner.
+// To avoid this, make it clear with {OwnerID: 0, RepoID: 1} for repo level runners.
 type ActionRunner struct {
 	ID          int64
 	UUID        string                 `xorm:"CHAR(36) UNIQUE"`
 	Name        string                 `xorm:"VARCHAR(255)"`
 	Version     string                 `xorm:"VARCHAR(64)"`
-	OwnerID     int64                  `xorm:"index"` // org level runner, 0 means system
+	OwnerID     int64                  `xorm:"index"`
 	Owner       *user_model.User       `xorm:"-"`
-	RepoID      int64                  `xorm:"index"` // repo level runner, if OwnerID also is zero, then it's a global
+	RepoID      int64                  `xorm:"index"`
 	Repo        *repo_model.Repository `xorm:"-"`
 	Description string                 `xorm:"TEXT"`
 	Base        int                    // 0 native 1 docker 2 virtual machine
@@ -157,7 +168,7 @@ func init() {
 type FindRunnerOptions struct {
 	db.ListOptions
 	RepoID        int64
-	OwnerID       int64
+	OwnerID       int64 // it will be ignored if RepoID is set
 	Sort          string
 	Filter        string
 	IsOnline      optional.Option[bool]
@@ -174,8 +185,7 @@ func (opts FindRunnerOptions) ToConds() builder.Cond {
 			c = c.Or(builder.Eq{"repo_id": 0, "owner_id": 0})
 		}
 		cond = cond.And(c)
-	}
-	if opts.OwnerID > 0 {
+	} else if opts.OwnerID > 0 { // OwnerID is ignored if RepoID is set
 		c := builder.NewCond().And(builder.Eq{"owner_id": opts.OwnerID})
 		if opts.WithAvailable {
 			c = c.Or(builder.Eq{"repo_id": 0, "owner_id": 0})
@@ -263,6 +273,11 @@ func DeleteRunner(ctx context.Context, id int64) error {
 
 // CreateRunner creates new runner.
 func CreateRunner(ctx context.Context, t *ActionRunner) error {
+	if t.OwnerID != 0 && t.RepoID != 0 {
+		// It's trying to create a runner that belongs to a repository, but OwnerID has been set accidentally.
+		// Remove OwnerID to avoid confusion; it's not worth returning an error here.
+		t.OwnerID = 0
+	}
 	return db.Insert(ctx, t)
 }
 

models/actions/runner_token.go

@@ -15,12 +15,23 @@ import (
 )
 
 // ActionRunnerToken represents runner tokens
+//
+// It can be:
+//  1. global token, OwnerID is 0 and RepoID is 0
+//  2. org/user level token, OwnerID is org/user ID and RepoID is 0
+//  3. repo level token, OwnerID is 0 and RepoID is repo ID
+//
+// Please note that it's not acceptable to have both OwnerID and RepoID to be non-zero,
+// or it will be complicated to find tokens belonging to a specific owner.
+// For example, conditions like `OwnerID = 1` will also return token {OwnerID: 1, RepoID: 1},
+// but it's a repo level token, not an org/user level token.
+// To avoid this, make it clear with {OwnerID: 0, RepoID: 1} for repo level tokens.
 type ActionRunnerToken struct {
 	ID       int64
 	Token    string                 `xorm:"UNIQUE"`
-	OwnerID  int64                  `xorm:"index"` // org level runner, 0 means system
+	OwnerID  int64                  `xorm:"index"`
 	Owner    *user_model.User       `xorm:"-"`
-	RepoID   int64                  `xorm:"index"` // repo level runner, if orgid also is zero, then it's a global
+	RepoID   int64                  `xorm:"index"`
 	Repo     *repo_model.Repository `xorm:"-"`
 	IsActive bool                   // true means it can be used
 
@@ -58,7 +69,14 @@ func UpdateRunnerToken(ctx context.Context, r *ActionRunnerToken, cols ...string
 }
 
 // NewRunnerToken creates a new active runner token and invalidate all old tokens
+// ownerID will be ignored and treated as 0 if repoID is non-zero.
 func NewRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) {
+	if ownerID != 0 && repoID != 0 {
+		// It's trying to create a runner token that belongs to a repository, but OwnerID has been set accidentally.
+		// Remove OwnerID to avoid confusion; it's not worth returning an error here.
+		ownerID = 0
+	}
+
 	token, err := util.CryptoRandomString(40)
 	if err != nil {
 		return nil, err
@@ -84,6 +102,12 @@ func NewRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerTo
 
 // GetLatestRunnerToken returns the latest runner token
 func GetLatestRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) {
+	if ownerID != 0 && repoID != 0 {
+		// It's trying to get a runner token that belongs to a repository, but OwnerID has been set accidentally.
+		// Remove OwnerID to avoid confusion; it's not worth returning an error here.
+		ownerID = 0
+	}
+
 	var runnerToken ActionRunnerToken
 	has, err := db.GetEngine(ctx).Where("owner_id=? AND repo_id=?", ownerID, repoID).
 		OrderBy("id DESC").Get(&runnerToken)

models/actions/schedule.go

@@ -13,8 +13,6 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/timeutil"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
-
-	"github.com/robfig/cron/v3"
 )
 
 // ActionSchedule represents a schedule of a workflow file
@@ -53,8 +51,6 @@ func GetReposMapByIDs(ctx context.Context, ids []int64) (map[int64]*repo_model.R
 	return repos, db.GetEngine(ctx).In("id", ids).Find(&repos)
 }
 
-var cronParser = cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)
-
 // CreateScheduleTask creates new schedule task.
 func CreateScheduleTask(ctx context.Context, rows []*ActionSchedule) error {
 	// Return early if there are no rows to insert
@@ -80,19 +76,21 @@ func CreateScheduleTask(ctx context.Context, rows []*ActionSchedule) error {
 		now := time.Now()
 
 		for _, spec := range row.Specs {
+			specRow := &ActionScheduleSpec{
+				RepoID:     row.RepoID,
+				ScheduleID: row.ID,
+				Spec:       spec,
+			}
 			// Parse the spec and check for errors
-			schedule, err := cronParser.Parse(spec)
+			schedule, err := specRow.Parse()
 			if err != nil {
 				continue // skip to the next spec if there's an error
 			}
 
+			specRow.Next = timeutil.TimeStamp(schedule.Next(now).Unix())
+
 			// Insert the new schedule spec row
-			if err = db.Insert(ctx, &ActionScheduleSpec{
-				RepoID:     row.RepoID,
-				ScheduleID: row.ID,
-				Spec:       spec,
-				Next:       timeutil.TimeStamp(schedule.Next(now).Unix()),
-			}); err != nil {
+			if err = db.Insert(ctx, specRow); err != nil {
 				return err
 			}
 		}

models/actions/schedule_spec.go

@@ -5,6 +5,8 @@ package actions
 
 import (
 	"context"
+	"strings"
+	"time"
 
 	"code.gitea.io/gitea/models/db"
 	repo_model "code.gitea.io/gitea/models/repo"
@@ -32,8 +34,29 @@ type ActionScheduleSpec struct {
 	Updated timeutil.TimeStamp `xorm:"updated"`
 }
 
+// Parse parses the spec and returns a cron.Schedule
+// Unlike the default cron parser, Parse uses UTC timezone as the default if none is specified.
 func (s *ActionScheduleSpec) Parse() (cron.Schedule, error) {
-	return cronParser.Parse(s.Spec)
+	parser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)
+	schedule, err := parser.Parse(s.Spec)
+	if err != nil {
+		return nil, err
+	}
+
+	// If the spec has specified a timezone, use it
+	if strings.HasPrefix(s.Spec, "TZ=") || strings.HasPrefix(s.Spec, "CRON_TZ=") {
+		return schedule, nil
+	}
+
+	specSchedule, ok := schedule.(*cron.SpecSchedule)
+	// If it's not a spec schedule, like "@every 5m", timezone is not relevant
+	if !ok {
+		return schedule, nil
+	}
+
+	// Set the timezone to UTC
+	specSchedule.Location = time.UTC
+	return specSchedule, nil
 }
 
 func init() {

models/actions/schedule_spec_test.go

@@ -0,0 +1,71 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package actions
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestActionScheduleSpec_Parse(t *testing.T) {
+	// Mock the local timezone is not UTC
+	local := time.Local
+	tz, err := time.LoadLocation("Asia/Shanghai")
+	require.NoError(t, err)
+	defer func() {
+		time.Local = local
+	}()
+	time.Local = tz
+
+	now, err := time.Parse(time.RFC3339, "2024-07-31T15:47:55+08:00")
+	require.NoError(t, err)
+
+	tests := []struct {
+		name    string
+		spec    string
+		want    string
+		wantErr assert.ErrorAssertionFunc
+	}{
+		{
+			name:    "regular",
+			spec:    "0 10 * * *",
+			want:    "2024-07-31T10:00:00Z",
+			wantErr: assert.NoError,
+		},
+		{
+			name:    "invalid",
+			spec:    "0 10 * *",
+			want:    "",
+			wantErr: assert.Error,
+		},
+		{
+			name:    "with timezone",
+			spec:    "TZ=America/New_York 0 10 * * *",
+			want:    "2024-07-31T14:00:00Z",
+			wantErr: assert.NoError,
+		},
+		{
+			name:    "timezone irrelevant",
+			spec:    "@every 5m",
+			want:    "2024-07-31T07:52:55Z",
+			wantErr: assert.NoError,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s := &ActionScheduleSpec{
+				Spec: tt.spec,
+			}
+			got, err := s.Parse()
+			tt.wantErr(t, err)
+
+			if err == nil {
+				assert.Equal(t, tt.want, got.Next(now).UTC().Format(time.RFC3339))
+			}
+		})
+	}
+}

models/actions/task.go

@@ -35,7 +35,7 @@ type ActionTask struct {
 	RunnerID int64              `xorm:"index"`
 	Status   Status             `xorm:"index"`
 	Started  timeutil.TimeStamp `xorm:"index"`
-	Stopped  timeutil.TimeStamp
+	Stopped  timeutil.TimeStamp `xorm:"index(stopped_log_expired)"`
 
 	RepoID            int64  `xorm:"index"`
 	OwnerID           int64  `xorm:"index"`
@@ -51,8 +51,8 @@ type ActionTask struct {
 	LogInStorage bool       // read log from database or from storage
 	LogLength    int64      // lines count
 	LogSize      int64      // blob size
-	LogIndexes   LogIndexes `xorm:"LONGBLOB"` // line number to offset
-	LogExpired   bool       // files that are too old will be deleted
+	LogIndexes   LogIndexes `xorm:"LONGBLOB"`                   // line number to offset
+	LogExpired   bool       `xorm:"index(stopped_log_expired)"` // files that are too old will be deleted
 
 	Created timeutil.TimeStamp `xorm:"created"`
 	Updated timeutil.TimeStamp `xorm:"updated index"`
@@ -470,6 +470,16 @@ func StopTask(ctx context.Context, taskID int64, status Status) error {
 	return nil
 }
 
+func FindOldTasksToExpire(ctx context.Context, olderThan timeutil.TimeStamp, limit int) ([]*ActionTask, error) {
+	e := db.GetEngine(ctx)
+
+	tasks := make([]*ActionTask, 0, limit)
+	// Check "stopped > 0" to avoid deleting tasks that are still running
+	return tasks, e.Where("stopped > 0 AND stopped < ? AND log_expired = ?", olderThan, false).
+		Limit(limit).
+		Find(&tasks)
+}
+
 func isSubset(set, subset []string) bool {
 	m := make(container.Set[string], len(set))
 	for _, v := range set {

models/actions/variable.go

@@ -5,7 +5,6 @@ package actions
 
 import (
 	"context"
-	"errors"
 	"strings"
 
 	"code.gitea.io/gitea/models/db"
@@ -15,6 +14,18 @@ import (
 	"xorm.io/builder"
 )
 
+// ActionVariable represents a variable that can be used in actions
+//
+// It can be:
+//  1. global variable, OwnerID is 0 and RepoID is 0
+//  2. org/user level variable, OwnerID is org/user ID and RepoID is 0
+//  3. repo level variable, OwnerID is 0 and RepoID is repo ID
+//
+// Please note that it's not acceptable to have both OwnerID and RepoID to be non-zero,
+// or it will be complicated to find variables belonging to a specific owner.
+// For example, conditions like `OwnerID = 1` will also return variable {OwnerID: 1, RepoID: 1},
+// but it's a repo level variable, not an org/user level variable.
+// To avoid this, make it clear with {OwnerID: 0, RepoID: 1} for repo level variables.
 type ActionVariable struct {
 	ID          int64              `xorm:"pk autoincr"`
 	OwnerID     int64              `xorm:"UNIQUE(owner_repo_name)"`
@@ -29,39 +40,40 @@ func init() {
 	db.RegisterModel(new(ActionVariable))
 }
 
-func (v *ActionVariable) Validate() error {
-	if v.OwnerID != 0 && v.RepoID != 0 {
-		return errors.New("a variable should not be bound to an owner and a repository at the same time")
+func InsertVariable(ctx context.Context, ownerID, repoID int64, name, data string) (*ActionVariable, error) {
+	if ownerID != 0 && repoID != 0 {
+		// It's trying to create a variable that belongs to a repository, but OwnerID has been set accidentally.
+		// Remove OwnerID to avoid confusion; it's not worth returning an error here.
+		ownerID = 0
 	}
-	return nil
-}
 
-func InsertVariable(ctx context.Context, ownerID, repoID int64, name, data string) (*ActionVariable, error) {
 	variable := &ActionVariable{
 		OwnerID: ownerID,
 		RepoID:  repoID,
 		Name:    strings.ToUpper(name),
 		Data:    data,
 	}
-	if err := variable.Validate(); err != nil {
-		return variable, err
-	}
 	return variable, db.Insert(ctx, variable)
 }
 
 type FindVariablesOpts struct {
 	db.ListOptions
-	OwnerID int64
 	RepoID  int64
+	OwnerID int64 // it will be ignored if RepoID is set
 	Name    string
 }
 
 func (opts FindVariablesOpts) ToConds() builder.Cond {
 	cond := builder.NewCond()
 	// Since we now support instance-level variables,
 	// there is no need to check for null values for `owner_id` and `repo_id`
-	cond = cond.And(builder.Eq{"owner_id": opts.OwnerID})
 	cond = cond.And(builder.Eq{"repo_id": opts.RepoID})
+	if opts.RepoID != 0 { // if RepoID is set
+		// ignore OwnerID and treat it as 0
+		cond = cond.And(builder.Eq{"owner_id": 0})
+	} else {
+		cond = cond.And(builder.Eq{"owner_id": opts.OwnerID})
+	}
 
 	if opts.Name != "" {
 		cond = cond.And(builder.Eq{"name": strings.ToUpper(opts.Name)})