Skip to content

Refactor/egress mitmproxy yaml config#975

Open
Pangjiping wants to merge 3 commits into
opensandbox-group:mainfrom
Pangjiping:refactor/egress-mitmproxy-yaml-config
Open

Refactor/egress mitmproxy yaml config#975
Pangjiping wants to merge 3 commits into
opensandbox-group:mainfrom
Pangjiping:refactor/egress-mitmproxy-yaml-config

Conversation

@Pangjiping
Copy link
Copy Markdown
Collaborator

@Pangjiping Pangjiping commented Jun 4, 2026

Summary

Move static mitmproxy options out of launch.go hardcodes into a baked-in config.yaml under the standard mitm confdir layout. launch.go keeps only per-deployment dynamic flags
(env-driven --set).

config.yaml lists only deviations from mitm built-in defaults:

mode:
  - transparent
listen_host: 127.0.0.1
stream_large_bodies: 10m
ssl_verify_upstream_trusted_confdir: /etc/ssl/certs
ignore_hosts: []   # default-matching, kept as discoverable extension point

Precedence: --set (env override) > config.yaml > mitm defaults.

Why

Two latent bugs in the launch.go surface:

  • stream_large_bodies set twice. launch.go wrote 1m, custom.py overrode to 10m via ctx.options. launch.go line was dead.
  • ignore_hosts multi-value silently overwritten. Each ;-separated entry was a separate --set ignore_hosts=..., and mitm --set on a list option REPLACES the list — only the
    last value survived.

config.yaml natively expresses lists, eliminates the double-set, and gives operators a single reviewable file for fleet-wide static defaults.

Removed env vars

  • OPENSANDBOX_EGRESS_MITMPROXY_CONFDIR — no internal use; would have broken config.yaml discovery.
  • OPENSANDBOX_EGRESS_MITMPROXY_IGNORE_HOSTS — replaced by yaml-native list (the env path was the source of the silent-overwrite bug).

Override paths (documented)

  1. Downstream image COPY over the baked-in path (recommended).
  2. K8s ConfigMap subPath mount at runtime.
  3. Single-option env --set for the documented dynamic env vars.

Backward compat

/egress symlink → /opt/opensandbox-egress/egress for tooling that still references the pre-#951 path.

Testing

  • Not run (explain why)
  • Unit tests
  • Integration tests
  • e2e / manual verification

Breaking Changes

  • None
  • Yes (describe impact and migration path)

Checklist

  • Linked Issue or clearly described motivation
  • Added/updated docs (if needed)
  • Added/updated tests (if needed)
  • Security impact considered
  • Backward compatibility considered

@Pangjiping Pangjiping force-pushed the refactor/egress-mitmproxy-yaml-config branch from 022c537 to 92581d3 Compare June 4, 2026 04:05
…ynamic)

Move fleet-wide, rarely-changing mitmproxy options into a baked-in
config.yaml under the standard mitm confdir layout, so launch.go only
emits per-deployment dynamic overrides via --set. This eliminates two
classes of bug along the way:

- stream_large_bodies was set in two places (launch.go --set 1m and
  custom.py ctx.options 10m), with the addon silently winning — making
  the launch.go line dead code. Now declared once in config.yaml (10m).
- ignore_hosts was env-driven with `;`-separated values, but each value
  was passed as a separate --set, and mitmproxy --set on a list option
  REPLACES the list — so configuring multiple bypass patterns silently
  only kept the last one. config.yaml uses a native YAML list with no
  override semantics.

Static options now in /var/lib/mitmproxy/.mitmproxy/config.yaml:
  mode, listen_host, connection_strategy (lazy — historical default
  preserved here; switching to eager is tracked in a separate change),
  stream_large_bodies (10m), http2, ignore_hosts (empty default),
  ssl_verify_upstream_trusted_confdir (default).

Dynamic overrides remain env-driven and applied as --set in launch.go
(precedence: --set > config.yaml > mitm defaults):
  OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT  (toggle)
  OPENSANDBOX_EGRESS_MITMPROXY_PORT
  OPENSANDBOX_EGRESS_MITMPROXY_SCRIPT
  OPENSANDBOX_EGRESS_MITMPROXY_SSL_INSECURE
  OPENSANDBOX_EGRESS_MITMPROXY_UPSTREAM_TRUST_DIR

Removed env vars (no internal use, replaced by config.yaml):
  OPENSANDBOX_EGRESS_MITMPROXY_CONFDIR  — confdir is the mitm user's
    home (/var/lib/mitmproxy), which is also where config.yaml lives;
    splitting them via env created an unused escape hatch that would
    have broken config.yaml discovery.
  OPENSANDBOX_EGRESS_MITMPROXY_IGNORE_HOSTS  — replaced by ignore_hosts
    in config.yaml (native list, no covert-overwrite bug).

The mitmproxy.Config struct loses its ConfDir field accordingly.
SyncRootCA still accepts an optional confDirEnv argument so the existing
candidate-path search behavior is preserved if a future caller needs to
plumb it back in.
…ConfigMap mount)

The previous draft told operators to edit components/egress/mitmproxy/config.yaml
and rebuild — true for the in-repo flow, but does not help operators consuming
a published egress image who want different static defaults. Add a section
spelling out the three supported override paths:

1. Build a downstream image that COPYs an alternate config.yaml over the
   baked-in path (recommended: version-controlled, reproducible).
2. Mount an override at /var/lib/mitmproxy/.mitmproxy/config.yaml at runtime
   (Kubernetes ConfigMap subPath mount example included).
3. Use the env-driven --set escape hatch for the small set of options exposed
   via environment variables.

Also warn against in-container edits, which are lost on restart and blocked
by the mitmproxy user's read-only access.
…ibility

PR opensandbox-group#951 moved the egress binary from /egress to /opt/opensandbox-egress/egress
so the supervisor and binary could share a single grouped directory. External
tooling and older deployment manifests may still reference the old /egress
path; add a symlink so both paths resolve to the same binary.

Symlink rather than COPY: zero extra image size, single source of truth for
chmod and replacement, and `exec /egress` resolves to the supervisor-managed
binary like before.
@Pangjiping Pangjiping force-pushed the refactor/egress-mitmproxy-yaml-config branch from 6b6824c to a90b629 Compare June 4, 2026 07:43
@Pangjiping Pangjiping marked this pull request as ready for review June 4, 2026 08:10
@Pangjiping Pangjiping requested review from hittyt and jwx0925 as code owners June 4, 2026 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants