Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a new command to set a predefined runner register token and also add a set-token parameter for registration-token API #32878

Closed
wants to merge 10 commits into from
Closed

Add a new command to set a predefined runner register token and also add a set-token parameter for registration-token API #32878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cc18967
Support put a redefined token for Gitea actions command or API
lunny Dec 17, 2024
1f453af
Improve the hint for putting a register token
lunny Dec 17, 2024
aeb6603
Fix test
lunny Dec 17, 2024
5833266
Use a new command to set runner register token
lunny Dec 18, 2024
a2f23db
Merge branch 'main' into lunny/support_predefined_runner_register_token
lunny Dec 18, 2024
4a1d263
Improve the API changes
lunny Dec 18, 2024
43d616d
Fix API parameter name
lunny Dec 18, 2024
dcad326
Fix typo
lunny Dec 18, 2024
d31fe7c
Merge branch 'main' into lunny/support_predefined_runner_register_token
lunny Dec 18, 2024
9993388
Revert wrong change
lunny Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions cmd/actions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var (
Usage: "Manage Gitea Actions",
Subcommands: []*cli.Command{
subcmdActionsGenRunnerToken,
subcmdActionsSetRunnerToken,
},
}

Expand All @@ -36,6 +37,27 @@ var (
},
},
}

subcmdActionsSetRunnerToken = &cli.Command{
Name: "set-runner-token",
Usage: "Set a new token for a runner to as register token",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But isn't "Set a new token" simply "adding a token"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because once the new token is added, all old tokens will be invalided. Only one token will be valid. So that I use set rather than add.

Action: runSetActionsRunnerToken,
Aliases: []string{"srt"},
Flags: []cli.Flag{
&cli.StringFlag{
Name: "scope",
Aliases: []string{"s"},
Value: "",
Usage: "{owner}[/{repo}] - leave empty for a global runner",
},
&cli.StringFlag{
Name: "token",
Aliases: []string{"t"},
Value: "",
Usage: "[{token}] - leave empty will generate a new token, otherwise will update the token to database. The token MUST be a 40 digital string containing only [0-9a-zA-Z]",
Copy link
Contributor

@wxiaoguang wxiaoguang Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without a tool, it is difficult for end users to generate it.

The whole design should be like this:

./gitea actions generate-runner-token --global # generate and update
./gitea actions generate-runner-token --scope owner/repo  # generate and update
./gitea actions generate-runner-token --display-only # display a token only
./gitea actions generate-runner-token --global --set-token the-token-value # set the token to the pre-generated one
./gitea actions generate-runner-token --scope owner/repo --set-token the-token-value # set the token to the pre-generated one

Do not introduce more technical debts

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my previous solution. I created a new subcommand now because you said generate-runner-token is conflicted with set-token(previous put-token).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my previous solution. I created a new subcommand now because you said generate-runner-token is conflicted with set-token(previous put-token).

That's not your previous solution. At least I can see huge difference.

Copy link
Contributor

@wxiaoguang wxiaoguang Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The differences:

  1. we should forbid the ./gitea actions generate-runner-token without arguments (maybe it is a breaking change, but I believe we should do it, and it is easy for end users to follow)
  2. we should add --display-only to help end users to generate the token
  3. --set-token only works with --global or --scope (put itself is an improper name, I do not remember where else we ever used it)

},
},
}
)

func runGenerateActionsRunnerToken(c *cli.Context) error {
Expand All @@ -53,3 +75,20 @@ func runGenerateActionsRunnerToken(c *cli.Context) error {
_, _ = fmt.Printf("%s\n", respText.Text)
return nil
}

func runSetActionsRunnerToken(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()

setting.MustInstalled()

scope := c.String("scope")
token := c.String("token")

respText, extra := private.SetActionsRunnerToken(ctx, scope, token)
if extra.HasError() {
return handleCliResponseExtra(extra)
}
_, _ = fmt.Printf("%s\n", respText.Text)
return nil
}
17 changes: 12 additions & 5 deletions models/actions/runner_token.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,24 @@ 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) {
func NewRunnerToken(ctx context.Context, ownerID, repoID int64, preDefinedToken string) (*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
token := preDefinedToken
if token == "" {
var err error
token, err = util.CryptoRandomString(40)
if err != nil {
return nil, err
}
} else if len(token) != 40 || !util.IsRandomStringValid(token) {
return nil, util.NewInvalidArgumentErrorf("invalid token: %s", token)
}

runnerToken := &ActionRunnerToken{
OwnerID: ownerID,
RepoID: repoID,
Expand All @@ -95,7 +102,7 @@ func NewRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerTo
return err
}

_, err = db.GetEngine(ctx).Insert(runnerToken)
_, err := db.GetEngine(ctx).Insert(runnerToken)
return err
})
}
Expand Down
17 changes: 16 additions & 1 deletion models/actions/runner_token_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/util"

