From 7bdcb9d0dbd4851a7225319fb962fa7bdd45bba9 Mon Sep 17 00:00:00 2001 From: Micaiah Martin <77340197+mimartin12@users.noreply.github.com> Date: Mon, 11 Jul 2022 15:07:12 +0000 Subject: [PATCH] Add functionality for passing multiple files into linter (#51) --- .gitignore | 1 + lint-workflow/action.yml | 11 ++- lint-workflow/lint.py | 105 ++++++++++++--------- lint-workflow/tests/test-alt.yml | 25 +++++ lint-workflow/tests/test.yml | 2 +- lint-workflow/tests/test_a.yaml | 27 ++++++ lint-workflow/tests/test_action_update.py | 2 +- lint-workflow/tests/test_lint.py | 16 +++- lint-workflow/tests/test_main.py | 58 ++++++++++++ lint-workflow/tests/test_workflow_files.py | 20 ++++ 10 files changed, 216 insertions(+), 51 deletions(-) create mode 100644 lint-workflow/tests/test-alt.yml create mode 100644 lint-workflow/tests/test_a.yaml create mode 100644 lint-workflow/tests/test_main.py create mode 100644 lint-workflow/tests/test_workflow_files.py diff --git a/.gitignore b/.gitignore index 43aa6e48..ed7c92de 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ .vscode/launch.json .vscode/settings.json +**/__pycache__/ diff --git a/lint-workflow/action.yml b/lint-workflow/action.yml index fb18a617..40cac09d 100644 --- a/lint-workflow/action.yml +++ b/lint-workflow/action.yml @@ -1,11 +1,18 @@ name: 'Lint Workflow' description: 'Lints GitHub Actions Workflow' +inputs: + workflows: + description: "Path to workflow file(s)" + required: true runs: using: "composite" steps: - run: pip install --user yamllint shell: bash - - run: python ${{ github.action_path }}/lint.py .github/workflows + - run: python ${{ github.action_path }}/lint.py "${{ inputs.workflows }}" shell: bash - - run: yamllint -f colored -c ${{ github.action_path }}/.yamllint.yml .github/workflows + - run: | + TRIM_PATH=$(echo ${{ inputs.workflows }} | tr '\n' ' ') + yamllint -f colored -c ${{ github.action_path }}/.yamllint.yml $TRIM_PATH shell: bash + working-directory: ${{ github.workspace }} diff --git a/lint-workflow/lint.py b/lint-workflow/lint.py index c2cb5915..21fa2b68 100644 --- a/lint-workflow/lint.py +++ b/lint-workflow/lint.py @@ -1,11 +1,9 @@ +import sys import argparse import os import yaml import json import urllib3 as urllib -import sys -from urllib3.util import Retry -from urllib3.exceptions import MaxRetryError import logging PROBLEM_LEVELS = { @@ -39,8 +37,8 @@ def get_max_error_level(findings): """Get max error level from list of findings.""" if len(findings) == 0: return 0 - max_problem= max(findings, key=lambda finding: PROBLEM_LEVELS[finding.level]) - max_problem_level=PROBLEM_LEVELS[max_problem.level] + max_problem = max(findings, key=lambda finding: PROBLEM_LEVELS[finding.level]) + max_problem_level = PROBLEM_LEVELS[max_problem.level] return max_problem_level @@ -69,7 +67,15 @@ def get_github_api_response(url, action_id): response = http.request("GET", url, headers=headers) if response.status == 403 and response.reason == "rate limit exceeded": - logging.error(f"Failed to call GitHub API for action: {action_id} due to rate limit exceeded.") + logging.error( + f"Failed to call GitHub API for action: {action_id} due to rate limit exceeded." + ) + return None + + if response.status == 401 and response.reason == "Unauthorized": + logging.error( + f"Failed to call GitHub API for action: {action_id}: {response.data}." + ) return None return response @@ -107,9 +113,28 @@ def action_repo_exists(action_id): return True +def workflow_files(input: str) -> list: + """ + Takes in an argument of directory and/or files in string format from the CLI. + Returns a sorted set of all workflow files in the path(s) specified. + """ + workflow_files = [] + for path in input.split(): + if os.path.isfile(path): + workflow_files.append(path) + elif os.path.isdir(path): + for subdir, dirs, files in os.walk(path): + for filename in files: + filepath = subdir + os.sep + filename + if filepath.endswith((".yml", ".yaml")): + workflow_files.append(filepath) + + return sorted(set(workflow_files)) + + def get_action_update(action_id): """ - Takes and action id (bitwarden/gh-actions/version-bump@03ad9a873c39cdc95dd8d77dbbda67f84db43945) + Takes in an action id (bitwarden/gh-actions/version-bump@03ad9a873c39cdc95dd8d77dbbda67f84db43945) and checks the action repo for the newest version. If there is a new version, return the url to the updated version. """ @@ -246,8 +271,11 @@ def lint(filename): ) if "uses" in step: - - path, hash = step["uses"].split("@") + try: + path, hash = step["uses"].split("@") + except ValueError: + logging.info("Skipping local action in workflow.") + break # If the step has a 'uses' key, check value hash. try: @@ -284,12 +312,12 @@ def lint(filename): path_list = path.split("/", 2) if "bitwarden" in path and len(path_list) < 3: - findings.append( - LintFinding( - f"Step {str(i)} of job key '{job_key}' does not have a valid action path. (missing name of the repository or workflow)", - "error", - ) + findings.append( + LintFinding( + f"Step {str(i)} of job key '{job_key}' does not have a valid action path. (missing name of the repository or workflow)", + "error", ) + ) elif len(path_list) < 2: findings.append( LintFinding( @@ -338,7 +366,11 @@ def lint(filename): return max_error_level -def main(): +def main(input_args=None): + + # Pull the arguments from the command line + if not input_args: + input_args = sys.argv[1:] # Read arguments from command line. parser = argparse.ArgumentParser() @@ -349,44 +381,29 @@ def main(): action="store_true", help="return non-zero exit code on warnings " "as well as errors", ) - args = parser.parse_args() - - # Set up list for files to lint. - input_files = [] - - # Check if argument is file, then append to input files. - if os.path.isfile(args.input): - input_files.append(args.input) - # Check if argument is directory, then recursively add all *.yml and *.yaml files to input files. - elif os.path.isdir(args.input): - for subdir, dirs, files in os.walk(args.input): - for filename in files: - filepath = subdir + os.sep + filename - - if filepath.endswith(".yml") or filepath.endswith(".yaml"): - input_files.append(filepath) - else: - print("File/Directory does not exist, exiting.") - return -1 - + args = parser.parse_args(input_args) # max_error_level = 0 # for filename in input_files: # prob_level = lint(filename) # max_error_level = max(max_error_level, prob_level) + input_files = workflow_files(args.input) + if len(input_files) > 0: + prob_levels = list(map(lint, input_files)) - prob_levels = list(map(lint, input_files)) + max_error_level = max(prob_levels) - max_error_level = max(prob_levels) + if max_error_level == PROBLEM_LEVELS["error"]: + return_code = 2 + elif max_error_level == PROBLEM_LEVELS["warning"]: + return_code = 1 if args.strict else 0 + else: + return_code = 0 - if max_error_level == PROBLEM_LEVELS["error"]: - return_code = 2 - elif max_error_level == PROBLEM_LEVELS["warning"]: - return_code = 1 if args.strict else 0 + return return_code else: - return_code = 0 - - return return_code + print(f'File(s)/Directory: "{args.input}" does not exist, exiting.') + return -1 if __name__ == "__main__": diff --git a/lint-workflow/tests/test-alt.yml b/lint-workflow/tests/test-alt.yml new file mode 100644 index 00000000..7d01fafe --- /dev/null +++ b/lint-workflow/tests/test-alt.yml @@ -0,0 +1,25 @@ +--- +name: Lint Test File, DO NOT USE + +on: + workflow_dispatch: + inputs: {} + +jobs: + + test-normal-action: + name: Download Latest + runs-on: ubuntu-20.04 + steps: + - name: Checkout + uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b + + - run: | + echo test + + test-local-action: + name: Testing a local action call + runs-on: ubuntu-20.04 + steps: + - name: local-action + uses: ./version-bump diff --git a/lint-workflow/tests/test.yml b/lint-workflow/tests/test.yml index fc9bf934..49d4138a 100644 --- a/lint-workflow/tests/test.yml +++ b/lint-workflow/tests/test.yml @@ -15,7 +15,7 @@ jobs: _CROWDIN_PROJECT_ID: "308189" steps: - name: Checkout repo - uses: actions/checkout@5a4ac9002d0be2fb38bd78e4b4dbde5606d7042f # v2.3.4 + uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b # v2.3.4 - name: Login to Azure uses: Azure/logi@77f1b2e3fb80c0e8645114159d17008b8a2e475a diff --git a/lint-workflow/tests/test_a.yaml b/lint-workflow/tests/test_a.yaml new file mode 100644 index 00000000..bd0cfb24 --- /dev/null +++ b/lint-workflow/tests/test_a.yaml @@ -0,0 +1,27 @@ +--- +name: Lint Test File, DO NOT USE + +on: + workflow_dispatch: + inputs: {} + +jobs: + call-workflow: + uses: bitwarden/server/.github/workflows/workflow-linter.yml@master + + test-normal-action: + name: Download Latest + runs-on: ubuntu-20.04 + steps: + - name: Checkout + uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b + + - run: | + echo test + + test-local-action: + name: Testing a local action call + runs-on: ubuntu-20.04 + steps: + - name: local-action + uses: ./version-bump diff --git a/lint-workflow/tests/test_action_update.py b/lint-workflow/tests/test_action_update.py index a0db67ac..f7d0e09b 100644 --- a/lint-workflow/tests/test_action_update.py +++ b/lint-workflow/tests/test_action_update.py @@ -5,7 +5,7 @@ http = urllib.PoolManager() def test_action_update(): - action_id = 'actions/checkout@86f86b36ef15e6570752e7175f451a512eac206b' + action_id = "actions/checkout@86f86b36ef15e6570752e7175f451a512eac206b" sub_string = "github.com" update_url = get_action_update(action_id) assert str(sub_string) in str(update_url) diff --git a/lint-workflow/tests/test_lint.py b/lint-workflow/tests/test_lint.py index 6752f4b9..1c951505 100644 --- a/lint-workflow/tests/test_lint.py +++ b/lint-workflow/tests/test_lint.py @@ -1,9 +1,19 @@ from lint import lint + def test_lint(capfd): file_path = "test.yml" lint_output = lint(file_path) out, err = capfd.readouterr() - assert "\x1b[33mwarning\x1b[0m Name value for workflow is not capitalized. [crowdin Pull]" in out - assert "\x1b[33mwarning\x1b[0m Step 1 of job key 'crowdin-pull' uses an outdated action, consider updating it" in out - assert "\x1b[31merror\x1b[0m Step 2 of job key 'crowdin-pull' uses an non-existing action: Azure/logi@77f1b2e3fb80c0e8645114159d17008b8a2e475a." in out + assert ( + "\x1b[33mwarning\x1b[0m Name value for workflow is not capitalized. [crowdin Pull]" + in out + ) + assert ( + "\x1b[33mwarning\x1b[0m Step 3 of job key 'crowdin-pull' uses an outdated action, consider updating it" + in out + ) + assert ( + "\x1b[31merror\x1b[0m Step 2 of job key 'crowdin-pull' uses an non-existing action: Azure/logi@77f1b2e3fb80c0e8645114159d17008b8a2e475a." + in out + ) diff --git a/lint-workflow/tests/test_main.py b/lint-workflow/tests/test_main.py new file mode 100644 index 00000000..7cc1be39 --- /dev/null +++ b/lint-workflow/tests/test_main.py @@ -0,0 +1,58 @@ +from lint import main + +# Tests for argparse inputs and outputs using capsys.readouterr() + + +def test_main_single_file(capsys): + main(["test.yml"]) + captured = capsys.readouterr() + result = captured.out + assert "test.yml" in result + + +def test_main_multiple_files(capsys): + main(["test.yml test-alt.yml"]) + captured = capsys.readouterr() + result = captured.out + assert isinstance(result, str) + assert "test.yml" in result + assert "test-alt.yml" in result + + +def test_main_folder(capsys): + main(["./"]) + captured = capsys.readouterr() + result = captured.out + assert isinstance(result, str) + assert "test.yml" in result + assert "test-alt.yml" in result + + +def test_main_folder_and_files(capsys): + main(["test.yml ./"]) + captured = capsys.readouterr() + result = captured.out + print(result) + + +def test_main_not_found(capsys): + # File that doesn't exist + main(["not-a-real-file.yml"]) + captured = capsys.readouterr() + result = captured.out + assert isinstance(result, str) + assert ( + 'File(s)/Directory: "not-a-real-file.yml" does not exist, exiting.' in result + ) + # Empty string + main([""]) + captured = capsys.readouterr() + result = captured.out + assert isinstance(result, str) + assert 'File(s)/Directory: "" does not exist, exiting.' in result + # Spaces in string + main([" "]) + captured = capsys.readouterr() + result = captured.out + assert isinstance(result, str) + assert 'File(s)/Directory: " " does not exist, exiting.' in result diff --git a/lint-workflow/tests/test_workflow_files.py b/lint-workflow/tests/test_workflow_files.py new file mode 100644 index 00000000..ef4b440c --- /dev/null +++ b/lint-workflow/tests/test_workflow_files.py @@ -0,0 +1,20 @@ +import os +from lint import workflow_files + + +def test_workflow_files(): + assert workflow_files("") == [] + assert workflow_files("not-a-real-file.yml") == [] + assert workflow_files("test.yml") == ["test.yml"] + # multiple files + assert workflow_files("test.yml test-alt.yml") == sorted( + ["test.yml", "test-alt.yml"] + ) + # directory + assert workflow_files("../tests") == sorted(set( + ["../tests/"+file for file in os.listdir("../tests") if file.endswith((".yml", ".yaml"))] + )) + # directory and files + assert workflow_files("test.yml ../tests") == sorted(set( + ["test.yml"] + ["../tests/"+file for file in os.listdir("./") if file.endswith((".yml", ".yaml"))] + ))