feat: allow inputs in runs-on

aahlenst commented

2025-11-01 15:39:04 +00:00

Member

Allows referencing the inputs context in jobs.<job_id>.runs-on. That is a prerequisite for enabling users to enter a label in the Forgejo UI during workflow_dispatch.

Example:

name: test
on:
  workflow_dispatch:
    inputs:
      image:
        required: true
        type: string

jobs:
  test:
    runs-on: ${{ inputs.image }}
    steps:
      - run: echo "Running on ${{ inputs.image }}"

While Forgejo already has the necessary UI to enter the variable, it does not pass the context to the jobparser. That still has to be done.

Apart from enabling inputs, this PR expands the test coverage of the contexts supported by runs-on.

features
- PR: feat: allow inputs in runs-on

Allows referencing the `inputs` context in `jobs.<job_id>.runs-on`. That is a prerequisite for enabling users to enter a label in the Forgejo UI during `workflow_dispatch`. Example: ```yaml name: test on: workflow_dispatch: inputs: image: required: true type: string jobs: test: runs-on: ${{ inputs.image }} steps: - run: echo "Running on ${{ inputs.image }}" ``` While Forgejo already has the necessary UI to enter the variable, it does not pass the context to the jobparser. That still has to be done. Apart from enabling `inputs`, this PR expands the test coverage of the contexts supported by `runs-on`.   - features - [PR](https://code.forgejo.org/forgejo/runner/pulls/1117): feat: allow inputs in runs-on

aahlenst added 1 commit

2025-11-01 15:39:04 +00:00

feat: allow inputs in runs-on

checks / validate pre-commit-hooks file (pull_request) Successful in 34s

Details

checks / validate mocks (pull_request) Successful in 50s

Details

checks / build and test (pull_request) Successful in 2m39s

Details

checks / runner exec tests (pull_request) Successful in 31s

Details

checks / runner integration tests (pull_request) Successful in 5m1s

Details

checks / integration tests (pull_request) Successful in 10m45s

Details

issue-labels / release-notes (pull_request_target) Successful in 7s

Details

cascade / debug (pull_request_target) Has been skipped

Details

cascade / end-to-end (pull_request_target) Successful in 10s

Details

cascade / forgejo (pull_request_target) Successful in 2m43s

Details

4636e542cf

proton-ab commented

2025-11-01 23:28:03 +00:00

First-time contributor

How does this affect Forgejo showing icon near workflow when no runner required by workflow is online? Is the icon always present?

earl-warren added the

Kind/Enhancement

run-end-to-end-tests

labels

2025-11-02 08:26:23 +00:00

cascading-pr referenced this pull request from actions/setup-forgejo

2025-11-02 08:27:39 +00:00

cascading-pr from https://code.forgejo.org/forgejo/runner refs/pull/1117/head to forgejo/runner-1117 #734

cascading-pr commented

2025-11-02 08:27:40 +00:00

Contributor

cascading-pr updated at actions/setup-forgejo#734

cascading-pr updated at https://code.forgejo.org/actions/setup-forgejo/pulls/734

earl-warren commented

2025-11-02 08:48:43 +00:00

Contributor

I think this should be fine as Forgejo can statically evaluate the workflow before it is submitted to the runner.

 			runsOn := origin.GetJob(id).RunsOn()
 			for i, v := range runsOn {
 				runsOn[i] = evaluator.Interpolate(v)
 			}
 			job.RawRunsOn = encodeRunsOn(runsOn)
 			swf := &SingleWorkflow{
 				Name:     workflow.Name,
 				RawOn:    workflow.RawOn,
 				Env:      workflow.Env,
 				Defaults: workflow.Defaults,
 			}
 			if err := swf.SetJob(id, job); err != nil {
 				return nil, fmt.Errorf("SetJob: %w", err)
 			}
 			ret = append(ret, swf)

Since runs-on is already interpolated, it will be replaced with the result, the only thing missing is the input.

@proton-ab Forgejo side workflows that generate multiple runs are first transformed in a series of workflows via jobparser, each of which is exactly one run. So, as long as runs-on is replaced with the actual value statically evaluated, it will be fine.

Since I overlooked the dynamic aspect causing problem with this other proposed change you authored, I would welcome a second opinion from @mfenniak

I think this should be fine as Forgejo can statically evaluate the workflow before it is submitted to the runner. https://code.forgejo.org/forgejo/runner/src/commit/41c1558fb5f8bcd57e086fbc567079c0bd35a682/act/jobparser/jobparser.go#L60-L74 Since `runs-on` is already interpolated, it will be replaced with the result, the only thing missing is the input. @proton-ab Forgejo side workflows that generate multiple runs are first transformed in a series of workflows via jobparser, each of which is exactly one run. So, as long as `runs-on` is replaced with the actual value statically evaluated, it will be fine. Since I overlooked the dynamic aspect causing problem with [this other proposed change you authored](https://code.forgejo.org/forgejo/runner/pulls/1114), I would welcome a second opinion from @mfenniak

mfenniak commented

2025-11-02 16:17:06 +00:00

Owner

Unfortunately the code in Forgejo doesn't seem to work that way. While running actions work as you're describing, @earl-warren, displaying the Actions list actually parses the workflow file in order to determine if the no_matching_online_runner_helper (No matching online runner with label: %s) label is displayed:

codeberg.org/forgejo/forgejo@c3412d0579/routers/web/repo/actions/actions.go (L114-L148)

Theoretically this could be updated on the Forgejo side. Rather than parsing the workflow, it could search the database action_run_job for runs that have been performed (in some time range?) with that workflow file and check their defined runs_on, and then display an error if there are no runners available. 🤔

But as it is, this seems incompatible with Forgejo's assumptions about runs-on being able to be evaluated straight from the workflow file.

Unfortunately the code in Forgejo doesn't seem to work that way. While *running* actions work as you're describing, @earl-warren, displaying the Actions list actually parses the workflow file in order to determine if the `no_matching_online_runner_helper` (`No matching online runner with label: %s`) label is displayed: https://codeberg.org/forgejo/forgejo/src/commit/c3412d0579acfc702224f1785fc44e5695daee90/routers/web/repo/actions/actions.go#L114-L148 Theoretically this could be updated on the Forgejo side. Rather than parsing the workflow, it could search the database `action_run_job` for runs that have been performed (in some time range?) with that workflow file and check their defined `runs_on`, and then display an error if there are no runners available. 🤔 But as it is, this seems incompatible with Forgejo's assumptions about `runs-on` being able to be evaluated straight from the workflow file.

proton-ab commented

2025-11-02 18:47:30 +00:00

First-time contributor

Perhaps we could change where the warning icon is displayed, from having it on workflow list, to showing it near name on active action that is already in waiting state (hence it was evaluated by runner beforehand)? Not sure if it would require changes in how runner communicates with Forgejo though.

This solution would also hide warning icon in environments where runner is dynamically spawned in response to needs from autoscaler software.

Perhaps we could change where the warning icon is displayed, from having it on workflow list, to showing it near name on active action that is already in waiting state (hence it was evaluated by runner beforehand)? Not sure if it would require changes in how runner communicates with Forgejo though. This solution would also hide warning icon in environments where runner is dynamically spawned in response to needs from autoscaler software.

proton-ab commented

2025-11-02 18:50:11 +00:00

First-time contributor

Wait, it seems that we already skip evaluating of expressions for runs-on?

codeberg.org/forgejo/forgejo@c3412d0579/routers/web/repo/actions/actions.go (L134)

Wait, it seems that we already skip evaluating of expressions for `runs-on`? https://codeberg.org/forgejo/forgejo/src/commit/c3412d0579acfc702224f1785fc44e5695daee90/routers/web/repo/actions/actions.go#L134

aahlenst commented

2025-11-02 18:50:56 +00:00

Author

Member

This comment suggests that not showing the tool tip when runs-on contains expressions is expected.

Theoretically this could be updated on the Forgejo side. Rather than parsing the workflow, it could search the database action_run_job for runs that have been performed (in some time range?) with that workflow file and check their defined runs_on, and then display an error if there are no runners available. 🤔

If you always want to be able to display whether a runner is available or not, looking at runs-on after it has been evaluated is a must. I have GitHub Actions workflows whose set of runs-on labels changes with every run because the labels are used to provision an on-demand runner for that specific run only.

[This comment](https://codeberg.org/forgejo/forgejo/src/commit/c3412d0579acfc702224f1785fc44e5695daee90/routers/web/repo/actions/actions.go#L135-L137) suggests that not showing the tool tip when `runs-on` contains expressions is expected. > Theoretically this could be updated on the Forgejo side. Rather than parsing the workflow, it could search the database `action_run_job` for runs that have been performed (in some time range?) with that workflow file and check their defined `runs_on`, and then display an error if there are no runners available. :thinking: If you always want to be able to display whether a runner is available or not, looking at `runs-on` after it has been evaluated is a must. I have GitHub Actions workflows whose set of `runs-on` labels changes with every run because the labels are used to provision an on-demand runner for that specific run only.

mfenniak commented

2025-11-02 21:10:09 +00:00

Owner

Ah... good spot, re: it's already skipped. That seems unfortunate as it would sure be hard to diagnose why an action isn't running. That isn't quite in the scope of this PR then, but it deserves a little criticism out to the world in general; "when my runs-on is complicated I lose the ability to find out if I don't have a runner for it, which is more likely the time that I need that ability".

I don't think that's a blocker for this PR.

This PR seems good to me. The only thing that prevents me from ✅ on it is that I'd like to hear that it's been integrated into Forgejo and tested end-to-end. Typically I do this by modifying go.mod and adding a new replace directive (eg. codeberg.org/forgejo/forgejo@c3412d0579/go.mod (L281)) that points at my local forgejo-runner checkout, then running a development Forgejo. Then the testing I'd expect is to create a test repo, register a runner, and use a workflow dispatch to create an input runs-on -- basically exercise the capability. The automated testing here just can't cover the whole scope of potential change, so I think that manual test is warranted before merging here, waiting for a release, upgrading Forgejo, and then finding something that prevents it from working as expected.

Ah... good spot, re: it's already skipped. That seems unfortunate as it would sure be hard to diagnose why an action isn't running. That isn't quite in the scope of this PR then, but it deserves a little criticism out to the world in general; "when my `runs-on` is complicated I lose the ability to find out if I don't have a runner for it, which is more likely the time that I need that ability". I don't think that's a blocker for this PR. This PR seems good to me. The only thing that prevents me from ✅ on it is that I'd like to hear that it's been integrated into Forgejo and tested end-to-end. Typically I do this by modifying `go.mod` and adding a new `replace` directive (eg. https://codeberg.org/forgejo/forgejo/src/commit/c3412d0579acfc702224f1785fc44e5695daee90/go.mod#L281) that points at my local `forgejo-runner` checkout, then running a development Forgejo. Then the testing I'd expect is to create a test repo, register a runner, and use a workflow dispatch to create an input runs-on -- basically exercise the capability. The automated testing here just can't cover the whole scope of potential change, so I think that manual test is warranted before merging here, waiting for a release, upgrading Forgejo, and then finding something that prevents it from working as expected.

aahlenst commented

2025-11-02 22:01:03 +00:00

Author

Member

The automated testing here just can't cover the whole scope of potential change, so I think that manual test is warranted before merging here, waiting for a release, upgrading Forgejo, and then finding something that prevents it from working as expected.

I have tested it locally. However, I'm not sure I covered every aspect of it. So it's probably best to wait until I have the PR ready for Forgejo. Then, you can look at the code and see for yourself whether I missed something or not. In any case, it will not be a perfect solution. For example, inputs should also work with reusable workflows. It does not and I have no idea why. But I prefer tiny steps over fixing every problem at once.

> The automated testing here just can't cover the whole scope of potential change, so I think that manual test is warranted before merging here, waiting for a release, upgrading Forgejo, and then finding something that prevents it from working as expected. I have tested it locally. However, I'm not sure I covered every aspect of it. So it's probably best to wait until I have the PR ready for Forgejo. Then, you can look at the code and see for yourself whether I missed something or not. In any case, it will not be a perfect solution. For example, `inputs` should also work with reusable workflows. It does not and I have no idea why. But I prefer tiny steps over fixing every problem at once.

aahlenst commented

2025-11-03 14:06:39 +00:00

Author

Member