diff --git a/pkg/custompluginmonitor/plugin/plugin.go b/pkg/custompluginmonitor/plugin/plugin.go index f3668eb51..1b4d66b9b 100644 --- a/pkg/custompluginmonitor/plugin/plugin.go +++ b/pkg/custompluginmonitor/plugin/plugin.go @@ -33,9 +33,18 @@ import ( "k8s.io/node-problem-detector/pkg/util/tomb" ) -// maxCustomPluginBufferBytes is the max bytes that a custom plugin is allowed to -// send to stdout/stderr. Any bytes exceeding this value will be truncated. -const maxCustomPluginBufferBytes = 1024 * 4 +// maxCustomPluginStdoutCeilingBytes is a hard safety ceiling on how many bytes +// of stdout NPD reads from a custom plugin, bounding memory use from a runaway +// plugin. The per-plugin max_output_length is what actually governs the message +// size; this ceiling only caps the (misconfigured) case where max_output_length +// itself exceeds it. It must be >= the largest supported max_output_length so a +// plugin's configured limit is never silently truncated by the read buffer. +const maxCustomPluginStdoutCeilingBytes = 1024 * 1024 + +// maxCustomPluginStderrBytes caps stderr. run() uses stderr only for diagnostic +// logging (logPluginStderr) and never includes it in the returned output, so it +// does not need to honor max_output_length and keeps a small fixed cap. +const maxCustomPluginStderrBytes = 1024 * 4 type Plugin struct { config cpmtypes.CustomPluginConfig @@ -210,14 +219,22 @@ func (p *Plugin) run(rule cpmtypes.CustomRule) (exitStatus cpmtypes.Status, outp stderrErr error ) + // Capture enough stdout to honor the configured max_output_length, bounded + // by a hard safety ceiling. Previously this was a fixed 4 KiB buffer that + // silently truncated plugins configured with a larger max_output_length. + stdoutCapture := int64(*p.config.PluginGlobalConfig.MaxOutputLength) + if stdoutCapture > maxCustomPluginStdoutCeilingBytes { + stdoutCapture = maxCustomPluginStdoutCeilingBytes + } + wg.Add(2) go func() { defer wg.Done() - stdout, stdoutErr = readFromReader(stdoutPipe, maxCustomPluginBufferBytes) + stdout, stdoutErr = readFromReader(stdoutPipe, stdoutCapture) }() go func() { defer wg.Done() - stderr, stderrErr = readFromReader(stderrPipe, maxCustomPluginBufferBytes) + stderr, stderrErr = readFromReader(stderrPipe, maxCustomPluginStderrBytes) }() // This will wait for the reads to complete. If the execution times out, the pipes // will be closed and the wait group unblocks. diff --git a/pkg/custompluginmonitor/plugin/plugin_test.go b/pkg/custompluginmonitor/plugin/plugin_test.go index 2988d3bd2..626faab4e 100644 --- a/pkg/custompluginmonitor/plugin/plugin_test.go +++ b/pkg/custompluginmonitor/plugin/plugin_test.go @@ -18,6 +18,7 @@ package plugin import ( "runtime" + "strings" "testing" "time" @@ -121,3 +122,44 @@ func TestNewPluginRun(t *testing.T) { }) } } + +// TestPluginRunCaptureRespectsMaxOutputLength verifies that a plugin emitting +// more than the old hardcoded 4 KiB capture buffer is captured up to the +// configured max_output_length, not silently truncated at 4096 bytes. +func TestPluginRunCaptureRespectsMaxOutputLength(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fixture is a shell script; this path is exercised on non-Windows platforms") + } + + ruleTimeout := 1 * time.Second + // Larger than the old 4096-byte capture buffer, and smaller than the + // 16384 bytes the fixture emits, so the result must be exactly this many + // bytes if (and only if) capture honors max_output_length. + maxOutputLength := 8192 + + conf := cpmtypes.CustomPluginConfig{} + // Set before ApplyConfiguration so it is not overwritten by the default, + // and use a fresh pointer so we don't mutate the package-level default. + conf.PluginGlobalConfig.MaxOutputLength = &maxOutputLength + if err := (&conf).ApplyConfiguration(); err != nil { + t.Fatalf("Failed to apply configuration: %v", err) + } + p := Plugin{config: conf} + + rule := cpmtypes.CustomRule{ + Path: "./test-data/large-stdout-with-ok-exit-status.sh", + Timeout: &ruleTimeout, + } + gotStatus, gotOutput := p.run(rule) + + if gotStatus != cpmtypes.OK { + t.Errorf("exit status: got %v, want %v", gotStatus, cpmtypes.OK) + } + if len(gotOutput) != maxOutputLength { + t.Errorf("output length: got %d, want %d (must be capped at max_output_length, not a fixed 4 KiB buffer)", + len(gotOutput), maxOutputLength) + } + if want := strings.Repeat("a", maxOutputLength); gotOutput != want { + t.Errorf("output content mismatch: got %d bytes, want %d 'a' bytes", len(gotOutput), len(want)) + } +} diff --git a/pkg/custompluginmonitor/plugin/test-data/large-stdout-with-ok-exit-status.sh b/pkg/custompluginmonitor/plugin/test-data/large-stdout-with-ok-exit-status.sh new file mode 100755 index 000000000..ec019553f --- /dev/null +++ b/pkg/custompluginmonitor/plugin/test-data/large-stdout-with-ok-exit-status.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash + +# Emits 16384 bytes of stdout (larger than the old hardcoded 4 KiB capture +# buffer and larger than the test's max_output_length) with a success exit +# code. Used to verify that plugin output capture honors max_output_length +# rather than a fixed internal buffer size. +head -c 16384 /dev/zero | tr '\0' 'a' +exit 0