feat(cli): add --keep-labels flag to forgejo actions register (#4610)

This commit adds a new flag, `--keep-labels`, to the runner registration CLI command. If this flag is present and the runner being registered already exists, it will prevent the runners' labels from being reset.

In order to accomplish this, the signature of the `RegisterRunner` function from the `models/actions` package has been modified so that the labels argument can be nil. If it is, the part of the function that updates the record will not change the runner.

Various tests have been added for this function, for the following cases: new runner with labels, new runner without label, existing runner with labels, existing runner without labels.

The flag has been added to the CLI command, the action function has been updated to read the labels parameters through a separate function (`getLabels`), and test cases for this function have been added.

<!--
Before submitting a PR, please read the contributing guidelines:
https://codeberg.org/forgejo/forgejo/src/branch/forgejo/CONTRIBUTING.md
-->

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4610
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Emmanuel BENOÎT <tseeker@nocternity.net>
Co-committed-by: Emmanuel BENOÎT <tseeker@nocternity.net>
This commit is contained in:
Emmanuel BENOÎT 2024-07-22 07:33:45 +00:00 committed by Earl Warren
parent 8030ebf64c
commit fdb1874ada
5 changed files with 241 additions and 11 deletions

View file

@ -86,6 +86,11 @@ func SubcmdActionsRegister(ctx context.Context) *cli.Command {
Value: "", Value: "",
Usage: "comma separated list of labels supported by the runner (e.g. docker,ubuntu-latest,self-hosted) (not required since v1.21)", Usage: "comma separated list of labels supported by the runner (e.g. docker,ubuntu-latest,self-hosted) (not required since v1.21)",
}, },
&cli.BoolFlag{
Name: "keep-labels",
Value: false,
Usage: "do not affect the labels when updating an existing runner",
},
&cli.StringFlag{ &cli.StringFlag{
Name: "name", Name: "name",
Value: "runner", Value: "runner",
@ -133,6 +138,17 @@ func validateSecret(secret string) error {
return nil return nil
} }
func getLabels(cliCtx *cli.Context) (*[]string, error) {
if !cliCtx.Bool("keep-labels") {
lblValue := strings.Split(cliCtx.String("labels"), ",")
return &lblValue, nil
}
if cliCtx.String("labels") != "" {
return nil, fmt.Errorf("--labels and --keep-labels should not be used together")
}
return nil, nil
}
func RunRegister(ctx context.Context, cliCtx *cli.Context) error { func RunRegister(ctx context.Context, cliCtx *cli.Context) error {
var cancel context.CancelFunc var cancel context.CancelFunc
if !ContextGetNoInit(ctx) { if !ContextGetNoInit(ctx) {
@ -153,9 +169,12 @@ func RunRegister(ctx context.Context, cliCtx *cli.Context) error {
return err return err
} }
scope := cliCtx.String("scope") scope := cliCtx.String("scope")
labels := cliCtx.String("labels")
name := cliCtx.String("name") name := cliCtx.String("name")
version := cliCtx.String("version") version := cliCtx.String("version")
labels, err := getLabels(cliCtx)
if err != nil {
return err
}
// //
// There are two kinds of tokens // There are two kinds of tokens
@ -179,7 +198,7 @@ func RunRegister(ctx context.Context, cliCtx *cli.Context) error {
return err return err
} }
runner, err := actions_model.RegisterRunner(ctx, owner, repo, secret, strings.Split(labels, ","), name, version) runner, err := actions_model.RegisterRunner(ctx, owner, repo, secret, labels, name, version)
if err != nil { if err != nil {
return fmt.Errorf("error while registering runner: %v", err) return fmt.Errorf("error while registering runner: %v", err)
} }

View file

@ -0,0 +1,88 @@
// Copyright The Forgejo Authors.
// SPDX-License-Identifier: MIT
package forgejo
import (
"fmt"
"testing"
"code.gitea.io/gitea/services/context"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/urfave/cli/v2"
)
func TestActions_getLabels(t *testing.T) {
type testCase struct {
args []string
hasLabels bool
hasError bool
labels []string
}
type resultType struct {
labels *[]string
err error
}
cases := []testCase{
{
args: []string{"x"},
hasLabels: true,
hasError: false,
labels: []string{""},
}, {
args: []string{"x", "--labels", "a,b"},
hasLabels: true,
hasError: false,
labels: []string{"a", "b"},
}, {
args: []string{"x", "--keep-labels"},
hasLabels: false,
hasError: false,
}, {
args: []string{"x", "--keep-labels", "--labels", "a,b"},
hasLabels: false,
hasError: true,
}, {
// this edge-case exists because that's what actually happens
// when no '--labels ...' options are present
args: []string{"x", "--keep-labels", "--labels", ""},
hasLabels: false,
hasError: false,
},
}
flags := SubcmdActionsRegister(context.Context{}).Flags
for _, c := range cases {
t.Run(fmt.Sprintf("args: %v", c.args), func(t *testing.T) {
// Create a copy of command to test
var result *resultType
app := cli.NewApp()
app.Flags = flags
app.Action = func(ctx *cli.Context) error {
labels, err := getLabels(ctx)
result = &resultType{labels, err}
return nil
}
// Run it
_ = app.Run(c.args)
// Test the results
require.NotNil(t, result)
if c.hasLabels {
assert.NotNil(t, result.labels)
assert.Equal(t, c.labels, *result.labels)
} else {
assert.Nil(t, result.labels)
}
if c.hasError {
assert.NotNil(t, result.err)
} else {
assert.Nil(t, result.err)
}
})
}
}

View file

@ -14,7 +14,7 @@ import (
gouuid "github.com/google/uuid" gouuid "github.com/google/uuid"
) )
func RegisterRunner(ctx context.Context, ownerID, repoID int64, token string, labels []string, name, version string) (*ActionRunner, error) { func RegisterRunner(ctx context.Context, ownerID, repoID int64, token string, labels *[]string, name, version string) (*ActionRunner, error) {
uuid, err := gouuid.FromBytes([]byte(token[:16])) uuid, err := gouuid.FromBytes([]byte(token[:16]))
if err != nil { if err != nil {
return nil, fmt.Errorf("gouuid.FromBytes %v", err) return nil, fmt.Errorf("gouuid.FromBytes %v", err)
@ -39,9 +39,10 @@ func RegisterRunner(ctx context.Context, ownerID, repoID int64, token string, la
hash := auth_model.HashToken(token, salt) hash := auth_model.HashToken(token, salt)
runner = ActionRunner{ runner = ActionRunner{
UUID: uuidString, UUID: uuidString,
TokenHash: hash, TokenHash: hash,
TokenSalt: salt, TokenSalt: salt,
AgentLabels: []string{},
} }
if err := CreateRunner(ctx, &runner); err != nil { if err := CreateRunner(ctx, &runner); err != nil {
@ -54,13 +55,17 @@ func RegisterRunner(ctx context.Context, ownerID, repoID int64, token string, la
// //
name, _ = util.SplitStringAtByteN(name, 255) name, _ = util.SplitStringAtByteN(name, 255)
cols := []string{"name", "owner_id", "repo_id", "version"}
runner.Name = name runner.Name = name
runner.OwnerID = ownerID runner.OwnerID = ownerID
runner.RepoID = repoID runner.RepoID = repoID
runner.Version = version runner.Version = version
runner.AgentLabels = labels if labels != nil {
runner.AgentLabels = *labels
cols = append(cols, "agent_labels")
}
if err := UpdateRunner(ctx, &runner, "name", "owner_id", "repo_id", "version", "agent_labels"); err != nil { if err := UpdateRunner(ctx, &runner, cols...); err != nil {
return &runner, fmt.Errorf("can't update the runner %+v %w", runner, err) return &runner, fmt.Errorf("can't update the runner %+v %w", runner, err)
} }

View file

@ -13,7 +13,7 @@ import (
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
func TestActions_RegisterRunner(t *testing.T) { func TestActions_RegisterRunner_Token(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase()) assert.NoError(t, unittest.PrepareTestDatabase())
ownerID := int64(0) ownerID := int64(0)
repoID := int64(0) repoID := int64(0)
@ -21,9 +21,127 @@ func TestActions_RegisterRunner(t *testing.T) {
labels := []string{} labels := []string{}
name := "runner" name := "runner"
version := "v1.2.3" version := "v1.2.3"
runner, err := RegisterRunner(db.DefaultContext, ownerID, repoID, token, labels, name, version) runner, err := RegisterRunner(db.DefaultContext, ownerID, repoID, token, &labels, name, version)
assert.NoError(t, err) assert.NoError(t, err)
assert.EqualValues(t, name, runner.Name) assert.EqualValues(t, name, runner.Name)
assert.EqualValues(t, 1, subtle.ConstantTimeCompare([]byte(runner.TokenHash), []byte(auth_model.HashToken(token, runner.TokenSalt))), "the token cannot be verified with the same method as routers/api/actions/runner/interceptor.go as of 8228751c55d6a4263f0fec2932ca16181c09c97d") assert.EqualValues(t, 1, subtle.ConstantTimeCompare([]byte(runner.TokenHash), []byte(auth_model.HashToken(token, runner.TokenSalt))), "the token cannot be verified with the same method as routers/api/actions/runner/interceptor.go as of 8228751c55d6a4263f0fec2932ca16181c09c97d")
} }
func TestActions_RegisterRunner_CreateWithLabels(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
ownerID := int64(0)
repoID := int64(0)
token := "0123456789012345678901234567890123456789"
name := "runner"
version := "v1.2.3"
labels := []string{"woop", "doop"}
labelsCopy := labels // labels may be affected by the tested function so we copy them
runner, err := RegisterRunner(db.DefaultContext, ownerID, repoID, token, &labels, name, version)
assert.NoError(t, err)
// Check that the returned record has been updated, except for the labels
assert.EqualValues(t, ownerID, runner.OwnerID)
assert.EqualValues(t, repoID, runner.RepoID)
assert.EqualValues(t, name, runner.Name)
assert.EqualValues(t, version, runner.Version)
assert.EqualValues(t, labelsCopy, runner.AgentLabels)
// Check that whatever is in the DB has been updated, except for the labels
after := unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: runner.ID})
assert.EqualValues(t, ownerID, after.OwnerID)
assert.EqualValues(t, repoID, after.RepoID)
assert.EqualValues(t, name, after.Name)
assert.EqualValues(t, version, after.Version)
assert.EqualValues(t, labelsCopy, after.AgentLabels)
}
func TestActions_RegisterRunner_CreateWithoutLabels(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
ownerID := int64(0)
repoID := int64(0)
token := "0123456789012345678901234567890123456789"
name := "runner"
version := "v1.2.3"
runner, err := RegisterRunner(db.DefaultContext, ownerID, repoID, token, nil, name, version)
assert.NoError(t, err)
// Check that the returned record has been updated, except for the labels
assert.EqualValues(t, ownerID, runner.OwnerID)
assert.EqualValues(t, repoID, runner.RepoID)
assert.EqualValues(t, name, runner.Name)
assert.EqualValues(t, version, runner.Version)
assert.EqualValues(t, []string{}, runner.AgentLabels)
// Check that whatever is in the DB has been updated, except for the labels
after := unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: runner.ID})
assert.EqualValues(t, ownerID, after.OwnerID)
assert.EqualValues(t, repoID, after.RepoID)
assert.EqualValues(t, name, after.Name)
assert.EqualValues(t, version, after.Version)
assert.EqualValues(t, []string{}, after.AgentLabels)
}
func TestActions_RegisterRunner_UpdateWithLabels(t *testing.T) {
const recordID = 12345678
token := "7e577e577e577e57feedfacefeedfacefeedface"
assert.NoError(t, unittest.PrepareTestDatabase())
unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: recordID})
newOwnerID := int64(1)
newRepoID := int64(1)
newName := "rennur"
newVersion := "v4.5.6"
newLabels := []string{"warp", "darp"}
labelsCopy := newLabels // labels may be affected by the tested function so we copy them
runner, err := RegisterRunner(db.DefaultContext, newOwnerID, newRepoID, token, &newLabels, newName, newVersion)
assert.NoError(t, err)
// Check that the returned record has been updated
assert.EqualValues(t, newOwnerID, runner.OwnerID)
assert.EqualValues(t, newRepoID, runner.RepoID)
assert.EqualValues(t, newName, runner.Name)
assert.EqualValues(t, newVersion, runner.Version)
assert.EqualValues(t, labelsCopy, runner.AgentLabels)
// Check that whatever is in the DB has been updated
after := unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: recordID})
assert.EqualValues(t, newOwnerID, after.OwnerID)
assert.EqualValues(t, newRepoID, after.RepoID)
assert.EqualValues(t, newName, after.Name)
assert.EqualValues(t, newVersion, after.Version)
assert.EqualValues(t, labelsCopy, after.AgentLabels)
}
func TestActions_RegisterRunner_UpdateWithoutLabels(t *testing.T) {
const recordID = 12345678
token := "7e577e577e577e57feedfacefeedfacefeedface"
assert.NoError(t, unittest.PrepareTestDatabase())
before := unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: recordID})
newOwnerID := int64(1)
newRepoID := int64(1)
newName := "rennur"
newVersion := "v4.5.6"
runner, err := RegisterRunner(db.DefaultContext, newOwnerID, newRepoID, token, nil, newName, newVersion)
assert.NoError(t, err)
// Check that the returned record has been updated, except for the labels
assert.EqualValues(t, newOwnerID, runner.OwnerID)
assert.EqualValues(t, newRepoID, runner.RepoID)
assert.EqualValues(t, newName, runner.Name)
assert.EqualValues(t, newVersion, runner.Version)
assert.EqualValues(t, before.AgentLabels, runner.AgentLabels)
// Check that whatever is in the DB has been updated, except for the labels
after := unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: recordID})
assert.EqualValues(t, newOwnerID, after.OwnerID)
assert.EqualValues(t, newRepoID, after.RepoID)
assert.EqualValues(t, newName, after.Name)
assert.EqualValues(t, newVersion, after.Version)
assert.EqualValues(t, before.AgentLabels, after.AgentLabels)
}

View file

@ -14,7 +14,7 @@
token_salt: "832f8529db6151a1c3c605dd7570b58f" token_salt: "832f8529db6151a1c3c605dd7570b58f"
last_online: 0 last_online: 0
last_active: 0 last_active: 0
agent_labels: '[""]' agent_labels: '["woop", "doop"]'
created: 1716104432 created: 1716104432
updated: 1716104432 updated: 1716104432
deleted: ~ deleted: ~