From 750eb129b0b8b6b287687ac38624d436fbe5c1b9 Mon Sep 17 00:00:00 2001 From: Ignacio Boudgouste Date: Wed, 2 Sep 2026 13:54:32 -0300 Subject: [PATCH] fix(k8s/diagnose): keep the results payload out of the argument list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit notify_results passed the aggregated check results to jq as a single --argjson value and then to np as a single --body value. Linux caps one execve argument at 128 KiB, which ulimit does not lift, so once a check began publishing application log tails the step died with "jq: Argument list too long" and diagnose published nothing. Keep the payload on disk end to end: xargs for the file list (already the case), --slurpfile instead of --argjson, and a file path for np --body. The .json suffix on the temp files is required — np reads --body from disk only when the value ends in .json, and otherwise sends the string itself as the request body. Also cap log lines at 2000 characters in lines_to_json_array. kubectl's --tail bounds how many lines we collect, not how long each one is, so a single serialized stack trace could still dominate the payload. Tests use a 2 MB payload so they fail on macOS too, where the limit is a total argv size rather than a per-argument cap. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + k8s/diagnose/tests/diagnose_utils.bats | 56 ++++++++++++++++++++++++++ k8s/diagnose/utils/diagnose_utils | 54 +++++++++++++++++++------ 3 files changed, 98 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff7387be..3486338d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Remove unused cloudwatch annotations from deployment objects - Fix: log queries on k8s scopes now return the time range that was selected, instead of the most recent lines whatever range was chosen - Fix: paging through logs on k8s scopes no longer repeats lines already shown, and now reaches the end of the selected range +- Fix: diagnose on k8s scopes no longer fails to publish its results when a check collects application logs ## [1.15.1] - 2026-08-12 - Fix: gRPC additional ports on k8s scopes now leave the declared port free for the application, so a gRPC server can bind the port configured in the scope instead of failing to start with "address already in use". gRPC ports now work the same way HTTP ones already did diff --git a/k8s/diagnose/tests/diagnose_utils.bats b/k8s/diagnose/tests/diagnose_utils.bats index bb218b81..640ac395 100644 --- a/k8s/diagnose/tests/diagnose_utils.bats +++ b/k8s/diagnose/tests/diagnose_utils.bats @@ -419,3 +419,59 @@ strip_ansi() { local clean=$(strip_ansi "$output") assert_contains "$clean" "⚠ No JSON result files found in $NP_OUTPUT_DIR" } + +@test "notify_results: sends payloads larger than the argv limit" { + # 2 MB of log text in one check: past the argv limit on both Linux and macOS. + local line + line=$(printf 'x%.0s' {1..1000}) + jq -nc --arg line "$line" \ + '{category: "logs", status: "success", evidence: {}, logs: [range(2000) | $line]}' \ + > "$NP_OUTPUT_DIR/big.json" + + local body_file="$(mktemp)" + export BODY_CAPTURE="$body_file" + # Mirror np: --body is read from disk only when the value ends in .json, + # otherwise it is sent verbatim. Resolved here because the caller cleans up. + np() { + local prev="" + for arg in "$@"; do + if [[ "$prev" == "--body" ]]; then + if [[ "$arg" == *.json ]]; then cat "$arg" > "$BODY_CAPTURE"; else printf '%s' "$arg" > "$BODY_CAPTURE"; fi + fi + prev="$arg" + done + return 0 + } + export -f np + + run notify_results + [ "$status" -eq 0 ] + + assert_equal "$(jq -r '.results.categories[0].category' "$body_file")" "logs" + assert_equal "$(jq -r '.results.categories[0].checks[0].logs | length' "$body_file")" "2000" + + rm -f "$body_file" + unset BODY_CAPTURE +} + +# ============================================================================= +# lines_to_json_array +# ============================================================================= +@test "lines_to_json_array: passes short lines through untouched" { + local out + out=$(printf 'alpha\n\nbeta\n' | lines_to_json_array) + + assert_equal "$(echo "$out" | jq -r 'length')" "2" + assert_equal "$(echo "$out" | jq -r '.[0]')" "alpha" + assert_equal "$(echo "$out" | jq -r '.[1]')" "beta" +} + +@test "lines_to_json_array: truncates a line past the character cap" { + local out + out=$(printf 'x%.0s' {1..5000} | EVIDENCE_LOG_LINE_MAX_CHARS=100 lines_to_json_array) + + assert_equal "$(echo "$out" | jq -r 'length')" "1" + assert_contains "$(echo "$out" | jq -r '.[0]')" "[truncated]" + # 100 kept characters plus the marker, nowhere near the original 5000. + [ "$(echo "$out" | jq -r '.[0] | length')" -lt 200 ] +} diff --git a/k8s/diagnose/utils/diagnose_utils b/k8s/diagnose/utils/diagnose_utils index 94bac3b8..9cd02f91 100644 --- a/k8s/diagnose/utils/diagnose_utils +++ b/k8s/diagnose/utils/diagnose_utils @@ -123,8 +123,16 @@ read_log_tail() { # Convert newline-delimited stdin into a JSON array of non-empty strings. # Used by read_log_tail and update_check_result to share one canonical # tail-text-to-JSON pipeline. +# +# Lines are truncated at $EVIDENCE_LOG_LINE_MAX_CHARS (default 2000): kubectl's +# --tail bounds line count, not line length. lines_to_json_array() { - jq -R -s 'split("\n") | map(select(length > 0))' + local max="${EVIDENCE_LOG_LINE_MAX_CHARS:-2000}" + jq -R -s --argjson max "$max" ' + split("\n") + | map(select(length > 0)) + | map(if length > $max then .[:$max] + "…[truncated]" else . end) + ' } # Append a JSON object to a bash indexed array (passed by name). Avoids the @@ -333,10 +341,18 @@ notify_results() { exit 1 fi - # Generate grouped results directly in memory - # Use xargs to avoid "Argument list too long" error - local grouped_results - grouped_results=$(echo "$json_files" | xargs jq -s ' + # The payload must never travel through argv: Linux caps a single execve + # argument at 128 KiB, and ulimit does not lift it. Hence xargs, then + # --slurpfile, then a file path for np --body. + # + # The .json suffix is required: np reads --body from disk only when the + # value ends in .json, otherwise it sends the string itself. + local work_dir results_file body_file rc + work_dir="$(mktemp -d)" + results_file="$work_dir/results.json" + body_file="$work_dir/body.json" + + if ! echo "$json_files" | xargs jq -s ' # category helper def cat: .category // "unknown"; @@ -355,19 +371,31 @@ notify_results() { }, checks: . }) - ') + ' > "$results_file"; then + print_error "Failed to group check results from $NP_OUTPUT_DIR" + rm -rf "$work_dir" + return 1 + fi # Extract action and service IDs ACTION_ID=$(echo "$NP_ACTION_CONTEXT" | jq -r '.notification.id') SERVICE_ID=$(echo "$NP_ACTION_CONTEXT" | jq -r '.notification.service.id') # Build action body with grouped results - ACTION_BODY=$(jq -nc \ - --argjson result "$grouped_results" \ - '{ - results: { categories: $result } - }') + if ! jq -nc \ + --slurpfile result "$results_file" \ + '{ + results: { categories: $result[0] } + }' > "$body_file"; then + print_error "Failed to build the action body" + rm -rf "$work_dir" + return 1 + fi # Send to np service - np service action patch --id "$ACTION_ID" --serviceId "$SERVICE_ID" --body "$ACTION_BODY" --no-output -} \ No newline at end of file + np service action patch --id "$ACTION_ID" --serviceId "$SERVICE_ID" --body "$body_file" --no-output + rc=$? + + rm -rf "$work_dir" + return $rc +}