"github.com/stretchr/testify/assert"
)
Expand All @@ -22,11 +23,25 @@ func TestGetLatestRunnerToken(t *testing.T) {

func TestNewRunnerToken(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
token, err := NewRunnerToken(db.DefaultContext, 1, 0)
token, err := NewRunnerToken(db.DefaultContext, 1, 0, "")
assert.NoError(t, err)
expectedToken, err := GetLatestRunnerToken(db.DefaultContext, 1, 0)
assert.NoError(t, err)
assert.EqualValues(t, expectedToken, token)

predefinedToken, err := util.CryptoRandomString(40)
assert.NoError(t, err)

token, err = NewRunnerToken(db.DefaultContext, 1, 0, predefinedToken)
assert.NoError(t, err)
assert.EqualValues(t, predefinedToken, token.Token)

expectedToken, err = GetLatestRunnerToken(db.DefaultContext, 1, 0)
assert.NoError(t, err)
assert.EqualValues(t, expectedToken, token)

_, err = NewRunnerToken(db.DefaultContext, 1, 0, "invalid-token")
assert.Error(t, err)
}

func TestUpdateRunnerToken(t *testing.T) {
Expand Down
17 changes: 17 additions & 0 deletions modules/private/actions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,20 @@ func GenerateActionsRunnerToken(ctx context.Context, scope string) (*ResponseTex

return requestJSONResp(req, &ResponseText{})
}

type SetTokenRequest struct {
Scope string
Token string
}

// SetActionsRunnerToken calls the internal GenerateActionsRunnerToken function
func SetActionsRunnerToken(ctx context.Context, scope, token string) (*ResponseText, ResponseExtra) {
reqURL := setting.LocalURL + "api/internal/actions/set_actions_runner_token"

req := newInternalRequest(ctx, reqURL, "POST", SetTokenRequest{
Scope: scope,
Token: token,
})

return requestJSONResp(req, &ResponseText{})
}
9 changes: 9 additions & 0 deletions modules/util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ func CryptoRandomInt(limit int64) (int64, error) {

const alphanumericalChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"

func IsRandomStringValid(s string) bool {
for _, c := range s {
if !strings.ContainsRune(alphanumericalChars, c) {
return false
}
}
return true
}

// CryptoRandomString generates a crypto random alphanumerical string, each byte is generated by [0,61] range
func CryptoRandomString(length int64) (string, error) {
buf := make([]byte, length)
Expand Down
5 changes: 5 additions & 0 deletions routers/api/v1/admin/runners.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ func GetRegistrationToken(ctx *context.APIContext) {
// produces:
// - application/json
// parameters:
// - name: set_token
// in: body
// description: set a runner register token instead of generating one.
// type: string
// required: false
// responses:
// "200":
// "$ref": "#/responses/RegistrationToken"
Expand Down
5 changes: 5 additions & 0 deletions routers/api/v1/org/action.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ func (Action) GetRegistrationToken(ctx *context.APIContext) {
// description: name of the organization
// type: string
// required: true
// - name: set_token
// in: body
// description: set a runner register token instead of generating one.
// type: string
// required: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's really strange that a GET request accepts "set_token" parameter. And it is anti-pattern to make the GET request updates the target.

https://docs.github.com/en/rest/actions/self-hosted-runners?apiVersion=2022-11-28#create-a-registration-token-for-an-organization

Does GitHub do so?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous API is GET but looks like GitHub uses POST and Github doesn't support a similar feature.

// responses:
// "200":
// "$ref": "#/responses/RegistrationToken"
Expand Down
5 changes: 5 additions & 0 deletions routers/api/v1/repo/action.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,11 @@ func (Action) GetRegistrationToken(ctx *context.APIContext) {
// description: name of the repo
// type: string
// required: true
// - name: set_token
// in: body
// description: set a runner register token instead of generating one.
// type: string
// required: false
// responses:
// "200":
// "$ref": "#/responses/RegistrationToken"
Expand Down
12 changes: 8 additions & 4 deletions routers/api/v1/shared/runners.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,18 @@ type RegistrationToken struct {
}

func GetRegistrationToken(ctx *context.APIContext, ownerID, repoID int64) {
token, err := actions_model.GetLatestRunnerToken(ctx, ownerID, repoID)
if errors.Is(err, util.ErrNotExist) || (token != nil && !token.IsActive) {
token, err = actions_model.NewRunnerToken(ctx, ownerID, repoID)
setToken := ctx.FormString("set_token")
var token *actions_model.ActionRunnerToken
var err error
if setToken == "" {
token, err = actions_model.GetLatestRunnerToken(ctx, ownerID, repoID)
}
if setToken != "" || errors.Is(err, util.ErrNotExist) || (token != nil && !token.IsActive) {
token, err = actions_model.NewRunnerToken(ctx, ownerID, repoID, setToken)
}
if err != nil {
ctx.InternalServerError(err)
return
}

ctx.JSON(http.StatusOK, RegistrationToken{Token: token.Token})
}
43 changes: 42 additions & 1 deletion routers/private/actions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func GenerateActionsRunnerToken(ctx *context.PrivateContext) {

token, err := actions_model.GetLatestRunnerToken(ctx, owner, repo)
if errors.Is(err, util.ErrNotExist) || (token != nil && !token.IsActive) {
token, err = actions_model.NewRunnerToken(ctx, owner, repo)
token, err = actions_model.NewRunnerToken(ctx, owner, repo, "")
if err != nil {
errMsg := fmt.Sprintf("error while creating runner token: %v", err)
log.Error("NewRunnerToken failed: %v", errMsg)
Expand Down Expand Up @@ -90,3 +90,44 @@ func parseScope(ctx *context.PrivateContext, scope string) (ownerID, repoID int6
repoID = r.ID
return ownerID, repoID, nil
}

// SetActionsRunnerToken set a runner token for a given scope
func SetActionsRunnerToken(ctx *context.PrivateContext) {
var setRequest private.SetTokenRequest
rd := ctx.Req.Body
defer rd.Close()

if err := json.NewDecoder(rd).Decode(&setRequest); err != nil {
log.Error("JSON Decode failed: %v", err)
ctx.JSON(http.StatusInternalServerError, private.Response{
Err: err.Error(),
})
return
}

owner, repo, err := parseScope(ctx, setRequest.Scope)
if err != nil {
log.Error("parseScope failed: %v", err)
ctx.JSON(http.StatusInternalServerError, private.Response{
Err: err.Error(),
})
}
if setRequest.Token == "" {
ctx.JSON(http.StatusInternalServerError, private.Response{
Err: "token is empty",
})
return
}

token, err := actions_model.NewRunnerToken(ctx, owner, repo, setRequest.Token)
if err != nil {
errMsg := fmt.Sprintf("error while creating runner token: %v", err)
log.Error("NewRunnerToken failed: %v", errMsg)
ctx.JSON(http.StatusInternalServerError, private.Response{
Err: errMsg,
})
return
}

ctx.PlainText(http.StatusOK, token.Token)
}
1 change: 1 addition & 0 deletions routers/private/internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ func Routes() *web.Router {
r.Post("/mail/send", SendEmail)
r.Post("/restore_repo", RestoreRepo)
r.Post("/actions/generate_actions_runner_token", GenerateActionsRunnerToken)
r.Post("/actions/set_actions_runner_token", SetActionsRunnerToken)

r.Group("/repo", func() {
// FIXME: it is not right to use context.Contexter here because all routes here should use PrivateContext
Expand Down
4 changes: 2 additions & 2 deletions routers/web/shared/actions/runners.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func RunnersList(ctx *context.Context, opts actions_model.FindRunnerOptions) {
var token *actions_model.ActionRunnerToken
token, err = actions_model.GetLatestRunnerToken(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)
token, err = actions_model.NewRunnerToken(ctx, opts.OwnerID, opts.RepoID, "")
if err != nil {
ctx.ServerError("CreateRunnerToken", err)
return
Expand Down Expand Up @@ -131,7 +131,7 @@ func RunnerDetailsEditPost(ctx *context.Context, runnerID, ownerID, repoID int64

// RunnerResetRegistrationToken reset registration token
func RunnerResetRegistrationToken(ctx *context.Context, ownerID, repoID int64, redirectTo string) {
_, err := actions_model.NewRunnerToken(ctx, ownerID, repoID)
_, err := actions_model.NewRunnerToken(ctx, ownerID, repoID, "")
if err != nil {
ctx.ServerError("ResetRunnerRegistrationToken", err)
return
Expand Down
20 changes: 20 additions & 0 deletions templates/swagger/v1_json.tmpl
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading