Skip to content

fix: isolate invalid origin transport and enforce discovery boundaries - #96

Merged
mannie-exe merged 6 commits into
mainfrom
dev
Oct 3, 2026
Merged

mannie-exe merged 6 commits into
mainfrom
dev

Conversation

@mannie-exe

@mannie-exe mannie-exe commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Changes

An empty namespace label selector incorrectly included namespaces without that label. HTTPRoute backend validation also accepted UDP/SCTP Service ports. This change requires label presence while preserving the union with named namespaces, and requires TCP for HTTP origins.

Effective origin validation now resolves tunnel defaults and route overrides before aggregation. Conflicting HTTP/2+h2c and HTTPS+h2c routes retain scoped HTTP 503 responses while valid siblings and backend revocations continue to publish. Matching Access-required denials keep precedence over public routes. Status, DNS discovery, Access dependency inventory, forwarding fallbacks, and final client validation share these constraints; tunnel changes wake route status reconciliation.

No CRD or dependency changes. The pinned connector's origin validation was inspected, including Unix TLS and both hello-world aliases. Documentation describes changed behavior and distinguishes stored configuration from actual connector application.

Validation

  • Reproduced missing-label DNS writes, UDP/SCTP forwarding, inherited transport aggregate rejection, and HTTPS+h2c publication with repository tests before the fixes.
  • All 12 unit packages passed with race detection. Focused final controller/client race tests, lint, build, formatting and code generation passed.
  • Regressions verify sibling publication, grant withdrawal, matching-denial precedence, transport aliases, explicit overrides, and backend/namespace negative cases.
  • Full local kind/Colima E2E at 7eebd50: 123 passed, 0 failed, 0 skipped, including teardown (24m40s). The live TLS test confirms healthy sibling responses, invalid transport HTTP503, and withdrawn backend HTTP500 through the connector, alongside stored configuration checks. An earlier deliberately interrupted attempt was normally finalized under a temporary manager, then its owned cluster was removed.

The user confirmed release order: cfgate alpha.11 after green review and integration checks, then chart 1.10.0. Chart preparation is tracked in cfgate/helm-chart#26.

Initial Greptile finding: CA-pool validation could skip a route before rendering its denial. Fixed by separating match validation from origin validation and preserving scoped denials for invalid origin annotations; regressions cover CA paths, malformed values and overlapping public forwarding. Final focused controller race tests passed.

- dns: require namespace label presence
- dns: retain explicit namespace union
- dns: exclude invalid origin settings
- routing: require TCP backend ports
- routing: preserve TCP default behavior
- routing: resolve inherited transports
- routing: keep scoped failure matches
- routing: publish sibling revocations
- access: preserve denial precedence
- access: exclude unusable dependencies
- status: report effective parent errors
- status: watch tunnel setting changes
- client: reject incompatible schemes
- client: check connector service aliases
- tests: cover writes and wire validation
- tests: exercise grant withdrawal
- e2e: retain a healthy sibling origin
- e2e: reject inherited conflicts
- e2e: reject HTTPS with h2c enabled
- e2e: withdraw backend grants in place
- e2e: inspect remote forwarding rules
- e2e: verify connector error responses
- e2e: keep live certificate coverage
- docs: explain scoped transport errors
- docs: clarify supported backend ports
- docs: distinguish stored and applied
- docs: require namespace label presence
- docs: preserve named namespace union
- docs: describe discovery restrictions
- docs: record next upgrade behavior
- docs: retain existing cleanup policies
- e2e: keep the HTTP/1 TLS origin usable
- e2e: disable its inherited HTTP/2
- e2e: isolate sibling transport errors
- connector: normalize HTTP URL spelling
- connector: cover WebSocket origins
- connector: cover hello-world aliases
- tests: retain matching error responses
@greptile-apps

greptile-apps Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[High risk] Adds validation of origin transport settings across route discovery and tunnel configuration.

The PR appears safe to merge based on the reviewed changes.

Summary

This PR tightens DNS namespace-label selection and HTTPRoute backend protocol checks, validates effective origin transport across controllers and the client, and retains matching denial responses for invalid routes without blocking valid siblings.

  • Route status, DNS discovery, Access inventory, and tunnel publication now account for effective transport settings.
  • Tests and documentation cover transport overrides, denial precedence, backend revocation, and connector behavior.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Route[HTTPRoute] --> Admission[Parent and match admission]
  Tunnel[CloudflareTunnel defaults] --> Transport[Effective transport validation]
  Admission --> Transport
  Transport -->|valid| Backend[Backend validation and forwarding]
  Transport -->|invalid| Denial[Matching HTTP 503]
  Backend --> Ingress[Tunnel ingress configuration]
  Denial --> Ingress
Loading

Reviews (2) · Last reviewed commit: "fix: preserve route denials before origi..."

- routing: retain invalid CA matches
- routing: isolate annotation failures
- routing: separate match validation
- access: keep protected denial ordering
- tests: reject public fallthrough
- tests: cover malformed origin values
- docs: identify the alpha.11 upgrade
@codecov

codecov Bot commented Oct 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.40506% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/controller/access_required.go 42.85% 4 Missing ⚠️
internal/controller/httproute_controller.go 66.66% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@mannie-exe
mannie-exe merged commit 7c1b8f2 into main Oct 3, 2026
9 checks passed
@mannie-exe
mannie-exe deleted the dev branch October 3, 2026 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant