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

Workflow validation #820

Merged
merged 10 commits into from
Jan 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

## v1.13dev

_..nothing yet.._
### Linting

* Added schema validation of GitHub action workflows to lint function [[#795](https://github.com/nf-core/tools/issues/795)]
* Fixed bug in schema title and description validation

## [v1.12.1 - Silver Dolphin](https://github.com/nf-core/tools/releases/tag/1.12.1) - [2020-12-03]

Expand Down
4 changes: 4 additions & 0 deletions docs/api/_src/lint_tests/actions_schema_validation.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
actions_schema_validation
===================

.. automethod:: nf_core.lint.PipelineLint.actions_schema_validation
2 changes: 2 additions & 0 deletions nf_core/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ class PipelineLint(nf_core.utils.Pipeline):
from .cookiecutter_strings import cookiecutter_strings
from .schema_lint import schema_lint
from .schema_params import schema_params
from .actions_schema_validation import actions_schema_validation

def __init__(self, wf_path, release_mode=False):
""" Initialise linting object """
Expand Down Expand Up @@ -144,6 +145,7 @@ def __init__(self, wf_path, release_mode=False):
"cookiecutter_strings",
"schema_lint",
"schema_params",
"actions_schema_validation",
]
if self.release_mode:
self.lint_tests.extend(["version_consistency"])
Expand Down
7 changes: 5 additions & 2 deletions nf_core/lint/actions_awsfulltest.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ def actions_awsfulltest(self):

fn = os.path.join(self.wf_path, ".github", "workflows", "awsfulltest.yml")
if os.path.isfile(fn):
with open(fn, "r") as fh:
wf = yaml.safe_load(fh)
try:
with open(fn, "r") as fh:
wf = yaml.safe_load(fh)
except:
return {"failed": ["Could not parse yaml file: {}".format(fn)]}

aws_profile = "-profile test "

Expand Down
7 changes: 5 additions & 2 deletions nf_core/lint/actions_awstest.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@ def actions_awstest(self):
if not os.path.isfile(fn):
return {"ignored": ["'awstest.yml' workflow not found: `{}`".format(fn)]}

with open(fn, "r") as fh:
wf = yaml.safe_load(fh)
try:
with open(fn, "r") as fh:
wf = yaml.safe_load(fh)
except:
return {"failed": ["Could not parse yaml file: {}".format(fn)]}

# Check that the action is only turned on for workflow_dispatch
try:
Expand Down
7 changes: 5 additions & 2 deletions nf_core/lint/actions_branch_protection.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,11 @@ def actions_branch_protection(self):
if not os.path.isfile(fn):
return {"ignored": ["Could not find branch.yml workflow: {}".format(fn)]}

with open(fn, "r") as fh:
branchwf = yaml.safe_load(fh)
try:
with open(fn, "r") as fh:
branchwf = yaml.safe_load(fh)
except:
return {"failed": ["Could not parse yaml file: {}".format(fn)]}

# Check that the action is turned on for PRs to master
try:
Expand Down
7 changes: 5 additions & 2 deletions nf_core/lint/actions_ci.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,11 @@ def actions_ci(self):
if not os.path.isfile(fn):
return {"ignored": ["'.github/workflows/ci.yml' not found"]}

with open(fn, "r") as fh:
ciwf = yaml.safe_load(fh)
try:
with open(fn, "r") as fh:
ciwf = yaml.safe_load(fh)
except:
return {"failed": ["Could not parse yaml file: {}".format(fn)]}

# Check that the action is turned on for the correct events
try:
Expand Down
7 changes: 5 additions & 2 deletions nf_core/lint/actions_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,11 @@ def actions_lint(self):
failed = []
fn = os.path.join(self.wf_path, ".github", "workflows", "linting.yml")
if os.path.isfile(fn):
with open(fn, "r") as fh:
lintwf = yaml.safe_load(fh)
try:
with open(fn, "r") as fh:
lintwf = yaml.safe_load(fh)
except:
return {"failed": ["Could not parse yaml file: {}".format(fn)]}

# Check that the action is turned on for push and pull requests
try:
Expand Down
60 changes: 60 additions & 0 deletions nf_core/lint/actions_schema_validation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#!/usr/bin/env python

import logging
import yaml
import json
import jsonschema
import os
import glob
import requests


def actions_schema_validation(self):
"""Checks that the GitHub Action workflow yml/yaml files adhere to the correct schema

nf-core pipelines use GitHub actions workflows to run CI tests, check formatting and also linting, among others.
These workflows are defined by ``yml``scripts in ``.github/workflows/``. This lint test verifies that these scripts are valid
by comparing them against the JSON schema for GitHub workflows <https://json.schemastore.org/github-workflow>

To pass this test, make sure that all your workflows contain the required properties ``on`` and ``jobs``and that
all other properties are of the correct type, as specified in the schema (link above).
"""
passed = []
failed = []

# Only show error messages from schema
logging.getLogger("nf_core.schema").setLevel(logging.ERROR)

# Get all workflow files
action_workflows = glob.glob(os.path.join(self.wf_path, ".github/workflows/*.y*ml"))

# Load the GitHub workflow schema
r = requests.get("https://json.schemastore.org/github-workflow", allow_redirects=True)
schema = r.json()

# Validate all workflows against the schema
for wf_path in action_workflows:
wf = os.path.basename(wf_path)

# load workflow
try:
with open(wf_path, "r") as fh:
wf_json = yaml.safe_load(fh)
except Exception as e:
failed.append("Could not parse yaml file: {}, {}".format(wf, e))
continue

# yaml parses 'on' as True --> try to fix it before schema validation
try:
wf_json["on"] = wf_json.pop(True)
except Exception as e:
failed.append("Missing 'on' keyword in {}.format(wf)")

# Validate the workflow
try:
jsonschema.validate(wf_json, schema)
passed.append("Workflow validation passed: {}".format(wf))
except Exception as e:
failed.append("Workflow validation failed for {}: {}".format(wf, e))

return {"passed": passed, "failed": failed}
2 changes: 1 addition & 1 deletion nf_core/lint/schema_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,6 @@ def schema_lint(self):
self.schema_obj.validate_schema_title_description()
passed.append("Schema title + description lint passed")
except AssertionError as e:
warned.append(e)
warned.append(str(e))

return {"passed": passed, "warned": warned, "failed": failed}
42 changes: 42 additions & 0 deletions tests/lint/actions_schema_validation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/usr/bin/env python

import os
import yaml
import nf_core.lint


def test_actions_schema_validation_missing_jobs(self):
"""Missing 'jobs' field should result in failure"""
new_pipeline = self._make_pipeline_copy()

with open(os.path.join(new_pipeline, ".github", "workflows", "awstest.yml"), "r") as fh:
awstest_yml = yaml.safe_load(fh)
awstest_yml["not_jobs"] = awstest_yml.pop("jobs")
with open(os.path.join(new_pipeline, ".github", "workflows", "awstest.yml"), "w") as fh:
yaml.dump(awstest_yml, fh)

lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()

results = lint_obj.actions_schema_validation()

assert "Workflow validation failed for awstest.yml: 'jobs' is a required property" in results["failed"][0]


def test_actions_schema_validation_missing_on(self):
"""Missing 'on' field should result in failure"""
new_pipeline = self._make_pipeline_copy()

with open(os.path.join(new_pipeline, ".github", "workflows", "awstest.yml"), "r") as fh:
awstest_yml = yaml.safe_load(fh)
awstest_yml["not_on"] = awstest_yml.pop(True)
with open(os.path.join(new_pipeline, ".github", "workflows", "awstest.yml"), "w") as fh:
yaml.dump(awstest_yml, fh)

lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()

results = lint_obj.actions_schema_validation()

assert results["failed"][0] == "Missing 'on' keyword in {}.format(wf)"
assert "Workflow validation failed for awstest.yml: 'on' is a required property" in results["failed"][1]
5 changes: 5 additions & 0 deletions tests/test_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,11 @@ def test_sphinx_rst_files(self):
test_actions_ci_fail_wrong_trigger,
)

from lint.actions_schema_validation import (
test_actions_schema_validation_missing_jobs,
test_actions_schema_validation_missing_on,
)


# def test_critical_missingfiles_example(self):
# """Tests for missing nextflow config and main.nf files"""
Expand Down