Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support to expose disableInlining via Compile API #4357

Closed
srlk opened this issue Feb 16, 2022 · 1 comment · Fixed by #4378
Closed

Support to expose disableInlining via Compile API #4357

srlk opened this issue Feb 16, 2022 · 1 comment · Fixed by #4378

Comments

@srlk
Copy link
Contributor

srlk commented Feb 16, 2022

Hello!

Following up the discussion on open-policy-agent/community#121

What part of OPA would you like to see improved?

It would be nice to expose disableInlining option via Compile API. That would mean

  • Complex functions can be implemented in REGO as now and they can be used for full evaluation
  • For partial evaluation scenario, evaluation response would not inline the functions inside the packages specified by disableInlining option.
  • Services calling partial evaluation may read/parse/generate predicates from support package based on the REGO implementation. Or they can decide to generate a service optimised query for functions inside the packages specified by disableInlining option.

Describe the ideal solution

disableInlining options is exposed via HTTP Compile API and command line as opa eval --disable-inlining

Additional Context

To explain with further details, for an example policy as below

# main.rego
package pkg

import data.func

allow {
    func.complex_func(input.unknownAttribute, input.knownAttribute)
}

# func.rego
package func

complex_func(collection1, collection2) {
    lhs := { x |
        x := collection1[_].value
        x.important
    }
    count(lhs) == 0
}

complex_func(collection1, collection2) {
    lhs := { x |
        x := collection1[_].value
        x.important
    }
    rhs := { x | x := collection2[_].value}
    count(lhs) == count(lhs & rhs)
}

With input

{
  "knownAttribute": [
    { "value": "a" }
  ]
}

Running partial evaluation via cli with the new flag --disable-inlining as below

 opa eval --partial -f source -d /.policy/func_test --unknowns input.unknownAttribute --input input.json --disable-inlining "data.func" "data.pkg.allow==true"

It would be expected to generate below partial evaluation output. One important thing to notice is that in the Query part of the response, function data.partial.func.complex_func is not inlined to conditions those make up the function.

# Query 1
data.partial.func.complex_func(input.unknownAttribute, [{"value": "a"}])

# Module 1
package partial.func

complex_func(__local0__2, __local1__2) {
        __local3__2 = {__local2__2 |
                __local2__2 = __local0__2[_].value
                __local2__2.important
        }

        count(__local3__2, __local10__2)
        __local10__2 = 0
}

complex_func(__local4__3, __local5__3) {
        __local7__3 = {__local6__3 |
                __local6__3 = __local4__3[_].value
                __local6__3.important
        }

        __local9__3 = {__local8__3 |
                __local8__3 = __local5__3[_].value
        }

        count(__local7__3, __local11__3)
        __local12__3 = __local7__3 & __local9__3
        count(__local12__3, __local13__3)
        __local11__3 = __local13__3
}

To expose this option via Compile API, we might have below options. But I am not very familiar with the code base and there could be a better way.

  1. To pass the disableInlining option as a query parameter i.e. http://opa/v1/compile?disableInlining=data.func similar to how explain parameter is passed on

    explainMode := getExplain(r.URL.Query()[types.ParamExplainV1], types.ExplainOffV1)

  2. The second option could be to enhance the partial evaluation request body with an additional field:

{
    "query": "data.pkg.allow == true",
    "options": {
        "disableInlining": ["data.func"]
    }
    "unknowns": [
        "input.resource.labels",
        "input.resource.id"
    ],
    "input": {
        "knownAttribute": [{
            "value": "a"
        }]
    }
}

type CompileRequestV1 struct {

@srenatus
Copy link
Contributor

👍 Adding options to CompileRequestV1 looks like a good idea. Thanks for digging into this!

srlk pushed a commit to srlk/opa that referenced this issue Feb 24, 2022
This change enables consumers of Compile API to specify packages those should not be inlined in partial evaluation response.

Fixes: open-policy-agent#4357
Signed-off-by: skosunda <skosunda@adobe.com>
srenatus pushed a commit that referenced this issue Feb 24, 2022
This change enables consumers of Compile API to specify packages
those should not be inlined in partial evaluation response.

Fixes: #4357

Signed-off-by: skosunda <skosunda@adobe.com>
damienjburks pushed a commit to damienjburks/opa that referenced this issue Mar 9, 2022
author Itay Shakury <itay@itaysk.com> 1646501263 +0200
committer Damien Burks <damien@damienjburks.com> 1646794189 -0600

docs: add missing title to annotations index (open-policy-agent#4406)

Signed-off-by: Itay Shakury <itay@itaysk.com>

Update philosophy.md (open-policy-agent#4412)

I am suggesting this change to fix grammatical errors which will increase readability for consumers of the OPA Philosophy document.

---

1. I noticed there is a missing word ("be") in the OPA Document Model section, just above the table that summarizes the different models for loading base documents into OPA. This change adds the omitted word.

2. I've also made a change to the first note at the bottom of the page ([1]). "However" is used as a conjunctive adverb, I've updated the sentence's punctuation to reflect this.

3. Lastly, I've made a punctuation change to note number three at the bottom of the page ([3])  to increase readability.

Signed-off-by: Arthur Jones <arthur.h.jones3@gmail.com>

Co-authored-by: Stephan Renatus <stephan.renatus@gmail.com>

docs: mention who created OPA and who owns it (open-policy-agent#4414)

These are two common questions that come up and we do not mention this
anywhere else in the docs.

Signed-off-by: Torin Sandall <torinsandall@gmail.com>

committing latest changes

Signed-off-by: Damien Burks <damien@damienjburks.com>

committing latest changes to unsued imports function

Signed-off-by: Damien Burks <damien@damienjburks.com>

removing print statements and adding more test cases

Signed-off-by: Damien Burks <damien@damienjburks.com>

server: exposing disableInlining option via Compile API (open-policy-agent#4378)

This change enables consumers of Compile API to specify packages
those should not be inlined in partial evaluation response.

Fixes: open-policy-agent#4357

Signed-off-by: skosunda <skosunda@adobe.com>

docs: Update Envoy primer to use variables instead of objects

Previously, the Envoy primer provided samples of policies for
status-code, body, and headers that explicitly returned an object
in individual rules instead of the more common and recommended
approach of using separate variables for each concern.

This change replaces those sample policies with ones that use
variables for each of allow, status_code, headers, body,
and it adds a section on the output expected by Envoy,
where we include the snippet combining those variables into
the JSON object expected by Envoy.

Signed-off-by: Tim Hinrichs <tim@styra.com>

make: Disable WASM by default on darwin/arm64 (open-policy-agent#4382)

Signed-off-by: Anders Eknert <anders@eknert.com>

docs: Update Istio tutorial to expose application to outside traffic (open-policy-agent#4384)

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>

fixing linting issues

Signed-off-by: Damien Burks <damien@damienjburks.com>

committing latest changes for PR

Signed-off-by: Damien Burks <damien@damienjburks.com>

adding info to strict.md

adding switch statement for 'in' operator

adding additional case to if statement

finalizing changes for broken test cases

Signed-off-by: Damien Burks <damien@damienjburks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants