-
Notifications
You must be signed in to change notification settings - Fork 2
fix: Suppress bare .svc hostnames and pin lychee version #25
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
Changes from 4 commits
79d970a
91afe9c
bc7feab
c0eb93a
91cad02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| # extract-broken-links.sh — Turn lychee JSON output into broken-link records. | ||
| # | ||
| # Reads a lychee --format json report and emits one JSON object per broken | ||
| # link (JSONL, one record per line). URLs pointing at unreachable-by-design | ||
| # hostnames (Kubernetes cluster-local .svc names, *.local, RFC1918 ranges) are | ||
| # suppressed, since an external scanner can never reach them. lychee status is | ||
| # normalized to enum tokens: numeric HTTP codes stay as-is; text statuses map | ||
| # to timeout / dns / unreachable / error / unknown. | ||
| # | ||
| # This logic was previously inline in link-health-scanner.sh; it is extracted | ||
| # here so it can be unit-tested against synthetic lychee fixtures. | ||
| # | ||
| # Usage: | ||
| # extract-broken-links.sh <lychee-json> <repo-name> <repos-prefix> | ||
| # cat report.json | extract-broken-links.sh - <repo-name> <repos-prefix> | ||
| # | ||
| # Arguments: | ||
| # <lychee-json> Path to a lychee JSON report, or - to read from stdin. | ||
| # <repo-name> Bare repo name (e.g. "kagenti"); emitted as "kagenti/<name>". | ||
| # <repos-prefix> Absolute path prefix stripped from lychee's file keys | ||
| # (e.g. "/home/claw/kagenti/kagenti/"). | ||
| # | ||
| # Output (stdout): JSONL, zero or more objects of the form | ||
| # {"repo":"kagenti/foo","file":"docs/x.md","url":"https://...","status":"404","category":"external"} | ||
| # | ||
| # Exit codes: | ||
| # 0 - success (may emit zero records) | ||
| # 1 - usage error (missing arguments or file not found) | ||
|
|
||
| # --- Argument handling --- | ||
| if [ $# -lt 3 ]; then | ||
| echo "Usage: extract-broken-links.sh <lychee-json> <repo-name> <repos-prefix>" >&2 | ||
| echo " cat report.json | extract-broken-links.sh - <repo-name> <repos-prefix>" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| LYCHEE_INPUT="$1" | ||
| REPO_NAME="$2" | ||
| REPOS_PREFIX="$3" | ||
|
|
||
| if [ "$LYCHEE_INPUT" != "-" ] && [ ! -f "$LYCHEE_INPUT" ]; then | ||
| echo "ERROR: File not found: $LYCHEE_INPUT" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Empty input produces no records. | ||
| if [ "$LYCHEE_INPUT" != "-" ] && [ ! -s "$LYCHEE_INPUT" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # --- Extract broken links from error_map (skip non-URL entries like "Error building URL") --- | ||
| # Normalize lychee status to enum tokens: numeric codes stay as-is, | ||
| # text statuses map to: timeout, dns, unreachable, error, unknown. | ||
| # Suppress URLs with unreachable-by-design hostnames (cluster-local, .local, RFC1918). | ||
| jq -r --arg repo "$REPO_NAME" --arg repos_dir "$REPOS_PREFIX" ' | ||
| .error_map // {} | to_entries[] | | ||
| .key as $filepath | | ||
| .value[] | | ||
| select(.url | test("^https?://")) | | ||
| # Suppress unreachable-by-design hostnames at URL level. | ||
| # Bare .svc covers both plain cluster-local service names (foo.ns.svc) and | ||
| # the fully-qualified foo.ns.svc.cluster.local form. | ||
| select(.url | test("://[^/]*\\.svc([:/]|$)") | not) | | ||
| select(.url | test("://[^/]*\\.svc\\.cluster\\.local([:/]|$)") | not) | | ||
| select(.url | test("://[^/]*\\.local([:/]|$)") | not) | | ||
| select(.url | test("://(10\\.[0-9]|172\\.(1[6-9]|2[0-9]|3[01])\\.[0-9]|192\\.168\\.[0-9])[0-9.]*([:/]|$)") | not) | | ||
| (.status.code // .status.text // null) as $raw_status | | ||
| ( | ||
| if $raw_status == null then "unknown" | ||
| elif ($raw_status | type) == "number" then ($raw_status | tostring) | ||
| elif ($raw_status | test("^[0-9]{3}$")) then $raw_status | ||
| elif ($raw_status | ascii_downcase | test("timeout")) then "timeout" | ||
| elif ($raw_status | ascii_downcase | test("resolve|dns")) then "dns" | ||
| elif ($raw_status | ascii_downcase | test("refused|reset|closed|unreachable|connect")) then "unreachable" | ||
| else "error" | ||
| end | ||
| ) as $status | | ||
| { | ||
| repo: ("kagenti/" + $repo), | ||
| file: ($filepath | ltrimstr($repos_dir) | ltrimstr("./")), | ||
| url: .url, | ||
| status: $status, | ||
| category: ( | ||
| if (.url | test("github\\.com/kagenti")) then "internal" | ||
| else "external" | ||
| end | ||
| ) | ||
| } | ||
| ' "${LYCHEE_INPUT/#-//dev/stdin}" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,163 @@ | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| # Test harness for extract-broken-links.sh | ||
| # Run: bash automation/tests/test-extract-broken-links.sh | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: The usage comment on line 5 says
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a stdin ( |
||
| # Exit code 0 = all tests pass, 1 = at least one failure. | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| EXTRACTOR="$SCRIPT_DIR/../scripts/extract-broken-links.sh" | ||
| TEST_TMPDIR=$(mktemp -d "/tmp/test-extract-broken-XXXXXX") | ||
| trap 'rm -rf "$TEST_TMPDIR"' EXIT | ||
|
|
||
| REPO="kagenti-extensions" | ||
| REPOS_PREFIX="/home/claw/kagenti/kagenti-extensions/" | ||
|
|
||
| PASS=0 | ||
| FAIL=0 | ||
|
|
||
| # --- Helper: write a lychee-shaped JSON fixture with a single error_map entry --- | ||
| # Args: <out-file> <filepath-key> <url> <status-json> | ||
| # status-json is the raw .status object, e.g. '{"text":"Network error"}' or '{"code":404}'. | ||
| write_fixture() { | ||
| local out="$1" filepath="$2" url="$3" status="$4" | ||
| jq -n \ | ||
| --arg fp "$filepath" \ | ||
| --arg url "$url" \ | ||
| --argjson status "$status" \ | ||
| '{ error_map: { ($fp): [ { url: $url, status: $status } ] } }' \ | ||
| > "$out" | ||
| } | ||
|
|
||
| # --- Helper: run extractor on a fixture, apply a jq check to the JSONL output --- | ||
| # Args: <name> <fixture-file> <jq-check> <expected> | ||
| # The extractor emits zero-or-more JSON objects (one per line); we slurp them. | ||
| run_test() { | ||
| local name="$1" fixture="$2" jq_check="$3" expected="$4" | ||
|
|
||
| local output actual | ||
| output=$("$EXTRACTOR" "$fixture" "$REPO" "$REPOS_PREFIX") | ||
| actual=$(printf '%s\n' "$output" | jq -s -r "$jq_check") | ||
|
|
||
| if [ "$actual" = "$expected" ]; then | ||
| echo " PASS: $name" | ||
| PASS=$((PASS + 1)) | ||
| else | ||
| echo " FAIL: $name" | ||
| echo " Expected: $expected" | ||
| echo " Got: $actual" | ||
| FAIL=$((FAIL + 1)) | ||
| fi | ||
| } | ||
|
|
||
| # ============================================================================= | ||
| # Fix #1: bare .svc hostnames must be suppressed | ||
| # ============================================================================= | ||
| echo "Test 1: bare .svc URL is suppressed" | ||
| write_fixture "$TEST_TMPDIR/svc.json" \ | ||
| "authbridge/demos/github-issue/demo-aiac.md" \ | ||
| "http://keycloak-service.keycloak.svc:8080/realms/" \ | ||
| '{"text":"Network error"}' | ||
| run_test "bare .svc suppressed (0 records)" "$TEST_TMPDIR/svc.json" 'length' '0' | ||
|
|
||
| # ============================================================================= | ||
| # Regressions: previously-suppressed classes stay suppressed | ||
| # ============================================================================= | ||
| echo "Test 2: .svc.cluster.local still suppressed" | ||
| write_fixture "$TEST_TMPDIR/clusterlocal.json" \ | ||
| "docs/a.md" \ | ||
| "http://svc.foo.svc.cluster.local:8080/x" \ | ||
| '{"text":"Network error"}' | ||
| run_test ".svc.cluster.local suppressed" "$TEST_TMPDIR/clusterlocal.json" 'length' '0' | ||
|
|
||
| echo "Test 3: .local still suppressed" | ||
| write_fixture "$TEST_TMPDIR/local.json" \ | ||
| "docs/b.md" \ | ||
| "http://my-box.local/status" \ | ||
| '{"text":"Network error"}' | ||
| run_test ".local suppressed" "$TEST_TMPDIR/local.json" 'length' '0' | ||
|
|
||
| echo "Test 4: RFC1918 ranges still suppressed" | ||
| write_fixture "$TEST_TMPDIR/rfc10.json" "docs/c.md" \ | ||
| "http://10.20.4.11:9090/api" '{"text":"Network error"}' | ||
| run_test "10.x suppressed" "$TEST_TMPDIR/rfc10.json" 'length' '0' | ||
|
|
||
| write_fixture "$TEST_TMPDIR/rfc172.json" "docs/c.md" \ | ||
| "http://172.16.0.5/api" '{"text":"Network error"}' | ||
| run_test "172.16-31.x suppressed" "$TEST_TMPDIR/rfc172.json" 'length' '0' | ||
|
|
||
| write_fixture "$TEST_TMPDIR/rfc192.json" "docs/c.md" \ | ||
| "http://192.168.1.1/api" '{"text":"Network error"}' | ||
| run_test "192.168.x suppressed" "$TEST_TMPDIR/rfc192.json" 'length' '0' | ||
|
|
||
| # ============================================================================= | ||
| # Genuine broken links must still pass through with correct fields | ||
| # ============================================================================= | ||
| echo "Test 5: genuine external broken link passes through" | ||
| write_fixture "$TEST_TMPDIR/ext.json" \ | ||
| "docs/d.md" \ | ||
| "https://example.invalid/gone" \ | ||
| '{"code":404}' | ||
| run_test "external record count" "$TEST_TMPDIR/ext.json" 'length' '1' | ||
| run_test "external status normalized" "$TEST_TMPDIR/ext.json" '.[0].status' '404' | ||
| run_test "external category" "$TEST_TMPDIR/ext.json" '.[0].category' 'external' | ||
| run_test "external repo prefixed" "$TEST_TMPDIR/ext.json" '.[0].repo' 'kagenti/kagenti-extensions' | ||
| run_test "external file prefix stripped" "$TEST_TMPDIR/ext.json" '.[0].file' 'docs/d.md' | ||
| run_test "external url preserved" "$TEST_TMPDIR/ext.json" '.[0].url' 'https://example.invalid/gone' | ||
|
|
||
| echo "Test 6: kagenti GitHub URL is category internal" | ||
| write_fixture "$TEST_TMPDIR/int.json" \ | ||
| "docs/e.md" \ | ||
| "https://github.com/kagenti/kagenti/blob/main/missing.md" \ | ||
| '{"code":404}' | ||
| run_test "internal category" "$TEST_TMPDIR/int.json" '.[0].category' 'internal' | ||
|
|
||
| echo "Test 7: text status normalized to unreachable" | ||
| write_fixture "$TEST_TMPDIR/unreach.json" \ | ||
| "docs/f.md" \ | ||
| "https://real-host.example.org/x" \ | ||
| '{"text":"Connection refused"}' | ||
| run_test "unreachable status token" "$TEST_TMPDIR/unreach.json" '.[0].status' 'unreachable' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: The
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in 91cad02: fixtures for the timeout, dns (both |
||
|
|
||
| # ============================================================================= | ||
| # Edge cases: empty / missing error_map yields no output; valid JSONL | ||
| # ============================================================================= | ||
| echo "Test 8: missing error_map yields no output" | ||
| echo '{"total":5,"errors":0}' > "$TEST_TMPDIR/noerrors.json" | ||
| run_test "no error_map -> empty" "$TEST_TMPDIR/noerrors.json" 'length' '0' | ||
|
|
||
| echo "Test 9: empty error_map yields no output" | ||
| echo '{"error_map":{}}' > "$TEST_TMPDIR/emptymap.json" | ||
| run_test "empty error_map -> empty" "$TEST_TMPDIR/emptymap.json" 'length' '0' | ||
|
|
||
| echo "Test 10: non-URL error entries are skipped" | ||
| jq -n '{ error_map: { "docs/g.md": [ { url: "Error building URL", status: {text:"Invalid"} } ] } }' \ | ||
| > "$TEST_TMPDIR/nonurl.json" | ||
| run_test "non-URL entry skipped" "$TEST_TMPDIR/nonurl.json" 'length' '0' | ||
|
|
||
| # ============================================================================= | ||
| # Stdin path: reading the report via "-" behaves like reading from a file | ||
| # ============================================================================= | ||
| echo "Test 11: stdin (-) path works" | ||
| write_fixture "$TEST_TMPDIR/stdin.json" \ | ||
| "docs/h.md" \ | ||
| "https://example.invalid/stdin-gone" \ | ||
| '{"code":404}' | ||
| STDIN_URL=$(cat "$TEST_TMPDIR/stdin.json" | "$EXTRACTOR" - "$REPO" "$REPOS_PREFIX" | jq -s -r '.[0].url') | ||
| if [ "$STDIN_URL" = "https://example.invalid/stdin-gone" ]; then | ||
| echo " PASS: stdin path emits the broken link" | ||
| PASS=$((PASS + 1)) | ||
| else | ||
| echo " FAIL: stdin path emits the broken link" | ||
| echo " Expected: https://example.invalid/stdin-gone" | ||
| echo " Got: $STDIN_URL" | ||
| FAIL=$((FAIL + 1)) | ||
| fi | ||
|
|
||
| # --- Summary --- | ||
| echo "" | ||
| echo "Results: $PASS passed, $FAIL failed" | ||
|
|
||
| if [ "$FAIL" -gt 0 ]; then | ||
| exit 1 | ||
| fi | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: The usage block documents reading from stdin via
-, buttest-extract-broken-links.shhas no test for this path. The PR body also counts "17 cases" but there are 18run_testcalls — minor mismatch worth fixing in the PR description.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both addressed: stdin (
-) path now covered by a test in c0eb93a, and the PR body updated to "18 assertions / 18/18 pass" (the file has 17run_testcalls plus one inline stdin assertion; line 35 is the helper definition, not a call).