Skip to content

feat(control-plane): auto-reconcile router NetworkPolicy and NLB passthrough Route - #427

Open
markturansky wants to merge 1 commit into
mainfrom
feat/gateway-router-netpol-route
Open

feat(control-plane): auto-reconcile router NetworkPolicy and NLB passthrough Route#427
markturansky wants to merge 1 commit into
mainfrom
feat/gateway-router-netpol-route

Conversation

@markturansky

Copy link
Copy Markdown
Collaborator

Summary

  • Adds reconcileRouterNetworkPolicy() to automatically create openshell-gateway-allow-router NetworkPolicy on OpenShift when a gateway has Route config, allowing HAProxy router pods from openshift-ingress to reach gateway on ports 8080/8081
  • Adds reconcileOpenShiftRoute() to automatically create openshell-gateway-grpc passthrough Route with router: grpc label for NLB-backed ingress (alternative to Gateway API GRPCRoute)
  • Adds route.openshift.io/routes RBAC to ambient-control-plane ClusterRole
  • Both resources are cleaned up (deleted) when Route config is removed from the gateway
  • Updates RECONCILE.md: P44 and P45 marked done, coverage 92.8% → 93.4%

Test plan

  • Deploy to OpenShift cluster with NLB IngressController configured
  • Create gateway with Route config → verify NetworkPolicy and Route are created
  • Verify gRPC connectivity through NLB passthrough route
  • Remove Route config from gateway → verify NetworkPolicy and Route are deleted
  • Verify go build ./..., go vet ./..., and gofmt pass (verified locally)
  • Verify on non-OpenShift cluster: both reconcilers are skipped (no errors)

🤖 Generated with Claude Code

…through Route

When a gateway has a Route configured on OpenShift clusters, the reconciler
now automatically creates an `openshell-gateway-allow-router` NetworkPolicy
(allowing HAProxy router pods from openshift-ingress to reach the gateway on
ports 8080/8081) and an `openshell-gateway-grpc` passthrough Route with the
`router: grpc` label for NLB-backed ingress. Both resources are cleaned up
when the Route config is removed. RBAC: `route.openshift.io/routes` added to
the control-plane ClusterRole.

Closes P44, P45 from RECONCILE.md gap table.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@markturansky

Copy link
Copy Markdown
Collaborator Author

Review from amber (Shell agent)

PR: feat(control-plane): auto-reconcile router NetworkPolicy and NLB passthrough Route

The approach is solid and fits the existing reconciler pattern well. A few observations from the OpenShell side:

What looks good

  • Both reconcilers are correctly gated inside r.isOpenShift — no impact on non-OpenShift deployments.
  • Cleanup on gw.Route == nil via deleteIfExists is clean and consistent with other reconcilers.
  • Passthrough TLS on the Route is correct for gRPC — not terminating at the HAProxy layer is the right call.
  • The haproxy.router.openshift.io/timeout: 3600s annotation is important for long-lived gRPC streams; good to see it included.

Questions / potential issues

  1. OwnerReference targets StatefulSet by name, but post-PR feat(gateway): add database provisioning for postgres/sqlite backends #415 the workload may be a Deployment. getWorkloadUID(ctx, namespace, "openshell-gateway") — does this resolve StatefulSet, Deployment, or both? If a postgres-backed gateway uses a Deployment, stsUID will be empty and the Route will have no owner reference, so it won't be GC'd when the workload is deleted. Worth verifying that getWorkloadUID handles both workload types.

  2. NetworkPolicy hard-codes ports 8080/8081. These match the current OpenShell defaults, but if they ever become configurable they'll need to be threaded through. A named constant or derivation from gw config would make this more robust.

  3. NetworkPolicy from has namespaceSelector only, no podSelector. This lets any pod in openshift-ingress reach the gateway on those ports. In practice HAProxy router pods are what you want. This is fine for now (OCP doesn't guarantee consistent labels on router pods across versions), just noting it as a conscious trade-off.

  4. Silent skip when both gw.Route.Host is empty and baseDomain is unset. The debug log is fine, but the reconciler returns nil without creating the Route. Any caller monitoring for the Route's existence won't get an error — they'll just see nothing. Consider whether a status condition or a warning-level log would be more observable here.

  5. Draft + unchecked test plan. All test plan items are unchecked. Flagging for when this moves to ready-for-review.

Overall direction is right. The OwnerReference / workload-type question is the one I'd want answered before merging.

@markturansky
markturansky marked this pull request as ready for review July 26, 2026 12:31
@github-actions github-actions Bot added auto-merge-pending PR eligible for auto-merge, waiting for checks component/control-plane component/manifests labels Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-pending PR eligible for auto-merge, waiting for checks component/control-plane component/manifests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant