Skip to content

feat(egress): auto-allow OTLP endpoint egress traffic - #1504

Open
Pangjiping wants to merge 5 commits into
opensandbox-group:mainfrom
Pangjiping:feat/egress-auto-allow-otlp-1491
Open

feat(egress): auto-allow OTLP endpoint egress traffic#1504
Pangjiping wants to merge 5 commits into
opensandbox-group:mainfrom
Pangjiping:feat/egress-auto-allow-otlp-1491

Conversation

@Pangjiping

Copy link
Copy Markdown
Collaborator

Closes #1491

Why

The egress component's default policy denies all outbound traffic, so users
had to manually inject allowlist rules for the OTLP endpoint to export
metrics. Since the egress sidecar shares the sandbox network namespace, its
own metric export is blocked by its own egress chain too.

What

  • Parse OTEL_EXPORTER_OTLP_METRICS_ENDPOINT (preferred) then
    OTEL_EXPORTER_OTLP_ENDPOINT at egress startup
    (components/internal/telemetry/endpoint.go), supporting URL,
    host:port, and bare-host forms with scheme-default ports.
  • Inject an always-allow egress rule for the endpoint host (domain rules are
    resolved via the DNS proxy, IP rules land in the static nft allow sets)
    (components/egress/telemetry_allow.go).
  • The rule is appended to the always-allow layer: it survives user
    POST/PATCH/DELETE policy updates and always-rule file reloads; operators
    can still override via deny.always, which takes precedence.
  • Rules are host-scoped (any port) matching the current egress rule model;
    per-rule port enforcement is not added.
  • Docs: components/egress/docs/opentelemetry.md.

Tests

  • components/internal/telemetry/endpoint_test.go: endpoint parsing
    (URL/host:port/bare host, scheme defaults, precedence, invalid input).
  • components/egress/telemetry_allow_test.go: rule generation, static IP
    sets, merge/evaluate behavior, and pass-through when unconfigured.
  • go test -mod=mod ./... in components/egress and components/internal all
    green (note: egress vendor/ is stale vs go.mod — pre-existing, unrelated).

…oup#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.
@github-actions github-actions Bot added component/egress component/ingress size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 13, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

export OTEL_EXPORTER_OTLP_METRICS_ENDPOINT="http://otel-collector:4318"

P2 Badge Use a Kubernetes-resolvable OTLP service name

In Kubernetes pods with the usual search domains, this single-label endpoint causes the resolver to query names such as otel-collector.<ns>.svc.cluster.local, while the auto-generated allow rule is only the exact host otel-collector; the DNS proxy returns NXDOMAIN for the search-expanded service name under default deny, so the documented example still cannot reach the collector. Use the full service DNS name in the example or teach the auto-allow logic to account for search-expanded service names.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread components/internal/telemetry/endpoint.go
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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2da97a8ca

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread components/egress/telemetry_allow.go
…nsandbox-group#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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e63b77cf0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread components/egress/telemetry_allow.go
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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e83886e3ea

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread components/internal/telemetry/endpoint.go Outdated
…opensandbox-group#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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 64c15604d1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread components/internal/telemetry/endpoint.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/egress component/execd component/ingress size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Automatically allow OpenTelemetry metric export traffic in opensandbox egress component

1 participant