mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2025-12-18 22:36:08 -05:00
fix(actions): replace hardcoded with dynamically determined workflow directory (#10411)
Some checks are pending
Integration tests for the release process / release-simulation (push) Waiting to run
/ release (push) Waiting to run
testing-integration / test-unit (push) Waiting to run
testing-integration / test-sqlite (push) Waiting to run
testing-integration / test-mariadb (v10.6) (push) Waiting to run
testing-integration / test-mariadb (v11.8) (push) Waiting to run
testing / backend-checks (push) Waiting to run
testing / frontend-checks (push) Waiting to run
testing / test-unit (push) Blocked by required conditions
testing / test-e2e (push) Blocked by required conditions
testing / test-remote-cacher (redis) (push) Blocked by required conditions
testing / test-remote-cacher (valkey) (push) Blocked by required conditions
testing / test-remote-cacher (garnet) (push) Blocked by required conditions
testing / test-remote-cacher (redict) (push) Blocked by required conditions
testing / test-mysql (push) Blocked by required conditions
testing / test-pgsql (push) Blocked by required conditions
testing / test-sqlite (push) Blocked by required conditions
testing / security-check (push) Blocked by required conditions
Some checks are pending
Integration tests for the release process / release-simulation (push) Waiting to run
/ release (push) Waiting to run
testing-integration / test-unit (push) Waiting to run
testing-integration / test-sqlite (push) Waiting to run
testing-integration / test-mariadb (v10.6) (push) Waiting to run
testing-integration / test-mariadb (v11.8) (push) Waiting to run
testing / backend-checks (push) Waiting to run
testing / frontend-checks (push) Waiting to run
testing / test-unit (push) Blocked by required conditions
testing / test-e2e (push) Blocked by required conditions
testing / test-remote-cacher (redis) (push) Blocked by required conditions
testing / test-remote-cacher (valkey) (push) Blocked by required conditions
testing / test-remote-cacher (garnet) (push) Blocked by required conditions
testing / test-remote-cacher (redict) (push) Blocked by required conditions
testing / test-mysql (push) Blocked by required conditions
testing / test-pgsql (push) Blocked by required conditions
testing / test-sqlite (push) Blocked by required conditions
testing / security-check (push) Blocked by required conditions
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.

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/<pull request number>.md` to be be used for the release notes instead of the title.
<!--start release-notes-assistant-->
## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Bug fixes
- [PR](https://codeberg.org/forgejo/forgejo/pulls/10411): <!--number 10411 --><!--line 0 --><!--description Zml4KGFjdGlvbnMpOiByZXBsYWNlIGhhcmRjb2RlZCB3aXRoIGR5bmFtaWNhbGx5IGRldGVybWluZWQgd29ya2Zsb3cgZGlyZWN0b3J5-->fix(actions): replace hardcoded with dynamically determined workflow directory<!--description-->
<!--end release-notes-assistant-->
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10411
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Reviewed-by: Cyborus <cyborus@disroot.org>
Co-authored-by: Andreas Ahlenstorf <andreas@ahlenstorf.ch>
Co-committed-by: Andreas Ahlenstorf <andreas@ahlenstorf.ch>
This commit is contained in:
parent
8ca4440cb1
commit
ca32cd3f8a
3 changed files with 177 additions and 109 deletions
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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])
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue