From 819ca78d30b092d9cd934ad28a4f4bbc7f1b19f7 Mon Sep 17 00:00:00 2001 From: Joseph Flinn Date: Mon, 8 Jan 2024 16:23:21 -0800 Subject: [PATCH] Finish in-code documentation --- lint-workflow/Pipfile | 1 + lint-workflow/Pipfile.lock | 50 +++- lint-workflow/Session.vim | 252 +++++++++++------- lint-workflow/cli.py | 2 +- lint-workflow/src/actions.py | 21 +- lint-workflow/src/lint.py | 107 +++++--- lint-workflow/src/load.py | 47 ++++ lint-workflow/src/rule.py | 46 +++- .../src/rules/job_environment_prefix.py | 35 ++- lint-workflow/src/rules/name_capitalized.py | 29 +- lint-workflow/src/rules/name_exists.py | 28 +- lint-workflow/src/rules/pinned_job_runner.py | 27 +- lint-workflow/src/rules/step_approved.py | 52 +++- lint-workflow/src/rules/step_pinned.py | 47 +++- lint-workflow/src/utils.py | 70 ++++- lint-workflow/tests/test_lint.py | 34 +++ lint-workflow/tests/test_utils.py | 16 +- 17 files changed, 685 insertions(+), 179 deletions(-) create mode 100644 lint-workflow/tests/test_lint.py diff --git a/lint-workflow/Pipfile b/lint-workflow/Pipfile index 6c0c5757..6918fd79 100644 --- a/lint-workflow/Pipfile +++ b/lint-workflow/Pipfile @@ -15,6 +15,7 @@ black = "*" pytest = "*" coverage = "*" pytest-cov = "*" +pylint = "*" [requires] python_version = "3.11" diff --git a/lint-workflow/Pipfile.lock b/lint-workflow/Pipfile.lock index 8635bd2a..5822b002 100644 --- a/lint-workflow/Pipfile.lock +++ b/lint-workflow/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "9c217f65befc18770734bee9ec6ee8420cdbad03a5eba07d13a77d3f0d082712" + "sha256": "c238af6c0c1de4a035edbf90e66bdbd46d0fd8ce05f368d1c89dd668427f6755" }, "pipfile-spec": 6, "requires": { @@ -320,6 +320,14 @@ } }, "develop": { + "astroid": { + "hashes": [ + "sha256:4a61cf0a59097c7bb52689b0fd63717cd2a8a14dc9f1eee97b82d814881c8c91", + "sha256:d6e62862355f60e716164082d6b4b041d38e2a8cf1c7cd953ded5108bac8ff5c" + ], + "markers": "python_full_version >= '3.8.0'", + "version": "==3.0.2" + }, "black": { "hashes": [ "sha256:0808494f2b2df923ffc5723ed3c7b096bd76341f6213989759287611e9837d50", @@ -414,6 +422,14 @@ "index": "pypi", "version": "==7.4.0" }, + "dill": { + "hashes": [ + "sha256:76b122c08ef4ce2eedcd4d1abd8e641114bfc6c2867f49f3c41facf65bf19f5e", + "sha256:cc1c8b182eb3013e24bd475ff2e9295af86c1a38eb1aff128dac8962a9ce3c03" + ], + "markers": "python_version >= '3.11'", + "version": "==0.3.7" + }, "iniconfig": { "hashes": [ "sha256:2d91e135bf72d31a410b17c16da610a82cb55f6b0477d1a902134b24a455b8b3", @@ -422,6 +438,22 @@ "markers": "python_version >= '3.7'", "version": "==2.0.0" }, + "isort": { + "hashes": [ + "sha256:48fdfcb9face5d58a4f6dde2e72a1fb8dcaf8ab26f95ab49fab84c2ddefb0109", + "sha256:8ca5e72a8d85860d5a3fa69b8745237f2939afe12dbf656afbcb47fe72d947a6" + ], + "markers": "python_full_version >= '3.8.0'", + "version": "==5.13.2" + }, + "mccabe": { + "hashes": [ + "sha256:348e0240c33b60bbdf4e523192ef919f28cb2c3d7d5c7794f74009290f236325", + "sha256:6c2d30ab6be0e4a46919781807b4f0d834ebdd6c6e3dca0bda5a15f863427b6e" + ], + "markers": "python_version >= '3.6'", + "version": "==0.7.0" + }, "mypy-extensions": { "hashes": [ "sha256:4392f6c0eb8a5668a69e23d168ffa70f0be9ccfd32b5cc2d26a34ae5b844552d", @@ -462,6 +494,14 @@ "markers": "python_version >= '3.8'", "version": "==1.3.0" }, + "pylint": { + "hashes": [ + "sha256:58c2398b0301e049609a8429789ec6edf3aabe9b6c5fec916acd18639c16de8b", + "sha256:7a1585285aefc5165db81083c3e06363a27448f6b467b3b0f30dbd0ac1f73810" + ], + "index": "pypi", + "version": "==3.0.3" + }, "pytest": { "hashes": [ "sha256:2cf0005922c6ace4a3e2ec8b4080eb0d9753fdc93107415332f50ce9e7994280", @@ -477,6 +517,14 @@ ], "index": "pypi", "version": "==4.1.0" + }, + "tomlkit": { + "hashes": [ + "sha256:75baf5012d06501f07bee5bf8e801b9f343e7aac5a92581f20f80ce632e6b5a4", + "sha256:b0a645a9156dc7cb5d3a1f0d4bab66db287fcb8e0430bdd4664a095ea16414ba" + ], + "markers": "python_version >= '3.7'", + "version": "==0.12.3" } } } diff --git a/lint-workflow/Session.vim b/lint-workflow/Session.vim index 2d3bdb42..5a3a81a0 100644 --- a/lint-workflow/Session.vim +++ b/lint-workflow/Session.vim @@ -14,43 +14,46 @@ else set shortmess=aoO endif badd +1 README.md -badd +1 cli.py +badd +24 cli.py badd +4 src/load.py badd +1 config.yaml badd +1 src/rules/__init__.py -badd +16 src/rules/name_capitalized.py -badd +17 src/rules/name_exists.py +badd +19 src/rules/name_capitalized.py +badd +13 src/rules/name_exists.py badd +1 src/rule.py badd +72 tests/test_rule.py badd +40 tests/rules/test_name_exists.py -badd +2 tests/rules/test_name_capitalized.py +badd +100 tests/rules/test_name_capitalized.py badd +4 rule_settings.py -badd +3 settings.py -badd +1 tests/fixtures/test.yml +badd +12 settings.py +badd +3 tests/fixtures/test.yml badd +1 tests/fixtures/test-min-incorrect.yaml badd +10 tests/test_load.py badd +2 src/rules/runs_on_pinned.py badd +3 src/rules/pinned_workflow_runner.py badd +3 tests/rules/test_pinned_workflow_runner.py badd +15 src/models/workflow.py -badd +9 src/rules/pinned_job_runner.py +badd +1 src/rules/pinned_job_runner.py badd +9 tests/rules/test_pinned_job_runner.py -badd +3 src/rules/job_environment_prefix.py -badd +14 src/models/job.py -badd +4 tests/rules/test_job_environment_prefix.py +badd +19 src/rules/job_environment_prefix.py +badd +18 src/models/job.py +badd +72 tests/rules/test_job_environment_prefix.py badd +19 src/rules/step_hex_length.py badd +43 tests/rules/test_step_hex_length.py badd +5 src/rules/step_hex.py badd +9 tests/rules/test_step_hex.py -badd +26 src/rules/step_approved.py -badd +59 tests/rules/test_step_approved.py +badd +47 src/rules/step_approved.py +badd +92 tests/rules/test_step_approved.py badd +0 src/utils.py -badd +28 src/rules/step_pinned.py -badd +82 tests/rules/test_step_pinned.py -badd +0 tests/fixtures/test-min.yaml +badd +16 src/rules/step_pinned.py +badd +100 tests/rules/test_step_pinned.py +badd +1 tests/fixtures/test-min.yaml badd +0 actions.json badd +0 src/models/step.py badd +0 tests/test_utils.py +badd +0 src/lint.py +badd +0 src/actions.py +badd +0 Taskfile.yml argglobal %argdel $argadd README.md @@ -58,6 +61,7 @@ set stal=2 tabnew +setlocal\ bufhidden=wipe tabnew +setlocal\ bufhidden=wipe tabnew +setlocal\ bufhidden=wipe +tabnew +setlocal\ bufhidden=wipe tabrewind edit README.md let s:save_splitbelow = &splitbelow @@ -70,6 +74,10 @@ vsplit 2wincmd h wincmd w wincmd w +wincmd _ | wincmd | +split +1wincmd k +wincmd w let &splitbelow = s:save_splitbelow let &splitright = s:save_splitright wincmd t @@ -79,11 +87,37 @@ set winminheight=0 set winheight=1 set winminwidth=0 set winwidth=1 -exe 'vert 1resize ' . ((&columns * 108 + 163) / 327) +exe 'vert 1resize ' . ((&columns * 109 + 163) / 327) exe 'vert 2resize ' . ((&columns * 108 + 163) / 327) -exe 'vert 3resize ' . ((&columns * 109 + 163) / 327) +exe '3resize ' . ((&lines * 32 + 33) / 67) +exe 'vert 3resize ' . ((&columns * 108 + 163) / 327) +exe '4resize ' . ((&lines * 31 + 33) / 67) +exe 'vert 4resize ' . ((&columns * 108 + 163) / 327) argglobal -balt tests/fixtures/test.yml +balt Taskfile.yml +setlocal fdm=manual +setlocal fde=0 +setlocal fmr={{{,}}} +setlocal fdi=# +setlocal fdl=0 +setlocal fml=1 +setlocal fdn=20 +setlocal fen +silent! normal! zE +let &fdl = &fdl +let s:l = 41 - ((36 * winheight(0) + 32) / 64) +if s:l < 1 | let s:l = 1 | endif +keepjumps exe s:l +normal! zt +keepjumps 41 +normal! 0 +wincmd w +argglobal +if bufexists(fnamemodify("Taskfile.yml", ":p")) | buffer Taskfile.yml | else | edit Taskfile.yml | endif +if &buftype ==# 'terminal' + silent file Taskfile.yml +endif +balt README.md setlocal fdm=manual setlocal fde=0 setlocal fmr={{{,}}} @@ -94,12 +128,12 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 61 - ((57 * winheight(0) + 32) / 65) +let s:l = 29 - ((28 * winheight(0) + 32) / 64) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 61 -normal! 04| +keepjumps 29 +normal! 021| wincmd w argglobal if bufexists(fnamemodify("settings.py", ":p")) | buffer settings.py | else | edit settings.py | endif @@ -117,12 +151,12 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 6 - ((5 * winheight(0) + 32) / 65) +let s:l = 5 - ((4 * winheight(0) + 16) / 32) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 6 -normal! 019| +keepjumps 5 +normal! 0 wincmd w argglobal if bufexists(fnamemodify("actions.json", ":p")) | buffer actions.json | else | edit actions.json | endif @@ -140,16 +174,19 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 6 - ((5 * winheight(0) + 32) / 65) +let s:l = 5 - ((2 * winheight(0) + 15) / 31) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 6 -normal! 068| +keepjumps 5 +normal! 021| wincmd w -exe 'vert 1resize ' . ((&columns * 108 + 163) / 327) +exe 'vert 1resize ' . ((&columns * 109 + 163) / 327) exe 'vert 2resize ' . ((&columns * 108 + 163) / 327) -exe 'vert 3resize ' . ((&columns * 109 + 163) / 327) +exe '3resize ' . ((&lines * 32 + 33) / 67) +exe 'vert 3resize ' . ((&columns * 108 + 163) / 327) +exe '4resize ' . ((&lines * 31 + 33) / 67) +exe 'vert 4resize ' . ((&columns * 108 + 163) / 327) tabnext edit cli.py let s:save_splitbelow = &splitbelow @@ -175,7 +212,7 @@ exe 'vert 1resize ' . ((&columns * 108 + 163) / 327) exe 'vert 2resize ' . ((&columns * 108 + 163) / 327) exe 'vert 3resize ' . ((&columns * 109 + 163) / 327) argglobal -balt settings.py +balt src/actions.py setlocal fdm=manual setlocal fde=0 setlocal fmr={{{,}}} @@ -186,19 +223,19 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 21 - ((20 * winheight(0) + 32) / 65) +let s:l = 50 - ((49 * winheight(0) + 32) / 64) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 21 -normal! 0 +keepjumps 50 +normal! 09| wincmd w argglobal -if bufexists(fnamemodify("src/utils.py", ":p")) | buffer src/utils.py | else | edit src/utils.py | endif +if bufexists(fnamemodify("src/actions.py", ":p")) | buffer src/actions.py | else | edit src/actions.py | endif if &buftype ==# 'terminal' - silent file src/utils.py + silent file src/actions.py endif -balt tests/test_utils.py +balt cli.py setlocal fdm=manual setlocal fde=0 setlocal fmr={{{,}}} @@ -209,19 +246,19 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 25 - ((24 * winheight(0) + 32) / 65) +let s:l = 16 - ((10 * winheight(0) + 32) / 64) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 25 -normal! 035| +keepjumps 16 +normal! 0 wincmd w argglobal -if bufexists(fnamemodify("tests/test_utils.py", ":p")) | buffer tests/test_utils.py | else | edit tests/test_utils.py | endif +if bufexists(fnamemodify("src/lint.py", ":p")) | buffer src/lint.py | else | edit src/lint.py | endif if &buftype ==# 'terminal' - silent file tests/test_utils.py + silent file src/lint.py endif -balt tests/test_load.py +balt cli.py setlocal fdm=manual setlocal fde=0 setlocal fmr={{{,}}} @@ -232,18 +269,19 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 1 - ((0 * winheight(0) + 32) / 65) +let s:l = 65 - ((1 * winheight(0) + 32) / 64) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 1 -normal! 0 +keepjumps 65 +normal! 036| wincmd w +3wincmd w exe 'vert 1resize ' . ((&columns * 108 + 163) / 327) exe 'vert 2resize ' . ((&columns * 108 + 163) / 327) exe 'vert 3resize ' . ((&columns * 109 + 163) / 327) tabnext -edit src/models/job.py +edit src/load.py let s:save_splitbelow = &splitbelow let s:save_splitright = &splitright set splitbelow splitright @@ -251,7 +289,10 @@ wincmd _ | wincmd | vsplit wincmd _ | wincmd | vsplit -2wincmd h +wincmd _ | wincmd | +vsplit +3wincmd h +wincmd w wincmd w wincmd w let &splitbelow = s:save_splitbelow @@ -263,11 +304,12 @@ set winminheight=0 set winheight=1 set winminwidth=0 set winwidth=1 -exe 'vert 1resize ' . ((&columns * 108 + 163) / 327) -exe 'vert 2resize ' . ((&columns * 108 + 163) / 327) -exe 'vert 3resize ' . ((&columns * 109 + 163) / 327) +exe 'vert 1resize ' . ((&columns * 81 + 163) / 327) +exe 'vert 2resize ' . ((&columns * 81 + 163) / 327) +exe 'vert 3resize ' . ((&columns * 81 + 163) / 327) +exe 'vert 4resize ' . ((&columns * 81 + 163) / 327) argglobal -balt src/rule.py +balt tests/test_load.py setlocal fdm=manual setlocal fde=0 setlocal fmr={{{,}}} @@ -278,19 +320,19 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 18 - ((17 * winheight(0) + 32) / 65) +let s:l = 67 - ((32 * winheight(0) + 32) / 64) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 18 -normal! 016| +keepjumps 67 +normal! 042| wincmd w argglobal -if bufexists(fnamemodify("src/rule.py", ":p")) | buffer src/rule.py | else | edit src/rule.py | endif +if bufexists(fnamemodify("tests/test_load.py", ":p")) | buffer tests/test_load.py | else | edit tests/test_load.py | endif if &buftype ==# 'terminal' - silent file src/rule.py + silent file tests/test_load.py endif -balt tests/test_rule.py +balt src/load.py setlocal fdm=manual setlocal fde=0 setlocal fmr={{{,}}} @@ -301,19 +343,19 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 19 - ((6 * winheight(0) + 32) / 65) +let s:l = 45 - ((44 * winheight(0) + 32) / 64) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 19 +keepjumps 45 normal! 0 wincmd w argglobal -if bufexists(fnamemodify("tests/test_rule.py", ":p")) | buffer tests/test_rule.py | else | edit tests/test_rule.py | endif +if bufexists(fnamemodify("src/utils.py", ":p")) | buffer src/utils.py | else | edit src/utils.py | endif if &buftype ==# 'terminal' - silent file tests/test_rule.py + silent file src/utils.py endif -balt tests/fixtures/test-min-incorrect.yaml +balt src/load.py setlocal fdm=manual setlocal fde=0 setlocal fmr={{{,}}} @@ -324,27 +366,48 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 48 - ((30 * winheight(0) + 32) / 65) +let s:l = 49 - ((45 * winheight(0) + 32) / 64) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 48 +keepjumps 49 normal! 09| wincmd w -exe 'vert 1resize ' . ((&columns * 108 + 163) / 327) -exe 'vert 2resize ' . ((&columns * 108 + 163) / 327) -exe 'vert 3resize ' . ((&columns * 109 + 163) / 327) +argglobal +if bufexists(fnamemodify("tests/test_utils.py", ":p")) | buffer tests/test_utils.py | else | edit tests/test_utils.py | endif +if &buftype ==# 'terminal' + silent file tests/test_utils.py +endif +balt tests/test_load.py +setlocal fdm=manual +setlocal fde=0 +setlocal fmr={{{,}}} +setlocal fdi=# +setlocal fdl=0 +setlocal fml=1 +setlocal fdn=20 +setlocal fen +silent! normal! zE +let &fdl = &fdl +let s:l = 36 - ((35 * winheight(0) + 32) / 64) +if s:l < 1 | let s:l = 1 | endif +keepjumps exe s:l +normal! zt +keepjumps 36 +normal! 0 +wincmd w +exe 'vert 1resize ' . ((&columns * 81 + 163) / 327) +exe 'vert 2resize ' . ((&columns * 81 + 163) / 327) +exe 'vert 3resize ' . ((&columns * 81 + 163) / 327) +exe 'vert 4resize ' . ((&columns * 81 + 163) / 327) tabnext -edit src/models/step.py +edit src/rule.py let s:save_splitbelow = &splitbelow let s:save_splitright = &splitright set splitbelow splitright wincmd _ | wincmd | vsplit -wincmd _ | wincmd | -vsplit -2wincmd h -wincmd w +1wincmd h wincmd w let &splitbelow = s:save_splitbelow let &splitright = s:save_splitright @@ -355,11 +418,10 @@ set winminheight=0 set winheight=1 set winminwidth=0 set winwidth=1 -exe 'vert 1resize ' . ((&columns * 109 + 163) / 327) -exe 'vert 2resize ' . ((&columns * 108 + 163) / 327) -exe 'vert 3resize ' . ((&columns * 108 + 163) / 327) +exe 'vert 1resize ' . ((&columns * 163 + 163) / 327) +exe 'vert 2resize ' . ((&columns * 163 + 163) / 327) argglobal -balt src/rules/step_approved.py +balt tests/test_rule.py setlocal fdm=manual setlocal fde=0 setlocal fmr={{{,}}} @@ -370,19 +432,19 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 33 - ((32 * winheight(0) + 32) / 65) +let s:l = 41 - ((40 * winheight(0) + 32) / 64) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 33 -normal! 044| +keepjumps 41 +normal! 0 wincmd w argglobal -if bufexists(fnamemodify("src/rules/step_approved.py", ":p")) | buffer src/rules/step_approved.py | else | edit src/rules/step_approved.py | endif +if bufexists(fnamemodify("tests/test_rule.py", ":p")) | buffer tests/test_rule.py | else | edit tests/test_rule.py | endif if &buftype ==# 'terminal' - silent file src/rules/step_approved.py + silent file tests/test_rule.py endif -balt src/models/step.py +balt tests/fixtures/test-min-incorrect.yaml setlocal fdm=manual setlocal fde=0 setlocal fmr={{{,}}} @@ -393,18 +455,18 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 43 - ((42 * winheight(0) + 32) / 65) +let s:l = 133 - ((56 * winheight(0) + 32) / 64) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 43 -normal! 018| +keepjumps 133 +normal! 012| wincmd w +exe 'vert 1resize ' . ((&columns * 163 + 163) / 327) +exe 'vert 2resize ' . ((&columns * 163 + 163) / 327) +tabnext +edit src/models/step.py argglobal -if bufexists(fnamemodify("tests/rules/test_step_approved.py", ":p")) | buffer tests/rules/test_step_approved.py | else | edit tests/rules/test_step_approved.py | endif -if &buftype ==# 'terminal' - silent file tests/rules/test_step_approved.py -endif balt src/rules/step_approved.py setlocal fdm=manual setlocal fde=0 @@ -416,17 +478,13 @@ setlocal fdn=20 setlocal fen silent! normal! zE let &fdl = &fdl -let s:l = 63 - ((35 * winheight(0) + 32) / 65) +let s:l = 23 - ((22 * winheight(0) + 32) / 64) if s:l < 1 | let s:l = 1 | endif keepjumps exe s:l normal! zt -keepjumps 63 +keepjumps 23 normal! 029| -wincmd w -exe 'vert 1resize ' . ((&columns * 109 + 163) / 327) -exe 'vert 2resize ' . ((&columns * 108 + 163) / 327) -exe 'vert 3resize ' . ((&columns * 108 + 163) / 327) -tabnext 1 +tabnext 2 set stal=1 if exists('s:wipebuf') && len(win_findbuf(s:wipebuf)) == 0 && getbufvar(s:wipebuf, '&buftype') isnot# 'terminal' silent exe 'bwipe ' . s:wipebuf @@ -434,8 +492,6 @@ endif unlet! s:wipebuf set winheight=1 winwidth=20 let &shortmess = s:shortmess_save -let &winminheight = s:save_winminheight -let &winminwidth = s:save_winminwidth let s:sx = expand(":p:r")."x.vim" if filereadable(s:sx) exe "source " . fnameescape(s:sx) diff --git a/lint-workflow/cli.py b/lint-workflow/cli.py index 7372f26a..0802f541 100644 --- a/lint-workflow/cli.py +++ b/lint-workflow/cli.py @@ -50,7 +50,7 @@ def main(input_args=None): args = parser.parse_args(input_args) if args.command == "lint": - return linter_cmd.run(args.files) + return linter_cmd.run(args.files, args.strict) if args.command == "actions": if args.actions_command == "add": diff --git a/lint-workflow/src/actions.py b/lint-workflow/src/actions.py index f25f9917..76937293 100644 --- a/lint-workflow/src/actions.py +++ b/lint-workflow/src/actions.py @@ -7,11 +7,26 @@ from dataclasses import asdict from typing import Union, Tuple -from src.utils import Colors, LintFinding, Settings, SettingsError, Action +from src.utils import Colors, Settings, SettingsError, Action class ActionsCmd: def __init__(self, settings: Settings = None) -> None: + """Command to manage the pre-approved list of Actions + + This class contains logic to manage the list of pre-approved actions + to include: + - updating the action data in the list + - adding a new pre-approved action to the list with the data from the + latest release + + This class also includes supporting logic to interact with GitHub + + Args: + settings: + A Settings object that contains any default, overriden, or custom settings + required anywhere in the application. + """ self.settings = settings @staticmethod @@ -20,6 +35,10 @@ def extend_parser(subparsers: argparse.ArgumentParser) -> argparse.ArgumentParse Add 'actions add' and 'actions update' to the CLI as sub-commands along with the options and arguments for each. + + Args: + subparsers: + The main argument parser to add sub commands and arguments to """ parser_actions = subparsers.add_parser("actions", help="actions help") parser_actions.add_argument( diff --git a/lint-workflow/src/lint.py b/lint-workflow/src/lint.py index 0bc37607..d4d6ded4 100644 --- a/lint-workflow/src/lint.py +++ b/lint-workflow/src/lint.py @@ -1,18 +1,26 @@ import argparse import os +from functools import reduce + from src.load import WorkflowBuilder, Rules from src.utils import Colors, LintFinding, Settings, SettingsError -PROBLEM_LEVELS = { - "warning": 1, - "error": 2, -} - - class LinterCmd: def __init__(self, settings: Settings = None) -> None: + """Command to lint GitHub Action Workflow files + + This class contains logic to lint workflows that are passed in. + Supporting logic is supplied to: + - build out the list of Rules desired + - select and validate the workflow files to lint + + Args: + settings: + A Settings object that contains any default, overriden, or custom settings + required anywhere in the application. + """ self.rules = Rules(settings=settings) @staticmethod @@ -20,6 +28,10 @@ def extend_parser(subparsers: argparse.ArgumentParser) -> argparse.ArgumentParse """Extends the CLI subparser with the options for LintCmd. Add 'lint' as a sub command along with its options and arguments + + Args: + subparsers: + The main argument parser to add sub commands and arguments to """ parser_lint = subparsers.add_parser("lint", help="lint help") parser_lint.add_argument( @@ -38,27 +50,35 @@ def extend_parser(subparsers: argparse.ArgumentParser) -> argparse.ArgumentParse return subparsers def get_max_error_level(self, findings: list[LintFinding]) -> int: - """Get max error level from list of findings.""" + """Get max error level from list of findings. + + Compute the maximum error level to determine the exit code required. + # if max(error) return exit(1); else return exit(0) + + Args: + findings: + All of the findings that the linter found while linting a workflows. + + Return: + The numeric value of the maximum lint finding + """ 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] - return max_problem_level - - def print_finding(self, finding: LintFinding) -> None: - """Print formatted and colored finding.""" - if finding.level == "warning": - color = Colors.yellow - elif finding.level == "error": - color = Colors.red - else: - color = Colors.white + return max(findings, key=lambda finding: finding.level.code).level.code - line = f" - \033[{color}{finding.level}\033[0m {finding.description}" + def lint_file(self, filename: str) -> int: + """Lint a single workflow. - print(line) + Run all of the Workflow, Job, and Step level rules that have been enabled. - def lint_file(self, filename: str) -> int: + Args: + filename: + The name of the file that contains the workflow to lint + + Returns: + The maximum error level found in the file (none, warning, error) to + calculate the exit code from. + """ findings = [] max_error_level = 0 @@ -81,17 +101,24 @@ def lint_file(self, filename: str) -> int: if len(findings) > 0: for finding in findings: - self.print_finding(finding) + print(f" - {finding}") print() max_error_level = self.get_max_error_level(findings) return max_error_level - def generate_files(self, files: list) -> list: - """ - Takes in an argument of directory and/or files in list format from the CLI. - Returns a sorted set of all workflow files in the path(s) specified. + def generate_files(self, files: list[str]) -> list[str]: + """Generate the list of files to lint. + + Searches the list of directory and/or files taken from the CLI. + + Args: + files: + list of file names or director names. + + Returns: + A sorted set of all workflow files in the path(s) specified. """ workflow_files = [] for path in files: @@ -106,19 +133,27 @@ def generate_files(self, files: list) -> list: return sorted(set(workflow_files)) - def run(self, input_files: list[str]) -> int: + def run(self, input_files: list[str], strict: bool = False) -> int: + """Execute the LinterCmd. + + Args: + input_files: + list of file names or director names. + strict: + fail on WARNING instead of succeed + + Returns + The return_code for the entire CLI to indicate success/failure + """ files = self.generate_files(input_files) if len(input_files) > 0: - prob_levels = list(map(self.lint_file, files)) - - max_error_level = max(prob_levels) + return_code = reduce( + lambda a, b: a if a > b else b, + map(self.lint_file, files) + ) - 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: + if return_code == 1 and not strict: return_code = 0 return return_code diff --git a/lint-workflow/src/load.py b/lint-workflow/src/load.py index 61c70f4c..59af3cfa 100644 --- a/lint-workflow/src/load.py +++ b/lint-workflow/src/load.py @@ -15,17 +15,38 @@ class WorkflowBuilderError(Exception): + """Custom Exception to indicate an error with the WorkflowBuilder.""" pass class WorkflowBuilder: @classmethod def __load_workflow_from_file(cls, filename: str) -> CommentedMap: + """Load YAML from disk. + + Args: + filename: + The name of the YAML file to read. + + Returns: + A CommentedMap that contains the dict() representation of the + yaml file. It includes the comments as a part of their respective + objects (depending on their location in the file). + """ with open(filename) as file: return yaml.load(file) @classmethod def __build_workflow(cls, loaded_yaml: CommentedMap) -> Workflow: + """Parse the YAML and build out the workflow to run Rules against. + + Args: + loaded_yaml: + YAML that was loaded from either code or a file + + Returns + A Workflow to run linting Rules against + """ return Workflow.from_dict( { **loaded_yaml, @@ -49,6 +70,20 @@ def __build_workflow(cls, loaded_yaml: CommentedMap) -> Workflow: def build( cls, filename: str = None, yaml: CommentedMap = None, from_file: bool = True ) -> Workflow: + """Build a Workflow from either code or a file. + + This is a method that assists in testing by abstracting the disk IO + and allows for passing in a YAML object in code. + + Args: + filename: + The name of the file to load the YAML workflow from + yaml: + Pre-loaded YAML of a workflow + from_file: + Flag to determine if the YAML has already been loaded or needs to + be loaded from disk + """ if from_file and filename is not None: return cls.__build_workflow(cls.__load_workflow_from_file(filename)) elif not from_file and yaml is not None: @@ -65,6 +100,17 @@ class Rules: step: List[Rule] = [] def __init__(self, settings: Settings) -> None: + """A collection of all of the types of rules. + + Rules is used as a collection of which Rules apply to which parts of the + workflow. It also assists in making sure the Rules that apply to multiple + types are not skipped. + + Args: + settings: + A Settings object that contains any default, overriden, or custom settings + required anywhere in the application. + """ for rule in settings.enabled_rules: module_name = rule.split(".") module_name = ".".join(module_name[:-1]) @@ -84,6 +130,7 @@ def __init__(self, settings: Settings) -> None: print(f"Error loading: {rule}\n{err}") def list(self) -> None: + """Print the loaded Rules.""" print("===== Loaded Rules =====") print("workflow rules:") for rule in self.workflow: diff --git a/lint-workflow/src/rule.py b/lint-workflow/src/rule.py index 8d615dc4..1492f535 100644 --- a/lint-workflow/src/rule.py +++ b/lint-workflow/src/rule.py @@ -3,19 +3,43 @@ from .models.workflow import Workflow from .models.job import Job from .models.step import Step -from .utils import LintFinding, Settings +from .utils import LintFinding, LintLevels, Settings class Rule: + """Base class of a Rule to extend to create a linting Rule.""" message: str = "error" on_fail: str = "error" compatibility: List[Union[Workflow, Job, Step]] = [Workflow, Job, Step] settings: Settings = None def fn(self, obj: Union[Workflow, Job, Step]) -> bool: + """Execute the Rule (this should be overriden in the extending class. + + Args: + obj: + The object that the Rule is to be run against + + Returns: + The success/failure of the result of the Rule ran on the input. + """ return False, self.message def build_lint_message(self, message: str, obj: Union[Workflow, Job, Step]) -> str: + """Build the lint failure message. + + Build the lint failure message depending on the type of object that the + Rule is being run against. + + Args: + message: + The message body of the failure + obj: + The object the Rule is being run against + + Returns: + The type specific failure message + """ obj_type = type(obj) if obj_type == Step: @@ -26,6 +50,22 @@ def build_lint_message(self, message: str, obj: Union[Workflow, Job, Step]) -> s return f"{obj_type.__name__} => {message}" def execute(self, obj: Union[Workflow, Job, Step]) -> Union[LintFinding, None]: + """Wrapper function to execute the overriden self.fn(). + + Run the Rule against the object and return the results. The result + could be an Exception message where the Rule cannot be run against + the object for whatever reason. If an exception doesn't occur, the + result is linting success or failure. + + Args: + obj: + The object the Rule is being run against + + Returns: + A LintFinding object that contains the message to print to the user + and a LintLevel that contains the level of error to calculate the + exit code with. + """ message = None if type(obj) not in self.compatibility: @@ -34,7 +74,7 @@ def execute(self, obj: Union[Workflow, Job, Step]) -> Union[LintFinding, None]: f"{type(obj).__name__} not compatible with {type(self).__name__}", obj, ), - "error", + LintLevels.ERROR, ) try: @@ -47,7 +87,7 @@ def execute(self, obj: Union[Workflow, Job, Step]) -> Union[LintFinding, None]: self.build_lint_message( f"failed to apply {type(self).__name__}\n{err}", obj ), - "error", + LintLevels.ERROR, ) return LintFinding(self.build_lint_message(message, obj), self.on_fail) diff --git a/lint-workflow/src/rules/job_environment_prefix.py b/lint-workflow/src/rules/job_environment_prefix.py index 6266a26d..95748277 100644 --- a/lint-workflow/src/rules/job_environment_prefix.py +++ b/lint-workflow/src/rules/job_environment_prefix.py @@ -1,18 +1,41 @@ -from typing import Tuple +from typing import Union, Tuple from ..rule import Rule from ..models.job import Job -from ..utils import Settings +from ..models.workflow import Workflow +from ..models.step import Step +from ..utils import LintLevels, Settings class RuleJobEnvironmentPrefix(Rule): def __init__(self, settings: Settings = None) -> None: - self.message = f"Job Environment vars should start with and underscore:" - self.on_fail = "error" - self.compatibility = [Job] - self.settings = settings + self.message: str = f"Job Environment vars should start with and underscore:" + 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]: + """Enforces the underscore prefix standard on job envs. + + Example: + --- + on: + workflow_dispatch: + + jobs: + job-key: + runs-on: ubuntu-22.04 + env: + _TEST_ENV: "test" + steps: + - run: echo test + + All keys under jobs.job-key.env should be prefixed with an underscore + as in _TEST_ENV. + + See tests/rules/test_job_environment_prefix.py for examples of + incorrectly names environment variables. + """ correct = True offending_keys = [] diff --git a/lint-workflow/src/rules/name_capitalized.py b/lint-workflow/src/rules/name_capitalized.py index 4926954e..e66417c0 100644 --- a/lint-workflow/src/rules/name_capitalized.py +++ b/lint-workflow/src/rules/name_capitalized.py @@ -4,16 +4,39 @@ from ..models.workflow import Workflow from ..models.job import Job from ..models.step import Step -from ..utils import Settings +from ..utils import LintLevels, Settings class RuleNameCapitalized(Rule): def __init__(self, settings: Settings = None) -> None: self.message = "name must capitalized" - self.on_fail = "error" - self.settings = settings + self.on_fail: LintLevels = LintLevels.ERROR + self.settings: Settings = settings def fn(self, obj: Union[Workflow, Job, Step]) -> Tuple[bool, str]: + """Enforces capitalization of the first letter of any name key. + + Example: + --- + name: Test Workflow + + on: + workflow_dispatch: + + jobs: + job-key: + name: Test + runs-on: ubuntu-latest + steps: + - name: Test + run: echo test + + 'Test Workflow', 'Test', and 'Test' all start with a capital letter. + + See tests/rules/test_name_capitalized.py for examples of incorrectly + capitalized names. This Rule DOES NOT enforce that the name exists. + It only enforces capitalization IF it does. + """ if obj.name: return obj.name[0].isupper(), self.message return True, "" # Force passing if obj.name doesn't exist diff --git a/lint-workflow/src/rules/name_exists.py b/lint-workflow/src/rules/name_exists.py index 8fe3e60a..165038b0 100644 --- a/lint-workflow/src/rules/name_exists.py +++ b/lint-workflow/src/rules/name_exists.py @@ -4,16 +4,38 @@ from ..models.workflow import Workflow from ..models.job import Job from ..models.step import Step -from ..utils import Settings +from ..utils import LintLevels, Settings class RuleNameExists(Rule): def __init__(self, settings: Settings = None) -> None: self.message = "name must exist" - self.on_fail = "error" - self.settings = settings + self.on_fail: LintLevels = LintLevels.ERROR + self.settings: Settings = settings def fn(self, obj: Union[Workflow, Job, Step]) -> Tuple[bool, str]: + """Enforces the existence of names. + + Example: + --- + name: Test Workflow + + on: + workflow_dispatch: + + jobs: + job-key: + name: Test + runs-on: ubuntu-latest + steps: + - name: Test + run: echo test + + 'Test Workflow', 'Test', and 'Test' all exist. + + See tests/rules/test_name_exists.py for examples where a name does not + exist. + """ if obj.name is not None: return True, "" return False, self.message diff --git a/lint-workflow/src/rules/pinned_job_runner.py b/lint-workflow/src/rules/pinned_job_runner.py index d4ce92a9..d2897f48 100644 --- a/lint-workflow/src/rules/pinned_job_runner.py +++ b/lint-workflow/src/rules/pinned_job_runner.py @@ -1,18 +1,35 @@ -from typing import Tuple +from typing import Union, Tuple from ..rule import Rule from ..models.job import Job -from ..utils import Settings +from ..models.workflow import Workflow +from ..models.step import Step +from ..utils import LintLevels, Settings class RuleJobRunnerVersionPinned(Rule): def __init__(self, settings: Settings = None) -> None: self.message = "Workflow runner must be pinned" - self.on_fail = "error" - self.compatibility = [Job] - self.settings = settings + 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]: + """Enforces runners are pinned to a version + + Example: + --- + on: + workflow_dispatch: + + jobs: + job-key: + runs-on: ubuntu-22.04 + steps: + - run: echo test + + 'runs-on' is pinned to '22.04' instead of 'latest' + """ if "latest" not in obj.runs_on: return True, "" return False, self.message diff --git a/lint-workflow/src/rules/step_approved.py b/lint-workflow/src/rules/step_approved.py index 9cfe4ad9..88a6673b 100644 --- a/lint-workflow/src/rules/step_approved.py +++ b/lint-workflow/src/rules/step_approved.py @@ -1,18 +1,25 @@ -from typing import Tuple +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 Settings +from ..utils import LintLevels, Settings class RuleStepUsesApproved(Rule): def __init__(self, settings: Settings = None) -> None: self.message = f"error" - self.on_fail = "warn" - self.compatibility = [Step] - self.settings = settings + self.on_fail: LintLevels = LintLevels.WARNING + self.compatibility: List[Union[Workflow, Job, Step]] = [Step] + self.settings: Settings = settings - def force_pass(self, obj: Step) -> bool: + def skip(self, obj: Step) -> bool: + """Skip this Rule on some Steps. + + This Rule does not apply to a few types of Steps. These + Rules are skipped. + """ ## Force pass for any shell steps if not obj.uses: return True, "" @@ -28,7 +35,38 @@ def force_pass(self, obj: Step) -> bool: return False def fn(self, obj: Step) -> Tuple[bool, str]: - if self.force_pass(obj): + """Enforces all externally used Actions are on the pre-approved list. + + The pre-approved list allows tight auditing on what Actions are trusted + and allowed to be run in our environments. This helps mitigate risks + against supply chain attacks in our pipelines. + + Example: + --- + on: + workflow_dispatch: + + jobs: + job-key: + runs-on: ubuntu-22.04 + steps: + - name: Checkout Branch + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + + - name: Test Bitwarden Action + uses: bitwarden/gh-actions/get-keyvault-secrets@main + + - name: Test Local Action + uses: ./actions/test-action + + - name: Test Run Action + run: echo "test" + + In this example, 'actions/checkout' must be on the pre-approved list + and the metadata must match in order to succeed. The other three + Steps will be skipped. + """ + if self.skip(obj): return True, "" path, hash = obj.uses.split("@") diff --git a/lint-workflow/src/rules/step_pinned.py b/lint-workflow/src/rules/step_pinned.py index 2899b9ee..a2fe2eee 100644 --- a/lint-workflow/src/rules/step_pinned.py +++ b/lint-workflow/src/rules/step_pinned.py @@ -1,18 +1,25 @@ -from typing import Tuple +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 Settings +from ..utils import LintLevels, Settings class RuleStepUsesPinned(Rule): def __init__(self, settings: Settings = None) -> None: self.message = f"error" - self.on_fail = "error" - self.compatibility = [Step] - self.settings = settings + self.on_fail: LintLevels = LintLevels.ERROR + self.compatibility: List[Union[Workflow, Job, Step]] = [Step] + self.settings: Settings = settings - def force_pass(self, obj: Step) -> bool: + def skip(self, obj: Step) -> bool: + """Skip this Rule on some Steps. + + This Rule does not apply to a few types of Steps. These + Rules are skipped. + """ if not obj.uses: return True, "" @@ -23,7 +30,33 @@ def force_pass(self, obj: Step) -> bool: return False def fn(self, obj: Step) -> Tuple[bool, str]: - if self.force_pass(obj): + """Enforces all Actions to be pinned in a specific way. + + Pinning external Action hashes prevents unknown updates that could + break the pipelines or be the entry point to a supply chain attack. + + Pinning internal Actions to branches allow for less updates as work + is done on those repos. This is mainly to support our Action + monorepo architecture of our Actions. + + Example: + - name: Checkout Branch + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + + - name: Test Bitwarden Action + uses: bitwarden/gh-actions/get-keyvault-secrets@main + + - name: Test Local Action + uses: ./actions/test-action + + - name: Test Run Action + run: echo "test" + + In this example, 'actions/checkout' must be pinned to the full commit + of the tag while 'bitwarden/gh-actions/get-keyvault-secrets' must be + pinned to 'main'. The other two Steps will be skipped. + """ + if self.skip(obj): return True, "" path, ref = obj.uses.split("@") diff --git a/lint-workflow/src/utils.py b/lint-workflow/src/utils.py index 74f8d34e..77b6ae47 100644 --- a/lint-workflow/src/utils.py +++ b/lint-workflow/src/utils.py @@ -1,4 +1,5 @@ -from dataclasses import dataclass +from dataclasses import asdict, dataclass +from enum import Enum from typing import Self @@ -17,20 +18,56 @@ class Colors: @dataclass +class LintLevel: + """Class to contain the numeric level and color of linting.""" + code: int + color: Colors + + +class LintLevels(LintLevel, Enum): + """Collection of the different types of LintLevels available.""" + NONE = 0, Colors.white + WARNING = 1, Colors.yellow + ERROR = 2, Colors.red + + class LintFinding: - """Represents a linting problem.""" + """Represents a problem detected by linting.""" + + def __init__( + self, + description: str = "", + level: LintLevel = None + ) -> None: + self.description = description + self.level = level - description: str = "" - level: str = None + def __str__(self) -> str: + """String representation of the class. + + Returns: + String representation of itself. + """ + return f"\033[{self.level.color}{self.level.name.lower()}\033[0m {self.description}" @dataclass class Action: + """Collection of the metadata associated with a GitHub Action.""" name: str version: str = "" sha: str = "" def __eq__(self, other: Self) -> bool: + """Override Action equality. + + Args: + other: + Another Action type object to compare + + Return + The state of eqaulity + """ return ( self.name == other.name and self.version == other.version @@ -38,23 +75,42 @@ def __eq__(self, other: Self) -> bool: ) def __ne__(self, other: Self) -> bool: + """Override Action unequality. + + Args: + other: + Another Action type object to compare + + Return + The negation of the state of eqaulity + """ return not self.__eq__(other) class SettingsError(Exception): + """Custom Exception to indicate an error with loading Settings.""" pass -@dataclass class Settings: enabled_rules: list[str] approved_actions: dict[str, Action] def __init__( self, - enabled_rules: list[str] = None, - approved_actions: dict[str, dict[str, str]] = None, + enabled_rules: list[str] = [], + approved_actions: dict[str, dict[str, str]] = {}, ): + """Settings object that can be overriden in settings.py. + + Args: + enabled_rules: + All of the python modules that implement a Rule to be run against + the workflows. These must be available somewhere on the PYTHONPATH + approved_actions: + The colleciton of GitHub Actions that are pre-approved to be used + in any workflow (Required by src.rules.step_approved) + """ self.enabled_rules = enabled_rules self.approved_actions = { name: Action(**action) for name, action in approved_actions.items() diff --git a/lint-workflow/tests/test_lint.py b/lint-workflow/tests/test_lint.py new file mode 100644 index 00000000..277aa374 --- /dev/null +++ b/lint-workflow/tests/test_lint.py @@ -0,0 +1,34 @@ +import json +import pytest + +from .conftest import FIXTURE_DIR +from .context import src + +from src.lint import LinterCmd +from src.utils import Settings, LintFinding, LintLevels + + +@pytest.fixture +def settings(): + return Settings() + + +def test_get_max_error_level(settings): + linter = LinterCmd(settings=settings) + + assert linter.get_max_error_level([ + LintFinding(level=LintLevels.WARNING), + LintFinding(level=LintLevels.WARNING) + ]) == 1 + + assert linter.get_max_error_level([ + LintFinding(level=LintLevels.ERROR), + LintFinding(level=LintLevels.ERROR) + ]) == 2 + + assert linter.get_max_error_level([ + LintFinding(level=LintLevels.ERROR), + LintFinding(level=LintLevels.ERROR), + LintFinding(level=LintLevels.WARNING), + LintFinding(level=LintLevels.WARNING) + ]) == 2 diff --git a/lint-workflow/tests/test_utils.py b/lint-workflow/tests/test_utils.py index e9b667c8..77830eb7 100644 --- a/lint-workflow/tests/test_utils.py +++ b/lint-workflow/tests/test_utils.py @@ -7,7 +7,7 @@ from .conftest import FIXTURE_DIR from .context import src -from src.utils import Action +from src.utils import Action, Colors, LintFinding, LintLevels def test_action_eq(): @@ -38,3 +38,17 @@ def test_action_ne(): assert (action_a == action_b) == False assert (action_a != action_b) == True + + +def test_lint_level(): + warning = LintLevels.WARNING + assert warning.code == 1 + assert warning.color == Colors.yellow + + +def test_lint_finding(): + warning = LintFinding(level=LintLevels.WARNING) + assert str(warning) == '\x1b[33mwarning\x1b[0m ' + + error = LintFinding(level=LintLevels.ERROR) + assert str(error) == '\x1b[31merror\x1b[0m '