Complete builtin coverage

[patrick-east]

As-is the SDK has a few builtins added:

And the list that OPA includes baked into the WASM binary is increasing, current list at: https://github.com/open-policy-agent/opa/blob/master/internal/planner/planner.go#L26

But there are still a number of missing ones (ex: #22 ).

Before we do anything we need to identify what functions are missing, and how we want to stay in sync with OPA. Over time it will include more builtins, so we will need to slowly deprecate and remove the ones we provide in the javascript SDK. We should figure out a plan for that..

We also need to document any missing functions to help users of the SDK understand what they can and cannot do with the SDK in its current form.

[tobowers]

Are there plans to allow the user to add builtins? The documentation says http could be implemented (for instance) but I don't think this package exposes that behavior?

[patrick-east]

Are there plans to allow the user to add builtins? The documentation says http could be implemented (for instance) but I don't think this package exposes that behavior?

Potentially, but it starts to have implications for portability of Rego policies. The behavior of some part of a policy may change if there are external implementations of the like standard builtin functions.

What we would probably want to do is allow for custom builtin functions to be defined by a user, similar to the golang API for OPA https://www.openpolicyagent.org/docs/latest/extensions/

[tobowers]

That would be really useful for me! Can I help implement?

[patrick-east]

After a quick review of the WASM stuff in OPA it seems like maybe the SDK wouldn't be able to distinguish between the like OPA builtins and a custom one.

So I guess what we would need to do is add an API into https://github.com/open-policy-agent/npm-opa-wasm/blob/master/src/opa.js which basically extends the function map

npm-opa-wasm/src/opa.js

Line 69 in f4be0d0

const builtinFuncs = builtIns;

that we use when an external builtin is called

npm-opa-wasm/src/opa.js

Lines 79 to 104 in f4be0d0

	function _builtinCall(wasmInstance, memory, builtins, builtin_id) {
	const builtInName = builtins[builtin_id];
	const impl = builtinFuncs[builtInName];
	if (impl === undefined) {
	throw {
	message:
	"not implemented: built-in function " +
	builtin_id +
	": " +
	builtins[builtin_id],
	};
	}
	var argArray = Array.prototype.slice.apply(arguments);
	let args = [];
	for (let i = 4; i < argArray.length; i++) {
	const jsArg = _dumpJSON(wasmInstance, memory, argArray[i]);
	args.push(jsArg);
	}
	const result = impl(...args);
	return _loadJSON(wasmInstance, memory, result);
	}

For anyone who compiles with a custom function (which would require using the OPA Golang API's to provide the function definition) it would call out into the SDK, and as long as there is one with the right name in that function map everything should be OK.

I guess for your original question around adding HTTP support, that gives you a potential hook to do it there.

[tbrannam]

I am able to monkey-patch the ./builtIn import to provide some implementations needed to evaluate some rego we use. And you are right it appears that it won't differentiate.

Would it be reasonable to add an option env argument to loadPolicy to allow the caller to add additional builtIns?

I think I can put that together pretty quick

[entropitor]

Allowing users to add there own implementation of a builtin makes sense to me. This makes sure that the user can use this library as is, even if it's not up to date because a certain builtin is not yet supported.