Skip to content

Translate AllowPort into Envoy egress RBAC policy - #5924

Closed
ChrisJBurns wants to merge 2 commits into
cburns/envoy-e2e-enablefrom
cburns/envoy-allow-port
Closed

ChrisJBurns wants to merge 2 commits into
cburns/envoy-e2e-enablefrom
cburns/envoy-allow-port

Conversation

@ChrisJBurns

@ChrisJBurns ChrisJBurns commented Jul 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Closes the AllowPort parity gap between the Envoy and Squid backends, and adds an e2e test that proves enforcement through a real Envoy container.

Closes #5915. Part of #5900.

The problem

With AllowHost: ["example.com"], AllowPort: [443], the intent is that only HTTPS traffic to example.com is permitted. Squid enforced this. Envoy ignored AllowPort entirely — plain HTTP to example.com (port 80) also got through.

The fix

AllowPort entries are translated into :authority suffix matchers (":443", ":80", …) in the RBAC ALLOW filter. In Envoy's RBAC, a policy fires when all its permissions match, so adding a port permission to the same policy as the host regex gives AND semantics — matching Squid's allowed_ports AND allowed_dsts ACL combination.

Profile Before After
AllowHost + AllowPort host only enforced (port ignored) host AND port both required
AllowPort only effectively allow-all any host on listed ports
AllowHost only any port any port (unchanged)
InsecureAllowAll everything everything (ignores port list, same as Squid)
Tests
  • TestBuildAllowlistPolicies_AllowPort — 4 unit table cases covering all combinations (config-shape level)
  • NetworkIsolationEnvoy / AllowPort enforcement — e2e test against a real Envoy container: allows https://example.com (port 443), blocks http://example.com (port 80)

Type of change

  • Bug fix / parity fix

Test plan

  • go build ./pkg/container/docker/... passes
  • golangci-lint clean
  • TestBuildAllowlistPolicies_AllowPort passes
  • go test -c ./test/e2e/ compiles
  • CI e2e proxy shard — AllowPort enforcement test runs against real Envoy

Generated with Claude Code

@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed size/S Small PR: 100-299 lines changed and removed size/M Medium PR: 300-599 lines changed labels Jul 22, 2026
@ChrisJBurns
ChrisJBurns force-pushed the cburns/envoy-allow-port branch from bb6ee6c to a9e65d9 Compare July 22, 2026 20:47
@ChrisJBurns
ChrisJBurns force-pushed the cburns/envoy-e2e-enable branch from fe7f377 to ecae5e0 Compare July 22, 2026 20:48
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/S Small PR: 100-299 lines changed size/M Medium PR: 300-599 lines changed labels Jul 22, 2026
@ChrisJBurns
ChrisJBurns force-pushed the cburns/envoy-allow-port branch from a9e65d9 to 234c3fc Compare July 22, 2026 20:57
ChrisJBurns and others added 2 commits July 22, 2026 21:57
Squid enforces AllowPort via the allowed_ports ACL, AND-d with AllowHost.
The Envoy backend previously ignored AllowPort, leaving allowlisted hosts
reachable on any port — a parity regression.

Translate AllowPort into :authority suffix matchers (":80", ":443", …) in
the RBAC ALLOW filter, mirroring Squid's AND semantics:
  - AllowHost + AllowPort  → host AND port must match in the same policy
  - AllowPort only         → any host on listed ports
  - AllowHost only         → any port (unchanged)
  - InsecureAllowAll       → ignores AllowPort (same as Squid)

Uses :authority suffix matching so plain HTTP ("host:port") and HTTPS
CONNECT ("host:port") are both covered. Remove the Known Limitation entry
and add a parity row to the comparison table.

Closes #5915.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Exercises the AllowPort fix end-to-end: starts an Envoy isolated server
with AllowHost:[example.com] + AllowPort:[443], then asserts that
https://example.com succeeds and http://example.com is blocked. This
closes the gap between unit tests (which verify the config shape) and
runtime behaviour through a real Envoy container.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ChrisJBurns
ChrisJBurns force-pushed the cburns/envoy-allow-port branch from 234c3fc to be9bc48 Compare July 22, 2026 20:57
@ChrisJBurns

Copy link
Copy Markdown
Collaborator Author

Recreating stacked on renamed branch

@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/M Medium PR: 300-599 lines changed size/S Small PR: 100-299 lines changed labels Jul 22, 2026
@codecov

codecov Bot commented Jul 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.70%. Comparing base (3f79384) to head (be9bc48).

Additional details and impacted files
@@                     Coverage Diff                     @@
##           cburns/envoy-e2e-enable    #5924      +/-   ##
===========================================================
+ Coverage                    71.68%   71.70%   +0.01%     
===========================================================
  Files                          700      700              
  Lines                        71918    71940      +22     
===========================================================
+ Hits                         51557    51582      +25     
- Misses                       16668    16669       +1     
+ Partials                      3693     3689       -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant