Skip to content

Commit b25bed1

Browse files
authored
x-pack/filebeat/input/streaming: fix CrowdStrike retry cap and soften transient handling (#51712)
x-pack/filebeat/input/streaming: fix CrowdStrike retry cap and soften transient handling The CrowdStrike FalconHose discover retry loop capped every run at 10 attempts: the unconfigured maximum was checked as an else-if on the configured branch, so max_attempts greater than 10 and infinite_retries were silently limited to 10, terminating the input and requiring a restart during longer upstream outages. Apply the unconfigured cap only when no retry policy is set so configured limits and infinite_retries are honoured. A 200 response with an empty discover body (io.EOF from Decode) is now reported as a distinct transient error rather than a generic decode failure, and DEGRADED is only reported after three consecutive failures so a single transient blip no longer churns the unit health status. The discover GET failure no longer updates status inside followSession; the retry loop owns those transitions. Add regression tests for the retry cap, infinite_retries, deferred DEGRADED reporting, and the empty-body discover response. Assisted-By: Cursor (Claude Opus 4.8)
1 parent 1f95636 commit b25bed1

4 files changed

Lines changed: 238 additions & 7 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Kind can be one of:
2+
# - breaking-change: a change to previously-documented behavior
3+
# - deprecation: functionality that is being removed in a later release
4+
# - bug-fix: fixes a problem in a previous version
5+
# - enhancement: extends functionality but does not break or fix existing behavior
6+
# - feature: new functionality
7+
# - known-issue: problems that we are aware of in a given version
8+
# - security: impacts on the security of a product or a user’s deployment.
9+
# - upgrade: important information for someone upgrading from a prior version
10+
# - other: does not fit into any of the other categories
11+
kind: bug-fix
12+
13+
# Change summary; a 80ish characters long description of the change.
14+
summary: Fix CrowdStrike streaming input retry cap so max_attempts and infinite_retries are honoured.
15+
16+
# Long description; in case the summary is not enough to describe the change
17+
# this field accommodate a description without length limits.
18+
# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
19+
description: |
20+
Fix the CrowdStrike streaming input retry loop so a configured max_attempts
21+
greater than 10 and infinite_retries are no longer silently capped at the
22+
unconfigured default of 10. The empty-body case from the discover endpoint is
23+
now reported as a distinct transient error, and the input no longer reports
24+
DEGRADED on the first transient failure.
25+
26+
# Affected component; a word indicating the component this changeset affects.
27+
component: filebeat
28+
29+
# PR URL; optional; the PR number that added the changeset.
30+
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
31+
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
32+
# Please provide it if you are adding a fragment for a different PR.
33+
#pr: https://github.com/owner/repo/1234
34+
35+
# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
36+
# If not present is automatically filled by the tooling with the issue linked to the PR number.
37+
#issue: https://github.com/owner/repo/1234

x-pack/filebeat/input/streaming/crowdstrike.go

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,10 @@ func (s *falconHoseStream) FollowStream(ctx context.Context) error {
248248
var err error
249249
attempt := 0
250250
const maxAttemptsUnconfigured = 10
251+
// Number of consecutive failures tolerated before reporting DEGRADED,
252+
// so a single transient blip (e.g. an empty discover response) does not
253+
// churn the unit health status.
254+
const degradeAfterAttempts = 3
251255
for {
252256
state, err = s.followSession(ctx, cli, state)
253257
if err != nil {
@@ -262,8 +266,14 @@ func (s *falconHoseStream) FollowStream(ctx context.Context) error {
262266

263267
attempt++
264268

265-
if s.cfg.Retry != nil && !s.cfg.Retry.InfiniteRetries && attempt >= s.cfg.Retry.MaxAttempts {
266-
return fmt.Errorf("max retry attempts (%d) exceeded: %w", s.cfg.Retry.MaxAttempts, err)
269+
// The unconfigured cap must only apply when no retry policy is
270+
// set. Keeping it as an else-if on the configured branch caused
271+
// infinite_retries and max_attempts > 10 to be silently capped
272+
// at 10.
273+
if s.cfg.Retry != nil {
274+
if !s.cfg.Retry.InfiniteRetries && attempt >= s.cfg.Retry.MaxAttempts {
275+
return fmt.Errorf("max retry attempts (%d) exceeded: %w", s.cfg.Retry.MaxAttempts, err)
276+
}
267277
} else if attempt >= maxAttemptsUnconfigured {
268278
return fmt.Errorf("max retry attempts (%d unconfigured) exceeded: %w", maxAttemptsUnconfigured, err)
269279
}
@@ -280,7 +290,9 @@ func (s *falconHoseStream) FollowStream(ctx context.Context) error {
280290
waitTime = rle.wait
281291
}
282292

283-
s.status.UpdateStatus(status.Degraded, err.Error())
293+
if attempt >= degradeAfterAttempts {
294+
s.status.UpdateStatus(status.Degraded, err.Error())
295+
}
284296
s.log.Warnw("session warning", "error", err, "attempt", attempt, "wait", waitTime.String())
285297

286298
select {
@@ -310,9 +322,10 @@ func (s *falconHoseStream) followSession(ctx context.Context, cli *http.Client,
310322
}
311323
resp, err := cli.Do(req)
312324
if err != nil {
313-
err = fmt.Errorf("failed GET to discover stream: %w", err)
314-
s.status.UpdateStatus(status.Degraded, err.Error())
315-
return state, err
325+
// Status transitions are owned by the FollowStream retry loop so
326+
// that a single transient failure does not immediately report
327+
// DEGRADED; just return the error here.
328+
return state, fmt.Errorf("failed GET to discover stream: %w", err)
316329
}
317330
defer resp.Body.Close()
318331

@@ -354,6 +367,13 @@ func (s *falconHoseStream) followSession(ctx context.Context, cli *http.Client,
354367
}
355368
err = dec.Decode(&body)
356369
if err != nil {
370+
// A 200 response with an empty body yields io.EOF from Decode. This
371+
// is a transient upstream condition (the discover endpoint
372+
// occasionally returns no content), distinct from a malformed body,
373+
// so surface it clearly rather than as a generic decode failure.
374+
if errors.Is(err, io.EOF) {
375+
return state, errors.New("discover stream returned an empty body")
376+
}
357377
return state, fmt.Errorf("failed to decode discover body: %w", err)
358378
}
359379
s.log.Debugw("stream discover metadata", logp.Namespace(s.ns), "meta", mapstr.M(body.Meta))

x-pack/filebeat/input/streaming/crowdstrike_test.go

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
v2 "github.com/elastic/beats/v7/filebeat/input/v2"
2525
cursor "github.com/elastic/beats/v7/filebeat/input/v2/input-cursor"
2626
"github.com/elastic/beats/v7/libbeat/beat"
27+
"github.com/elastic/beats/v7/libbeat/management/status"
2728
"github.com/elastic/elastic-agent-libs/logp"
2829
"github.com/elastic/elastic-agent-libs/logp/logptest"
2930
"github.com/elastic/elastic-agent-libs/monitoring"
@@ -297,6 +298,157 @@ func TestCrowdstrikeOAuthHTTPClientRespectsConfiguredTimeout(t *testing.T) {
297298
}
298299
}
299300

301+
func TestFollowStreamRetryCapHonorsMaxAttempts(t *testing.T) {
302+
log := logptest.NewTestingLogger(t, t.Name())
303+
304+
discoverURL, tokenURL, hits := startEmptyDiscover(t)
305+
const maxAttempts = 15 // Deliberately > the unconfigured cap of 10.
306+
cfg := emptyDiscoverConfig(t, discoverURL, tokenURL, &retry{MaxAttempts: maxAttempts, WaitMin: time.Millisecond, WaitMax: time.Millisecond})
307+
s := newEmptyDiscoverFollower(t, cfg, nil, log)
308+
309+
err := s.FollowStream(context.Background())
310+
if err == nil {
311+
t.Fatal("FollowStream() error = nil; want max-attempts error")
312+
}
313+
// Regression: a configured MaxAttempts greater than 10 must be honoured,
314+
// not silently capped at the unconfigured default of 10.
315+
want := fmt.Sprintf("max retry attempts (%d) exceeded", maxAttempts)
316+
if !strings.Contains(err.Error(), want) {
317+
t.Errorf("FollowStream() error = %v; want substring %q", err, want)
318+
}
319+
if got := hits.Load(); got != int64(maxAttempts) {
320+
t.Errorf("discover attempts = %d; want %d", got, maxAttempts)
321+
}
322+
}
323+
324+
func TestFollowStreamInfiniteRetriesDoesNotCap(t *testing.T) {
325+
log := logptest.NewTestingLogger(t, t.Name())
326+
327+
discoverURL, tokenURL, hits := startEmptyDiscover(t)
328+
// MaxAttempts is intentionally tiny: InfiniteRetries must override it and
329+
// the unconfigured cap of 10 entirely.
330+
cfg := emptyDiscoverConfig(t, discoverURL, tokenURL, &retry{InfiniteRetries: true, MaxAttempts: 1, WaitMin: time.Millisecond, WaitMax: time.Millisecond})
331+
s := newEmptyDiscoverFollower(t, cfg, nil, log)
332+
333+
ctx, cancel := context.WithCancel(context.Background())
334+
defer cancel()
335+
done := make(chan error, 1)
336+
go func() { done <- s.FollowStream(ctx) }()
337+
338+
// Wait until well past the unconfigured cap to prove the input keeps
339+
// retrying, then cancel.
340+
deadline := time.After(10 * time.Second)
341+
for hits.Load() <= 12 {
342+
select {
343+
case <-deadline:
344+
t.Fatalf("discover attempts = %d before timeout; want > 12", hits.Load())
345+
case err := <-done:
346+
t.Fatalf("FollowStream returned early after %d attempts: %v", hits.Load(), err)
347+
case <-time.After(5 * time.Millisecond):
348+
}
349+
}
350+
cancel()
351+
352+
select {
353+
case err := <-done:
354+
if err != nil {
355+
t.Errorf("FollowStream() error = %v; want nil after context cancel", err)
356+
}
357+
case <-time.After(5 * time.Second):
358+
t.Fatal("timed out waiting for FollowStream to return after cancel")
359+
}
360+
}
361+
362+
func TestFollowStreamDefersDegraded(t *testing.T) {
363+
log := logptest.NewTestingLogger(t, t.Name())
364+
365+
discoverURL, tokenURL, _ := startEmptyDiscover(t)
366+
// With MaxAttempts=5 and the degrade threshold of 3, the input reports
367+
// DEGRADED only on attempts 3 and 4, then terminates on attempt 5 without
368+
// reporting DEGRADED on the earlier transient failures.
369+
cfg := emptyDiscoverConfig(t, discoverURL, tokenURL, &retry{MaxAttempts: 5, WaitMin: time.Millisecond, WaitMax: time.Millisecond})
370+
rec := &degradedRecorder{}
371+
s := newEmptyDiscoverFollower(t, cfg, rec, log)
372+
373+
if err := s.FollowStream(context.Background()); err == nil {
374+
t.Fatal("FollowStream() error = nil; want max-attempts error")
375+
}
376+
if got := rec.count(); got != 2 {
377+
t.Errorf("DEGRADED reports = %d; want 2 (attempts 3 and 4 only)", got)
378+
}
379+
}
380+
381+
// startEmptyDiscover starts a token endpoint and a discover endpoint that
382+
// returns 200 with an empty body (the transient EOF condition observed from
383+
// CrowdStrike), returning the discover URL, token URL, and a counter of
384+
// discover requests.
385+
func startEmptyDiscover(t *testing.T) (discoverURL, tokenURL string, hits *atomic.Int64) {
386+
t.Helper()
387+
tokenSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
388+
w.Header().Set("Content-Type", "application/json")
389+
_, _ = io.WriteString(w, `{"access_token":"token","token_type":"bearer","expires_in":3600}`)
390+
}))
391+
t.Cleanup(tokenSrv.Close)
392+
393+
hits = new(atomic.Int64)
394+
discoverSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
395+
hits.Add(1)
396+
w.Header().Set("Content-Type", "application/json")
397+
}))
398+
t.Cleanup(discoverSrv.Close)
399+
return discoverSrv.URL, tokenSrv.URL, hits
400+
}
401+
402+
func emptyDiscoverConfig(t *testing.T, discoverURL, tokenURL string, r *retry) config {
403+
t.Helper()
404+
u, err := url.Parse(discoverURL)
405+
if err != nil {
406+
t.Fatalf("failed to parse discover URL: %v", err)
407+
}
408+
return config{
409+
Type: "crowdstrike",
410+
URL: &urlConfig{u},
411+
Auth: authConfig{
412+
OAuth2: oAuth2Config{
413+
ClientID: "id",
414+
ClientSecret: "secret",
415+
TokenURL: tokenURL,
416+
},
417+
},
418+
CrowdstrikeAppID: "test",
419+
Retry: r,
420+
Program: `
421+
state.response.decode_json().as(body,{
422+
"events": [body],
423+
})`,
424+
}
425+
}
426+
427+
func newEmptyDiscoverFollower(t *testing.T, cfg config, stat status.StatusReporter, log *logp.Logger) StreamFollower {
428+
t.Helper()
429+
env := v2.Context{ID: "crowdstrike_test", MetricsRegistry: monitoring.NewRegistry()}
430+
s, err := NewFalconHoseFollower(context.Background(), env, cfg, nil, &testPublisher{log}, stat, log, time.Now)
431+
if err != nil {
432+
t.Fatalf("failed to construct follower: %v", err)
433+
}
434+
return s
435+
}
436+
437+
// degradedRecorder counts the number of times DEGRADED is reported.
438+
type degradedRecorder struct {
439+
n atomic.Int64
440+
}
441+
442+
func (r *degradedRecorder) UpdateStatus(s status.Status, _ string) {
443+
if s == status.Degraded {
444+
r.n.Add(1)
445+
}
446+
}
447+
448+
func (r *degradedRecorder) count() int64 {
449+
return r.n.Load()
450+
}
451+
300452
func TestFollowStreamCancelsRefreshOnReconnect(t *testing.T) {
301453
const totalSessions = 10
302454

x-pack/filebeat/input/streaming/crowdstrike_unit_test.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/elastic/beats/v7/libbeat/beat"
1818
"github.com/elastic/elastic-agent-libs/logp"
19+
"github.com/elastic/elastic-agent-libs/logp/logptest"
1920
"github.com/elastic/elastic-agent-libs/monitoring"
2021

2122
cursor "github.com/elastic/beats/v7/filebeat/input/v2/input-cursor"
@@ -64,6 +65,27 @@ func TestFollowSession_FirehoseHTTPError(t *testing.T) {
6465
}
6566
}
6667

