From 3b2da7e4aeed8e2d070b9ce5c66282b16eeb87f1 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 27 Sep 2023 13:37:48 +0800 Subject: [PATCH] Redefine the meaning of column is_active to make Actions Registration Token generation easier (#27143) Partially Fix #25041 This PR redefined the meaning of column `is_active` in table `action_runner_token`. Before this PR, `is_active` means whether it has been used by any runner. If it's true, other runner cannot use it to register again. In this PR, `is_active` means whether it's validated to be used to register runner. And if it's true, then it can be used to register runners until it become false. When creating a new `is_active` register token, any previous tokens will be set `is_active` to false. --- models/actions/runner_token.go | 26 ++++++++++++++++++-------- routers/api/actions/runner/runner.go | 6 +++--- routers/private/actions.go | 4 ++-- routers/web/shared/actions/runners.go | 6 +++--- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/models/actions/runner_token.go b/models/actions/runner_token.go index fabd6c644c..cf4e90f3f1 100644 --- a/models/actions/runner_token.go +++ b/models/actions/runner_token.go @@ -22,7 +22,7 @@ type ActionRunnerToken struct { Owner *user_model.User `xorm:"-"` RepoID int64 `xorm:"index"` // repo level runner, if orgid also is zero, then it's a global Repo *repo_model.Repository `xorm:"-"` - IsActive bool + IsActive bool // true means it can be used Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated"` @@ -57,7 +57,7 @@ func UpdateRunnerToken(ctx context.Context, r *ActionRunnerToken, cols ...string return err } -// NewRunnerToken creates a new runner token +// NewRunnerToken creates a new active runner token and invalidate all old tokens func NewRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) { token, err := util.CryptoRandomString(40) if err != nil { @@ -66,17 +66,27 @@ func NewRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerTo runnerToken := &ActionRunnerToken{ OwnerID: ownerID, RepoID: repoID, - IsActive: false, + IsActive: true, Token: token, } - _, err = db.GetEngine(ctx).Insert(runnerToken) - return runnerToken, err + + return runnerToken, db.WithTx(ctx, func(ctx context.Context) error { + if _, err := db.GetEngine(ctx).Where("owner_id =? AND repo_id = ?", ownerID, repoID).Cols("is_active").Update(&ActionRunnerToken{ + IsActive: false, + }); err != nil { + return err + } + + _, err = db.GetEngine(ctx).Insert(runnerToken) + return err + }) } -// GetUnactivatedRunnerToken returns a unactivated runner token -func GetUnactivatedRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) { +// GetLastestRunnerToken returns the latest runner token +func GetLastestRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) { var runnerToken ActionRunnerToken - has, err := db.GetEngine(ctx).Where("owner_id=? AND repo_id=? AND is_active=?", ownerID, repoID, false).OrderBy("id DESC").Get(&runnerToken) + has, err := db.GetEngine(ctx).Where("owner_id=? AND repo_id=?", ownerID, repoID). + OrderBy("id DESC").Get(&runnerToken) if err != nil { return nil, err } else if !has { diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go index cb206f5685..8df6f297ce 100644 --- a/routers/api/actions/runner/runner.go +++ b/routers/api/actions/runner/runner.go @@ -47,11 +47,11 @@ func (s *Service) Register( runnerToken, err := actions_model.GetRunnerToken(ctx, req.Msg.Token) if err != nil { - return nil, errors.New("runner token not found") + return nil, errors.New("runner registration token not found") } - if runnerToken.IsActive { - return nil, errors.New("runner token has already been activated") + if !runnerToken.IsActive { + return nil, errors.New("runner registration token has been invalidated, please use the latest one") } labels := req.Msg.Labels diff --git a/routers/private/actions.go b/routers/private/actions.go index 2403b9c41a..74515256da 100644 --- a/routers/private/actions.go +++ b/routers/private/actions.go @@ -41,8 +41,8 @@ func GenerateActionsRunnerToken(ctx *context.PrivateContext) { }) } - token, err := actions_model.GetUnactivatedRunnerToken(ctx, owner, repo) - if errors.Is(err, util.ErrNotExist) { + token, err := actions_model.GetLastestRunnerToken(ctx, owner, repo) + if errors.Is(err, util.ErrNotExist) || (token != nil && !token.IsActive) { token, err = actions_model.NewRunnerToken(ctx, owner, repo) if err != nil { err := fmt.Sprintf("error while creating runner token: %v", err) diff --git a/routers/web/shared/actions/runners.go b/routers/web/shared/actions/runners.go index 7ff1f3e33f..cd60e9cbee 100644 --- a/routers/web/shared/actions/runners.go +++ b/routers/web/shared/actions/runners.go @@ -35,15 +35,15 @@ func RunnersList(ctx *context.Context, opts actions_model.FindRunnerOptions) { // ownid=0,repo_id=0,means this token is used for global var token *actions_model.ActionRunnerToken - token, err = actions_model.GetUnactivatedRunnerToken(ctx, opts.OwnerID, opts.RepoID) - if errors.Is(err, util.ErrNotExist) { + token, err = actions_model.GetLastestRunnerToken(ctx, opts.OwnerID, opts.RepoID) + if errors.Is(err, util.ErrNotExist) || (token != nil && !token.IsActive) { token, err = actions_model.NewRunnerToken(ctx, opts.OwnerID, opts.RepoID) if err != nil { ctx.ServerError("CreateRunnerToken", err) return } } else if err != nil { - ctx.ServerError("GetUnactivatedRunnerToken", err) + ctx.ServerError("GetLastestRunnerToken", err) return }