Figure out how engines other than OPA can point to own docs for built-in functions

[anderseknert]

For auto-completion and hover info, we display a link for any given built-in function, which takes users to the built-in reference page of the OPA docs for that function. However, I don't think we had different engines in mind when we wrote these functions, and testing this with EOPA built-in functions this evening we (me and @chendrix) noticed that the links point to OPA docs even for those.

These functions should of course point to EOPA's reference docs over built-in functions.

The challenge here is how to best differentiate the origin of a built-in function, as the capabilities file/object is just one big list of functions, and even when using EOPA or another engine, one would still want OPA built-in functions to point to the OPA docs.

[charlieegan3]

I've been having a little look into this, it seems when loading custom capabilities we're using ast.LoadCapabilitiesJSON like this:

    caps, err := ast.LoadCapabilitiesJSON(fd)
    if err != nil {
        return nil, fmt.Errorf("failed to load capabilities due to error: %w", err)
    }

This creates ast.Builtin like this:

type Builtin struct {
    Name        string `json:"name"`                  // Unique name of built-in function, e.g., <name>(arg1,arg2,...,argN)
    Description string `json:"description,omitempty"` // Description of what the built-in function does.

    // Categories of the built-in function. Omitted for namespaced
    // built-ins, i.e. "array.concat" is taken to be of the "array" category.
    // "minus" for example, is part of two categories: numbers and sets. (NOTE(sr): aspirational)
    Categories []string `json:"categories,omitempty"`

    Decl             *types.Function `json:"decl"`               // Built-in function type declaration.
    Infix            string          `json:"infix,omitempty"`    // Unique name of infix operator. Default should be unset.
    Relation         bool            `json:"relation,omitempty"` // Indicates if the built-in acts as a relation.
    deprecated       bool            // Indicates if the built-in has been deprecated.
    Nondeterministic bool            `json:"nondeterministic,omitempty"` // Indicates if the built-in returns non-deterministic results.
}

However, the EOPA capabilities don't seem to have the description etc set. So my first question is: should we be loading caps.json from elsewhere?

But, that's mostly unrelated to how we generate the correct links...

I think, having looked at how we use the ast.Builtins all over the project, it'd be better to make the link generation function parameterised, so we can pass an OPA one, or an EOPA one (with OPA fallback).

I am a little unsure how to make this work for custom json files of caps, but if a user is using config like this:

capabilities:
  from:
    engine: eopa
    version: v1.27.1

Then, I think that we can update the link generation function to at least support the EOPA builtins with an OPA fallback. Or do we need to cover more capabilities cases here?

[anderseknert]

Thanks @charlieegan3! Yeah, looking at the capabilities files in the eopa directory, it looks like they've all been stripped of description attributes. Most of them are present when in the output of eopa capabilities --current, so it looks like they've just been filtered out somewhere along the way. So the quick fix would be to... well, fix that.

However, the EOPA-specific built-in seem to lack many of the description attributes that OPA built-ins have, like those describing the args and the return values. So as far as I can tell, some additional work on documenting those in the EOPA project would be needed for parity with the OPA built-ins.

CC @srenatus @chendrix

(only tangentially related, but I wonder if there's some better / more compact way to store capabilities for each version... 🤔 as we're currently storing 99% identical data in all those files. We could just provide diffs from the first, or current version or something)

[srenatus]

So as far as I can tell, some additional work on documenting those in the EOPA project would be needed for parity with the OPA built-ins.

EOPA doesn't generate builtins_metadata.json like OPA does -- but the data should be present, like

    sqlSend = &ast.Builtin{
        Name:        sqlSendName,
        Description: "Returns query result rows to the given SQL query.",
        Decl: types.NewFunction(
            types.Args(
                types.Named("request", types.NewObject(nil, types.NewDynamicProperty(types.S, types.A))),
            ),
            types.Named("response", types.NewObject(nil, types.NewDynamicProperty(types.A, types.A))),
        ),
        Nondeterministic: true,
    }

only tangentially related, but I wonder if there's some better / more compact way to store capabilities for each version... 🤔 as we're currently storing 99% identical data in all those files. We could just provide diffs from the first, or current version or something

I was hoping that the "embed" mechanism's compression takes care of that problem. Have you been noticing a steeper than expected increase in binary sizes? 📈

[anderseknert]

EOPA doesn't generate builtins_metadata.json like OPA

That's not the file I'm looking for here :) I'm saying that if the capabilities.json files from EOPA stored in this repo matched the output of the eopa capabilities command, we'd have what we need for making the tooltip for EOPA built-ins look the same as for OPA's.

but the data should be present

It's not though. The struct in your example only has a top-level description but none for the args / return value. Compare to e.g.

var CryptoHmacSha256 = &Builtin{
    Name:        "crypto.hmac.sha256",
    Description: "Returns a string representing the SHA256 HMAC of the input message using the input key.",
    Decl: types.NewFunction(
        types.Args(
            types.Named("x", types.S).Description("input string"),
            types.Named("key", types.S).Description("key to use"),
        ),
        types.Named("y", types.S).Description("SHA256-HMAC of `x`"),
    ),
}

The only remaining issue then would be the docs link, but I think we had a decent solution proposed for that.

[srenatus]

The struct in your example only has a top-level description but none for the args / return value.

Are we looking at the same struct? I see named arguments and return values here:

        Decl: types.NewFunction(
            types.Args(
                types.Named("request", types.NewObject(nil, types.NewDynamicProperty(types.S, types.A))),
            ),
            types.Named("response", types.NewObject(nil, types.NewDynamicProperty(types.A, types.A))),
        ),

[anderseknert]

They are named, but they have no .Description()

[srenatus]

💡 Thanks! I mistook the names for their description 🙈 I'll take care of that.

[anderseknert]

👍 To be fair, a number of (mostly recent) OPA built-in functions seem to be missing this too. I'll create an issue about that for OPA, as it would be good to do this consistently.

Edit: done https://github.com/open-policy-agent/opa/issues/7151