Skip to content

feat: expose the productized crowdstrike receiver options - #28

Open
alltilla wants to merge 1 commit into
mainfrom
feat/crowdstrike-receiver-productization
Open

feat: expose the productized crowdstrike receiver options#28
alltilla wants to merge 1 commit into
mainfrom
feat/crowdstrike-receiver-productization

Conversation

@alltilla

@alltilla alltilla commented Aug 3, 2026

Copy link
Copy Markdown
Member

Summary

The CrowdStrike receiver gained checkpoint persistence, an NG-SIEM search poller and a separate cadence for it (axoflow/opentelemetry-collector-contrib#39). Wire the new options through the connector: point the receiver at the file_storage extension the connector already runs, so a restart resumes from the stored checkpoint instead of replaying or skipping data, and surface initial_lookback, disable_alerts and the ngsiem_search block as CROWDSTRIKE_* environment variables. poll_interval gains an explicit 30s default.

The entrypoint now unsets CROWDSTRIKE_* variables that are set but empty: the collector expands ${env:VAR:-default} with POSIX - semantics (default only when unset), so an empty variable — docker run -e VAR="${VAR}" with no VAR in the shell, or a Helm value: "" — would resolve to null, zero poll_interval, and fail the receiver's validation at startup.

Requires an image bump

The Dockerfile references axoflow-otel-collector:0.156.0-axoflow.4, which is not published yet — it needs the receiver PR merged, a receiver/crowdstrikereceiver/v0.156.0-axoflow.2 tag cut from it, and the releases-repo pin bumped and published (same dependency shape as the Idira connector, #26).

Test plan

  • Empty-env normalization verified in dash and busybox ash plus an end-to-end entrypoint run with a stub collector (empty vars removed, non-empty preserved, provider detection unaffected)
  • Receiver behaviour behind these options live-validated against a Falcon tenant (checkpoint restart with zero redelivery/zero gap, dup-free NG-SIEM drain)
  • Full connector config validates once the 0.156.0-axoflow.4 image exists

Reviewed with: Claude Opus (adversarial review harness).

Summary by CodeRabbit

  • New Features

    • Added configurable CrowdStrike alert polling, initial lookback, and alert-disable settings.
    • Added optional NG-SIEM event collection with repository, query, and polling configuration.
    • Added persistent checkpoint storage support.
    • Empty CrowdStrike environment variables are now ignored so default settings apply.
  • Documentation

    • Expanded setup and configuration guidance, including Docker usage and alert-only operation.
  • Chores

    • Updated the OpenTelemetry Collector base image.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds CrowdStrike alert and NG-SIEM settings, persistent storage configuration, and startup handling for empty environment variables. It also updates related documentation and the AxoFlow OpenTelemetry Collector base image.

Changes

CrowdStrike collector configuration

Layer / File(s) Summary
Receiver settings and documentation
connectors/crowdstrike/config.yaml, README.md, connectors/crowdstrike/README.md
The receiver adds polling, lookback, alert disabling, NG-SIEM, and file-storage settings. Documentation describes these settings, checkpointing, and Docker usage.
Startup environment cleanup
entrypoint.sh
The startup script exports STORAGE_DIRECTORY, ignores empty provider variables, and unsets empty CrowdStrike variables before launching the collector.
Collector base image
Dockerfile
The Docker build uses 0.156.0-axoflow.4 instead of 0.156.0-axoflow.3.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: csatib02

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: exposing productized CrowdStrike receiver options through the connector.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/crowdstrike-receiver-productization

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
connectors/crowdstrike/README.md (1)

124-136: 📐 Maintainability & Code Quality | 🔵 Trivial

Validate this example against the published image.

The example uses ghcr.io/axoflow/axocloudconnectors:latest, but the PR objective states that the image containing these receiver changes is not published yet. Validate the example after the receiver tag and releases pin are updated, or pin it to a compatible published image.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@connectors/crowdstrike/README.md` around lines 124 - 136, Update the Docker
command in the README to reference a compatible published image, or validate and
replace it with the receiver tag and releases pin once those images are
published. Do not leave the example pointing to an image that does not contain
the receiver changes.
entrypoint.sh (1)

16-16: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Replace eval with a safe variable lookup.

The current callers pass fixed names, so this is not an active command-injection path. eval still reparses $name and makes future callers unsafe. Use Bash indirect expansion or an allowlist. Keep an explicit Bash interpreter because this helper also uses local.

🛠️ Proposed refactor
-        eval "value=\${$name-}"
+        value="${!name:-}"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@entrypoint.sh` at line 16, Replace the eval-based lookup in the helper
containing the local variable with Bash indirect expansion or an explicit
allowlist, preserving retrieval by the variable name in name. Ensure
entrypoint.sh continues to run under an explicit Bash interpreter because the
helper uses local.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@connectors/crowdstrike/config.yaml`:
- Line 65: Implement the documented NG-SIEM query default by updating
connectors/crowdstrike/config.yaml lines 65-65 to use the environment fallback
*. In entrypoint.sh lines 48-58, add CROWDSTRIKE_NGSIEM_QUERY_STRING to
unset_empty_vars so empty values also fall back to *. In README.md lines 97-97
and connectors/crowdstrike/README.md lines 124-136, add the variable to the
Docker examples. Leave CROWDSTRIKE_NGSIEM_REPOSITORY unchanged so an empty value
continues disabling the poller.
- Line 67: Update the storage configuration near the file_storage setting to use
/var/lib/axoflow-otel-collector/storage as the fallback when STORAGE_DIRECTORY
is unset, matching the documented entrypoint.sh default and preserving
checkpoint persistence.

In `@Dockerfile`:
- Line 1: Update the Dockerfile base image reference to the currently published
0.156.0-axoflow.3 tag, or publish and validate the 0.156.0-axoflow.4 image
before retaining the new reference.

In `@entrypoint.sh`:
- Around line 48-58: Move the unset_empty_vars call for the CROWDSTRIKE_*
configuration variables before provider detection so empty values are removed
before detect_provider evaluates configured providers. Preserve the existing
cleanup set and multiple-provider behavior for genuinely non-empty provider
configuration.

---

Nitpick comments:
In `@connectors/crowdstrike/README.md`:
- Around line 124-136: Update the Docker command in the README to reference a
compatible published image, or validate and replace it with the receiver tag and
releases pin once those images are published. Do not leave the example pointing
to an image that does not contain the receiver changes.

In `@entrypoint.sh`:
- Line 16: Replace the eval-based lookup in the helper containing the local
variable with Bash indirect expansion or an explicit allowlist, preserving
retrieval by the variable name in name. Ensure entrypoint.sh continues to run
under an explicit Bash interpreter because the helper uses local.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6cdbc242-5c13-4c4e-8737-d815c0b75f32

📥 Commits

Reviewing files that changed from the base of the PR and between eca4126 and e369ab9.

📒 Files selected for processing (5)
  • Dockerfile
  • README.md
  • connectors/crowdstrike/README.md
  • connectors/crowdstrike/config.yaml
  • entrypoint.sh

Comment thread connectors/crowdstrike/config.yaml
Comment thread connectors/crowdstrike/config.yaml
Comment thread Dockerfile
Comment thread entrypoint.sh
@alltilla
alltilla force-pushed the feat/crowdstrike-receiver-productization branch from e369ab9 to 104026b Compare August 3, 2026 20:36

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Line 94: Update the CROWDSTRIKE_INITIAL_LOOKBACK description in README.md by
changing “afterwards” to the American English spelling “afterward,” leaving the
rest of the documentation unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 52bfece9-7c15-431b-8fdd-29216d99e230

📥 Commits

Reviewing files that changed from the base of the PR and between e369ab9 and 104026b.

📒 Files selected for processing (5)
  • Dockerfile
  • README.md
  • connectors/crowdstrike/README.md
  • connectors/crowdstrike/config.yaml
  • entrypoint.sh
🚧 Files skipped from review as they are similar to previous changes (3)
  • connectors/crowdstrike/config.yaml
  • Dockerfile
  • connectors/crowdstrike/README.md

Comment thread README.md Outdated
The receiver gained checkpoint persistence, an NG-SIEM search poller and
a separate cadence for it. Wire them through: point the receiver at the
file_storage extension the connector already runs, so a restart resumes
from the stored checkpoint instead of replaying or skipping data, and
surface initial_lookback, disable_alerts and the ngsiem_search block as
environment variables.

poll_interval also gains an explicit 30s default: it is no longer a
pointer in the receiver, so an unset environment variable would decode
as zero and fail validation.

That default alone is not enough, so the entrypoint now unsets the
CROWDSTRIKE_* variables that are set but empty. The collector expands
${env:VAR:-default} with POSIX `-` semantics, not `:-`: the default
applies only while VAR is unset, and an empty one resolves to null and
overrides it. Empty is the common case -- `docker run -e VAR="${VAR}"`
with no VAR in the shell, or a Helm `value: ""` -- and it would leave
poll_interval at 0s, which the receiver's validation rejects, so the
collector would refuse to start.

Provider detection has the same blind spot: `env` lists a set-but-empty
variable, so an AWS deployment carrying an empty CROWDSTRIKE_* leftover
was rejected as a multi-provider one. Detection now requires a value.

STORAGE_DIRECTORY's default was assigned to a shell variable but never
exported, so the collector expanded ${env:STORAGE_DIRECTORY} to empty
and the documented default never reached file_storage -- which the
checkpoints now depend on. Export it.

Bumps the collector image to 0.156.0-axoflow.4, which carries the
receiver.

Signed-off-by: Attila Szakacs-Bertok <attila.szakacs@axoflow.com>
Assisted-by: Claude Fable 5
@alltilla
alltilla force-pushed the feat/crowdstrike-receiver-productization branch from 104026b to b2c2b5c Compare August 4, 2026 08:02

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@OverOrion
OverOrion requested review from OverOrion and csatib02 August 4, 2026 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants