diff --git a/api/v1/nebariapp_types.go b/api/v1/nebariapp_types.go index bb25dbe..2dd29b3 100644 --- a/api/v1/nebariapp_types.go +++ b/api/v1/nebariapp_types.go @@ -18,6 +18,7 @@ package v1 import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" ) // NebariAppSpec defines the desired state of NebariApp @@ -104,6 +105,15 @@ type RoutingConfig struct { // Each entry uses the same RouteMatch format as routes, supporting both // PathPrefix (default) and Exact matching via the pathType field. // Example: [{pathPrefix: "/api/v1/health", pathType: "Exact"}] + // + // PublicRoutes is for carving out a *subset* of paths that should bypass + // auth while the rest stays protected. It is not a way to disable auth for + // the whole app: if a public path also matches the main route (including the + // default "/" match when routes is empty) while auth.enforceAtGateway is + // true, both a protected and an unprotected HTTPRoute claim that path and + // the effective auth posture is undefined. To turn gateway auth off entirely, + // set auth.enforceAtGateway: false (keep provisioning the Keycloak client) or + // auth.enabled: false (no client at all) instead of listing every path here. // +optional PublicRoutes []RouteMatch `json:"publicRoutes,omitempty"` @@ -120,6 +130,27 @@ type RoutingConfig struct { // annotations always take precedence to avoid breaking internal behaviour. // +optional Annotations map[string]string `json:"annotations,omitempty"` + + // Filters are appended, in order, to every HTTPRouteRule the operator + // generates for this NebariApp (both the protected route and the public + // route built from publicRoutes). This is a pass-through to the upstream + // gateway.networking.k8s.io/v1 HTTPRouteFilter spec, mirroring the + // Annotations escape hatch: the operator does not interpret, reorder, or + // strip the filters, it only forwards them. Use it to adjust request or + // response headers at the gateway (for example, stripping an overpermissive + // CORS header an upstream sets, or adding security headers) without forking + // the operator or rebuilding the backend image. + // + // The field is schemaless in the CRD: inlining the full HTTPRouteFilter + // OpenAPI schema would copy its nested x-kubernetes-validations (CEL) rules + // into the NebariApp CRD and blow the per-CRD CEL cost budget. As a + // pass-through, the filters are instead validated by Gateway API when the + // operator applies them to the generated HTTPRoute. Each entry must still be + // a valid HTTPRouteFilter object (it is decoded into the typed Go struct). + // +optional + // +kubebuilder:validation:Schemaless + // +kubebuilder:pruning:PreserveUnknownFields + Filters []gatewayv1.HTTPRouteFilter `json:"filters,omitempty"` } // RouteMatch defines a path-based routing rule. @@ -230,6 +261,10 @@ type AuthConfig struct { // When false, the operator provisions the OIDC client and stores credentials // in a Secret, but does NOT create a SecurityPolicy - the application is // expected to handle OAuth natively (e.g., Grafana's built-in generic_oauth). + // This is the correct switch for "no gateway-level auth"; do not try to + // achieve that by listing every path under routing.publicRoutes, which + // instead produces overlapping protected and unprotected routes for the same + // path (see PublicRoutes). // +kubebuilder:default=true // +optional EnforceAtGateway *bool `json:"enforceAtGateway,omitempty"` diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index ab03041..30fcdf1 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -23,6 +23,7 @@ package v1 import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" + apisv1 "sigs.k8s.io/gateway-api/apis/v1" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -448,6 +449,13 @@ func (in *RoutingConfig) DeepCopyInto(out *RoutingConfig) { (*out)[key] = val } } + if in.Filters != nil { + in, out := &in.Filters, &out.Filters + *out = make([]apisv1.HTTPRouteFilter, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoutingConfig. diff --git a/config/crd/bases/reconcilers.nebari.dev_nebariapps.yaml b/config/crd/bases/reconcilers.nebari.dev_nebariapps.yaml index a93fd38..7e8174d 100644 --- a/config/crd/bases/reconcilers.nebari.dev_nebariapps.yaml +++ b/config/crd/bases/reconcilers.nebari.dev_nebariapps.yaml @@ -132,6 +132,10 @@ spec: When false, the operator provisions the OIDC client and stores credentials in a Secret, but does NOT create a SecurityPolicy - the application is expected to handle OAuth natively (e.g., Grafana's built-in generic_oauth). + This is the correct switch for "no gateway-level auth"; do not try to + achieve that by listing every path under routing.publicRoutes, which + instead produces overlapping protected and unprotected routes for the same + path (see PublicRoutes). type: boolean forwardAccessToken: description: |- @@ -442,6 +446,25 @@ spec: These annotations are merged with any operator-managed annotations; operator annotations always take precedence to avoid breaking internal behaviour. type: object + filters: + description: |- + Filters are appended, in order, to every HTTPRouteRule the operator + generates for this NebariApp (both the protected route and the public + route built from publicRoutes). This is a pass-through to the upstream + gateway.networking.k8s.io/v1 HTTPRouteFilter spec, mirroring the + Annotations escape hatch: the operator does not interpret, reorder, or + strip the filters, it only forwards them. Use it to adjust request or + response headers at the gateway (for example, stripping an overpermissive + CORS header an upstream sets, or adding security headers) without forking + the operator or rebuilding the backend image. + + The field is schemaless in the CRD: inlining the full HTTPRouteFilter + OpenAPI schema would copy its nested x-kubernetes-validations (CEL) rules + into the NebariApp CRD and blow the per-CRD CEL cost budget. As a + pass-through, the filters are instead validated by Gateway API when the + operator applies them to the generated HTTPRoute. Each entry must still be + a valid HTTPRouteFilter object (it is decoded into the typed Go struct). + x-kubernetes-preserve-unknown-fields: true publicRoutes: description: |- PublicRoutes specifies paths that should bypass OIDC authentication. @@ -450,6 +473,15 @@ spec: Each entry uses the same RouteMatch format as routes, supporting both PathPrefix (default) and Exact matching via the pathType field. Example: [{pathPrefix: "/api/v1/health", pathType: "Exact"}] + + PublicRoutes is for carving out a *subset* of paths that should bypass + auth while the rest stays protected. It is not a way to disable auth for + the whole app: if a public path also matches the main route (including the + default "/" match when routes is empty) while auth.enforceAtGateway is + true, both a protected and an unprotected HTTPRoute claim that path and + the effective auth posture is undefined. To turn gateway auth off entirely, + set auth.enforceAtGateway: false (keep provisioning the Keycloak client) or + auth.enabled: false (no client at all) instead of listing every path here. items: description: RouteMatch defines a path-based routing rule. properties: diff --git a/docs/api-reference.md b/docs/api-reference.md index c8ca244..e5a97e9 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -41,7 +41,7 @@ _Appears in:_ | `scopes` _string array_ | Scopes defines the OIDC scopes to request during authentication.
Common scopes: openid, profile, email, roles, groups
If not specified, defaults to: ["openid", "profile", "email"] | | Optional: \{\}
| | `groups` _string array_ | Groups specifies the list of groups that should have access to this application.
When specified, only users belonging to these groups will be authorized.
Group matching is case-sensitive and depends on the OIDC provider's group claim. | | Optional: \{\}
| | `provisionClient` _boolean_ | ProvisionClient determines whether the operator should automatically provision
an OIDC client in the provider. When true, the operator will create a client
(e.g., in Keycloak) and store the credentials in a Secret.
Only supported for provider="keycloak".
Defaults to true if not specified. | true | Optional: \{\}
| -| `enforceAtGateway` _boolean_ | EnforceAtGateway determines whether the operator should create an Envoy Gateway
SecurityPolicy to enforce authentication at the gateway level.
When true (default), the operator creates a SecurityPolicy that handles
the OIDC flow at the gateway before requests reach the application.
When false, the operator provisions the OIDC client and stores credentials
in a Secret, but does NOT create a SecurityPolicy - the application is
expected to handle OAuth natively (e.g., Grafana's built-in generic_oauth). | true | Optional: \{\}
| +| `enforceAtGateway` _boolean_ | EnforceAtGateway determines whether the operator should create an Envoy Gateway
SecurityPolicy to enforce authentication at the gateway level.
When true (default), the operator creates a SecurityPolicy that handles
the OIDC flow at the gateway before requests reach the application.
When false, the operator provisions the OIDC client and stores credentials
in a Secret, but does NOT create a SecurityPolicy - the application is
expected to handle OAuth natively (e.g., Grafana's built-in generic_oauth).
This is the correct switch for "no gateway-level auth"; do not try to
achieve that by listing every path under routing.publicRoutes, which
instead produces overlapping protected and unprotected routes for the same
path (see PublicRoutes). | true | Optional: \{\}
| | `forwardAccessToken` _boolean_ | ForwardAccessToken instructs the gateway-enforced OIDC filter to forward
the user's OAuth2 access token to the upstream service via the
`Authorization: Bearer ` header. Use this when the application
needs to read the JWT itself - for example to extract the user's groups
claim and apply per-user authorization decisions on top of the gateway's
authentication. By default the gateway only stores the token in an
encrypted session cookie that backends cannot decode.
Only applies when enforceAtGateway is true. | | Optional: \{\}
| | `denyRedirect` _[DenyRedirectHeader](#denyredirectheader) array_ | DenyRedirect configures headers that, when matched, prevent the OIDC filter
from redirecting to the identity provider. Instead, matching requests receive
a 401 response. This prevents PKCE race conditions when SPAs fire multiple
requests on page load (e.g., the main page and AJAX calls simultaneously),
each of which would otherwise start a separate OAuth flow and overwrite
each other's state cookies.
Only applies when enforceAtGateway is true. | | Optional: \{\}
| | `issuerURL` _string_ | IssuerURL specifies the OIDC issuer URL for generic-oidc provider.
Required when provider="generic-oidc", ignored for other providers.
Example: https://accounts.google.com, https://login.microsoftonline.com//v2.0 | | Optional: \{\}
| @@ -309,9 +309,10 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `routes` _[RouteMatch](#routematch) array_ | Routes defines path-based routing rules for the application.
If not specified, all traffic to the hostname will be routed to the service.
When specified, only traffic matching these path prefixes will be routed.
Example: ["/app-1", "/api/v1"] | | Optional: \{\}
| -| `publicRoutes` _[RouteMatch](#routematch) array_ | PublicRoutes specifies paths that should bypass OIDC authentication.
When auth is enabled and these are specified, these paths will be routed
via a separate HTTPRoute that is not protected by the SecurityPolicy.
Each entry uses the same RouteMatch format as routes, supporting both
PathPrefix (default) and Exact matching via the pathType field.
Example: [\{pathPrefix: "/api/v1/health", pathType: "Exact"\}] | | Optional: \{\}
| +| `publicRoutes` _[RouteMatch](#routematch) array_ | PublicRoutes specifies paths that should bypass OIDC authentication.
When auth is enabled and these are specified, these paths will be routed
via a separate HTTPRoute that is not protected by the SecurityPolicy.
Each entry uses the same RouteMatch format as routes, supporting both
PathPrefix (default) and Exact matching via the pathType field.
Example: [\{pathPrefix: "/api/v1/health", pathType: "Exact"\}]
PublicRoutes is for carving out a *subset* of paths that should bypass
auth while the rest stays protected. It is not a way to disable auth for
the whole app: if a public path also matches the main route (including the
default "/" match when routes is empty) while auth.enforceAtGateway is
true, both a protected and an unprotected HTTPRoute claim that path and
the effective auth posture is undefined. To turn gateway auth off entirely,
set auth.enforceAtGateway: false (keep provisioning the Keycloak client) or
auth.enabled: false (no client at all) instead of listing every path here. | | Optional: \{\}
| | `tls` _[RoutingTLSConfig](#routingtlsconfig)_ | TLS configures TLS certificate management and termination behavior.
When TLS is enabled (the default), the operator creates a cert-manager Certificate
for the application's hostname and adds a per-app HTTPS listener to the shared Gateway. | | Optional: \{\}
| | `annotations` _object (keys:string, values:string)_ | Annotations defines additional annotations to merge onto the generated HTTPRoute.
Useful for tools like ArgoCD that track resources via annotations
(e.g. argocd.argoproj.io/tracking-id).
These annotations are merged with any operator-managed annotations; operator
annotations always take precedence to avoid breaking internal behaviour. | | Optional: \{\}
| +| `filters` _HTTPRouteFilter array_ | Filters are appended, in order, to every HTTPRouteRule the operator
generates for this NebariApp (both the protected route and the public
route built from publicRoutes). This is a pass-through to the upstream
gateway.networking.k8s.io/v1 HTTPRouteFilter spec, mirroring the
Annotations escape hatch: the operator does not interpret, reorder, or
strip the filters, it only forwards them. Use it to adjust request or
response headers at the gateway (for example, stripping an overpermissive
CORS header an upstream sets, or adding security headers) without forking
the operator or rebuilding the backend image.
The field is schemaless in the CRD: inlining the full HTTPRouteFilter
OpenAPI schema would copy its nested x-kubernetes-validations (CEL) rules
into the NebariApp CRD and blow the per-CRD CEL cost budget. As a
pass-through, the filters are instead validated by Gateway API when the
operator applies them to the generated HTTPRoute. Each entry must still be
a valid HTTPRouteFilter object (it is decoded into the typed Go struct). | | Schemaless: \{\}
Optional: \{\}
| --- diff --git a/docs/reconcilers/routing.md b/docs/reconcilers/routing.md index 5e83b0a..0c1bc35 100644 --- a/docs/reconcilers/routing.md +++ b/docs/reconcilers/routing.md @@ -330,12 +330,50 @@ spec: ``` **Key details:** -- Public routes always use `PathPrefix` matching +- Public routes default to `Exact` matching (safer for auth bypass); set `pathType: PathPrefix` per entry to widen - The public HTTPRoute has the label `nebari.dev/route-type: public` - Both routes point to the same backend service - If auth is disabled, no public route is created (all routes are already public) - Cleanup removes both HTTPRoutes when the NebariApp is deleted +> **`publicRoutes` is for a subset of paths, not "disable auth".** It carves +> specific paths out of the authenticated app. If a public path also matches the +> main route (including the default `/` match when `routes` is empty) while +> `auth.enforceAtGateway` is `true`, both a protected and an unprotected HTTPRoute +> claim that path and the effective auth posture is left to Gateway API conflict +> resolution. To turn gateway auth off for the whole app, set +> `auth.enforceAtGateway: false` (keep provisioning the Keycloak client) or +> `auth.enabled: false` (no client at all) — do not list every path under +> `publicRoutes`. + +### Route Filters + +`spec.routing.filters` is a pass-through to the upstream Gateway API +[`HTTPRouteFilter`](https://gateway-api.sigs.k8s.io/api-types/httproute/#filters) +list. The operator appends the filters you supply, in order, to the rule of +**both** the protected and the public HTTPRoute; it does not interpret, reorder, +or strip them. This mirrors the `annotations` escape hatch and is the supported +way to adjust request/response headers at the gateway without forking the +operator or rebuilding the backend image — for example, stripping an +overpermissive CORS header an upstream sets, or adding security headers. + +```yaml +spec: + routing: + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + remove: + - Access-Control-Allow-Credentials + set: + - name: Strict-Transport-Security + value: "max-age=63072000; includeSubDomains" +``` + +Because the operator owns the generated HTTPRoute (`ownerReferences.controller: +true`), a manual `kubectl patch` of these fields is reverted on the next +reconcile — `routing.filters` is the durable way to express them. + ### Backend References The operator creates backend references using the service details from the NebariApp spec: diff --git a/internal/controller/reconcilers/auth/providers/keycloak.go b/internal/controller/reconcilers/auth/providers/keycloak.go index 71fba4e..140ef02 100644 --- a/internal/controller/reconcilers/auth/providers/keycloak.go +++ b/internal/controller/reconcilers/auth/providers/keycloak.go @@ -614,7 +614,7 @@ func (p *KeycloakProvider) updateExistingClient(ctx context.Context, kcClient *g // Update client configuration redirectURIs := p.buildRedirectURLs(nebariApp) existingClient.RedirectURIs = &redirectURIs - existingClient.WebOrigins = &[]string{"*"} + existingClient.WebOrigins = ptr.To(p.buildWebOrigins(nebariApp)) existingClient.StandardFlowEnabled = gocloak.BoolP(true) // Ensure post-logout redirect URIs are set (preserving any existing attributes) @@ -653,7 +653,7 @@ func (p *KeycloakProvider) createNewClient(ctx context.Context, kcClient *gocloa Name: gocloak.StringP(fmt.Sprintf("%s OIDC Client", nebariApp.Name)), Secret: gocloak.StringP(clientSecret), RedirectURIs: &redirectURIs, - WebOrigins: &[]string{"*"}, + WebOrigins: ptr.To(p.buildWebOrigins(nebariApp)), Attributes: &map[string]string{"post.logout.redirect.uris": p.buildPostLogoutRedirectURIs(nebariApp)}, PublicClient: gocloak.BoolP(false), StandardFlowEnabled: gocloak.BoolP(true), @@ -683,6 +683,18 @@ func (p *KeycloakProvider) buildRedirectURLs(nebariApp *appsv1.NebariApp) []stri } } +// buildWebOrigins constructs the CORS WebOrigins for the client, scoped to the +// NebariApp's hostname (both schemes) rather than a "*" wildcard. This mirrors +// the host-scoping already applied to RedirectURIs and keeps the client to the +// principle of least privilege: only the app's own origin may make CORS requests +// to Keycloak's token endpoint for this client. +func (p *KeycloakProvider) buildWebOrigins(nebariApp *appsv1.NebariApp) []string { + return []string{ + fmt.Sprintf("https://%s", nebariApp.Spec.Hostname), + fmt.Sprintf("http://%s", nebariApp.Spec.Hostname), + } +} + // buildPostLogoutRedirectURIs constructs the Keycloak post.logout.redirect.uris attribute value. // Keycloak stores multiple URIs as "##"-delimited strings in this client attribute. // Envoy Gateway sends the app's base URL as post_logout_redirect_uri when hitting /logout. @@ -1107,7 +1119,7 @@ func (p *KeycloakProvider) provisionSPAClient(ctx context.Context, kcClient *goc if existingSPAClient != nil { // Update existing SPA client existingSPAClient.RedirectURIs = &redirectURIs - existingSPAClient.WebOrigins = &[]string{"*"} + existingSPAClient.WebOrigins = ptr.To(p.buildWebOrigins(nebariApp)) existingSPAClient.PublicClient = gocloak.BoolP(true) existingSPAClient.StandardFlowEnabled = gocloak.BoolP(true) @@ -1128,7 +1140,7 @@ func (p *KeycloakProvider) provisionSPAClient(ctx context.Context, kcClient *goc ClientID: gocloak.StringP(spaClientID), Name: gocloak.StringP(fmt.Sprintf("%s SPA Client", nebariApp.Name)), RedirectURIs: &redirectURIs, - WebOrigins: &[]string{"*"}, + WebOrigins: ptr.To(p.buildWebOrigins(nebariApp)), PublicClient: gocloak.BoolP(true), StandardFlowEnabled: gocloak.BoolP(true), DirectAccessGrantsEnabled: gocloak.BoolP(false), diff --git a/internal/controller/reconcilers/auth/providers/keycloak_test.go b/internal/controller/reconcilers/auth/providers/keycloak_test.go index 51f6bdd..625d3e7 100644 --- a/internal/controller/reconcilers/auth/providers/keycloak_test.go +++ b/internal/controller/reconcilers/auth/providers/keycloak_test.go @@ -344,6 +344,36 @@ func TestKeycloakProvider_BuildRedirectURLs(t *testing.T) { } } +// TestKeycloakProvider_BuildWebOrigins asserts that the client's CORS WebOrigins +// are scoped to the NebariApp hostname (both schemes) rather than a "*" wildcard +// (see #37). +func TestKeycloakProvider_BuildWebOrigins(t *testing.T) { + provider := &KeycloakProvider{Config: config.KeycloakConfig{}} + + nebariApp := &appsv1.NebariApp{ + ObjectMeta: metav1.ObjectMeta{Name: "test-app", Namespace: "default"}, + Spec: appsv1.NebariAppSpec{ + Hostname: "test.example.com", + Auth: &appsv1.AuthConfig{Enabled: true}, + }, + } + + got := provider.buildWebOrigins(nebariApp) + want := []string{"https://test.example.com", "http://test.example.com"} + + if len(got) != len(want) { + t.Fatalf("expected %d origins, got %d: %v", len(want), len(got), got) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("origin[%d]: expected %s, got %s", i, want[i], got[i]) + } + if got[i] == "*" { + t.Errorf("origin[%d] is the wildcard '*'; WebOrigins must be host-scoped", i) + } + } +} + func TestKeycloakProvider_StoreClientSecret(t *testing.T) { scheme := runtime.NewScheme() _ = corev1.AddToScheme(scheme) diff --git a/internal/controller/reconcilers/routing/httproute.go b/internal/controller/reconcilers/routing/httproute.go index 14bd12e..9e1d6a7 100644 --- a/internal/controller/reconcilers/routing/httproute.go +++ b/internal/controller/reconcilers/routing/httproute.go @@ -163,17 +163,8 @@ func (r *RoutingReconciler) buildHTTPRoute(nebariApp *appsv1.NebariApp, gatewayN routeName := naming.HTTPRouteName(nebariApp) namespace := gatewayv1.Namespace(constants.GatewayNamespace) - // Determine which Gateway listener to use - // Priority: tlsListenerName (from TLS reconciler) > TLS enabled ("https") > TLS disabled ("http") - sectionName := gatewayv1.SectionName("https") - tlsEnabled := true - if nebariApp.Spec.Routing != nil && nebariApp.Spec.Routing.TLS != nil && nebariApp.Spec.Routing.TLS.Enabled != nil && !*nebariApp.Spec.Routing.TLS.Enabled { - sectionName = gatewayv1.SectionName("http") - tlsEnabled = false - } - if tlsListenerName != "" && tlsEnabled { - sectionName = gatewayv1.SectionName(tlsListenerName) - } + // Determine which Gateway listener to use. + sectionName, tlsEnabled := resolveListenerSection(nebariApp, tlsListenerName) // Build HTTPRoute annotations: start with user-supplied annotations from the // routing spec, then apply operator-managed ones so they always take precedence. @@ -221,41 +212,82 @@ func (r *RoutingReconciler) buildHTTPRoute(nebariApp *appsv1.NebariApp, gatewayN return route, nil } -// buildHTTPRouteRules generates HTTPRoute rules based on NebariApp routes +// buildHTTPRouteRules generates the HTTPRoute rule for the protected (main) +// route. Matches come from routing.routes and default to PathPrefix; when none +// are specified the matches slice is empty and Gateway API applies its own +// default "/" (PathPrefix) match. 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 } + return []gatewayv1.HTTPRouteRule{ + r.buildRouteRule(nebariApp, buildRouteMatches(routes, gatewayv1.PathMatchPathPrefix)), + } +} + +// buildRouteRule assembles a single HTTPRouteRule from the given matches, +// attaching the shared backend refs and any user-supplied routing.filters. +// Both the protected route and the public route flow through here, so filters +// apply to each consistently and in the order the user listed them. +func (r *RoutingReconciler) buildRouteRule(nebariApp *appsv1.NebariApp, matches []gatewayv1.HTTPRouteMatch) gatewayv1.HTTPRouteRule { + return gatewayv1.HTTPRouteRule{ + Matches: matches, + BackendRefs: r.buildBackendRefs(nebariApp), + Filters: userFilters(nebariApp), + } +} - // 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. +// buildRouteMatches converts NebariApp RouteMatch entries into Gateway API path +// matches. defaultPathType is applied when an entry omits pathType, letting the +// main route default to PathPrefix and public routes to Exact. An empty input +// yields an empty slice so callers can rely on Gateway API's default "/" match. +func buildRouteMatches(routes []appsv1.RouteMatch, defaultPathType gatewayv1.PathMatchType) []gatewayv1.HTTPRouteMatch { matches := make([]gatewayv1.HTTPRouteMatch, 0, len(routes)) for _, route := range routes { - pathType := gatewayv1.PathMatchPathPrefix - if route.PathType == "Exact" { + pathType := defaultPathType + switch route.PathType { + case "Exact": pathType = gatewayv1.PathMatchExact + case "PathPrefix": + pathType = gatewayv1.PathMatchPathPrefix } - pathValue := route.PathPrefix - match := gatewayv1.HTTPRouteMatch{ + matches = append(matches, gatewayv1.HTTPRouteMatch{ Path: &gatewayv1.HTTPPathMatch{ Type: &pathType, Value: &pathValue, }, - } - matches = append(matches, match) + }) } + return matches +} - return []gatewayv1.HTTPRouteRule{ - { - Matches: matches, - BackendRefs: r.buildBackendRefs(nebariApp), - }, +// userFilters returns the user-supplied routing.filters for this NebariApp, or +// nil when none are configured. Pass-through to the Gateway API HTTPRouteFilter +// spec: the operator does not interpret, reorder, or strip them. +func userFilters(nebariApp *appsv1.NebariApp) []gatewayv1.HTTPRouteFilter { + if nebariApp.Spec.Routing == nil || len(nebariApp.Spec.Routing.Filters) == 0 { + return nil } + return nebariApp.Spec.Routing.Filters +} + +// resolveListenerSection determines which Gateway listener section the route +// attaches to, and whether TLS is enabled. Priority: the per-app TLS listener +// from the TLS reconciler (when TLS is on) > "https" (TLS on) > "http" (TLS +// explicitly disabled). +func resolveListenerSection(nebariApp *appsv1.NebariApp, tlsListenerName string) (gatewayv1.SectionName, bool) { + sectionName := gatewayv1.SectionName("https") + tlsEnabled := true + if nebariApp.Spec.Routing != nil && nebariApp.Spec.Routing.TLS != nil && nebariApp.Spec.Routing.TLS.Enabled != nil && !*nebariApp.Spec.Routing.TLS.Enabled { + sectionName = gatewayv1.SectionName("http") + tlsEnabled = false + } + if tlsListenerName != "" && tlsEnabled { + sectionName = gatewayv1.SectionName(tlsListenerName) + } + return sectionName, tlsEnabled } // buildBackendRefs generates backend references for the HTTPRoute @@ -393,31 +425,11 @@ func (r *RoutingReconciler) buildPublicHTTPRoute(nebariApp *appsv1.NebariApp, ga routeName := naming.PublicHTTPRouteName(nebariApp) namespace := gatewayv1.Namespace(constants.GatewayNamespace) - sectionName := gatewayv1.SectionName("https") - tlsEnabled := true - if nebariApp.Spec.Routing != nil && nebariApp.Spec.Routing.TLS != nil && nebariApp.Spec.Routing.TLS.Enabled != nil && !*nebariApp.Spec.Routing.TLS.Enabled { - sectionName = gatewayv1.SectionName("http") - tlsEnabled = false - } - if tlsListenerName != "" && tlsEnabled { - sectionName = gatewayv1.SectionName(tlsListenerName) - } + sectionName, tlsEnabled := resolveListenerSection(nebariApp, tlsListenerName) - // Build matches for each public route (default to Exact for safer auth bypass) - matches := make([]gatewayv1.HTTPRouteMatch, 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, - }, - }) - } + // Public routes default to Exact matching (safer for auth bypass than a broad + // prefix). User-supplied routing.filters apply here too, via buildRouteRule. + matches := buildRouteMatches(nebariApp.Spec.Routing.PublicRoutes, gatewayv1.PathMatchExact) route := &gatewayv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ @@ -447,10 +459,7 @@ func (r *RoutingReconciler) buildPublicHTTPRoute(nebariApp *appsv1.NebariApp, ga gatewayv1.Hostname(nebariApp.Spec.Hostname), }, Rules: []gatewayv1.HTTPRouteRule{ - { - Matches: matches, - BackendRefs: r.buildBackendRefs(nebariApp), - }, + r.buildRouteRule(nebariApp, matches), }, }, } diff --git a/internal/controller/reconcilers/routing/httproute_test.go b/internal/controller/reconcilers/routing/httproute_test.go index a828318..4f73795 100644 --- a/internal/controller/reconcilers/routing/httproute_test.go +++ b/internal/controller/reconcilers/routing/httproute_test.go @@ -18,6 +18,7 @@ package routing import ( "context" + "reflect" "testing" corev1 "k8s.io/api/core/v1" @@ -406,6 +407,86 @@ func TestBuildHTTPRouteRules(t *testing.T) { } } +// TestUserFiltersPropagateToRoutes asserts that user-supplied routing.filters are +// appended, in order, to both the protected and the public HTTPRoute (see #137). +func TestUserFiltersPropagateToRoutes(t *testing.T) { + scheme := runtime.NewScheme() + _ = appsv1.AddToScheme(scheme) + _ = gatewayv1.Install(scheme) + + reconciler := &RoutingReconciler{Scheme: scheme} + + filters := []gatewayv1.HTTPRouteFilter{ + { + Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, + ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Remove: []string{"Access-Control-Allow-Credentials"}, + }, + }, + { + Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Add: []gatewayv1.HTTPHeader{{Name: "X-Forwarded-Tenant", Value: "acme"}}, + }, + }, + } + + nebariApp := &appsv1.NebariApp{ + ObjectMeta: metav1.ObjectMeta{Name: "test-app", Namespace: "default"}, + Spec: appsv1.NebariAppSpec{ + Hostname: "test.example.com", + Service: appsv1.ServiceReference{Name: "test-service", Port: 8080}, + Routing: &appsv1.RoutingConfig{ + Filters: filters, + PublicRoutes: []appsv1.RouteMatch{{PathPrefix: "/healthz", PathType: "Exact"}}, + }, + }, + } + + mainRoute, err := reconciler.buildHTTPRoute(nebariApp, "nebari-gateway", "") + if err != nil { + t.Fatalf("buildHTTPRoute returned error: %v", err) + } + publicRoute, err := reconciler.buildPublicHTTPRoute(nebariApp, "nebari-gateway", "") + if err != nil { + t.Fatalf("buildPublicHTTPRoute returned error: %v", err) + } + + for _, tc := range []struct { + name string + route *gatewayv1.HTTPRoute + }{ + {"main", mainRoute}, + {"public", publicRoute}, + } { + if len(tc.route.Spec.Rules) != 1 { + t.Fatalf("%s route: expected 1 rule, got %d", tc.name, len(tc.route.Spec.Rules)) + } + got := tc.route.Spec.Rules[0].Filters + if !reflect.DeepEqual(got, filters) { + t.Errorf("%s route: filters not propagated in order.\n got: %#v\nwant: %#v", tc.name, got, filters) + } + } +} + +// TestUserFiltersAbsentWhenUnset asserts the rule carries no filters when the +// user configures none. +func TestUserFiltersAbsentWhenUnset(t *testing.T) { + reconciler := &RoutingReconciler{Scheme: runtime.NewScheme()} + nebariApp := &appsv1.NebariApp{ + Spec: appsv1.NebariAppSpec{ + Service: appsv1.ServiceReference{Name: "svc", Port: 80}, + }, + } + rules := reconciler.buildHTTPRouteRules(nebariApp) + if len(rules) != 1 { + t.Fatalf("expected 1 rule, got %d", len(rules)) + } + if len(rules[0].Filters) != 0 { + t.Errorf("expected no filters, got %d", len(rules[0].Filters)) + } +} + func TestReconcileRouting(t *testing.T) { scheme := runtime.NewScheme() _ = appsv1.AddToScheme(scheme)