Enhancement thoughts

[willhausman]

These can be broken out into their own issues, but I felt it would be important context to list them all together first.

A year ago, I built out a strategy for using Opa.Wasm to handle authorization in my micro-service architecture. It is currently working in production using version 0.20.0, and I'm happy about that. I want to use my knowledge of how the library is used to provide a genericized approach to relieve others of working out the strategies to do so, and to make my own implementation simpler.

Abstractions

First, is the lack of abstractions on the implementations. I am dependent on loading Wasmtime and if that changes in the future, I have to alter all of my registrations. Depending on interfaces eases this ache, especially when combined with first-class dependency injection extensions. I am not married to these exact interfaces, but they show the direction I'm currently thinking.

/// Manages concerns for a wasm runtime and provides ways to load modules on that runtime
public interface IOpaRuntime : IDisposable
{
    IOpaModule LoadModule(string wasmFilePath);
    IOpaModule LoadModule(string moduleName, byte[] contents);

    // these two methods could probably be accomplished with the byte array overload with a builder in dependency injection.
    IOpaModule LoadModuleFromBundle(string opaBundlePath); // handle loading an opa bundle for users
    IOpaModule LoadModuleFromRego(string regoPath); // this is my ultimate goal, but I haven't figured out how yet
}

/// Manages memory for wasm modules, and instantiates policies.
public interface IOpaModule : IDisposable
{
    IOpaSerializer Serializer { get; } // probably wouldn't be public, but would be associated with a particular module
    IOpaPolicy CreatePolicy();

    // common things to a module that can be passed to every instance of a policy
    IReadOnlyDictionary<string, object> Data { get; } // convenience for knowing what data will be given to every instance of policy
    void AddData(string key, object obj); // Some mechanism for loading data needed by all instances of the module
    void RegisterBuiltin(string builtinName, Func callback);
    void RegisterBuiltin(string builtinName, Func callback);
}

/// Mostly the same as the current OpaPolicy, but enhancements are great
public interface IOpaPolicy : IDisposable
{
    IReadOnlyDictionary<string, object> Data { get; } // convenience for knowing what data is loaded in the policy instance

    string EvaluateRaw(string inputJson); // other params here too. brevity. directly uses LoadJson() and returns DumpJson()
    T Evaluate<T>(object input); // uses Serializer configured from module ontop of LoadJson() and DumpJson()

    void RegisterBuiltinRaw(string builtinName, Func<string> callback); // directly use LoadJson/DumpJson.  Basically, the library is only responsible for memory lookup.
    void RegisterBuiltin<T>(string builtinName, Func<T> callback);

    void AddData(string key, object obj); // merge data onto initial data from IOpaModule
}

/// Normal serializer as expected, but allows the library to abstract away the serialization concerns for a policy.
/// Also, with dependency injection, it could be configured to use the calling code's serialization settings, or use an unsupported serializer
/// (I assume this library would default to JST settings for performance, but support Newtonsoft for common settings that calling code might be using)
public interface IOpaSerializer
{
    string Serialize(object obj);
    T Deserialize<T>(string json);
}

Dependency Injection Extensions

This section is the original reason I started talking about enhancements. I planned to build out a Opa.Wasm.DependencyInjection nuget, and while I have something that could work, it would be better if that could be rolled directly into this repository (even if it is still published as an extra nuget).

In my current implementation, I built up a concrete class specifically for loading modules, and register it to my project as a singleton so the module compilation step is only hit once. From there, I register a transient IPolicyProvider<T : IPolicy>, which uses the singleton factory as a dependency to create an instance of OpaPolicy, set whatever data is needed by the policy to be effectively executed, and then return an instance of the IPolicy requested, which is generally scoped.

  services.AddSingleton<OpaModuleFactory>();
  services.AddTransient<IPolicyProvider<IMyPolicy>, ProviderImplementation>();
  services.AddScoped<IMyPolicy>(provider => provider.GetRequiredService<IPolicyProvider<IMyPolicy>>().GetPolicy().GetAwaiter().GetResult())

Depending on use, the provider could also be scoped. By taking some inspiration from Microsoft's IBuilder services.AddHttpClient<TClient>(configure), and the builtin logging extensions, ILogger<TCategoryName>, I was building towards a solution similar to this:

  services.AddOpaPolicy<TModule>(wasmFilePath)
    .UseNewtsonsoft(customSerializerSettingsAndContractResolvers)
    .RegisterBuiltin("math.random", () => random.Next()) // not positive this will be useful without a service provider
    .AddData(new ThingLoadedFromEmbeddedResource()));

After this, any service that needs that particular module, could inject into their class, IOpaPolicy<TModule> and get a transient instance of OpaPolicy specific to the already loaded module with the service collection handling disposal.

A user can call AddOpaPolicy() as many times as they need, loading different modules. Ideally, it doesn't matter where this gets called, (e.g. if an enterprise application has multiple tiers building up one host application, separate tiers might call AddOpaPolicy() without affecting the other injections (provided no name collisions, which should be validated on startup).

Documentation

Adding some documentation on intended use of the nuget to help users understand how best to manage an opa policy. Tips and hints would be good as well. For example, build steps to add regos to their repositories that get automatically compiled with a project, but left out of a repository.

[willhausman]

I also have some thoughts on restructuring the internal implementation of OpaPolicy to make it depend on abstractions instead of Wasmtime specifics, but I'm presently more interested in the outer interfaces. Besides, the Wasmtime Store can't be accessed simultaneously per their docs (though I'm unsure if this means it lacks a semaphore, or it does have a lock so there's a time hit, or if it's just not thread safe at all).

OPA is appearing more and more frequently in solutions, and the capability to bring it directly to c# with web assembly is just too incredible to pass up. A great library is just what the community needs!

[christophwille]

Regarding runtime: I did change the runtime once (Wasmer to Wasmtime) and I had to change my object model for a couple of major Wasmtime changes. Those were super-breaking changes to how Opa.Wasm worked. So I don't think abstracting the actual runtime will work great.

[christophwille]

Regarding serialization: what would be a real-world use case to not use STJ? For Web APIs, I understand the use cases. But deserializing for from/to OPA wasm is very specific and not really up to "configuration".

[christophwille]

Regarding DI/policy provider: I have a very old prototype in the spikes folder - AspNetAuthZwithOpa (actually that was the initial reason to build this whole library, but never got around to build it out). It had a policies store idea (because there would be a store (resources, files, db, OCI registry), which even might provide newer versions for reload), but it had no cache for the compiled modules because of threading (see https://github.com/christophwille/dotnet-opa-wasm/blob/master/spikes/AspNetAuthZwithOpa/Authorization/OpaPolicyHandler.cs#L33)

The idea to separate store from runtime cache was that the store could be a shared remote Redis (think Web server farm), whereas the compiled module cache would be per machine (after all remoting that would be pointless).

What sort of stopped me was "ok, now how do I decorate my controllers with data & pass input". My ideal use case was "don't check roles only, but provide input as to the item in question" (role Editor but only for Sports). And maybe call a builtin callback that does a eg database check.

Also, as a kind of note we should never use Policy without prefixing it with Opa - otherwise authZ in ASP.NET gets really hard do understand re: what comes from where.

After plucking my memory on what I wanted to do and the use cases I envisoned: I was thinking about hiding the OPA workings behind a layer that was more business layer-y. Not exposing the OPA lowest layer to the consumer of the library. Having said that, I think interfaces on that lowest layer is a bit excessive (don't add flexibility unless you can prove you need it).

[willhausman]

Regarding serialization: what would be a real-world use case to not use STJ? For Web APIs, I understand the use cases. But deserializing for from/to OPA wasm is very specific and not really up to "configuration".

Blindly serializing with STJ is ignoring that people other than OPA have serialization concerns.

If my application has custom contract resolvers, I wouldn't be able to use them. Worse, if I had already written them, but my application is using the more mature Json.Net, I would be forced to rewrite them just to use a library. The ultimate output is valid json, so who cares if calling serialize on a giant object results in a small representation, and vice versa on deserialize.

Another example is casing on properties. Teams have opinions about how things should be serialized, and OPA is case sensitive on inputs and data. A library should not force me to use their opinion so long as my usage doesn't actually break anything.

Another example is quoted numbers. This case can cause OPA problems if using a different number culture, as I demonstrated in a playground earlier, but it's absolutely a thing OPA can handle with to_number if the user wants to.

[christophwille]

Which calls specifically accept an object? (eval and setdata take strings in the current api surface)

[willhausman]

Which calls specifically accept an object? (eval and setdata take strings in the current api surface)

I was trying to expose both as possibilities. eval(object) takes an object and returns a deserialized response (including parsing out Opa's IEnumerable thing so just the actual result is returned). EvaluateRaw(string) takes json and returns json without any manipulations at all, just DumpJson() and LoadJson().

SetData() could do something similar, but I was hoping to add the ability to see WHAT data you've shoved into the policy. I can't tell you how many debugging sessions I've had over the past couple of weeks where it would have been great to know exactly what got loaded without going backwards in my callchain to find where I serialized the objects. It's not that I can't do that, it's just easier to know right away. Storing it as a dictionary of some sort also means it's simple to merge a new json object and call SetData() again without asking the user to do that.

[christophwille]

So mixing the "metal" layer with a "convenience" layer. The current method signatures play along the lines of the JS API. And I want to keep it simple at the lowest layer because otherwise reasoning where the bug comes from is super-painful.

Oh, and issue #5 has a couple of API discussions too that I totally forgot about.

[willhausman]

Regarding runtime: I did change the runtime once (Wasmer to Wasmtime) and I had to change my object model for a couple of major Wasmtime changes. Those were super-breaking changes to how Opa.Wasm worked. So I don't think abstracting the actual runtime will work great.

I remember looking at your Wasmer code (I've been silently following this repo for over a year), and thought I had a way that supports both, but I'd have to go back and look again. Besides, I never said major breaking changes can't be a thing. It would just be easier to avoid major breaking changes for consumers if they are dependent on an interface contract instead of a concrete class.

[willhausman]

Also, as a kind of note we should never use Policy without prefixing it with Opa - otherwise authZ in ASP.NET gets really hard do understand re: what comes from where.

I agree for a general library. I was mixing terminology between what I have in my current implementation using 0.20.0, and what I was suggesting for DI in the library. My implementation has an entirely custom authorization structure, so policies have nothing to do with ASP.NET in my codebase.

[willhausman]

So mixing the "metal" layer with a "convenience" layer. The current method signatures play along the lines of the JS API. And I want to keep it simple at the lowest layer because otherwise reasoning where the bug comes from is super-painful.

Fair enough. This could be resolved with a decorator pattern. Provide both IOpaPolicy and IRawOpaPolicy that can be injected. IOpaPolicy only exposes the typed methods, accepts IRawOpaPolicy as a dependency. It calls handles all serialization/deserialization needs and calls IRawOpaPolicy. If a user needs the lowest level layer, they inject IRawOpaPolicy instead. The builtin delegates would be an issue to resolve, though.

[christophwille]

I have cooked up an unfinished draft of how a Factory could work: https://github.com/christophwille/dotnet-opa-wasm/tree/master/spikes/HigherLevelApisSpike - start at Program.cs plus the comments in the IFactory... file.

The idea here is that most everything for PolicyFactory is optional except loading. One could use it with module caching, with a custom serializer, with SetData being called before the Policy object is handed out. The naming comes from ArrayPool.

Note that it doesn't compile at all. It really is intended for discussion what would be needed.

[willhausman]

Regarding DI/policy provider: I have a very old prototype in the spikes folder - AspNetAuthZwithOpa (actually that was the initial reason to build this whole library, but never got around to build it out). It had a policies store idea (because there would be a store (resources, files, db, OCI registry), which even might provide newer versions for reload), but it had no cache for the compiled modules because of threading (see https://github.com/christophwille/dotnet-opa-wasm/blob/master/spikes/AspNetAuthZwithOpa/Authorization/OpaPolicyHandler.cs#L33)

The idea to separate store from runtime cache was that the store could be a shared remote Redis (think Web server farm), whereas the compiled module cache would be per machine (after all remoting that would be pointless).

What sort of stopped me was "ok, now how do I decorate my controllers with data & pass input". My ideal use case was "don't check roles only, but provide input as to the item in question" (role Editor but only for Sports). And maybe call a builtin callback that does a eg database check.

What I did was leave the compiled module in memory, and just put the data to be loaded into an instance of the module in Redis. To my knowledge of the current implementation, the Store (and maybe the Linker) is the only thing that isn't thread safe, and those are made per policy instance. I'll hunt for the code later, but I also did some threading tests and timing to confirm that the instances work as expected.

[willhausman]

I have cooked up an unfinished draft of how a Factory could work: https://github.com/christophwille/dotnet-opa-wasm/tree/master/spikes/HigherLevelApisSpike - start at Program.cs plus the comments in the IFactory... file.

The idea here is that most everything for PolicyFactory is optional except loading. One could use it with module caching, with a custom serializer, with SetData being called before the Policy object is handed out. The naming comes from ArrayPool.

Note that it doesn't compile at all. It really is intended for discussion what would be needed.

This is very similar to what I am suggesting, with different names. Policy and OpaPolicy are exactly the decorator pattern I was just suggesting. It has one important difference, though. The OpaPolicy itself is cached instead of just the module. I've already run tests to find out that loading the module is the time hit, not instantiating the OpaPolicy. The instance is what has specific data, as well. What I found is that calling SetData within my factory was only helpful for static information. Especially in the case of user authorization, it was more helpful to provide the policy as an injection, and let the calling code set whatever it needs - calling a db, pulling info from cached reflection, transforming information from elasticsearch, etc (I do exactly this). For exactly this reason, I don't think caching the policies themselves is a good idea. A user can set whatever they want in there, and it might need to be transient or scoped. Trying to pool the policies themselves is only sweeping the threading problem under the rug. If they need a synchronization mechanism, use one on every memory manipulation.

[willhausman]

As an aside, I think the ability to load a module from an opa bundle, not just the wasm, is important because bundles have a data.json file which we should honor on first load of any instance of the policy.

[christophwille]

I stuck to the pattern I am using with the benchmarks project: https://github.com/christophwille/dotnet-opa-wasm/blob/master/src/Opa.Wasm.Benchmarks/Program.cs#L19 caching the Wasmtime.Module, and then instantiating per-call OpaPolicy objects. The idea is to pool & rent out Wasmtime.Module to OpaPolicy.

Policy as an injection: yeah, I thought about a "C# wrapper per OPA policy" - that gets unwieldy pretty quickly if you have lots of them.

[willhausman]

That was not obvious to me from the sample code. So, that means we are talking about doing the same thing. Having a thing that supplies instances of policies to users. I don't understand why you need an object pool for the modules, though. As far as I can tell, the Store is the only thing that needs special care for threading, and current implementation already makes a new instance of that for every OpaPolicy. It has nothing to do with the Module itself, just the creating policy.

Here's the factory I had started writing a few days ago based on my other implementation. https://github.com/willhausman/dotnet-opa-wasm/blob/dependency-injection/src/Opa.Wasm.DependencyInjection/OpaPolicyFactory.cs and some tests for some clarification on how it gets used https://github.com/willhausman/dotnet-opa-wasm/blob/dependency-injection/src/Opa.Wasm.UnitTests/DependencyInjectionTests.cs.

Ignoring how DI itself works, you basically tell the factory about what modules you care about (either by name or by Type, but I didn't flesh out doing it by name. It's the same thing, though). And then you can ask it for an instance of that policy. There is a single instance of the module loaded into memory with the factory, rather than a pool of potential cached modules to pick from. Associating them with types/names allows the factory to easily manage multiple policies loaded into a single execution, as shown by the test with two implementations registered.

[christophwille]

Maybe my memory is failing me what is thread-safe and what isn't.... will have to ask (my ASP.NET sample was way old, so there might be the issue).

see bytecodealliance/wasmtime-dotnet#90

[christophwille]

I have adapted the spike to the discussion with Peter (I seemingly did factor the object model according to his recommendations).

With the clarification of the Module being thread-safe I was able to strip everything down to basically https://github.com/christophwille/dotnet-opa-wasm/blob/master/spikes/HigherLevelApisSpike/IPolicyFactory.cs#L7 only serde being needed for the Policy, and the PolicyFactory being simple as can be.

That now really begs the question if everything we need to do is have serde configurability on the existing OpaPolicy class + the factory pieces - and do away with the pesky additional layer of Policy object.

[willhausman]

If you want to entirely replace the string Evaluate(string json) with T? Evaluate<T>(object input) and always use the IOpaSerializer, then no, the decorator Policy object is not needed. That's the implementation I suggested above, minus the EvaluateRaw() execution path.

I don't think you need the abstraction for IOpaModuleCache though. It's really just a dictionary of policynames to either Module instances or bytes, no? ConcurrentDictionary will do that and we don't have to worry about thread locks with a cache object.

Also, see bytecodealliance/wasmtime-dotnet#89. We may be fine for caching modules, but the instances themselves still have a potential threading issue that we might want to try and address. I don't know how well it work, but one thought that comes to mind is wrapping the implementation of Evaluate() and SetData() with a lock. Super heavy-handed, but it would mean only one thing can execute at a time and alter state within that instance.

[christophwille]

OpaPolicy should remain a one-off instance that is not reuseable. No locks. A cheap throw-away object. Module caching it is.

My separate interface for caching was a stand-in for IMemoryCache DI use cases. Yes, mostly because of expirations and stuff that it can do in addition to ConcurrentDictionary. Otherwise it would do the trick. And yes, you can certainly use it by implementing the interface (maybe a good idea to provide one for unit testing).

Currently I am leaning towards adding typed methods like EvaluateTyped and SetDataTyped (I am not too keen on renaming the existing ones to *Raw although we are not API stable at all at this point - maybe SetDataJson and EvaluateJson?).

[willhausman]

Recently discovered IMemoryCache isn't thread safe in .NET Core like it is in .NET Framework. FYI.

I'm more partial to SetDataJson and EvaluateJson, since I see those as "the advanced use case" scenarios.

[willhausman]

We need to use the same instance of Engine for instantiations, so that will need to be stored along with the module, per bytecodealliance/wasmtime-dotnet#90. I just do this with a little record in the factory.

private sealed record Runtime(OpaRuntime OpaRuntime, Module WasmModule);

So, after pulling it from the cache, it's just new OpaPolicy(runtime.OpaRuntime, runtime.WasmModule);. Probably something like InstantiationContext would be a more appropriate name than Runtime, though.

[christophwille]

With the discussion in bytecodealliance/wasmtime-dotnet#90 (comment) (the need to keep module & engine around when "caching" the compilation step): instead of an extra data structure, why not adapt the object model to the realities (it is mostly a turning OpaRuntime into OpaPolicyModule).

// With bytes, even new OpaPolicyModule would work - if we always only accept bytes, this would  be ok (helper for files)
// Although I don't like the idea of ctors throwing or taking a long time (compilation step)
using var rbacModule = OpaPolicyModule.CreateFromBytes(bytesArr);
using var rbacPolicy = rbcModule.CreatePolicyInstance();

In this "one-off" use case the Wasmtime.Engine is owned and dispsed by the OpaPolicyModule.

using Wasmtime.Engine engine = OpaPolicyModule.CreateEngine();
using var rbacModule = OpaPolicyModule.Load(bytesArr, engine);
using var rbacPolicy = rbcModule.CreatePolicyInstance();

Here the user is responsible for the lifetime of the engine, the module only has a reference. The CreatePolicyInstance method comes from the fact that there'd remain only one ctor on OpaPolicy, and I'd like to make it internal (after all, the OpaPolicyModule is a Factory for OpaPolicy).

And yes, have an interface on OpaPolicy so it can be mocked.

[willhausman]

I'm thinking IOpaSerializer should be customizable per module, not for the whole nuget. Reason being, it's no harder to code, and it's more flexible for calling code to be able to share wasms between projects with different settings (which I am doing, as some older code gets merged with newer code).

I don't want to lose sight of the goal I actually set out to accomplish here - getting a public interface on at least the concrete OpaPolicy, and a way to manage dependency injections without needing to know how to instantiate a policy. In other words, with just a wasm file path. In the most optimal solution, I can also register an injection of a particular type, similar to injecting ILogger<MyClass> for any service that wants to categorize its logs by MyClass. IOpaPolicy<MyClass>. That's just a decorator that only a factory would have use for, but I don't want to lose sight of it because the implementation of said factory could make that difficult to accomplish.

Here's a quick gist of how it could work in general. It makes use of an enumerable injected OpaModuleBuilder that can be configured at startup, and then defer populating the factory until instantiation. Those could be used directly in the IOpaCache idea, or whatever. I am interested in this deferred approach because it allows me to distribute calls to .AddOpaPolicy() to let extensions live with their respective projects. A policy in my Domain.csproj for complex business logic doesn't necessarily need to know about the policy I've defined in my Web.csproj for authorization, etc. https://gist.github.com/willhausman/8ca89dafb1b8743413d3b6567cb8071c

[willhausman]

I took forever to write that comment and you commented before I finished. Ha! I don't think the two ideas are incompatible. I even referenced an OpaModule object that keeps ahold of the Engine. I think the two can be put together easily enough.

[christophwille]

I am still trying to grok your gist (a bit too early in the morning for that much indirection).

Before I get into this, my background (see also #5): I came at this from the perspective of a server-side application. From observability. From managing the policies is a different person from the developer of the application. From the location of the policies better be a central place (server farm scenario).

Esp. the latter point makes AddOpaPolicy look out of place for my approach. Is injecting the factory a huge deal compared to injecting a "something" that consists mostly of marker interfaces?

That however brought the topic of the consumer to my mind... a problem from the very beginning bugging me: does a call site want to deal with OPA directly or use something that knows about OPA and nicely provides an API for that specific policy:

public class MyRbacPolicy
{
 public MyRbacPolicy(IOpaPolicyFactory factory) {}
 public bool CallMe();
}

And adding that wrapper .AddTransient - wouldn't that make more sense? All OPA is hidden from the call site, the custom object deals with the factory and all OPA specifics.

[christophwille]

I went ahead and did the low-level refactoring OpaRuntime -> OpaPolicyModule. That made the thread safety test go away because you can't pass a new Engine to an existing Module any more. Unexpected win, but nice.

https://github.com/christophwille/dotnet-opa-wasm/blob/master/src/Opa.Wasm/OpaPolicyModule.cs

Doesn't look too dissimilar to OpaRuntime, Load is the factory method. Simple search & replace worked on the unit tests, so not much hassle for current users of the library. What are your thoughts now that this "real"?

[willhausman]

Recall that I am thinking of a separate nuget package for dependency injection. Almost my entire gist could live inside a package specifically for dependency injection, e.g. Opa.Wasm.Extensions.DependencyInjection or something similar. The only thing it really needs in core Opa.Wasm is OpaPolicy : IOpaPolicy. It would be convenient if OpaPolicy<TClient> : OpaPolicy, IOpaPolicyClient<TClient> also lived there to ensure the contracts were always up to date, but even that could be a decorator instead of a subclass if needed.

This makes loading policies directly with your application a capability, rather than the intended use case. A server farm of policies maintained by separate people are one capability, and this is another. My thinking here is quite simple: OPA can make complicated logic ridiculously simple and fast. It can be used for domain-specific business logic. I am presently using it to build out row and column security for dynamic sql tables in a micro service that other microservices don't know anything about, nor do they know how to obtain the metadata to populate the data for that policy. It even uses a builtin to call another wasm that other microservices do know about. CICD allows the .rego for this one domain-specific need to live with the code and be re-built to always be up-to-date with deployments.

It boils down to this guy from my gist being able to call OpaPolicyModule.LoadModule(name, content) (with renames between OpaPolicyModule and OpaModule. They're the same thing):

internal class OpaModuleFactory : IDisposable
{
  private readonly bytes[] wasmContents;
  private readonly IOpaSerializer serializer;
  public string PolicyName { get; }

  public OpaModule CreateModule();
}

[willhausman]

public class MyRbacPolicy
{
 public MyRbacPolicy(IOpaPolicyFactory factory) {}
 public bool CallMe();
}

Regarding this, my answer is yes with a caveat. That is exactly what the intention of Consumer.cs is, except it has already done the step of calling factory.Create(name) to get an instance of the policy to run with CallMe(). If the calling code does not want to register a specific IOpaPolicy<TClient>, they could just inject IOpaPolicyFactory and call factory.Create(name) themselves. Both routes work (and could be simultaneously supported). It's just one step further to make managing even the policyName automated down to a type.

public class MyRbacPolicy
{
 public MyRbacPolicy(IOpaPolicy<MyRbacPolicy> policy) {}
 private Setup(); // register builtins, etc.
 public bool CallMe() => policy.Evaluate(my params);
}

In fact, I didn't fully code it out, but my gist actually does support both of these scenarios.

[christophwille]

I am not at all convinced about IOpaPolicy - especially because that is doing zero zilch nothing for the library itself (compare it to IStream eg). There isn't going to be another class implementing IOpaPolicy.

[christophwille]

Regarding IOpaSerializer - willing to add that because there is a clear-cut use case there. Do we add it as optional param to CreatePolicyInstance()? (plus a public getter/setter)

[willhausman]

I am not at all convinced about IOpaPolicy - especially because that is doing zero zilch nothing for the library itself (compare it to IStream eg). There isn't going to be another class implementing IOpaPolicy.

And yes, have an interface on OpaPolicy so it can be mocked.

Sounds like you're tired and talking back and forth on yourself.. :)

Does it hurt something to provide a public contract in the form of an interface? If a policy instance can be directly injected without a factory, it becomes much harder to mock.

[christophwille]

What's the point of the interface is the better question. Just for DI shenenigans? Not a good reason.

[willhausman]

so it can be mocked.

You said it yourself. With good design, you depend on abstractions, not concrete implementations, so it is easier to test in isolation, and refactor in isolation. The library itself does not need to have a use for the interface beyond enabling calling code to follow good coding practices. Dependency Injection can take advantage of that (obviously, it's the Dependency Inversion Principle in SOLID), but it is not the only reason to do so.

[willhausman]

Updated my gist https://gist.github.com/willhausman/8ca89dafb1b8743413d3b6567cb8071c to show the 3 use cases all supported in Consumer.cs. It ultimately all comes down to two signatures,

namespace Opa.Wasm.DependencyInjection;

public interface IOpaPolicyFactory
{
  IOpaPolicy CreatePolicy(string policyName);
  IOpaPolicy CreatePolicy<TPolicyName>();
}

Which under the hood are the exact same call to the OpaPolicyModule.CreatePolicy() method you created, after finding the right module based on policyName.

[christophwille]

Status: played some more with https://github.com/christophwille/dotnet-opa-wasm/tree/master/spikes/AspNetAuthZwithOpa to see how prescriptive / opinionated things can / should be. Not convinced yet on any one strategy or providing it at all.