[Proposal] Use actionlint to parse Actions workflow files instead of act

wolfogre commented 1 year ago

This idea was inspired during a discussion with the nektos maintainers team.

Background

Act runner depends on act to run jobs, as act provides the necessary infrastructure. However, Gitea also depends on Act because Gitea needs the models defined in Act to parse workflow files and assign tasks.

For example, Gitea needs to extract on from the workflow file to decide which event to trigger, and also runs-on of jobs to match runners.

Unfortunately, the models defined in act are not incomplete because some fields do not matter when running workflows locally (which is what act is designed to do), such as permissions and concurrency.

Solution

If we add full syntax support to act or other packages, we would be reinventing the wheel, because actionlint has already done it.

Although actionlint could be used as a command-line tool, it is also designed as a library. See the document which describes how to use actionlint as a Go library. Actually, act imports it too.

So we could replace act in Gitea with actionlint to parse workflow files.

Possible difficulties

The forked act has a new package act/pkg/jobparser to split jobs in single files. Like

name: test
jobs:
  job0:
    runs-on: ubuntu-latest
    steps:
      - run: uname -a
  job1:
    strategy:
      matrix:
        os: [ubuntu-22.04, ubuntu-20.04]
        version: [1.17, 1.18, 1.19]
    runs-on: ${{ matrix.os }}
    steps:
      - uses: actions/setup-go@v3
        with:
          go-version: ${{ matrix.version }}
      - run: uname -a && go version

# Will be splited into
---
name: test
jobs:
  job0:
    runs-on: ubuntu-latest
    steps:
      - run: uname -a
---
name: test
jobs:
  job1:
    name: job1 (ubuntu-20.04, 1.17)
    runs-on: ubuntu-20.04
    steps:
      - uses: actions/setup-go@v3
        with:
          go-version: ${{ matrix.version }}
      - run: uname -a && go version
    strategy:
      matrix:
        os:
          - ubuntu-20.04
        version:
          - 1.17
---
name: test
jobs:
  job1:
    name: job1 (ubuntu-20.04, 1.18)
    runs-on: ubuntu-20.04
    steps:
      - uses: actions/setup-go@v3
        with:
          go-version: ${{ matrix.version }}
      - run: uname -a && go version
    strategy:
      matrix:
        os:
          - ubuntu-20.04
        version:
          - 1.18
---
name: test
jobs:
  job1:
    name: job1 (ubuntu-20.04, 1.19)
    runs-on: ubuntu-20.04
    steps:
      - uses: actions/setup-go@v3
        with:
          go-version: ${{ matrix.version }}
      - run: uname -a && go version
    strategy:
      matrix:
        os:
          - ubuntu-20.04
        version:
          - 1.19
---
name: test
jobs:
  job1:
    name: job1 (ubuntu-22.04, 1.17)
    runs-on: ubuntu-22.04
    steps:
      - uses: actions/setup-go@v3
        with:
          go-version: ${{ matrix.version }}
      - run: uname -a && go version
    strategy:
      matrix:
        os:
          - ubuntu-22.04
        version:
          - 1.17
---
name: test
jobs:
  job1:
    name: job1 (ubuntu-22.04, 1.18)
    runs-on: ubuntu-22.04
    steps:
      - uses: actions/setup-go@v3
        with:
          go-version: ${{ matrix.version }}
      - run: uname -a && go version
    strategy:
      matrix:
        os:
          - ubuntu-22.04
        version:
          - 1.18
---
name: test
jobs:
  job1:
    name: job1 (ubuntu-22.04, 1.19)
    runs-on: ubuntu-22.04
    steps:
      - uses: actions/setup-go@v3
        with:
          go-version: ${{ matrix.version }}
      - run: uname -a && go version
    strategy:
      matrix:
        os:
          - ubuntu-22.04
        version:
          - 1.19

This is for distributing jobs to multiple runners. The jobs may run on different platforms and run concurrently.

It may be difficult to do it again with actionlint. A half-baked idea of mine is:

Don't split the workflow file into parts.
Pass the entire workflow file to the runner, but specify which job to run and which set of matrix to use.

IIRC, act supports specifying job but not matrix. So, there may be more work to do.

wolfogre commented 1 year ago

Originally posted by @ChristopherHX

I saw an issue about actionlint on gitea's issue tracker...

There are pros and cons for actionlint pros:

better errors

error using undefined yaml fields

cons:

undocumented features of GitHub Actions are shown as syntax errors

${{ insert }}, based on an action runner member unsupported feature, however available as of 2019 and official linter supports it there is also a js parser now under mit license (golang is private)

As far as I know it ignores case for case sensitive keys like on, jobs runs-on etc.

TheFox0x7 commented 1 year ago

I assume by js parser he means one in https://github.com/actions/languageservices which might be useful for validating before commit? Also what is insert? It's as you say undocumented and I have no clue what it could be.

Regarding ignoring case - it's changed on main branch of actionlint, but not yet released.

ChristopherHX commented 1 year ago

Yes I mean that repo by js parser.

You would have to add the gitea context everywhere in the workflowv1 json and tweak it to allow gitea action extensions...

What is insert?

it inserts a mapping into a mapping from an expression result. Due to limitations of the workflow validator values on the right side may have to be expressions instead of a yaml mapping.

Yes I read the source code of action/runner carefully, it is somewhat documented there.

on: push
jobs:
  _:
    strategy:
      matrix:
        os:
        - ubuntu-latest
        - macos-latest
        include:
        - os: ubuntu-latest # Only ubuntu-latest has an extra env var MYVAR
          env:
            MYVAR: myval
            MYVAR2: myval2
    env:
      SOME_ENV: B
      ${{ insert }}: ${{ matrix.env || fromjson('{}') }}
    runs-on: ${{ matrix.os }}
    steps:
    - run: env

