Skip to content

Commit

Permalink
Refactor Workflow Linter (#231)
Browse files Browse the repository at this point in the history
* Refactoring the linter into an opinionated reformatter

* migrating how the rules are created for a more streamlined experience

* removing the keyword from the step model since there are no linting tests run on it or any of it's children

* Switch from pydantic to dataclasses

* Update to use the correct loader (Model.from_dict({})). Add extra data tests

* Got to a somewhat working breakdown of the rules (not including checking the actions)

* Major additions, test writing, and rule import framework

* update pip lockfile

* Build out the majority of the rules

* Add sub commands to manage the list of approved actions

* Add test coverage and get test coverage of the rules to 100%

* Update Python version in lint-ci to match refactored linter

* Removing old files. Update verbosity

* Finish in-code documentation

* Run black over the code

* Clean up the unused tests

* Expanded docs on how to add a new Rule

* Remove old linter

* First linting error fixes

* Fix all non-rule linter issues

* Switch to the cleaner json.load() function

* Fix unit tests

* Fix linting issues on job_environment_prefix

* Fix linting issues for RuleNameCapitalized

* Fix linting warnings for RuleNameExists

* Fix unit tests. Time to add some githooks...

* Update with linter findings for RuleStepUsesApproved

* Remove 'message' attribute from Rule base class. Fix linting findings for RuleStepPinned

* Fix linter findings

* Finish linter finding cleanup

* Reformat

* Fix type hinting

* Move lint-workflow to vs directory restore lint-workflow

* Fix tests after type fixes. Add tests to CI

* Add new workflow path to the PR trigger

* Update trigger to all pull_request events instead of push events

* Update workflow job names to be more helpful

* Switch back to python 3.9 for v1 linter

* Fix edge case where 'env' doesn't exist in a job

* Remove the end-to-end testing since it fails (as expected) and prevents a successful CI run

* Working on packaging with Hatch

* Update to Nix Unstable packages to allow for editable paths

* Updating file structure. Tests passing

* Got to installable and importable module

* Switch setting from a python file to a yaml file

* Update README with the new yaml settings

* Debugging E2E tests. Need to handle creating the Jobs that don't have Steps (see test_a.yaml)

* Save the latest vim session

* Fixed the complex workflow loading

* Update the JobRunnerVersionPinned rule to support callable workflows

* Fix the lint command to be compatible with callable workflows

* Format code and fix type annotations

* Clean up of local opinionated development environment

* Moving actions sub command to be in beta since we haven't decided to adopt that pattern

* Update all spelling errors identified and path issues (Thanks @vgrassia!)

* Add hacky safety measure to alert on GitHub Response Schema changes

* Fixed spelling issues
  • Loading branch information
joseph-flinn authored Mar 7, 2024
1 parent 631b9c1 commit 4a07a79
Show file tree
Hide file tree
Showing 55 changed files with 4,940 additions and 3 deletions.
35 changes: 32 additions & 3 deletions .github/workflows/lint-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
name: CI-Lint

on:
push:
pull_request:
paths:
- "lint-workflow/**"
- "lint-workflow-v2/**"
workflow_dispatch: {}

jobs:
CI:
name: CI
ci-lint:
name: CI workflow-linter (v1)
runs-on: ubuntu-22.04
steps:
- name: Checkout
Expand All @@ -31,3 +32,31 @@ jobs:
- name: Test lint
working-directory: lint-workflow
run: pipenv run pytest tests


ci-lint-v2:
name: CI workflow-linter (v2)
runs-on: ubuntu-22.04
steps:
- name: Checkout
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1

- name: Set up Python
uses: actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c # v5.0.0
with:
python-version: "3.11"

- name: Install dependencies
working-directory: lint-workflow-v2
run: |
python -m pip install --upgrade pip
pip install pipenv
pipenv install --dev
- name: Test lint
working-directory: lint-workflow-v2
run: pipenv run pytest tests --cov=src

- name: Check type hinting
working-directory: lint-workflow-v2
run: pipenv run pytype src
7 changes: 7 additions & 0 deletions lint-workflow-v2/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.coverage
dist

## Dev Environments
Session.vim
flake.nix
flake.lock
36 changes: 36 additions & 0 deletions lint-workflow-v2/.yamllint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---

extends: default

rules:
braces:
level: warning
brackets:
level: warning
colons:
level: warning
commas:
level: warning
comments:
min-spaces-from-content: 1
empty-lines:
level: warning
hyphens:
level: warning
indentation:
level: warning
spaces: 2
key-duplicates:
level: warning
line-length:
level: warning
max: 120
new-line-at-end-of-file:
level: warning
new-lines:
level: warning
trailing-spaces:
level: warning
truthy:
check-keys: false
level: warning
24 changes: 24 additions & 0 deletions lint-workflow-v2/Pipfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
pyyaml = "*"
urllib3 = "*"
pydantic = "*"
"ruamel.yaml" = "*"
dataclasses-json = "*"

[dev-packages]
black = "*"
pytest = "*"
coverage = "*"
pytest-cov = "*"
pylint = "*"
pytype = "*"
hatchling = "*"
build = "*"

[requires]
python_version = "3.11"
830 changes: 830 additions & 0 deletions lint-workflow-v2/Pipfile.lock

Large diffs are not rendered by default.

146 changes: 146 additions & 0 deletions lint-workflow-v2/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
# Bitwarden Workflow Linter

## Installation

## PyPi
```
Not yet implemented
```

### Locally
```
git clone [email protected]:bitwarden/gh-actions.git
cd gh-actions/lint-workflow-v2
pip install -e .
```

## Usage
### Setup settings.yaml

If a non-default configuration is desired (different than `src/bitwarden_workflow_linter/default_settings.yaml`), copy
the below and create a `settings.yaml` in the directory that `bwwl` will be running from.

```yaml
enabled_rules:
- bitwarden_workflow_linter.rules.name_exists.RuleNameExists
- bitwarden_workflow_linter.rules.name_capitalized.RuleNameCapitalized
- bitwarden_workflow_linter.rules.pinned_job_runner.RuleJobRunnerVersionPinned
- bitwarden_workflow_linter.rules.job_environment_prefix.RuleJobEnvironmentPrefix
- bitwarden_workflow_linter.rules.step_pinned.RuleStepUsesPinned

approved_actions_path: default_actions.json
```
```
usage: bwwl [-h] [-v] {lint,actions} ...

positional arguments:
{lint,actions}
lint Verify that a GitHub Action Workflow follows all of the Rules.
actions Add or Update Actions in the pre-approved list.

options:
-h, --help show this help message and exit
-v, --verbose
```

## Development
### Requirements

- Python 3.11
- pipenv

### Setup

```
pipenv install --dev
pipenv shell
```

### Testing

All built-in `src/bitwarden_workflow_linter/rules` should have 100% code coverage and we should shoot for an overall coverage of 80%+.
We are lax on the
[imperative shell](https://www.destroyallsoftware.com/screencasts/catalog/functional-core-imperative-shell)
(code interacting with other systems; ie. disk, network, etc), but we strive to maintain a high coverage over the
functional core (objects and models).

```
pipenv shell
pytest tests --cov=src
```

### Code Reformatting

We adhere to PEP8 and use `black` to maintain this adherence. `black` should be run on any change being merged
to `main`.

```
pipenv shell
black .
```

### Linting

We loosely use [Google's Python style guide](https://google.github.io/styleguide/pyguide.html), but yield to
`black` when there is a conflict

```
pipenv shell
pylint --rcfile pylintrc src/ tests/
```

### Add a new Rule

A new Rule is created by extending the Rule base class and overriding the `fn(obj: Union[Workflow, Job, Step])` method.
Available attributes of `Workflows`, `Jobs` and `Steps` can be found in their definitons under `src/models`.

For a simple example, we'll take a look at enforcing the existence of the `name` key in a Job. This is already done by
default with the src.rules.name_exists.RuleNameExists, but provides a simple enough example to walk through.

```python
from typing import Union, Tuple

from ..rule import Rule
from ..models.job import Job
from ..models.workflow import Workflow
from ..models.step import Step
from ..utils import LintLevels, Settings


class RuleJobNameExists(Rule):
def __init__(self, settings: Settings = None) -> None:
self.message = "name must exist"
self.on_fail: LintLevels = LintLevels.ERROR
self.compatibility: List[Union[Workflow, Job, Step]] = [Job]
self.settings: Settings = settings

def fn(self, obj: Job) -> Tuple[bool, str]:
"""<doc block goes here> """
if obj.name is not None:
return True, ""
return False, self.message
```

By default, a new Rule needs five things:

- `self.message`: The message to return to the user on a lint failure
- `self.on_fail`: The level of failure on a lint failure (NONE, WARNING, ERROR).
NONE and WARNING will exit with a code of 0 (unless using `strict` mode for WARNING).
ERROR will exit with a non-zero exit code
- `self.compatibility`: The list of objects this rule is compatible with. This is used to create separate instances of
the Rule for each object in the Rules collection.
- `self.settings`: In general, this should default to what is shown here, but allows for overrides
- `self.fn`: The function doing the actual work to check the object and enforce the standard.

`fn` can be as simple or as complex as it needs to be to run a check on a _single_ object. This linter currently does
not support Rules that check against multiple objects at a time OR file level formatting (one empty between each step or
two empty lines between each job)


### ToDo

- [ ] Add Rule to assert correct format for single line run

71 changes: 71 additions & 0 deletions lint-workflow-v2/Taskfile.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# https://taskfile.dev

version: '3'

tasks:
fmt:
silent: true
cmds:
- pipenv run black .

lint:
silent: true
cmds:
- pipenv run pylint --rcflie pylintrc {{.CLI_ARGS}}

type:
silent: true
cmds:
- pipenv run pytype src

update:
silent: true
cmds:
- |
deps=$(pipenv requirements --exclude-markers | tail -n +2 | awk '{print "\t\""$0"\","}')
export DEPS=$(printf "$deps")
envsubst < pyproject.toml.tpl > pyproject.toml
install:
silent: true
cmds:
- task: update
- pipenv run python -m pip install -e .

test:unit:
cmds:
- pipenv run pytest tests

test:unit:single:
cmds:
- pipenv run pytest {{.CLI_ARGS}}

test:cov:
cmds:
- pipenv run pytest --cov-report term --cov=src tests

test:cov:detailed:
cmds:
- pipenv run pytest --cov-report term-missing --cov=src tests

test:e2e:lint:
cmds:
- pipenv run bwwl lint --files tests/fixtures

test:e2e:lint:single:
cmds:
- pipenv run bwwl lint --files tests/fixtures/test_a.yml

test:e2e:actions:add:
cmds:
- pipenv run bwwl actions --output test.json add bitwarden/sm-action

test:e2e:actions:update:
cmds:
- pipenv run bwwl actions --output test.json update

dist:
silent: true
cmds:
- task: update
- pipenv run python -m build
32 changes: 32 additions & 0 deletions lint-workflow-v2/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: 'Lint Workflow'
description: 'Lints GitHub Actions Workflow'
inputs:
workflows:
description: "Path to workflow file(s)"
required: true
runs:
using: "composite"
steps:
- name: Install dependencies
run: pip install --user yamllint
shell: bash

- name: Setup
id: setup
run: |
FORMAT_PATH=$(echo ${{ inputs.workflows }} | sed 's/ *$//')
echo "path=$FORMAT_PATH" >> $GITHUB_OUTPUT
shell: bash

- name: Python lint
run: python ${{ github.action_path }}/lint.py "${{ steps.setup.outputs.path }}"
shell: bash

- name: YAML lint
run: |
WORKFLOWS=($(echo "${{ steps.setup.outputs.path }}" | tr ' ' '\n'))
for WORKFLOW in "${WORKFLOWS[@]}"; do
yamllint -f colored -c ${{ github.action_path }}/.yamllint.yml $WORKFLOW
done
shell: bash
working-directory: ${{ github.workspace }}
Loading

0 comments on commit 4a07a79

Please sign in to comment.