Skip to content

Add support to reconcile TCPRoute#410

Open
prabhjotbawa wants to merge 4 commits into
firestoned:mainfrom
prabhjotbawa:add-tcproute-scout-rs
Open

Add support to reconcile TCPRoute#410
prabhjotbawa wants to merge 4 commits into
firestoned:mainfrom
prabhjotbawa:add-tcproute-scout-rs

Conversation

@prabhjotbawa

@prabhjotbawa prabhjotbawa commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Add support for Scout to reconcile TCPRoute and create Arecord

@dgunzy dgunzy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Will defer to @ebourgeois to approve

bradpenney
bradpenney previously approved these changes Jul 9, 2026

@bradpenney bradpenney 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.

Approved, but please wait for @ebourgeois

@ebourgeois

Copy link
Copy Markdown
Contributor
  1. TCPRoute has no spec.hostnames in the Gateway API (design/correctness)

TCPRouteSpec here declares a hostnames: Option<Vec> field and the whole flow keys off spec.hostnames[]. The real Gateway API TCPRoute (and UDPRoute) have no hostnames field — they're pure L4, no SNI/Host. Only HTTPRoute, GRPCRoute, and TLSRoute carry hostnames.

Consequences for any real TCPRoute (where spec.hostnames is always absent):

  • effective_tcproute_hostnames returns [""] only if a record-name annotation is set, otherwise [] → the loop never runs and nothing is created, silently. A user who sets scout-enabled + zone + ip but forgets record-name gets a no-op with no error.
  • The docs and CHANGELOG claim "creates one ARecord per hostname in spec.hostnames[]," and scout.md says TCPRoute is treated "identically" to HTTP/TLS. That's misleading — for TCPRoute the record name can only come from the record-name annotation.
  1. Docs ClusterRole block still missing tcproutes (RBAC bug)

docs/src/guide/scout.md has two copy-pasteable gateway RBAC blocks. The PR updated the comment on the first one (~line 480) to say "HTTPRoutes/TLSRoutes/TCPRoutes" but left the actual list unchanged:

resources: ["httproutes", "tlsroutes", "gateways"] # ← no tcproutes

The second block (~line 689) was fixed. A user following the first block won't grant TCPRoute watch permission, and Scout's new controller will fail list/watch with RBAC-forbidden errors on a loop.

  1. deploy/scout.yaml inline RBAC not updated (sync gap)

Per this repo's own CLAUDE.md CRITICAL rule, RBAC changes must sync across bootstrap.rs, deploy/scout/clusterrole.yaml, and deploy/scout.yaml's inline ClusterRole. The PR updated the first two but deploy/scout.yaml:54 still reads ["httproutes", "tlsroutes", "gateways"]. Anyone deploying via the bundled manifest gets no TCPRoute permission.

  1. Add docs

Minor

  • examples/README.md uses 1a. numbering, which renders oddly in ordered-list markdown.
  • TCPRouteSpec.rules: Option<Vec<serde_json::Value>> is declared but never read — harmless, but dead.
  • reconcile_tcproute (the ~230-line core) has no direct test; only the pure helpers are covered. Consistent with the HTTP/TLS reconcilers, so acceptable, but the interesting logic (silent no-op path, gateway-IP fallback) is untested.

Signed-off-by: Prabhjot Singh Bawa <prabhjotbawa@gmail.com>
Signed-off-by: Prabhjot Singh Bawa <prabhjotbawa@gmail.com>
Signed-off-by: Prabhjot Singh Bawa <prabhjotbawa@gmail.com>
@prabhjotbawa prabhjotbawa force-pushed the add-tcproute-scout-rs branch from e217ec5 to a180c7b Compare July 10, 2026 17:01
Signed-off-by: Prabhjot Singh Bawa <prabhjotbawa@gmail.com>
@prabhjotbawa

Copy link
Copy Markdown
Contributor Author

@ebourgeois I have made all the changes, I don't seem to able to find the file deploy/scout.yaml. Does this exist?

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.

4 participants