diff --git a/cmd/forgejo/actions.go b/cmd/forgejo/actions.go index 70f9452cb8..1560b10fac 100644 --- a/cmd/forgejo/actions.go +++ b/cmd/forgejo/actions.go @@ -86,6 +86,11 @@ func SubcmdActionsRegister(ctx context.Context) *cli.Command { Value: "", 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{ Name: "name", Value: "runner", @@ -133,6 +138,17 @@ func validateSecret(secret string) error { 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 { var cancel context.CancelFunc if !ContextGetNoInit(ctx) { @@ -153,9 +169,12 @@ func RunRegister(ctx context.Context, cliCtx *cli.Context) error { return err } scope := cliCtx.String("scope") - labels := cliCtx.String("labels") name := cliCtx.String("name") version := cliCtx.String("version") + labels, err := getLabels(cliCtx) + if err != nil { + return err + } // // There are two kinds of tokens @@ -179,7 +198,7 @@ func RunRegister(ctx context.Context, cliCtx *cli.Context) error { 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 { return fmt.Errorf("error while registering runner: %v", err) } diff --git a/cmd/forgejo/actions_test.go b/cmd/forgejo/actions_test.go new file mode 100644 index 0000000000..9a9edb7eb2 --- /dev/null +++ b/cmd/forgejo/actions_test.go @@ -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) + } + }) + } +} diff --git a/models/actions/forgejo.go b/models/actions/forgejo.go index 243262facd..29e8588e2c 100644 --- a/models/actions/forgejo.go +++ b/models/actions/forgejo.go @@ -14,7 +14,7 @@ import ( 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])) if err != nil { 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) runner = ActionRunner{ - UUID: uuidString, - TokenHash: hash, - TokenSalt: salt, + UUID: uuidString, + TokenHash: hash, + TokenSalt: salt, + AgentLabels: []string{}, } 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) + cols := []string{"name", "owner_id", "repo_id", "version"} runner.Name = name runner.OwnerID = ownerID runner.RepoID = repoID 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) } diff --git a/models/actions/forgejo_test.go b/models/actions/forgejo_test.go index a8583c3d00..1bc81e7130 100644 --- a/models/actions/forgejo_test.go +++ b/models/actions/forgejo_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestActions_RegisterRunner(t *testing.T) { +func TestActions_RegisterRunner_Token(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) ownerID := int64(0) repoID := int64(0) @@ -21,9 +21,127 @@ func TestActions_RegisterRunner(t *testing.T) { labels := []string{} name := "runner" 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.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") } + +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) +} diff --git a/models/fixtures/action_runner.yml b/models/fixtures/action_runner.yml index d2615f08eb..94deac998e 100644 --- a/models/fixtures/action_runner.yml +++ b/models/fixtures/action_runner.yml @@ -14,7 +14,7 @@ token_salt: "832f8529db6151a1c3c605dd7570b58f" last_online: 0 last_active: 0 - agent_labels: '[""]' + agent_labels: '["woop", "doop"]' created: 1716104432 updated: 1716104432 deleted: ~