From 424d5215e16167477db3c5cdc1a842740107157b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=AB=98=E7=84=B6?= Date: Thu, 13 Aug 2026 22:12:29 +0800 Subject: [PATCH 1/5] feat(egress): auto-allow OTLP endpoint egress traffic (#1491) Parse OTEL_EXPORTER_OTLP_METRICS_ENDPOINT / OTEL_EXPORTER_OTLP_ENDPOINT at egress startup and inject an always-allow rule for the endpoint host so telemetry export works under the default deny-all policy without manual allowlist rules. The rule survives user policy updates and always-rule file reloads; deny.always still takes precedence. --- components/egress/docs/opentelemetry.md | 19 ++++ components/egress/main.go | 1 + components/egress/policy_server.go | 2 + components/egress/telemetry_allow.go | 54 +++++++++++ components/egress/telemetry_allow_test.go | 90 +++++++++++++++++++ components/internal/telemetry/endpoint.go | 80 +++++++++++++++++ .../internal/telemetry/endpoint_test.go | 82 +++++++++++++++++ 7 files changed, 328 insertions(+) create mode 100644 components/egress/telemetry_allow.go create mode 100644 components/egress/telemetry_allow_test.go create mode 100644 components/internal/telemetry/endpoint.go create mode 100644 components/internal/telemetry/endpoint_test.go diff --git a/components/egress/docs/opentelemetry.md b/components/egress/docs/opentelemetry.md index a7830def8..2461dd85c 100644 --- a/components/egress/docs/opentelemetry.md +++ b/components/egress/docs/opentelemetry.md @@ -91,6 +91,25 @@ Metric export is enabled only when at least one OTLP endpoint is set. If both are unset, egress keeps metrics local (no OTLP export). +### Automatic Egress Allow Rule + +When an OTLP endpoint is configured, egress automatically injects an +always-allow egress rule for the endpoint host (domain or IP, any port), so +telemetry export works under the default deny-all policy without manually +managing allowlist rules. This also covers the egress sidecar's own metric +export, which shares the sandbox network namespace and would otherwise be +blocked by its own egress chain. + +- The rule follows the standard precedence: `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` + wins over `OTEL_EXPORTER_OTLP_ENDPOINT`. +- The host is taken from the endpoint URL (`https://host:4318/v1/metrics`), + `host:port`, or bare `host` forms. +- The rule lives in the always-allow layer: it survives user `POST`/`PATCH`/`DELETE` + policy updates and always-rule file reloads. Operators can still block the target + with `deny.always`, which takes precedence. +- Rules are host-scoped (any port), matching the egress rule model; ports are not + enforced per rule. + ### Minimal Example ```bash diff --git a/components/egress/main.go b/components/egress/main.go index d74c98c23..d6aeb6c79 100644 --- a/components/egress/main.go +++ b/components/egress/main.go @@ -78,6 +78,7 @@ func main() { if err != nil { log.Fatalf("failed to load always allow/deny rule files: %v", err) } + alwaysAllow = withTelemetryAllow(alwaysAllow) allowIPs := allowIps() mode := parseMode() diff --git a/components/egress/policy_server.go b/components/egress/policy_server.go index ac510f3cc..b6a05ff1e 100644 --- a/components/egress/policy_server.go +++ b/components/egress/policy_server.go @@ -713,6 +713,8 @@ func (s *policyServer) reloadAlwaysRules() (bool, error) { if !changed { return false, nil } + allow = withTelemetryAllow(allow) + s.setAlwaysRules(deny, allow) s.proxy.UpdateAlwaysRules(deny, allow) return true, nil } diff --git a/components/egress/telemetry_allow.go b/components/egress/telemetry_allow.go new file mode 100644 index 000000000..c960ae8f6 --- /dev/null +++ b/components/egress/telemetry_allow.go @@ -0,0 +1,54 @@ +// Copyright 2026 Alibaba Group Holding Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "github.com/alibaba/opensandbox/egress/pkg/log" + "github.com/alibaba/opensandbox/egress/pkg/policy" + inttelemetry "github.com/alibaba/opensandbox/internal/telemetry" +) + +// telemetryAllowRules returns an always-allow egress rule for the configured OTLP +// endpoint (OTEL_EXPORTER_OTLP_METRICS_ENDPOINT / OTEL_EXPORTER_OTLP_ENDPOINT), so +// metric export works under the default deny-all policy without operator-provided +// allowlist rules. The rule targets the endpoint host (any port), matching the +// egress rule model. Operators can still block the target via deny.always, which +// takes precedence. Returns nil when no endpoint is configured. +func telemetryAllowRules() []policy.EgressRule { + host, port, ok := inttelemetry.OTLPEndpointHostPort() + if !ok { + return nil + } + rule, err := policy.ParseValidatedEgressRule(policy.ActionAllow, host) + if err != nil { + log.Warnf("telemetry: skipping auto egress allow for OTLP endpoint host %q: %v", host, err) + return nil + } + log.Infof("telemetry: auto-allowing egress to OTLP endpoint %s:%s (deny.always can override)", host, port) + return []policy.EgressRule{rule} +} + +// withTelemetryAllow appends the auto-generated OTLP allow rule(s) to the +// always-allow list so every effective-policy merge (startup, policy updates, +// always-file reloads) keeps telemetry egress open. +func withTelemetryAllow(allow []policy.EgressRule) []policy.EgressRule { + rules := telemetryAllowRules() + if len(rules) == 0 { + return allow + } + out := make([]policy.EgressRule, 0, len(allow)+len(rules)) + out = append(out, allow...) + return append(out, rules...) +} diff --git a/components/egress/telemetry_allow_test.go b/components/egress/telemetry_allow_test.go new file mode 100644 index 000000000..cf6912beb --- /dev/null +++ b/components/egress/telemetry_allow_test.go @@ -0,0 +1,90 @@ +// Copyright 2026 Alibaba Group Holding Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "testing" + + "github.com/alibaba/opensandbox/egress/pkg/policy" + "github.com/stretchr/testify/require" +) + +func TestTelemetryAllowRulesUnconfigured(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "") + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") + require.Nil(t, telemetryAllowRules()) + + existing := []policy.EgressRule{{Action: policy.ActionAllow, Target: "a.example.com"}} + require.Equal(t, existing, withTelemetryAllow(existing), "no telemetry rules must not mutate the input") +} + +func TestTelemetryAllowRulesFromMetricsEndpoint(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "https://collector.example:4318/v1/metrics") + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") + rules := telemetryAllowRules() + require.Len(t, rules, 1) + require.Equal(t, policy.ActionAllow, rules[0].Action) + require.Equal(t, "collector.example", rules[0].Target) + + merged := policy.MergeAlwaysOverlay(policy.DefaultDenyPolicy(), nil, rules) + require.Equal(t, policy.ActionAllow, merged.Evaluate("collector.example."), "domain rule must allow DNS resolution") + allowV4, allowV6, _, _ := merged.StaticIPSets() + require.Empty(t, allowV4) + require.Empty(t, allowV6) +} + +func TestTelemetryAllowRulesFallbackEndpoint(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "") + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "otel-collector:4318") + rules := telemetryAllowRules() + require.Len(t, rules, 1) + require.Equal(t, "otel-collector", rules[0].Target) +} + +func TestTelemetryAllowRulesIPEndpoint(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "http://10.0.0.5:4317") + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") + rules := telemetryAllowRules() + require.Len(t, rules, 1) + require.Equal(t, "10.0.0.5", rules[0].Target) + + merged := policy.MergeAlwaysOverlay(policy.DefaultDenyPolicy(), nil, rules) + allowV4, allowV6, _, _ := merged.StaticIPSets() + require.Equal(t, []string{"10.0.0.5"}, allowV4, "IP target must land in the static allow v4 set") + require.Empty(t, allowV6) +} + +func TestTelemetryAllowRulesInvalidEndpoint(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "http://") + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") + require.Nil(t, telemetryAllowRules()) +} + +func TestWithTelemetryAllowAppends(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "collector.example:4318") + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") + existingRule, err := policy.ParseValidatedEgressRule(policy.ActionAllow, "a.example.com") + require.NoError(t, err) + existing := []policy.EgressRule{existingRule} + rules := withTelemetryAllow(existing) + require.Len(t, rules, 2) + require.Equal(t, "a.example.com", rules[0].Target) + require.Equal(t, "collector.example", rules[1].Target) + + merged := policy.MergeAlwaysOverlay(policy.DefaultDenyPolicy(), nil, rules) + require.Equal(t, policy.ActionDeny, merged.Evaluate("other.example.com.")) + require.Equal(t, policy.ActionAllow, merged.Evaluate("a.example.com.")) + require.Equal(t, policy.ActionAllow, merged.Evaluate("collector.example.")) +} diff --git a/components/internal/telemetry/endpoint.go b/components/internal/telemetry/endpoint.go new file mode 100644 index 000000000..dbdaf2f2e --- /dev/null +++ b/components/internal/telemetry/endpoint.go @@ -0,0 +1,80 @@ +// Copyright 2026 Alibaba Group Holding Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package telemetry + +import ( + "net" + "net/url" + "strings" +) + +// OTLPEndpointHostPort returns the host and port of the configured OTLP +// endpoint. Endpoint precedence matches the exporters: +// OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, then OTEL_EXPORTER_OTLP_ENDPOINT. +// A missing port falls back to the scheme default (https->443, http->80); +// a bare host:port or host without a scheme is treated as https. ok is false +// when no endpoint is configured or it cannot be parsed. +func OTLPEndpointHostPort() (host, port string, ok bool) { + raw := otlpEndpointFromEnv() + if raw == "" { + return "", "", false + } + return parseOTLPEndpoint(raw) +} + +func parseOTLPEndpoint(raw string) (host, port string, ok bool) { + raw = strings.TrimSpace(raw) + if raw == "" { + return "", "", false + } + if strings.Contains(raw, "://") { + u, err := url.Parse(raw) + if err != nil { + return "", "", false + } + host = strings.TrimSpace(u.Hostname()) + if host == "" { + return "", "", false + } + port = u.Port() + if port == "" { + port = defaultPortForScheme(u.Scheme) + } + return host, port, true + } + if h, p, err := net.SplitHostPort(raw); err == nil { + host, port = strings.TrimSpace(h), strings.TrimSpace(p) + if host == "" { + return "", "", false + } + return host, port, true + } + host = strings.TrimSpace(raw) + if host == "" { + return "", "", false + } + // No scheme: per OTLP spec the https scheme (port 443) is assumed. + return host, "443", true +} + +func defaultPortForScheme(scheme string) string { + switch strings.ToLower(scheme) { + case "https": + return "443" + case "http": + return "80" + } + return "" +} diff --git a/components/internal/telemetry/endpoint_test.go b/components/internal/telemetry/endpoint_test.go new file mode 100644 index 000000000..22aea9447 --- /dev/null +++ b/components/internal/telemetry/endpoint_test.go @@ -0,0 +1,82 @@ +// Copyright 2026 Alibaba Group Holding Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package telemetry + +import "testing" + +func TestParseOTLPEndpoint(t *testing.T) { + cases := []struct { + name string + raw string + host string + port string + ok bool + }{ + {name: "empty", raw: "", ok: false}, + {name: "whitespace", raw: " ", ok: false}, + {name: "url with port and path", raw: "https://collector.example:4318/v1/metrics", host: "collector.example", port: "4318", ok: true}, + {name: "url without port", raw: "https://collector.example/v1/metrics", host: "collector.example", port: "443", ok: true}, + {name: "http url without port", raw: "http://collector.example/v1/metrics", host: "collector.example", port: "80", ok: true}, + {name: "ip url", raw: "http://10.0.0.1:4317", host: "10.0.0.1", port: "4317", ok: true}, + {name: "ipv6 url", raw: "http://[::1]:4318/v1/metrics", host: "::1", port: "4318", ok: true}, + {name: "host port", raw: "collector.example:4318", host: "collector.example", port: "4318", ok: true}, + {name: "ip port", raw: "10.0.0.1:4318", host: "10.0.0.1", port: "4318", ok: true}, + {name: "bare host", raw: "collector.example", host: "collector.example", port: "443", ok: true}, + {name: "bare ip", raw: "10.0.0.1", host: "10.0.0.1", port: "443", ok: true}, + {name: "scheme only", raw: "http://", ok: false}, + {name: "malformed url", raw: "https://:443", ok: false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + host, port, ok := parseOTLPEndpoint(tc.raw) + if ok != tc.ok { + t.Fatalf("parseOTLPEndpoint(%q) ok=%v, want %v", tc.raw, ok, tc.ok) + } + if host != tc.host || port != tc.port { + t.Fatalf("parseOTLPEndpoint(%q) = (%q, %q), want (%q, %q)", tc.raw, host, port, tc.host, tc.port) + } + }) + } +} + +func TestOTLPEndpointHostPortPrecedence(t *testing.T) { + t.Setenv(envOTLPMetricsEndpoint, "") + t.Setenv(envOTLPEndpoint, "") + host, _, ok := OTLPEndpointHostPort() + if ok { + t.Fatal("expected no endpoint when both env vars are unset") + } + if host != "" { + t.Fatalf("expected empty host, got %q", host) + } + + t.Setenv(envOTLPEndpoint, "fallback.example:4318") + host, port, ok := OTLPEndpointHostPort() + if !ok || host != "fallback.example" || port != "4318" { + t.Fatalf("fallback endpoint parsed as (%q, %q, %v)", host, port, ok) + } + + t.Setenv(envOTLPMetricsEndpoint, "https://primary.example:4317/v1/metrics") + host, port, ok = OTLPEndpointHostPort() + if !ok || host != "primary.example" || port != "4317" { + t.Fatalf("metrics endpoint should win; parsed as (%q, %q, %v)", host, port, ok) + } + + t.Setenv(envOTLPMetricsEndpoint, " ") + host, port, ok = OTLPEndpointHostPort() + if !ok || host != "fallback.example" || port != "4318" { + t.Fatalf("blank metrics endpoint should fall back; parsed as (%q, %q, %v)", host, port, ok) + } +} From e2da97a8ca1777626ce0edade635472b65bd7962 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=AB=98=E7=84=B6?= Date: Fri, 14 Aug 2026 10:17:44 +0800 Subject: [PATCH 2/5] docs(egress): use FQDN in OTLP endpoint example (#1491) Single-label names are subject to search-domain expansion; the deny-all DNS proxy answers expanded names with NXDOMAIN without fallback, so the example would not reach the collector. --- components/egress/docs/opentelemetry.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/components/egress/docs/opentelemetry.md b/components/egress/docs/opentelemetry.md index 2461dd85c..b5ecf335a 100644 --- a/components/egress/docs/opentelemetry.md +++ b/components/egress/docs/opentelemetry.md @@ -110,10 +110,23 @@ blocked by its own egress chain. - Rules are host-scoped (any port), matching the egress rule model; ports are not enforced per rule. +> **Note**: use a fully-qualified service name or an IP in the endpoint. +> Single-label names (e.g. `otel-collector`) are subject to resolver +> search-domain expansion, and the deny-all DNS proxy answers the expanded +> names (e.g. `otel-collector..svc.cluster.local`) with NXDOMAIN without +> falling back to the bare name, so the auto-generated exact-host allow rule +> would not be reached. + ### Minimal Example ```bash -export OTEL_EXPORTER_OTLP_METRICS_ENDPOINT="http://otel-collector:4318" +export OTEL_EXPORTER_OTLP_METRICS_ENDPOINT="http://otel-collector.sandbox.svc.cluster.local:4318" +``` + +An IP endpoint works as well: + +```bash +export OTEL_EXPORTER_OTLP_METRICS_ENDPOINT="http://10.0.0.5:4318" ``` ### Service Name From 1e63b77cf0d98bda7220e8c6c502a195956b291f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=AB=98=E7=84=B6?= Date: Fri, 14 Aug 2026 11:00:25 +0800 Subject: [PATCH 3/5] fix(egress): cover fallback OTLP endpoint and trailing-dot FQDNs (#1491) - Auto-allow the exporter fallback node IP (HOST_IP / /etc/hostinfo) when no OTEL endpoint env var is set, so egress's own metric export is not blocked by its own deny-all chain in that configuration. - Trim the trailing root dot from FQDN endpoint hosts before building the rule so it matches DNS policy normalization. --- components/egress/docs/opentelemetry.md | 19 ++++++++------ components/egress/telemetry_allow.go | 18 ++++++++----- components/egress/telemetry_allow_test.go | 26 +++++++++++++++++++ components/internal/telemetry/endpoint.go | 23 +++++++++++++--- .../internal/telemetry/endpoint_test.go | 17 ++++++++++++ 5 files changed, 85 insertions(+), 18 deletions(-) diff --git a/components/egress/docs/opentelemetry.md b/components/egress/docs/opentelemetry.md index b5ecf335a..b2f660ef5 100644 --- a/components/egress/docs/opentelemetry.md +++ b/components/egress/docs/opentelemetry.md @@ -93,17 +93,20 @@ If both are unset, egress keeps metrics local (no OTLP export). ### Automatic Egress Allow Rule -When an OTLP endpoint is configured, egress automatically injects an -always-allow egress rule for the endpoint host (domain or IP, any port), so -telemetry export works under the default deny-all policy without manually -managing allowlist rules. This also covers the egress sidecar's own metric -export, which shares the sandbox network namespace and would otherwise be -blocked by its own egress chain. +When an OTLP destination is configured — the endpoint env vars below, or the +exporter fallback node IP (`HOST_IP` / `/etc/hostinfo`) when both are unset — +egress automatically injects an always-allow egress rule for that host +(domain or IP, any port), so telemetry export works under the default deny-all +policy without manually managing allowlist rules. This also covers the egress +sidecar's own metric export, which shares the sandbox network namespace and +would otherwise be blocked by its own egress chain. - The rule follows the standard precedence: `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` - wins over `OTEL_EXPORTER_OTLP_ENDPOINT`. + wins over `OTEL_EXPORTER_OTLP_ENDPOINT`; the fallback node IP applies when + neither is set. - The host is taken from the endpoint URL (`https://host:4318/v1/metrics`), - `host:port`, or bare `host` forms. + `host:port`, or bare `host` forms; a trailing root dot on FQDNs is trimmed to + match DNS policy normalization. - The rule lives in the always-allow layer: it survives user `POST`/`PATCH`/`DELETE` policy updates and always-rule file reloads. Operators can still block the target with `deny.always`, which takes precedence. diff --git a/components/egress/telemetry_allow.go b/components/egress/telemetry_allow.go index c960ae8f6..5513fb3dc 100644 --- a/components/egress/telemetry_allow.go +++ b/components/egress/telemetry_allow.go @@ -20,14 +20,20 @@ import ( inttelemetry "github.com/alibaba/opensandbox/internal/telemetry" ) -// telemetryAllowRules returns an always-allow egress rule for the configured OTLP -// endpoint (OTEL_EXPORTER_OTLP_METRICS_ENDPOINT / OTEL_EXPORTER_OTLP_ENDPOINT), so -// metric export works under the default deny-all policy without operator-provided -// allowlist rules. The rule targets the endpoint host (any port), matching the -// egress rule model. Operators can still block the target via deny.always, which -// takes precedence. Returns nil when no endpoint is configured. +// telemetryAllowRules returns an always-allow egress rule for the OTLP +// destination the exporter will dial, so metric export works under the default +// deny-all policy without operator-provided allowlist rules. The destination +// is the configured endpoint +// (OTEL_EXPORTER_OTLP_METRICS_ENDPOINT / OTEL_EXPORTER_OTLP_ENDPOINT) or, when +// neither is set, the exporter fallback node IP (HOST_IP / /etc/hostinfo). The +// rule targets the host (any port), matching the egress rule model. Operators +// can still block the target via deny.always, which takes precedence. Returns +// nil when no OTLP destination is configured. func telemetryAllowRules() []policy.EgressRule { host, port, ok := inttelemetry.OTLPEndpointHostPort() + if !ok { + host, port, ok = inttelemetry.OTLPEndpointFallbackHostPort() + } if !ok { return nil } diff --git a/components/egress/telemetry_allow_test.go b/components/egress/telemetry_allow_test.go index cf6912beb..70ff43f51 100644 --- a/components/egress/telemetry_allow_test.go +++ b/components/egress/telemetry_allow_test.go @@ -53,6 +53,32 @@ func TestTelemetryAllowRulesFallbackEndpoint(t *testing.T) { require.Equal(t, "otel-collector", rules[0].Target) } +func TestTelemetryAllowRulesFallbackNodeIP(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "") + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") + t.Setenv("HOST_IP", "10.0.0.9") + rules := telemetryAllowRules() + require.Len(t, rules, 1) + require.Equal(t, "10.0.0.9", rules[0].Target) + + merged := policy.MergeAlwaysOverlay(policy.DefaultDenyPolicy(), nil, rules) + allowV4, allowV6, _, _ := merged.StaticIPSets() + require.Equal(t, []string{"10.0.0.9"}, allowV4, "fallback node IP must land in the static allow v4 set") + require.Empty(t, allowV6) +} + +func TestTelemetryAllowRulesFQDNTrailingDot(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "http://otel-collector.ns.svc.cluster.local.:4318") + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") + rules := telemetryAllowRules() + require.Len(t, rules, 1) + require.Equal(t, "otel-collector.ns.svc.cluster.local", rules[0].Target) + + merged := policy.MergeAlwaysOverlay(policy.DefaultDenyPolicy(), nil, rules) + require.Equal(t, policy.ActionAllow, merged.Evaluate("otel-collector.ns.svc.cluster.local."), "trailing-dot host must match DNS policy normalization") + require.Equal(t, policy.ActionDeny, merged.Evaluate("other.ns.svc.cluster.local.")) +} + func TestTelemetryAllowRulesIPEndpoint(t *testing.T) { t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "http://10.0.0.5:4317") t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") diff --git a/components/internal/telemetry/endpoint.go b/components/internal/telemetry/endpoint.go index dbdaf2f2e..6e7091bb6 100644 --- a/components/internal/telemetry/endpoint.go +++ b/components/internal/telemetry/endpoint.go @@ -24,8 +24,10 @@ import ( // endpoint. Endpoint precedence matches the exporters: // OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, then OTEL_EXPORTER_OTLP_ENDPOINT. // A missing port falls back to the scheme default (https->443, http->80); -// a bare host:port or host without a scheme is treated as https. ok is false -// when no endpoint is configured or it cannot be parsed. +// a bare host:port or host without a scheme is treated as https. Domain +// hosts are returned without the trailing dot, matching DNS policy +// normalization. ok is false when no endpoint is configured or it cannot +// be parsed. func OTLPEndpointHostPort() (host, port string, ok bool) { raw := otlpEndpointFromEnv() if raw == "" { @@ -34,6 +36,18 @@ func OTLPEndpointHostPort() (host, port string, ok bool) { return parseOTLPEndpoint(raw) } +// OTLPEndpointFallbackHostPort returns the exporter fallback destination used +// when no standard OTEL endpoint env var is set: the resolved node IP +// (HOST_IP, then /etc/hostinfo) on the default OTLP/HTTP port 4318. ok is +// false when no node IP can be resolved. +func OTLPEndpointFallbackHostPort() (host, port string, ok bool) { + ip, ok := resolveNodeIP() + if !ok { + return "", "", false + } + return ip, otlpHTTPPort, true +} + func parseOTLPEndpoint(raw string) (host, port string, ok bool) { raw = strings.TrimSpace(raw) if raw == "" { @@ -44,7 +58,7 @@ func parseOTLPEndpoint(raw string) (host, port string, ok bool) { if err != nil { return "", "", false } - host = strings.TrimSpace(u.Hostname()) + host = strings.TrimRight(strings.TrimSpace(u.Hostname()), ".") if host == "" { return "", "", false } @@ -56,12 +70,13 @@ func parseOTLPEndpoint(raw string) (host, port string, ok bool) { } if h, p, err := net.SplitHostPort(raw); err == nil { host, port = strings.TrimSpace(h), strings.TrimSpace(p) + host = strings.TrimRight(host, ".") if host == "" { return "", "", false } return host, port, true } - host = strings.TrimSpace(raw) + host = strings.TrimRight(strings.TrimSpace(raw), ".") if host == "" { return "", "", false } diff --git a/components/internal/telemetry/endpoint_test.go b/components/internal/telemetry/endpoint_test.go index 22aea9447..9aad5c779 100644 --- a/components/internal/telemetry/endpoint_test.go +++ b/components/internal/telemetry/endpoint_test.go @@ -35,6 +35,9 @@ func TestParseOTLPEndpoint(t *testing.T) { {name: "ip port", raw: "10.0.0.1:4318", host: "10.0.0.1", port: "4318", ok: true}, {name: "bare host", raw: "collector.example", host: "collector.example", port: "443", ok: true}, {name: "bare ip", raw: "10.0.0.1", host: "10.0.0.1", port: "443", ok: true}, + {name: "fqdn url trailing dot", raw: "http://otel-collector.ns.svc.cluster.local.:4318", host: "otel-collector.ns.svc.cluster.local", port: "4318", ok: true}, + {name: "fqdn trailing dot", raw: "otel-collector.ns.svc.cluster.local.:4318", host: "otel-collector.ns.svc.cluster.local", port: "4318", ok: true}, + {name: "bare fqdn trailing dot", raw: "collector.example.", host: "collector.example", port: "443", ok: true}, {name: "scheme only", raw: "http://", ok: false}, {name: "malformed url", raw: "https://:443", ok: false}, } @@ -80,3 +83,17 @@ func TestOTLPEndpointHostPortPrecedence(t *testing.T) { t.Fatalf("blank metrics endpoint should fall back; parsed as (%q, %q, %v)", host, port, ok) } } + +func TestOTLPEndpointFallbackHostPort(t *testing.T) { + t.Setenv(envHostIP, "10.0.0.9") + host, port, ok := OTLPEndpointFallbackHostPort() + if !ok || host != "10.0.0.9" || port != otlpHTTPPort { + t.Fatalf("fallback from HOST_IP parsed as (%q, %q, %v)", host, port, ok) + } + + t.Setenv(envHostIP, " ") + host, _, ok = OTLPEndpointFallbackHostPort() + if ok { + t.Fatalf("expected no fallback without a resolvable node IP, got %q", host) + } +} From e83886e3ea9a9405fb2199f9ec2de8a4442fd1ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=AB=98=E7=84=B6?= Date: Fri, 14 Aug 2026 11:13:20 +0800 Subject: [PATCH 4/5] fix(execd): reduce ptyViewerClientReadLoop cognitive complexity Refactor the nested switch/if message handling into small helpers so the function stays under the gocognit threshold (37 > 30). The execd CI lint (installing golangci-lint@latest) fails every PR on this pre-existing issue. --- components/execd/pkg/web/controller/pty_ws.go | 72 ++++++++++++------- 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/components/execd/pkg/web/controller/pty_ws.go b/components/execd/pkg/web/controller/pty_ws.go index d369884d6..b444ee309 100644 --- a/components/execd/pkg/web/controller/pty_ws.go +++ b/components/execd/pkg/web/controller/pty_ws.go @@ -500,38 +500,60 @@ func ptyViewerClientReadLoop( switch msgType { case websocket.BinaryMessage: - if len(data) > 0 && data[0] == model.BinStdin { - if !readOnlyError() { - return - } + if !ptyViewerHandleBinaryMessage(data, readOnlyError) { + return } case websocket.TextMessage: - var frame model.ClientFrame - if json.Unmarshal(data, &frame) != nil { - continue - } - switch frame.Type { - case "stdin", "signal", "resize": - if !readOnlyError() { - return - } - case "ping": - if err := writeJSON(model.ServerFrame{Type: "pong"}); err != nil { - cancelOnce() - } - default: - if err := writeJSON(model.ServerFrame{ - Type: "error", - Code: model.WSErrCodeInvalidFrame, - Error: fmt.Sprintf("unknown frame type %q", frame.Type), - }); err != nil { - cancelOnce() - } + if !ptyViewerHandleTextMessage(data, writeJSON, readOnlyError, cancelOnce) { + return } } } } +// ptyViewerHandleBinaryMessage reports stdin payloads on a read-only viewer; +// returns false when the read loop should exit. +func ptyViewerHandleBinaryMessage(data []byte, readOnlyError func() bool) bool { + if len(data) > 0 && data[0] == model.BinStdin { + return readOnlyError() + } + return true +} + +// ptyViewerHandleTextMessage handles client frames on a read-only viewer; +// returns false when the read loop should exit. +func ptyViewerHandleTextMessage(data []byte, writeJSON func(any) error, readOnlyError func() bool, cancelOnce func()) bool { + var frame model.ClientFrame + if json.Unmarshal(data, &frame) != nil { + return true + } + switch frame.Type { + case "stdin", "signal", "resize": + return readOnlyError() + case "ping": + ptyViewerReplyPong(writeJSON, cancelOnce) + default: + ptyViewerReplyInvalidFrame(writeJSON, cancelOnce, frame.Type) + } + return true +} + +func ptyViewerReplyPong(writeJSON func(any) error, cancelOnce func()) { + if err := writeJSON(model.ServerFrame{Type: "pong"}); err != nil { + cancelOnce() + } +} + +func ptyViewerReplyInvalidFrame(writeJSON func(any) error, cancelOnce func(), frameType string) { + if err := writeJSON(model.ServerFrame{ + Type: "error", + Code: model.WSErrCodeInvalidFrame, + Error: fmt.Sprintf("unknown frame type %q", frameType), + }); err != nil { + cancelOnce() + } +} + // ptyPingLoop sends periodic WebSocket pings until cancelCh is closed. func ptyPingLoop(conn *websocket.Conn, connMu *sync.Mutex, cancelCh <-chan struct{}, cancelOnce func()) { t := time.NewTicker(wsPingInterval) From 64c15604d101a5027b21cdfe5ba0aab7e72a6f04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=AB=98=E7=84=B6?= Date: Fri, 14 Aug 2026 13:31:35 +0800 Subject: [PATCH 5/5] fix(egress): require URL-form OTLP endpoint and gate node-IP fallback (#1491) The otlpmetrichttp exporter parses endpoint env vars with url.Parse and reads u.Host, so bare host:port values become opaque URLs with an empty host and are never dialed; only scheme://host URLs are valid. Restrict parseOTLPEndpoint to URL form to avoid injecting allow rules for hosts the exporter never connects to. Also skip the node-IP fallback when an endpoint env var is set but unparseable: metricsClientOptions never falls back once the env var is non-empty, so the rule would open unrelated node-IP egress. --- components/egress/docs/opentelemetry.md | 12 ++-- components/egress/telemetry_allow.go | 19 +++++-- components/egress/telemetry_allow_test.go | 16 +++++- components/internal/telemetry/endpoint.go | 56 +++++++++---------- .../internal/telemetry/endpoint_test.go | 33 ++++++++--- 5 files changed, 85 insertions(+), 51 deletions(-) diff --git a/components/egress/docs/opentelemetry.md b/components/egress/docs/opentelemetry.md index b2f660ef5..7f2d738ee 100644 --- a/components/egress/docs/opentelemetry.md +++ b/components/egress/docs/opentelemetry.md @@ -102,11 +102,13 @@ sidecar's own metric export, which shares the sandbox network namespace and would otherwise be blocked by its own egress chain. - The rule follows the standard precedence: `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` - wins over `OTEL_EXPORTER_OTLP_ENDPOINT`; the fallback node IP applies when - neither is set. -- The host is taken from the endpoint URL (`https://host:4318/v1/metrics`), - `host:port`, or bare `host` forms; a trailing root dot on FQDNs is trimmed to - match DNS policy normalization. + wins over `OTEL_EXPORTER_OTLP_ENDPOINT`; the fallback node IP applies only + when neither is set. A set-but-invalid endpoint never falls back (the + exporter does not either), so no rule is injected in that case. +- The endpoint must be a URL (`https://host:4318/v1/metrics`) — the + `otlpmetrichttp` env-var form. Bare `host:port` or `host` values are not + accepted (the exporter parses them as opaque URLs with an empty host); a + trailing root dot on FQDNs is trimmed to match DNS policy normalization. - The rule lives in the always-allow layer: it survives user `POST`/`PATCH`/`DELETE` policy updates and always-rule file reloads. Operators can still block the target with `deny.always`, which takes precedence. diff --git a/components/egress/telemetry_allow.go b/components/egress/telemetry_allow.go index 5513fb3dc..1c8f3b298 100644 --- a/components/egress/telemetry_allow.go +++ b/components/egress/telemetry_allow.go @@ -23,15 +23,22 @@ import ( // telemetryAllowRules returns an always-allow egress rule for the OTLP // destination the exporter will dial, so metric export works under the default // deny-all policy without operator-provided allowlist rules. The destination -// is the configured endpoint -// (OTEL_EXPORTER_OTLP_METRICS_ENDPOINT / OTEL_EXPORTER_OTLP_ENDPOINT) or, when -// neither is set, the exporter fallback node IP (HOST_IP / /etc/hostinfo). The -// rule targets the host (any port), matching the egress rule model. Operators -// can still block the target via deny.always, which takes precedence. Returns -// nil when no OTLP destination is configured. +// is the endpoint env var +// (OTEL_EXPORTER_OTLP_METRICS_ENDPOINT / OTEL_EXPORTER_OTLP_ENDPOINT, URL form +// as required by otlpmetrichttp) or, only when neither is set, the exporter +// fallback node IP (HOST_IP / /etc/hostinfo). A set-but-unparseable endpoint +// is not treated as unset: the exporter never falls back to the node IP in +// that case, so no rule is injected. The rule targets the host (any port), +// matching the egress rule model. Operators can still block the target via +// deny.always, which takes precedence. Returns nil when no OTLP destination +// is configured. func telemetryAllowRules() []policy.EgressRule { host, port, ok := inttelemetry.OTLPEndpointHostPort() if !ok { + if inttelemetry.OTLPEndpointEnvSet() { + log.Warnf("telemetry: configured OTLP endpoint is not a valid URL; skipping auto egress allow") + return nil + } host, port, ok = inttelemetry.OTLPEndpointFallbackHostPort() } if !ok { diff --git a/components/egress/telemetry_allow_test.go b/components/egress/telemetry_allow_test.go index 70ff43f51..cce684944 100644 --- a/components/egress/telemetry_allow_test.go +++ b/components/egress/telemetry_allow_test.go @@ -47,7 +47,7 @@ func TestTelemetryAllowRulesFromMetricsEndpoint(t *testing.T) { func TestTelemetryAllowRulesFallbackEndpoint(t *testing.T) { t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "") - t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "otel-collector:4318") + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "https://otel-collector:4318") rules := telemetryAllowRules() require.Len(t, rules, 1) require.Equal(t, "otel-collector", rules[0].Target) @@ -98,8 +98,20 @@ func TestTelemetryAllowRulesInvalidEndpoint(t *testing.T) { require.Nil(t, telemetryAllowRules()) } +func TestTelemetryAllowRulesInvalidEndpointSkipsNodeIPFallback(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "http://") + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") + t.Setenv("HOST_IP", "10.0.0.9") + require.Nil(t, telemetryAllowRules(), "configured-but-invalid endpoint must not open node-IP egress") + + t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "") + rules := telemetryAllowRules() + require.Len(t, rules, 1, "unset endpoint should fall back to the node IP") + require.Equal(t, "10.0.0.9", rules[0].Target) +} + func TestWithTelemetryAllowAppends(t *testing.T) { - t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "collector.example:4318") + t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "https://collector.example:4318") t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") existingRule, err := policy.ParseValidatedEgressRule(policy.ActionAllow, "a.example.com") require.NoError(t, err) diff --git a/components/internal/telemetry/endpoint.go b/components/internal/telemetry/endpoint.go index 6e7091bb6..00ae587c9 100644 --- a/components/internal/telemetry/endpoint.go +++ b/components/internal/telemetry/endpoint.go @@ -15,7 +15,6 @@ package telemetry import ( - "net" "net/url" "strings" ) @@ -23,9 +22,11 @@ import ( // OTLPEndpointHostPort returns the host and port of the configured OTLP // endpoint. Endpoint precedence matches the exporters: // OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, then OTEL_EXPORTER_OTLP_ENDPOINT. -// A missing port falls back to the scheme default (https->443, http->80); -// a bare host:port or host without a scheme is treated as https. Domain -// hosts are returned without the trailing dot, matching DNS policy +// The value must be a URL (scheme://host[:port][/path]), matching the +// otlpmetrichttp env-var form; bare host:port or host values are invalid +// because the exporter parses them as opaque URLs with an empty host. +// A missing port falls back to the scheme default (https->443, http->80). +// Domain hosts are returned without the trailing dot, matching DNS policy // normalization. ok is false when no endpoint is configured or it cannot // be parsed. func OTLPEndpointHostPort() (host, port string, ok bool) { @@ -36,8 +37,17 @@ func OTLPEndpointHostPort() (host, port string, ok bool) { return parseOTLPEndpoint(raw) } +// OTLPEndpointEnvSet reports whether any OTEL endpoint env var is non-blank, +// regardless of whether it parses. Callers that also use +// OTLPEndpointFallbackHostPort need this to distinguish "unset" from +// "configured but invalid": the exporter never falls back to the node IP once +// an endpoint env var is set, so neither should the auto-allow logic. +func OTLPEndpointEnvSet() bool { + return otlpEndpointFromEnv() != "" +} + // OTLPEndpointFallbackHostPort returns the exporter fallback destination used -// when no standard OTEL endpoint env var is set: the resolved node IP +// only when no OTEL endpoint env var is set: the resolved node IP // (HOST_IP, then /etc/hostinfo) on the default OTLP/HTTP port 4318. ok is // false when no node IP can be resolved. func OTLPEndpointFallbackHostPort() (host, port string, ok bool) { @@ -50,38 +60,22 @@ func OTLPEndpointFallbackHostPort() (host, port string, ok bool) { func parseOTLPEndpoint(raw string) (host, port string, ok bool) { raw = strings.TrimSpace(raw) - if raw == "" { + if !strings.Contains(raw, "://") { return "", "", false } - if strings.Contains(raw, "://") { - u, err := url.Parse(raw) - if err != nil { - return "", "", false - } - host = strings.TrimRight(strings.TrimSpace(u.Hostname()), ".") - if host == "" { - return "", "", false - } - port = u.Port() - if port == "" { - port = defaultPortForScheme(u.Scheme) - } - return host, port, true - } - if h, p, err := net.SplitHostPort(raw); err == nil { - host, port = strings.TrimSpace(h), strings.TrimSpace(p) - host = strings.TrimRight(host, ".") - if host == "" { - return "", "", false - } - return host, port, true + u, err := url.Parse(raw) + if err != nil { + return "", "", false } - host = strings.TrimRight(strings.TrimSpace(raw), ".") + host = strings.TrimRight(strings.TrimSpace(u.Hostname()), ".") if host == "" { return "", "", false } - // No scheme: per OTLP spec the https scheme (port 443) is assumed. - return host, "443", true + port = u.Port() + if port == "" { + port = defaultPortForScheme(u.Scheme) + } + return host, port, true } func defaultPortForScheme(scheme string) string { diff --git a/components/internal/telemetry/endpoint_test.go b/components/internal/telemetry/endpoint_test.go index 9aad5c779..d4ebbfe73 100644 --- a/components/internal/telemetry/endpoint_test.go +++ b/components/internal/telemetry/endpoint_test.go @@ -31,13 +31,13 @@ func TestParseOTLPEndpoint(t *testing.T) { {name: "http url without port", raw: "http://collector.example/v1/metrics", host: "collector.example", port: "80", ok: true}, {name: "ip url", raw: "http://10.0.0.1:4317", host: "10.0.0.1", port: "4317", ok: true}, {name: "ipv6 url", raw: "http://[::1]:4318/v1/metrics", host: "::1", port: "4318", ok: true}, - {name: "host port", raw: "collector.example:4318", host: "collector.example", port: "4318", ok: true}, - {name: "ip port", raw: "10.0.0.1:4318", host: "10.0.0.1", port: "4318", ok: true}, - {name: "bare host", raw: "collector.example", host: "collector.example", port: "443", ok: true}, - {name: "bare ip", raw: "10.0.0.1", host: "10.0.0.1", port: "443", ok: true}, + {name: "host port without scheme", raw: "collector.example:4318", ok: false}, + {name: "ip port without scheme", raw: "10.0.0.1:4318", ok: false}, + {name: "bare host", raw: "collector.example", ok: false}, + {name: "bare ip", raw: "10.0.0.1", ok: false}, {name: "fqdn url trailing dot", raw: "http://otel-collector.ns.svc.cluster.local.:4318", host: "otel-collector.ns.svc.cluster.local", port: "4318", ok: true}, - {name: "fqdn trailing dot", raw: "otel-collector.ns.svc.cluster.local.:4318", host: "otel-collector.ns.svc.cluster.local", port: "4318", ok: true}, - {name: "bare fqdn trailing dot", raw: "collector.example.", host: "collector.example", port: "443", ok: true}, + {name: "fqdn trailing dot without scheme", raw: "otel-collector.ns.svc.cluster.local.:4318", ok: false}, + {name: "bare fqdn trailing dot", raw: "collector.example.", ok: false}, {name: "scheme only", raw: "http://", ok: false}, {name: "malformed url", raw: "https://:443", ok: false}, } @@ -65,7 +65,7 @@ func TestOTLPEndpointHostPortPrecedence(t *testing.T) { t.Fatalf("expected empty host, got %q", host) } - t.Setenv(envOTLPEndpoint, "fallback.example:4318") + t.Setenv(envOTLPEndpoint, "https://fallback.example:4318") host, port, ok := OTLPEndpointHostPort() if !ok || host != "fallback.example" || port != "4318" { t.Fatalf("fallback endpoint parsed as (%q, %q, %v)", host, port, ok) @@ -84,6 +84,25 @@ func TestOTLPEndpointHostPortPrecedence(t *testing.T) { } } +func TestOTLPEndpointEnvSet(t *testing.T) { + t.Setenv(envOTLPMetricsEndpoint, "") + t.Setenv(envOTLPEndpoint, "") + if OTLPEndpointEnvSet() { + t.Fatal("expected env unset") + } + + t.Setenv(envOTLPEndpoint, "http://") + if !OTLPEndpointEnvSet() { + t.Fatal("expected env set even when unparseable") + } + + t.Setenv(envOTLPEndpoint, "") + t.Setenv(envOTLPMetricsEndpoint, "https://collector.example:4318") + if !OTLPEndpointEnvSet() { + t.Fatal("expected metrics endpoint env set") + } +} + func TestOTLPEndpointFallbackHostPort(t *testing.T) { t.Setenv(envHostIP, "10.0.0.9") host, port, ok := OTLPEndpointFallbackHostPort()