diff --git a/api/v1/nebariapp_types.go b/api/v1/nebariapp_types.go index 164fc36..1956997 100644 --- a/api/v1/nebariapp_types.go +++ b/api/v1/nebariapp_types.go @@ -25,12 +25,23 @@ type NebariAppSpec struct { // Hostname is the fully qualified domain name where the application should be accessible. // This will be used to generate HTTPRoute. // Example: "myapp.nebari.local" or "api.example.com" + // + // Each NebariApp exposes exactly one public hostname and is backed by exactly + // one Kubernetes Service (spec.service). Packs that need multiple hostnames, + // or that genuinely need to fan out to multiple Services, must be split into + // multiple NebariApps. This is an intentional boundary so a NebariApp's TLS, + // auth, landing-page card, and routing concerns all scope to a single + // user-visible URL backed by a single Service. To fan out by path under one + // hostname to different ports on that Service, use routing.routes[].port. // +kubebuilder:validation:Required // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$` Hostname string `json:"hostname"` - // Service defines the backend Kubernetes Service that should receive traffic. + // Service defines the single backend Kubernetes Service that receives traffic + // for this NebariApp. spec.service.port is the default backend port for routes + // that don't override via routing.routes[].port. Multi-backend (multiple Services + // per NebariApp) is not supported by design — use multiple NebariApps instead. // +kubebuilder:validation:Required Service ServiceReference `json:"service"` @@ -63,26 +74,24 @@ type NebariAppSpec struct { LandingPage *LandingPageConfig `json:"landingPage,omitempty"` } -// ServiceReference identifies the Kubernetes Service that backs this application. +// ServiceReference identifies a Kubernetes Service in the NebariApp's own +// namespace. Cross-namespace backends are not supported: the operator-generated +// HTTPRoute would require a Gateway API ReferenceGrant in the target namespace +// to actually carry traffic, and the operator does not create one. Workloads +// that need to talk across namespaces should do so via in-cluster DNS rather +// than via the public HTTPRoute. type ServiceReference struct { - // Name is the name of the Kubernetes Service in the same namespace. + // Name is the name of the Kubernetes Service in the NebariApp's namespace. // +kubebuilder:validation:Required // +kubebuilder:validation:MinLength=1 Name string `json:"name"` - // Port is the port number on the Service to route traffic to. + // Port is the default port number on the Service to route traffic to. + // Individual routes may override this via routing.routes[].port. // +kubebuilder:validation:Required // +kubebuilder:validation:Minimum=1 // +kubebuilder:validation:Maximum=65535 Port int32 `json:"port"` - - // Namespace is the namespace of the Service (if different from the NebariApp). - // If not specified, defaults to the NebariApp's namespace. - // This allows referencing services in other namespaces for centralized service architectures. - // Note: The operator has cluster-scoped permissions to read Services across all namespaces. - // +optional - // +kubebuilder:validation:MinLength=1 - Namespace string `json:"namespace,omitempty"` } // RoutingConfig configures routing behavior for the application. @@ -125,7 +134,7 @@ type RoutingConfig struct { // RouteMatch defines a path-based routing rule. type RouteMatch struct { // PathPrefix specifies the path prefix to match for routing. - // Traffic matching this prefix will be routed to the service. + // Traffic matching this prefix will be routed to spec.service on the resolved port. // Must start with "/". Example: "/app-1", "/api/v1" // +kubebuilder:validation:Required // +kubebuilder:validation:Pattern=`^/.*` @@ -140,6 +149,16 @@ type RouteMatch struct { // +kubebuilder:validation:Enum=PathPrefix;Exact // +optional PathType string `json:"pathType,omitempty"` + + // Port optionally overrides the default backend port (spec.service.port) + // for this route. The referenced port must be exposed by spec.service. + // When omitted, the route forwards to spec.service.port. This is the only + // mechanism for path-based port differentiation; per-route backend Services + // are not supported (use multiple NebariApps instead). + // +optional + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=65535 + Port *int32 `json:"port,omitempty"` } // RoutingTLSConfig controls TLS termination for the HTTPRoute. diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index ab03041..07e032e 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -411,6 +411,11 @@ func (in *ResourceReference) DeepCopy() *ResourceReference { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RouteMatch) DeepCopyInto(out *RouteMatch) { *out = *in + if in.Port != nil { + in, out := &in.Port, &out.Port + *out = new(int32) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RouteMatch. @@ -429,12 +434,16 @@ func (in *RoutingConfig) DeepCopyInto(out *RoutingConfig) { if in.Routes != nil { in, out := &in.Routes, &out.Routes *out = make([]RouteMatch, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.PublicRoutes != nil { in, out := &in.PublicRoutes, &out.PublicRoutes *out = make([]RouteMatch, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.TLS != nil { in, out := &in.TLS, &out.TLS diff --git a/config/crd/bases/reconcilers.nebari.dev_nebariapps.yaml b/config/crd/bases/reconcilers.nebari.dev_nebariapps.yaml index 265852f..ef8c064 100644 --- a/config/crd/bases/reconcilers.nebari.dev_nebariapps.yaml +++ b/config/crd/bases/reconcilers.nebari.dev_nebariapps.yaml @@ -325,6 +325,14 @@ spec: Hostname is the fully qualified domain name where the application should be accessible. This will be used to generate HTTPRoute. Example: "myapp.nebari.local" or "api.example.com" + + Each NebariApp exposes exactly one public hostname and is backed by exactly + one Kubernetes Service (spec.service). Packs that need multiple hostnames, + or that genuinely need to fan out to multiple Services, must be split into + multiple NebariApps. This is an intentional boundary so a NebariApp's TLS, + auth, landing-page card, and routing concerns all scope to a single + user-visible URL backed by a single Service. To fan out by path under one + hostname to different ports on that Service, use routing.routes[].port. minLength: 1 pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string @@ -445,7 +453,7 @@ spec: pathPrefix: description: |- PathPrefix specifies the path prefix to match for routing. - Traffic matching this prefix will be routed to the service. + Traffic matching this prefix will be routed to spec.service on the resolved port. Must start with "/". Example: "/app-1", "/api/v1" pattern: ^/.* type: string @@ -461,6 +469,17 @@ spec: - PathPrefix - Exact type: string + port: + description: |- + Port optionally overrides the default backend port (spec.service.port) + for this route. The referenced port must be exposed by spec.service. + When omitted, the route forwards to spec.service.port. This is the only + mechanism for path-based port differentiation; per-route backend Services + are not supported (use multiple NebariApps instead). + format: int32 + maximum: 65535 + minimum: 1 + type: integer required: - pathPrefix type: object @@ -477,7 +496,7 @@ spec: pathPrefix: description: |- PathPrefix specifies the path prefix to match for routing. - Traffic matching this prefix will be routed to the service. + Traffic matching this prefix will be routed to spec.service on the resolved port. Must start with "/". Example: "/app-1", "/api/v1" pattern: ^/.* type: string @@ -493,6 +512,17 @@ spec: - PathPrefix - Exact type: string + port: + description: |- + Port optionally overrides the default backend port (spec.service.port) + for this route. The referenced port must be exposed by spec.service. + When omitted, the route forwards to spec.service.port. This is the only + mechanism for path-based port differentiation; per-route backend Services + are not supported (use multiple NebariApps instead). + format: int32 + maximum: 65535 + minimum: 1 + type: integer required: - pathPrefix type: object @@ -514,25 +544,21 @@ spec: type: object type: object service: - description: Service defines the backend Kubernetes Service that should - receive traffic. + description: |- + Service defines the single backend Kubernetes Service that receives traffic + for this NebariApp. spec.service.port is the default backend port for routes + that don't override via routing.routes[].port. Multi-backend (multiple Services + per NebariApp) is not supported by design — use multiple NebariApps instead. properties: name: description: Name is the name of the Kubernetes Service in the - same namespace. - minLength: 1 - type: string - namespace: - description: |- - Namespace is the namespace of the Service (if different from the NebariApp). - If not specified, defaults to the NebariApp's namespace. - This allows referencing services in other namespaces for centralized service architectures. - Note: The operator has cluster-scoped permissions to read Services across all namespaces. + NebariApp's namespace. minLength: 1 type: string port: - description: Port is the port number on the Service to route traffic - to. + description: |- + Port is the default port number on the Service to route traffic to. + Individual routes may override this via routing.routes[].port. format: int32 maximum: 65535 minimum: 1 diff --git a/docs/api-reference.md b/docs/api-reference.md index 0a56b6b..6d559ab 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -232,8 +232,8 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `hostname` _string_ | Hostname is the fully qualified domain name where the application should be accessible.
This will be used to generate HTTPRoute.
Example: "myapp.nebari.local" or "api.example.com" | | MinLength: 1
Pattern: `^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
Required: \{\}
| -| `service` _[ServiceReference](#servicereference)_ | Service defines the backend Kubernetes Service that should receive traffic. | | Required: \{\}
| +| `hostname` _string_ | Hostname is the fully qualified domain name where the application should be accessible.
This will be used to generate HTTPRoute.
Example: "myapp.nebari.local" or "api.example.com"
Each NebariApp exposes exactly one public hostname and is backed by exactly
one Kubernetes Service (spec.service). Packs that need multiple hostnames,
or that genuinely need to fan out to multiple Services, must be split into
multiple NebariApps. This is an intentional boundary so a NebariApp's TLS,
auth, landing-page card, and routing concerns all scope to a single
user-visible URL backed by a single Service. To fan out by path under one
hostname to different ports on that Service, use routing.routes[].port. | | MinLength: 1
Pattern: `^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
Required: \{\}
| +| `service` _[ServiceReference](#servicereference)_ | Service defines the single backend Kubernetes Service that receives traffic
for this NebariApp. spec.service.port is the default backend port for routes
that don't override via routing.routes[].port. Multi-backend (multiple Services
per NebariApp) is not supported by design — use multiple NebariApps instead. | | Required: \{\}
| | `routing` _[RoutingConfig](#routingconfig)_ | Routing configures routing behavior including path-based rules and TLS. | | Optional: \{\}
| | `auth` _[AuthConfig](#authconfig)_ | Auth configures authentication/authorization for the application.
When enabled, the application will require OIDC authentication via supporting OIDC Provider. | | Optional: \{\}
| | `gateway` _string_ | Gateway specifies which shared Gateway to use for routing.
Valid values are "public" (default) or "internal". | public | Enum: [public internal]
Optional: \{\}
| @@ -287,8 +287,9 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `pathPrefix` _string_ | PathPrefix specifies the path prefix to match for routing.
Traffic matching this prefix will be routed to the service.
Must start with "/". Example: "/app-1", "/api/v1" | | Pattern: `^/.*`
Required: \{\}
| +| `pathPrefix` _string_ | PathPrefix specifies the path prefix to match for routing.
Traffic matching this prefix will be routed to spec.service on the resolved port.
Must start with "/". Example: "/app-1", "/api/v1" | | Pattern: `^/.*`
Required: \{\}
| | `pathType` _string_ | PathType specifies how the path should be matched.
Valid values:
- "PathPrefix": Match requests with the specified path prefix
- "Exact": Match requests with the exact path
When used in routing.routes, defaults to "PathPrefix".
When used in routing.publicRoutes, defaults to "Exact" (safer for auth bypass). | | Enum: [PathPrefix Exact]
Optional: \{\}
| +| `port` _integer_ | Port optionally overrides the default backend port (spec.service.port)
for this route. The referenced port must be exposed by spec.service.
When omitted, the route forwards to spec.service.port. This is the only
mechanism for path-based port differentiation; per-route backend Services
are not supported (use multiple NebariApps instead). | | Maximum: 65535
Minimum: 1
Optional: \{\}
| --- @@ -373,16 +374,20 @@ _Appears in:_ #### ServiceReference -ServiceReference identifies the Kubernetes Service that backs this application. +ServiceReference identifies a Kubernetes Service in the NebariApp's own +namespace. Cross-namespace backends are not supported: the operator-generated +HTTPRoute would require a Gateway API ReferenceGrant in the target namespace +to actually carry traffic, and the operator does not create one. Workloads +that need to talk across namespaces should do so via in-cluster DNS rather +than via the public HTTPRoute. _Appears in:_ - [NebariAppSpec](#nebariappspec) | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `name` _string_ | Name is the name of the Kubernetes Service in the same namespace. | | MinLength: 1
Required: \{\}
| -| `port` _integer_ | Port is the port number on the Service to route traffic to. | | Maximum: 65535
Minimum: 1
Required: \{\}
| -| `namespace` _string_ | Namespace is the namespace of the Service (if different from the NebariApp).
If not specified, defaults to the NebariApp's namespace.
This allows referencing services in other namespaces for centralized service architectures.
Note: The operator has cluster-scoped permissions to read Services across all namespaces. | | MinLength: 1
Optional: \{\}
| +| `name` _string_ | Name is the name of the Kubernetes Service in the NebariApp's namespace. | | MinLength: 1
Required: \{\}
| +| `port` _integer_ | Port is the default port number on the Service to route traffic to.
Individual routes may override this via routing.routes[].port. | | Maximum: 65535
Minimum: 1
Required: \{\}
| --- diff --git a/internal/controller/reconcilers/core/reconciler.go b/internal/controller/reconcilers/core/reconciler.go index e3be172..0710bab 100644 --- a/internal/controller/reconcilers/core/reconciler.go +++ b/internal/controller/reconcilers/core/reconciler.go @@ -115,43 +115,58 @@ func ValidateNamespaceOptIn(ctx context.Context, c client.Client, nebariApp *app return nil } -// ValidateService checks if the referenced service exists in the namespace and has the specified port. -// Returns an error if the service doesn't exist or the port is not exposed. +// ValidateService checks that spec.service exists in the NebariApp's namespace +// and exposes both the default port (spec.service.port) and any per-route port +// overrides declared in routing.routes[].port and routing.publicRoutes[].port. +// Cross-namespace backends are not supported; the Service is always looked up +// in the NebariApp's own namespace. func ValidateService(ctx context.Context, c client.Client, nebariApp *appsv1.NebariApp) error { service := &corev1.Service{} - - // Use specified service namespace, or default to NebariApp's namespace - serviceNamespace := nebariApp.Spec.Service.Namespace - if serviceNamespace == "" { - serviceNamespace = nebariApp.Namespace - } - serviceKey := client.ObjectKey{ Name: nebariApp.Spec.Service.Name, - Namespace: serviceNamespace, + Namespace: nebariApp.Namespace, } if err := c.Get(ctx, serviceKey, service); err != nil { if errors.IsNotFound(err) { return fmt.Errorf("service %s not found in namespace %s", - nebariApp.Spec.Service.Name, serviceNamespace) + nebariApp.Spec.Service.Name, nebariApp.Namespace) } return fmt.Errorf("failed to get service: %w", err) } - // Validate that the specified port exists on the service - portFound := false + exposed := map[int32]struct{}{} for _, port := range service.Spec.Ports { - if port.Port == nebariApp.Spec.Service.Port { - portFound = true - break - } + exposed[port.Port] = struct{}{} } - if !portFound { + if _, ok := exposed[nebariApp.Spec.Service.Port]; !ok { return fmt.Errorf("service %s does not expose port %d", nebariApp.Spec.Service.Name, nebariApp.Spec.Service.Port) } + if nebariApp.Spec.Routing == nil { + return nil + } + + for _, route := range nebariApp.Spec.Routing.Routes { + if route.Port == nil { + continue + } + if _, ok := exposed[*route.Port]; !ok { + return fmt.Errorf("route %q: service %s does not expose port %d", + route.PathPrefix, nebariApp.Spec.Service.Name, *route.Port) + } + } + for _, route := range nebariApp.Spec.Routing.PublicRoutes { + if route.Port == nil { + continue + } + if _, ok := exposed[*route.Port]; !ok { + return fmt.Errorf("public route %q: service %s does not expose port %d", + route.PathPrefix, nebariApp.Spec.Service.Name, *route.Port) + } + } + return nil } diff --git a/internal/controller/reconcilers/core/reconciler_test.go b/internal/controller/reconcilers/core/reconciler_test.go index 3ba56ff..4524062 100644 --- a/internal/controller/reconcilers/core/reconciler_test.go +++ b/internal/controller/reconcilers/core/reconciler_test.go @@ -27,6 +27,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" appsv1 "github.com/nebari-dev/nebari-operator/api/v1" + "github.com/nebari-dev/nebari-operator/internal/controller/utils/ptr" ) func TestValidateNamespaceOptIn(t *testing.T) { @@ -184,11 +185,11 @@ func TestValidateService(t *testing.T) { expectError: true, }, { - name: "Cross-namespace service reference", + name: "Route port override not exposed by service", service: &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: "external-service", - Namespace: "other-namespace", + Name: "test-service", + Namespace: "default", }, Spec: corev1.ServiceSpec{ Ports: []corev1.ServicePort{ @@ -203,9 +204,52 @@ func TestValidateService(t *testing.T) { }, Spec: appsv1.NebariAppSpec{ Service: appsv1.ServiceReference{ - Name: "external-service", - Namespace: "other-namespace", - Port: 8080, + Name: "test-service", + Port: 8080, + }, + Routing: &appsv1.RoutingConfig{ + Routes: []appsv1.RouteMatch{ + { + PathPrefix: "/api", + Port: ptr.To(int32(8000)), // not exposed by the Service + }, + }, + }, + }, + }, + expectError: true, + }, + { + name: "Route port override exposed by service passes", + service: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + Namespace: "default", + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + {Port: 80}, + {Port: 8000}, + }, + }, + }, + nebariApp: &appsv1.NebariApp{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-app", + Namespace: "default", + }, + Spec: appsv1.NebariAppSpec{ + Service: appsv1.ServiceReference{ + Name: "test-service", + Port: 80, + }, + Routing: &appsv1.RoutingConfig{ + Routes: []appsv1.RouteMatch{ + { + PathPrefix: "/api", + Port: ptr.To(int32(8000)), + }, + }, }, }, }, diff --git a/internal/controller/reconcilers/routing/httproute.go b/internal/controller/reconcilers/routing/httproute.go index 14bd12e..ed6bfc1 100644 --- a/internal/controller/reconcilers/routing/httproute.go +++ b/internal/controller/reconcilers/routing/httproute.go @@ -221,75 +221,78 @@ func (r *RoutingReconciler) buildHTTPRoute(nebariApp *appsv1.NebariApp, gatewayN return route, nil } -// buildHTTPRouteRules generates HTTPRoute rules based on NebariApp routes +// buildHTTPRouteRules generates HTTPRoute rules based on NebariApp routes. +// +// When routing.routes is empty, the result is a single rule with no matches +// (Gateway API treats this as "/" PathPrefix) and spec.service.port as the +// backend port. When routing.routes is non-empty, the result is one rule per +// route, each with its own match and its own resolved port (the route's port +// override if set, else spec.service.port). All rules point at spec.service. func (r *RoutingReconciler) buildHTTPRouteRules(nebariApp *appsv1.NebariApp) []gatewayv1.HTTPRouteRule { - // Get routes from routing config if specified var routes []appsv1.RouteMatch if nebariApp.Spec.Routing != nil { routes = nebariApp.Spec.Routing.Routes } - // Build a single rule with multiple matches (one per route) - // All matches route to the same backend, so we use one rule - // If no routes specified, we create an empty matches array. Gateway API will automatically - // add a default path match of "/" (PathPrefix) when matches is empty or null. - matches := make([]gatewayv1.HTTPRouteMatch, 0, len(routes)) - for _, route := range routes { - pathType := gatewayv1.PathMatchPathPrefix - if route.PathType == "Exact" { - pathType = gatewayv1.PathMatchExact - } - - pathValue := route.PathPrefix - match := gatewayv1.HTTPRouteMatch{ - Path: &gatewayv1.HTTPPathMatch{ - Type: &pathType, - Value: &pathValue, + if len(routes) == 0 { + return []gatewayv1.HTTPRouteRule{ + { + Matches: []gatewayv1.HTTPRouteMatch{}, + BackendRefs: r.buildBackendRefs(nebariApp.Spec.Service.Name, nebariApp.Spec.Service.Port), }, } - matches = append(matches, match) } - return []gatewayv1.HTTPRouteRule{ - { - Matches: matches, - BackendRefs: r.buildBackendRefs(nebariApp), - }, + rules := make([]gatewayv1.HTTPRouteRule, 0, len(routes)) + for _, route := range routes { + rules = append(rules, gatewayv1.HTTPRouteRule{ + Matches: []gatewayv1.HTTPRouteMatch{routeToMatch(route, gatewayv1.PathMatchPathPrefix)}, + BackendRefs: r.buildBackendRefs(nebariApp.Spec.Service.Name, resolveRoutePort(route, nebariApp.Spec.Service.Port)), + }) } + return rules } -// buildBackendRefs generates backend references for the HTTPRoute -func (r *RoutingReconciler) buildBackendRefs(nebariApp *appsv1.NebariApp) []gatewayv1.HTTPBackendRef { - // weight := int32(100) - // if nebariApp.Spec.Service.Weight != nil { - // weight = *nebariApp.Spec.Service.Weight - // } - - port := nebariApp.Spec.Service.Port - - // Use specified service namespace, or default to NebariApp's namespace - serviceNamespace := nebariApp.Spec.Service.Namespace - if serviceNamespace == "" { - serviceNamespace = nebariApp.Namespace - } - - backendRef := gatewayv1.BackendObjectReference{ - Name: gatewayv1.ObjectName(nebariApp.Spec.Service.Name), - Port: &port, +// routeToMatch converts a RouteMatch into a Gateway API HTTPRouteMatch. +// defaultPathType is used when the route does not explicitly set PathType. +func routeToMatch(route appsv1.RouteMatch, defaultPathType gatewayv1.PathMatchType) gatewayv1.HTTPRouteMatch { + pathType := defaultPathType + switch route.PathType { + case "Exact": + pathType = gatewayv1.PathMatchExact + case "PathPrefix": + pathType = gatewayv1.PathMatchPathPrefix + } + pathValue := route.PathPrefix + return gatewayv1.HTTPRouteMatch{ + Path: &gatewayv1.HTTPPathMatch{ + Type: &pathType, + Value: &pathValue, + }, } +} - // Only set namespace if it's different from the HTTPRoute's namespace - // to support cross-namespace service references - if serviceNamespace != nebariApp.Namespace { - ns := gatewayv1.Namespace(serviceNamespace) - backendRef.Namespace = &ns +// resolveRoutePort returns the route's per-route port override if set, +// otherwise falls back to the NebariApp's default spec.service.port. +func resolveRoutePort(route appsv1.RouteMatch, defaultPort int32) int32 { + if route.Port != nil { + return *route.Port } + return defaultPort +} +// buildBackendRefs generates backend references for a single HTTPRouteRule +// pointing at the given Service name and port. The Service must live in the +// NebariApp's own namespace (cross-namespace backends are not supported), so +// the generated BackendObjectReference never sets a namespace field. +func (r *RoutingReconciler) buildBackendRefs(serviceName string, port int32) []gatewayv1.HTTPBackendRef { return []gatewayv1.HTTPBackendRef{ { BackendRef: gatewayv1.BackendRef{ - BackendObjectReference: backendRef, - // Weight: &weight, + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: gatewayv1.ObjectName(serviceName), + Port: &port, + }, }, }, } @@ -403,19 +406,14 @@ func (r *RoutingReconciler) buildPublicHTTPRoute(nebariApp *appsv1.NebariApp, ga sectionName = gatewayv1.SectionName(tlsListenerName) } - // Build matches for each public route (default to Exact for safer auth bypass) - matches := make([]gatewayv1.HTTPRouteMatch, 0, len(nebariApp.Spec.Routing.PublicRoutes)) + // One rule per public route, each with its own resolved port (the route's + // port override if set, else spec.service.port). PublicRoutes default to + // Exact matching for safer auth bypass. + rules := make([]gatewayv1.HTTPRouteRule, 0, len(nebariApp.Spec.Routing.PublicRoutes)) for _, route := range nebariApp.Spec.Routing.PublicRoutes { - pathType := gatewayv1.PathMatchExact - if route.PathType == "PathPrefix" { - pathType = gatewayv1.PathMatchPathPrefix - } - pathValue := route.PathPrefix - matches = append(matches, gatewayv1.HTTPRouteMatch{ - Path: &gatewayv1.HTTPPathMatch{ - Type: &pathType, - Value: &pathValue, - }, + rules = append(rules, gatewayv1.HTTPRouteRule{ + Matches: []gatewayv1.HTTPRouteMatch{routeToMatch(route, gatewayv1.PathMatchExact)}, + BackendRefs: r.buildBackendRefs(nebariApp.Spec.Service.Name, resolveRoutePort(route, nebariApp.Spec.Service.Port)), }) } @@ -446,12 +444,7 @@ func (r *RoutingReconciler) buildPublicHTTPRoute(nebariApp *appsv1.NebariApp, ga Hostnames: []gatewayv1.Hostname{ gatewayv1.Hostname(nebariApp.Spec.Hostname), }, - Rules: []gatewayv1.HTTPRouteRule{ - { - Matches: matches, - BackendRefs: r.buildBackendRefs(nebariApp), - }, - }, + Rules: rules, }, } diff --git a/internal/controller/reconcilers/routing/httproute_test.go b/internal/controller/reconcilers/routing/httproute_test.go index a828318..ea68efe 100644 --- a/internal/controller/reconcilers/routing/httproute_test.go +++ b/internal/controller/reconcilers/routing/httproute_test.go @@ -196,7 +196,7 @@ func TestBuildHTTPRoute(t *testing.T) { gatewayName: constants.PublicGatewayName, expectedHostname: "test.nebari.local", expectedBackendPort: 8080, - expectedRulesCount: 1, // Single rule with multiple matches + expectedRulesCount: 2, // One rule per route }, } @@ -346,7 +346,7 @@ func TestBuildHTTPRouteRules(t *testing.T) { expectedPathType: gatewayv1.PathMatchPathPrefix, // Not checked when checkPathType=false }, { - name: "Multiple custom routes with different path types", + name: "Multiple custom routes emit one rule per route", nebariApp: &appsv1.NebariApp{ Spec: appsv1.NebariAppSpec{ Service: appsv1.ServiceReference{ @@ -367,8 +367,8 @@ func TestBuildHTTPRouteRules(t *testing.T) { }, }, }, - expectedRulesCount: 1, // Single rule with multiple matches - expectedMatchesCount: 2, + expectedRulesCount: 2, // One rule per route + expectedMatchesCount: 1, // Each rule carries a single match checkPathType: false, }, } @@ -381,10 +381,10 @@ func TestBuildHTTPRouteRules(t *testing.T) { t.Errorf("expected %d rules, got %d", tt.expectedRulesCount, len(rules)) } - // Check matches count for first rule - if len(rules) > 0 { - if len(rules[0].Matches) != tt.expectedMatchesCount { - t.Errorf("expected %d matches in first rule, got %d", tt.expectedMatchesCount, len(rules[0].Matches)) + // Check matches count for each rule + for i, rule := range rules { + if len(rule.Matches) != tt.expectedMatchesCount { + t.Errorf("rule %d: expected %d matches, got %d", i, tt.expectedMatchesCount, len(rule.Matches)) } } @@ -729,29 +729,31 @@ func TestBuildPublicHTTPRoute(t *testing.T) { if len(route.Spec.Hostnames) != 1 || string(route.Spec.Hostnames[0]) != tt.nebariApp.Spec.Hostname { t.Errorf("expected hostname=%s, got hostnames=%v", tt.nebariApp.Spec.Hostname, route.Spec.Hostnames) } - if len(route.Spec.Rules) != 1 { - t.Fatalf("expected 1 rule, got %d", len(route.Spec.Rules)) - } - rule := route.Spec.Rules[0] - if len(rule.Matches) != tt.expectedMatchesCount { - t.Errorf("expected %d matches, got %d", tt.expectedMatchesCount, len(rule.Matches)) + // Each public route emits its own rule with a single match. + if len(route.Spec.Rules) != tt.expectedMatchesCount { + t.Fatalf("expected %d rules (one per public route), got %d", tt.expectedMatchesCount, len(route.Spec.Rules)) } - for i, match := range rule.Matches { + for i, rule := range route.Spec.Rules { + if len(rule.Matches) != 1 { + t.Errorf("rule %d: expected 1 match, got %d", i, len(rule.Matches)) + continue + } + match := rule.Matches[0] if match.Path == nil { - t.Errorf("match %d: path is nil", i) + t.Errorf("rule %d: path is nil", i) continue } if i < len(tt.expectedPathTypes) && *match.Path.Type != tt.expectedPathTypes[i] { - t.Errorf("match %d: expected %s, got %s", i, tt.expectedPathTypes[i], *match.Path.Type) + t.Errorf("rule %d: expected %s, got %s", i, tt.expectedPathTypes[i], *match.Path.Type) } if i < len(tt.expectedPaths) && *match.Path.Value != tt.expectedPaths[i] { - t.Errorf("match %d: expected path=%s, got=%s", i, tt.expectedPaths[i], *match.Path.Value) + t.Errorf("rule %d: expected path=%s, got=%s", i, tt.expectedPaths[i], *match.Path.Value) + } + if len(rule.BackendRefs) != 1 { + t.Errorf("rule %d: expected 1 backend ref, got %d", i, len(rule.BackendRefs)) + } else if string(rule.BackendRefs[0].Name) != tt.nebariApp.Spec.Service.Name { + t.Errorf("rule %d: expected backend name=%s, got=%s", i, tt.nebariApp.Spec.Service.Name, rule.BackendRefs[0].Name) } - } - if len(rule.BackendRefs) != 1 { - t.Errorf("expected 1 backend ref, got %d", len(rule.BackendRefs)) - } else if string(rule.BackendRefs[0].Name) != tt.nebariApp.Spec.Service.Name { - t.Errorf("expected backend name=%s, got=%s", tt.nebariApp.Spec.Service.Name, rule.BackendRefs[0].Name) } }) } @@ -1112,80 +1114,102 @@ func TestBuildBackendRefs(t *testing.T) { } tests := []struct { - name string - nebariApp *appsv1.NebariApp - expectNamespace bool - expectedNamespace string + name string + svcName string + port int32 }{ - { - name: "Same namespace - namespace not set in backend ref", - nebariApp: &appsv1.NebariApp{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-app", - Namespace: "default", - }, - Spec: appsv1.NebariAppSpec{ - Service: appsv1.ServiceReference{ - Name: "test-service", - Port: 8080, - }, - }, - }, - expectNamespace: false, - }, - { - name: "Cross-namespace - namespace set in backend ref", - nebariApp: &appsv1.NebariApp{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-app", - Namespace: "default", - }, - Spec: appsv1.NebariAppSpec{ - Service: appsv1.ServiceReference{ - Name: "external-service", - Namespace: "other-namespace", - Port: 8080, - }, - }, - }, - expectNamespace: true, - expectedNamespace: "other-namespace", - }, + {name: "Default service port", svcName: "test-service", port: 8080}, + {name: "Per-route port override", svcName: "test-service", port: 9000}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - backendRefs := reconciler.buildBackendRefs(tt.nebariApp) + backendRefs := reconciler.buildBackendRefs(tt.svcName, tt.port) if len(backendRefs) != 1 { t.Fatalf("expected 1 backend ref, got %d", len(backendRefs)) } backendRef := backendRefs[0] - - // Check service name - if string(backendRef.Name) != tt.nebariApp.Spec.Service.Name { - t.Errorf("expected name=%s, got=%s", tt.nebariApp.Spec.Service.Name, backendRef.Name) + if string(backendRef.Name) != tt.svcName { + t.Errorf("expected name=%s, got=%s", tt.svcName, backendRef.Name) } - - // Check port - expectedPort := tt.nebariApp.Spec.Service.Port - if *backendRef.Port != expectedPort { - t.Errorf("expected port=%d, got=%d", expectedPort, *backendRef.Port) + if backendRef.Port == nil || *backendRef.Port != tt.port { + t.Errorf("expected port=%d, got=%v", tt.port, backendRef.Port) } - - // Check namespace - if tt.expectNamespace { - if backendRef.Namespace == nil { - t.Error("expected namespace to be set, but it was nil") - } else if string(*backendRef.Namespace) != tt.expectedNamespace { - t.Errorf("expected namespace=%s, got=%s", tt.expectedNamespace, *backendRef.Namespace) - } - } else { - if backendRef.Namespace != nil { - t.Errorf("expected namespace to be nil, got=%s", *backendRef.Namespace) - } + // Same-namespace contract: namespace must never be set on the rendered backendRef. + if backendRef.Namespace != nil { + t.Errorf("expected namespace to be nil, got=%s", *backendRef.Namespace) } }) } } + +func TestBuildHTTPRouteRules_PerRoutePort(t *testing.T) { + scheme := runtime.NewScheme() + _ = appsv1.AddToScheme(scheme) + + reconciler := &RoutingReconciler{Scheme: scheme} + apiPort := int32(8000) + + nebariApp := &appsv1.NebariApp{ + ObjectMeta: metav1.ObjectMeta{Name: "app", Namespace: "default"}, + Spec: appsv1.NebariAppSpec{ + Hostname: "app.example.com", + Service: appsv1.ServiceReference{Name: "app-svc", Port: 80}, + Routing: &appsv1.RoutingConfig{ + Routes: []appsv1.RouteMatch{ + {PathPrefix: "/api", Port: &apiPort}, + {PathPrefix: "/"}, + }, + }, + }, + } + + rules := reconciler.buildHTTPRouteRules(nebariApp) + if len(rules) != 2 { + t.Fatalf("expected 2 rules (one per route), got %d", len(rules)) + } + + // First rule: /api → app-svc:8000 (port override) + if string(rules[0].BackendRefs[0].Name) != "app-svc" { + t.Errorf("expected first rule to point at app-svc, got %s", rules[0].BackendRefs[0].Name) + } + if rules[0].BackendRefs[0].Port == nil || *rules[0].BackendRefs[0].Port != 8000 { + t.Errorf("expected first rule port 8000, got %+v", rules[0].BackendRefs[0].Port) + } + + // Second rule: / → app-svc:80 (fallback to spec.service.port) + if string(rules[1].BackendRefs[0].Name) != "app-svc" { + t.Errorf("expected second rule to point at app-svc, got %s", rules[1].BackendRefs[0].Name) + } + if rules[1].BackendRefs[0].Port == nil || *rules[1].BackendRefs[0].Port != 80 { + t.Errorf("expected second rule port 80 (default), got %+v", rules[1].BackendRefs[0].Port) + } +} + +func TestBuildHTTPRouteRules_NoRoutes(t *testing.T) { + scheme := runtime.NewScheme() + _ = appsv1.AddToScheme(scheme) + + reconciler := &RoutingReconciler{Scheme: scheme} + + nebariApp := &appsv1.NebariApp{ + ObjectMeta: metav1.ObjectMeta{Name: "app", Namespace: "default"}, + Spec: appsv1.NebariAppSpec{ + Hostname: "app.example.com", + Service: appsv1.ServiceReference{Name: "svc", Port: 80}, + }, + } + + rules := reconciler.buildHTTPRouteRules(nebariApp) + if len(rules) != 1 { + t.Fatalf("expected exactly 1 rule when no routes configured, got %d", len(rules)) + } + if len(rules[0].Matches) != 0 { + t.Errorf("expected empty matches (Gateway API defaults to '/'), got %d", len(rules[0].Matches)) + } + if len(rules[0].BackendRefs) != 1 || string(rules[0].BackendRefs[0].Name) != "svc" { + t.Errorf("expected single rule pointing at spec.service, got %+v", rules[0].BackendRefs) + } +}