#1563 made changes to the `exprparser` library to allow detecting access to `env.*` and `secrets.*`. Two bugs were discovered during failed refactoring efforts (#1566):
- `env.*` and `secrets.*` weren't detected as access; in the event that these expressions were used in an `if` block they wouldn't have caused Forgejo to defer execution to the runner rather than doing server-side `if` evaluation
- `toJSON(matrix)` outputs a wrapping `"Matrix"` element, which was missed when dynamic `runs-on` capabilities were added, but missed until tests triggered similar problems in `env`
More comprehensive unit test cases around both are fixed here. Practically speaking neither bug is likely to occur in reality.
<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- bug fixes
- [PR](https://code.forgejo.org/forgejo/runner/pulls/1572): <!--number 1572 --><!--line 0 --><!--description Zml4OiBleHByZXNzaW9uIGV2YWx1YXRpb24gZml4ZXMgZGlzY292ZXJlZCB3aGlsZSByZWZhY3RvcmluZyAnaWYnIGRldGVjdGlvbg==-->fix: expression evaluation fixes discovered while refactoring 'if' detection<!--description-->
<!--end release-notes-assistant-->
Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/1572
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.code.forgejo.org>
The intent of this change is to allow Forgejo to evaluate `if` without having to send jobs to a runner. When you send a job to a runner just for it to return "skipped!", it takes up 1 runner capacity for a `fetch_interval` period, which can be avoided if Forgejo can evaluate the `if` condition itself.
Creates a new `EvaluateIf` function in the jobparser library's `Job` which evaluates the `if` clause, with these conditions:
- Requires that the `Job` is parsed by `jobparser.Parse` and has parsing context, such as job inputs, job results. If `Job` was serialized to yaml, then deserialized, this information would be missing and `EvaluateIf` would return an error.
- Accessing `env` and `secrets` is prohibited, and returns an error. Forgejo will react to this specific error by falling back to the current behaviour: send the task to the runner for `if` to be evaluated on the runner.
- `env` -- Currently Forgejo Runner can have configured [environment variables](22ebc7d1ab/internal/pkg/config/config.example.yaml (L26-L32)). As these values wouldn't be known to Forgejo, it doesn't make sense to access `env` in an `if` block from Forgejo.
- `secrets` -- Currently Forgejo doesn't provide secrets to the `jobparser`. I think it's risky to change this -- giving this information to the jobparser opens the door for it to end up in a marshalled `SingleWorkflow` through some error or future change, and be stored in the database insecurely and unencrypted.
Forgejo's implementation using these new APIs is https://codeberg.org/forgejo/forgejo/pulls/13030.
<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- features
- [PR](https://code.forgejo.org/forgejo/runner/pulls/1563): <!--number 1563 --><!--line 0 --><!--description ZmVhdChqb2JwYXJzZXIpOiBpbXBsZW1lbnQgam9icGFyc2VyIGNhcGFiaWxpdHkgdG8gZXZhbHVhdGUgJ2lmJyBjbGF1c2Vz-->feat(jobparser): implement jobparser capability to evaluate 'if' clauses<!--description-->
<!--end release-notes-assistant-->
Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/1563
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.code.forgejo.org>
Reduce Forgejo Runner's dependency on go-git further by incorporating its gitignore parsing and matching functionality into Forgejo Runner's source tree. Various alternative libraries were considered. However, those are either abandoned or have compatibility issues, or both.
Some unused functions in `dir.go` have been removed. go-git's internal abstractions were replaced with functionality provided by Go's standard library.
The original code can be found in 5b47ae8a22/plumbing/format/gitignore.
<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- other
- [PR](https://code.forgejo.org/forgejo/runner/pulls/1497): <!--number 1497 --><!--line 0 --><!--description Y2hvcmU6IGluY29ycG9yYXRlIGdvLWdpdCdzIGdpdGlnbm9yZSBtYXRjaGluZw==-->chore: incorporate go-git's gitignore matching<!--description-->
<!--end release-notes-assistant-->
Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/1497
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.code.forgejo.org>
This PR contains two fixes to the production code:
- 20d56ace53adf26f66e7eb14f3f899087aec70eb fixes the `hashFile` function in an action misinterpreting paths on Windows
- 4e2609329dc2e3e630a67c3237e38a92dd7dc784 fixes the `validate` command outputting full paths due to OS-unspecific path trimming logic
The remaining changes adopt tests to properly deal with
- Docker with Linux containers not being available on standard Windows runners c492fe121e2f2d2eb6dbc8fd5b1cef543437fa99
- Windows paths (6781d823683428d5cf8b9bac4753a458882b5994), (8ca14d0f4d4274e808fb027e021bac701279381e), (eb70aca72c89c08cdc03c35dd26f3e57bdeaacac), (4e2609329dc2e3e630a67c3237e38a92dd7dc784), (c00d4e0a81634c098086b822884eb6b525ebe933)
- Revert a change causing a test-runner panic on Windows (6608fec705f7e7ed48c7ffed3839f2b4c4c1defb)
- Exclude the daemon suicide test because it cannot be properly observed on Windows (3ef33b93b6c3422ce47325bc5330d2d954998cf4)
or in one case separate dedicated Windows and Linux test setups (709a1d3fdbfc8a6149a57fcf00570e842ef4d040)
I tried to keep the changes as minimal as possible and focussed on getting the tests to run again.
I only added skips for tests that I could not get to run due to the Docker requirements, or the different shutdown behaviors on Linux and Windows.
In the future, I will investigate why the defensive `chdir` in `linux_container_environment_extensions_test.go` (6608fec705f7e7ed48c7ffed3839f2b4c4c1defb) is necessary and whether I can get all of the docker tests (c492fe121e2f2d2eb6dbc8fd5b1cef543437fa99) to run on GitHub Actions
The passing of all tests on windows has been confirmed [here](https://github.com/Crown0815/Forgejo-runner-windows-builder/actions/runs/19344650459).
<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- other
- [PR](https://code.forgejo.org/forgejo/runner/pulls/1152): <!--number 1152 --><!--line 0 --><!--description V2luZG93cy1zcGVjaWZpYy1maXhlcw==-->Windows-specific-fixes<!--description-->
<!--end release-notes-assistant-->
Co-authored-by: Felix Kröner <felix.kroener@bruker.com>
Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/1152
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.code.forgejo.org>
Co-authored-by: Crown0815 <crown0815@noreply.code.forgejo.org>
Co-committed-by: Crown0815 <crown0815@noreply.code.forgejo.org>
To prepare for a smooth merge in the runner codebase.
- run with --fix for gofumpt and golangci
- manual edits for
- disabling useless package naming warning
- rename variables that had underscore in their name
- remove trailing else at the end of a few functions
Reviewed-on: https://code.forgejo.org/forgejo/act/pulls/206
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
