Skip to content

Fix pprof broad exposition #2821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jotak merged 3 commits into from
Jun 25, 2026
Merged

Fix pprof broad exposition #2821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
78a1a57
Fix pprof broad exposition
jotak Jun 19, 2026
8ac438d
bump flp/agent deps
jotak Jun 25, 2026
a000bce
fix agent tests
jotak Jun 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .mk/local.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ IMAGE_FOR_HELM := $(word 1,$(subst :, ,${IMAGE}))
VERSION_FOR_HELM := $(word 2,$(subst :, ,${IMAGE}))
.PHONY: helm-install
helm-install: prereqs-helm ## Install the operator and its pre-requisites to a running cluster, using Helm
cd helm && helm dependency update --skip-refresh ; cd ..

@coderabbitai coderabbitai Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Failed helm dependency update is silently swallowed.

cd helm && helm dependency update --skip-refresh ; cd .. — the trailing ; cd .. always exits 0, masking a non-zero exit from helm dependency update. Make sees a successful step and proceeds to install with potentially stale/missing chart dependencies.

🛠️ Proposed fix 
-	cd helm && helm dependency update --skip-refresh ; cd ..
+	cd helm && helm dependency update --skip-refresh && cd ..
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cd helm && helm dependency update --skip-refresh ; cd ..
cd helm && helm dependency update --skip-refresh && cd ..
🤖 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 @.mk/local.mk at line 10, The helm dependency update step in the local.mk
target is swallowing failures because the trailing cd .. makes the overall shell
command succeed even when helm dependency update fails. Update this recipe so
the dependency update command in the helm context is the only result that
determines success, and ensure the Make target stops on a non-zero exit from
helm dependency update instead of proceeding to later steps.

helm repo add cert-manager https://charts.jetstack.io
helm upgrade --install cert-manager -n cert-manager --create-namespace cert-manager/cert-manager --set crds.enabled=true
helm upgrade --install trust-manager -n cert-manager oci://quay.io/jetstack/charts/trust-manager --wait
Expand Down
10 changes: 6 additions & 4 deletions api/flowcollector/v1beta2/flowcollector_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1395,18 +1395,20 @@ type AdvancedProcessorConfig struct {
//+kubebuilder:validation:Maximum=65535
//+kubebuilder:default:=8080
//+optional
// `healthPort` is a collector HTTP port in the Pod that exposes the health check API
// `healthPort` is a collector HTTP port in the Pod that exposes the health check API.
HealthPort *int32 `json:"healthPort,omitempty"`

//+kubebuilder:validation:Minimum=0
//+kubebuilder:validation:Maximum=65535
//+optional
// `profilePort` allows setting up a Go pprof profiler listening to this port
// `profilePort` allows setting up a Go pprof profiler listening to this port.
// This is for debugging purpose only. This port should not be exposed, you can
// access it through local port-forwarding.
ProfilePort *int32 `json:"profilePort,omitempty"`

//+kubebuilder:default:=true
//+optional
// `enableKubeProbes` is a flag to enable or disable Kubernetes liveness and readiness probes
// `enableKubeProbes` is a flag to enable or disable Kubernetes liveness and readiness probes.
EnableKubeProbes *bool `json:"enableKubeProbes,omitempty"`

//+kubebuilder:default:=true
Expand All @@ -1416,7 +1418,7 @@ type AdvancedProcessorConfig struct {

//+kubebuilder:default:="30s"
//+optional
// `conversationHeartbeatInterval` is the time to wait between "tick" events of a conversation
// `conversationHeartbeatInterval` is the time to wait between "tick" events of a conversation.
ConversationHeartbeatInterval *metav1.Duration `json:"conversationHeartbeatInterval,omitempty"`

//+kubebuilder:default:="10s"
Expand Down
16 changes: 16 additions & 0 deletions api/flowcollector/v1beta2/flowcollector_validation_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ func (r *FlowCollector) Validate(_ context.Context, fc *FlowCollector) (admissio
v.validateAgent()
v.validateFLP()
v.warnLogLevels()
v.warnProfiling()
v.warnLokiDemo()
return v.warnings, errors.Join(v.errors...)
}
Expand All @@ -87,6 +88,21 @@ func (v *validator) warnLogLevels() {
}
}

func (v *validator) warnProfiling() {
warning := "This is for debugging purpose only. The profiling port should not be exposed, you can access it through local port-forwarding."
if v.fc.Agent.EBPF.Advanced != nil {
if env, ok := v.fc.Agent.EBPF.Advanced.Env["PPROF_ADDR"]; ok && env != "" {
v.warnings = append(v.warnings, "Profiling is enabled on the eBPF agent. "+warning)
if strings.HasPrefix(env, ":") || strings.HasPrefix(env, "0.0.0.0:") {
v.warnings = append(v.warnings, "Profiling is enabled for all network interfaces, make sure access is restricted e.g. with a network policy.")
}
Comment on lines +96 to +98

@coderabbitai coderabbitai Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Broad-bind check misses the IPv6 wildcard [::]:.

PPROF_ADDR=[::]:6060 binds all interfaces but slips past the prefix checks, so the "all network interfaces" warning is never emitted for the IPv6 wildcard case.

🛡️ Proposed fix 
-			if strings.HasPrefix(env, ":") || strings.HasPrefix(env, "0.0.0.0:") {
+			if strings.HasPrefix(env, ":") || strings.HasPrefix(env, "0.0.0.0:") || strings.HasPrefix(env, "[::]:") {
 				v.warnings = append(v.warnings, "Profiling is enabled for all network interfaces, make sure access is restricted e.g. with a network policy.")
 			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if strings.HasPrefix(env, ":") || strings.HasPrefix(env, "0.0.0.0:") {
v.warnings = append(v.warnings, "Profiling is enabled for all network interfaces, make sure access is restricted e.g. with a network policy.")
}
if strings.HasPrefix(env, ":") || strings.HasPrefix(env, "0.0.0.0:") || strings.HasPrefix(env, "[::]:") {
v.warnings = append(v.warnings, "Profiling is enabled for all network interfaces, make sure access is restricted e.g. with a network policy.")
}
🤖 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 `@api/flowcollector/v1beta2/flowcollector_validation_webhook.go` around lines
96 - 98, The broad-bind warning in flowcollector validation currently only
checks for ":" and "0.0.0.0:" prefixes, so it misses the IPv6 wildcard bind
case. Update the warning logic in the validation webhook’s environment address
check to also detect the "[::]:" wildcard pattern alongside the existing checks,
so the profiling warning is emitted whenever PPROF_ADDR binds all interfaces.

}
}
if v.fc.Processor.Advanced != nil && v.fc.Processor.Advanced.ProfilePort != nil && *v.fc.Processor.Advanced.ProfilePort > 0 {
v.warnings = append(v.warnings, "Profiling is enabled on flowlogs-pipeline. "+warning)
}
}

func (v *validator) warnLokiDemo() {
if v.fc.Loki.Mode == LokiModeMonolithic && v.fc.Loki.Monolithic.InstallDemoLoki != nil && *v.fc.Loki.Monolithic.InstallDemoLoki {
v.warnings = append(v.warnings, "InstallDemoLoki option is enabled. This is useful for development and demo purposes but should not be used in production.")
Expand Down
12 changes: 7 additions & 5 deletions bundle/manifests/flows.netobserv.io_flowcollectors.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4378,7 +4378,7 @@ spec:
conversationHeartbeatInterval:
default: 30s
description: '`conversationHeartbeatInterval` is the time
to wait between "tick" events of a conversation'
to wait between "tick" events of a conversation.'
type: string
conversationTerminatingTimeout:
default: 5s
Expand All @@ -4394,7 +4394,7 @@ spec:
enableKubeProbes:
default: true
description: '`enableKubeProbes` is a flag to enable or disable
Kubernetes liveness and readiness probes'
Kubernetes liveness and readiness probes.'
type: boolean
env:
additionalProperties:
Expand All @@ -4408,7 +4408,7 @@ spec:
healthPort:
default: 8080
description: '`healthPort` is a collector HTTP port in the
Pod that exposes the health check API'
Pod that exposes the health check API.'
format: int32
maximum: 65535
minimum: 1
Expand All @@ -4424,8 +4424,10 @@ spec:
minimum: 1025
type: integer
profilePort:
description: '`profilePort` allows setting up a Go pprof profiler
listening to this port'
description: |-
`profilePort` allows setting up a Go pprof profiler listening to this port.
This is for debugging purpose only. This port should not be exposed, you can
access it through local port-forwarding.
format: int32
maximum: 65535
minimum: 0
Expand Down
11 changes: 7 additions & 4 deletions config/crd/bases/flows.netobserv.io_flowcollectors.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4029,7 +4029,7 @@ spec:
type: string
conversationHeartbeatInterval:
default: 30s
description: '`conversationHeartbeatInterval` is the time to wait between "tick" events of a conversation'
description: '`conversationHeartbeatInterval` is the time to wait between "tick" events of a conversation.'
type: string
conversationTerminatingTimeout:
default: 5s
Expand All @@ -4041,7 +4041,7 @@ spec:
type: boolean
enableKubeProbes:
default: true
description: '`enableKubeProbes` is a flag to enable or disable Kubernetes liveness and readiness probes'
description: '`enableKubeProbes` is a flag to enable or disable Kubernetes liveness and readiness probes.'
type: boolean
env:
additionalProperties:
Expand All @@ -4054,7 +4054,7 @@ spec:
type: object
healthPort:
default: 8080
description: '`healthPort` is a collector HTTP port in the Pod that exposes the health check API'
description: '`healthPort` is a collector HTTP port in the Pod that exposes the health check API.'
format: int32
maximum: 65535
minimum: 1
Expand All @@ -4070,7 +4070,10 @@ spec:
minimum: 1025
type: integer
profilePort:
description: '`profilePort` allows setting up a Go pprof profiler listening to this port'
description: |-
`profilePort` allows setting up a Go pprof profiler listening to this port.
This is for debugging purpose only. This port should not be exposed, you can
access it through local port-forwarding.
format: int32
maximum: 65535
minimum: 0
Expand Down
10 changes: 6 additions & 4 deletions docs/FlowCollector.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8711,7 +8711,7 @@ This delay is ignored when a FIN packet is collected for TCP flows (see `convers
<td><b>conversationHeartbeatInterval</b></td>
<td>string</td>
<td>
`conversationHeartbeatInterval` is the time to wait between "tick" events of a conversation<br/>
`conversationHeartbeatInterval` is the time to wait between "tick" events of a conversation.<br/>
<br/>
<i>Default</i>: 30s<br/>
</td>
Expand All @@ -8738,7 +8738,7 @@ This delay is ignored when a FIN packet is collected for TCP flows (see `convers
<td><b>enableKubeProbes</b></td>
<td>boolean</td>
<td>
`enableKubeProbes` is a flag to enable or disable Kubernetes liveness and readiness probes<br/>
`enableKubeProbes` is a flag to enable or disable Kubernetes liveness and readiness probes.<br/>
<br/>
<i>Default</i>: true<br/>
</td>
Expand All @@ -8757,7 +8757,7 @@ in edge debug or support scenarios.<br/>
<td><b>healthPort</b></td>
<td>integer</td>
<td>
`healthPort` is a collector HTTP port in the Pod that exposes the health check API<br/>
`healthPort` is a collector HTTP port in the Pod that exposes the health check API.<br/>
<br/>
<i>Format</i>: int32<br/>
<i>Default</i>: 8080<br/>
Expand All @@ -8783,7 +8783,9 @@ By convention, some values are forbidden. It must be greater than 1024 and diffe
<td><b>profilePort</b></td>
<td>integer</td>
<td>
`profilePort` allows setting up a Go pprof profiler listening to this port<br/>
`profilePort` allows setting up a Go pprof profiler listening to this port.
This is for debugging purpose only. This port should not be exposed, you can
access it through local port-forwarding.<br/>
<br/>
<i>Format</i>: int32<br/>
<i>Minimum</i>: 0<br/>
Expand Down
26 changes: 13 additions & 13 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ require (
github.com/coreos/go-semver v0.3.1
github.com/google/go-cmp v0.7.0
github.com/grafana/loki/operator/apis/loki v0.0.0-20241021105923-5e970e50b166
github.com/netobserv/flowlogs-pipeline v1.11.4-community.0.20260521155724-7ee2fbbc7651
github.com/netobserv/netobserv-ebpf-agent v1.11.3-community.0.20260505134559-24f8cbea14ad
github.com/netobserv/flowlogs-pipeline v1.11.5-community.0.20260625083056-0aefbc7d051e
github.com/netobserv/netobserv-ebpf-agent v1.11.5-community.0.20260625084536-cd2c2a1fa4fb
github.com/onsi/ginkgo/v2 v2.31.0
github.com/onsi/gomega v1.41.0
github.com/openshift/api v0.0.0-20250707164913-2cd5821c9080
Expand Down Expand Up @@ -42,7 +42,7 @@ require (
github.com/evanphx/json-patch v5.9.11+incompatible // indirect
github.com/evanphx/json-patch/v5 v5.9.11 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
github.com/fsnotify/fsnotify v1.9.0 // indirect
github.com/fsnotify/fsnotify v1.10.1 // indirect
github.com/fxamacker/cbor/v2 v2.9.0 // indirect
github.com/go-logr/logr v1.4.3 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
Expand All @@ -69,7 +69,7 @@ require (
github.com/google/gnostic-models v0.7.1 // indirect
github.com/google/pprof v0.0.0-20260402051712-545e8a4df936 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/jpillora/backoff v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
Expand All @@ -87,12 +87,12 @@ require (
github.com/x448/float16 v0.8.4 // indirect
go.opentelemetry.io/auto/sdk v1.2.1 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.65.0 // indirect
go.opentelemetry.io/otel v1.43.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.43.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.43.0 // indirect
go.opentelemetry.io/otel/metric v1.43.0 // indirect
go.opentelemetry.io/otel/sdk v1.43.0 // indirect
go.opentelemetry.io/otel/trace v1.43.0 // indirect
go.opentelemetry.io/otel v1.44.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.44.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.44.0 // indirect
go.opentelemetry.io/otel/metric v1.44.0 // indirect
go.opentelemetry.io/otel/sdk v1.44.0 // indirect
go.opentelemetry.io/otel/trace v1.44.0 // indirect
go.opentelemetry.io/proto/otlp v1.10.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.yaml.in/yaml/v2 v2.4.4 // indirect
Expand All @@ -102,14 +102,14 @@ require (
golang.org/x/net v0.55.0 // indirect
golang.org/x/oauth2 v0.36.0 // indirect
golang.org/x/sync v0.20.0 // indirect
golang.org/x/sys v0.45.0 // indirect
golang.org/x/sys v0.46.0 // indirect
golang.org/x/term v0.43.0 // indirect
golang.org/x/text v0.37.0 // indirect
golang.org/x/time v0.15.0 // indirect
golang.org/x/tools v0.44.0 // indirect
gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa // indirect
google.golang.org/grpc v1.81.1 // indirect
google.golang.org/protobuf v1.36.11 // indirect
gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect
Expand Down
Loading
Loading