feat: expose the productized crowdstrike receiver options - #28
Conversation
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
📝 WalkthroughWalkthroughThe 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. ChangesCrowdStrike collector configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
connectors/crowdstrike/README.md (1)
124-136: 📐 Maintainability & Code Quality | 🔵 TrivialValidate 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 winReplace
evalwith a safe variable lookup.The current callers pass fixed names, so this is not an active command-injection path.
evalstill reparses$nameand makes future callers unsafe. Use Bash indirect expansion or an allowlist. Keep an explicit Bash interpreter because this helper also useslocal.🛠️ 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
📒 Files selected for processing (5)
DockerfileREADME.mdconnectors/crowdstrike/README.mdconnectors/crowdstrike/config.yamlentrypoint.sh
e369ab9 to
104026b
Compare
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
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
📒 Files selected for processing (5)
DockerfileREADME.mdconnectors/crowdstrike/README.mdconnectors/crowdstrike/config.yamlentrypoint.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- connectors/crowdstrike/config.yaml
- Dockerfile
- connectors/crowdstrike/README.md
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
104026b to
b2c2b5c
Compare
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
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_storageextension the connector already runs, so a restart resumes from the stored checkpoint instead of replaying or skipping data, and surfaceinitial_lookback,disable_alertsand thengsiem_searchblock asCROWDSTRIKE_*environment variables.poll_intervalgains an explicit30sdefault.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 Helmvalue: ""— would resolve to null, zeropoll_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, areceiver/crowdstrikereceiver/v0.156.0-axoflow.2tag cut from it, and the releases-repo pin bumped and published (same dependency shape as the Idira connector, #26).Test plan
0.156.0-axoflow.4image existsReviewed with: Claude Opus (adversarial review harness).
Summary by CodeRabbit
New Features
Documentation
Chores