Repository navigation
docs: update learn page statistics and add CI/CD workflow - #230
Conversation
528d756 to
dcf7cd4
Compare
|
CI is failing @parthrohit22 - have a look at it! |
dcf7cd4 to
22b3900
Compare
FIXED . THANK YOU |
22b3900 to
6ed2530
Compare
|
@ritiksah141 , review this PR pls, since your approved it earlier and @parthrohit22 has done some new force pushes. |
|
@Vishnu2707 I approved this PR as it is related to the his own learns website and doesn't affect the core of the repo and mainly his own seperate deployment |
6ed2530 to
ab28e01
Compare
|
Rebased onto The shift came from AZ-KV-006 landing in #235 after this branch was cut. The script picked it up on its own — KeyVault moved 5 → 6 in the category chart, MEDIUM 22 → 23, both totals 65 → 66, no hand-editing anywhere. That's the behaviour the issue was asking for, so it was a decent accidental test of it. Everything from the earlier round is in:
Updated the PR body and #228 to match the current numbers. One thing I'd like a second opinion on before this goes in. The workflow commits straight to @Vishnu2707 could you check Settings → Branches and say which you'd prefer? Happy to switch it to the PR-based flow if that's the safer default — it's a small change and I'd rather do it now than find out from a red X. Known limitation, calling it out rather than leaving it quiet. The severity grid is a hardcoded three-column HIGH/MEDIUM/LOW layout, so the one CRITICAL rule (AZ-SC-005) has nowhere to render. At 66 rules the three boxes sum to 65, which is visible to anyone who adds them up. I'd rather not widen this PR any further — happy to add a fourth box or relabel the grid as a follow-up, whichever reads better to you. Two things found while working on this, both filed separately rather than folded in:
|
|
@ritiksah141 - review this pls. |
ritiksah141
left a comment
There was a problem hiding this comment.
Blocking issue — .github/workflows/update-learn-page.yml:47
actions/checkout leaves the runner in a detached HEAD state by default. As a result, the final bare git push has no current branch/upstream and will fail with fatal: You are not currently on a branch. The workflow therefore cannot publish the refreshed documentation after a push to dev.
Please either check out the branch explicitly or push the detached commit to the target branch, for example:
git push origin HEAD:${{ github.ref_name }}Please verify the commit-and-push path in CI before merging.
|
@parthrohit22 , the conflicts and requested changes needs to be addressed to go ahead with the merge. |
ab28e01 to
c368685
Compare
Ready for reviewPR #230 has been updated with the review follow-up:
Validation completed:
Review commits: |
TFT444
left a comment
There was a problem hiding this comment.
Great work @parthrohit22, the fail-loud substitution tracking and idempotent design are exactly right. One blocker:
- Rules/playbooks mismatch: the regenerated docs now say 95 rules / 96 playbooks, but the README row still claims "Every rule ships with a matching playbook". Cause:
playbooks/cli/review_enterprise_resilience.shis an orphan no rule references. Your "0 orphans" check was true at 66/66 but dev moved. Please add a parity check to the script (fail/warn on rule count != playbook count), since this is exactly the drift class this PR exists to catch.
Two nits, same push if you like: collect_rule_stats() counts files without RULE_ID while count_rules() excludes them, so the severity boxes can disagree with the headline total; and html.escape(name) in render_category_rows() would be cheap insurance.
Fix the parity issue and this is good to go from my side.
36615e1 to
780113a
Compare
|
@TFT444 Updated:
Ran the updater against the current tree -- clean exit, no further diff. Ruff check/format clean. CI is green including CI Summary. Latest commit: |
|
@parthrohit22, all good just need to be rebased once the #317 Lands with the dev and good to go |
780113a to
9a84012
Compare
|
@ritiksah141 Rebased onto the latest dev (9a84012). Ran the generator afterward - no drift, statistics are already current. Full suite and ruff still clean. One CI note so it's not mistaken for something new: |
Signed-off-by: Parth Rohit <parthrohit60@gmail.com>
update-learn-page.yml: actions/checkout leaves the runner in detached HEAD, so the final `git push` had no upstream and would fail with "You are not currently on a branch". Push HEAD to the triggering ref by name instead. Rebased onto dev and re-ran update_learn_page.py against the current rule set rather than hand-editing numbers that would be stale again immediately. The README feature-table regex still had the old rule-category wording from before this branch was cut, so it failed loud as designed instead of silently no-op'ing; updated it to match the current sentence. Removed the "known cleanup item" note about KeyVault vs Key Vault naming now that the category chart renders the declared category string directly, and corrected the Known Gaps bullet that still said README.md counts were updated by hand, now that render_readme() covers both files. Signed-off-by: Parth Rohit <parthrohit60@gmail.com>
github-advanced-security flagged interpolating ${{ github.ref_name }}
directly into the Commit and push run: script as a potential
script-injection pattern. Pass it through an env var instead and
reference it as a shell variable, which keeps the same behavior
(push HEAD to the triggering ref) without templating untrusted
workflow context straight into a shell command.
Signed-off-by: Parth Rohit <parthrohit60@gmail.com>
Signed-off-by: PARTH J ROHIT <parthrohit60@gmail.com>
Signed-off-by: PARTH J ROHIT <parthrohit60@gmail.com>
count_playbooks() globbed every *.sh in playbooks/cli/, which also picked up review_enterprise_resilience.sh -- a shared helper several fix_*.sh wrappers exec into, not a playbook any single rule owns. That inflated the count past rule_count (95 rules / 96 playbooks) and broke the "every rule ships with a matching playbook" claim in the README. find_matching_playbooks() now mirrors the fix_<rule filename stem>.sh convention ci.yml's playbook_check step already enforces, derives the count from actual rule-to-playbook matches, and fails loudly if a rule has no matching playbook or the two counts disagree. Unmatched files (shared helpers) are reported as a non-fatal warning instead of silently inflating the total. Also: collect_rule_stats() now skips files with no parseable RULE_ID, matching count_rules()'s own filter, so the severity-box totals can no longer disagree with the headline rule count; and render_category_rows() HTML-escapes the category name before embedding it. Regenerated docs/learn/index.html and README.md: 95 rules / 95 playbooks. Signed-off-by: PARTH J ROHIT <parthrohit60@gmail.com>
9a84012 to
7633665
Compare
TFT444
left a comment
There was a problem hiding this comment.
The requested fixes are addressed and the current checks pass. Approving.
|
@parthrohit22, this is approved and green, but it is now two commits behind |
What does this PR do?
Closes out the remaining drift documented in #228. Fixes every stale count on the Learn page
(not just the five the first pass covered), fixes the four stale counts in
README.md, andextends
.github/scripts/update_learn_page.py/.github/workflows/update-learn-page.ymlsoboth files are regenerated from the codebase on every push to
devand can no longer drift.Type of change
Rule details (if applicable)
Not applicable — this PR adds no scanner rule. It only reads rule metadata to produce counts.
Testing
Verification actually performed:
ruff check .andruff format --check .— clean repo-wide.python -m bandit -r .github/scripts/— no issues.python -c "import yaml; yaml.safe_load(open('.github/workflows/update-learn-page.yml'))"— valid.ls scanner/rules/az_*.py | wc -l→ 65,ls playbooks/cli/*.sh | wc -l→ 65,grep -l 'SEVERITY = "HIGH"' scanner/rules/az_*.py | wc -l→ 38.Cross-checked every rule's declared
PLAYBOOKfield against the filename-derived path — 0 orphanseither direction.
docs/learn/index.htmlandREADME.mdto their pre-PRcommitted content, ran the script — output was byte-identical to the version already staged
in this PR; a second run was a true no-op (
git diffempty).match — the script correctly named exactly that pattern and exited 1, and confirmed no file
was written on that failure.
RULES_DIR/PLAYBOOKS_DIRat an empty tempdirectory —
main()returned 1 and refused to write zeroes.SEVERITY/CATEGORYwarning path, tested in isolation with synthetic rule files —correctly named the offending file without failing the run.
python scripts/check_dco.py <base> <head>)against this branch — sign-off verified.
Related issue
Closes #228
Checklist
Signed-off-bytrailer (git commit -s; seedocs/dco.md)repo's Ruff/PEP 8 and type-hint standards
docs/update-learn-page-and-add-cicd, matchingthe
docs/descriptionform in CONTRIBUTING.mdTrue numbers (derived from the filesystem, verified before any edit)
scanner/rules/az_*.pywithRULE_ID)playbooks/cli/*.sh)Rebased onto
devafter AZ-KV-006 merged in #235; the script picked the new rule up on its ownand moved KeyVault 5 → 6, MEDIUM 22 → 23 and both totals 65 → 66 with no hand-editing. That is
the drift-resistance this PR is for, so the numbers above will keep moving as rules land.
The 1 CRITICAL rule (
AZ-SC-005) has no box in the Learn page's severity-distribution UI,which is hardcoded to a 3-column HIGH/MEDIUM/LOW grid — documented as a known limitation, not
fixed here (see "Notes for reviewers").
Changes
docs/learn/index.html39 → 66 dynamic checks; severity boxes HIGH22 → 38, MEDIUM13 → 23(LOW was already correct at 4); "Coverage by category" chart fully regenerated — previously omitted Supply Chain (8) and Kubernetes (6) entirely and undercounted Identity4 → 15; removed a "known cleanup item" note aboutKeyVaultvsKey Vaultnaming that the chart fix resolves (the chart now renders the category string the code actually declares); rewrote the "Known gaps" card's "counts are checked and updated with each release" claim to accurately describe what's CI-generated vs. hand-maintained.README.md51 → 66, and added the previously-unmentioned "supply chain" category), Remediation Playbooks row (51 → 66), and both Mermaid diagram nodes (Scanner Engine,Azure CLI Playbooks)..github/scripts/update_learn_page.pyrender_readme()so the same script now also fixes README.md. Every substitution is tracked viare.subn; if any pattern matches zero times the script prints which one and exits non-zero instead of silently doing nothing and exiting 0. Rule files with no parseableSEVERITY/CATEGORYare now warned about by name. Still idempotent..github/workflows/update-learn-page.ymlgit add, and commitREADME.mdalongsidedocs/learn/index.html. Renamed to reflect the wider scope. SHA pins andpermissions: contents: writeunchanged from the first pass.Decision: README.md is now automated, not left as a manual step
The task called for deciding — and stating clearly — whether to extend the workflow to also
keep
README.mdcurrent, or leave that update as a one-time manual fix. I extended it: thewhole point of this issue is closing the exact class of drift where a doc's numbers silently
fall out of sync with the codebase, and
README.md's 4 stale counts are the same failure modein a second file. The substitution surface is small and well-bounded (2 feature-table rows, 2
Mermaid nodes), reuses the same tested
apply_replacements()/fail-loud machinery already builtfor the Learn page, and is the only choice consistent with the "Known gaps" card now stating
that these stats are CI-generated.
README.md's "30+ scan rules" roadmap checklist item andother prose are unrelated to this PR's scope and were left untouched.
Notes for reviewers
every stale location by file/line and the true numbers — see the issue thread.
scanner/engine.py::load_rules()— the code that actually executes rules at scan time —discovers rule modules with
RULES_DIR.glob("*.py"), excluding only filenames starting with_. It does not require theaz_*.pyprefix or aRULE_ID, unlikeci.yml'sstructure-validation job and this script, which both use the narrower convention. Today all
three agree on 65 only because
scanner/rules/happens to contain no stray file outsidethose two patterns — nothing enforces that. A leftover or misnamed
.pyfile with ascan()function would be loaded and executed in production without being validated by CI or counted
in either doc.
dev, from the first review round: I don't have admin visibilityinto the repo's protection rules (only push access), so I can't fully confirm whether
GITHUB_TOKENcan push directly todev.GET /repos/.../rules/branches/dev(readable withplain read access) currently returns no effective rules, which suggests direct pushes are not
blocked, but please verify in Settings → Branches before relying on that. If pushes are
blocked, this workflow needs to open a PR instead of committing directly — I have not made
that change silently.