Honor max_output_length for custom plugin stdout capture#1288
Conversation
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kv-cache The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @kv-cache! |
|
Hi @kv-cache. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Custom plugin stdout was read into a hardcoded 4 KiB buffer (maxCustomPluginBufferBytes = 1024*4) and only then truncated to the configured max_output_length, so the effective limit was min(4096, max_output_length). Any plugin configured with max_output_length > 4096 was silently capped at 4096 bytes, and a JSON-emitting plugin would be cut mid-token into invalid output. Drive the stdout read limit from max_output_length, bounded by a 1 MiB safety ceiling against a runaway plugin. stderr keeps a small fixed cap since it is only used for diagnostic logging and never becomes the plugin output. Adds a regression test and fixture asserting that a plugin emitting more than 4 KiB is captured up to max_output_length rather than the old fixed buffer size. Signed-off-by: Vineeth Rao Kanaparthi <vineeth0025@gmail.com>
ecb0aec to
25f099b
Compare
What this PR does / why we need it:
Custom-plugin stdout is read into a hardcoded 4 KiB buffer
(
maxCustomPluginBufferBytes = 1024 * 4) and only then truncated to theconfigured
max_output_length, so the effective limit ismin(4096, max_output_length). Any plugin configured withmax_output_length > 4096is silently capped at 4096 bytes — the configuredvalue has no effect. For a plugin that emits JSON, the cut lands mid-token and
produces invalid output that a strict consumer discards entirely.
This drives the stdout read limit from
max_output_length, bounded by a 1 MiBsafety ceiling to protect against a runaway plugin. stderr keeps a small fixed
cap since it is used only for diagnostic logging (
logPluginStderr) and is neverincluded in the returned output. No behavior change for the default config
(
max_output_length = 80).Which issue(s) this PR fixes:
NONE
Special notes for your reviewer:
Adds
TestPluginRunCaptureRespectsMaxOutputLengthplus a fixture that emits16384 bytes. With
max_output_length = 8192the captured output must be exactly8192 bytes — this fails on the current code (capped at 4096) and passes with the
fix.
go test -short -race ./pkg/custompluginmonitor/...passes; gofmt/vet clean.This raises the capture ceiling so a plugin's configured
max_output_lengthtakes effect; it does not change that output beyond
max_output_lengthis stillbyte-truncated.
/kind bug
Does this PR introduce a user-facing change?:
Note:
Made with AI