68+
func TestFollowSession_EmptyDiscoverBody(t *testing.T) {
69+
discoverSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
70+
// A 200 OK with an empty body, as observed from the CrowdStrike
71+
// discover endpoint; Decode returns io.EOF.
72+
w.Header().Set("Content-Type", "application/json")
73+
}))
74+
defer discoverSrv.Close()
75+
76+
s := newTestStream(t, discoverSrv.URL, discoverSrv.Client())
77+
state, err := s.followSession(context.Background(), discoverSrv.Client(), map[string]any{})
78+
if err == nil {
79+
t.Fatal("expected error from followSession, got nil")
80+
}
81+
if want := "discover stream returned an empty body"; !strings.Contains(err.Error(), want) {
82+
t.Errorf("followSession() error = %v; want substring %q", err, want)
83+
}
84+
if state == nil {
85+
t.Error("expected non-nil state on non-hard error")
86+
}
87+
}
88+
6789
func TestFollowSession_NonObjectMessage(t *testing.T) {
6890
logp.TestingSetup()
6991

@@ -153,7 +175,7 @@ func newTestStream(t *testing.T, discoverURL string, firehoseClient *http.Client
153175

154176
func newTestStreamWithPublisher(t *testing.T, discoverURL string, firehoseClient *http.Client, pub cursor.Publisher) *falconHoseStream {
155177
t.Helper()
156-
log := logp.L()
178+
log := logptest.NewTestingLogger(t, t.Name())
157179
reg := monitoring.NewRegistry()
158180
m := newInputMetrics(reg, log)
159181

0 commit comments

Comments
 (0)