-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(egress): auto-allow OTLP endpoint egress traffic #1504
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.
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
424d521
e2da97a
1e63b77
e83886e
64c1560
e8c51c2
0122d64
a42cde2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,6 +78,7 @@ func main() { | |
| if err != nil { | ||
| log.Fatalf("failed to load always allow/deny rule files: %v", err) | ||
| } | ||
| alwaysAllow = withTelemetryAllow(alwaysAllow) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This changes externally visible default-deny behavior by adding an implicit always-allow rule, but the published source-of-truth page AGENTS.md reference: AGENTS.md:L69-L79 Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the endpoint parses successfully but Useful? React with 👍 / 👎. |
||
|
|
||
| allowIPs := allowIps() | ||
| mode := parseMode() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -713,6 +713,8 @@ func (s *policyServer) reloadAlwaysRules() (bool, error) { | |
| if !changed { | ||
| return false, nil | ||
| } | ||
| allow = withTelemetryAllow(allow) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When only Useful? React with 👍 / 👎. |
||
| s.setAlwaysRules(deny, allow) | ||
| s.proxy.UpdateAlwaysRules(deny, allow) | ||
| return true, nil | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| // 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 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 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() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| 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() | ||
|
Pangjiping marked this conversation as resolved.
|
||
| } | ||
| if !ok { | ||
| return nil | ||
| } | ||
| rule, err := policy.ParseValidatedEgressRule(policy.ActionAllow, host) | ||
|
Pangjiping marked this conversation as resolved.
hittyt marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For an endpoint such as Useful? React with 👍 / 👎. |
||
| 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...) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| // 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", "https://otel-collector:4318") | ||
| rules := telemetryAllowRules() | ||
| require.Len(t, rules, 1) | ||
| 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", "") | ||
| 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 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", "https://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.")) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 { | ||
|
Comment on lines
+516
to
+518
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This extraction changes AGENTS.md reference: AGENTS.md:L60-L63 Useful? React with 👍 / 👎. |
||
| 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) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This newly documents that
HOST_IPor/etc/hostinfoenables the exporter fallback when both OTEL endpoint variables are unset, but the immediately preceding configuration section still states that export is disabled in exactly that situation. SincemetricsEnabled(false)does use the node-IP fallback for egress, operators may incorrectly assume metrics remain local; update the earlier description to match the behavior documented here.AGENTS.md reference: AGENTS.md:L44-L44
Useful? React with 👍 / 👎.