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

Add docs for the conditional policy #820

Merged
merged 2 commits into from
Jul 24, 2018
Merged

Conversation

davidor
Copy link
Contributor

@davidor davidor commented Jul 24, 2018

No description provided.

@davidor davidor requested a review from a team as a code owner July 24, 2018 13:28
@mikz mikz requested a review from a team July 24, 2018 13:28
Copy link
Contributor

@jmprusi jmprusi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! I hope we can abstract from the user having to set the "types".

Stupid question: Does the "right" side has types? can we use a regex?

@davidor
Copy link
Contributor Author

davidor commented Jul 24, 2018

@jmprusi not for now. The only available operations are "==" and "!=". However, we could add operations like "matches" or "starts_with" that work with regexes, if there's a need for it. It should be pretty straightforward.

@unleashed
Copy link
Contributor

If we had nand (or nor, but harder) for combining conditions we could have a full algebra even though multiple evaluation of the same expressions could be an issue.

Extra operations would also be interesting, ie. >=.

I foresee we'll need more things, combining stuff with nands without associativity other than left-to-right or top-down can be hard, but this might be enough for now.

@unleashed
Copy link
Contributor

How about adding the possibility to choose a policy as the one that will be passed responsibility to say yay/nay:

"condition": {
  "external": {
    "name": "a_policy_that_talks_to_a_decision_engine",
    "parameters": "all"
  }
}

@davidor
Copy link
Contributor Author

davidor commented Jul 24, 2018

@jmprusi regarding the types, I added a clarification in the document.
Both the left and the right operands can be evaluated as liquid or as plain strings. The latter is the default.

@davidor
Copy link
Contributor Author

davidor commented Jul 24, 2018

I opened a new issue to keep track of the operations that we need to support according to what has been discussed here. Feel free to add more: #821

@davidor
Copy link
Contributor Author

davidor commented Jul 24, 2018

@unleashed That doesn't really fit the policy model we currently have.

Each policy needs to implement a function for each of the nginx phases it wants to run on. However, what you need is a method that evaluates a condition and returns a boolean.

What you could do is implement your own conditional policy. It would be the same as this one but you could choose to describe and evaluate the conditions as you wish.

@davidor davidor merged commit ab474ec into master Jul 24, 2018
@davidor davidor deleted the conditional-policy-docs branch July 24, 2018 16:55
@jmprusi
Copy link
Contributor

jmprusi commented Jul 25, 2018

yes at some moment, having a policy that can use for ex. Open Policy Agent would be really cool :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants