feat: routing.filters pass-through, host-scoped WebOrigins, unified route builders#141
Open
viniciusdc wants to merge 3 commits into
Open
feat: routing.filters pass-through, host-scoped WebOrigins, unified route builders#141viniciusdc wants to merge 3 commits into
viniciusdc wants to merge 3 commits into
Conversation
…oute builders Unify the protected and public HTTPRoute builders onto a shared path, then build the additive changes on top so they land in one place for both routes. Refactor (routing/httproute.go): - Extract resolveListenerSection (TLS listener/section resolution), buildRouteMatches (path-match construction, parameterized by the default pathType so the main route stays PathPrefix and public routes stay Exact), and buildRouteRule (single rule with backend refs + filters). The main and public builders were ~95% duplicated; they now share these helpers. routing.filters pass-through: - Add RoutingConfig.Filters []gatewayv1.HTTPRouteFilter, appended in order to the rule of both the protected and public HTTPRoute via buildRouteRule. The operator does not interpret, reorder, or strip them -- same escape-hatch pattern as the existing Annotations field. Lets deployers fix gateway-side header issues (e.g. an upstream's invalid CORS header combo) without forking the operator or rebuilding the backend image. Host-scoped Keycloak WebOrigins: - Replace the wildcard WebOrigins ["*"] with the NebariApp hostname (both schemes) on all four client paths (main client create/update, SPA client create/update), via a buildWebOrigins helper mirroring buildRedirectURLs. Keeps the OIDC client to least privilege for CORS to the token endpoint. publicRoutes vs enforceAtGateway docs: - Cross-reference the two fields in the CRD comments and routing.md, calling out that publicRoutes carves out a subset of paths and is not the way to disable auth for the whole app (use enforceAtGateway: false). The reconciler/admission guard for the overlap case is intentionally left out of this change -- Gateway API precedence makes most overlaps intended, so the guard needs a precedence-aware design pass. Regenerated zz_generated.deepcopy.go, the CRD, and docs/api-reference.md. New unit tests cover filter propagation+ordering on both routes and the host-scoped WebOrigins. Closes #137 Closes #37
…budget Inlining the full gateway.networking.k8s.io/v1 HTTPRouteFilter OpenAPI schema copied its nested x-kubernetes-validations (CEL) rules -- notably the cors filter's allowOrigins/allowMethods and the externalAuth/requestMirror backendRef rules -- into the NebariApp CRD. That pushed the per-CRD CEL cost total past the Kubernetes budget (~1.22x over, with cors.allowOrigins alone 7.2x over its per-rule limit), so the CRD failed to install and the E2E, Helm, and install checks all failed. Mark Filters as Schemaless + PreserveUnknownFields (Type=array) so controller-gen emits a permissive array schema with no nested CEL. The Go type stays []gatewayv1.HTTPRouteFilter, so each entry is still decoded into the typed struct and applied to the generated HTTPRoute, where Gateway API validates it -- the pass-through contract is unchanged. Shrinks the generated CRD by ~1300 lines.
The previous attempt set Type=array together with Schemaless, which produced
a `type: array` node with no `items` -- a structural schema requires `items`
for arrays, so the CRD still failed to install ("filters.items: Required
value: must be specified").
Drop the explicit Type marker and keep Schemaless + PreserveUnknownFields, so
controller-gen emits the field as `{x-kubernetes-preserve-unknown-fields: true}`
(the canonical "arbitrary content" schema) with no type constraint and no nested
CEL. Verified by installing the regenerated CRD into a real apiserver via
envtest: CRD installation now succeeds.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 2 of the routing/SecurityPolicy consolidation (#139). Unifies the protected and public HTTPRoute builders onto a shared path, then builds the additive changes on top so they land in one place for both routes.
Refactor first
buildHTTPRoute/buildPublicHTTPRouteand their rule construction were ~95% duplicated. Extracted three shared helpers inrouting/httproute.go:resolveListenerSection— TLS listener/section resolution (was copy-pasted in both builders)buildRouteMatches— path-match construction, parameterized by the defaultpathTypeso the main route staysPathPrefixand public routes stayExactbuildRouteRule— a single rule with backend refs + filters, used by both routesrouting.filterspass-through (closes #137)RoutingConfig.Filters []gatewayv1.HTTPRouteFilter, appended in order to the rule of both the protected and public HTTPRoute viabuildRouteRule. The operator does not interpret, reorder, or strip them — same escape-hatch contract as the existingAnnotationsfield.Allow-Credentials: true+Allow-Origin: *combo) without forking the operator or rebuilding the backend image. The originating backend bug was fixed upstream in https://redirect.github.com/nebari-dev/nebi/pull/385; this field is the general escape hatch for the next one.routing.corshalf of the original Routing: expose HTTPRouteFilter pass-through on generated HTTPRoutes #137 was split to Routing: curated CORS block on the generated SecurityPolicy #138 and is not in this PR.Host-scoped Keycloak WebOrigins (closes #37)
WebOrigins: ["*"]with the NebariApp hostname (both schemes) on all four client paths — main client create/update and SPA client create/update — via abuildWebOriginshelper mirroringbuildRedirectURLs. Keeps the OIDC client to least privilege for CORS to the token endpoint.publicRoutesvsenforceAtGatewaydocs (partial #118)routing.md, calling out thatpublicRoutescarves out a subset of paths and is not the way to disable auth for the whole app (useenforceAtGateway: false).Exact> longer-prefix > shorter-prefix) makes most "overlaps" the intended behavior (the Pattern A use ofpublicRoutes), so a CEL/webhook guard risks rejecting valid configs. The guard needs a precedence-aware design pass; NebariApp: publicRoutes + default enforceAtGateway produces two competing HTTPRoutes for the same path #118 stays open for it. The docs (the issue's "at minimum") land here.Generated
zz_generated.deepcopy.go, the CRD, anddocs/api-reference.mdregenerated. The CRD grows substantially because thefiltersfield inlines the full upstreamHTTPRouteFilterschema — expected for a pass-through.Test plan
go build ./...,go vet ./...,gofmt -lclean.go testacross all unit packages (excl. e2e) passes, including:TestUserFiltersPropagateToRoutes— filters appear, in order, on both the protected and public routeTestUserFiltersAbsentWhenUnset— no filters when unconfiguredTestKeycloakProvider_BuildWebOrigins— host-scoped, no wildcardinternal/controllerenvtest suite fails locally on a pre-existing scheme-registration gap (also fails on cleanmain; needsmake test's CRD setup) — unrelated to this change.