From ca32cd3f8adfabc96efa4d6ff058a58266698744 Mon Sep 17 00:00:00 2001 From: Andreas Ahlenstorf Date: Thu, 18 Dec 2025 18:33:23 +0100 Subject: [PATCH] fix(actions): replace hardcoded with dynamically determined workflow directory (#10411) When manually triggering a Forgejo Actions workflow, Forgejo always assumed the workflow directory to be `.forgejo/workflows`, even if the workflows were found in `.gitea/workflows` or `.github/workflows`. As a consequence, the executed workflows were misidentified in the UI. Furthermore, the context variable `${{ forgejo.workflow_ref }}`, which contains the full path to the workflow file, pointed to a non-existent file. The workflow directory is now determined dynamically. Existing database entries are left unmodified. The screenshot shows the old behaviour for run 3 and the new, correct behaviour for run 4. ![workflow-dispatch](/attachments/563557a5-9cff-4c81-aec3-ffb18c831b52) The PR is marked as WIP because it requires https://codeberg.org/forgejo/forgejo/pulls/10276 to be merged first. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. ## Release notes - Bug fixes - [PR](https://codeberg.org/forgejo/forgejo/pulls/10411): fix(actions): replace hardcoded with dynamically determined workflow directory Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10411 Reviewed-by: Mathieu Fenniak Reviewed-by: Cyborus Co-authored-by: Andreas Ahlenstorf Co-committed-by: Andreas Ahlenstorf --- services/actions/workflows.go | 28 ++-- tests/integration/actions_trigger_test.go | 116 ++++++++++------- tests/integration/api_repo_actions_test.go | 142 +++++++++++++-------- 3 files changed, 177 insertions(+), 109 deletions(-) diff --git a/services/actions/workflows.go b/services/actions/workflows.go index df0590bd9a..1affce3379 100644 --- a/services/actions/workflows.go +++ b/services/actions/workflows.go @@ -41,10 +41,11 @@ func IsInputRequiredErr(err error) bool { } type Workflow struct { - WorkflowID string - Ref string - Commit *git.Commit - GitEntry *git.TreeEntry + WorkflowDirectory string + WorkflowID string + Ref string + Commit *git.Commit + GitEntry *git.TreeEntry } type InputValueGetter func(key string) string @@ -74,6 +75,10 @@ func resolveDispatchInput(key, value string, input act_model.WorkflowDispatchInp return value, nil } +func (entry *Workflow) WorkflowPath() string { + return entry.WorkflowDirectory + "/" + entry.WorkflowID +} + func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGetter, repo *repo_model.Repository, doer *user.User) (r *actions_model.ActionRun, j []string, err error) { content, err := actions.GetContentFromEntry(entry.GitEntry) if err != nil { @@ -85,7 +90,7 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette return nil, nil, err } - fullWorkflowID := ".forgejo/workflows/" + entry.WorkflowID + fullWorkflowID := entry.WorkflowPath() title := wf.Name if len(title) < 1 { @@ -137,7 +142,7 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette Repo: repo, OwnerID: repo.OwnerID, WorkflowID: entry.WorkflowID, - WorkflowDirectory: ".forgejo/workflows", + WorkflowDirectory: entry.WorkflowDirectory, TriggerUserID: doer.ID, TriggerUser: doer, Ref: entry.Ref, @@ -199,7 +204,7 @@ func GetWorkflowFromCommit(gitRepo *git.Repository, ref, workflowID string) (*Wo return nil, err } - _, entries, err := actions.ListWorkflows(commit) + workflowDirectory, entries, err := actions.ListWorkflows(commit) if err != nil { return nil, err } @@ -216,10 +221,11 @@ func GetWorkflowFromCommit(gitRepo *git.Repository, ref, workflowID string) (*Wo } return &Workflow{ - WorkflowID: workflowID, - Ref: ref, - Commit: commit, - GitEntry: workflowEntry, + WorkflowDirectory: workflowDirectory, + WorkflowID: workflowID, + Ref: ref, + Commit: commit, + GitEntry: workflowEntry, }, nil } diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index 9d46ef0cf0..8c05367770 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -796,58 +796,82 @@ func TestActionsCreateDeleteRefEvent(t *testing.T) { } func TestActionsWorkflowDispatch(t *testing.T) { + testCases := []struct { + name string + workflowID string + workflowDirectory string + }{ + { + name: "GitHub", + workflowID: "dispatch.yml", + workflowDirectory: ".github/workflows", + }, + { + name: "Gitea", + workflowID: "test.yml", + workflowDirectory: ".gitea/workflows", + }, + { + name: "Forgejo", + workflowID: "build.yml", + workflowDirectory: ".forgejo/workflows", + }, + } onApplicationRun(t, func(t *testing.T, u *url.URL) { - user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - // create the repo - repo, sha, f := tests.CreateDeclarativeRepo(t, user2, "repo-workflow-dispatch", - []unit_model.Type{unit_model.TypeActions}, nil, - []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: ".gitea/workflows/dispatch.yml", - ContentReader: strings.NewReader( - "name: test\n" + - "on: [workflow_dispatch]\n" + - "jobs:\n" + - " test:\n" + - " runs-on: ubuntu-latest\n" + - " steps:\n" + - " - run: echo helloworld\n", - ), - }, - }, - ) - defer f() + // create the repo + repo, sha, f := tests.CreateDeclarativeRepo(t, user2, "repo-workflow-dispatch", + []unit_model.Type{unit_model.TypeActions}, nil, + []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: fmt.Sprintf("%s/%s", testCase.workflowDirectory, testCase.workflowID), + ContentReader: strings.NewReader( + "name: test\n" + + "on: [workflow_dispatch]\n" + + "jobs:\n" + + " test:\n" + + " runs-on: ubuntu-latest\n" + + " steps:\n" + + " - run: echo helloworld\n", + ), + }, + }, + ) + defer f() - gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, repo) - require.NoError(t, err) - defer gitRepo.Close() + gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, repo) + require.NoError(t, err) + defer gitRepo.Close() - workflow, err := actions_service.GetWorkflowFromCommit(gitRepo, "main", "dispatch.yml") - require.NoError(t, err) - assert.Equal(t, "refs/heads/main", workflow.Ref) - assert.Equal(t, sha, workflow.Commit.ID.String()) + workflow, err := actions_service.GetWorkflowFromCommit(gitRepo, "main", testCase.workflowID) + require.NoError(t, err) + assert.Equal(t, "refs/heads/main", workflow.Ref) + assert.Equal(t, sha, workflow.Commit.ID.String()) - inputGetter := func(key string) string { - return "" + inputGetter := func(key string) string { + return "" + } + + var r *actions_model.ActionRun + var j []string + r, j, err = workflow.Dispatch(db.DefaultContext, inputGetter, repo, user2) + require.NoError(t, err) + + assert.Equal(t, 1, unittest.GetCount(t, &actions_model.ActionRun{RepoID: repo.ID})) + + assert.Equal(t, "test", r.Title) + assert.Equal(t, testCase.workflowID, r.WorkflowID) + assert.Equal(t, testCase.workflowDirectory, r.WorkflowDirectory) + assert.Equal(t, sha, r.CommitSHA) + assert.Equal(t, actions_module.GithubEventWorkflowDispatch, r.TriggerEvent) + assert.Len(t, j, 1) + assert.Equal(t, "test", j[0]) + }) } - - var r *actions_model.ActionRun - var j []string - r, j, err = workflow.Dispatch(db.DefaultContext, inputGetter, repo, user2) - require.NoError(t, err) - - assert.Equal(t, 1, unittest.GetCount(t, &actions_model.ActionRun{RepoID: repo.ID})) - - assert.Equal(t, "test", r.Title) - assert.Equal(t, "dispatch.yml", r.WorkflowID) - // .forgejo/workflows is wrong. It should be .gitea/workflows because the workflow is saved there during setup. - assert.Equal(t, ".forgejo/workflows", r.WorkflowDirectory) - assert.Equal(t, sha, r.CommitSHA) - assert.Equal(t, actions_module.GithubEventWorkflowDispatch, r.TriggerEvent) - assert.Len(t, j, 1) - assert.Equal(t, "test", j[0]) }) } diff --git a/tests/integration/api_repo_actions_test.go b/tests/integration/api_repo_actions_test.go index c831ac68b1..ae2591c0a8 100644 --- a/tests/integration/api_repo_actions_test.go +++ b/tests/integration/api_repo_actions_test.go @@ -18,6 +18,7 @@ import ( "forgejo.org/models/unittest" user_model "forgejo.org/models/user" api "forgejo.org/modules/structs" + "forgejo.org/modules/webhook" files_service "forgejo.org/services/repository/files" "forgejo.org/tests" @@ -102,19 +103,42 @@ func TestActionsAPISearchActionJobs_RepoRunnerAllPendingJobs(t *testing.T) { } func TestActionsAPIWorkflowDispatchReturnInfo(t *testing.T) { - onApplicationRun(t, func(t *testing.T, u *url.URL) { - workflowName := "dispatch.yml" - user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - token := getUserToken(t, user2.LowerName, auth_model.AccessTokenScopeWriteRepository) + testCases := []struct { + name string + workflowID string + workflowDirectory string + }{ + { + name: "GitHub", + workflowID: "dispatch.yml", + workflowDirectory: ".github/workflows", + }, + { + name: "Gitea", + workflowID: "test.yml", + workflowDirectory: ".gitea/workflows", + }, + { + name: "Forgejo", + workflowID: "build.yml", + workflowDirectory: ".forgejo/workflows", + }, + } - // create the repo - repo, _, f := tests.CreateDeclarativeRepo(t, user2, "api-repo-workflow-dispatch", - []unit_model.Type{unit_model.TypeActions}, nil, - []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: fmt.Sprintf(".forgejo/workflows/%s", workflowName), - ContentReader: strings.NewReader(`name: WD + onApplicationRun(t, func(t *testing.T, u *url.URL) { + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + token := getUserToken(t, user2.LowerName, auth_model.AccessTokenScopeWriteRepository) + + // create the repo + repo, _, f := tests.CreateDeclarativeRepo(t, user2, "api-repo-workflow-dispatch", + []unit_model.Type{unit_model.TypeActions}, nil, + []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: fmt.Sprintf("%s/%s", testCase.workflowDirectory, testCase.workflowID), + ContentReader: strings.NewReader(`name: WD on: [workflow-dispatch] jobs: t1: @@ -126,51 +150,65 @@ jobs: steps: - run: echo "test 2" `, + ), + }, + }, + ) + defer f() + + req := NewRequestWithJSON( + t, + http.MethodPost, + fmt.Sprintf( + "/api/v1/repos/%s/%s/actions/workflows/%s/dispatches", + repo.OwnerName, repo.Name, testCase.workflowID, ), - }, - }, - ) - defer f() + &api.DispatchWorkflowOption{ + Ref: repo.DefaultBranch, + ReturnRunInfo: true, + }, + ) + req.AddTokenAuth(token) - req := NewRequestWithJSON( - t, - http.MethodPost, - fmt.Sprintf( - "/api/v1/repos/%s/%s/actions/workflows/%s/dispatches", - repo.OwnerName, repo.Name, workflowName, - ), - &api.DispatchWorkflowOption{ - Ref: repo.DefaultBranch, - ReturnRunInfo: true, - }, - ) - req.AddTokenAuth(token) + res := MakeRequest(t, req, http.StatusCreated) + run := new(api.DispatchWorkflowRun) + DecodeJSON(t, res, run) - res := MakeRequest(t, req, http.StatusCreated) - run := new(api.DispatchWorkflowRun) - DecodeJSON(t, res, run) + assert.NotZero(t, run.ID) + assert.NotZero(t, run.RunNumber) + assert.Len(t, run.Jobs, 2) - assert.NotZero(t, run.ID) - assert.NotZero(t, run.RunNumber) - assert.Len(t, run.Jobs, 2) + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: run.ID}) + assert.Equal(t, "WD", actionRun.Title) + assert.Equal(t, repo.ID, actionRun.RepoID) + assert.Equal(t, repo.OwnerID, actionRun.OwnerID) + assert.Equal(t, testCase.workflowID, actionRun.WorkflowID) + assert.Equal(t, testCase.workflowDirectory, actionRun.WorkflowDirectory) + assert.Equal(t, user2.ID, actionRun.TriggerUserID) + assert.Zero(t, actionRun.ScheduleID) + assert.Equal(t, "refs/heads/main", actionRun.Ref) + assert.Equal(t, webhook.HookEventType("workflow_dispatch"), actionRun.Event) + assert.Equal(t, "workflow_dispatch", actionRun.TriggerEvent) - req = NewRequestWithJSON( - t, - http.MethodPost, - fmt.Sprintf( - "/api/v1/repos/%s/%s/actions/workflows/%s/dispatches", - repo.OwnerName, repo.Name, workflowName, - ), - &api.DispatchWorkflowOption{ - Ref: repo.DefaultBranch, - ReturnRunInfo: false, - }, - ) - req.AddTokenAuth(token) - res = MakeRequest(t, req, http.StatusNoContent) - body, err := io.ReadAll(res.Body) - require.NoError(t, err) - assert.Empty(t, body) // 204 No Content doesn't support a body, so should be empty + req = NewRequestWithJSON( + t, + http.MethodPost, + fmt.Sprintf( + "/api/v1/repos/%s/%s/actions/workflows/%s/dispatches", + repo.OwnerName, repo.Name, testCase.workflowID, + ), + &api.DispatchWorkflowOption{ + Ref: repo.DefaultBranch, + ReturnRunInfo: false, + }, + ) + req.AddTokenAuth(token) + res = MakeRequest(t, req, http.StatusNoContent) + body, err := io.ReadAll(res.Body) + require.NoError(t, err) + assert.Empty(t, body) // 204 No Content doesn't support a body, so should be empty + }) + } }) }