Rename l4/l7 related controls#194
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the tracer’s configuration semantics to rename and invert the “capture stopped” flag into an explicit “capture enabled” flag, aligning the Go-side config parsing with the BPF-side flag check.
Changes:
- Renamed config-map keys related to capture/dissection controls (e.g.,
STOPPED→DISSECTION_ENABLED,RAW_CAPTURE→RAW_CAPTURE_ENABLED). - Renamed/inverted the configuration bit from
CONFIGURATION_FLAG_CAPTURE_STOPPEDtoCONFIGURATION_FLAG_CAPTURE_ENABLED. - Updated BPF
program_disabled()logic to disable programs when capture is not enabled.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/kubernetes/config.go | Renames config keys and computes the new “capture enabled” bit from config-map values. |
| bpf/include/maps.h | Renames the BPF configuration flag macro to CONFIGURATION_FLAG_CAPTURE_ENABLED. |
| bpf/common.c | Updates program gating to use the new “capture enabled” flag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var dissectionEnabled bool | ||
| var rawCaptureEnabled bool | ||
| if dissectionEnabled, err = strconv.ParseBool(configMap.Data[CONFIG_DISSECTION_ENABLED]); err != nil { | ||
| log.Error().Err(err).Str("config", CONFIG_DISSECTION_ENABLED).Send() | ||
| } | ||
| if rawCapture, err = strconv.ParseBool(configMap.Data[CONFIG_RAW_CAPTURE]); err != nil { | ||
| log.Error().Err(err).Str("config", CONFIG_RAW_CAPTURE).Send() | ||
| if rawCaptureEnabled, err = strconv.ParseBool(configMap.Data[CONFIG_RAW_CAPTURE_ENABLED]); err != nil { | ||
| log.Error().Err(err).Str("config", CONFIG_RAW_CAPTURE_ENABLED).Send() | ||
| } | ||
| if stopped && !rawCapture { | ||
| settings |= CONFIGURATION_FLAG_CAPTURE_STOPPED | ||
| if dissectionEnabled || rawCaptureEnabled { | ||
| settings |= CONFIGURATION_FLAG_CAPTURE_ENABLED | ||
| } |
There was a problem hiding this comment.
SyncConfig now only sets CONFIGURATION_FLAG_CAPTURE_ENABLED when DISSECTION_ENABLED or RAW_CAPTURE_ENABLED parses to true. If these keys are missing (or empty), ParseBool returns an error and leaves both booleans false, which disables capture by default and can also spam logs on every config sync. Consider treating missing/empty values as a non-error and applying a safe default (e.g., dissection enabled by default), only logging when a value is present but invalid.
| SUFFIX_CONFIG_MAP = "config-map" | ||
| CONFIG_POD_REGEX = "POD_REGEX" | ||
| CONFIG_NAMESPACES = "NAMESPACES" | ||
| CONFIG_DISSECTION_ENABLED = "DISSECTION_ENABLED" | ||
|
|
||
| CONFIG_RAW_CAPTURE = "RAW_CAPTURE" | ||
| CONFIG_RAW_CAPTURE_ENABLED = "RAW_CAPTURE_ENABLED" | ||
| ) |
There was a problem hiding this comment.
Renaming config-map keys from STOPPED/RAW_CAPTURE to DISSECTION_ENABLED/RAW_CAPTURE_ENABLED is a breaking config schema change. If there are existing deployments/controllers still writing the old keys, this tracer will ignore them. Consider supporting both old and new keys during a deprecation window (e.g., check for the new key first, then fall back to the old key names) to avoid silent behavior changes.
| if (s && !(s->flags & CONFIGURATION_FLAG_CAPTURE_ENABLED)) { | ||
| return 1; | ||
| } |
There was a problem hiding this comment.
program_disabled() returns early when CAPTURE_ENABLED is not set, which means the later special-case "system always enabled" logic is bypassed whenever capture is disabled. Either move the PROGRAM_DOMAIN_CAPTURE_SYSTEM check before the CAPTURE_ENABLED gate (if system truly must always run), or adjust the comment/behavior to match the intended semantics.
There was a problem hiding this comment.
@copilot does this rename causes this or it was like that before? Current PR goal is adjusting naming for clearer understanding of what those toggles do
Towards https://github.com/kubeshark/devops/issues/111