[release-v3.30] fix(dikastes): register ALPCheckProvider to restore L7 policy enforcement#12004
[release-v3.30] fix(dikastes): register ALPCheckProvider to restore L7 policy enforcement#12004electricjesus wants to merge 3 commits intorelease-v3.30from
Conversation
…ment The staged policies refactoring (931244d) introduced a CheckProvider plugin interface but never registered a provider in OSS. This caused all ext_authz Check requests to return UNKNOWN, breaking L7 application layer policy for all OSS users since v3.30.0. Add an ALPCheckProvider that wraps the existing checkStore logic and register it in dikastes, restoring policy evaluation for the ext_authz Check RPC. Fixes #11857
…tests Extract the check server setup (provider registration + gRPC service registration) into newCheckServer(), called by both runServer() and the FV tests. This ensures the tests exercise the exact same wiring as production, so removing a provider from newCheckServer will fail FV tests immediately. Also uses the existing RegisterGRPCServices() method, removing the need for the v2/v2alpha authz imports from dikastes.go.
electricjesus
added
release-note-required
There was a problem hiding this comment.
Pull request overview
Restores dikastes L7 application-layer policy (ALP) enforcement on the release-v3.30 branch by registering the missing ALPCheckProvider, and adds end-to-end FV coverage to prevent regressions.
Changes:
- Register
ALPCheckProvidervia a sharednewCheckServer()wiring function used by both production and tests. - Add a new
ALPCheckProviderimplementation that delegates to existingcheckStoreevaluation. - Add FV tests that exercise the full Felix sync → policy store → ext_authz
CheckgRPC flow.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app-policy/cmd/dikastes/dikastes.go | Centralizes ext_authz server/provider registration in newCheckServer() and removes redundant v2/v2alpha wiring. |
| app-policy/cmd/dikastes/dikastes_fv_test.go | Adds FV tests covering allow/deny and HTTP method L7 matching through real gRPC + UDS plumbing. |
| app-policy/checker/alp_check_provider.go | Introduces ALPCheckProvider to evaluate ALP using existing policy store logic. |
| app-policy/checker/alp_check_provider_test.go | Unit tests for ALPCheckProvider behavior. |
| app-policy/checker/server_test.go | Updates server tests to register providers and validate expected status codes. |
| func (e *dikastesTestEnv) cleanup() { | ||
| _ = e.authzConn.Close() | ||
| e.dikastesGRPC.GracefulStop() | ||
| _ = os.RemoveAll(e.socketDir) | ||
| } |
There was a problem hiding this comment.
dikastesTestEnv.cleanup() stops the dikastes gRPC server but never stops the fake Felix PolicySync gRPC server (testSyncServer.grpcServer) or closes its listener. That can leak goroutines/file descriptors across tests and makes failures harder to diagnose. Consider adding a Stop/GracefulStop method on testSyncServer and invoking it from cleanup (and closing the listener).
| grpcServer *grpc.Server | ||
| listener net.Listener | ||
| cLock sync.Mutex | ||
| cancelFns []func() | ||
| } |
There was a problem hiding this comment.
testSyncServer keeps cLock and cancelFns state that is only ever appended to in Sync() and never used elsewhere. If restart/connection-cancel behavior isn't needed in these FV tests, removing this unused state would simplify the fake server; otherwise, wire it into cleanup/stop logic so it has an effect.
On release-v3.30, TierInfo.IngressPolicies is []string (not []*PolicyID) and Policy has no Tier field. Update the FV test to match the proto types on this branch.
Cherry-pick of #11986 to
release-v3.30.The staged policies refactoring (931244d, #9804) introduced a
CheckProviderplugin interface in dikastes but never registered a provider in OSS. This caused all ext_authz Check requests to return UNKNOWN, which Envoy treats as a denial — breaking L7 application layer policy for all OSS users since v3.30.0.This PR adds an
ALPCheckProviderthat wraps the existingcheckStorelogic and registers it in dikastes, restoring policy evaluation for the ext_authz Check RPC.Fixes #11857
Release note: