Hi all, I (also) think the OPA module needs * unit tests (as they are there) * integration tests using the "opa" testcontainer, but in the polaris-extensions-auth-opa module. I don't see integration tests that actually require something "from Polaris". * a "smoke integration test" in polaris-runtime-service, also using the "opa" testcontainer, but have the OPA dependencies as int-test-only * have a runtimeOnly() dependency to OPA in polaris-runtime-server
Having a separate module for integration tests that requires a separate Quarkus build slows down CI quite a bit. On Wed, Oct 15, 2025 at 12:43 AM Dmitri Bourlatchkov <[email protected]> wrote: > > Hi Yufei, Sung, > > I agree with Yufei's point about the awkward relationship between > runtime-service and OPA extension that current PR (and layout option 3 in > the doc) causes. > > However, I do not agree that this can be resolved in a follow-up PR. I > believe it is preferable to ensure proper isolation of concerns upfront as > it will affect downstream projects that build on top of runtime-service > (which is a centre piece for all REST APIs). > > I wonder what specific issues prevent running integration tests inside the > extension modules. > > For example: > extension/auth/opa > - impl (includes unit tests without extra deps) > - src/main/java, src/test/java, etc... We can have more advanced > "intTests" using OPA containers here (if required), but no Polaris runtime > infrastructure. > > - tests (depends on both impl and runtime-service and contains end-to-end > tests to validate integration into Polaris runtime services) > - src/intTests/java, test fixtures, etc. > > Runtime-service does not get any new deps. Runtime-server (Quarkus) > includes extension/auth/opa/impl as a runtime dependency (but does not run > any new tests). There is a slight coverage gap here that we have to trust > Quarkus to do the same thing with OPA CDI beans here that was validated > in extension/auth/opa/tests, but I think we can actually trust Quarkus on > that :) > > Note: Runtime-server is not meant for reuse downstream (unlike > runtime-service), it makes the server which is released with the usual > Apache Polaris binary artifacts. > > WDYT? > > Thanks, > Dmitri. > > On Tue, Oct 14, 2025 at 2:42 PM Yufei Gu <[email protected]> wrote: > > > Hi Sung, > > > > Thanks a lot for the write-up! I’m good with the separate modules approach. > > Just two concerns to flag: > > > > 1. The current integration tests still reside in the runtime-service > > module, which depends on the OPA module. This might force downstream users > > who aren’t using OPA to disable those tests manually. > > 2. Deeper integration could involve additional changes in the service > > module. For example, grant/revoke operations might need to interact with > > the OPA service. We'll need to ensure that integration works smoothly, > > possibly by introducing an interface and injecting an OPA-specific > > implementation for grant/revoke persistence. > > > > That said, the PR is looking solid. We can resolve these concerns in follow > > ups. I’ll take one more pass, but it should be good to go. > > > > > > Yufei > > > > > > On Sat, Oct 11, 2025 at 5:23 PM Sung Yun <[email protected]> wrote: > > > > > Hi everyone, > > > > > > Thanks for all the thoughtful feedback on the OpaPolarisAuthorizer RFC[1] > > > and PR[2]. There’s been a healthy discussion around the project > > structure, > > > so I’ve added an RFC addendum summarizing the options and trade-offs to > > > help us reach consensus and move the PR forward. > > > > > > My recommendation is Option 3, a split approach where the OPA > > > implementation lives in polaris-extensions-auth-opa and integration tests > > > in polaris-runtime-services. > > > > > > This keeps the architecture extensible and avoids circular dependencies. > > > Most OPA logic will stay outside core for now, making it easy to migrate > > > later if we decide to adopt it as a core component. > > > > > > Please take a look at the RFC addendum and PR #2680 and share any final > > > thoughts. If there’s general agreement, I’d like to proceed with merging > > > and start gathering real-world feedback. > > > > > > Thanks again for all the reviews and feedback! > > > > > > Cheers, > > > Sung > > > > > > [1] > > > > > https://docs.google.com/document/d/1HadMFygjbuZathZZPanO6cFVorx0Ju0FopkICxX1tCE/edit?tab=t.0#heading=h.o1ltugri8fl3 > > > [2] https://github.com/apache/polaris/pull/2680 > > > > > > On 2025/10/08 22:17:58 Yufei Gu wrote: > > > > Sung, thanks for working on this. Did one round of review. > > > > > > > > Dimitri, it makes sense to mark it as "beta". It's a big feature, we > > > > couldn't finish it in one PR. We could focus on the first PR, and add > > > more > > > > later. > > > > > > > > Yufei > > > > > > > > > > > > On Wed, Oct 8, 2025 at 1:45 PM Dmitri Bourlatchkov <[email protected]> > > > wrote: > > > > > > > > > Hi Sung, > > > > > > > > > > Re: beta - since OPA is not "on" by default, I think it should be > > > enough to > > > > > mention "beta" in docs / javadoc / examples that show how it gets > > > enabled. > > > > > Maybe add a ProductionReadinessCheck for it?. > > > > > > > > > > Cheers, > > > > > Dmitri. > > > > > > > > > > On Tue, Oct 7, 2025 at 10:08 PM Sung Yun <[email protected]> wrote: > > > > > > > > > > > Hi Yufei and Dmitri, > > > > > > > > > > > > Thank you both for following up on this thread! > > > > > > > > > > > > And thank you to everyone who’s taken the time to review the RFC - > > > I’ve > > > > > > incorporated much of the feedback to improve the document and made > > a > > > few > > > > > > minor updates to the PR as well. > > > > > > > > > > > > Dmitri - now that the RFC has been thoroughly reviewed, I’ve > > adopted > > > the > > > > > > feedback and formally opened the PR for review. I agree that > > marking > > > the > > > > > > initial implementation as beta makes sense. Could you clarify the > > > process > > > > > > for that - is it simply a matter of documentation? > > > > > > > > > > > > And yes, I’m also happy to follow up with a separate PR to add the > > > > > > relevant documentation updates. > > > > > > > > > > > > Sung > > > > > > > > > > > > On 2025/10/03 16:28:52 Dmitri Bourlatchkov wrote: > > > > > > > It's a very nice and useful proposal, IMHO. Thanks for driving > > it, > > > > > Sung! > > > > > > > > > > > > > > I added some minor comments to the doc. The rough edges related > > to > > > > > > > per-realm configuration and federated principals can probably be > > > > > > addressed > > > > > > > later. > > > > > > > > > > > > > > What is your plan for opening the related GH PR for general > > review > > > > > (it's > > > > > > > still a "draft" ATM)? > > > > > > > > > > > > > > In terms of bundling OPA into Polaris distributions, we discussed > > > > > > > supporting flexing options for end users in the last sync call. > > At > > > this > > > > > > > state, though, I think standard Polaris images are still pretty > > > > > > monolithic > > > > > > > so I think OPA will have to be a built-in component, if we want > > to > > > open > > > > > > it > > > > > > > to users of the binary distribution (which is fine from my POV). > > > > > > > > > > > > > > Custom downstream builds still have the option of including or > > > > > excluding > > > > > > > the OPA authorizer module from their build-time dependencies. > > > > > > > > > > > > > > Since the OPA authorizer configuration involves a schema for > > > outgoing > > > > > > > requests to OPA agents, it might be prudent to mark the initial > > > > > > > implementation as "beta" to allow this schema to evolve quickly > > > over > > > > > the > > > > > > > next few releases without incurring too much backward > > compatibility > > > > > > > overhead. WDYT? > > > > > > > > > > > > > > Would you be open to contributing Polaris doc pages for OPA > > > (later)? > > > > > > > > > > > > > > Cheers, > > > > > > > Dmitri. > > > > > > > > > > > > > > On Wed, Oct 1, 2025 at 5:47 PM Sung Yun <[email protected]> > > > wrote: > > > > > > > > > > > > > > > Hi folks, > > > > > > > > > > > > > > > > I'm seeking feedback on an RFC to add Open Policy Agent (OPA) > > as > > > an > > > > > > > > opt-in authorizer plugin for Polaris. The motivation is > > > > > > > > straightforward: as deployments scale, RBAC alone struggles > > with > > > > > > > > context (purpose of use, data sensitivity, workload identity) > > and > > > > > > > > often devolves into role explosion. Policy engines like OPA > > > enable us > > > > > > > > to decouple policy from code and express richer attribute-based > > > rules > > > > > > > > in a Rego, improving auditability and testability without > > > changing > > > > > > > > Polaris’ catalog semantics. > > > > > > > > > > > > > > > > Delegating policy decisions to OPA will also enable > > > organizations to > > > > > > > > reuse their existing, centralized policy store. Polaris can run > > > OPA > > > > > > > > locally as a sidecar while OPA fetches bundles from the > > > centralized > > > > > > > > policy distribution pipeline, which may be a necessity due to a > > > > > > > > streamlined governance strategy. > > > > > > > > > > > > > > > > The proposal is ready for review (so is the PR) and has been > > > > > > > > intentionally designed to be safe to trial. The existing > > > > > > > > PolarisAuthorizerImpl will remain the default and the proposed > > > > > > > > OpaPolarisAuthorizer is strictly opt-in through configurations. > > > > > > > > Implementation details, configuration, and security options are > > > in > > > > > the > > > > > > > > RFC. > > > > > > > > > > > > > > > > I'd appreciate your review and feedback! > > > > > > > > > > > > > > > > Thanks, > > > > > > > > Sung > > > > > > > > > > > > > > > > Google Doc: > > > > > > > > > > > > > > > > > > > > > > > > https://docs.google.com/document/d/1HadMFygjbuZathZZPanO6cFVorx0Ju0FopkICxX1tCE/edit?tab=t.0 > > > > > > > > PR: https://github.com/apache/polaris/pull/2680 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
