diff --git a/common/path_limiter.go b/common/path_limiter.go new file mode 100644 index 0000000..4125fd3 --- /dev/null +++ b/common/path_limiter.go @@ -0,0 +1,92 @@ +package common + +import ( + "sync" + + "github.com/coroot/coroot-node-agent/flags" + "k8s.io/klog/v2" +) + +// OtherPath is the bucket every path beyond the per-container cap collapses +// into. Requests are still counted, they just stop minting new series. +const OtherPath = "{other}" + +// PathLimiter bounds the number of distinct HTTP `path` label values a single +// container may contribute to container_http_requests_total. +// +// `path` is the only L7 label an outside party controls. An internet-facing +// ingress is probed for vulnerabilities around the clock, and every unique junk +// path (/axds.php, /HNAP1, ...) becomes a permanent series. Normalization cannot +// collapse them because they are genuinely distinct literals, so the series count +// grows without bound for as long as the scanners keep scanning. +// +// Paths seen before the cap is reached always pass through, so a steady +// application keeps reporting its real routes; only paths first observed after +// the cap fills are folded into OtherPath. Real applications serve far fewer +// routes than the default cap, which should only ever engage on scanner traffic. +// +// The zero value is not usable; construct with NewPathLimiter. +type PathLimiter struct { + mu sync.RWMutex + seen map[string]struct{} + full bool // latched once the cap is hit, so the hot path skips the map entirely + + owner string // container_id, for the one-shot warning +} + +func NewPathLimiter(owner string) *PathLimiter { + return &PathLimiter{seen: make(map[string]struct{}), owner: owner} +} + +// Limit returns path if it may be reported as-is, or OtherPath if admitting it +// would push this container past flags.MaxHttpPathsPerContainer. Callers should +// normalize the path first so that a parameterized route consumes one slot +// rather than one per parameter value. +// A nil receiver is tolerated (returns path unchanged) so that an L7Stats built +// as a zero value rather than through its constructor degrades to the old +// unbounded behaviour instead of panicking in the L7 hot path. +func (l *PathLimiter) Limit(path string) string { + limit := flags.GetInt(flags.MaxHttpPathsPerContainer) + // An empty path is what non-HTTP protocols and invalid-UTF8 requests report; + // it must not consume a slot. + if l == nil || limit <= 0 || path == "" { + return path + } + + l.mu.RLock() + _, seen := l.seen[path] + full := l.full + l.mu.RUnlock() + if seen { + return path + } + if full { + return OtherPath + } + + l.mu.Lock() + defer l.mu.Unlock() + // Re-check: another request may have admitted this path, or filled the cap, + // while we waited for the write lock. + if _, seen := l.seen[path]; seen { + return path + } + if len(l.seen) >= limit { + if !l.full { + l.full = true + klog.Warningf("HTTP path cardinality cap (%d) reached for %s, further unseen paths reported as %s", + limit, l.owner, OtherPath) + } + return OtherPath + } + l.seen[path] = struct{}{} + return path +} + +// Len reports how many distinct paths have been admitted. For tests and +// diagnostics. +func (l *PathLimiter) Len() int { + l.mu.RLock() + defer l.mu.RUnlock() + return len(l.seen) +} diff --git a/common/path_limiter_test.go b/common/path_limiter_test.go new file mode 100644 index 0000000..eb02b2a --- /dev/null +++ b/common/path_limiter_test.go @@ -0,0 +1,121 @@ +package common + +import ( + "fmt" + "sync" + "testing" + + "github.com/coroot/coroot-node-agent/flags" +) + +// withPathCap sets the cap for the duration of a test. Flags are not parsed in +// test binaries (flags.init bails out on the .test suffix), so the pointer holds +// a zero value rather than the declared default. +func withPathCap(t *testing.T, limit int) { + t.Helper() + prev := *flags.MaxHttpPathsPerContainer + *flags.MaxHttpPathsPerContainer = limit + t.Cleanup(func() { *flags.MaxHttpPathsPerContainer = prev }) +} + +func TestPathLimiterAdmitsUpToCapThenCollapses(t *testing.T) { + withPathCap(t, 3) + l := NewPathLimiter("/k8s/test/pod/app") + + for _, p := range []string{"/a", "/b", "/c"} { + if got := l.Limit(p); got != p { + t.Fatalf("Limit(%q) = %q, want it admitted", p, got) + } + } + + if got := l.Limit("/d"); got != OtherPath { + t.Errorf("Limit(%q) = %q, want %q once the cap is reached", "/d", got, OtherPath) + } + + // Paths admitted before the cap keep reporting under their real value — a + // steady application must not lose its routes to scanner traffic that arrives + // later. + if got := l.Limit("/b"); got != "/b" { + t.Errorf("Limit(%q) = %q, want the already-admitted path", "/b", got) + } +} + +func TestPathLimiterUnlimitedWhenCapIsZero(t *testing.T) { + withPathCap(t, 0) + l := NewPathLimiter("/k8s/test/pod/app") + + for i := 0; i < 500; i++ { + p := fmt.Sprintf("/p%d", i) + if got := l.Limit(p); got != p { + t.Fatalf("Limit(%q) = %q, want no cap when the flag is 0", p, got) + } + } +} + +func TestPathLimiterPassesEmptyPathThrough(t *testing.T) { + withPathCap(t, 1) + l := NewPathLimiter("/k8s/test/pod/app") + + // Non-HTTP protocols and invalid-UTF8 requests report "", which must neither + // consume a cap slot nor be rewritten to OtherPath. + for i := 0; i < 10; i++ { + if got := l.Limit(""); got != "" { + t.Fatalf("Limit(\"\") = %q, want \"\"", got) + } + } + if got := l.Limit("/real"); got != "/real" { + t.Errorf("Limit(%q) = %q, want it admitted — empty paths must not fill the cap", "/real", got) + } +} + +func TestPathLimiterIsBoundedUnderConcurrency(t *testing.T) { + const cap = 50 + withPathCap(t, cap) + l := NewPathLimiter("/k8s/test/pod/app") + + var wg sync.WaitGroup + for w := 0; w < 8; w++ { + wg.Add(1) + go func(worker int) { + defer wg.Done() + for i := 0; i < 200; i++ { + l.Limit(fmt.Sprintf("/w%d/p%d", worker, i)) + } + }(w) + } + wg.Wait() + + // The whole point of the cap: concurrent admission must never overshoot it, + // otherwise the series bound is not actually a bound. + if n := l.Len(); n > cap { + t.Errorf("admitted %d distinct paths, want at most %d", n, cap) + } +} + +func TestPathLimiterNilReceiverIsSafe(t *testing.T) { + withPathCap(t, 1) + var l *PathLimiter + + // Limit runs on every HTTP request; a zero-value L7Stats must degrade to + // unbounded rather than panic. + if got := l.Limit("/a"); got != "/a" { + t.Errorf("(*PathLimiter)(nil).Limit(%q) = %q, want it returned unchanged", "/a", got) + } + if got := l.Limit("/b"); got != "/b" { + t.Errorf("(*PathLimiter)(nil).Limit(%q) = %q, want it returned unchanged", "/b", got) + } +} + +func TestPathLimiterIsPerInstance(t *testing.T) { + withPathCap(t, 1) + a, b := NewPathLimiter("container-a"), NewPathLimiter("container-b") + + // One noisy container filling its cap must not silence another's routes. + a.Limit("/only") + if got := a.Limit("/second"); got != OtherPath { + t.Errorf("a.Limit(%q) = %q, want %q", "/second", got, OtherPath) + } + if got := b.Limit("/second"); got != "/second" { + t.Errorf("b.Limit(%q) = %q, want it admitted on an independent limiter", "/second", got) + } +} diff --git a/containers/l7.go b/containers/l7.go index bd60344..f8a905b 100644 --- a/containers/l7.go +++ b/containers/l7.go @@ -106,6 +106,11 @@ type L7Stats struct { latency map[l7.Protocol]*prometheus.HistogramVec initialized map[l7.Protocol]bool promConstLabels prometheus.Labels // container_id, app_id, machine_id, system_uuid, az, region + + // pathLimiter caps the distinct `path` label values this container can emit. + // Scanner traffic against a public ingress otherwise grows the series count + // without bound; see common.PathLimiter. + pathLimiter *common.PathLimiter } func NewL7Stats(constLabels prometheus.Labels) L7Stats { @@ -114,6 +119,7 @@ func NewL7Stats(constLabels prometheus.Labels) L7Stats { latency: make(map[l7.Protocol]*prometheus.HistogramVec), initialized: make(map[l7.Protocol]bool), promConstLabels: constLabels, + pathLimiter: common.NewPathLimiter(constLabels["container_id"]), } } @@ -155,7 +161,7 @@ func (s *L7Stats) observe(protocol l7.Protocol, status, method, path string, dur counterLabelValues = append(counterLabelValues, labelInterner.intern(method)) case l7.ProtocolHTTP: if ValidUtf8([]byte(path)) { - counterLabelValues = append(counterLabelValues, labelInterner.intern(normalizeHttpPath(path))) + counterLabelValues = append(counterLabelValues, labelInterner.intern(s.pathLimiter.Limit(normalizeHttpPath(path)))) } else { counterLabelValues = append(counterLabelValues, "") } diff --git a/flags/flags.go b/flags/flags.go index ad65da0..b2ffab5 100644 --- a/flags/flags.go +++ b/flags/flags.go @@ -75,6 +75,17 @@ var ( HttpPathNormalizationRules = kingpin.Flag("http-path-normalization-rules", "Custom HTTP path normalization rules in format 'pattern1:replacement1,pattern2:replacement2'").Envar("HTTP_PATH_NORMALIZATION_RULES").String() + // MaxHttpPathsPerContainer bounds the number of distinct `path` label values a + // single container may contribute to container_http_requests_total. Path is the + // only L7 label an outside party controls: an internet-facing ingress gets probed + // for vulnerabilities around the clock, and every unique junk path (/axds.php, + // /HNAP1, ...) becomes a permanent series that normalization cannot collapse + // because the paths are genuinely distinct literals. Past the cap, further unseen + // paths collapse to "{other}" so the request count stays correct while the series + // count stops growing. Real applications serve far fewer than the default; the + // cap should only ever engage on scanner traffic. + MaxHttpPathsPerContainer = kingpin.Flag("max-http-paths-per-container", "Max distinct HTTP path label values per container, excess collapses to {other} (0 = unlimited)").Default("1000").Envar("MAX_HTTP_PATHS_PER_CONTAINER").Int() + AggregateEphemeralWorkloads = kingpin.Flag("aggregate-ephemeral-workloads", "Aggregate metrics for bare pods and standalone Jobs using standard labels to reduce series cardinality").Default("true").Envar("AGGREGATE_EPHEMERAL_WORKLOADS").Bool() // CollapseInternalDestinations replaces the raw IP:port value of the @@ -98,6 +109,13 @@ func GetString(fl *string) string { return *fl } +func GetInt(fl *int) int { + if fl == nil { + return 0 + } + return *fl +} + func init() { if strings.HasSuffix(os.Args[0], ".test") { return