From 22ab3cf55192b074378c5bac82f8e1bb8fdb7dc1 Mon Sep 17 00:00:00 2001 From: Kris Coleman Date: Wed, 10 Jun 2026 16:07:27 -0400 Subject: [PATCH] feat(redact): make MAX_CONCURRENT_REDACTORS configurable via env var (sc-138321) (#2057) Replace the hardcoded MAX_CONCURRENT_REDACTORS = 10 ceiling in pkg/collect/redact.go with a runtime-resolved value driven by the TROUBLESHOOT_MAX_CONCURRENT_REDACTORS env var. The default (10) is unchanged, so behavior is identical unless an operator opts in. - DefaultMaxConcurrentRedactors exported as the default - MaxConcurrentRedactorsEnvVar exported as the env var name - maxConcurrentRedactors() helper parses the env, logs on invalid input, and falls back to the default on missing/empty/non-numeric/<=0 values - Table-driven tests in pkg/collect/redact_test.go cover unset, empty, positive override, default-equal, zero, negative, non-numeric, and whitespace-padded inputs Unblocks Pixee's standalone support-bundle pipeline, which hits the 10-concurrent ceiling on large bundles. Refs: sc-138321, replicated-collab/pixee-replicated#131 --- pkg/collect/redact.go | 44 +++++++++++++++--- pkg/collect/redact_test.go | 94 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 6 deletions(-) create mode 100644 pkg/collect/redact_test.go diff --git a/pkg/collect/redact.go b/pkg/collect/redact.go index 9e990a27..84c880a7 100644 --- a/pkg/collect/redact.go +++ b/pkg/collect/redact.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "strconv" "strings" "sync" @@ -16,18 +17,49 @@ import ( "k8s.io/klog/v2" ) -// Max number of concurrent redactors to run -// Ensure the number is low enough since each of the redactors -// also spawns goroutines to redact files in tar archives and -// other goroutines for each redactor spec. -const MAX_CONCURRENT_REDACTORS = 10 +// Default cap on concurrent file redactors. Each redactor also spawns +// goroutines to redact files in tar archives and goroutines for each +// redactor spec, so the ceiling is intentionally low. +const DefaultMaxConcurrentRedactors = 10 + +// MaxConcurrentRedactorsEnvVar is the environment variable name used to +// override DefaultMaxConcurrentRedactors at runtime. Operators set this on +// the support-bundle binary (e.g. inside a Job/initContainer) when the +// default ceiling becomes a bottleneck for very large bundles. +const MaxConcurrentRedactorsEnvVar = "TROUBLESHOOT_MAX_CONCURRENT_REDACTORS" + +// maxConcurrentRedactors returns the active cap on concurrent redactors. +// It reads MaxConcurrentRedactorsEnvVar; if unset, empty, non-numeric, or +// <= 0 it falls back to DefaultMaxConcurrentRedactors (with a klog warning +// for non-empty invalid input so silent misconfiguration is hard to miss). +func maxConcurrentRedactors() int { + raw, ok := os.LookupEnv(MaxConcurrentRedactorsEnvVar) + if !ok || raw == "" { + return DefaultMaxConcurrentRedactors + } + + n, err := strconv.Atoi(raw) + if err != nil { + klog.Warningf("Invalid %s=%q (not an integer); falling back to default %d", MaxConcurrentRedactorsEnvVar, raw, DefaultMaxConcurrentRedactors) + return DefaultMaxConcurrentRedactors + } + if n <= 0 { + klog.Warningf("Invalid %s=%d (must be > 0); falling back to default %d", MaxConcurrentRedactorsEnvVar, n, DefaultMaxConcurrentRedactors) + return DefaultMaxConcurrentRedactors + } + + if n != DefaultMaxConcurrentRedactors { + klog.Infof("Overriding concurrent redactor cap: %s=%d (default %d)", MaxConcurrentRedactorsEnvVar, n, DefaultMaxConcurrentRedactors) + } + return n +} func RedactResult(bundlePath string, input CollectorResult, additionalRedactors []*troubleshootv1beta2.Redact) error { wg := &sync.WaitGroup{} // Error channel to capture errors from goroutines errorCh := make(chan error, len(input)) - limitCh := make(chan struct{}, MAX_CONCURRENT_REDACTORS) + limitCh := make(chan struct{}, maxConcurrentRedactors()) defer close(limitCh) for k, v := range input { diff --git a/pkg/collect/redact_test.go b/pkg/collect/redact_test.go new file mode 100644 index 00000000..68a57082 --- /dev/null +++ b/pkg/collect/redact_test.go @@ -0,0 +1,94 @@ +package collect + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_maxConcurrentRedactors(t *testing.T) { + tests := []struct { + name string + setEnv bool + value string + want int + }{ + { + name: "env unset returns default", + setEnv: false, + want: DefaultMaxConcurrentRedactors, + }, + { + name: "empty value returns default", + setEnv: true, + value: "", + want: DefaultMaxConcurrentRedactors, + }, + { + name: "valid positive int overrides default", + setEnv: true, + value: "50", + want: 50, + }, + { + name: "value equal to default is honored", + setEnv: true, + value: "10", + want: DefaultMaxConcurrentRedactors, + }, + { + name: "zero falls back to default", + setEnv: true, + value: "0", + want: DefaultMaxConcurrentRedactors, + }, + { + name: "negative value falls back to default", + setEnv: true, + value: "-3", + want: DefaultMaxConcurrentRedactors, + }, + { + name: "non-numeric value falls back to default", + setEnv: true, + value: "potato", + want: DefaultMaxConcurrentRedactors, + }, + { + name: "whitespace-padded value falls back to default", + setEnv: true, + value: " 4 ", + want: DefaultMaxConcurrentRedactors, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Snapshot inherited state so each subtest is hermetic regardless + // of the order Go picks. t.Setenv would mask "env unset" cases + // with an empty value, so we manage the env directly here. + prev, hadPrev := os.LookupEnv(MaxConcurrentRedactorsEnvVar) + t.Cleanup(func() { + if hadPrev { + _ = os.Setenv(MaxConcurrentRedactorsEnvVar, prev) + } else { + _ = os.Unsetenv(MaxConcurrentRedactorsEnvVar) + } + }) + + if tt.setEnv { + if err := os.Setenv(MaxConcurrentRedactorsEnvVar, tt.value); err != nil { + t.Fatalf("setenv: %v", err) + } + } else { + if err := os.Unsetenv(MaxConcurrentRedactorsEnvVar); err != nil { + t.Fatalf("unsetenv: %v", err) + } + } + + got := maxConcurrentRedactors() + assert.Equal(t, tt.want, got) + }) + } +}