openapi: Extend the /v1/compile API

[philipaconrad]

What changed?

This PR tracks a planned extension to the Enterprise OPA /v1/compile API, as part of our intended support for UCAST and data filtering experiments.

Currently, my approach is to add a single additional string option under "options" ➡️ "targetFormat". Allowed values for the field include: json_ast (the default), sql, and ucast.

Here's what an example /v1/compile input might look like:

POST /v1/compile HTTP/1.1
Content-Type: application/json

{
  "query": "data.example.allow == true",
  "input": {
    "subject": {
      "clearance_level": 4
    }
  },
  "options": {
    "disableInlining": [],
    "targetFormat": "sql"
  },
  "unknowns": [
    "data.reports"
  ]
}

The response would then resemble:

{
  "result": {
    "queries": [
      [
        {
          "index": 0,
          "sql": "WHERE ...\n-- not actually sure what this SQL query would look like for the reference query."
        }
      ]
    ]
  }
}

[srenatus]

I believe we may need further per-targetFormat options?

For example "ucast" would have different valid inputs (PE fragment) depending on whether it's meant for conversion with ucast.as_sql, or ucast-linq or ucast-prisma. Could be a "flavour", akin to database dialects? 🤔

Same for SQL, if we want to expand the set of builtins that we can translate, this could depend on the target dialect, too.

[philipaconrad]

Right, that's a good point. 🤔 I suppose we could switch this to be an object type or something that allows us to nest target-specific options.

[chendrix]

I was interested in seeing if we could actually separate out the response types for different targets into different application/json+<target> MIME types.

I don't want to overload the response to just generic arbitrary JSON

[philipaconrad]

One concern I've seen come up in discussions around the new Compile targets is about handling multiple output targets in a single request.

The argument is that firing up the PE machinery for a single target vs several targets is roughly the same amount of work, so we might as well be able to handle N-many targets in our responses.

I propose the following:

• If the Accept header is provided on its own, we should use that to generate a single target in the response.
• If a targetDialects array field is provided in the options part of the input, we should use that to return the multiple output types together.
  • If both Accept header and targetDialects are present, targetDialects overrides. (This is an arbitrary choice, fwiw.)

EDIT: A third option might be to provide a application/vnd.styra.compile+json, or mixed+json header variation. That would make some validation checks a little easier, and would force API clients to opt-in to multi-target behavior. 🤔

[srenatus]

Thanks for pushing this forward!

[srenatus]

Why an array? I think we're constructing a single condition, so if "queries" from PE are translated, they'll become and OR-of-AND condition object, a single one. So let's do

Suggested change

	queries:
	type: array
	description: Array of UCAST JSON objects describing each query.
	ucast:
	type: object
	description: UCAST JSON objects describing the conditions under which the query is true.

[philipaconrad]

The array was a holdover from the original PE output format, which might contain results for several queries, as I understood it. I'll refactor this tomorrow.

[srenatus]

Suggested change

	queries:
	type: array
	description: Array of strings representing the SQL equivalent of each query.
	sql:
	type: string
	description: A string representing the SQL WHERE clauses for the conditions under which the query is true.

If we have a response like

{
  result: {
    ucast: { ... }
  }
}

for UCAST and

{
  result: {
    sql: { ... }
  }
}

for SQL targets, the mixed response could be

{
  result: {
    ucast: { ... },
    sql: "..."
  }
}

OTOH if we want to support multiple SQL dialects in a single "mixed" request, then we might need to return something like

{
  result: {
    ucast+prisma: { ... },
    ucast+linq: { ... },
    sql+postgres: "...",
    sql+mysql: "..."
  }
}

For UCAST, it might not be necessary, because the encoding of the it is the same for different dialects -- only the set of builtins varies, but if your data policy is valid for LINQ, it's valid for Prisma, too, the same UCAST IR.

But for SQL, we'll have to differentiate...

[philipaconrad]

ℹ️ After thinking on it a bit, I find the "third option" (explicit header variant for mixed/multi-target use case) is appealing because it locks down the different things the implementation might have to do, and it prevents the whole "if options.targetDialects is present, change our behavior" issue. Validation becomes easier, and more stuff can be spelled out directly in the OpenAPI spec then.

I'll push up the change to the spec shortly...

[srenatus]

Thanks for pushing this forward. I think the end is near ✨

[srenatus]

So far, my understanding (and the way the code works right now) is a bit different: the UCAST format is always the same, so there's no difference in form depending on the dialects used.

The supported builtins are different, and this is what I thought we'd want to support when querying for multiple dialects: if you want UCAST for Prisma and LINQ, the set of supported builtins and features is the intersection of the builtins and features supported by each of them. But the outcome will be a single UCAST object.

For the SQL-target dialects, it's similar, but their out form can also be different, so we need to differentiate them.

Does that make sense? I think I'd expect an output for mixed targets like this:

{
  result: {
    sqlMSSQL: "...",
    sqlPostgres: " ...", // if these two have been requested for target SQL
    ucast: { ... } // one field only, for all the requested dialects.
  }
}

[philipaconrad]

Ah! Interesting. I'd misunderstood-- my thought was that the policy would be run through PE potentially N-many times, allowing each compilation to use the "best" builtins/form for each target. But! That would be a hideously expensive approach (given that PE is expensive right now), so a more "constricted" shared target (essentially the lowest common denominator) makes sense. 🤔

[philipaconrad]

I've refactored the spec to match the "single UCAST target" idea. Do we want to update the description to explain why, or is the "baseline" description enough, you think?

[srenatus]

Thanks! I think the end is near 🥳

[srenatus]

Do you think we need the extra {"ucast": {"query": ... }} wrapping? I think I'd drop the inner wrap, i.e. only keep {"ucast": ... }.

Same for the SQL target/dialect pairs... 🤔

[srenatus]

Suggested change

	queries:
	query:

But to be aligned with below, I think I'd rather drop "query"/"queries" altogether, except for where the legacy Compile API demands it (plain AST). So SQL would become

{ "result": "WHERE ticket.tenant = 3" }

etc