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

Creating rego interceptor implementation #1043

Closed
wants to merge 1 commit into from

Conversation

jmcshane
Copy link
Contributor

Changes

Closes #484

Demo implementation of rego interceptor using some dynamic parameter capabilities

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes tests (if functionality changed/added)
  • Includes docs (if user facing)
  • Commit messages follow commit message best practices
  • Release notes block has been filled in or deleted (only if no user facing changes)

See the contribution guide for more details.

Release Notes

Create rego interceptor

@tekton-robot tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Apr 11, 2021
@tekton-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign khrm
You can assign the PR to them by writing /assign @khrm in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 11, 2021
@jmcshane
Copy link
Contributor Author

Docs would be done if this implementation was determined appropriate to move forward

@tekton-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/interceptors/rego/rego.go Do not exist 88.4%

@tekton-robot
Copy link

@jmcshane: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-tekton-triggers-build-tests 0ed860a link /test pull-tekton-triggers-build-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@jmcshane
Copy link
Contributor Author

@afrittoli not sure what to do here because the license files for these two dependencies are named LICENSE-APACHE-2.0.txt. Could use some help from plumbing to resolve

@jmcshane
Copy link
Contributor Author

jmcshane commented Apr 12, 2021

This really needs some design consideration before moving the implementation in tree. If there was an interceptors repository, we wouldn't have to do all the interceptors in tree and we could document "custom" interceptors separately from the main triggers project.

Here's some things that I've mapped out:

  • We need some standard way to query the policy to find out if the interceptor should "continue" processing. You can see some ways to determine "truth" here: https://www.openpolicyagent.org/docs/latest/policy-testing/. The question is whether there should be some magic query hidden in the interceptor implementation, or just allow consumers to specify the module and query together.
  • The query statement for filter must return a single boolean value for us to be able to determine whether the interceptor should continue. This is the rs[0].Expressions[0].Value function on the result set
  • By itself, a filter query using rego could be a great feature (it is a policy language after all). Still, it makes sense that there would be a way to specify extensions. In my mind, this can be done using additional queries that are paired with extension "keys". You can see how this is implemented within the PR (see pkg/interceptors/rego). I am not a huge fan of the current design, but you can take a look at the rego go-api to see how these additional resources could be queried: https://pkg.go.dev/github.com/open-policy-agent/opa/rego#example-Rego.Eval-MultipleBindings

@jmcshane
Copy link
Contributor Author

jmcshane commented Apr 15, 2021

Due to license issues, this is going to be challenging for a while. I think we just assume this will be stuck in experimental for some time.

@jmcshane jmcshane closed this Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Rego interceptor
2 participants