Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Workflow Linter #231

Merged
merged 60 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 57 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
cb14f1f
Refactoring the linter into an opinionated reformatter
joseph-flinn Jan 26, 2023
ae9bfb2
Merge branch 'master' into feature/refactor-linter
joseph-flinn Apr 14, 2023
cdc06a1
migrating how the rules are created for a more streamlined experience
joseph-flinn Apr 21, 2023
704a5ed
removing the keyword from the step model since there are no linting tโ€ฆ
joseph-flinn Apr 21, 2023
4b36ac8
Switch from pydantic to dataclasses
joseph-flinn May 5, 2023
1ef4a3b
Update to use the correct loader (Model.from_dict({})). Add extra datโ€ฆ
joseph-flinn May 5, 2023
87adafd
Got to a somewhat working breakdown of the rules (not including checkโ€ฆ
joseph-flinn May 5, 2023
691cc9f
Major additions, test writing, and rule import framework
joseph-flinn Dec 29, 2023
9096b98
merge main and fix conflicts
joseph-flinn Dec 29, 2023
42ad11a
update pip lockfile
joseph-flinn Dec 29, 2023
34a4319
Build out the majority of the rules
joseph-flinn Jan 3, 2024
e7943e2
Add sub commands to manage the list of approved actions
joseph-flinn Jan 5, 2024
81cc449
Add test coverage and get test coverage of the rules to 100%
joseph-flinn Jan 5, 2024
8e4f866
Update Python version in lint-ci to match refactored linter
joseph-flinn Jan 5, 2024
e0a3ad8
Removing old files. Update verbosity
joseph-flinn Jan 5, 2024
819ca78
Finish in-code documentation
joseph-flinn Jan 9, 2024
060a33d
Run black over the code
joseph-flinn Jan 9, 2024
33a9e74
Clean up the unused tests
joseph-flinn Jan 9, 2024
d35110c
Expanded docs on how to add a new Rule
joseph-flinn Jan 9, 2024
1029baa
Remove old linter
joseph-flinn Jan 10, 2024
0232bd5
First linting error fixes
joseph-flinn Jan 11, 2024
3a233a5
Fix all non-rule linter issues
joseph-flinn Jan 11, 2024
a2930d8
Switch to the cleaner json.load() function
joseph-flinn Jan 11, 2024
fc9cbf7
Fix unit tests
joseph-flinn Jan 11, 2024
2131363
Fix linting issues on job_environment_prefix
joseph-flinn Jan 12, 2024
bdc35b8
Fix linting issues for RuleNameCapitalized
joseph-flinn Jan 12, 2024
9d91c1f
Fix linting warnings for RuleNameExists
joseph-flinn Jan 12, 2024
527c41b
Fix unit tests. Time to add some githooks...
joseph-flinn Jan 12, 2024
b52eaab
Update with linter findings for RuleStepUsesApproved
joseph-flinn Jan 12, 2024
057c2c5
Remove 'message' attribute from Rule base class. Fix linting findingsโ€ฆ
joseph-flinn Jan 12, 2024
40925be
Fix linter findings
joseph-flinn Jan 12, 2024
8ca7b95
Finish linter finding cleanup
joseph-flinn Jan 12, 2024
5e4c980
Reformat
joseph-flinn Jan 12, 2024
e49cde4
Fix type hinting
joseph-flinn Jan 12, 2024
4ca8b81
Move lint-workflow to vs directory restore lint-workflow
joseph-flinn Jan 12, 2024
f263128
Fix tests after type fixes. Add tests to CI
joseph-flinn Jan 12, 2024
3e18bed
Add new workflow path to the PR trigger
joseph-flinn Jan 12, 2024
17361c3
Update trigger to all pull_request events instead of push events
joseph-flinn Jan 12, 2024
f61921a
Update workflow job names to be more helpful
joseph-flinn Jan 12, 2024
8cf9043
Switch back to python 3.9 for v1 linter
joseph-flinn Jan 12, 2024
d5b0f77
Fix edge case where 'env' doesn't exist in a job
joseph-flinn Jan 13, 2024
a957c84
Remove the end-to-end testing since it fails (as expected) and prevenโ€ฆ
joseph-flinn Jan 26, 2024
e133520
Working on packaging with Hatch
joseph-flinn Jan 26, 2024
b8c0b8a
Update to Nix Unstable packages to allow for editable paths
joseph-flinn Feb 2, 2024
78d5561
Updating file structure. Tests passing
joseph-flinn Feb 2, 2024
2d057f0
Got to installable and importable module
joseph-flinn Feb 2, 2024
9ddbb7a
Switch setting from a python file to a yaml file
joseph-flinn Feb 23, 2024
0880b30
Update README with the new yaml settings
joseph-flinn Feb 23, 2024
a8c70b9
Debugging E2E tests. Need to handle creating the Jobs that don't haveโ€ฆ
joseph-flinn Feb 23, 2024
17a8fc9
Save the latest vim session
joseph-flinn Feb 23, 2024
14e5ae1
Fixed the complex workflow loading
joseph-flinn Feb 28, 2024
2dc3cde
Update the JobRunnerVersionPinned rule to support callable workflows
joseph-flinn Feb 28, 2024
92a85c7
Fix the lint command to be compatible with callable workflows
joseph-flinn Feb 28, 2024
3f231e9
Format code and fix type annotations
joseph-flinn Feb 28, 2024
db49c77
Merge branch 'main' into feature/refactor-linter
joseph-flinn Feb 28, 2024
86b9bd9
Clean up of local opinionated development environment
joseph-flinn Feb 28, 2024
ed12be6
Moving actions sub command to be in beta since we haven't decided to โ€ฆ
joseph-flinn Feb 28, 2024
1c0c8f3
Update all spelling errors identified and path issues (Thanks @vgrassโ€ฆ
joseph-flinn Feb 29, 2024
e1951c7
Add hacky safety measure to alert on GitHub Response Schema changes
joseph-flinn Feb 29, 2024
de9be6f
Fixed spelling issues
joseph-flinn Mar 6, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.

148 changes: 148 additions & 0 deletions lint-workflow-v2/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
# 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/rules` should have 100% code coverage and we should shoot for an overall coverage of 80%+.
vgrassia marked this conversation as resolved.
Show resolved Hide resolved
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
funcationl core (objects and models).
vgrassia marked this conversation as resolved.
Show resolved Hide resolved

```
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
```

[TODO: Is this enough documentation on how to use?]
vgrassia marked this conversation as resolved.
Show resolved Hide resolved

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 standardenforcing.
vgrassia marked this conversation as resolved.
Show resolved Hide resolved

`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 update --output test.json

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