This example won't work in nektos/act.

insert has been implemented under jobs.<id>.strategy.matrix of nektos/act.

BTW this insert has been copied from the original Azure Pipelines Code Base, it is also undocumented there.

ChristopherHX commented 1 year ago

Here are the references of the actions/runner: https://github.com/actions/runner/blob/143639ddacc089464ea97d9dba44c3068f156d3f/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateUnraveler.cs#L440

                    // Mapping key, beginning an "insert" mapping insertion
                    // For example:
                    //   - job: a
                    //     variables:
                    //       ${{ insert }}: ${{ parameters.jobVariables }}

However that example still uses azure pipelines yaml syntax

ChristopherHX commented 1 year ago

Seems like action lint doesn't validate workflow keys of expression enabled objects.

So it doesn't make ${{ insert }} an error as of now.

In Github Actions is the following also possible (jobs.<id>.env allows expressions recursively and not only for the value)

Actionlint handles this one perfectly fine

on: push
jobs:
  _:
    strategy:
      matrix:
        include:
        - os: ubuntu-latest
          env:
            MYVAR: myval
            MYVAR2: myval2
    env: ${{ matrix.env }}
    runs-on: ${{ matrix.os }}
    steps:
    - run: env

However doesn't parse ${{ matrix.key }}, but this could be fixed.

on: push
jobs:
  _:
    strategy:
      matrix:
        include:
        - os: ubuntu-latest
          key: MY_ENV
          value: MY_VAL
    env:
      SOME_ENV: B
      ${{ matrix.key }}: ${{ matrix.val }}
    runs-on: ${{ matrix.os }}
    steps:
    - run: env

All in all Actionlint is pretty good, while having it's own parser.

TheFox0x7 commented 1 year ago

So it doesn't make ${{ insert }} an error as of now.

It does when a expression linter rule is applied, but that's after parsing and we have no reason to use it.

Since you probably know - when are expressions expanded? Taking your example:

on: push
jobs:
  _:
    strategy:
      matrix: #this is loaded before env
        include:
        - os: ubuntu-latest
          key: MY_ENV
          value: MY_VAL
    env: #I assume loading env resolves the expressions server side?
      SOME_ENV: B
      ${{ matrix.key }}: ${{ matrix.val }}
    runs-on: ${{ matrix.os }} #Same here
    steps: # And all steps are probably ignored on a server and resolved on the runner? 
    - run: env

ChristopherHX commented 1 year ago

It does when a expression linter rule is applied

Isn't it enabled by default in the online playground? I only see errors and syntax highlighting about expressions in yaml values (not keys).

matrix: #this is loaded before env

Yes and this is server side expanded, then the strategy is expanded to a list of sibling jobs.

However the whole strategy key is an expandable expression. Including all keys and values of sub objects.

The last time I looked at the Gitea Actions code, this is not expanded. Only processed to create sibling jobs.

env: #I assume loading env resolves the expressions server side?

No, the runner side expands the expression. The runner get's a list of parsed MappingToken of each workflow level.

I reused actions/runner to enable GitHub Actions on my Gitea Server in 2021. However it has flaws with securty, db support and webpage design. So I know exactly what goes to the actions/runner and what not.

runs-on: ${{ matrix.os }} #Same here

yes this is server side, Gitea Actions is expanding it somehow already on the server before scheduling the job

You can look at the log of my GitHub Actions Emulator to see, when which expression is expanded on the server side.

The order of some fields doesn't matter while expanding expression, while it is important for other fields.

TheFox0x7 commented 8 months ago

I was tinkering with this for a long while and have some "drafts", though I haven't tested them at yet.

I got stuck (unsurprisingly, and multiple times) at making a jobparser version based on actionlint. Currently, I'm stuck at the following:

As far as I know, gitea splits the job when the workflow is triggered and inserts the number of runs into the database. This works for simple jobs with no matrix. If the matrix is expanded into more jobs (or runs however you call them) we get more entries. The issue is, if we don't know about the amount of jobs that the matrix would expand to - we cannot insert them. Picture a case when someone decides one of matrix label in the output of another job, when this one depends on - this is valid because strategy supports needs context. We know that there will be at least one job - but we almost cannot touch it until the prerequisite finishes.

I'm thinking about something like this: 1) Process only the workflow, check the jobs for needs: 1) If needs are empty, we can add the job to the database as we can resolve all context that we have to. 2) If not store it until the needed jobs are completed at which point process them with context. 2) When handling job report, check if this unlocks another job to be processed

Here is a rough concept I have:

Make a middle layer in the database. Idea being that since the job!=run, we can split them into separate tables (one-to-many relations): Workflows->Jobs->Runs

Parsing workflow would create a finite amount of jobs equal to the jobs in the workflow - I don't have any concrete proposal for the layout of such table, but it could just be a storage for the job metadata (is it a matrix job, is it finished, what permissions would it need, concurrency markers, etc.) kinda like it's now except it's not used for actual runs of the job. Next from the job we could create a run entry for processing and here we actually can expand the matrix properly into separate jobs.

There are a lot of issues with this that I can think of:

duplication of yamls (not expanded in the job, expanded n-times in the run)
lot of work for a workflow with one job and no matrix
migration to this system might be complicated (never done it though)

If it was possible, I'd like to combine this move with outsourcing steps to the runner #24604 - to avoid even thinking about how to port step generation to this.

Alternatively, we could try to insert the job with the matrix and later expand it... but I feel like this would end up being more complex to track.

I'm open to different/better ideas though. Maybe someone here sees a less complex way of handling this.

go-gitea / gitea