3scale / APIcast

3scale API Gateway
Apache License 2.0
305 stars 171 forks source link

Apply policies conditionally #744

Closed mikz closed 6 years ago

mikz commented 6 years ago

Use Cases

Change Upstream API on matching HTTP Request Header. (https://github.com/3scale/apicast/pull/429) Two endpoints of an API need different policies (for example rate limits).

Concerns

If every policy is left to implement own matching then are going to be left with a lots of duplication. Also policy would need to be changed to implement matching.

Proposals

Conditional policy

As outlined in https://github.com/3scale/apicast/issues/173#issuecomment-393808026. This policy would be itself a policy chain and execute the chain only when the precondition matches.

That would allow us to implement all condition matching only in this policy and just nest desired actions within it.

Match phase

Match phase would be some shared code that the policies would get by default. The APIcast policy schema should define default properties for the policy schema to have extra configuration properties available.

Match phase would be executed once per policy to see if it applies to the request.

Conditions per "rule"

Some policies can have several "rules" like: add header "foo", add header "bar", rate limit /test to 100 rps, etc. Each of those rules might need different condition. Offering some standard definition of conditions in the APIcast policy schema would allow policies to define conditions for rules easily. This would have to be opt-in feature made by policy developers. For examples of such conditions see https://github.com/3scale/ostia/issues/17#issuecomment-396913897.

Comparison

We need to do deeper comparison of both approaches and possibly PoCs before committing to something.

Concerns

Defining conditions in JSON schema can be quite hard especially because missing oneOf support. It might be possible to use Liquid somehow, but not really see how. But that would offer the best flexibility at least in the beginning.

davidor commented 6 years ago

@mikz I implemented a first version of the 'Conditional policy' option. Check this branch: https://github.com/3scale/apicast/tree/conditional-policy

It's just a first version without much documentation and there are some things that can be improved, but it shows how it would work, and has tests showing that it behaves as expected.

davidor commented 6 years ago

I implemented another prototype just to have another option to evaluate. It's very similar to the other one but it uses the sandbox lib (https://github.com/apitools/sandbox.lua) to run lua code instead of liquid: https://github.com/3scale/apicast/tree/conditional-policy-lua-sandbox

I think that there are 2 different topics for discussion. One is how to evaluate the condition specified in the policy (liquid, lua code, etc.) and the other is where to apply the matching (new conditional policy, match phase, etc.)

davidor commented 6 years ago

I pushed a new branch that achieves the same using a different approach: https://github.com/3scale/apicast/tree/policy-conditions

In this branch, I modified the policy phases to check whether self.condition is true and run the phase only when that's the case. The good thing about this approach is that it does not need a new policy. However, I don't see a way to avoid adding the condition property on each config schema so the 3scale UI can show this field.

Notice that both in this branch and the other branches I pushed, the condition is evaluated in every phase. I think this can be useful because if we use the policies shared context as part of the env for evaluating the condition, it can change between phases. For example, some policy might add something to the context in rewrite(). This is the downside I see in the 'match phase' approach. Using that approach, the condition would be applied just once per request.

@mikz let me know what you think.

y-tabata commented 6 years ago

This is just an idea. Regarding the 'Conditional policy', I imagine something like a flow chart. So more intuitively, the following implementation can be thought of an option, I think.

"policy_chain": {
  "policies": [
    { "id": 1, "name": "apicast.policy.token_introspection" },
    { "id": 2, "name": "apicast.policy.rate_limit" },
    { "id": 3, "name": "apicast.policy.rate_limit" },
    { "id": 4, "name": "apicast.policy.apicast" }
  ],
  "chains": [
    { "from": 1, "to": 2, "condition": "a = b" },
    { "from": 1, "to": 3, "condition": "a ~= b" },
    { "from": 2, "to": 4 },
    { "from": 3, "to": 4 }
  ]
}

How do you think? Is it difficult to implement or troublesome to configure?

davidor commented 6 years ago

@y-tabata your solution allows modeling more complex flows. However, we should check for cycles, for example. I would personally choose the simple solution for now. However, if we discover a use case that needs complex flows we'll keep your solution in mind.

davidor commented 6 years ago

After some discussion with @mikz this is what we agreed on:

pimg commented 6 years ago

Does the conditional policy still covers the first use case mentioned in this issue: Change Upstream API on matching HTTP Request Header. (#429)

If so, is it possible to give a small example of the configuration?

davidor commented 6 years ago

@pimg I'll add some docs and possibly some tests that show that the conditional policy covers that use case.

davidor commented 6 years ago

@pimg I added a small change to be able to support that use case : https://github.com/3scale/apicast/pull/819

Tomorrow, I'll add a specific README for the conditional policy and I'll include this use case as an example of what can be achieved with it.

davidor commented 6 years ago

@pimg I opened a PR with more docs https://github.com/3scale/apicast/pull/820 Hope it helps.

pimg commented 6 years ago

Thanks! The docs make it a lot clearer.